LeBW commented on a change in pull request #12215:
URL: https://github.com/apache/pulsar/pull/12215#discussion_r717220199
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -632,25 +632,29 @@ protected void
internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
return;
}
}
+ // Only tries to delete the authentication policies for
partitioned topic when all its partitions are
+ // successfully deleted
+
pulsar().getBrokerService().deleteTopicAuthenticationWithRetry(topicName.toString());
Review comment:
`NamespaceResources#setPoliciesAsync()` is an atomic update to zookeeper
with optimistic lock, so maybe this is already an atomic update?
Besides, I use retry logic here (as it's typical to generate
`BadVersionException` here), so maybe this is an at-least-once semantics?
As for the `asyncResponse`, I used to think that this deletion operation
shouldn't affect the response because it's not the main goal of the method. Do
you mean that when the deletion of authentication policies fails, the whole
method should throw an exception?
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -632,25 +632,29 @@ protected void
internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
return;
}
}
+ // Only tries to delete the authentication policies for
partitioned topic when all its partitions are
+ // successfully deleted
+
pulsar().getBrokerService().deleteTopicAuthenticationWithRetry(topicName.toString());
Review comment:
Got it. Thanks for your comments.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
