lhotari commented on code in PR #22454:
URL: https://github.com/apache/pulsar/pull/22454#discussion_r1555263091
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -338,11 +337,14 @@ public synchronized void removeConsumer(Consumer
consumer, boolean isResetCursor
if (!isResetCursor) {
try {
topic.getManagedLedger().deleteCursor(cursor.getName());
+
topic.getManagedLedger().removeWaitingCursor(cursor);
} catch (InterruptedException | ManagedLedgerException
e) {
log.warn("[{}] [{}] Failed to remove non durable
cursor", topic.getName(), subName, e);
}
}
});
+ } else {
+ topic.getManagedLedger().removeWaitingCursor(cursor);
Review Comment:
Thanks @Technoboy- , I got it reproduced with
https://github.com/lhotari/pulsar/commit/e2efe24.
Regardless of this, I think that the problem is deeper.
I was expecting that the race condition would be fixed by adding this to
ManagedCursorImpl.checkForNewEntries method:
```
if (isClosed()) {
callback.readEntriesFailed(new
CursorAlreadyClosedException("Cursor was already closed"), ctx);
return;
}
```
However, it doesn't work since when a cursor is deleted, it's status isn't
switched to closed. This isn't very logical in the ManagedCursorImpl code and I
think it should be fixed.
I made this commit in my experiment
https://github.com/lhotari/pulsar/commit/43da5894 . It resolves the race
condition that exists in `checkForNewEntries`.
--
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]
