dragosvictor commented on code in PR #21682:
URL: https://github.com/apache/pulsar/pull/21682#discussion_r1424406261
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Subscription.java:
##########
@@ -64,13 +65,13 @@ default long getNumberOfEntriesDelayed() {
List<Consumer> getConsumers();
- CompletableFuture<Void> close();
Review Comment:
You're right, this is not in the PIP. It's an issue I ran into while working
out the consumer changes.
There is an overlap between what `close` and `disconnect` do as of right
now. `close` fails if `disconnect` has not been called, while `disconnect`
internally invokes `close`, which is not all that obvious. I cleaned up the
interface a bit to make these behaviors clearer.
If I were to keep the definition in the interface, it would look like this:
```java
default CompletableFuture<Void> close() {
return disconnect(true /* disconnectAllConsumers */,
Optional.empty() /* assignedBrokerLookupData */);
}
```
which further contributes to the confusion. My idea was that this is a
method internal to the broker itself, thus an implementation detail.
I'm open to suggestions on what's the best course of action here.
--
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]
