fix max topic for namespace does not work #9193
Conversation
|
/pulsarbot run-failure-checks |
2 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
| @@ -812,7 +812,7 @@ protected boolean isNamespaceReplicated(NamespaceName namespaceName) { | |||
| try { | |||
| String topicPartitionPath = joinPath(MANAGED_LEDGER_PATH_ZNODE, | |||
| namespaceName.toString(), topicDomain.value()); | |||
| List<String> topics = globalZk().getChildren(topicPartitionPath, false); | |||
| List<String> topics = localZk().getChildren(topicPartitionPath, false); | |||
There was a problem hiding this comment.
Is it possible to add a unit test? This will make pulsar more robust
There was a problem hiding this comment.
There is already a test AdminApiTest2#testMaxTopicsPerNamespace
There was a problem hiding this comment.
Thanks @aloyszhang , This change lgtm. As @codelipenghui mentioned, it would be great if we have it reproduced and fixed in the unit test.
How about we create 2 mocked zk in the MockedPulsarServiceBaseTest, 1 zk for localZk() and another for globalZk? By this way, we could reproduce this error in the original code, and verify it fixed by this change.
There was a problem hiding this comment.
Thanks for your suggestion @jiazhai @codelipenghui .
I have test the AdminApiTest2#testMaxTopicsPerNamespace and found the globalZk and localZk return the same result , this is why the original test passed.
I'll try to add two different zk for local and global respectively.
codelipenghui
requested review from
hangc0276,
315157973,
jiazhai,
merlimat and
sijie
|
@aloyszhang opened issue #9207 to track the changes of 2 zk in test base. |
This PR fix max topic for namespace does not work. Max topic configuration for namespace does not work well. This is caused by currently we use globalZK to get the children node of "/managed-ledgers/tenant/namespace/domain" when caculate the number of existed topics. Znode of "/managed-ledgers/tenant/namespace/domain" should be accessed by localZK instead.
…cal in MockedPulsarServiceBaseTest (apache#9222) Fixes apache#9207 This PR is a continue for apache#9193 , since apache#9193 is only a partial fix for mis-use of local/global zk. This PR fix others places of mis-use and provide different zk for global/local in `MockedPulsarServiceBaseTest`
Motivation
This PR fix max topic for namespace does not work.
Max topic configuration for namespace does not work well. This is caused by currently we use globalZK to get the children node of "/managed-ledgers/tenant/namespace/domain" when caculate the number of existed topics.
Znode of "/managed-ledgers/tenant/namespace/domain" should be accessed by localZK instead.