fix(core): draining PC stops polling - 10kHz busy-spin + zombie partition hold (#857-family silent stall) - #80
Merged
Conversation
…same-repo only) Optional, non-gating fast feedback that runs unit, integration and performance as parallel matrix jobs on a self-hosted many-core runner (label `highcpu`), guarded to same-repo PRs so fork code never executes on it. The GitHub-hosted gate in maven.yml stays the required check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…rkflow # Conflicts: # .github/actionlint.yaml
…-only The high-CPU self-hosted matrix now runs unit + integration + performance + mutation as parallel jobs (a strict superset of the mac runner, which is offline indefinitely). The mac workflow is switched to workflow_dispatch-only so it stops queuing dead jobs on every PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
failingHttpCall drove an HTTP call at a dotless bogus hostname ("xxxxxxxxx") and asserted the failure
was a DNS *resolution* error ("failed resolve"). That only holds where the name fails to resolve - on any
network with a local resolver + search domain (e.g. a Pi-hole LAN) the name resolves and the failure mode
becomes "connection refused", so the test fails there while passing on GitHub's public DNS.
Point the bad request at a closed local port (127.0.0.1:1) instead: an immediate, deterministic
"connection refused" on every environment, no DNS lookup at all (and faster than one). Keep the real
failed==true check; assert the deterministic "connection refused" message. Found bringing up the
self-hosted high-CPU runner.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…with failingHttpCall) The failingHttpCall fix (127.0.0.1:1 -> connection refused) also flipped this sibling test through the shared getBadRequest() helper, so its assertion is updated to match. Caught by ce-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Load was ~18/32 with 34GB free - the box is I/O-bound (cold cache), not CPU-bound - so use all the cores. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…ateCommittedOffsetIT Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…epo guard (ce-review P2) The if: guard lives in the PR-head file, so the real RCE backstop is the repo Actions setting "require approval for all outside collaborators". Documented in the workflow header + referenced SELF_HOSTED_RUNNER.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Unit forkCount 1C->2C, Integration 8->16 (probe the oversubscription ceiling). Mutation PIT threads 2->16 (CPU-bound, stays in the matrix). Performance split into its own job that needs the matrix, so the throughput benchmark measures on a quiet box (solo 1m54 vs 7m18 when it shares the box). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…formance Doubling was flat (unit 2C=1C) to worse (integration 16 slower than 8), so back to the knee. Performance returns to the concurrent matrix - it is a correctness gate here; noisy timing is fine and isolation wrecked total wall-clock (it queued behind a 17-min mutation). Mutation keeps threads=16. Accurate benchmarking -> separate on-demand run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Unit/Integration now also enable JUnit method-parallelism (-Dparallel-tests=true) on top of forking, forks reduced so forks x threads ~= cores - tests whether thread-parallelism is still flaky after recent fixes (revert to fork-only if red). ci-mutation-test.sh: on a PR (GITHUB_BASE_REF) mutate ONLY changed core classes, skip if none - the full internal.* sweep has never completed. Full sweep still runs for push/nightly (no base ref). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…eads finding fork×threads (parallel-tests=true) was safe (threading no longer races - unit green; integration red was the known flaky PartitionStateCommittedOffsetIT) but no faster than fork-only, so back to 1C/8. PIT: grep no-match under set -euo pipefail made the changed-class skip exit non-zero (0m19 red); wrap grep with || true so it exits 0. Recorded the thread-parallelism-vs-forking finding in inflight next to the shelved Step-2 question. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…bserve a green board TEMPORARY - not a fix. Disables the known-flaky PartitionStateCommittedOffsetIT.committedOffsetRemoved (awaitility timeout, in inflight, fails on GitHub-hosted too) purely to see a fully-green highcpu run. Revert and harden the test properly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Now that ci-mutation-test.sh scopes PIT to changed classes on PRs (GITHUB_BASE_REF), it is fast enough for the 2-core hosted runner - so mutation runs on GitHub-hosted as well, not just the self-hosted box. Non-gating (continue-on-error). fetch-depth: 0 so the changed-class diff resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…already passed Each GitHub-hosted test job, before running its slow 2-core suite, polls whether the self-hosted highcpu equivalent for the same commit already PASSED (ci-concede-check.sh). If so it skips and goes green; the hosted job stays the required gate but finishes faster when grumpy wins. Never depends on the self-hosted runner: offline/queued/slow/failed -> run the tests normally. Bounded wait, continue-on-error so a check bug can never fail the gate. Caveat documented: conceding trusts the self-hosted env (env-specific failures could be missed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
… the green-board experiment) Reverts the temporary experiment disable. The test is still the known flaky awaitility timeout (tracked in inflight) - harden it (adaptive await / underlying timing) rather than leave it disabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…fluentinc#857 hunt) DEBUG-only, behaviour-preserving observability for the silent-stall family: - ShardManager.getWorkIfAvailable: under-served detector - when less work is returned than requested while work is still tracked, log the breakdown [awaitingSelection, inFlight, retryQueue.size, readyToRetry] so 'asked for work / got none / but it's in the queues (or missing)' can be told apart from normal in-flight back-pressure. - WorkManager.isSufficientlyLoaded: log the throttle-decision inputs (awaitingSelection + outForProcessing vs target*loadingFactor). This gates broker-poller pause/resume, so numberRecordsOutForProcessing counter drift (confluentinc#857) shows up here as 'loaded' with no real work. - KafkaClientUtils.buildPc: unique myId per test PC so pc-control-PCn / pc-broker-poll-PCn thread names and the pcId MDC make concurrent instances attributable in logs (the myId hook was never wired; all instances collided). Safe: RebalanceEoSDeadlockTest matches thread names with .contains(). - logback-test.xml: labelled (commented) logger set for the stall hunt. These diagnostics fired during the forkCount=16 reproductions documented in docs/solutions/test-flakiness/pc-silent-stall-under-contention-2026-07-29.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
…ain-path spin + zombie partition hold (confluentinc#857 family) Research report assessing PR #75's red Integration check: not the grumpy runner, and NOT a benign flaky timeout - a PC under contention polls zero records even with a 120s bound (a stall, not slowness). No test was masked. Root-caused a drain-path defect: drain() fires ConsumerManager.signalStop() whose shutdownRequested flag short-circuits poll() so consumer.poll() is never called - defeating the intended 'paused 2s long poll = drain-loop sleep' (handlePoll() comment). Measured 92,880 poll-loop iterations in 9.0s (~10kHz busy-spin). With consumer.poll() never invoked, a draining PC cannot join rebalances while background heartbeats keep it a live member - a zombie holding its partitions (up to max.poll.interval.ms) while burning a core. Maps directly onto confluentinc#857's 'paused consumption after rebalance' (partition- hold hypothesis credit: Antony). Report includes a drain fail-safe design review (honour the 2s poll; hard deadline + explicit LeaveGroup; eager leave trade-offs; spin watchdog) and the reproduction recipe. inflight.md entry reframed accordingly: DO NOT MASK, route to confluentinc#857 (PR #29). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
|
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ci/grumpy-runner-workflow #80 +/- ##
===============================================================
- Coverage 78.01% 68.36% -9.65%
+ Complexity 1072 851 -221
===============================================================
Files 81 76 -5
Lines 4039 3806 -233
Branches 372 370 -2
===============================================================
- Hits 3151 2602 -549
- Misses 712 1007 +295
- Partials 176 197 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…anch experiment Checked both in-flight stall PRs against the zombie-drain root cause: - PR #29 (confluentinc#857): drain() still calls signalStop() first and the while(!shutdownRequested) short-circuit is intact - the ~10kHz spin and zombie partition hold survive unchanged. It fixes a SIBLING mechanism (onPartitionsAssigned resets pausedForThrottling - RUNNING-state throttle-pause) and even adds trace logging inside the loop that spins. Its 10-20% residual chaos-stall rate is a testable prediction: the chaos monkey's stop/start cycling is a drain-window factory. - PR #31 (confluentinc#909): main-code diff is only ProcessingShard.java - no overlap. All three efforts chase the same 'alive but not progressing' symptom via different, non-conflicting mechanisms, so the report now recommends an uber-branch experiment: merge #29 + #31 + the minimal drain fix (defer signalStop() to CLOSING), then measure #29's chaos run and this report's forkCount=16 recipe - if the residual stall rate drops to ~0, the zombie-drainer mechanism explains the residue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
The check name is "<workflow> / <job>", and both halves carried "High-CPU"
plus verbose filler ("Build & Test - ...", "self-hosted, optional"), pushing
past the PR merge-box truncation width.
- workflow name: "PR High-CPU Fast Feedback" -> "highcpu"
- job name: "Build & Test - <suite> (high-CPU self-hosted, optional)"
-> "<suite> (optional)" (keeps the non-gating signal)
Result: "highcpu / Unit (optional)", "highcpu / Integration (optional)", etc.
- longest ~35 chars, well under the cutoff.
Kept the concede matcher in sync (it resolves the highcpu run/jobs by name):
bin/ci-concede-check.sh WORKFLOW default -> "highcpu", JOB_PREFIX -> "${SUITE}"
(the job is now named "<suite> (optional)", so a suite-name prefix still
uniquely matches). Added a keep-in-sync comment on the job. Updated the
SELF_HOSTED_RUNNER.md reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
… never invoked while draining BrokerPollSystemDrainTest parks in-flight work on a latch, starts close(DRAIN) on another thread, and counts consumer.poll() invocations on the consumer spy across three long-poll periods of the drain window. CHARACTERISATION (asserts the CURRENT broken behaviour): the count is ZERO. drain() fires ConsumerManager.signalStop() before entering DRAINING, and poll()'s while(!shutdownRequested) guard then short-circuits without ever calling consumer.poll() - so the loop's intended sleep (the paused 2s long poll, per handlePoll()'s own comment) never happens (~10kHz spin measured), and the consumer cannot participate in rebalances while background heartbeats keep it a live member zombie-holding its partitions. The follow-up commit collapses the duplicated lifecycle flag and flips this test to assert the desired behaviour (poll continues at long-poll cadence). Also adds the implementation plan: docs/plans/2026-07-29-002-fix-drain-zombie-spin-and-uber-experiment-plan.md Full unit suite green (one unrelated pre-existing ReactorPCTest flake, passed on rerun). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
astubbs
changed the title
…down flag into poll-system lifecycle ConsumerManager.shutdownRequested was a private shadow of BrokerPollSystem.runState, settable out of phase - and it was: drain() set it on entry to DRAINING, so poll()'s while(!shutdownRequested) guard short-circuited and consumer.poll() was never invoked again during the drain window. Measured consequences: the poll loop busy-spun at ~10k iterations/s (the paused 2s long poll that handlePoll()'s own comment designates as the loop's sleep never ran), and the consumer became a rebalance-unresponsive zombie group member - background heartbeats kept it alive holding its full partition assignment (up to max.poll.interval.ms) while consuming nothing, starving same-group siblings. Maps onto upstream issue confluentinc#857's 'paused consumption after rebalance'. The fix collapses the duplicated state rather than re-timing it: - ConsumerManager: shutdownRequested + signalStop() deleted. The poll guard, SASL retryBackOff abort, and commitSync retry abort now consult an injected closeInProgressSignal (defaults to 'never' for standalone use). close() no longer raises a flag - by then the poll system is already CLOSING/CLOSED so the signal reports true to in-flight retry loops. Side-benefit: commits/retries during DRAIN are no longer wrongly aborted. - BrokerPollSystem: wires closeInProgressSignal = (runState == CLOSING or CLOSED) in its constructor - runState is the single source of truth, so the desync class is structurally impossible. Both signalStop() call sites removed; a draining consumer keeps polling (paused, long-poll cadence) and can promptly ack revokes/rejoin. runState is now volatile (it was already read cross-thread; the signal adds committer-thread readers). Test flipped RED->GREEN: BrokerPollSystemDrainTest, committed first as a characterisation of the defect (0 polls during drain), now asserts the desired property (>=1 poll per window at long-poll cadence, bounded above - no spin) and that close still completes cleanly. Full unit suite green (one unrelated pre-existing ReactorPCTest flake, passed on rerun, identical on the pre-fix baseline run). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
The build's README generator embeds CHANGELOG.adoc; this syncs the generated README.adoc with the changelog entry added in the drain-fix commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
…ring handover during drain Integration-level guard for the drain fix, calibrated against the planted defect on the experiment arms: - GREEN on this branch (fix present): B, joining mid-drain, consumed 200 records while A was still draining; A closed promptly on release; ledger: A=1100, B=200, duplicates=100 - exactly the parked in-flight tail (maxConcurrency), NOT a wholesale reprocess: the handover honoured A's commits. - RED on the no-fix arm (defect present): the group stayed frozen while A drained (B starved) until the worker-pool force-interrupt bailed A out - the mid-drain liveness window could never be established. Encodes the drain semantics explicitly (see report design review): a draining member CORRECTLY keeps its partitions to finish + commit (that is what bounds duplicates); what it must also do is stay rebalance-responsive so a mid-drain join never freezes the group. Report updated in the same spirit: design-review option 3 (eager LeaveGroup at drain start) REJECTED - it would guarantee duplicate reprocessing of in-flight work; explicit LeaveGroup belongs only in the deadline-expiry fail-safe (option 2). PR #29/#31 non-fix verification notes sharpened. Also the W1 skateboard scenario of the Chaos Pain Suite design (separate branch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
|
Claude finished @astubbs's task in 50s —— View job Re-review after synchronize (commit
|
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
Follow-on from fixing the comment in quarantine-lane.yml: the same superseded description had spread to three more places, all of them read by someone trying to work out when the lane actually runs. - bin/check-quarantine-registry.sh and bin/check-quarantine-owners.sh both said they are run by "the nightly lane". - QuarantinedAnnotationContractTest had a test whose NAME asserted the dead design - perPrWorkflowRunsTheAuditAndTheNightlyWorkflowRunsTheLane - plus a local variable and an assertion message to match. The test's assertions were already correct, and its sibling laneWorkflowDeclaresItsTriggers pins `pull_request:` explicitly - so the executable part of the contract had kept up while every human-readable part of it had not. That is the failure mode worth naming: a test can encode the right behaviour and still teach the wrong one, because people read the name. Renamed to perPrWorkflowRunsTheAuditAndTheLaneWorkflowRunsTheTests. No coverage change; all 11 tests in the class pass, and both scripts still run clean (registry reports 0 entries - master's #80 merge emptied the lane). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
…deferred thread-parallel check The header now says that work your own PR resolves is tracked by that PR, so its entry goes in that PR - never leave a marker on master saying to delete it later. A merge is exactly when nobody is reading this file, so such markers outlive the work: the one #80 left behind survived the merge and read as live afterwards. Also restores an open item this rewrite dropped. #68 made integration reliable by forking per broker, which sidesteps the upstream confluentinc#857 deadlock rather than proving it gone - so re-running with -Dparallel-tests=true on a shared broker, once #29 lands, is still outstanding. Recorded against #29, which is what gates it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
The stacked-PR gap is closed: an "All branches: PR dependency gate" ruleset (~ALL, requiring only "Check PR Dependencies") now covers PRs whose base is a feature branch, which the master-only ruleset never matched. Verified live on #112. The concede optimizer is abandoned, not parked - it was removed from #75 by review, re-introduced and dropped again on #80, and highcpu stays purely advisory. A five-point revival checklist for something nobody intends to revive is the "keep it in case" habit this file is meant to resist; the findings survive in #75's and #80's review history if it ever comes back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
Merged
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
…s stop colliding (#112) The ledger had reached 677 lines and become a record of finished work. Entries told to "delete when #98 merges" outlived three merges; the rebalance-commit fix was still filed as awaiting PR after landing as #100; a static-state flake was still listed as deliberately-unfixed after #101 fixed it at source. Meanwhile #80, carrying the drain-zombie fix and the largest thing actually in flight, appeared only as a footnote about quarantine ownership. The file's own scope rule says entries vanish when work lands; it had stopped being applied. Every surviving claim is now checked against GitHub, git and the code. Obsolete entries are deleted rather than annotated: the jscpd cap is above baseline, no workflow has path filters so the docs-only inconsistency cannot happen, the ManagedPCInstance header carries its Modifications line, the CommitFailedException follow-up shipped in #108, and the stacked-PR gap is closed by a new all-branches ruleset. Upstream references now follow the convention the changelog already uses: fork numbering reaches #111, so every bare reference at or above #162 was silently upstream and read as ours. With the content correct, the file became a directory. It appeared in 26 of the last 30 master commits, so unrelated PRs conflicted on it constantly - not because they disagreed, but because their notes were adjacent, and the merge that resolves such a conflict silently drops one side (it happened between #108 and #110). Each item is now its own file, named <category>-<slug>.md; two PRs recording unrelated work touch disjoint files and cannot conflict, and finishing work is `git rm`, which never conflicts with an edit elsewhere. The prefix is the structure - `ls docs/inflight/` shows the shape of what is open without reading anything. There is deliberately no committed index: it would be edited by every PR, which is the problem the directory solves, and docs/TODO_INDEX.md is the cautionary case. This was parked on docs/inflight-as-directory with "migrating ~600 lines of existing entries" named as the reason not to do it. The audit is what made it cheap. docs/refactoring.md stays a single file - 2 of the last 30 commits touched it, so it has none of this problem. The manifest had drifted the same way, which matters more, because it is the declared source of truth that a future session trusts instead of re-deriving. Five entries disagreed with reality: bug-857 recorded no fork PR while #29 was open, #100 merged and #80 in review; fix-909 recorded no PR though #31 is open; bug-912 was in-progress when the schema's word for pushed-but-unPR'd is ready. Nothing catches this - upstream-map.py validate only checks the schema and upstream-sweep.sh only watches upstream, so "prs: []" beside an open PR passes every check we have. Hence the AGENTS.md rule to update it at every lifecycle transition, not only when starting. Four rules now live in docs/inflight/AGENTS.md so they are inherited rather than rediscovered: delete an entry in the PR that resolves it and never leave a "delete when #NN merges" marker on master; never record what gh or git can answer, which is why the open-PR table is gone; known code defects belong here even when an issue exists, because an agent scans this directory and will not read the tracker; and new guidance about how these notes are written goes into that file too. Four deferred code items moved to docs/refactoring.md, where deferred work lives: the SpotBugs thread-visibility findings, the produce-lock double release surfaced by #110, the commitAndWait message that misstates its own wait, and jacoco's single exec file under forked surefire. Also shares the agent-tooling gitignore rules that until now existed only in one checkout's .git/info/exclude, and deletes an empty duplicate-code-cross-check clone - a git init that never fetched - rather than ignoring it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
… from the commit log (#113) The unreleased section had reached 15 entries and 1,828 words - longer than every 0.5.x release note combined - and every one was Build & CI. Median entry 104 words against 8-15 in the upstream-era sections; the longest, at 316 words, explained that a tool-grant string did prefix matching. That is a postmortem, and postmortems belong in the commit message, the PR body and docs/solutions/, which is where this project already puts them. The problem was audience, not length. A changelog answers one question - should I upgrade, and will anything change for me - so an entry now has to pass a test: can a user or operator observe it without reading our repo? CI lanes, reviewer tool grants, PR checklists and TODO indexes cannot, and are gone. What survives of that work is a single Build & CI entry listing the five things that tell a reader how carefully the library is tested. Surviving entries are one sentence of about 25 words: what a reader would have SEEN, plus who it hits when that is not everyone. The rule names both failure modes, because the upstream-era entries show the other one - "fix: Paused consumption across multiple consumers" is six words from which you cannot tell whether you are affected. Section total: 2,570 words to 569. Two headings that both meant "not released" are collapsed into one. Entries were split between == Unreleased and == 0.6.0.0 while neither had shipped, which is how #80's drain fix - the most user-visible change in the release - ended up filed apart from everything else. Beyond that, a PR now does nothing about the changelog at all. The file appeared in 30 of the last 30 master commits and dragged the generated README.adoc with it, and it forced an entry to cite a PR number that does not exist when the entry is written. Everything up to 0.6.0.0 is frozen; from the next release an agent reads git log <last-tag>..HEAD and drafts the section, and a human re-applies the same judgement before it freezes. What is written down is that judgement - the entry test, the one-sentence rule, and assembling the release as a SET, merging related commits and dropping what did not matter, which no per-PR entry could ever do. The cost is that a commit message now does double duty, so a lazy subject line becomes a worse changelog. The README stops embedding the changelog - 505 lines - and keeps a Change Log section pointing at it. The checklist line goes too: with nothing to do per PR, a box that always reads N/A is the ritual this removes. Also corrects the "where things live" table, which still said README.adoc is generated FROM CHANGELOG.adoc. It is still generated, from README_TEMPLATE.adoc, but no longer from the changelog. That contradiction existed in neither change alone - #112 rewrote the row, this PR removed the include - and appeared only when the merge put both in one file. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
4 tasks
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
…ng them
committedOffsetRemoved[1] latest failed intermittently on the 2-core hosted
runner and passed on a 12-core laptop, which read as a race. It is arithmetic.
awaitWithTopicNudge produces one nudge record per Awaitility poll iteration
(BrokerIntegrationTest:252) - 1s interval, 10s cap, so 1..10 records, and how
many depends on load. Two places still assumed exactly one:
producedCount = producedCount + 1; // "run sends one"
checkHowManyRecordsWithKeyPresent("key-50", 2, TO_PRODUCE + 2);
The window TO_PRODUCE + 2 assumes the two compaction records sit at offsets
200/201. With N nudges they sit at 200+N/201+N, so the scan stops before
reaching them:
1 nudge -> keys at 201,202 -> scan sees the first -> passes
2 nudges -> keys at 202,203 -> scan sees neither -> expected 2, but was 1
which is exactly the CI failure, down to the surviving record being the
ORIGINAL key-50 (value-50) rather than the compactor. The ambient probe said so
too: "probe clean - the fault is likely in the test itself".
git log -L dates the "+ 1" to upstream 28ccc1d, when runPcUntilOffset really
did send one record up front. #80 later moved nudging INSIDE the await and made
the count unbounded, without updating the arithmetic that depended on it.
Why [1] latest specifically: EXPECTED_RESET_OFFSET is producedCount only under
LATEST (EARLIEST uses 0), so the miscount is harmless for EARLIEST - and LATEST
needs more nudges to get past the offset-reset race, making N>=2 likelier.
The fix removes the assumption rather than correcting it. The broker knows how
many records exist, so both call sites now ask: checkHowManyRecordsWithKeyPresent
reads its bound from endOffsets and drops the parameter, and producedCount comes
from currentEndOffset(). Widening the constant was rejected - it restores green
while leaving the same latent assumption, and this test has already cost one
quarantine cycle.
Verified by prediction rather than by absence of failure. Injecting ONE extra
record reproduced the exact signature deterministically on an idle machine, in
BOTH parameters. After the fix, TWO extra records - a harsher trigger than the
one that broke it - give 3 tests, 0 failures. A soak of 8 runs with only 2 cores
left free also passes 8/8, though that is corroboration, not proof: a soak can
only fail to disprove.
Unrelated and still open: [3] none errored once with RebalanceInProgressException
during this work. Different fault, not addressed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
astubbs
added a commit
that referenced
this pull request
Aug 5, 2026
…nst #80 This branch was cut before the rebrand and before the rest of the confluentinc#857 family landed, so it still targeted `master-confluent` and sat 61 commits behind. Three conflicts, none of them mechanical: - `ConsumerManager` - the branch added `claimConsumerOwnership()` and `signalStop()` side by side. #80 deleted the whole `shutdownRequested` flag, collapsing it into the poll system's `runState` (see `BrokerPollSystemDrainTest`), and rewrote the drain path that was `signalStop()`'s only caller. Kept the ownership claim, dropped `signalStop()` as dead - reinstating it would restore the second source of truth #80 removed. - `ManagedPCInstance` - both sides added it. Master's copy is a strict superset (MDC cleanup on pooled threads, `extraConsumerProps`, try/finally), so master's wins outright. - `pom.xml` - took master's test dependency versions and the byte-buddy pin, and dropped the branch's duplicate `archunit-junit5` declaration; master already declares it at 1.4.2 for all modules. Also brought the branch's headers under the fork's provenance policy, which postdates it: fork-original files (`ThreadConfinedConsumer`, `ArchitectureTest`, `ShardManagerStaleContainerTest`, `ManagedPCInstanceLifecycleTest`) now carry the fork header rather than a Confluent one, and the five modified upstream-derived files carry the modifications line. `MultiInstanceRebalanceTest` also had its Confluent year bumped to 2026 on the branch; restored to 2020-2022 per policy. Compiles clean across main, test and test-integration sources.
9 tasks
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…ach the gate about anchors Two things the reference sweep surfaced in Java sources. The quarantine script tests built their fixture registries with "Owner: PR #999", "PR #80" and "PR #123". The last two are real fork PRs, so the fixtures read as genuine references to anyone grepping, and #999 is close enough to the live range to be mistaken for one. They are now #999999 and #999998 - unmistakably fake, and above the threshold where a bare number is ambiguous. Two distinct values, because the owner-mismatch tests compare a @Quarantined value against a registry value and need them to differ. 27 tests pass. The gate also flagged references that were already qualified by an html anchor - TransactionMarkersTest has <a href="...confluentinc/issues/329">Github issue confluentinc#329</a>, where the href names the repo and the number in the link text is unambiguous to any reader. stripQualified removed bare URLs but not the anchor element, so the visible "confluentinc#329" looked bare. It now strips the whole element, with a test confirming a bare number elsewhere on the same line is still caught. Together these take the tree-wide backlog from 390 to 374, and the Java share from 53 to 36. The follow-up note now carries the finished classification of all 36 Java references - file, line, and which repo each means, resolved against both - so that pass does not repeat this research. Also records that @tag("confluentinc#355") is a tag-string rename rather than a prose prefix, and that nothing selects on that tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 7, 2026
Reviewing the new solution doc surfaced a hole in the checklist that same PR had just added: it said to check open PRs, which is a collision check. The PR that already solved something in your file is by definition merged, so searching only the open list finds nothing and feels like diligence - worse than not looking, because it produces false confidence. Same for issues: the useful ones are usually closed. Searching merged PRs BY TOUCHED FILE - not by keywords in their titles - finds, for this investigation: - #110, the sibling flake on this same producerTransactionLock, fixed four days earlier and the source of the control-arm method - #86, which introduced the ambient probe whose "probe clean" verdict this work had to qualify - #98, the backpressure test that only passed by racing its own setup - the sibling rule this doc deliberately does not merge with None would surface from a symptom search or from the open list. All three are now cited in the solution doc, with #80, #115 and #68 from the same family, and the confluentinc#803 / confluentinc#809 / confluentinc#833 cluster upstream. Adds to the doc what the prior-art search actually covered, including what it did NOT (the highcpu and quarantine lanes), so the next reader knows where the gaps are instead of assuming there are none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 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.
One flaky CI check (
PartitionStateCommittedOffsetIT.committedOffsetRemoved[latest]) turned out to conceal two independent bugs - one real production defect in the close path, one long-lived race in the test harness. This PR fixes both, with each fix proven RED→GREEN against its captured defect, and lands the diagnostics that made the hunt possible so the next investigation costs hours instead of days.Bug 1 (product): draining consumer busy-spins and zombie-holds its partitions
ConsumerManager.shutdownRequestedwas a private shadow ofBrokerPollSystem.runState, set at the wrong lifecycle phase:drain()raised it on entry to DRAINING, sopoll()'s guard short-circuited andconsumer.poll()was never invoked again during the drain window. Measured consequences:handlePoll()'s own comment designates as the loop's sleep never ran;poll(), so the draining consumer became a rebalance-unresponsive zombie: background heartbeats kept it a live group member holding its full partition assignment (up tomax.poll.interval.ms) while consuming nothing - freezing same-group siblings. Relates to upstream Paused consumption across multiple consumers confluentinc/parallel-consumer#857's "paused consumption after rebalance".Fix: collapse the duplicated lifecycle state rather than re-time it - the flag and
signalStop()are deleted;ConsumerManagerderives "close in progress" from the poll system'srunState(now volatile) via an injected signal. One source of truth; the desync class is structurally impossible. Drain semantics preserved: a draining member correctly keeps its partitions to finish in-flight work and commit (that is what bounds duplicates) - what it must also do, and now does, is stay rebalance-responsive.Guards:
BrokerPollSystemDrainTest(unit; committed first as a characterisation of the defect, then flipped RED→GREEN by the fix) andDrainingMemberRebalanceIT(integration: PC-B joining mid-drain makes progress while PC-A drains; clean close; correctness ledger - no records lost, duplicates exactly the parked in-flight tail, proving the handover honoured A's commits).Bug 2 (test harness): the
auto.offset.reset=latestnudge raceThe flake itself was not a product stall: a consumer with LATEST reset and no committed offset resolves its start position when the reset executes.
runPcUntilOffsetproduced its single nudge record before the await - under contention, bootstrap takes 1-3s, the reset resolves after the nudge, and the consumer is positioned past every record that will ever exist: the await is unwinnable at any timeout. Only the[latest]param can lose this race - and every observed failure ever was[latest]. Captured via the Kafka client's own (normally silenced) internals:Resetting offset ... to position FetchPosition{offset=201}on a topic holding exactly 201 records.Fix, DRY'd per the ensureTopic drift lesson: shared
BrokerIntegrationTest#awaitWithTopicNudge- produces a nudge inside each await attempt (the patternrunPcCheckStartIsalready used inline) and self-diagnoses on timeout (topic end vs group committed vs nudges sent). Both helpers delegate to it; no timeout enlarged, no assertion weakened.LatestResetTailNudgeITbottles the race deterministically (RED with the old pattern in ~23s every run; GREEN with the primitive).Validation
committedOffsetRemoved0/20 (historical failure rate ~33% → P(luck) ≈ 0.03%); 15/20 fully clean; zero stall-class failures. Residuals catalogued indocs/inflight.mdas the load-tightness flake family (fast-fail assertion flakes, pass isolated).Also included
ShardManagerunder-served detector,WorkManagerthrottle-decision log, per-PCmyIdlog attribution, correlation markers on tail-nudge awaits, a standing (commented) kafka-client bootstrap DEBUG harness, andSubscriptionStateat INFO (offset-reset lines are rare and decisive).docs/solutions/test-flakiness/pc-silent-stall-under-contention-2026-07-29.md(the investigation + drain design review) andlatest-reset-nudge-race-committedoffsetremoved-2026-07-30.md(the solved mechanism + diagnosability lessons).docs/inflight.mdupdated throughout.docs/plans/.ChaosChurnStormIT.churnStormMeetsSlosAndBalancesLedger,PartitionStateCommittedOffsetIT.committedOffsetRemoved): their fixes are this PR, so the@Quarantinedannotations anddocs/QUARANTINED_TESTS.mdentries are deleted here - the registry is now empty. Triggered by the Quarantine Lane's unexpected-pass detection on this branch.highcpuwith a trivially-passing job and make the required gate skip real tests), somaven.ymlis now byte-identical to master andbin/ci-concede-check.shis deleted; collapsed three doubled copyright headers (amaven-license-pluginartifact) inBrokerIntegrationTest,KafkaClientUtils,RetriesTest; and pruned satisfieddocs/inflight.mdentries.Checklist
CHANGELOG.adoc, under== Unreleased) - drain-zombie fix entrydocs/solutions/write-ups,docs/inflight.md,docs/QUARANTINED_TESTS.md, plan docBrokerPollSystemDrainTest(RED→GREEN characterisation),DrainingMemberRebalanceIT,LatestResetTailNudgeIT; two quarantined tests re-enabledmaven.ymlis byte-identical to master. A stale "concede" mechanism that would have let a self-hosted result satisfy the required GitHub-hosted gate (rejected from PR ci: self-hosted high-CPU fast-feedback workflow + PR-scoped advisory PIT #75 as a gate-spoof) had ridden in via an old merge and has been removed here.🤖 Generated with Claude Code
https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18