Skip to content

Conversation

@jianghuazhu
Copy link
Contributor

What changes were proposed in this pull request?

In the Ratis project and other projects, the default value of TermIndex is used. Here we create a default value for TermIndex.

What is the link to the Apache JIRA

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

How was this patch tested?

Make sure UT passes.

@jianghuazhu
Copy link
Contributor Author

@szetszwo , can you help review this PR?
Thanks

return Optional.ofNullable(proto).map(p -> valueOf(p.getTerm(), p.getIndex())).orElse(null);
}

static final TermIndex DEFAULT_TERMINDEX = valueOf(0, -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

@jianghuazhu , thanks a lot for working on this!

Let's call it INITIAL_VALUE, move it to the top and add some javadoc.

BTW, could you also add javadoc for EMPTY_ARRAY?

+++ b/ratis-server-api/src/main/java/org/apache/ratis/server/protocol/TermIndex.java
@@ -19,12 +19,25 @@ package org.apache.ratis.server.protocol;
 
 import org.apache.ratis.proto.RaftProtos.LogEntryProto;
 import org.apache.ratis.proto.RaftProtos.TermIndexProto;
+import org.apache.ratis.server.raftlog.RaftLog;
 
 import java.util.Comparator;
 import java.util.Optional;
 
 /** The term and the log index defined in the Raft consensus algorithm. */
 public interface TermIndex extends Comparable<TermIndex> {
+  /**
+   * The initial value.
+   * When a new Raft group starts,
+   * all the servers has term 0 and index -1 (= {@link RaftLog#INVALID_LOG_INDEX}).
+   * Note that term is incremented during leader election
+   * and index is incremented when writing to the {@link RaftLog}.
+   * The least term and index possibly written to the {@link RaftLog}
+   * are respectively 1 and 0 (= {@link RaftLog#LEAST_VALID_LOG_INDEX}).
+   */
+  TermIndex INITIAL_VALUE = valueOf(0, RaftLog.INVALID_LOG_INDEX);
+
+  /** An empty {@link TermIndex} array. */
   TermIndex[] EMPTY_ARRAY = {};
 
   /** @return the term. */

Copy link
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.

@jianghuazhu
Copy link
Contributor Author

Thanks @szetszwo for the comment and review.
Here are some CI run failures that don't seem to be related to this PR.
Here are my tests for my personal branch:
https://github.com/jianghuazhu/ratis/actions/runs/11165969820

@szetszwo
Copy link
Contributor

szetszwo commented Oct 4, 2024

@jianghuazhu , agree that the failures were not related to this.

@szetszwo szetszwo merged commit a0c4255 into apache:master Oct 4, 2024
@jianghuazhu
Copy link
Contributor Author

jianghuazhu commented Oct 5, 2024

Thanks @szetszwo .
I created a related issue in the Ozone project, HDDS-11529.
I will resolve it when the next new version of Apache Ratis is imported.

szetszwo pushed a commit to szetszwo/ratis that referenced this pull request Mar 30, 2025
szetszwo pushed a commit to szetszwo/ratis that referenced this pull request May 23, 2025
szetszwo pushed a commit to szetszwo/ratis that referenced this pull request May 23, 2025
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.

2 participants