szetszwo commented on code in PR #1322:
URL: https://github.com/apache/ratis/pull/1322#discussion_r2586709748


##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogCache.java:
##########
@@ -610,7 +614,7 @@ long getLastIndexInClosedSegments() {
 
   TermIndex getLastTermIndex() {
     try (AutoCloseableLock readLock = closedSegments.readLock()) {
-      return (openSegment != null && openSegment.numOfEntries() > 0) ?
+      return (openSegment != null && openSegment.getLastTermIndex() != null) ?
           openSegment.getLastTermIndex() :
           (closedSegments.isEmpty() ? null :
               closedSegments.get(closedSegments.size() - 
1).getLastTermIndex());

Review Comment:
   We should get rid of the readLock here:
   ```java
     TermIndex getLastTermIndex() {
       final LogSegment open = openSegment;
       if (open != null) {
         final TermIndex last = open.getLastTermIndex();
         if (last != null) {
           return last;
         }
       }
       final LogSegment last = closedSegments.getLast();
       return last != null ? last.getLastTermIndex() : null;
     }
   ```



##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java:
##########
@@ -387,7 +424,8 @@ long getTotalCacheSize() {
   synchronized void truncate(long fromIndex) {

Review Comment:
   It should be synchronized since this is a bulk operation.



##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java:
##########
@@ -293,15 +333,21 @@ long getStartIndex() {
   }
 
   long getEndIndex() {
-    return endIndex;
+    if (!isOpen) {
+      return endIndex;
+    }
+    if (records.getLast() == null) {
+      return getStartIndex() - 1;
+    }
+    return records.getLast().getTermIndex().getIndex();
   }

Review Comment:
   We should not call getLast() twice:
   ```java
     long getEndIndex() {
       if (!isOpen()) {
         return endIndex;
       }
       final LogRecord last = records.getLast();
       return last == null ? getStartIndex() - 1 : 
last.getTermIndex().getIndex();
     }
   ```



-- 
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]

Reply via email to