Leave the consumer group promptly on shutdown - #2838
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 2 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.5 #2838 +/- ##
===================================================
- Coverage 75.72% 75.57% -0.15%
===================================================
Files 200 200
Lines 13926 13979 +53
===================================================
+ Hits 10545 10565 +20
- Misses 3371 3404 +33
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
36e0cf0 to
7fa9cb0
Compare
6ef41ff to
ab03ef9
Compare
Rewrite close() as an async _close() with the callback-based close(cb) kept as a thin shim for existing callers. The async.waterfall becomes sequential awaits and the event/interval waits become small Promise helpers; behavior is unchanged. Done as a dedicated commit ahead of the shutdown fix, per the repo's async/await migration guidance. Issue: BB-833
close() unsubscribed then waited for the rebalance callback to un-assign, but librdkafka delivers no such callback when the consumer holds no assignment, and postpones the unsubscribe outright while a rebalance is in progress. The wait was unbounded, so close() never returned, the SIGTERM handler never reached process.exit, and the pod was SIGKILLed with the member still registered -- the group then held zero partitions until session.timeout.ms (45s) evicted it. rd_kafka_consumer_close() (reached via disconnect()) performs the whole protocol exit from any state, including a postponed unsubscribe, and sends the LeaveGroup on its own; its one requirement is that any rebalance callback it delivers is answered. So instead of gating disconnect() on an un-assign that may never come: - wait for the drain only when a revoke is actually coming (one already draining, or triggered by the unsubscribe because we still hold partitions), bounded so a postponed unsubscribe gives up after a short grace and lets disconnect() leave; - answer every rebalance callback raised once shutting down (decline a grant, un-assign a revoke) so disconnect() cannot wedge on an unanswered one; - bound the disconnect wait itself; - stop fetching once the shutdown has started, so the pipeline the drain is waiting on cannot refill; - bound the wait for an in-flight offset publish, rather than rescheduling close() indefinitely. Issue: BB-833
ab03ef9 to
2dfe4d5
Compare
Problem
close()unsubscribed then waited for the rebalance callback to un-assign before disconnecting:librdkafka delivers no such callback when the consumer holds no assignment, and postpones the unsubscribe outright while a rebalance is in progress. So the wait never completed:
close()never returned, the SIGTERM handler never reachedprocess.exit, and the pod was SIGKILLed with the member still registered at the broker. The group then held zero partitions untilsession.timeout.ms(45s) evicted it — the takeover stall this ticket is about.Mechanism
rd_kafka_consumer_close()(reached viadisconnect()) already performs the whole protocol exit from any join state. Itsterminate0calls the internal unsubscribe withleave=true, which bypasses the postponement that blocks the application-levelunsubscribe(), and it sends theLeaveGroupon its own. Its one requirement is that any rebalance callback it delivers on the close queue is answered — withunassign()(assign()is refused once the close has started).sequenceDiagram participant App as close() participant RK as node-rdkafka participant CG as librdkafka cgrp App->>RK: unsubscribe() Note over CG: postponed while a rebalance is in progress App->>RK: disconnect() RK->>CG: consumer_close() then terminate0 Note over CG: internal unsubscribe(leave=true)<br/>bypasses the postponement CG-->>App: REVOKE (if partitions still held) App->>RK: unassign() — answered at once CG->>CG: LeaveGroup sent RK-->>App: 'disconnected'This corrects the
F_LEAVE_ON_UNASSIGN_DONEframing in #2819: the leave comes fromterminate0, and the explicit pre-disconnect unsubscribe/unassign are only there to drain in-flight work first, not to arm the leave.Approach
Rather than gate
disconnect()on an un-assign that may never arrive:unassignatmaxPollIntervalMs - 1000); a revoke that never starts (an unsubscribe postponed by an in-flight rebalance) is given only a short grace, then we letdisconnect()perform the leave. Waiting the full budget there would run past the pod's grace period.disconnect()cannot wedge on an unanswered one.close()indefinitely if the ZooKeeper publish never calls back.The first commit migrates
close()to async/await (a behavior-preserving refactor,close(cb)kept as a thin shim), per the repo's async/await migration guidance; the second is the fix above.The un-assign wait uses a guarded
_getAssignments()(mirroring_getSubscription()), sinceassignments()throwsERR__STATEon a closing client (BB-845).close()flowflowchart TD A["close(cb)"] --> B["_closing = true<br/>stop fetch loop, cron, circuit breaker"] B --> C{"consumer connected?"} C -->|no| Z["disconnect step"] C -->|yes| D["best-effort unsubscribe()"] D --> E{"waiting on a drain<br/>or partitions held?"} E -->|no| Z E -->|yes| W["wait for 'unassign' (bounded)"] W -->|"'unassign' emitted<br/>(drain done or watchdog)"| Z W -->|"grace elapsed,<br/>revoke in progress: keep waiting"| W W -->|"grace elapsed,<br/>no revoke started"| G["give up: unassign()"] G --> Z Z --> H["_disconnecting = true<br/>disconnect(), bounded 5s"] H --> I["cb()"]Rebalance callbacks during shutdown
Only the disconnect phase short-circuits the drain; the same single drain path handles a revoke while running and while shutting down.
stateDiagram-v2 direction LR [*] --> Running Running --> ShuttingDown : close() called ShuttingDown --> Disconnecting : drain done or bound hit Disconnecting --> [*] : disconnected note right of Running ASSIGN -> assign(partitions) REVOKE -> drain, then unassign() end note note right of ShuttingDown fetch loop stopped ASSIGN -> decline: unassign() REVOKE -> drain, then unassign() end note note right of Disconnecting ASSIGN -> decline: unassign() REVOKE -> unassign() now, no drain end noteRelationship to the earlier stack
Supersedes the substance of #2819 / #2834 / #2835, rebuilt as one change on
development/9.5(not stacked on #2818). It keeps a single drain path — the existing revoke handler — rather than adding a second one for shutdown. The double-close()coalescing (#2836) is intentionally left out.Known residual
When the leaving member has its own
JoinGroupin flight (it was revoked and is rejoining), its explicitLeaveGroupis transport-blocked behind that request.close()still returns within its bound and the remaining members take over via a normal rebalance; only that member's fast explicit leave is delayed. This is a librdkafka transport-ordering limitation, not specific to this change; loweringsession.timeout.ms(BB-844) bounds its cost.Tests
close()returning within its bound when called during a rebalance.Issue: BB-833