Skip to content

Fix for issue #857 - #882

Merged
John Byrne (johnbyrnejb) merged 2 commits into
confluentinc:masterfrom
sangreal:fix/issue-857
Aug 7, 2025
Merged

John Byrne (johnbyrnejb) merged 2 commits into
confluentinc:masterfrom
sangreal:fix/issue-857

Conversation

@sangreal

@sangreal Martyn Ye (sangreal) commented Aug 2, 2025 •

Copy link
Copy Markdown
Contributor

Description...
This is potential fix for issue #857.
I suspect there are race condition issue during cleaning up stale containers during partition reassignment. After this point, we lose the chance to clean them up.
In this pr:
(1) I improve the removal logic of stale containers
(2) add removal logic during fetching available work containers

Checklist

  • Documentation (if applicable)
  • Changelog

…re are stale containers missed to be deleted 2. improve the delete logic of the stale containers
@sangreal

Copy link
Copy Markdown
Contributor Author

John Byrne (@johnbyrnejb) Edward Vaisman (@eddyv)
I am not sure if you are the right person to review and activate the CI for UT.
If you are still working on PC, please help review the trigger and CI

@johnbyrnejb
John Byrne (johnbyrnejb) merged commit a92d51c into confluentinc:master Aug 7, 2025
Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Aug 18, 2026
… correct cluster 3's record

RENAME: pausedForThrottling -> subscriptionsPausedForBackPressure

"paused" means three different things in this codebase and the wrong reading is easy
to reach - I reached it myself while reviewing this branch, and concluded the flag
was a redundant mirror of ConsumerManager.getPausedPartitionSize() when it is not.
The javadoc now separates them explicitly:

  1. this flag        - WE called consumer.pause(assignment) to apply back pressure
                        because the pipeline is full. Set only by doPause(), cleared
                        only by resumeIfPaused(), both on the poll thread.
  2. State.PAUSED     - the USER asked PC to stop. Stops the poll loop and work
                        registration; does NOT pause any partition. Unrelated.
  3. consumer.paused() - Kafka's own record, and the source of truth. This flag
                        mirrors it.

It also records why being a mirror is the hazard, which is the actual defect in this
cluster: Kafka clears pause state for every partition on an EAGER rebalance (the
assignment map is replaced - SubscriptionState.assignFromSubscribed) but KEEPS it for
partitions retained across a COOPERATIVE one. resumeIfPaused() is gated on the flag,
so when the two disagree nothing resumes and consumption stops silently - which is
confluentinc#857's own symptom.

Also updates the one caller, which used Lombok's generated isPausedForThrottling().
Worth noting for the next rename: grepping the field name does not find callers of a
Lombok getter, because the generated name capitalises differently.

CORRECTION to the cluster decomposition plan

An earlier draft called cluster 3's threading problem "pre-existing". It is not. On
master every mutation of numberRecordsOutForProcessing runs on the CONTROL thread, so
a plain int is correct there. The cluster adds a fifth site,
adjustOutForProcessingOnRevoke, called from onPartitionsRevoked - the POLL thread,
inside the rebalance callback. It makes a single-threaded counter cross-thread, on
the very counter whose drift is the silent-stall signature. Corrected in place rather
than silently, because "pre-existing" is the judgement that would have let it land.

The plan now also states what the drift measurement is FOR: not how to fix the two
defects, but whether the cluster should exist. Its target symptom predates
#100, #80 and confluentinc#882. If the drift is gone on current master,
all 42 lines go; if it remains, it gets rewritten with the control thread owning the
counter.
Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Aug 27, 2026
…eam's own fix shipped and did not hold

Both timelines paged in full. Nearly every `referenced` event is this fork's
commit-citation convention firing on itself; three items are real.

**confluentinc#882 merged upstream and did not fix the issue.** sangreal's
stale-container rework landed 2025-08-07, and this fork carries that logic -
`ProcessingShard.removeStaleWorkContainersFromShard` and
`PartitionStateManager.removeStaleContainers` are its shape. The reporter came back
on 2025-11-24, after upgrading past it: the problem persisted, recurring after two
months of uninterrupted traffic and triggered by a BROKER LEADER ELECTION rather
than a consumer-group rebalance. Worth stating plainly because the stale-container
theory is the one most likely to be re-proposed - it has already been tried,
shipped and outlived.

**confluentinc#875 (fork mirror #183) is in this family on a third party's
assumption.** Its reporter describes a silently SKIPPED offset - 4 never delivered
between 3 and 5 - lag growing until consumption stops, and a restart making the
missing message reappear, with no rebalance mentioned anywhere. Somebody else said
"potentially the same issue as confluentinc#857"; the reporter never did. A
skipped-then-recoverable offset is the shape of the offset-completion defects
(#344, #108), not of a lock, and it should be assessed on its own
rather than inherited into this bucket.

**The no-rebalance stall is rare, which makes the two sightings of it more
interesting rather than less.** Every other duplicate report ties the pause to a
rebalance, a redeploy or that one leader election. The exceptions are sangreal's
July log reading - "not paused, but somehow polling 0 records" - and dmironowicz on
this PR. Two independent observations is thin, and neither has ever been chased.
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