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?




-- 
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]


Reply via email to