Skip to content

Abort the rebalance when a grant supersedes a parked revoke - #2839

Draft
delthas wants to merge 1 commit into
improvement/BB-833/bound-consumer-closefrom
improvement/BB-835/abort-on-superseded-rebalance
Draft

Abort the rebalance when a grant supersedes a parked revoke#2839
delthas wants to merge 1 commit into
improvement/BB-833/bound-consumer-closefrom
improvement/BB-835/abort-on-superseded-rebalance

Conversation

@delthas

@delthas delthas commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Alternative to #2837 for BB-835, taking the approach francoisferrand asked for in review: fit the buggy event into the existing shutdown automaton and fail like the drain timeout, instead of carrying on with an assignment whose generation has moved on.

Stacked on #2838, which is what makes this viable: leaving the group is now prompt and bounded, so aborting no longer risks the 45s takeover stall that BB-833 describes.

The bug

When a revoke arrives with tasks in flight, the un-assign is deferred until the processing queue and offset ledger drain. librdkafka answers a subscribed-topic metadata change with an immediate rejoin without waiting for that un-assign, so a grant can land mid-drain for a newer generation. The deferred un-assign then discarded that grant, leaving a live group member that owns every partition at the broker with no local assignment, healthy on every liveness signal, consuming nothing, with nothing left to trigger a rebalance.

sequenceDiagram
    participant BB as BackbeatConsumer
    participant RK as librdkafka
    participant B as broker

    RK->>BB: revoke [p0..p4]
    Note over BB: tasks in flight,<br/>defer unassign() until drained
    B-->>RK: metadata: subscribed topic looks changed
    Note over RK: rejoins now, revoke still unanswered
    RK->>B: JoinGroup + SyncGroup
    B-->>RK: new generation
    RK->>BB: assign [p0..p4]
    Note over BB: abort: accept the grant, stop consuming
    Note over BB: pending drain commits, then un-assigns
    BB->>RK: close, LeaveGroup
    Note over BB,B: partitions taken over at the committed offset,<br/>this consumer is restarted by the liveness probe
Loading

Why abort rather than keep the assignment

Continuing is not just a matter of skipping the stale un-assign. The accepted grant resolves fetch positions from the last commit, so the uncommitted tail comes back around while the previous generation's copies of those same entries are still being processed. For consumers that order by message key the two copies serialize, but GarbageCollector and LifecycleBucketProcessor both set orderByFunc: null, so there the same entry can be processed twice concurrently. Aborting avoids that state entirely rather than relying on every task tolerating it.

What it does

  • Detect: a grant arriving while _waitingDrain is set means librdkafka rejoined without our answer. One condition, no generation counter needed, and it covers the asynchronous offset-publish window because the flag stays set until the un-assign runs.
  • Accept the grant, do not decline it: the draining entries can only store their offsets while the partitions are held. This is what makes the abort lossless.
  • Stop consuming, and drop entries delivered by a consume request that was already outstanding, since those are the draining entries coming back around.
  • Let the pending drain run unchanged: it commits and un-assigns exactly as today. A stale un-assign is harmless once we are leaving, so the drain path needs no modification.
  • Leave: _close() from Leave the consumer group promptly on shutdown #2838, then report unhealthy so the liveness probe restarts us, with the same CRASH_ON_REBALANCE_TIMEOUT exit S3C uses for the drain timeout.

Trade-off against #2837

#2837 guard this PR
duplicate work uncommitted tail replayed none, the drain commits first
two-generation overlap possible, concurrent for unordered consumers none
availability per occurrence none one restart
extra rebalances none 2, our leave and rejoin
behavior under repeated triggers extra churn repeated restarts, bounded by the kubelet backoff

This is strictly for 2.12 and later. On librdkafka 2.3.0 the trigger fires about once a second under metadata thrash, which would be a restart loop rather than a recovery.

Tests

  • A functional test manufacturing the interleaving against a real broker with partition-count bumps: it asserts the trigger fired, the drain completed, the consumer left and reports unhealthy, the held entry was committed, and it was processed exactly once. It fails on the base branch, where the entry is processed twice.
  • Unit tests for accepting the grant, stopping consumption, dropping a late delivery, leaving once the drain committed, and the two cases that must not abort.

Issue: BB-835

https://claude.ai/code/session_01JQoM2qBo8JABC43pXSUiF8

On ERR__REVOKE_PARTITIONS the un-assign is deferred until in-flight work
drains. librdkafka however answers a subscribed-topic metadata change
with an immediate rejoin, without waiting for that un-assign, so a grant
can arrive for a generation that has already moved past the work still
draining. Carrying on would mean consuming those partitions again from
their last commit while still finishing the previous generation's work:
the same entries processed twice, concurrently for consumers that do not
order by key. The deferred un-assign then discarded the granted
assignment outright, leaving a live group member owning partitions at
the broker with no local assignment and nothing left to trigger a
rebalance.

Treat it like the drain timeout instead. Accept the grant, since the
draining entries can only store their offsets while the partitions are
held, stop consuming, let the pending drain commit and un-assign as it
always does, then leave the group so the partitions are taken over and
the liveness probe restarts this consumer.

Entries delivered by a consume request that was already outstanding are
dropped: the accepted grant reset the fetch position to the last commit,
so those are the draining entries coming back around.

Issue: BB-835
Claude-Session: https://claude.ai/code/session_01JQoM2qBo8JABC43pXSUiF8
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.58%. Comparing base (2dfe4d5) to head (765dccc).

Files with missing lines Patch % Lines
lib/BackbeatConsumer.js 85.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/constants.js 100.00% <ø> (ø)
lib/BackbeatConsumer.js 94.05% <85.00%> (-0.40%) ⬇️
Components Coverage Δ
Bucket Notification 80.27% <ø> (ø)
Core Library 81.43% <85.00%> (+<0.01%) ⬆️
Ingestion 70.09% <ø> (ø)
Lifecycle 80.46% <ø> (ø)
Oplog Populator 85.83% <ø> (ø)
Replication 62.01% <ø> (ø)
Bucket Scanner 85.76% <ø> (ø)
@@                             Coverage Diff                             @@
##           improvement/BB-833/bound-consumer-close    #2839      +/-   ##
===========================================================================
+ Coverage                                    75.57%   75.58%   +0.01%     
===========================================================================
  Files                                          200      200              
  Lines                                        13979    13998      +19     
===========================================================================
+ Hits                                         10565    10581      +16     
- Misses                                        3404     3407       +3     
  Partials                                        10       10              
Flag Coverage Δ
api:retry 9.05% <0.00%> (-0.02%) ⬇️
api:routes 8.82% <0.00%> (-0.02%) ⬇️
bucket-scanner 85.76% <ø> (ø)
ft_test:queuepopulator 9.18% <15.00%> (-0.02%) ⬇️
ingestion 12.19% <0.00%> (-0.02%) ⬇️
lib 9.24% <85.00%> (+0.10%) ⬆️
lifecycle 19.45% <20.00%> (-0.01%) ⬇️
notification 1.01% <0.00%> (-0.01%) ⬇️
oplogPopulator 0.13% <0.00%> (-0.01%) ⬇️
replication 19.06% <20.00%> (+0.06%) ⬆️
unit 55.53% <85.00%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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