fix:remove the loadbalance/bundle-data node - #13164
Conversation
|
@leizhiyuan:Thanks for your contribution. For this PR, do we need to update docs? |
|
@leizhiyuan:Thanks for providing doc info! |
github-actions
Bot
added
doc-not-needed
wolfstudy
left a comment
wolfstudy
left a comment
There was a problem hiding this comment.
Can you add test case for testDeleteTenant?
|
|
||
| // clear resource of `/loadbalance/bundle-data/{tenant}/` for zk-node | ||
| public CompletableFuture<Void> deleteBundleDataTenantAsync(String tenant) { | ||
| final String namespaceBundlePath = joinPath(BUNDLE_DATA_BASE_PATH, tenant); |
There was a problem hiding this comment.
Maybe tenantBundlePath is good name then namespaceBundlePath
| final String namespaceBundlePath = joinPath(BUNDLE_DATA_BASE_PATH, tenant); | ||
| CompletableFuture<Void> future = new CompletableFuture<Void>(); | ||
| deleteRecursiveAsync(this, namespaceBundlePath).whenComplete((ignore, ex) -> { | ||
| if (ex != null && ex.getCause() instanceof KeeperException.NoNodeException) { |
There was a problem hiding this comment.
If you use ex.getCause(), you get should not be a KeeperException, but an exception of the megastore that encapsulates the zk client. So here should use ex.getCause().getCause()?
| final String namespaceBundlePath = joinPath(BUNDLE_DATA_BASE_PATH, ns.toString()); | ||
| CompletableFuture<Void> future = new CompletableFuture<Void>(); | ||
| deleteRecursiveAsync(this, namespaceBundlePath).whenComplete((ignore, ex) -> { | ||
| if (ex != null && ex.getCause() instanceof KeeperException.NoNodeException) { |
There was a problem hiding this comment.
Please use ex.getCause().getCause()
There was a problem hiding this comment.
ex.getCause().getCause() will be null
|
need to rerun the test case，It is not related with this pr。 |
| final String namespaceBundlePath = joinPath(BUNDLE_DATA_BASE_PATH, ns.toString()); | ||
| CompletableFuture<Void> future = new CompletableFuture<Void>(); | ||
| deleteRecursiveAsync(this, namespaceBundlePath).whenComplete((ignore, ex) -> { | ||
| if (ex != null && ExceptionUtils.getRootCause(ex) instanceof KeeperException.NoNodeException) { |
There was a problem hiding this comment.
We should use org.apache.pulsar.metadata.api.MetadataStoreException.NotFoundException to replace KeeperException.NoNodeException. The underlying metadata store supports multi implementations now.
Fixes apache#13162 ### Motivation fix: remove the loadbalance/bundle-data node, when deleting namespace. it will cause zk node leak, fix apache#13162 ### Modifications remove the bundle-data Recursively
6 participants
Fixes #13162
Motivation
fix: remove the loadbalance/bundle-data node, when deleting namespace. it will cause zk node leak, fix #13162
Modifications
remove the bundle-data Recursively
Verifying this change
Extended integration test for recovery after broker failure
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)