codelipenghui commented on code in PR #24569:
URL: https://github.com/apache/pulsar/pull/24569#discussion_r2240302991


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



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

Reply via email to