lhotari commented on code in PR #24569:
URL: https://github.com/apache/pulsar/pull/24569#discussion_r2240329579
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -354,6 +354,16 @@ public synchronized void removeConsumer(Consumer consumer,
boolean isResetCursor
if (dispatcher != null && dispatcher.getConsumers().isEmpty()) {
deactivateCursor();
+ // Remove the cursor from the waiting cursors list.
+ // For durable cursors, we should *not* cancel the pending read
with cursor.cancelPendingReadRequest.
+ // This is because internally, in the dispatcher implementations,
there is a "havePendingRead" flag
+ // that is not reset. If the pending read is cancelled, the
dispatcher will not continue reading from
+ // the managed ledger when a new consumer is added to the
dispatcher since based on the "havePendingRead"
+ // state, it will continue to expect that a read is pending and
will not submit a new read.
+ // For non-durable cursors, there's no difference since the cursor
is not expected to be used again.
+
+ // remove waiting cursor from the managed ledger, this applies to
both durable and non-durable cursors.
+ topic.getManagedLedger().removeWaitingCursor(cursor);
Review Comment:
> My concern is that we used some internal concepts from the ManagedLedger
in PersistentSubscription. We should find a way to avoid that. The state of
`waitingCursors` should be self-managed by ManagedLedger.
I completely agree that it should be avoided. There should have been some
sort of event for handling this.
btw. The call `topic.getManagedLedger().removeWaitingCursor(cursor)` wasn't
added in this PR. Before #24551, there was a similar call. The call
`topic.getManagedLedger().removeWaitingCursor(cursor)` was added to
`PersistentSubscription` in #13939. Another PR in this area was #22454.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
