[GitHub] [pulsar] eolivelli commented on a change in pull request #8890: Add entry cached ledger handle

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #8890:
URL: https://github.com/apache/pulsar/pull/8890#discussion_r540031097



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCache.java
##########
@@ -115,6 +117,15 @@ void asyncReadEntry(ReadHandle lh, long firstEntry, long lastEntry, boolean isSl
      */
     void asyncReadEntry(ReadHandle lh, PositionImpl position, ReadEntryCallback callback, Object ctx);
 
+    /**
+     * Read a sequence of entries from the cache or from bookkeeper.
+     *
+     * @param firstEntry id of first entry of sequence
+     * @param lastEntry  id of last entry of sequence, inclusive
+     * @return an handle to the result of the operation
+     */
+    CompletableFuture<LedgerEntries> asyncReadEntry(ReadHandle lh, long firstEntry, long lastEntry);

Review comment:
       please remember that LedgerEntries is to be "closed" after use, otherwise you will have a memory leak.
   
   we should document this fact in the javadoc and also implement a proper lifecycle for what we are putting into the cache




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

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