[fix][test] flaky AdminApi2Test.cleanup #17861
Merged
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
|
LGTM. |
poorbarcode
force-pushed
the
flaky/testModularLoadManagerSplitBundle
branch
from
b9c8317 to
814ba41
Compare
congbobo184
requested review from
congbobo184,
gaoran10,
eolivelli,
liangyepianzhou and
Technoboy-
poorbarcode
changed the title
|
Could you take a look? Thanks |
gaoran10
approved these changes
Oct 9, 2022
eolivelli
approved these changes
Oct 9, 2022
poorbarcode
force-pushed
the
flaky/testModularLoadManagerSplitBundle
branch
from
cc4351d to
a5c5dd8
Compare
|
rebase master |
Technoboy-
approved these changes
Oct 11, 2022
poorbarcode
changed the title
|
@poorbarcode you may change the title to |
poorbarcode
changed the title
|
Hi @tisonkun Got it! Thanks |
poorbarcode
changed the title
|
Can this PR merge? (^_^) |
| @Override | ||
| protected MetadataStoreExtended createConfigurationMetadataStore() throws MetadataStoreException { | ||
| // use MockZooKeeperSession to provide a unique session id for each instance | ||
| return new ZKMetadataStore(MockZooKeeperSession.newInstance(mockZooKeeperGlobal), MetadataStoreConfig.builder() |
There was a problem hiding this comment.
it shouldn't be better to make this the default behavior in MockedPulsarServiceBaseTest ?
There was a problem hiding this comment.
Good idea, already changed
poorbarcode
force-pushed
the
flaky/testModularLoadManagerSplitBundle
branch
from
3869e7a to
f5c5ced
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17861 +/- ##
=============================================
+ Coverage 34.91% 46.85% +11.93%
- Complexity 5707 17912 +12205
=============================================
Files 607 1573 +966
Lines 53396 128213 +74817
Branches 5712 14100 +8388
=============================================
+ Hits 18644 60072 +41428
- Misses 32119 61953 +29834
- Partials 2633 6188 +3555
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
/pulsarbot rerun-failure-checks |
1 similar comment
|
/pulsarbot rerun-failure-checks |
poorbarcode
force-pushed
the
flaky/testModularLoadManagerSplitBundle
branch
from
5697306 to
4479b1a
Compare
|
/pulsarbot rerun-failure-checks |
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Dec 7, 2022
(cherry picked from commit e365afe)
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: #16980
Fixes: #17530
Motivation
When runs
AdminApi2Test.testTopicPoliciesWithMultiBrokerorBrokerServiceLookupTest.testModularLoadManagerSplitBundle, you can see some logs like this:Location problem
These test uses
setup()andPulsarService pulsar_new = startBroker(conf_new)to create two brokers, but the both brokers use the samezkc, so both brokers will share thesessionofZK, which will lead to the possibility that the two brokers will constantly elect for the leader. see code:pulsar/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/LeaderElectionImpl.java
Lines 143 to 147 in b89c145
Impact
BrokerServiceLookupTestAdminApi2Test.cleanupModifications
sessionDocumentation
doc-not-neededMatching PR in forked repository
PR in forked repository: