You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/06 09:28:54 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
poorbarcode opened a new pull request, #17487:
URL: https://github.com/apache/pulsar/pull/17487
Fixes #17386
### Motivation
In this test, we expect it works like this:
1. persistent broker data to ZK
2. receive notice: broker data changes, and refresh bundle data in memory
3. persistent bundle data to ZK
4. verify bundle data persist correctly
5. delete namespace
6. verify bundle data deleted
But the flow works like this:
1. persistent broker data to ZK
2. <strong> (High light) race condition </strong>
2-x. receive notice: broker data changes, and refresh bundle data in memory
2-x. persistent bundle data to ZK ( If the data in memory is not updated, it will not be written to ZK )
3. ......
### Modifications
- guarantee `step-3` executed after `step-2`
- In this test, the original codes mock `ModularLoadManagerWrapper` to make `isCentralized` always return `true`. But this mock is completely unnecessary because that's `ModularLoadManagerWrapper.isCentralized` was exactly always returned `true`, so remove the mock.
### Documentation
- [ ] `doc-required`
- [ ] `doc-not-needed`
- [ ] `doc`
- [ ] `doc-complete`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #17487:
URL: https://github.com/apache/pulsar/pull/17487
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1239363843
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1237933492
@heesung-sn Please help review this PR, thanks.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1238538546
@poorbarcode Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#discussion_r963631211
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java:
##########
@@ -792,12 +794,46 @@ public void testModularLoadManagerRemoveBundleAndLoad() throws Exception {
TimeUnit.SECONDS.sleep(5);
// update broker bundle report to zk
- loadManager.writeLoadReportOnZookeeper();
- loadManager.writeResourceQuotasToZooKeeper();
+ waitResourceDataUpdateToZK(loadManager,
+ loadData -> !loadData.getBundleData().containsKey(bundleName));
getResult = pulsar.getLocalMetadataStore().get(path).get();
assertFalse(getResult.isPresent());
+ }
+ private void waitResourceDataUpdateToZK(
+ LoadManager loadManager, Predicate<LoadData> utilChecker) throws Exception {
Review Comment:
That was left over from the previous solution and I forgot to delete it. Already deleted, Thanks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1238950852
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1237900962
This PR should merge to `branch-2.10` and `branch-2.11` and `master`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1238833974
Hi @Technoboy-
> Better to add some comments for line 809~814
Already add code-comment, thanks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] heesung-sn commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1454347491
// Wait until "ModularLoadManager" completes processing the ZK notification.
ModularLoadManagerWrapper modularLoadManagerWrapper = (ModularLoadManagerWrapper) loadManager;
ModularLoadManagerImpl modularLoadManager = (ModularLoadManagerImpl) modularLoadManagerWrapper.getLoadManager();
ScheduledExecutorService scheduler = Whitebox.getInternalState(modularLoadManager, "scheduler");
We don't have the `scheduler` anymore in ModularLoadManagerImpl. I think this test is failing now.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] heesung-sn commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
heesung-sn commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1240901351
LGTM
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#issuecomment-1239363508
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17487: [fix][flaky-test]NamespaceServiceTest.flaky/testModularLoadManagerRemoveBundleAndLoad
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17487:
URL: https://github.com/apache/pulsar/pull/17487#discussion_r963625079
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java:
##########
@@ -792,12 +794,46 @@ public void testModularLoadManagerRemoveBundleAndLoad() throws Exception {
TimeUnit.SECONDS.sleep(5);
// update broker bundle report to zk
- loadManager.writeLoadReportOnZookeeper();
- loadManager.writeResourceQuotasToZooKeeper();
+ waitResourceDataUpdateToZK(loadManager,
+ loadData -> !loadData.getBundleData().containsKey(bundleName));
getResult = pulsar.getLocalMetadataStore().get(path).get();
assertFalse(getResult.isPresent());
+ }
+ private void waitResourceDataUpdateToZK(
+ LoadManager loadManager, Predicate<LoadData> utilChecker) throws Exception {
Review Comment:
It seems the param `utilChecker` is useless.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org