-
Notifications
You must be signed in to change notification settings - Fork 444
RATIS-2559. Add linearizable check for streaming read request #1490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
eed01a4
2704558
4509555
ed2f358
8bb145c
dbf7594
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ | |
| import org.apache.ratis.proto.RaftProtos.RoleInfoProto; | ||
| import org.apache.ratis.proto.RaftProtos.StartLeaderElectionReplyProto; | ||
| import org.apache.ratis.proto.RaftProtos.StartLeaderElectionRequestProto; | ||
| import org.apache.ratis.protocol.ClientId; | ||
| import org.apache.ratis.protocol.ClientInvocationId; | ||
| import org.apache.ratis.protocol.GroupInfoReply; | ||
| import org.apache.ratis.protocol.GroupInfoRequest; | ||
|
|
@@ -260,6 +261,8 @@ public long[] getFollowerMatchIndices() { | |
| private final ExecutorService clientExecutor; | ||
| private final ThreadGroup threadGroup; | ||
|
|
||
| private final CompletableFuture<RaftClientReply> dummySuccessReply; | ||
|
|
||
| RaftServerImpl(RaftGroup group, StateMachine stateMachine, RaftServerProxy proxy, RaftStorage.StartupOption option) | ||
| throws IOException { | ||
| final RaftPeerId id = proxy.getId(); | ||
|
|
@@ -303,6 +306,13 @@ public long[] getFollowerMatchIndices() { | |
| RaftServerConfigKeys.ThreadPool.clientSize(properties), | ||
| id + "-client"); | ||
| this.threadGroup = new ThreadGroup(proxy.getThreadGroup(), getMemberId().toString()); | ||
|
|
||
| this.dummySuccessReply = CompletableFuture.completedFuture(RaftClientReply.newBuilder() | ||
| .setClientId(ClientId.emptyClientId()) | ||
| .setServerId(id) | ||
| .setGroupId(group.getGroupId()) | ||
| .setSuccess() | ||
| .build()); | ||
| } | ||
|
|
||
| private long getCommitIndex(RaftPeerId id) { | ||
|
|
@@ -1113,11 +1123,12 @@ private CompletableFuture<RaftClientReply> readAsync(RaftClientRequest request) | |
| if (request.getType().getRead().getPreferNonLinearizable() | ||
| || readOption == RaftServerConfigKeys.Read.Option.DEFAULT) { | ||
| final CompletableFuture<RaftClientReply> reply = checkLeaderState(request); | ||
| if (reply != null) { | ||
| return reply; | ||
| } | ||
| return queryStateMachine(request); | ||
| } else if (readOption == RaftServerConfigKeys.Read.Option.LINEARIZABLE){ | ||
| if (reply != null) { | ||
| return reply; | ||
| } | ||
| return isDummyRead(request) ? CompletableFuture.completedFuture(newSuccessReply(request)) | ||
| : queryStateMachine(request); | ||
|
Comment on lines
+1129
to
+1130
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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);
} |
||
| } else if (readOption == RaftServerConfigKeys.Read.Option.LINEARIZABLE) { | ||
| final LeaderStateImpl leader = role.getLeaderState().orElse(null); | ||
| final CompletableFuture<Long> replyFuture; | ||
| if (leader != null) { | ||
|
|
@@ -1136,12 +1147,17 @@ private CompletableFuture<RaftClientReply> readAsync(RaftClientRequest request) | |
| return replyFuture | ||
| .thenCompose(readIndex -> getState().getReadRequests().waitToAdvance(readIndex, | ||
| () -> getReadException("add", snapshotInstallationHandler.getInProgressInstallSnapshotIndex(), false))) | ||
| .thenCompose(readIndex -> queryStateMachine(request)) | ||
| .thenCompose(readIndex -> isDummyRead(request) | ||
| ? CompletableFuture.completedFuture(newSuccessReply(request)) : queryStateMachine(request)) | ||
| .exceptionally(e -> readException2Reply(request, e)); | ||
| } else { | ||
| throw new IllegalStateException("Unexpected read option: " + readOption); | ||
| } | ||
| } | ||
| private static boolean isDummyRead(RaftClientRequest request) { | ||
| return request.getMessage() != null && OrderedAsync.DUMMY.getContent().equals(request.getMessage().getContent()); | ||
| } | ||
|
Comment on lines
+1157
to
+1159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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;
} |
||
|
|
||
| private RaftClientReply readException2Reply(RaftClientRequest request, Throwable e) { | ||
| e = JavaUtils.unwrapCompletionException(e); | ||
| if (e instanceof StateMachineException ) { | ||
|
|
@@ -1183,6 +1199,9 @@ private CompletableFuture<RaftClientRequest> streamEndOfRequestAsync(RaftClientR | |
| } | ||
|
|
||
| CompletableFuture<RaftClientReply> queryStateMachine(RaftClientRequest request) { | ||
| if (request.getType().getRead().getDummy()) { | ||
| return dummySuccessReply; | ||
| } | ||
| return processQueryFuture(stateMachine.query(request.getMessage()), request); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is to run the linearizable check; see #1469 (comment)