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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
