Re: [DISCUSS] BP-51: BookKeeper client memory limits
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
Re: [DISCUSS] BP-51: BookKeeper client memory limits
>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 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
Re: [DISCUSS] BP-51: BookKeeper client memory limits
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 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 > > > > >>> ha scritto: > > > > > > > > We have improved the memory issue with backpres
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 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 > > > > > >> 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 > > > > > >>> wrote: > > > >>> > > > >>> Yong, > > > >>> > > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang > > > >>> 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 > > > >>>
Re: [DISCUSS] BP-51: BookKeeper client memory limits
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 > > > >> 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 > > > >>> wrote: > > >>> > > >>> Yong, > > >>> > > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang > > >>> 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 于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
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 > >> 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 > >>> wrote: > >>> > >>> Yong, > >>> > >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang > >>> 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 >>> > 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 于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
> > 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? Best, Dave > > Thanks, > Yong > > >> On Thu, 29 Sept 2022 at 12:30, Michael Marshall >> 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 >>> wrote: >>> >>> Yong, >>> >>> Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang >>> 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 >> 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 于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
On 2022/09/29 04:28:31 Michael Marshall 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. Really good comments Michael. Thanks for bringing this up to discussion. Yes, back pressure should be signalled in a non-blocking way. > The implementation could be > something similar to Netty's channel writability events. yes. that's a very good example. If someone is wondering about the example, it's about Netty's solution, where it has a high and low watermark for an output channel buffer. When the buffer size goes over the high watermark or goes back under the low watermark, Netty will call the "channelWritabilityChange" method on the ChannelInboundHandler . https://netty.io/4.1/api/io/netty/channel/ChannelInboundHandler.html#channelWritabilityChanged-io.netty.channel.ChannelHandlerContext- The channel's "isWritable" method will return false if the high watermark was exceeded. (https://netty.io/4.1/api/io/netty/channel/WriteBufferWaterMark.html explains this) In Netty, this is the way how to detect that data that is pushed to the output channel is getting buffered. By using the channelWritabilityChange event, it's possible to implement backpressure that reacts (it is reactive) to the changes in the buffering conditions. Something similar could be useful for BP-51 so that it would support implementing non-blocking backpressure. Blocking with Thread.sleep is extremely harmful in Netty IO threads, since the same threads are shared across a lot of independent connections by default. All processing will be blocked on all connections using the same thread when the thread is blocked by locks, synchronized methods or Thread.sleep. I commented on blocking Netty IO threads on a Pulsar issue today, https://github.com/apache/pulsar/pull/17870#issuecomment-1262048972 . That contains references in "Netty in Action" book where the importance of non-blocking code in Netty IO threads is emphasized at least in two locations in the book. I'll share the references here too: On page 83 "Normally each ChannelHandler in the ChannelPipeline processes events that are passed to it by its EventLoop (the I/O thread). It’s critically important not to block this thread as it would have a negative effect on the overall handling of I/O." and on page 103 "We stated earlier the importance of not blocking the current I/O thread. We’ll say it again in another way: “Never put a long-running task in the execution queue, because it will block any other task from executing on the same thread.” If you must make blocking calls or execute long-running tasks, we advise the use of a dedicated EventExecutor." I hope that we can find ways to implement non-blocking backpressure for BookKeeper client with BP-51. Regards, -Lari On 2022/09/29 04:28:31 Michael Marshall 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 wrote: > > > > Yong, > > > > Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang > > 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 > > > request
Re: [DISCUSS] BP-51: BookKeeper client memory limits
Thanks, Michael！
>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
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?
Can I understand the client application as the Pulsar broker?
Something like this:
class MemoryListener {
public void onMemoryFull() {
// client application change state to stop adding
}
public void on MemoryReleased() {
// client application change state to continue adding
}
}
>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.
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.
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.
Thanks,
Yong
On Thu, 29 Sept 2022 at 12:30, Michael Marshall
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
> wrote:
> >
> > Yong,
> >
> > Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang
> > 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 >
> > > 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)
> >
Re: [DISCUSS] BP-51: BookKeeper client memory limits
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 wrote: > > Yong, > > Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang > 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 > > 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 于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
Yong, Il giorno mer 28 set 2022 alle ore 10:23 Yong Zhang 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 > 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 于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
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, Yong On Thu, 21 Apr 2022 at 19:17, r...@apache.org 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 于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
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 于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 >
