You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Michael Marshall <mm...@apache.org> on 2022/10/01 05:02:03 UTC
Re: [DISCUSS] BP-51: BookKeeper client memory limits
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 <zh...@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 <eo...@gmail.com> wrote:
>
> > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher
> > <wa...@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 <mmarshall@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 <eolivelli@gmail.com
> > >
> > > >>> wrote:
> > > >>>
> > > >>> Yong,
> > > >>>
> > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang
> > > >>> <zh...@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, rxl@apache.org <
> > ranxiaolong716@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 <yo...@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
> > > >>>>>>
> > > >>>>>
> > > >>
> >
Re: [DISCUSS] BP-51: BookKeeper client memory limits
Posted by Yong Zhang <zh...@gmail.com>.
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 <zh...@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 <an...@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 <mm...@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 <zh...@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 <eo...@gmail.com>
>> > wrote:
>> > >
>> > > > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher
>> > > > <wa...@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 <
>> > mmarshall@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 <
>> > eolivelli@gmail.com
>> > > > >
>> > > > > >>> wrote:
>> > > > > >>>
>> > > > > >>> Yong,
>> > > > > >>>
>> > > > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang
>> > > > > >>> <zh...@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, rxl@apache.org <
>> > > > ranxiaolong716@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 <yo...@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
>>
>
Re: [DISCUSS] BP-51: BookKeeper client memory limits
Posted by Yong Zhang <zh...@gmail.com>.
>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 <an...@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 <mm...@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 <zh...@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 <eo...@gmail.com>
> > wrote:
> > >
> > > > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher
> > > > <wa...@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 <
> > mmarshall@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 <
> > eolivelli@gmail.com
> > > > >
> > > > > >>> wrote:
> > > > > >>>
> > > > > >>> Yong,
> > > > > >>>
> > > > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang
> > > > > >>> <zh...@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, rxl@apache.org <
> > > > ranxiaolong716@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 <yo...@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
>
Re: [DISCUSS] BP-51: BookKeeper client memory limits
Posted by Andrey Yegorov <an...@datastax.com>.
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 <mm...@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 <zh...@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 <eo...@gmail.com>
> wrote:
> >
> > > Il giorno gio 29 set 2022 alle ore 15:06 Dave Fisher
> > > <wa...@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 <
> mmarshall@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 <
> eolivelli@gmail.com
> > > >
> > > > >>> wrote:
> > > > >>>
> > > > >>> Yong,
> > > > >>>
> > > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang
> > > > >>> <zh...@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, rxl@apache.org <
> > > ranxiaolong716@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 <yo...@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