-
Notifications
You must be signed in to change notification settings - Fork 447
RATIS-2509. Introduce local read API to reduce serde cost #1448
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
base: master
Are you sure you want to change the base?
Changes from all commits
d7b82e5
6b1c00b
a5a54fa
f92f163
714931d
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 |
|---|---|---|
|
|
@@ -37,12 +37,14 @@ | |
| import org.apache.ratis.proto.RaftProtos.RaftRpcRequestProto; | ||
| import org.apache.ratis.proto.RaftProtos.ReadIndexReplyProto; | ||
| import org.apache.ratis.proto.RaftProtos.ReadIndexRequestProto; | ||
| import org.apache.ratis.proto.RaftProtos.ReadRequestTypeProto; | ||
| import org.apache.ratis.proto.RaftProtos.ReplicationLevel; | ||
| import org.apache.ratis.proto.RaftProtos.RequestVoteReplyProto; | ||
| import org.apache.ratis.proto.RaftProtos.RequestVoteRequestProto; | ||
| 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; | ||
|
|
@@ -1099,7 +1101,12 @@ private CompletableFuture<ReadIndexReplyProto> sendReadIndexAsync(RaftClientRequ | |
| if (leaderId == null) { | ||
| return JavaUtils.completeExceptionally(new ReadIndexException(getMemberId() + ": Leader is unknown.")); | ||
| } | ||
| final ReadIndexRequestProto request = toReadIndexRequestProto(clientRequest, getMemberId(), leaderId); | ||
| return sendReadIndexAsync(clientRequest.getClientId(), clientRequest.getType().getRead(), leaderId); | ||
| } | ||
|
|
||
| private CompletableFuture<ReadIndexReplyProto> sendReadIndexAsync( | ||
| ClientId clientId, ReadRequestTypeProto readRequestType, RaftPeerId leaderId) { | ||
| final ReadIndexRequestProto request = toReadIndexRequestProto(clientId, readRequestType, getMemberId(), leaderId); | ||
| try { | ||
| return getServerRpc().async().readIndexAsync(request); | ||
| } catch (IOException e) { | ||
|
|
@@ -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); | ||
| } | ||
|
|
||
| final long installSnapshot = snapshotInstallationHandler.getInProgressInstallSnapshotIndex(); | ||
| if (installSnapshot != RaftLog.INVALID_LOG_INDEX) { | ||
| return JavaUtils.completeExceptionally(getReadException("get", installSnapshot, false)); | ||
| } | ||
|
|
||
| final RaftPeerId leaderId = getInfo().getLeaderId(); | ||
| if (leaderId == null) { | ||
| return JavaUtils.completeExceptionally(new ReadIndexException(getMemberId() + ": Leader is unknown.")); | ||
| } | ||
|
|
||
| return getReadIndex(sendReadIndexAsync(clientId, readRequestType, leaderId)); | ||
| } | ||
|
|
||
| private <T> CompletableFuture<T> checkLeaderStateForReadOnly() { | ||
| if (!getInfo().isLeader()) { | ||
| return JavaUtils.completeExceptionally(generateNotLeaderException()); | ||
| } | ||
| if (!getInfo().isLeaderReady()) { | ||
| return JavaUtils.completeExceptionally(new LeaderNotReadyException(getMemberId())); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private static <T> CompletableFuture<T> supplyReadOnly(Supplier<CompletableFuture<T>> query) { | ||
| try { | ||
| return Objects.requireNonNull(query.get(), "query returned null"); | ||
| } catch (Throwable t) { | ||
| return JavaUtils.completeExceptionally(t); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public <T> CompletableFuture<T> readOnlyAsync(ClientId clientId, ReadRequestTypeProto readRequestType, | ||
| Supplier<CompletableFuture<T>> query) throws IOException { | ||
| Objects.requireNonNull(clientId, "clientId == null"); | ||
| Objects.requireNonNull(readRequestType, "readRequestType == null"); | ||
| Objects.requireNonNull(query, "query == null"); | ||
| assertLifeCycleState(LifeCycle.States.RUNNING); | ||
| if (readRequestType.getPreferNonLinearizable() || readOption == RaftServerConfigKeys.Read.Option.DEFAULT) { | ||
| final CompletableFuture<T> reply = checkLeaderStateForReadOnly(); | ||
| return reply != null ? reply : supplyReadOnly(query); | ||
| } else if (readOption == RaftServerConfigKeys.Read.Option.LINEARIZABLE) { | ||
| return getReadIndexForReadOnly(clientId, readRequestType) | ||
| .thenCompose(readIndex -> getState().getReadRequests().waitToAdvance(readIndex, | ||
| () -> getReadException("add", snapshotInstallationHandler.getInProgressInstallSnapshotIndex(), false))) | ||
| .thenCompose(readIndex -> supplyReadOnly(query)); | ||
|
Comment on lines
+1178
to
+1181
Contributor
Author
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. I wonder whether it is better to use thenComposeAsync to improve performance. return getReadIndexForReadOnly(clientId, readRequestType)
.thenComposeAsync(readIndex -> getReadRequests().waitToAdvance(readIndex), clientExecutor)
.thenComposeAsync(readIndex -> supplyReadOnly(query), clientExecutor);or just the supplyReadOnly return getReadIndexForReadOnly(clientId, readRequestType)
.thenCompose(readIndex -> getReadRequests().waitToAdvance(readIndex))
.thenComposeAsync(readIndex -> supplyReadOnly(query), clientExecutor);Seems currently with Let me benchmark it. Can also be applied to
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 should use clientExecutor as earliest as possible. It will keep using the executor for the later calls; see below: public static void main(String[] args) {
final ExecutorService poolA = Executors.newCachedThreadPool(ConcurrentUtils.newThreadFactory("pool-A"));
final ExecutorService poolB = Executors.newCachedThreadPool(ConcurrentUtils.newThreadFactory("pool-B"));
printCurrentThread("main");
CompletableFuture.supplyAsync(() -> printCurrentThread("supplyAsync"), poolA)
.thenCompose(s -> CompletableFuture.completedFuture(printCurrentThread("thenCompose after " + s)))
.thenComposeAsync(s -> CompletableFuture.completedFuture(printCurrentThread("thenComposeAsync after " + s)), poolB)
.thenCompose(s -> CompletableFuture.completedFuture(printCurrentThread("thenCompose again after " + s)))
;
}
static String printCurrentThread(String label) {
final String name = Thread.currentThread().getName();
System.out.printf("%40s: %s%n", label, name);
return name;
} |
||
| } else { | ||
| throw new IllegalStateException("Unexpected read option: " + readOption); | ||
| } | ||
| } | ||
|
|
||
| private CompletableFuture<RaftClientReply> readAsync(RaftClientRequest request) { | ||
| if (request.getType().getRead().getPreferNonLinearizable() | ||
| || readOption == RaftServerConfigKeys.Read.Option.DEFAULT) { | ||
|
|
@@ -1123,14 +1198,7 @@ private CompletableFuture<RaftClientReply> readAsync(RaftClientRequest request) | |
| if (leader != null) { | ||
| replyFuture = getReadIndex(request, leader); | ||
| } else { | ||
| replyFuture = sendReadIndexAsync(request).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)); | ||
| } | ||
| }); | ||
| replyFuture = getReadIndex(sendReadIndexAsync(request)); | ||
| } | ||
|
|
||
| return replyFuture | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,9 @@ | |
| import org.apache.ratis.client.RaftClient; | ||
| import org.apache.ratis.conf.RaftProperties; | ||
| import org.apache.ratis.proto.RaftProtos.LogEntryProto; | ||
| import org.apache.ratis.protocol.ClientId; | ||
| import org.apache.ratis.protocol.Message; | ||
| import org.apache.ratis.protocol.RaftClientRequest; | ||
| import org.apache.ratis.protocol.RaftClientReply; | ||
| import org.apache.ratis.protocol.RaftPeerId; | ||
| import org.apache.ratis.protocol.exceptions.RaftRetryFailureException; | ||
|
|
@@ -42,11 +44,14 @@ | |
|
|
||
| import java.lang.reflect.Field; | ||
| import java.lang.reflect.Method; | ||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.List; | ||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.CompletionException; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
| import java.util.function.Supplier; | ||
|
|
||
| public abstract class ReadOnlyRequestTests<CLUSTER extends MiniRaftCluster> | ||
| extends BaseTest | ||
|
|
@@ -85,15 +90,28 @@ public void testReadOnly() throws Exception { | |
|
|
||
| static <C extends MiniRaftCluster> void runTestReadOnly(C cluster) throws Exception { | ||
| try { | ||
| RaftTestUtil.waitForLeader(cluster); | ||
| final RaftPeerId leaderId = cluster.getLeader().getId(); | ||
| final RaftServer.Division leader = RaftTestUtil.waitForLeader(cluster); | ||
| final RaftPeerId leaderId = leader.getId(); | ||
|
|
||
| try (final RaftClient client = cluster.createClient(leaderId)) { | ||
| for (int i = 1; i <= 10; i++) { | ||
| assertReplyExact(i, client.io().send(INCREMENT)); | ||
| assertReplyExact(i, client.io().sendReadOnly(QUERY)); | ||
| Assertions.assertEquals(i, readOnlyAsync( | ||
| leader, () -> CompletableFuture.completedFuture(getCount(leader))).get()); | ||
| } | ||
| } | ||
|
|
||
| if (RaftServerConfigKeys.Read.option(cluster.getProperties()) == RaftServerConfigKeys.Read.Option.DEFAULT) { | ||
| final RaftServer.Division follower = cluster.getFollowers().get(0); | ||
| final AtomicBoolean callbackInvoked = new AtomicBoolean(); | ||
| final CompletableFuture<Long> read = readOnlyAsync(follower, () -> { | ||
| callbackInvoked.set(true); | ||
| return CompletableFuture.completedFuture(getCount(follower)); | ||
| }); | ||
| Assertions.assertThrows(CompletionException.class, read::join); | ||
| Assertions.assertFalse(callbackInvoked.get()); | ||
| } | ||
| } finally { | ||
| cluster.shutdown(); | ||
| } | ||
|
|
@@ -218,6 +236,20 @@ static int retrieve(RaftClientReply reply) { | |
| return Integer.parseInt(reply.getMessage().getContent().toString(StandardCharsets.UTF_8)); | ||
| } | ||
|
|
||
| static long getCount(RaftServer.Division server) { | ||
| return ((CounterStateMachine) server.getStateMachine()).getCount(); | ||
| } | ||
|
|
||
| static <T> CompletableFuture<T> readOnlyAsync( | ||
| RaftServer.Division server, Supplier<CompletableFuture<T>> query) throws IOException { | ||
| return server.readOnlyAsync(ClientId.randomId(), RaftClientRequest.readRequestType().getRead(), query); | ||
|
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. Just one suggestion. Maybe we can overload the Division interface with something like: so that callers can avoid importing |
||
| } | ||
|
|
||
| static <T> CompletableFuture<T> readOnlyAsyncPreferNonLinearizable( | ||
| RaftServer.Division server, Supplier<CompletableFuture<T>> query) throws IOException { | ||
| return server.readOnlyAsync(ClientId.randomId(), RaftClientRequest.readRequestType(true).getRead(), query); | ||
| } | ||
|
|
||
| public static void assertReplyExact(int expectedCount, RaftClientReply reply) { | ||
| Assertions.assertTrue(reply.isSuccess()); | ||
| final int retrieved = retrieve(reply); | ||
|
|
@@ -231,6 +263,11 @@ static void assertReplyAtLeast(int minCount, RaftClientReply reply) { | |
| () -> "retrieved = " + retrieved + " < minCount = " + minCount + ", reply=" + reply); | ||
| } | ||
|
|
||
| public static void assertLongAtLeast(int minCount, long retrieved) { | ||
| Assertions.assertTrue(retrieved >= minCount, | ||
| () -> "retrieved = " + retrieved + " < minCount = " + minCount); | ||
| } | ||
|
|
||
| /** | ||
| * CounterStateMachine support 3 operations | ||
| * 1. increment | ||
|
|
||
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.
Isn't this going to bypass the
writeIndexCache?Say if a caller constructs a
ReadRequestTypeProtowithreadAfterWriteConsistentas true and passes it toreadOnlyAsync, isn't the flag getting ignored when the node is the leader?Maybe we can add a Javadoc saying
readAfterWriteConsistentis not supported here, or we can throw an exception likeIllegalArgumentExceptionin casereadAfterWriteConsistentflag is set so that we get some visibility into this behaviour?