Repository navigation
Fixes #547 - Cleanup WorkManager#numberRecordsOutForProcessing when w… - #549
Merged
Roman Kolesnev (rkolesnev) merged 1 commit intoFeb 21, 2023
Conversation
|
ravikalasapur (RaviKalasapur)
force-pushed
the
bugs/bad-consumer-state
branch
from
February 16, 2023 20:18
35bee28 to
894c5d5
Compare
Roman Kolesnev (rkolesnev)
self-requested a review
February 20, 2023 11:27
Roman Kolesnev (rkolesnev)
suggested changes
Feb 20, 2023
…ssing when work is stale
ravikalasapur (RaviKalasapur)
force-pushed
the
bugs/bad-consumer-state
branch
from
February 21, 2023 04:25
894c5d5 to
bafb2bd
Compare
ravikalasapur (RaviKalasapur)
requested a review
from Roman Kolesnev (rkolesnev)
February 21, 2023 04:27
Antony Stubbs (astubbs)
added a commit
to astubbs/parallel-consumer
that referenced
this pull request
Aug 18, 2026
…he probe that disproved it
Removes WorkManager.adjustOutForProcessingOnRevoke and
ShardManager.countInflightForPartitions - 42 lines - because the drift they were
written for does not exist, and they cause drift themselves.
MEASURED, not argued. A deterministic probe runs the real engine on a MockConsumer
with the user function gated so records are genuinely out with the pool, drives a
revoke through the engine's own listener, releases, waits for quiescence, then
compares the counter against ground truth:
master, 5 revoke+reassign cycles + 1 revoke-without-reassign:
counter at quiesce 0 every time, truth 0. Poller resumed, 20/20 fresh records
processed after the last revoke.
with this code present:
-8, -16, -20, -20, -20 against truth 0. By cycles 3-4 the counter reads 0
while 4 records are genuinely in flight.
So it corrupts the counter DOWNWARD - the over-fetch direction, isSufficientlyLoaded
false while work is outstanding - and the clamp only runs at the next revoke, so it
never repairs the interval that matters. It also made a single-threaded counter
cross-thread: every other mutation site runs on the control thread, while this one is
called from onPartitionsRevoked on the poll thread, on a plain int.
WHY THE DRIFT IT TARGETED IS NOT REAL. Every worker-side exit returns the container to
the mailbox and every mailbox branch decrements - including the stale branch, which
has decremented since upstream confluentinc#549 (fdd245e, 2023-02-21), three years
before this code was written. So the accounting was already balanced. The most
consistent reading of the original observation is that the counter was TRUTHFULLY
high: in-flight work never returned because user functions were wedged behind the
commit-path deadlock. #100 removed that. The "counter-drift signature" comment
in isSufficientlyLoaded therefore describes a symptom whose only known cause was the
deadlock - which is why one April session saw both.
Lands the probe as OutForProcessingCounterDriftProbeTest. It is the only thing in the
repo that can tell the counter from reality, it failed against this code and passes
without it, and it would have prevented the change being written. Two residual leak
paths it found are unrelated to rebalance and left alone: a non-Exception Throwable
(e.g. StackOverflowError) escaping runUserFunction's catch, and a mixed stale/active
batch whose callback throws.
Unit suite 383, 0 failures. Copyright gate clean at 250 files.
Antony Stubbs (astubbs)
added a commit
to astubbs/parallel-consumer
that referenced
this pull request
Aug 18, 2026
…fluentinc#857 work The knowledge was spread across a dated plan, four inflight notes, an April investigation log and this file - and most of those are designed to be deleted. The plan goes stale when the work lands, the sighting notes are deleted per sighting as their seeds are explained, and docs/BUG_857_INVESTIGATION.md is superseded. Whatever was only in them would have gone with them, which is the wrong outcome for a defect class that has now produced two deadlocks in the same thread pair. This file now carries what should outlive the PR: - The four independent changes bundled in the April commit, and what happened to each: the deadlock fix kept and proven, the confinement guard kept after fixing its predicate, the counter adjustment deleted, the pause mirror deleted and replaced. - Both A/B soaks - 20/20 then 60/60 failures on master against 0/60 on the fix - and why the skip-log count is the number that makes them mean anything: a clean fixed arm with a zero skip-count is indistinguishable from a probe that never opened the window, which is how the fix looked unproven for four months. - That the shipped reproducer passes 5/5 on the DEFECT arm and fails 5/5 on the fixed one, so it reports the fix as a regression. - The counter-drift disproof, including that the balancing decrement has existed since upstream confluentinc#549 (2023) - so the drift was never an accounting bug, and the counter was truthfully high because work never returned from behind the deadlock. - The close-path finding: a WARN a user cannot act on is a bug in disguise, a note to the developer at the wrong level, or a real consequence stated as internals - and logging a throwable is running its author's code. - All seven sightings in one table WITH THEIR COMMIT MODE, which the ledger never recorded and without which none of them can be attributed. Each row carries its signature and its replayable seed, plus the replay command; the passing control-arm seeds are listed separately, because one was already circulated as a failure by a handoff written from a truncated log. - That retrieving the evidence is itself a trap: three times in one day a console log destroyed it, once producing a false negative that parsed as a clean run while the artifact carried the violation. Uses the in-file issue-refs opt-out for the sightings table, whose #N row labels are not issue references. The gate earned its keep on the way: among ten false positives it caught one genuinely unqualified fork ref.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ork is stale
Description...
Checklist