RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite.#1446
RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite.#1446slfan1989 wants to merge 1 commit intoapache:RATIS-1931_grpc-zero-copyfrom
Conversation
szetszwo
left a comment
There was a problem hiding this comment.
@slfan1989 , this problem probably has already be fixed in the master branch. Since the difference between two branches are huge. I suggest doing a merge/rebase first.
- RATIS-1931_grpc-zero-copy 238 commits ahead of and 350 commits behind master.
| client.async().sendReadAfterWrite(queryMessage).thenAccept(reply -> { | ||
| Assertions.assertEquals(2, retrieve(reply)); | ||
| }); | ||
| client.async().send(incrementMessage).get(); |
There was a problem hiding this 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,
- initial: X = 1
- writeAsync: set X = 3
- 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.
There was a problem hiding this 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.
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:
What is the link to the Apache JIRA
RATIS-2514. Fix flaky TestReadOnlyRequestWithGrpc.testReadAfterWrite.
How was this patch tested?
Tested with:
Result: