Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19042 +/- ##
============================================
+ Coverage 48.04% 48.06% +0.02%
Complexity 604 604
============================================
Files 5570 5572 +2
Lines 409051 410022 +971
Branches 53848 54061 +213
============================================
+ Hits 196512 197063 +551
- Misses 212539 212959 +420 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Initial independent self-review completed for
Validation:
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. |
| if (Objects.equals(lastCommittedProgressForRetention, committedRegionProgress) | ||
| && Objects.nonNull(lastRetainedWalFileForRetention) | ||
| && lastRetainedWalFileForRetention.exists()) { | ||
| return false; | ||
| } | ||
|
|
||
| // The previous boundary still applies while its sealed requirement is uncovered, or while | ||
| // it is the current live WAL file. Both cases require monotonically advancing progress. | ||
| // it is the current live WAL file. Recheck even unchanged progress: a roll may have sealed | ||
| // the formerly live file after its last ACK, making that file reclaimable without new writes. |
There was a problem hiding this comment.
Is it possible to check only if it is the live WAL file?
There was a problem hiding this comment.
Applied in 135535a5f6. The unchanged-progress shortcut now explicitly requires the retained boundary to still be the current live WAL file. After a roll, the version no longer matches, so the sealed boundary is rechecked even without another ACK. The existing cached requirement check still keeps an uncovered sealed file retained.
| private int locateTargetFile(final long targetSearchIndex) { | ||
| if (walFiles.length == 0 | ||
| || (liveWalNode != null && targetSearchIndex > liveWalNode.getCurrentSearchIndex())) { | ||
| if (walFiles.length == 0) { | ||
| return -1; | ||
| } | ||
| // Pending delivery may have advanced one past the live tail. Still realign to its file after | ||
| // verifying coverage of the sealed prefix, before committed history can be reclaimed. | ||
| return WALFileUtils.binarySearchFileBySearchIndex(walFiles, targetSearchIndex); | ||
| } |
There was a problem hiding this comment.
Is it possible to locate the last file directly in this case?
There was a problem hiding this comment.
Applied in 135535a5f6. When the target is beyond the live search-index tail, locateTargetFile() now returns walFiles.length - 1 directly. advanceTo() still checks buffered requests and writer-progress coverage of the skipped prefix before repositioning, including follower entries without a local search index.
|
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 after a roll, while keeping the unchanged-progress shortcut only when the boundary is still the current live WAL. Position replay past covered sealed history without reopening an already passed predecessor, and locate the last WAL directly when the target is beyond the live tail. Buffered requests and newly skipped files still require writer-progress coverage, including follower entries without a local search index.
Validation of
135535a5f673846243485bf0e1bbe2bb99e5496b:mvn -B -ntp -Ddevelocity.enabled=false clean test -pl iotdb-core/datanode -am -Dtest=ConsensusPrefetchingQueueTest,ProgressWALIteratorTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false -Dtest.port.closed=true -DforkCount=1(GITHUB_ACTIONS=true).ConsensusPrefetchingQueueTest: 53 tests, zero failures/errors/skips.ProgressWALIteratorTest: 33 tests, zero failures/errors, one existing opt-in performance test skipped.mvn spotless:apply -P with-integration-tests -pl iotdb-core/datanodeandgit diff --checkpassed.The regression coverage includes unchanged ACK progress across a WAL roll, repeated cursor advances after reclaiming predecessor files, tail-plus-one probes after reclaiming covered history, and preservation of uncovered follower entries. No 24-hour endurance run was performed for this update. The results above are local validation; CI is tracked separately.
The PR remains Draft pending a fresh independent review of the review-feedback update.