RATIS-2559. Add linearizable check for streaming read request#1490
RATIS-2559. Add linearizable check for streaming read request#1490peterxcli wants to merge 1 commit into
Conversation
Signed-off-by: peterxcli <peterxcli@gmail.com>
| if (reply != null) { | ||
| return reply; | ||
| } | ||
| return isDummyRead(request) ? CompletableFuture.completedFuture(newSuccessReply(request)) |
There was a problem hiding this comment.
Just to learn the context, why do we need this dummy read in Ratis streaming read while we do not have it for Ratis streaming write?
There was a problem hiding this comment.
It is to run the linearizable check; see #1469 (comment)
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@peterxcli , thanks for working on this!
Please see the comments inlined and also https://issues.apache.org/jira/secure/attachment/13082966/1490_review.patch
| private static boolean isDummyRead(RaftClientRequest request) { | ||
| return request.getMessage() != null && OrderedAsync.DUMMY.getContent().equals(request.getMessage().getContent()); | ||
| } |
There was a problem hiding this comment.
We cannot use DUMMY since user applications can set any message. They may already have used it in their messages. We probably need to add a flag to the proto:
+++ b/ratis-proto/src/main/proto/Raft.proto
@@ -302,6 +302,7 @@ message ForwardRequestTypeProto {
message ReadRequestTypeProto {
bool preferNonLinearizable = 1;
bool readAfterWriteConsistent = 2;
+ bool dummy = 3;
}| return isDummyRead(request) ? CompletableFuture.completedFuture(newSuccessReply(request)) | ||
| : queryStateMachine(request); |
There was a problem hiding this comment.
Let's add a DUMMY_SUCCESS_REPLY and move it to queryStateMachine:
static final CompletableFuture<RaftClientReply> DUMMY_SUCCESS_REPLY
= CompletableFuture.completedFuture(RaftClientReply.newBuilder().setSuccess().build()); CompletableFuture<RaftClientReply> queryStateMachine(RaftClientRequest request) {
if (request.getType().getRead().getDummy()) {
return DUMMY_SUCCESS_REPLY;
}
return processQueryFuture(stateMachine.query(request.getMessage()), request);
}| final RaftClientReply terminalReply = toReadStreamReply(request, reply); | ||
| if (!reply.isSuccess()) { | ||
| ctx.writeAndFlush(newDataStreamReplyByteBuffer(requestBuf, terminalReply)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Since server returns a DUMMY_SUCCESS_REPLY, we could build the failed reply directly:
if (!readCheckReply.isSuccess()) {
final RaftClientReply reply = RaftClientReply.newBuilder()
.setRequest(request)
.setSuccess(false)
.build();
ctx.writeAndFlush(newDataStreamReplyByteBuffer(requestBuf, reply));
return;
}
final ReadStream stream = new ReadStream(request, requestBuf.getStreamId(), ctx);| final RaftClientRequest.Builder builder = RaftClientRequest.newBuilder() | ||
| .setClientId(request.getClientId()) | ||
| .setGroupId(request.getRaftGroupId()) | ||
| .setCallId(request.getCallId()) | ||
| .setMessage(OrderedAsync.DUMMY) | ||
| .setType(request.getType()) | ||
| .setRepliedCallIds(request.getRepliedCallIds()) | ||
| .setSlidingWindowEntry(request.getSlidingWindowEntry()) | ||
| .setRoutingTable(request.getRoutingTable()) | ||
| .setTimeoutMs(request.getTimeoutMs()) | ||
| .setSpanContext(request.getSpanContext()); | ||
| if (request.isToLeader()) { | ||
| builder.setLeaderId(request.getServerId()); | ||
| } else { | ||
| builder.setServerId(request.getServerId()); | ||
| } | ||
| return builder.build(); |
There was a problem hiding this comment.
Let's add a set(RaftClientRequest request) to Builder to copy everything.
3 participants
What changes were proposed in this pull request?
ReadStreamManagementto perform an asynchronous linearizability check using a "dummy read" request before invoking the state machine for read-only queries. The state machine query is only performed if the check succeeds, and an appropriate reply is sent if it fails.RaftServerImplto recognize and handle dummy read requests by returning a success reply immediately, avoiding unnecessary state machine queries.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2559
How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)