On Jul 21, 2017 12:59 AM, "Enrico Olivelli" <eolive...@gmail.com> wrote:
Hi, I have just file this issue https://github.com/apache/bookkeeper/issues/271 After the switch to Netty 4 I noticed this "API change", that is very important to be documented and/or to be addresses. I think documentation is important in this case. We should make sure it documented at the javadoc of this class. Inside LedgerEntry object we retain a ByteBuf, which is turn is automatically 'released' only in this cases: - getEntry() is called - getEntryInputStream() is called and the InputStream is closed - the ByteBuf is manually closed by the client The real tricky thing is that if the client calls readEntry and the Enumeration is not fully processed we are going to leak ByteBufs and so head/direct memory. I am wondering when this will happen - if you already called read entries, why you don't enumerate the entries. The first two approaches are backward compatible with existing behavior without changing any codes. The third one is the new API that gave the client flexibility on managing the memory usage itself. My proposal: introduce some "entry reference counting" and ensure that all generated entries by a LedgerHandler are "disposed" on LedgerHandler.close() and make sure that when BookKeeper client is closed all of the LedgerHandlers process their own disposal procedure I am not sure if we need this. It seems to be over engineering to me. -- Enrico
