tulios / kafkajs

A modern Apache Kafka client for node.js
https://kafka.js.org
MIT License
3.71k stars 525 forks source link

Fix #1556: Reduce CPU usage on idle consumers (avoid infinite setTimeout loop with no inflight requests) #1667

Open sergeyevstifeev opened 7 months ago

sergeyevstifeev commented 7 months ago

I've noticed that consumers spent an inordinate amount of time while idle on topics with no new messages. After setting up a sample consumer on an idle topic I arrived at the same conclusion as some of the commenters on Issue 1556 - scheduleCheckPendingRequests is eating all of the CPU. Debugging it further, the issue is:

scheduleCheckPendingRequests() {
    let scheduleAt = this.throttledUntil - Date.now() // throttledUntil is -1, so scheduleAt is negative
    if (!this.throttleCheckTimeoutId) {
      if (this.pending.length > 0) { // this branch isn't run since there's no pending requests, so scheduleAt stays negative
        scheduleAt = scheduleAt > 0 ? scheduleAt : CHECK_PENDING_REQUESTS_INTERVAL
      }
      this.throttleCheckTimeoutId = setTimeout(() => {
        this.throttleCheckTimeoutId = null
        this.checkPendingRequests()
      }, scheduleAt) // we setTimeout with negative scheduleAt, resulting in an infinite checkPendingRequests->scheduleCheckPendingRequests setTimeout loop
    }
  }

The fix addresses the infinite loop and reduces CPU consumption substantially on idle consumers. 🌱 Additionally, there is no need in CHECK_PENDING_REQUESTS_INTERVAL of 10ms if there is not throttling, so this is also removed. I looked at 1532 and I think the fix here should still address the issue that was being addressed in that PR - if there are pending requests we always schedule next check (Math.max(timeUntilUnthrottled, 0)), so there should be no regression.

Fixes #1556

gpratte commented 1 month ago

@sergeyevstifeev I see this cannot be merged because "This branch is out-of-date with the base branch". Are you going to fix this?

sergeyevstifeev commented 1 month ago

@gpratte it doesn't seem like this repo is actively maintained anymore, so I don't really see a point.