Hi folks, I rewrite the proposal with watermark way, and update the proposal here https://github.com/apache/bookkeeper/issues/3231#issue-1210800448
And if you are interested in the implementation, I wrote a prototype here https://github.com/apache/bookkeeper/pull/3139/files Here are some logs from my testing code. https://github.com/apache/bookkeeper/pull/3139#issuecomment-1274359607 Hope to hear any suggestions! Thanks! Yong On Sun, 9 Oct 2022 at 11:26, Yong Zhang <zhangyong1025...@gmail.com> wrote: > >What if the BK client followed netty's decision on this: raise a flag > (e.g. > isWritable())? > > Bookie clients can write when there have AQ bookies are alive. It also > can change the ledger's bookie ensemble if there has a bookie failure. > So looks like it is difficult to use netty's decision. > > On Tue, 4 Oct 2022 at 07:23, Andrey Yegorov <andrey.yego...@datastax.com> > wrote: > >> What if the BK client followed netty's decision on this: raise a flag >> (e.g. >> isWritable())? >> In this case it would be up to Pulsar (or any other app) to decide what to >> do. >> >> On Fri, Sep 30, 2022 at 10:02 PM Michael Marshall <mmarsh...@apache.org> >> wrote: >> >> > Thank you for your points, Lari. They expanded on my thoughts very well. >> > >> > One important design aspect of Netty's channel writability status is >> > that it is not strictly enforced. It is up to the application to stop >> > writing to an unwritable channel. Similarly, with a reactive solution, >> > it would be up to the client application to stop submitting add >> > requests to the bookkeeper client. >> > >> > > I am trying understand this. Correct me if I am wrong. >> > > Do you mean we should let the client application to register a >> listener >> > > on the memory controller? If there hasn't memory, notify the client >> > > to stop adding. And if memory released, notify the client continue? >> > >> > Yes, that is essentially what I meant. As Lari mentioned, one part of >> > the design can have high and low water marks so that the memory is >> > able to be drained a bit before telling the client to add more >> > entries. >> > >> > > Can I understand the client application as the Pulsar broker? >> > >> > Correct. In my view, non-blocking back pressure is important for the >> > Pulsar Broker because the broker needs to propagate back pressure to >> > its producers. With a blocking implementation, the broker will know >> > that the `addEntry` method hasn't returned, but it won't know that it >> > is because of high memory consumption. >> > >> > > Yes, reading also has memory problems. But I want to make this >> proposal >> > > more focus on the writing. Maybe we can use another proposal to >> resolve >> > > the reading. >> > >> > It's reasonable to focus on one part of the problem for this BP. I >> > hope we find a solution that will integrate well with limiting reads >> > too. >> > >> > Thanks, >> > Michael >> > >> > On Thu, Sep 29, 2022 at 8:56 PM Yong Zhang <zhangyong1025...@gmail.com> >> > wrote: >> > > >> > > Sorry for the typo. I mean WQ > AQ. >> > > >> > > Thanks for your information, Lari! >> > > >> > > Let me try to reconsider this proposal with the watermark way. >> > > >> > > Yong >> > > >> > > On Thu, Sep 29, 2022 at 21:11 Enrico Olivelli <eolive...@gmail.com> >> > wrote: >> > > >> > > > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher >> > > > <wave4d...@comcast.net> ha scritto: >> > > > > >> > > > > >> > > > > > >> > > > > > I think I need to change this proposal title to `BookKeeper >> client >> > > > write >> > > > > > memory >> > > > > > limits` to make it clearly. What we observed is bookie will >> easily >> > OOM >> > > > when >> > > > > > WQ < AQ. So the main problem we want to use this proposal to >> > resolve is >> > > > > > limit >> > > > > > the adds request memory usage. >> > > > > >> > > > > What is the use case for WQ < AQ? >> > > > >> > > > it is a typo, WQ must be always >= QA >> > > > >> > > > Enrico >> > > > > >> > > > > Best, >> > > > > Dave >> > > > > > >> > > > > > Thanks, >> > > > > > Yong >> > > > > > >> > > > > > >> > > > > >> On Thu, 29 Sept 2022 at 12:30, Michael Marshall < >> > mmarsh...@apache.org >> > > > > >> > > > > >> wrote: >> > > > > >> >> > > > > >> I support adding back pressure based on client memory limits to >> > the >> > > > > >> bookkeeper client. >> > > > > >> >> > > > > >> My biggest concern is how the back pressure is propagated to >> the >> > > > > >> client application. If I am reading the draft implementation >> > > > > >> correctly, it is via a blocking operation on the calling thread >> > for >> > > > > >> the `BookieClientImpl#addEntry` method. >> > > > > >> >> > > > > >> In my use case (the Pulsar broker), I think a blocking >> > implementation >> > > > > >> will make this feature very hard to use. One quick thought is >> that >> > > > > >> maybe some kind of event or listener could meet the >> requirements >> > > > > >> without also blocking an application? The implementation could >> be >> > > > > >> something similar to Netty's channel writability events. Then, >> > client >> > > > > >> applications could reactively get notified of the bookie >> client's >> > > > > >> state. Non blocking back pressure allows for client >> applications >> > to >> > > > > >> continue processing other >> > > > > >> >> > > > > >> Additionally, I think client memory limits should affect the >> > bookie >> > > > > >> client reading from the inbound connection. Otherwise, the >> bookie >> > > > > >> client could dispatch many read requests and then end up >> filling >> > up >> > > > > >> memory when the requests arrive in the client's direct memory. >> > When >> > > > > >> the bookie is starting to exceed its memory consumption, it'd >> be >> > > > > >> beneficial to stop reading from the connection and to let the >> TCP >> > > > > >> connection propagate back pressure to the server. In this >> case, we >> > > > > >> would need a reactive solution since it is an anti-pattern to >> > block on >> > > > > >> a netty event loop. >> > > > > >> >> > > > > >> Thanks, >> > > > > >> Michael >> > > > > >> >> > > > > >>> On Wed, Sep 28, 2022 at 4:36 AM Enrico Olivelli < >> > eolive...@gmail.com >> > > > > >> > > > > >>> wrote: >> > > > > >>> >> > > > > >>> Yong, >> > > > > >>> >> > > > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang >> > > > > >>> <zhangyong1025...@gmail.com> ha scritto: >> > > > > >>>> >> > > > > >>>> We have improved the memory issue with backpressure with PR >> > > > > >>>> https://github.com/apache/bookkeeper/pull/3324 >> > > > > >>>> >> > > > > >>>> The backpressure way can prevent there have too many Add >> > requests >> > > > > >>>> pending to the client and waiting for the response. It makes >> > the add >> > > > > >>>> requests >> > > > > >>>> fail fast, so if the channel is not writable, it will fail >> the >> > > > request >> > > > > >> as >> > > > > >>>> soon as >> > > > > >>>> possible and then release the memory. >> > > > > >>>> >> > > > > >>>> But that depends on the time. If your throughput is very >> > smooth, and >> > > > > >> you >> > > > > >>>> have enough memory for the bookie client. With backpressure, >> it >> > > > would >> > > > > >> work >> > > > > >>>> fine. >> > > > > >>>> If you have a huge adds to the bookie in a very short time, >> it >> > > > > >> acquires a >> > > > > >>>> lot of >> > > > > >>>> memory, then the bookie crashed with OOM. >> > > > > >>>> So we still need this proposal. >> > > > > >>>> >> > > > > >>>> I will continue to work on this. If there haven't objected, I >> > will >> > > > > >> start a >> > > > > >>>> VOTE later >> > > > > >>> >> > > > > >>> Thanks >> > > > > >>> >> > > > > >>> Enrico >> > > > > >>> >> > > > > >>>> Thanks, >> > > > > >>>> Yong >> > > > > >>>> >> > > > > >>>> On Thu, 21 Apr 2022 at 19:17, r...@apache.org < >> > > > ranxiaolong...@gmail.com >> > > > > >>> >> > > > > >>>> wrote: >> > > > > >>>> >> > > > > >>>>> Hello Yong: >> > > > > >>>>> >> > > > > >>>>> It seems to be a very useful feature. In the production >> > > > environment, >> > > > > >> you >> > > > > >>>>> can often see similar phenomena happening. >> > > > > >>>>> >> > > > > >>>>> +1 (non-binding) >> > > > > >>>>> >> > > > > >>>>> -- >> > > > > >>>>> Thanks >> > > > > >>>>> Xiaolong Ran >> > > > > >>>>> >> > > > > >>>>> Yong Zhang <y...@apache.org> 于2022年4月21日周四 18:29写道： >> > > > > >>>>> >> > > > > >>>>>> Hi all, >> > > > > >>>>>> >> > > > > >>>>>> The BP-51 BookKeeper client memory limits is ready for >> review. >> > > > > >>>>>> The proposal is here: >> > > > > >> https://github.com/apache/bookkeeper/issues/3231 >> > > > > >>>>>> And the PR is here: >> > > > https://github.com/apache/bookkeeper/pull/3139 >> > > > > >>>>>> >> > > > > >>>>>> Please help to review this proposal. >> > > > > >>>>>> >> > > > > >>>>>> Thanks! >> > > > > >>>>>> Yong >> > > > > >>>>>> >> > > > > >>>>> >> > > > > >> >> > > > >> > >> >> >> -- >> Andrey Yegorov >> >
