lhotari commented on code in PR #22454:
URL: https://github.com/apache/pulsar/pull/22454#discussion_r1555180329
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -3815,7 +3815,7 @@ public void removeWaitingCursor(ManagedCursor cursor) {
public void addWaitingCursor(ManagedCursorImpl cursor) {
if (cursor instanceof NonDurableCursorImpl) {
- if (cursor.isActive()) {
+ if (cursors.get(cursor.getName()) != null) {
Review Comment:
Is it possible to add a comment to explain this condition?
It seems that the original reason to add this was to fix #22157 in #22191 .
It would be great to have the rational explained in a comment.
When looking at the code, it seems that this would always evaluate to true
since non durable cursors are always "cached" and part of the `cursors` map.
Only when the non durable cursor is deleted, would it be removed from `cursors`.
Is the logic in `addWaitingCursor` useful? Do we have a separate test case
for #22191 that proves that it fixes the memory leak?
--
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]
