Guozhang / Ismael, yes agreed on the plurality of the naming. I've updated the KIP.
Ron, idempotent allocations are certainly possible, but as you pointed out it might not be needed. It would require some additional book-keeping by the controller to recall what was the last producer ID block allocated for each broker. It would also open us up to bugs where the same ID block could be given out repeatedly in the case of a broker providing some wrong information in its request. It also doesn't help in the case of a broker restarting since the broker won't know what its last block was (unless it adds some local state management). Generally, I think the added complexity is not worth it. The RPC rate limiting should be sufficient to protect us from exhausting the ID space. If you agree, I can update the discussion section of the KIP with this conclusion. Thanks for the feedback so far, everyone! -David On Wed, Apr 14, 2021 at 4:37 PM Ismael Juma <ism...@juma.me.uk> wrote: > Hi Guozhang, > > That was my original suggestion, so I am naturally +1 :) > > Ismael > > On Wed, Apr 14, 2021 at 11:44 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > Hi David, > > > > Just putting my paranoid hat here :) Could we name the req/resp name as > > "AllocateProducerIds" instead of "AllocateProducerId"? Otherwise, LGTM! > > > > Guozhang > > > > On Thu, Apr 8, 2021 at 2:23 PM Ron Dagostino <rndg...@gmail.com> wrote: > > > > > Hi David. I'm wondering if it might be a good idea to have the broker > > > send information about the last block it successfully received when it > > > requests a new block. As the RPC stands right now it can't be > > > idempotent -- it just tells the controller "provide me a new block, > > > please". One case where it might be useful for the RPC to be > > > idempotent is if the broker never receives the response from the > > > controller such that it asks again. That would result in the burning > > > of the block that the controller provided but that the broker never > > > received. Now, granted, the ID space is 64 bits, so we would have to > > > make ~2^54 requests to burn the entire space, and that isn't going to > > > happen. So whether this is actually needed is questionable. And it > > > might not be worth it to write the controller side code to make it act > > > idempotently even if we added the request field to make it possible. > > > But I figured this is worth mentioning even if we explicitly decide to > > > reject it. > > > > > > Ron > > > > > > On Thu, Apr 8, 2021 at 3:16 PM Ron Dagostino <rndg...@gmail.com> > wrote: > > > > > > > > Oh, I see. Yes, my mistake -- I read it wrong. You are right that > > > > all we need in the metadata log is the latest value allocated. > > > > > > > > Ron > > > > > > > > On Thu, Apr 8, 2021 at 11:21 AM David Arthur <mum...@gmail.com> > wrote: > > > > > > > > > > Ron -- I considered making the RPC response and record use the same > > (or > > > > > very similar) fields, but the use case is slightly different. A > > broker > > > > > handling the RPC needs to know the bounds of the block since it has > > no > > > idea > > > > > what the block size is. Also, the brokers will normally see > > > non-contiguous > > > > > blocks. > > > > > > > > > > For the metadata log, we can just keep track of the latest producer > > Id > > > that > > > > > was allocated. It's kind of like a high watermark for producer IDs. > > > This > > > > > actually saves us from needing an extra field in the record (the > KIP > > > has > > > > > just ProducerIdEnd => int64 in the record). > > > > > > > > > > Does that make sense? > > > > > > > > > > On Wed, Apr 7, 2021 at 8:44 AM Ron Dagostino <rndg...@gmail.com> > > > wrote: > > > > > > > > > > > Thanks for the KIP, David. > > > > > > > > > > > > With the RPC returning a start and length, should the record in > the > > > > > > metadata log do the same thing for consistency and to save the > byte > > > > > > per record? > > > > > > > > > > > > Ron > > > > > > > > > > > > > > > > > > On Tue, Apr 6, 2021 at 11:06 PM Ismael Juma <ism...@juma.me.uk> > > > wrote: > > > > > > > > > > > > > > Great, thanks. Instead of calling it "bridge release", can we > say > > > 3.0? > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > On Tue, Apr 6, 2021 at 7:48 PM David Arthur <mum...@gmail.com> > > > wrote: > > > > > > > > > > > > > > > Thanks for the feedback, Ismael. Renaming the RPC and using > > > start+len > > > > > > > > instead of start+end sounds fine. > > > > > > > > > > > > > > > > And yes, the controller will allocate the IDs in ZK mode for > > the > > > bridge > > > > > > > > release. > > > > > > > > > > > > > > > > I'll update the KIP to reflect these points. > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > On Tue, Apr 6, 2021 at 7:30 PM Ismael Juma < > ism...@juma.me.uk> > > > wrote: > > > > > > > > > > > > > > > > > Sorry, one more question: the allocation of ids will be > done > > > by the > > > > > > > > > controller even in ZK mode, right? > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > On Tue, Apr 6, 2021 at 4:26 PM Ismael Juma < > > ism...@juma.me.uk> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > One additional comment: if you return the number of ids > > > instead of > > > > > > the > > > > > > > > > end > > > > > > > > > > range, you can use an int32. > > > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > > > On Tue, Apr 6, 2021 at 4:25 PM Ismael Juma < > > > ism...@juma.me.uk> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > >> Thanks for the KIP, David. Any reason not to rename > > > > > > > > > >> AllocateProducerIdBlockRequest to > > > AllocateProducerIdsRequest? > > > > > > > > > >> > > > > > > > > > >> Ismael > > > > > > > > > >> > > > > > > > > > >> On Tue, Apr 6, 2021 at 3:51 PM David Arthur < > > > mum...@gmail.com> > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > >>> Hello everyone, > > > > > > > > > >>> > > > > > > > > > >>> I'd like to start the discussion for KIP-730 > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-730%3A+Producer+ID+generation+in+KRaft+mode > > > > > > > > > >>> > > > > > > > > > >>> This KIP proposes a new RPC for generating blocks of > IDs > > > for > > > > > > > > > >>> transactional > > > > > > > > > >>> and idempotent producers. > > > > > > > > > >>> > > > > > > > > > >>> Cheers, > > > > > > > > > >>> David Arthur > > > > > > > > > >>> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > David Arthur > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > David Arthur > > > > > > > > > -- > > -- Guozhang > > > -- David Arthur
