I've updated the KIP with a section on idempotence to reflect Ron's comments in this thread. I'm going to open the vote thread shortly.
Thanks! David On Fri, Apr 16, 2021 at 7:04 PM Ron Dagostino <rndg...@gmail.com> wrote: > Thanks, David. Yeah, I agree. I was more bringing it up to make sure we > explicitly discussed it. > > Ron > > > On Apr 16, 2021, at 2:15 PM, David Arthur <mum...@gmail.com> wrote: > > > > 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 > -- David Arthur
