[GitHub] [ratis] OneSizeFitsQuorum commented on a diff in pull request #925: RATIS-1894. Implement ReadOnly based on leader lease

["OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>]

OneSizeFitsQuorum commented on code in PR #925:
URL: https://github.com/apache/ratis/pull/925#discussion_r1335338912


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -1091,7 +1091,7 @@ public boolean checkLeadership() {
    * 4. If majority respond success, returns readIndex.
    * @return current readIndex.
    */
-  CompletableFuture<Long> getReadIndex(Long readAfterWriteConsistentIndex) {
+  CompletableFuture<Long> getReadIndex(RaftServerConfigKeys.Read.Option option, Long readAfterWriteConsistentIndex) {

Review Comment:
   It seems that with lease read we need to change the name of this function (and maybe some other functions on the call stack). The important logic for both readIndex and leaseRead is three steps.
   1. Verify that the current node is the Leader (readIndex uses a logical clock to initiate heartbeat, lease read uses a physical clock with an error upper bound),
   2. Wait for the local applyIndex to reach the current commitIndex
   3. Query the state machine.
   
   Maybe we could name the first step confirmLeader or something



##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -176,14 +176,17 @@ enum Option {
       DEFAULT,
 
       /** Use ReadIndex (see Raft Paper section 6.4). Maintains linearizability */
-      LINEARIZABLE
+      LINEARIZABLE,
+
+      /** Use Leader lease. Efficient than ReadIndex if clock drift is bounded */
+      LEASE

Review Comment:
   It seems that both DEFAULT and LINEARIZABLE are talking about consistency issues. Lease and ReadIndex are implementation techniques for implementing LINEARIZABLE, so it seems inappropriate to mention lease alone alongside the other two options?



-- 
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: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org