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 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
> 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
>> 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
>> > 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
>> > wrote:
>> > >
>> > > > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher
>> > > > 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 reactive
