Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][broker] revert remove duplicate topics name when deleteNamespace #21087

Merged
merged 1 commit into from
Sep 5, 2023
Merged

[fix][broker] revert remove duplicate topics name when deleteNamespace #21087

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,6 @@ private void internalRetryableDeleteNamespaceAsync0(boolean force, int retryTime
return old;
});
}
allUserCreatedTopics.removeIf(t ->
allPartitionedTopics.contains(TopicName.get(t).getPartitionedTopicName()));
allSystemTopics.removeIf(t ->
allPartitionedTopics.contains(TopicName.get(t).getPartitionedTopicName()));
topicPolicy.removeIf(t ->
allPartitionedTopics.contains(TopicName.get(t).getPartitionedTopicName()));
Comment on lines -282 to -287
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a test to protect the behavior?

Copy link
Contributor Author

@TakaHiR07 TakaHiR07 Sep 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java

Lines 328 to 342 in e59c850

if (rc instanceof MetadataStoreException) {
if (rc.getCause() != null && rc.getCause() instanceof KeeperException.NotEmptyException) {
log.info("[{}] There are in-flight topics created during the namespace deletion, "
+ "retry to delete the namespace again.", namespaceName);
final int next = retryTimes - 1;
if (next > 0) {
// async recursive
internalRetryableDeleteNamespaceAsync0(force, next, callback);
} else {
callback.completeExceptionally(
new RestException(Status.CONFLICT, "The broker still have in-flight topics"
+ " created during namespace deletion, please try again."));
// drop out recursive
}
return;
The retry logic make it hard to add test. Because this wrong behaviour also throw KeeperException.NotEmptyException, which would be caught and retry internalRetryableDeleteNamespaceAsync0(). Actually it should succeed without retry

return markDeleteFuture.thenCompose(__ ->
internalDeleteTopicsAsync(allUserCreatedTopics))
.thenCompose(ignore ->
Expand Down