bug: autocommit keeps committing during a rebalance - #370
Conversation
|
@ShogunPanda Hello! I will be glad to receive feedback from you? |
| // Retry after a delay, not immediately: if many members get fenced around the same | ||
| // rebalance, resending ConsumerGroupHeartbeat with no delay at all can flood the | ||
| // coordinator faster than the group can converge. | ||
| this.#heartbeatInterval = setTimeout(() => { |
There was a problem hiding this comment.
Can you clear the timer first?
There was a problem hiding this comment.
It's already cleared at the top of this if (error) block via #cancelHeartbeat() before the fenced check runs, and nothing between there and here is async, so #heartbeatInterval is guaranteed null already.
@ShogunPanda what about failing tests?
I am not sure that it is related to my changes.
If all looks good can I ask you to merge and create release?
thanks, for feedback!
There was a problem hiding this comment.
Forget about test failures, there is some flakiness.
Can you please rebase and then I'll merge?
There was a problem hiding this comment.
@ShogunPanda done.
I have spend a few hours to understand this flakiness. Unfortunately I was not able.
Can you please create an issue with detailed description? Each time it is scary me, I will be glad to help :-)
Signed-off-by: W1sdomPanda <vlad1slavskrygun@gmail.com>
Fix: consumer group rebalance can stall under the right conditions
What's the problem?
A few places in the consumer internals retry or resend requests without checking whether the consumer is actually in a state where that makes sense. Normally this is harmless — a rebalance takes a moment and resolves on its own. But if the retry fires fast enough, and enough consumers in the group are doing it at the same time, the coordinator never gets a clean moment to finish the rebalance, since new join/heartbeat requests keep arriving during the exact window it needed to be quiet.
There are three spots with this same pattern. This PR fixes all three.
Fix 1: autocommit keeps committing during a rebalance
src/clients/consumer/messages-stream.ts
The autocommit timer fires on a fixed interval and only checks whether there's anything queued to commit — it never checks whether the consumer is currently mid-rebalance. So it keeps trying to commit anyway. Every failed attempt triggers an automatic rejoin internally, and depending on the commit interval and group size, that can add up to a steady stream of rejoin requests hitting the coordinator, which is exactly what prevents the rebalance from finishing.
The fix skips the commit attempt while the consumer isn't active. Nothing is lost — pending offsets just stay queued and commit on the next tick once things settle.
Fix 2: heartbeat retries instantly after being "fenced"
src/clients/consumer/consumer.ts
Same idea, different code path. When the broker tells a consumer its membership is stale (FENCED_MEMBER_EPOCH), the client immediately resends a heartbeat with no delay at all. If multiple consumers get fenced around the same rebalance, they can all hit the coordinator at once with no breathing room.
The branch just below this one already handles other heartbeat errors correctly, with a short delay before retrying. This fix applies that same delay here too, for consistency.
Fix 3: lag monitoring keeps polling even when the consumer is down
src/clients/consumer/consumer.ts
If startLagMonitoring is enabled, it keeps checking lag on a timer regardless of consumer state, even while disconnected or rejoining. That's unnecessary load on a cluster that might already be struggling, for a number that wouldn't be meaningful right now anyway.