szetszwo commented on code in PR #1369:
URL: https://github.com/apache/ratis/pull/1369#discussion_r2921452899
##########
ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java:
##########
@@ -85,6 +85,9 @@ private void run() {
LOG.info(this + " was interrupted: " + e);
} catch (InterruptedIOException e) {
LOG.info(this + " I/O was interrupted: " + e);
+ } catch (IllegalStateException e) {
Review Comment:
@symious , you are right that we should not restart LogAppender.
Instead of catching IllegalStateException. Let's check if the RaftLog is
open. Also, LogAppender isRunning() should return false.
```diff
diff --git
a/ratis-server-api/src/main/java/org/apache/ratis/server/raftlog/RaftLog.java
b/ratis-server-api/src/main/java/org/apache/ratis/server/raftlog/RaftLog.java
index e504462b8..e194f865e 100644
---
a/ratis-server-api/src/main/java/org/apache/ratis/server/raftlog/RaftLog.java
+++
b/ratis-server-api/src/main/java/org/apache/ratis/server/raftlog/RaftLog.java
@@ -43,6 +43,9 @@ public interface RaftLog extends RaftLogSequentialOps,
Closeable {
/** Invalid log index is used to indicate that the log index is missing.
*/
long INVALID_LOG_INDEX = LEAST_VALID_LOG_INDEX - 1;
+ /** Is this log already opened but not yet closed? */
+ boolean isOpened();
+
/** Does this log contains the given {@link TermIndex}? */
default boolean contains(TermIndex ti) {
Objects.requireNonNull(ti, "ti == null");
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderBase.java
b/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderBase.java
index 5a27cda51..be0404da3 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderBase.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderBase.java
@@ -124,7 +124,10 @@ public abstract class LogAppenderBase implements
LogAppender {
@Override
public boolean isRunning() {
- return daemon.isWorking() && server.getInfo().isLeader();
+ return daemon.isWorking()
+ && server.getInfo().isAlive()
+ && server.getInfo().isLeader()
+ && getRaftLog().isOpened();
}
@Override
@@ -133,8 +136,8 @@ public abstract class LogAppenderBase implements
LogAppender {
}
void restart() {
- if (!server.getInfo().isAlive()) {
- LOG.warn("Failed to restart {}: server {} is not alive", this,
server.getMemberId());
+ if (!isRunning()) {
+ LOG.warn("{} is not running: skipping restart", this);
return;
}
getLeaderState().restart(this);
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java
index 8c2b66f96..48b410147 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java
@@ -113,7 +113,7 @@ public abstract class RaftLogBase implements RaftLog {
state.assertOpen();
}
- /** Is this log already opened? */
+ @Override
public boolean isOpened() {
return state.isOpened();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
