Skip to content

fix(nodejs): answer queued rebalance events when the consumer stops - #90348

Draft
jose-sequeira wants to merge 2 commits into
jose-sequeira/e2e-global-setupfrom
jose-sequeira/consumer-v2-shutdown-rebalance
Draft

fix(nodejs): answer queued rebalance events when the consumer stops#90348
jose-sequeira wants to merge 2 commits into
jose-sequeira/e2e-global-setupfrom
jose-sequeira/consumer-v2-shutdown-rebalance

Conversation

@jose-sequeira

@jose-sequeira jose-sequeira commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

A KafkaConsumerV2 that stops while a rebalance is in progress can hang forever in disconnect(). In production that is a pod that never finishes shutting down after SIGTERM. In CI it surfaced as the rebalance integration suite wedging past its test timeout, with the worker process surviving --forceExit.

  • rebalanceCallback only queues ASSIGN/REVOKE events; the run loop answers them with incrementalAssign/incrementalUnassign.
  • stopConsuming() flips running, and the loop exits with whatever is still in rebalanceQueue unanswered.
  • librdkafka blocks disconnect() until the application responds to the pending cooperative rebalance, so it never returns.
  • The existing special case in rebalanceCallback only covers events that arrive after the flip, not ones queued just before it.

Found while making the rebalance suite concurrent, which turns that window from rare into 1 run in 5.

Changes

  • Shutdown no longer hangs mid-rebalance: after the shutdown drain, the loop answers every queued event (unassign revokes, accept assigns; nothing polls any more, and disconnect()'s final revoke releases them).
  • KafkaConsumerV2Config gains optional maxBackgroundTasks and rebalanceTimeoutMs, defaulting to the defaultConfig values as before, so a consumer's budgets can be set per instance instead of by mutating the global.
  • The rebalance integration suite runs its nine tests concurrently: 91 s → 27.5 s locally. Each test already owned its topic and group; the three that mutated defaultConfig now pass budgets through the new fields (mechanical, but prettier re-wraps every it.concurrent( call, hence the large diff).
  • The suite tracks every consumer it creates and disconnects leftovers in afterAll, so a test that dies before its finally cannot keep the worker alive.
  • One assertion changed to the documented contract: the fence is per partition, so a laggard batch may still store offsets for a partition the consumer kept. The old assertion (retained partition unchanged) only passed by reading before the 500 ms auto-commit tick; with a 1.5 s pause it failed in isolation too.

Note

Stacked on #90032 so that PR stays test-only. This layer is the one that touches consumer-v2.ts.

How did you test this code?

  • Reproduced the hang with phase markers in the failing test: the body completed at +15.4 s, stopConsuming() returned, and rdKafkaConsumer.disconnect() never called back, with the consumer holding one partition and its second revoke hook not yet run.
  • With the fix, 15 consecutive runs of the concurrent suite passed, 27.5 s each. Before the fix, at the same concurrency, 2 of 11 runs wedged.
  • consumer-v2.test.ts and consumer-v2.integration.test.ts pass locally (35 tests).
  • The retained-partition assertion: adding a 1.5 s pause before it on the untouched test made it fail in isolation, confirming the old expectation was a read-before-commit race.
  • Not run: the full Node.js suite locally; hogli review (the Greptile CLI does not install in this environment).

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude Code (Claude Fable 5) under @jose-sequeira's direction, continuing from #90032. Skills invoked: /writing-tests, /stacking-prs, /writing-pr-descriptions. Sequence: the suite was made concurrent, one test wedged intermittently, instrumentation showed the hang was in teardown rather than in any wait, and the shutdown path was fixed at the source rather than by making the test wait for the rebalance to settle. The per-consumer config fields were added at the reviewer's request instead of keeping the global mutation.

@jose-sequeira jose-sequeira changed the title jose sequeira/consumer v2 shutdown rebalance fix(nodejs): answer queued rebalance events when the consumer stops Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 CI report

Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

@trunk-io

trunk-io Bot commented Aug 27, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

stopConsuming flips running and the loop exits with any rebalance event
still in rebalanceQueue unanswered. librdkafka blocks disconnect() until
the application responds to a pending cooperative rebalance, so a stop
that lands mid-rebalance hangs the process. Answer whatever is left in
the queue after the shutdown drain: unassign revokes, accept assigns (no
loop polls any more, and disconnect's final revoke releases them).
Each test already owns its topic and consumer group. What kept them
sequential was three tests mutating defaultConfig for the drain budget
and the background task cap, which KafkaConsumerV2 read at construction.
Add both as optional per-consumer config so the tests pass them directly,
run the nine tests with it.concurrent, and track every consumer so
afterAll closes what a timed-out test leaves behind.

The fenced-batch test asserted the retained partition's committed offset
stayed at the pre-gate mark. The fence is per partition, so the laggard's
store for a partition the consumer still holds is valid and lands on the
next auto-commit tick; the old assertion passed only by reading before
that tick. Assert the documented contract instead.
@jose-sequeira
jose-sequeira force-pushed the jose-sequeira/consumer-v2-shutdown-rebalance branch from 18c9e30 to 05b2bdb Compare August 28, 2026 08:25
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.

1 participant