(ratis) branch master updated: RATIS-2134. `logMetadata` could miss appending the `metadataEntry` with the lastCommitIndex (#1130)

[szetszwo]

This is an automated email from the ASF dual-hosted git repository.

szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git


The following commit(s) were added to refs/heads/master by this push:
     new 4ba463789 RATIS-2134.  `logMetadata` could miss appending the 
`metadataEntry` with the lastCommitIndex (#1130)
4ba463789 is described below

commit 4ba463789df7876837c728ce6477fd18b787fed9
Author: Chung En Lee <wfps1...@gmail.com>
AuthorDate: Tue Aug 6 22:01:31 2024 +0800

    RATIS-2134.  `logMetadata` could miss appending the `metadataEntry` with 
the lastCommitIndex (#1130)
---
 .../apache/ratis/server/impl/LeaderStateImpl.java  | 17 ++++++++++------
 .../apache/ratis/server/raftlog/RaftLogBase.java   | 23 +++-------------------
 2 files changed, 14 insertions(+), 26 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 cea8b585e..7af3c2c21 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
@@ -948,10 +948,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) {
@@ -960,7 +957,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();
     }
@@ -980,8 +984,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 4dd9fc1da..ba5dff2aa 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
@@ -241,27 +241,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;
-    }
-    ReferenceCountedObject<LogEntryProto> ref = null;
-    try {
-      ref = retainLog(newCommitIndex);
-      if (ref.get().hasMetadataEntry()) {
-        // do not log the metadata entry
-        return false;
-      }
-    } catch(RaftLogIOException e) {
-      LOG.error("Failed to get log entry for index " + newCommitIndex, e);
-    } finally {
-      if (ref != null) {
-        ref.release();
-      }
     }
-    return true;
+    final LogEntryProto last = lastMetadataEntry.get();
+    // do not log entries with a smaller commit index.
+    return last == null || newCommitIndex > 
last.getMetadataEntry().getCommitIndex();
   }
 
   @Override