Skip to content

RATIS-2499. Allow the LogAppender restart when LogAppenderDaemon exception#1425

Merged
szetszwo merged 4 commits intoapache:masterfrom
xichen01:RATIS-2499
Apr 8, 2026
Merged

RATIS-2499. Allow the LogAppender restart when LogAppenderDaemon exception#1425
szetszwo merged 4 commits intoapache:masterfrom
xichen01:RATIS-2499

Conversation

@xichen01
Copy link
Copy Markdown
Contributor

@xichen01 xichen01 commented Apr 7, 2026

What changes were proposed in this pull request?

RATIS-2427 let the LogAppender do not restart when the LogAppenderDaemon in CLOSING or CLOSED or EXCEPTION LifeCycle.States . However, if only LogAppenderDaemon is EXCEPTION LifeCycle.States, we should still restart LogAppender.

LeaderStateImpl#restart will create a new GrpcLogAppender (see addAndStartSenders -> addSenders -> LeaderStateImpl#newLogAppender) and a new LogAppenderDaemon. The old LogAppenderDaemon will be shutdown

Therefore, if the old LogAppenderDaemon enters the EXCEPTION lifecycle for some reason, we should attempt to restart LogAppender

What is the link to the Apache JIRA

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

How was this patch tested?

new added test

Copy link
Copy Markdown
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

@xichen01 , thanks for catching this bug! The change looks good. Please see the comment inlined.

This bug actually is quite serious that the Ratis 3.2.2 release becomes unusable. Isn't it?

Sorry that the bug was from my review. Cc @OneSizeFitsQuorum , @adoroszlai , @ivandika3 , @symious

Comment on lines 131 to 135
private boolean isLeaderReady() {
return server.getInfo().isAlive()
&& server.getInfo().isLeader()
&& getRaftLog().isOpened();
}
Copy link
Copy Markdown
Contributor

@szetszwo szetszwo Apr 7, 2026

Choose a reason for hiding this comment

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

  • Since we already have defined isLeaderReady(), which means that a new leader has committed the first entry. Let's use a different name isLeaderAlive() to avoid confusion.
  • Also, please use 4 spaces for indentation. It really looks better in this case.
  private boolean isLeaderAlive() {
    return server.getInfo().isAlive()
        && server.getInfo().isLeader()
        && getRaftLog().isOpened();
  }

Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. LGTM +1. Great to see the test to catch any LogAppender restart regression.

Copy link
Copy Markdown
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo szetszwo changed the title RATIS-2499. Allow the LogAppender restart when LogAppenderDaemon exce… RATIS-2499. Allow the LogAppender restart when LogAppenderDaemon exception Apr 8, 2026
@szetszwo szetszwo merged commit df2a302 into apache:master Apr 8, 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