Thanks David! So, with 3 binding +1 votes (David, Tom, Guozhang), the vote passes.
Thanks everyone! Luke On Thu, Mar 3, 2022 at 4:34 PM David Jacot <dja...@confluent.io.invalid> wrote: > +1 (binding). Thanks for the KIP! > > On Fri, Jan 28, 2022 at 6:13 PM Tom Bentley <tbent...@redhat.com> wrote: > > > > Hi Luke, > > > > Thanks for the KIP, +1 (binding). > > > > Kind regards, > > > > Tom > > > > On Wed, 19 Jan 2022 at 13:16, Luke Chen <show...@gmail.com> wrote: > > > > > Hi all, > > > > > > Bump this thread to see if there are other comments to this KIP. > > > So far, I have one +1 vote (binding), and need more votes. > > > > > > Thank you. > > > Luke > > > > > > On Tue, Dec 21, 2021 at 10:33 AM Luke Chen <show...@gmail.com> wrote: > > > > > > > Hi all, > > > > > > > > Bump this thread to see if there are other comments to this KIP. > > > > > > > > Thank you. > > > > Luke > > > > > > > > On Fri, Dec 17, 2021 at 1:27 AM Colin McCabe <cmcc...@apache.org> > wrote: > > > > > > > >> Thanks for the explanation, Luke. That makes sense. > > > >> > > > >> best, > > > >> Colin > > > >> > > > >> On Thu, Dec 9, 2021, at 13:31, Guozhang Wang wrote: > > > >> > Thanks Luke, in that case I'm +1 on this KIP. > > > >> > > > > >> > On Thu, Dec 9, 2021 at 1:46 AM Luke Chen <show...@gmail.com> > wrote: > > > >> > > > > >> >> Hi Guozhang, > > > >> >> > > > >> >> Thanks for your comment. > > > >> >> > > > >> >> > we need to make sure the old-versioned leader would be able to > > > >> ignore the > > > >> >> new > > > >> >> field during an upgrade e.g. without crashing. > > > >> >> > > > >> >> Yes, I understand. I'll be careful to make sure it won't crash > the > > > old > > > >> >> versioned leader. > > > >> >> But basically, it won't, because we appended the new field into > the > > > >> last of > > > >> >> the ConsumerProtocolSubscription, which means, when > read/deserialize > > > >> the > > > >> >> Subscription metadata, the old versioned leader will just read > the > > > head > > > >> >> part of the data. > > > >> >> > > > >> >> Thanks for the reminder! > > > >> >> > > > >> >> Luke > > > >> >> > > > >> >> On Thu, Dec 9, 2021 at 4:00 AM Guozhang Wang <wangg...@gmail.com > > > > > >> wrote: > > > >> >> > > > >> >> > Hi Luke, > > > >> >> > > > > >> >> > Thanks for the KIP. > > > >> >> > > > > >> >> > One thing I'd like to double check is that, since the > > > >> >> > ConsumerProtocolSubscription is not auto generated from the > json > > > >> file, we > > > >> >> > need to make sure the old-versioned leader would be able to > ignore > > > >> the > > > >> >> new > > > >> >> > field during an upgrade e.g. without crashing. Other than > that, the > > > >> KIP > > > >> >> > lgtm. > > > >> >> > > > > >> >> > Guozhang > > > >> >> > > > > >> >> > On Tue, Dec 7, 2021 at 6:16 PM Luke Chen <show...@gmail.com> > > > wrote: > > > >> >> > > > > >> >> > > Hi Colin, > > > >> >> > > > > > >> >> > > I'm not quite sure if I understand your thoughts correctly. > > > >> >> > > If I was wrong, please let me know. > > > >> >> > > > > > >> >> > > Also, I'm not quite sure how I could lock this feature to a > new > > > IBP > > > >> >> > > version. > > > >> >> > > I saw "KIP-584: Versioning scheme for features" is still > under > > > >> >> > development. > > > >> >> > > Not sure if I need to lock the IBP version, how should I do? > > > >> >> > > > > > >> >> > > Thank you. > > > >> >> > > Luke > > > >> >> > > > > > >> >> > > On Tue, Dec 7, 2021 at 9:41 PM Luke Chen <show...@gmail.com> > > > >> wrote: > > > >> >> > > > > > >> >> > > > Hi Colin, > > > >> >> > > > > > > >> >> > > > Thanks for your comments. I've updated the KIP to mention > about > > > >> the > > > >> >> KIP > > > >> >> > > > won't affect current broker side behavior. > > > >> >> > > > > > > >> >> > > > > One scenario that we need to consider is what happens > during > > > a > > > >> >> > rolling > > > >> >> > > > upgrade. If the coordinator moves back and forth between > > > brokers > > > >> with > > > >> >> > > > different IBPs, it seems that the same epoch numbers could > be > > > >> reused > > > >> >> > for > > > >> >> > > a > > > >> >> > > > group, if things are done in the obvious manner (old IBP = > > > don't > > > >> read > > > >> >> > or > > > >> >> > > > write epoch, new IBP = do) > > > >> >> > > > > > > >> >> > > > I think this KIP doesn't care about the group epoch number > at > > > >> all. > > > >> >> The > > > >> >> > > > subscription metadata is passed from each member to group > > > >> >> coordinator, > > > >> >> > > and > > > >> >> > > > then the group coordinator pass all of them back to the > > > consumer > > > >> >> lead. > > > >> >> > So > > > >> >> > > > even if the epoch number is reused in a group, it should be > > > >> fine. On > > > >> >> > the > > > >> >> > > > other hand, the group coordinator will have no idea if the > join > > > >> group > > > >> >> > > > request sent from consumer containing the new subscription > > > >> >> "generation" > > > >> >> > > > field or not, because group coordinator won't deserialize > the > > > >> >> metadata. > > > >> >> > > > > > > >> >> > > > I've added also added them into the KIP. > > > >> >> > > > > > > >> >> > > > Thank you. > > > >> >> > > > Luke > > > >> >> > > > > > > >> >> > > > On Mon, Dec 6, 2021 at 10:39 AM Colin McCabe < > > > cmcc...@apache.org > > > >> > > > > >> >> > wrote: > > > >> >> > > > > > > >> >> > > >> Hi Luke, > > > >> >> > > >> > > > >> >> > > >> Thanks for the explanation. > > > >> >> > > >> > > > >> >> > > >> I don't see any description of how the broker decides to > use > > > >> the new > > > >> >> > > >> version of ConsumerProtocolSubscription or not. This > probably > > > >> needs > > > >> >> to > > > >> >> > > be > > > >> >> > > >> locked to a new IBP version. > > > >> >> > > >> > > > >> >> > > >> One scenario that we need to consider is what happens > during a > > > >> >> rolling > > > >> >> > > >> upgrade. If the coordinator moves back and forth between > > > brokers > > > >> >> with > > > >> >> > > >> different IBPs, it seems that the same epoch numbers > could be > > > >> reused > > > >> >> > > for a > > > >> >> > > >> group, if things are done in the obvious manner (old IBP = > > > don't > > > >> >> read > > > >> >> > or > > > >> >> > > >> write epoch, new IBP = do). > > > >> >> > > >> > > > >> >> > > >> best, > > > >> >> > > >> Colin > > > >> >> > > >> > > > >> >> > > >> > > > >> >> > > >> On Fri, Dec 3, 2021, at 18:46, Luke Chen wrote: > > > >> >> > > >> > Hi Colin, > > > >> >> > > >> > Thanks for your comment. > > > >> >> > > >> > > > > >> >> > > >> >> How are we going to avoid the situation where the > broker > > > >> >> restarts, > > > >> >> > > and > > > >> >> > > >> > the same generation number is reused? > > > >> >> > > >> > > > > >> >> > > >> > Actually, this KIP doesn't have anything to do with the > > > >> brokers. > > > >> >> The > > > >> >> > > >> > "generation" field I added, is in the subscription > metadata, > > > >> which > > > >> >> > > will > > > >> >> > > >> not > > > >> >> > > >> > be deserialized by brokers. The metadata is only > > > deserialized > > > >> by > > > >> >> > > >> consumer > > > >> >> > > >> > lead. And for the consumer lead, the only thing the lead > > > cared > > > >> >> > about, > > > >> >> > > is > > > >> >> > > >> > the highest generation of the ownedPartitions among all > the > > > >> >> > consumers. > > > >> >> > > >> With > > > >> >> > > >> > the highest generation of the ownedPartitions, the > consumer > > > >> lead > > > >> >> can > > > >> >> > > >> > distribute the partitions as sticky as possible, and > most > > > >> >> > importantly, > > > >> >> > > >> > without errors. > > > >> >> > > >> > > > > >> >> > > >> > That is, after this KIP, if the broker restarts, and the > > > same > > > >> >> > > generation > > > >> >> > > >> > number is reused, it won't break current rebalance > behavior. > > > >> But > > > >> >> > it'll > > > >> >> > > >> help > > > >> >> > > >> > the consumer lead do the sticky assignments correctly. > > > >> >> > > >> > > > > >> >> > > >> > Thank you. > > > >> >> > > >> > Luke > > > >> >> > > >> > > > > >> >> > > >> > On Fri, Dec 3, 2021 at 6:30 AM Colin McCabe < > > > >> co...@cmccabe.xyz> > > > >> >> > > wrote: > > > >> >> > > >> > > > > >> >> > > >> >> How are we going to avoid the situation where the > broker > > > >> >> restarts, > > > >> >> > > and > > > >> >> > > >> the > > > >> >> > > >> >> same generation number is reused? > > > >> >> > > >> >> > > > >> >> > > >> >> best, > > > >> >> > > >> >> Colin > > > >> >> > > >> >> > > > >> >> > > >> >> On Tue, Nov 30, 2021, at 16:36, Luke Chen wrote: > > > >> >> > > >> >> > Hi all, > > > >> >> > > >> >> > > > > >> >> > > >> >> > I'd like to start the vote for KIP-792: Add > "generation" > > > >> field > > > >> >> > into > > > >> >> > > >> >> > consumer protocol. > > > >> >> > > >> >> > > > > >> >> > > >> >> > The goal of this KIP is to allow the > assignor/consumer > > > >> >> > coordinator > > > >> >> > > to > > > >> >> > > >> >> have > > > >> >> > > >> >> > a way to identify the out-of-date > members/assignments, to > > > >> avoid > > > >> >> > > >> rebalance > > > >> >> > > >> >> > stuck issues in current protocol. > > > >> >> > > >> >> > > > > >> >> > > >> >> > Detailed description can be found here: > > > >> >> > > >> >> > > > > >> >> > > >> >> > > > >> >> > > >> > > > >> >> > > > > > >> >> > > > > >> >> > > > >> > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614 > > > >> >> > > >> >> > > > > >> >> > > >> >> > Any feedback is welcome. > > > >> >> > > >> >> > > > > >> >> > > >> >> > Thank you. > > > >> >> > > >> >> > Luke > > > >> >> > > >> >> > > > >> >> > > >> > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > > >> >> > -- > > > >> >> > -- Guozhang > > > >> >> > > > > >> >> > > > >> > > > > >> > > > > >> > -- > > > >> > -- Guozhang > > > >> > > > > > > > >
