Re: [PR] RATIS-2509. Introduce local read API to reduce serde cost [ratis]

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

spacemonkd commented on code in PR #1448:
URL: https://github.com/apache/ratis/pull/1448#discussion_r3613569013


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -1109,6 +1116,74 @@ private CompletableFuture<ReadIndexReplyProto> sendReadIndexAsync(RaftClientRequ
   private CompletableFuture<Long> getReadIndex(RaftClientRequest request, LeaderStateImpl leader) {
     return writeIndexCache.getWriteIndexFuture(request).thenCompose(leader::getReadIndex);
   }
+  private CompletableFuture<Long> getReadIndex(CompletableFuture<ReadIndexReplyProto> readIndexReply) {
+    return readIndexReply.thenApply(reply -> {
+      if (reply.getServerReply().getSuccess()) {
+        return reply.getReadIndex();
+      } else {
+        throw new CompletionException(new ReadIndexException(getId()
+            + ": Failed to get read index from the leader: " + reply));
+      }
+    });
+  }
+
+  private CompletableFuture<Long> getReadIndexForReadOnly(ClientId clientId, ReadRequestTypeProto readRequestType) {
+    final LeaderStateImpl leader = role.getLeaderState().orElse(null);
+    if (leader != null) {
+      return leader.getReadIndex(null);

Review Comment:
   Isn't this going to bypass the `writeIndexCache`?
   Say if a caller constructs a `ReadRequestTypeProto` with `readAfterWriteConsistent` as true and passes it to `readOnlyAsync`, isn't the flag getting ignored when the node is the leader?
   
   Maybe we can add a Javadoc saying `readAfterWriteConsistent` is not supported here, or we can throw an exception like `IllegalArgumentException` in case `readAfterWriteConsistent` flag is set so that we get some visibility into this behaviour?



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