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
