Closing this vote. With four binding +1s and zero -1s, this vote passes. Binding +1: * David Arthur * Guozhang Wang * Tom Bentley * Colin McCabe
Non-binding +1: * Ron Dagostino Thanks everyone! David On Tue, May 11, 2021 at 1:07 PM Colin McCabe <cmcc...@apache.org> wrote: > Thanks, David. I don't feel strongly about preserving the extra records > in the snapshot, so let's just leave it as is. +1 (binding) > > cheers, > Colin > > > On Fri, May 7, 2021, at 09:51, David Arthur wrote: > > Colin, thanks for the feedback. I like the record name you proposed. I've > > also updated the first paragraph in the Controller section to: > > > > In both ZK and KRaft modes, the controller will now be responsible for > > > generating new blocks of IDs and persisting the latest generated > block. In > > > ZK mode, the controller will use the existing ZNode and JSON format for > > > persistence. In KRaft mode, the controller will commit a new record to > the > > > metadata log. Since the controller (in either mode) uses a single > threaded > > > event model, we can simply calculate the next block of IDs based on > what is > > > currently in memory. The controller will need to persist generated PID > > > block so it can be “consumed” and never used again. > > > > > > > This doesn't change any semantics of the KIP, just clarifies the > > implementation (as you suggested). > > > > As for including extra records in the snapshot, I agree it might be > > useful, but I wonder if it might violate assumptions about snapshots. If > > snapshots are meant to contain a minimal set of records to represent the > > state of the metadata, including additional records (one per broker in > this > > case) might go against that. Either way, can we consider this an > > implementation detail and defer the decision? > > > > Thanks to all who have voted so far! > > -David > > > > On Thu, May 6, 2021 at 4:01 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > Sorry, I meant to write "AllocateProducerIdsRecord" in the previous > > > message. -C. > > > > > > On Thu, May 6, 2021, at 12:58, Colin McCabe wrote: > > > > Hi David, > > > > > > > > Thanks for the KIP -- it looks good. > > > > > > > > It seems like we should be clear that the new RPC should be used for > > > > both the ZK and KRaft cases. I think that is implied, but it would > be > > > > good to spell it out just to be clear. As the KIP explains, this is > > > > needed for the bridge release. > > > > > > > > I think AllocateProducersIdRecord would be a nicer name than > > > > ProducerIdRecord -- what do you think? > > > > > > > > In the snapshot, does it make sense to store the latest producer ID > > > > allocation record for every broker? This might be useful for > debugging > > > > purposes, and it's unlikely to be that many records.... On the other > > > > hand, as you mention, we only really need the highest one for > > > > correctness. > > > > > > > > best, > > > > Colin > > > > > > > > > > > > On Thu, May 6, 2021, at 11:53, Tom Bentley wrote: > > > > > Hi David, > > > > > > > > > > Thanks for the KIP, +1 binding. > > > > > > > > > > Tom > > > > > > > > > > On Thu, May 6, 2021 at 7:16 PM Guozhang Wang <wangg...@gmail.com> > > > wrote: > > > > > > > > > > > LGTM! Thanks David. > > > > > > > > > > > > On Thu, May 6, 2021 at 10:03 AM Ron Dagostino <rndg...@gmail.com > > > > > wrote: > > > > > > > > > > > > > Thanks again for the KIP, David. +1 (non-binding) from me. > > > > > > > > > > > > > > Ron > > > > > > > > > > > > > > On Tue, May 4, 2021 at 11:21 AM David Arthur <mum...@gmail.com > > > > > wrote: > > > > > > > > > > > > > > > Hello everyone, I'd like to start the vote on KIP-730 which > adds > > > a new > > > > > > > RPC > > > > > > > > for producer ID generation in KRaft mode. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-730%3A+Producer+ID+generation+in+KRaft+mode > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > David Arthur > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -- Guozhang > > > > > > > > > > > > > > > > > > > > > > > > -- > > David Arthur > > > -- David Arthur
