You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "SzyWilliam (via GitHub)" <gi...@apache.org> on 2023/09/01 04:07:43 UTC
[GitHub] [ratis] SzyWilliam opened a new pull request, #913: RATIS-1882. Support read-after-write consistency
SzyWilliam opened a new pull request, #913:
URL: https://github.com/apache/ratis/pull/913
## What changes were proposed in this pull request?
Support read-after-write consistency.
Thanks @szetszwo very much for the patch. Only made minor changes and added unit tests to the patch.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1882
## How was this patch tested?
New unit tests.
--
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
[GitHub] [ratis] tisonkun commented on pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #913:
URL: https://github.com/apache/ratis/pull/913#issuecomment-1712709142
Cool! I'm checking this out.
--
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
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1314354637
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/WriteIndexCache.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.server.impl;
+
+import org.apache.ratis.conf.RaftProperties;
+import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.protocol.RaftClientRequest;
+import org.apache.ratis.server.RaftServerConfigKeys;
+import org.apache.ratis.thirdparty.com.google.common.cache.Cache;
+import org.apache.ratis.thirdparty.com.google.common.cache.CacheBuilder;
+import org.apache.ratis.util.TimeDuration;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.atomic.AtomicReference;
+
+/** Caching the per client write index in order to support read-after-write consistency. */
+class WriteIndexCache {
+ private final Cache<ClientId, AtomicReference<CompletableFuture<Long>>> cache;
+
+ WriteIndexCache(RaftProperties properties) {
+ this(RaftServerConfigKeys.Read.ReadAfterWriteConsistent.writeIndexCacheExpiryTime(properties));
+ }
+
+ /**
+ * @param cacheExpiryTime time for a cache entry to expire.
+ */
+ WriteIndexCache(TimeDuration cacheExpiryTime) {
+ this.cache = CacheBuilder.newBuilder()
+ .expireAfterAccess(cacheExpiryTime.getDuration(), cacheExpiryTime.getUnit())
+ .build();
+ }
+
+ void add(ClientId key, CompletableFuture<Long> future) {
+ final AtomicReference<CompletableFuture<Long>> ref;
+ try {
+ ref = cache.get(key, AtomicReference::new);
+ } catch (ExecutionException e) {
+ throw new IllegalStateException(e);
+ }
+ ref.set(future);
+ }
+
+ CompletableFuture<Long> getWriteIndexFuture(RaftClientRequest request) {
+ if (request != null && request.getType().getRead().getReadAfterWriteConsistent()) {
+ final AtomicReference<CompletableFuture<Long>> ref = cache.getIfPresent(request.getClientId());
+ final CompletableFuture<Long> future = ref != null? ref.get(): null;
+ if (future != null) {
+ return future;
+ }
Review Comment:
Done
--
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
[GitHub] [ratis] szetszwo commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1313385530
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/AsyncImpl.java:
##########
@@ -51,6 +51,11 @@ public CompletableFuture<RaftClientReply> sendReadOnly(Message message, RaftPeer
return send(RaftClientRequest.readRequestType(), message, server);
}
+ @Override
+ public CompletableFuture<RaftClientReply> sendReadAfterWrite(Message message) {
+ return send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null);
Review Comment:
@SzyWilliam , Should `sendReadAfterWrite` use `UnorderedAsync.send(..)`? Indeed, all the linearizable reads should use `UnorderedAsync`.
--
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
[GitHub] [ratis] szetszwo commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1315151038
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/AsyncImpl.java:
##########
@@ -53,7 +53,7 @@ public CompletableFuture<RaftClientReply> sendReadOnly(Message message, RaftPeer
@Override
public CompletableFuture<RaftClientReply> sendReadAfterWrite(Message message) {
- return send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null);
+ return UnorderedAsync.send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null, client);
Review Comment:
Sorry, it actually not working since `UnorderedAsync` use a different channel. It can arrive the server earlier than the previous write request and get a small readIndex. `TestReadAfterWrite` will fail.
- Using unordered async
```
2023-09-04 11:16:59,888 DEBUG server.RaftServer$Division (LeaderStateImpl.java:getReadIndex(1099)) - readIndex=1, readAfterWriteConsistentIndex=1
```
- Using ordered async
```
2023-09-04 11:11:36,917 DEBUG server.RaftServer$Division (LeaderStateImpl.java:getReadIndex(1099)) - readIndex=3, readAfterWriteConsistentIndex=3
```
--
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
[GitHub] [ratis] szetszwo commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1313214237
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/WriteIndexCache.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.server.impl;
+
+import org.apache.ratis.conf.RaftProperties;
+import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.protocol.RaftClientRequest;
+import org.apache.ratis.server.RaftServerConfigKeys;
+import org.apache.ratis.thirdparty.com.google.common.cache.Cache;
+import org.apache.ratis.thirdparty.com.google.common.cache.CacheBuilder;
+import org.apache.ratis.util.TimeDuration;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.atomic.AtomicReference;
+
+/** Caching the per client write index in order to support read-after-write consistency. */
+class WriteIndexCache {
+ private final Cache<ClientId, AtomicReference<CompletableFuture<Long>>> cache;
+
+ WriteIndexCache(RaftProperties properties) {
+ this(RaftServerConfigKeys.Read.ReadAfterWriteConsistent.writeIndexCacheExpiryTime(properties));
+ }
+
+ /**
+ * @param cacheExpiryTime time for a cache entry to expire.
+ */
+ WriteIndexCache(TimeDuration cacheExpiryTime) {
+ this.cache = CacheBuilder.newBuilder()
+ .expireAfterAccess(cacheExpiryTime.getDuration(), cacheExpiryTime.getUnit())
+ .build();
+ }
+
+ void add(ClientId key, CompletableFuture<Long> future) {
+ final AtomicReference<CompletableFuture<Long>> ref;
+ try {
+ ref = cache.get(key, AtomicReference::new);
+ } catch (ExecutionException e) {
+ throw new IllegalStateException(e);
+ }
+ ref.set(future);
+ }
+
+ CompletableFuture<Long> getWriteIndexFuture(RaftClientRequest request) {
+ if (request != null && request.getType().getRead().getReadAfterWriteConsistent()) {
+ final AtomicReference<CompletableFuture<Long>> ref = cache.getIfPresent(request.getClientId());
+ final CompletableFuture<Long> future = ref != null? ref.get(): null;
+ if (future != null) {
+ return future;
+ }
Review Comment:
Just found that these few lines can be simplified as below.
```java
if (ref != null) {
return ref.get();
}
```
--
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
[GitHub] [ratis] szetszwo merged pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #913:
URL: https://github.com/apache/ratis/pull/913
--
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
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1314354856
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/AsyncImpl.java:
##########
@@ -51,6 +51,11 @@ public CompletableFuture<RaftClientReply> sendReadOnly(Message message, RaftPeer
return send(RaftClientRequest.readRequestType(), message, server);
}
+ @Override
+ public CompletableFuture<RaftClientReply> sendReadAfterWrite(Message message) {
+ return send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null);
Review Comment:
That's true! Thanks for pointing out, I'll change all the linearizable reads to use `UnorderedAsync`
--
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
[GitHub] [ratis] SzyWilliam commented on pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on PR #913:
URL: https://github.com/apache/ratis/pull/913#issuecomment-1704480264
@szetszwo Thanks very much for the patch! It's good to know `CodeInjectionForTesting` can be used for blocking. I'll try using Injection to replace the sleep time in the linearizable read index tests.
--
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
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #913:
URL: https://github.com/apache/ratis/pull/913#discussion_r1315276236
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/AsyncImpl.java:
##########
@@ -53,7 +53,7 @@ public CompletableFuture<RaftClientReply> sendReadOnly(Message message, RaftPeer
@Override
public CompletableFuture<RaftClientReply> sendReadAfterWrite(Message message) {
- return send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null);
+ return UnorderedAsync.send(RaftClientRequest.readAfterWriteConsistentRequestType(), message, null, client);
Review Comment:
You are right. Just investigated on `OrderedAsync` and `UnorderedAsync` and now I know why we should choose `OrderedAsync`. Thanks very much for your time spent and the detailed explanation!
--
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
[GitHub] [ratis] tisonkun commented on pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #913:
URL: https://github.com/apache/ratis/pull/913#issuecomment-1712709183
May I ask what is the target release version of this feature?
--
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
[GitHub] [ratis] szetszwo commented on pull request #913: RATIS-1882. Support read-after-write consistency
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #913:
URL: https://github.com/apache/ratis/pull/913#issuecomment-1716149464
We should include this to both 3.0.0 and the next branch 2 releases.
--
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