Skip to content

bug: autocommit keeps committing during a rebalance - #370

Merged
ShogunPanda merged 4 commits into
platformatic:mainfrom
W1sdomPanda:bug/rebalance-autocommit
Aug 25, 2026
Merged

bug: autocommit keeps committing during a rebalance#370
ShogunPanda merged 4 commits into
platformatic:mainfrom
W1sdomPanda:bug/rebalance-autocommit

Conversation

@W1sdomPanda

Copy link
Copy Markdown
Contributor

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.

Signed-off-by: W1sdomPanda <vlad1slavskrygun@gmail.com>
Signed-off-by: W1sdomPanda <vlad1slavskrygun@gmail.com>
Signed-off-by: W1sdomPanda <vlad1slavskrygun@gmail.com>
@W1sdomPanda

Copy link
Copy Markdown
Contributor Author

@ShogunPanda Hello! I will be glad to receive feedback from you?

@ShogunPanda ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but it LGTM.

// 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(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clear the timer first?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget about test failures, there is some flakiness.
Can you please rebase and then I'll merge?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 :-)

@ShogunPanda
ShogunPanda merged commit 08a9aca into platformatic:main Aug 25, 2026
28 checks passed
ShogunPanda pushed a commit that referenced this pull request Aug 25, 2026
Signed-off-by: W1sdomPanda <vlad1slavskrygun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants