rdhabalia commented on a change in pull request #5599: [pulsar-broker] close
managed-ledgers before giving up bundle ownership to avoid bad zk-version
URL: https://github.com/apache/pulsar/pull/5599#discussion_r345980392
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1860,7 +1860,7 @@ protected void unloadTopic(TopicName topicName, boolean
authoritative) {
validateTopicOwnership(topicName, authoritative);
try {
Topic topic = getTopicReference(topicName);
- topic.close().get();
+ topic.close(false).get();
Review comment:
umm.. I think closing forcefully can be represent with boolean flag as we do
similar thing at multiple places: `PersistentTopic::delete(boolean... flags)`
Also, I was also trying to think about how to accommodate enum here instead
of flag. One thing I can think of is to add below enum under `Topic` instead
of flag.
```
enum CLOSE_ACTION { CLOSE_ALL, CLOSE_WITHOUT_CLIENT_WAIT }
```
But I feel enum is not helping much. Instead we can rename the flag to give
more meaning `closeWithoutWaitingClientDisconnect`.
So, for `PersistentTopic` if flag is enabled then broker skips waiting on
client-disconnect and immediately closes managed-ledger before giving up bundle
ownership.
And for `NonPersistentTopic` just completes the close if flag is enabled.
Can you please let me know if I am missing anything while renaming flag
instead making enum.? any thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
