Skip to content

Fix consensus subscription WAL retention and replay cursor - #19042

Open
Caideyipi wants to merge 1 commit into
masterfrom
fix/subscription-wal-roll-retention-20261028
Open

Caideyipi wants to merge 1 commit into
masterfrom
fix/subscription-wal-roll-retention-20261028

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

When a live WAL rolls after its last ACK, consensus subscription retention can keep the formerly live file pinned even though committed progress already covers it. Replay can also revisit reclaimed predecessor files during successive cursor advances, or fail to realign when pending delivery is one position beyond the live tail.

Recheck the retention boundary when the WAL rolls and position the replay iterator past covered sealed history without reopening an already passed predecessor. Uncovered follower entries still prevent fast-forwarding. The PR adds focused regression coverage for these conditions.

Validation:

  • In the source checkout at 74f882ef1556b30bb8c0ede80b51628ef8435e9c, with the same product fix and unit-test changes: ConsensusPrefetchingQueueTest passed 53 tests; ProgressWALIteratorTest passed 33 tests with 1 existing skip.
  • The clean reactor and node-template build passed. On final MR revision d1db4dcb29aaea1d9109f82f051d0a15e74b36ab, after a clean rebuild of the changed integration-test module, the Tree and Table recovery-window classes each passed 4 tests with 0 failures, errors, or skips. These are Timecho results, not an upstream-checkout test run.
  • Spotless and git diff --check passed. A separate upstream clean build/test run has not been performed.

GitHub CI status at publication: the zh-locale-compile check failed in distribution packaging because the workflow could not find the server, CLI, and ConfigNode snapshot ZIPs; this was not a Java compilation error in the changed files. SonarCloud Code Analysis also failed with reported findings outside this PR's changed files; no defect in the changed files was confirmed from that report. Most other checks passed. The PR remains Draft for an independent review.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.04%. Comparing base (d452ddd) to head (efc5310).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...cription/broker/consensus/ProgressWALIterator.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #19042   +/-   ##
=========================================
  Coverage     48.04%   48.04%           
  Complexity      604      604           
=========================================
  Files          5570     5570           
  Lines        409051   409047    -4     
  Branches      53848    53845    -3     
=========================================
+ Hits         196512   196521    +9     
+ Misses       212539   212526   -13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Caideyipi
Caideyipi marked this pull request as ready for review October 29, 2026 04:16
@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Initial independent self-review completed for efc531082ef8bd2398cb6f4738816542afe670d7, against base 8cdc9681d0141fa51cfd0dc120b67a7383270865. I inspected the complete diff, all four affected files, and the relevant WAL roll, retention, commit, replay, and seek call paths. No actionable source-behavior or test-logic issues were found; no fixes were needed.

  • Retention now rechecks a formerly live WAL after a roll even when ACK progress is unchanged. The old writer is sealed before the new version is published; uncovered requirements, progress rollback, and metadata-read failures remain conservative.
  • Fast-forward distinguishes an already-passed predecessor from a current or deferred reader. Buffered requests and the newly skipped prefix still require writer-progress coverage, including followers without local search indexes. Tail-plus-one positioning preserves those checks. I found no new exception, lock-order, or resource-lifecycle issue in the affected paths.
  • The regression assertions exercise the unchanged-ACK roll, repeated advances after reclaiming the prefix and intermediate file, and tail-plus-one probes after reclaiming covered history. They also retain the uncovered-follower assertion. Static tracing shows where the original logic would fail these assertions; counter-runs against the original implementation were not executed.

Validation:

  • git diff --check passed.
  • The supplied clean-reactor log from a separate source tree reports 86 tests: 85 passed, zero failures/errors, and one existing opt-in performance test skipped. This does not establish an independent clean build of this exact upstream revision.
  • This round's independent clean-reactor attempt timed out fetching the exact source revision before Maven started. The build, focused tests, and original-implementation counter-runs therefore remain unrun. Local fallback lacked sufficient available virtual memory.
  • Upstream CI reports passing JDK 17 unit-test jobs on Linux and Windows. ZH compilation failed at distribution dependency resolution for missing server/CLI/ConfigNode ZIP artifacts. The SonarCloud quality gate remains failed; the retrieved annotations in the affected production files are outside the changed hunks. These statuses are not reported as passing.

The independent review gate passed. I rechecked that the remote head still matched the reviewed revision, marked the PR ready for review, and verified its non-draft state.

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