[fix][test] Fix flaky testCreateTopicWithZombieReplicatorCursor #20037
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Before this patch: After this patch: Example logs of updating replication clusters: We can see the interval is only 4 milliseconds but they happened in different threads ( |
BewareMyPower
changed the title
poorbarcode
reviewed
Apr 11, 2023
| @@ -1542,7 +1542,13 @@ public CompletableFuture<Void> checkReplication() { | |||
| continue; | |||
| } | |||
| if (!replicators.containsKey(cluster)) { | |||
| futures.add(startReplicator(cluster)); | |||
| futures.add(checkReplicationCluster(cluster).thenCompose(clusterExists -> { | |||
There was a problem hiding this comment.
The method checkReplicationCluster is guarantee that the remote cluster is still in the collection topicPolicies. But the line-1524 and line-1540 also did this
There was a problem hiding this comment.
Yes, it's not related to the flaky test. I've reverted it and changed the PR description. The tests could still pass after that.
Fixes apache#20010 ### Motivation `PersistentTopicTest.testCreateTopicWithZombieReplicatorCursor` is flaky because `onPoliciesUpdate` is asynchronous, while `testCreateTopicWithZombieReplicatorCursor` updates the namespace policy nearly the same time, so there is a race with the order of updating `AbstractTopic#topicPolicies`. Sometimes the policies update might fail because the topic might be deleted in `PersistentTopic#checkReplication`: > Deleting topic [xxx] because local cluster is not part of global namespace repl list [remote] ### Modifications - Sleep 100ms between two calls of updating the replication clusters - Add the local cluster to the replication cluster list - Add the retry logic for `initialize`
BewareMyPower
changed the title
BewareMyPower
force-pushed
the
bewaremypower/issue-20010
branch
from
f8a5806 to
bb254f1
Compare
Codecov Report
@@ Coverage Diff @@
## master #20037 +/- ##
=============================================
+ Coverage 34.65% 72.94% +38.28%
- Complexity 12429 31840 +19411
=============================================
Files 1606 1865 +259
Lines 125026 138040 +13014
Branches 13667 15167 +1500
=============================================
+ Hits 43332 100693 +57361
+ Misses 76081 29364 -46717
- Partials 5613 7983 +2370
Flags with carried forward coverage won't be shown. Click here to find out more.
|
cbornet
approved these changes
Apr 11, 2023
|
As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label |
liangyepianzhou
pushed a commit
that referenced
this pull request
Jul 7, 2023
(cherry picked from commit f2076b4)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #20010
Motivation
PersistentTopicTest.testCreateTopicWithZombieReplicatorCursoris flaky becauseonPoliciesUpdateis asynchronous, whiletestCreateTopicWithZombieReplicatorCursorupdates the namespace policy nearly the same time, so there is a race with the order of updatingAbstractTopic#topicPolicies.Sometimes the policies update might fail because the topic might be deleted in
PersistentTopic#checkReplication:Modifications
initializeDocumentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: BewareMyPower#24