Skip to content

Guard the deferred un-assign against a superseding rebalance - #2837

Open
delthas wants to merge 2 commits into
development/9.5from
improvement/BB-835/superseded-unassign-guard
Open

Guard the deferred un-assign against a superseding rebalance#2837
delthas wants to merge 2 commits into
development/9.5from
improvement/BB-835/superseded-unassign-guard

Conversation

@delthas

@delthas delthas commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

A consumer that defers answering a partition revoke can lose an assignment it was granted in the meantime, and stall forever.

The bug

When a revoke arrives while tasks are in flight, BackbeatConsumer does not call unassign() right away: it waits for the processing queue and the offset ledger to drain. While we owe librdkafka that answer, the rebalance protocol is effectively on hold — but a subscribed topic that looks changed in metadata is handled outside of it: librdkafka rejoins the group immediately, without waiting for our answer. The new generation can grant the partitions right back, and we apply them… then the drain completes, and the deferred unassign() throws that fresh assignment away.

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
    Note over RK: waiting for unassign()…
    B-->>RK: metadata: subscribed topic looks changed
    Note over RK: …but a topic change makes librdkafka<br/>rejoin now, revoke still unanswered
    RK->>B: JoinGroup + SyncGroup
    B-->>RK: new generation, same partitions
    RK->>BB: assign [p0..p4]
    BB->>RK: assign() — consuming again
    Note over BB: drain completes
    BB->>RK: unassign() — discards the fresh assignment
    Note over BB,B: broker: member still owns p0..p4 · local: nothing assigned<br/>membership unchanged → no rebalance will ever fix it
Loading

What makes a topic "look changed": librdkafka compares consecutive snapshots of its local metadata cache, restricted to the subscription, on topic name, partition count and (when client.rack is set) replica racks. Anything that makes two snapshots differ counts:

  • a partition count change (what the test below uses);
  • the subscribed topic being created or deleted;
  • replica rack changes, on consumers configured with client.rack (follower fetching);
  • a transiently wrong answer from a metadata-stale broker during a rolling restart: a topic-level UNKNOWN_TOPIC_OR_PART for an existing topic plants a short-lived negative cache entry, so the topic "vanishes" for one observation and "reappears" at the next — two spurious rebalances with no actual change to the topic;
  • TOPIC_AUTHORIZATION_FAILED flaps (ACL churn), same mechanism with a longer-lived negative entry;
  • on librdkafka 2.3.0, nothing at all was needed — see Versions below.

Leader changes, ISR changes, and refreshes that return the same data do not trigger it.

The end state is silent and permanent: the consumer is connected, a group member, owns every partition at the broker and passes every liveness check — and consumes nothing. Observed in a CTST run as a GC pod idling 2h35m with 188 of 199 messages stranded.

The fix

Only the newest rebalance may act:

  • on every rebalance event, disarm the previous revoke's pending drain triggers and watchdog, so a superseded un-assign can no longer fire. As a side effect this also stops a second revoke from leaving an orphaned watchdog behind, which would disconnect a healthy consumer ~5 minutes later;
  • the un-assign continuation can already be in flight when the newer event lands (through the asynchronous offset publish), so it also checks a rebalance counter before releasing the assignment.

Reproduction

The new functional test manufactures the interleaving deterministically against a real broker: one held task as the in-flight work, a partition-count bump as the topic change. It is red on development/9.5 and green with this fix — in two variants: the change observed by an application getMetadata() call, and by librdkafka's own periodic topic.metadata.refresh.interval.ms refresh, i.e. with no application metadata call involved at all.

Versions

On librdkafka 2.3.0 the trigger fired constantly: every full metadata refresh wiped the metadata cache, so even a completely static topic read as "vanished" then "reappeared" on each refresh (fixed upstream in 2.4.0 by confluentinc/librdkafka#4677). The rejoin-while-a-revoke-is-unanswered behaviour itself is unchanged through 2.12.0, where the test reproduces the stall with a genuine topic change.

Replaces #2818.

Issue: BB-835

@bert-e

bert-e commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Hello delthas,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval
/bypass_build_status Bypass the build and test status
/bypass_commit_size Bypass the check on the size of the changeset TBA
/bypass_incompatible_branch Bypass the check on the source branch prefix
/bypass_jira_check Bypass the Jira issue check
/bypass_peer_approval Bypass the pull request peers' approval
/bypass_leader_approval Bypass the pull request leaders' approval
/bypass_source_branch_lineage Bypass the cross-branch contamination check
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request.
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e

bert-e commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.51%. Comparing base (56a3c42) to head (02a7eb7).

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/BackbeatConsumer.js 94.33% <100.00%> (+0.58%) ⬆️

... and 2 files with indirect coverage changes

Components Coverage Δ
Bucket Notification 80.27% <ø> (ø)
Core Library 81.31% <100.00%> (-0.52%) ⬇️
Ingestion 70.09% <ø> (ø)
Lifecycle 80.46% <ø> (ø)
Oplog Populator 85.83% <ø> (ø)
Replication 62.01% <ø> (ø)
Bucket Scanner 85.76% <ø> (ø)
@@                 Coverage Diff                 @@
##           development/9.5    #2837      +/-   ##
===================================================
- Coverage            75.72%   75.51%   -0.21%     
===================================================
  Files                  200      200              
  Lines                13926    13934       +8     
===================================================
- Hits                 10545    10522      -23     
- Misses                3371     3402      +31     
  Partials                10       10              
Flag Coverage Δ
api:retry 9.10% <0.00%> (-0.01%) ⬇️
api:routes 8.86% <0.00%> (-0.01%) ⬇️
bucket-scanner 85.76% <ø> (ø)
ft_test:queuepopulator 9.16% <12.50%> (-1.87%) ⬇️
ingestion 12.25% <0.00%> (-0.01%) ⬇️
lib 8.87% <75.00%> (+0.06%) ⬆️
lifecycle 19.32% <75.00%> (+0.01%) ⬆️
notification 1.02% <0.00%> (-0.01%) ⬇️
oplogPopulator 0.13% <0.00%> (-0.01%) ⬇️
replication 18.86% <75.00%> (+0.03%) ⬆️
unit 55.17% <100.00%> (+0.23%) ⬆️

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.

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 even while that revoke is still unanswered, so
a fresh assignment can be granted and applied before the drain
completes -- and the deferred un-assign then discarded it, leaving a
live group member owning partitions at the broker with no local
assignment, consuming nothing, with nothing left to trigger a rebalance.

Disarm the pending drain triggers and watchdog on every rebalance event,
so a superseded un-assign can no longer fire; the watchdog disarm also
stops a second revoke from leaving an orphaned timer that would later
disconnect a healthy consumer. The async offset publish can still be in
flight when a newer rebalance lands, so the un-assign continuation also
checks a rebalance counter before releasing the assignment.

Issue: BB-835
Manufactures the interleaving deterministically against a real broker:
a revoke deferred behind an in-flight task, then a partition count
change observed by a group-updating metadata request, which makes
librdkafka rejoin and apply a fresh assignment while the revoke is still
unanswered. The deferred un-assign must then leave that assignment
alone.

Two variants differ only in which request observes the change: an
application getMetadata() call, and librdkafka's own periodic
topic.metadata.refresh.interval.ms refresh, which fires with no
application metadata call at all.

Both are red without the rebalance guard (the consumer ends with an
empty local assignment, ready but consuming nothing) and green with it.

Issue: BB-835
@delthas
delthas force-pushed the improvement/BB-835/superseded-unassign-guard branch from b76f6d0 to 02a7eb7 Compare September 2, 2026 09:27
@delthas
delthas marked this pull request as ready for review September 2, 2026 09:30

@francoisferrand francoisferrand 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.

I think we should point clearly that this actually breaks our rebalance handling: we would still be processing previous tasks, yet not be able to commit them...

Would be safer to fit these "unexpected" (and buggy) events in the same automaton, e.g. sending the same events eventually/etc ; and possibly fail like when our own timeout is reached...

Comment thread lib/BackbeatConsumer.js
// pending: disarm its drain triggers and watchdog, and stamp
// this event so a continuation already in flight (the async
// offset publish below) can tell it has been superseded.
const rebalanceId = ++this._rebalanceId;

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.

what events can we get, is this condition not too strong?

  • if we receive UNASSIGN while already unassigned, we can just ignore it (and continue). It's weird, but best to finish what was already going on
  • if we receive ASSIGN, then we should abort indeed and print a message ; however if wonder if that aborting should not immediately call "doUnassign()" : so we resume partitions, unassign the customer (but not the newly assigned partitions...) and emit signal...

Comment thread lib/BackbeatConsumer.js
});
return;
}
this._resumePausedPartitions();

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.

should we not resume paused partitions as well?
this may happen when breakbeat engages - if we don't do it everything is stuck...

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.

3 participants