Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-16 Thread Colin McCabe
Blocking in the constructor to fetch metadata doesn't solve the problem, since metadata still needs to be periodically refreshed over time. best, Colin On Wed, Jun 16, 2021, at 09:51, Jesse Feinman wrote: > Hi Moses, > > Specifically, on blocking in the constructor to fetch metadata, while I

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-16 Thread Jesse Feinman
Hi Moses, Specifically, on blocking in the constructor to fetch metadata, while I like the idea of specifying the topics in the constructor and fetching the metadata then, I think it leads to a few scenarios that could be unexpected. First is if you try to use a topic that wasn't included in

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-02 Thread Nakamura
Hi Colin, > Sure, we organize buffers by broker currently. However, we could set some maximum buffer size for records that haven't been assigned to a broker yet. OK, I think we're probably aligned then. I think we were using slightly different terminology (queue vs buffer) but we were actually

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-02 Thread Colin McCabe
On Tue, Jun 1, 2021, at 12:22, Nakamura wrote: > I think we're talking past each other a bit. I know about non-blocking > I/O. The problem I'm facing is how to preserve the existing semantics > without blocking. Right now callers assume their work is enqueued in-order > after

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-02 Thread Colin McCabe
On Tue, Jun 1, 2021, at 12:12, Ryanne Dolan wrote: > Colin, the issue for me isn't so much whether non-blocking I/O is used or > not, but the fact that the caller observes a long time between calling > send() and receiving the returned future. This behavior can be considered > "blocking" whether

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-01 Thread Nakamura
Hi Colin, > KafkaProducer#send is supposed to initiate non-blocking I/O, but not wait for it to complete. > > There's more information about non-blocking I/O in Java here: > https://en.wikipedia.org/wiki/Non-blocking_I/O_%28Java%29 I think we're talking past each other a bit. I know about

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-01 Thread Ryanne Dolan
Colin, the issue for me isn't so much whether non-blocking I/O is used or not, but the fact that the caller observes a long time between calling send() and receiving the returned future. This behavior can be considered "blocking" whether or not I/O is involved. > How are the ordering semantics of

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-01 Thread Colin McCabe
On Tue, Jun 1, 2021, at 07:00, Nakamura wrote: > Hi Colin, > > Sorry, I still don't follow. > > Right now `KafkaProducer#send` seems to trigger a metadata fetch. Today, > we block on that before returning. Is your proposal that we move the > metadata fetch out of `KafkaProducer#send` entirely?

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-06-01 Thread Nakamura
Hi Colin, Sorry, I still don't follow. Right now `KafkaProducer#send` seems to trigger a metadata fetch. Today, we block on that before returning. Is your proposal that we move the metadata fetch out of `KafkaProducer#send` entirely? Even if the metadata fetch moves to be non-blocking, I

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-30 Thread Colin McCabe
Hi Matthew, I think you might be misunderstanding the current behavior. Currently during a call to produce(), if the producer's buffer fills up, it will block the calling thread until the buffer is no longer full. This behavior could be changed (but not by using infinite buffering, as I

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-30 Thread Colin McCabe
On Tue, May 25, 2021, at 11:26, Nakamura wrote: > Hey Colin, > > For the metadata case, what would fixing the bug look like? I agree that > we should fix it, but I don't have a clear picture in my mind of what > fixing it should look like. Can you elaborate? > If the blocking metadata fetch

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Josep Prat
Hi Ryanne, Apologies to bring this here then! :) On Wed, May 26, 2021 at 5:27 PM Ryanne Dolan wrote: > Josep, that is being done as KIP-707. Looking forward to that as well :) > > Ryanne > > On Wed, May 26, 2021, 9:08 AM Josep Prat > wrote: > > > Sorry, I meant `CompletionStage` (instead of

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Ryanne Dolan
Josep, that is being done as KIP-707. Looking forward to that as well :) Ryanne On Wed, May 26, 2021, 9:08 AM Josep Prat wrote: > Sorry, I meant `CompletionStage` (instead of CompletableFuture) as this is > the interface. > > Best, > ——— > Josep Prat > > Aiven Deutschland GmbH > >

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Josep Prat
Sorry, I meant `CompletionStage` (instead of CompletableFuture) as this is the interface. Best, ——— Josep Prat Aiven Deutschland GmbH Immanuelkirchstraße 26, 10405 Berlin Amtsgericht Charlottenburg, HRB 209739 B Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen m: +491715557497 w: aiven.io

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Josep Prat
Hi, If I may, I would like to suggest that instead of using Java's `Future` class on the API's, it would be better to use `CompletableFuture`. This would offer the possibility of applying callbacks on its completion for example. Best, On Wed, May 26, 2021 at 3:28 PM Matthew de Detrich wrote: >

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Matthew de Detrich
Maybe there was a miscommunication but I agree with everything you said, I was just clarifying what my definition of blocking is (because I think there was a misunderstanding). And yes you are right, there is a limited amount of threads which is why blocking is a bad thing because having threads

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Ryanne Dolan
Matthew, it's more than performance tho. In many frameworks the number of request threads is purposefully constrained, and blocking one means you have one less to handle requests with. When you're handling a large amount of requests with a small number of threads, any blocking can lead to thread

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-26 Thread Matthew de Detrich
I think we may need to clarify terminology here, at least to me blocking means suspending a current thread to wait for some operation (which is wasteful if we are dealing with IO bound tasks). In other words, the "blocking" is an implementation detail on how to wait rather than whether we need to

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-25 Thread Nakamura
Hey Colin, For the metadata case, what would fixing the bug look like? I agree that we should fix it, but I don't have a clear picture in my mind of what fixing it should look like. Can you elaborate? Best, Moses On Mon, May 24, 2021 at 1:54 PM Colin McCabe wrote: > Hi all, > > I agree that

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-24 Thread Colin McCabe
Hi all, I agree that we should give users the option of having a fully async API, but I don't think external thread pools or queues are the right direction to go here. They add performance overheads and don't address the root causes of the problem. There are basically two scenarios where we

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-20 Thread Nakamura
> > My suggestion was just do this in multiple steps/phases, firstly let's fix > the issue of send being misleadingly asynchronous (i.e. internally its > blocking) and then later one we can make the various > threadpools configurable with a sane default. I like that approach. I updated the "Which

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-20 Thread Matthew de Detrich
@ Nakamura On Wed, May 19, 2021 at 7:35 PM Nakamura wrote: > @Ryanne: > In my mind's eye I slightly prefer the throwing the "cannot enqueue" > exception to satisfying the future immediately with the "cannot enqueue" > exception? But I agree, it would be worth doing more research. > > @Matthew:

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-19 Thread Nakamura
@Ryanne: In my mind's eye I slightly prefer the throwing the "cannot enqueue" exception to satisfying the future immediately with the "cannot enqueue" exception? But I agree, it would be worth doing more research. @Matthew: > 3. Using multiple thread pools is definitely recommended for

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-19 Thread Matthew de Detrich
Here are my two cents here (note that I am only seeing this on a surface level) 1. If we are going this road it makes sense to do this "properly" (i.e. using queues as Ryan suggested). The reason I am saying this is that it seems that the original goal of the KIP is for it to be used in other

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-18 Thread Ryanne Dolan
I was thinking the sender would typically wrap send() in a backoff/retry loop, or else ignore any failures and drop sends on the floor (fire-and-forget), and in both cases I think failing immediately is better than blocking for a new spot in the queue or asynchronously failing somehow. I think a

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-18 Thread Nakamura
Hi Ryanne, Hmm, that's an interesting idea. Basically it would mean that after calling send, you would also have to check whether the returned future had failed with a specific exception. I would be open to it, although I think it might be slightly more surprising, since right now the paradigm

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-18 Thread Ryanne Dolan
Moses, in the case of a full queue, could we just return a failed future immediately? Ryanne On Tue, May 18, 2021, 10:39 AM Nakamura wrote: > Hi Alexandre, > > Thanks for bringing this up, I think I could use some feedback in this > area. There are two mechanisms here, one for slowing down

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-18 Thread Nakamura
Hi Alexandre, Thanks for bringing this up, I think I could use some feedback in this area. There are two mechanisms here, one for slowing down when we don't have the relevant metadata, and the other for slowing down when a queue has filled up. Although the first one applies backpressure

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-16 Thread Alexandre Dupriez
Hello Nakamura, Thanks for proposing this change. I can see how the blocking behaviour can be a problem when integrating with reactive frameworks such as Akka. One of the questions I would have is how you would handle back pressure and avoid memory exhaustion when the producer's buffer is full

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-14 Thread Ryanne Dolan
Makes sense! Ryanne On Fri, May 14, 2021, 9:39 AM Nakamura wrote: > Hey Ryanne, > > I see what you're saying about serde blocking, but I think we should > consider it out of scope for this patch. Right now we've nailed down a > couple of use cases where we can unambiguously say, "I can make

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-14 Thread Nakamura
Hey Ryanne, I see what you're saying about serde blocking, but I think we should consider it out of scope for this patch. Right now we've nailed down a couple of use cases where we can unambiguously say, "I can make progress now" or "I cannot make progress now", which makes it possible to

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-13 Thread Ryanne Dolan
re serialization, my concern is that serialization often accounts for a lot of the cycles spent before returning the future. It's not blocking per se, but it's the same effect from the caller's perspective. Moreover, serde impls often block themselves, e.g. when fetching schemas from a registry.

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-13 Thread Nakamura
Hey Ryanne, I agree we should add an additional constructor (or else an additional overload in KafkaProducer#send, but the new constructor would be easier to understand) if we're targeting the "user provides the thread" approach. >From looking at the code, I think we can keep record

Re: [DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-13 Thread Ryanne Dolan
Hey Moses, I like the direction here. My thinking is that a single additional work queue, s.t. send() can enqueue and return, seems like the lightest touch. However, I don't think we can trivially process that queue in an internal thread pool without subtly changing behavior for some users. For

[DISCUSS] KIP-739: Block Less on KafkaProducer#send

2021-05-13 Thread Nakamura
Hey Folks, I just posted a new proposal in the wiki. I think we have an opportunity to improve the KafkaProducer#send user experience. It would certainly make our lives easier. Please take a look! There are two