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 > > >
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)
>
