Re: [PR] [fix][ml] Fix metadata listener leaks caused by unclosed temporary read handles [pulsar]

["Denovo1998 (via GitHub)" <gi...@apache.org>]

Denovo1998 commented on code in PR #26817:
URL: https://github.com/apache/pulsar/pull/26817#discussion_r4230986548


##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -679,6 +682,7 @@ protected void recoverFromLedger(final ManagedCursorInfo info, final VoidCallbac
                 try {
                     positionInfo.parseFrom(entry.getEntry());
                 } catch (Exception e) {
+                    ManagedLedgerImpl.closeReadHandleAsync(lh, log);

Review Comment:
   Could we also cover failures that happen after `parseFrom()` succeeds?
   
   We still call `recoverIndividualDeletedMessages()`, `recoverBatchDeletedIndexes()`, and `recoveredCursor()` before the handle ownership is fully transferred. For example, a malformed but decodable deletion range could throw an `IllegalArgumentException` during recovery.
   
   In that case, we might leave the read handle open and never complete the recovery callback.
   
   Maybe we could guard the remaining recovery steps and close the handle if ownership wasn't transferred successfully? A test with a decodable but invalid cursor snapshot would be useful too.



-- 
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-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org