You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "slfan1989 (via GitHub)" <gi...@apache.org> on 2026/05/05 08:02:16 UTC

[PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

slfan1989 opened a new pull request, #1446:
URL: https://github.com/apache/ratis/pull/1446

   ## What changes were proposed in this pull request?
   
   This pull request fixes a flaky assertion in `TestReadOnlyRequestWithGrpc.testReadAfterWrite`.
   
   The previous test sent multiple async writes without waiting for them to complete, then issued a linearizable read and a read-after-write request concurrently. It asserted that the read-after-write result should be greater than or equal to the concurrent linearizable read result.
   
   That assumption is unreliable because the two reads may have different linearization points and may observe different subsets of concurrent writes.
   
   This patch keeps the original async write workload and both read paths, but makes the test deterministic by:
   - waiting for the async writes to complete before issuing the reads;
   - verifying that both linearizable read and read-after-write observe the completed prior writes;
   - waiting for followers to catch up before cluster shutdown to avoid shutdown-time leak detection races.
   
   
   ## What is the link to the Apache JIRA
   
   RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite.
   
   ## How was this patch tested?
   
   ```
   ./mvnw -pl ratis-test -am -Pgrpc-tests -Dtest=TestReadOnlyRequestWithGrpc#testReadAfterWrite test
   ```


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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1446:
URL: https://github.com/apache/ratis/pull/1446#issuecomment-4949368463

   This PR has been marked as stale due to 60 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in ~30 days.


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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #1446: RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite.
URL: https://github.com/apache/ratis/pull/1446


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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #1446:
URL: https://github.com/apache/ratis/pull/1446#discussion_r3192577009


##########
ratis-server/src/test/java/org/apache/ratis/ReadOnlyRequestTests.java:
##########
@@ -299,20 +300,40 @@ private void testReadAfterWriteImpl(CLUSTER cluster) throws Exception {
       Assertions.assertEquals(1, retrieve(blockReply));
 
       // test asynchronous read-after-write
-      client.async().send(incrementMessage);
-      client.async().sendReadAfterWrite(queryMessage).thenAccept(reply -> {
-        Assertions.assertEquals(2, retrieve(reply));
-      });
+      client.async().send(incrementMessage).get();

Review Comment:
   @szetszwo Thanks for pointing this out! I agree that this test should preserve the async write-then-read-after-write ordering; calling get() between the two async calls would make the test ineffective for the intended read-after-write semantics.
   
   Given that `RATIS-1931_grpc-zero-copy` is currently far behind `master`, and this area may already have been fixed or moved to LinearizableReadTests on master, I agree the better next step is to merge/rebase from master first. After syncing the branch, I will re-check whether the issue still exists before making any local test changes.



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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #1446:
URL: https://github.com/apache/ratis/pull/1446#discussion_r3192577009


##########
ratis-server/src/test/java/org/apache/ratis/ReadOnlyRequestTests.java:
##########
@@ -299,20 +300,40 @@ private void testReadAfterWriteImpl(CLUSTER cluster) throws Exception {
       Assertions.assertEquals(1, retrieve(blockReply));
 
       // test asynchronous read-after-write
-      client.async().send(incrementMessage);
-      client.async().sendReadAfterWrite(queryMessage).thenAccept(reply -> {
-        Assertions.assertEquals(2, retrieve(reply));
-      });
+      client.async().send(incrementMessage).get();

Review Comment:
   @szetszwo Thanks for pointing this out! I agree that this test should preserve the async write-then-read-after-write ordering; calling get() between the two async calls would make the test ineffective for the intended read-after-write semantics.
   
   Given that `RATIS-1931_grpc-zero-copy` is currently far behind master, and this area may already have been fixed or moved to LinearizableReadTests on master, I agree the better next step is to merge/rebase from master first. After syncing the branch, I will re-check whether the issue still exists before making any local test changes.



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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1446:
URL: https://github.com/apache/ratis/pull/1446#discussion_r3189695705


##########
ratis-server/src/test/java/org/apache/ratis/ReadOnlyRequestTests.java:
##########
@@ -299,20 +300,40 @@ private void testReadAfterWriteImpl(CLUSTER cluster) throws Exception {
       Assertions.assertEquals(1, retrieve(blockReply));
 
       // test asynchronous read-after-write
-      client.async().send(incrementMessage);
-      client.async().sendReadAfterWrite(queryMessage).thenAccept(reply -> {
-        Assertions.assertEquals(2, retrieve(reply));
-      });
+      client.async().send(incrementMessage).get();

Review Comment:
   The sendReadAfterWrite feature is for two async calls, (1) writeAsync and then (2) readAfterWriteAsync such that (2) must be able see the change by (1).  So, we cannot call get() here.  Otherwise, the test becomes useless.  For exampe,
   
   0. initial: X  = 1
   1. writeAsync: set X = 3
   2. readAfterWriteAsync: must see X = 3 (it is a bug if it can see X = 1)
   
   This may be a bug already fixed (and moved to LinearizableReadTests) in the master branch.



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


Re: [PR] RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite. [ratis]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1446:
URL: https://github.com/apache/ratis/pull/1446#issuecomment-5304906350

   Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.


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