Skip to content

RATIS-2647. Trigger shutdown for async log flush failure - #1643

Merged
szetszwo merged 3 commits into
apache:masterfrom
ivandika3:RATIS-2647
Oct 16, 2026
Merged

szetszwo merged 3 commits into
apache:masterfrom
ivandika3:RATIS-2647

Conversation

@ivandika3

@ivandika3 ivandika3 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Trigger shutdown for async log flush failure. It should shutdown the server async flush failure since it is a fatal RaftLog error.

Generated-by: GPT 5.6 Terra.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2647

How was this patch tested?

UT. Introduce a new injected failure for FileChannel#force through the FILE_CHANNEL_FORCE and check that the flush index is not increased and failure is set when failure was injected.

@ivandika3
ivandika3 requested a review from szetszwo October 15, 2026 03:25
.whenComplete((v, e) -> {
updateFlushedIndexIncreasingly(lastWrittenIndex);
logSyncTimerContext.stop();
if (e == null) {

@amaliujia amaliujia Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be concrete, should we check IOException?

  CompletableFuture<Void> asyncFlush(ExecutorService executor) throws IOException {
    try {
      return out.asyncFlush(executor);
    } catch (IOException ioe) {
      String msg = "Failed to asyncFlush " + this;
      LOG.error(msg, ioe);
      throw new IOException(msg, ioe);
    }
  }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IOException will be caught in SegmentedRaftLogWorker.

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ivandika3 , thanks for working on this!

Please see the comment inlined.

Comment on lines +433 to +435
} else if (failureHandler != null) {
failureHandler.accept(e);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's simply call server.close()?

          if (e == null) {
            updateFlushedIndexIncreasingly(scheduledFlushIndex);
          } else {
            LOG.error("{} Failed asyncFlush", this, e);
            server.close();
          }

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.

Thanks, updated.

.whenComplete((v, e) -> {
updateFlushedIndexIncreasingly(lastWrittenIndex);
logSyncTimerContext.stop();
if (e == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IOException will be caught in SegmentedRaftLogWorker.

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the chnage looks good.

@szetszwo
szetszwo merged commit 78a1f27 into apache:master Oct 16, 2026
16 checks passed
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.

3 participants