Jason918 commented on a change in pull request #14656:
URL: https://github.com/apache/pulsar/pull/14656#discussion_r824400327



##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java
##########
@@ -331,7 +331,6 @@ public synchronized void removeConsumer(Consumer consumer, 
boolean isResetCursor
                 // when topic closes: it iterates through 
concurrent-subscription map to close each subscription. so,
                 // topic.remove again try to access same map which creates 
deadlock. so, execute it in different thread.
                 topic.getBrokerService().pulsar().getExecutor().submit(() ->{
-                    topic.removeSubscription(subName);

Review comment:
       I think there may be some side effect to remove this line, if 
`removeConsumer` is not called from `doUnsubscribe`. Maybe it's better to make 
`topic.removeSubscription` support removing non-exists subName (Just add a null 
check in `removeSubscription`).




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