poorbarcode commented on code in PR #21946:
URL: https://github.com/apache/pulsar/pull/21946#discussion_r1473783582
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Replicator.java:
##########
@@ -29,9 +29,9 @@ public interface Replicator {
ReplicatorStatsImpl getStats();
- CompletableFuture<Void> disconnect();
+ CompletableFuture<Void> terminate();
- CompletableFuture<Void> disconnect(boolean b);
+ CompletableFuture<Void> terminate(boolean b);
Review Comment:
> This API looks weird. We should not fail the termination because
failIfHasBacklog? I suppose failIfHasBacklog should be a part of replication
disabling, not termination.
It is only used by `Topic GC` 😂, so it looks weird.
I reviewed the code, `Topic GC` used `disconnect(true /* failIfHasBacklog
*/)` to check there is no backlog of Replicator, and deleted the topic after
the disconnect was successful. So, it should want to close the remote producer
not terminate the Replicator. I added the method `disconnect` back, and the
implementation is just close the remote producer.
--
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]
