sijie commented on a change in pull request #727: Issue 693: add interface and 
implementation of LedgerEntries
URL: https://github.com/apache/bookkeeper/pull/727#discussion_r152645332
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/LedgerEntries.java
 ##########
 @@ -35,10 +35,15 @@
     LedgerEntry getEntry(long entryId);
 
     /**
-     * In this, It does not retain the ByteBuf references for the entries in 
this LedgerEntries.
+     * In this method, It does not increment the reference counts of ByteBuf 
for the entries in this LedgerEntries.
      * The caller who calls {@link #iterator()} should be careful for not 
releasing the references.
      *
-     * @return the iterator of type LedgerEntry
+     * when iterator is called, you are handing out the entries, you may not 
know when the caller will
 
 Review comment:
   @ivankelly it is not a `move`. `retain` is to increment the reference count. 
there is no move here, as the caller don't really know how many reference 
counts are referencing the underneath buffer. 
   
   The whole idea behind `retainedIterator` is following what `ByteBuf` has and 
provide exactly same behavior as what people would expect from `ByteBuf`. There 
is already some sort of reference counting mechanisms that bookkeeper is using. 
We can just follow what it already has and have a consistent naming, semantic 
and behavior as `ByteBuf`. Consistent naming and semantic is more important to 
me.
   
   retain - https://netty.io/4.1/api/io/netty/buffer/ByteBuf.html#retain--
   retainedSlice - 
https://netty.io/4.1/api/io/netty/buffer/ByteBuf.html#retainedSlice-int-int-
   
   Anyway, let's move forward without `retainedIterator`. It is not a blocker 
for 4.6.0. We can circle back in future if it is needed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to