Skip to content

Fix WAL roll recovery after closed channel - #18653

Open
jt2594838 wants to merge 1 commit into
masterfrom
fix/wal-roll-closed-channel-recovery
Open

jt2594838 wants to merge 1 commit into
masterfrom
fix/wal-roll-closed-channel-recovery

Conversation

@jt2594838

Copy link
Copy Markdown
Contributor

Problem

A DataNode can remain read-only after disk recovery because WAL rotation may fail after the current channel is closed, leaving the buffer in a state that cannot progress to the next WAL file.

Changes

  • Preserve WAL rotation progress and continue with the next file after a closed-channel failure.
  • Propagate channel/force failures so callers do not acknowledge an incomplete WAL write.
  • Require complete metadata persistence before considering a WAL file closed.
  • Add regression coverage for roll recovery and closed-channel behavior.

Validation

  • WAL tests: 22 tests passed.
  • Maven build: SUCCESS.

addFileNum(1);
File lastFile = currentWALFileWriter.getLogFile();
if (!hasPendingRoll()) {
// Record the boundary only after sealing and forcing the old WAL have both succeeded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserves the sealed WAL and current rotation stage across retries, so a later sync task can continue after disk space is restored without closing or counting the old file twice.

final long startTime = System.nanoTime();

if (syncFailure != null) {
failListeners(syncFailure);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propagates a failed sync batch to all listeners and keeps the buffer recoverable. SET SYSTEM TO RUNNING alone cannot repair an incomplete WAL record, so retry state is cleared only after the pending roll completes.

logChannel.write(headerBuffer);
}
while (buffer.hasRemaining()) {
logChannel.write(buffer);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consumes the full buffer and lets ClosedChannelException reach the caller. A partial or failed write must trigger WAL recovery instead of being acknowledged as a successful flush.

logChannel.write(buffer);
// A successful seal is the recovery boundary for switching to the next WAL file.
while (buffer.hasRemaining()) {
logChannel.write(buffer);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Persists the complete metadata trailer before treating the file as sealed; this trailer is the recovery boundary for switching to the next WAL file.

*/
@Test
public void testResumeBeforeWritingAfterRepeatedOpenFailures() throws Exception {
writeAndAwait(entry(1, 1, "before"), Status.SUCCESS);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covers repeated successor creation failures and verifies recovery resumes at the saved roll stage without duplicate sealing, counter inflation, or data loss.

/** Unexpected channel closure must propagate to the buffer instead of acknowledging a write. */
@Test
public void testClosedChannelWriteAndForceFail() throws IOException {
WALWriter writer = new WALWriter(walFile);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirms closed-channel write and force operations fail visibly and leave the WAL unchanged.

@hongzhi-gao

Copy link
Copy Markdown
Contributor

LGTM

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.84%. Comparing base (a05c63e) to head (bce8126).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
.../db/storageengine/dataregion/wal/io/LogWriter.java 57.14% 6 Missing ⚠️
...storageengine/dataregion/wal/buffer/WALBuffer.java 93.33% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18653      +/-   ##
============================================
+ Coverage     42.82%   42.84%   +0.01%     
  Complexity      442      442              
============================================
  Files          5451     5451              
  Lines        395425   395477      +52     
  Branches      51805    51818      +13     
============================================
+ Hits         169349   169431      +82     
+ Misses       226076   226046      -30     

☔ 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.

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