On Jul 21, 2017 9:17 AM, "Enrico Olivelli" <eolive...@gmail.com> wrote:
Il ven 21 lug 2017, 16:55 Enrico Olivelli <eolive...@gmail.com> ha scritto: > (I am writing comments and proposals on the issue, I don't know where is > the best place to talk) > https://github.com/apache/bookkeeper/issues/271 > > > I think that we could break existing downstream applications, we must > address this problem > > I think that we should change in a braking way the readEntries API in > order not to provide only a "simple" Enumeration but a class with a close() > method (like implementing AutoClosable) and make it clear to clients that > the MUST close that resource (see the issue for the concrete proposal) > > We already broke the API from 4.4 to 4.5 so another change will not be so > terrible > A more conservative approch would be not to expose the bytebuf and create the byte[] immediately and release the buffer. I don't know if on yahoo/pulsar side this solution would be ok I am kind of -1 to this approach. Bytebuf gave the application the ability of zero copy when this buffer will be used again and again. I'd like to step back before talking about any solutions. Lets discuss when the client will not process entries after get the enumeration. > > -- Enrico > > > > > > > 2017-07-21 16:40 GMT+02:00 Venkateswara Rao Jujjuri <jujj...@gmail.com>: > >> Can we also add some stats around this to track these. ? Discipline >> around >> memory release is new concept to Java world. >> >> >> On Fri, Jul 21, 2017 at 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. >> > >> > 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. >> > >> > 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 >> > >> > >> > -- Enrico >> > >> -- >> Sent from iPhone >> > > -- -- Enrico Olivelli
