This is an automated email from the ASF dual-hosted git repository. szetszwo pushed a commit to branch branch-3.1.1_review in repository https://gitbox.apache.org/repos/asf/ratis.git
commit 49f1247a0d5b55e4aa3105eb4816e1ddfc10342a Author: Chung En Lee <[email protected]> AuthorDate: Tue Aug 6 22:01:31 2024 +0800 RATIS-2134. `logMetadata` could miss appending the `metadataEntry` with the lastCommitIndex (#1130) --- .../org/apache/ratis/server/impl/LeaderStateImpl.java | 17 +++++++++++------ .../org/apache/ratis/server/raftlog/RaftLogBase.java | 17 +++-------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java index eacf50615..32f9dbeed 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java @@ -946,10 +946,7 @@ class LeaderStateImpl implements LeaderState { private void updateCommit(LogEntryHeader[] entriesToCommit) { final long newCommitIndex = raftLog.getLastCommittedIndex(); - if (logMetadataEnabled) { - logMetadata(newCommitIndex); - } - commitIndexChanged(); + long lastCommitIndex = RaftLog.INVALID_LOG_INDEX; boolean hasConfiguration = false; for (LogEntryHeader entry : entriesToCommit) { @@ -958,7 +955,14 @@ class LeaderStateImpl implements LeaderState { } hasConfiguration |= entry.getLogEntryBodyCase() == LogEntryBodyCase.CONFIGURATIONENTRY; raftLog.getRaftLogMetrics().onLogEntryCommitted(entry); + if (entry.getLogEntryBodyCase() != LogEntryBodyCase.METADATAENTRY) { + lastCommitIndex = entry.getIndex(); + } + } + if (logMetadataEnabled && lastCommitIndex != RaftLog.INVALID_LOG_INDEX) { + logMetadata(lastCommitIndex); } + commitIndexChanged(); if (hasConfiguration) { checkAndUpdateConfiguration(); } @@ -978,8 +982,9 @@ class LeaderStateImpl implements LeaderState { } private void logMetadata(long commitIndex) { - raftLog.appendMetadata(currentTerm, commitIndex); - notifySenders(); + if (raftLog.appendMetadata(currentTerm, commitIndex) != RaftLog.INVALID_LOG_INDEX) { + notifySenders(); + } } private void checkAndUpdateConfiguration() { 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 14851b1a4..d241bcd20 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 @@ -237,21 +237,10 @@ public abstract class RaftLogBase implements RaftLog { if (newCommitIndex <= 0) { // do not log the first conf entry return false; - } else if (Optional.ofNullable(lastMetadataEntry.get()) - .filter(e -> e.getIndex() == newCommitIndex || e.getMetadataEntry().getCommitIndex() >= newCommitIndex) - .isPresent()) { - //log neither lastMetadataEntry, nor entries with a smaller commit index. - return false; - } - try { - if (get(newCommitIndex).hasMetadataEntry()) { - // do not log the metadata entry - return false; - } - } catch(RaftLogIOException e) { - LOG.error("Failed to get log entry for index " + newCommitIndex, e); } - return true; + final LogEntryProto last = lastMetadataEntry.get(); + // do not log entries with a smaller commit index. + return last == null || newCommitIndex > last.getMetadataEntry().getCommitIndex(); } @Override
