On Jul 21, 2017 9:25 AM, "Enrico Olivelli" <eolive...@gmail.com> wrote:
Il ven 21 lug 2017, 18:19 Sijie Guo <guosi...@gmail.com> ha scritto: > 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. > It can happen if you have an application error while processing the entries and so you stop calling getEntry for the remaing part of the enumeration. If you stop calling get entries, because you application is closing after errors, or? If that is the case, the application is closing, you don't even need to care about the memory, no? > > 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 > -- -- Enrico Olivelli
