Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-03-02 Thread Luke Chen
Hi David, If you don't have other comments, would you vote for the KIP? Thank you. Luke On Tue, Feb 22, 2022 at 3:13 PM David Jacot wrote: > Thanks, Luke! > > Le mar. 22 févr. 2022 à 08:02, Luke Chen a écrit : > > > Hi David, > > > > Thanks for the comment. > > I've updated the KIP, to add th

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-21 Thread David Jacot
Thanks, Luke! Le mar. 22 févr. 2022 à 08:02, Luke Chen a écrit : > Hi David, > > Thanks for the comment. > I've updated the KIP, to add the method will be added into `Subscription` > class: > > // new added, the generationId getter > public int generationId() { > return generationId; > } > >

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-21 Thread Luke Chen
Hi David, Thanks for the comment. I've updated the KIP, to add the method will be added into `Subscription` class: // new added, the generationId getter public int generationId() { return generationId; } Thank you. Luke On Mon, Feb 21, 2022 at 5:24 PM David Jacot wrote: > Hi Luke, > > I a

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-21 Thread David Jacot
Hi Luke, I apologize for my late reply. I was out for a while. Coming back to my previous point, could you also spell out the new method(s) that we need to add to the Subscription class? Thanks, David On Mon, Feb 14, 2022 at 6:28 PM Guozhang Wang wrote: > > Thanks Luke, no more comments from m

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-14 Thread Guozhang Wang
Thanks Luke, no more comments from me, nice work! On Mon, Feb 14, 2022 at 5:22 AM Luke Chen wrote: > Hi Guozhang, > > Thanks for your comments. I've updated the KIP. > Here's what I've updated: > > * In the motivation section, I've added this paragraph after > cooperativeStickyAssignor like this

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-14 Thread Luke Chen
Hi Guozhang, Thanks for your comments. I've updated the KIP. Here's what I've updated: * In the motivation section, I've added this paragraph after cooperativeStickyAssignor like this: *On the other hand, `StickyAssignor` is also adding "generation" field plus the "ownedPartitions" into subscri

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-08 Thread Guozhang Wang
Hello Luke, Thanks for the updated KIP, I've taken a look at it and still LGTM. Just a couple minor comments in the wiki: * Both `StickyAssignor` and `CooperativeStickyAssignor` that there's already generation is encoded in user-data bytes, the difference is that the `StickyAssignor`'s user bytes

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-07 Thread Luke Chen
Hi David, Thanks for your comments. I've updated the KIP to add changes in Subscription class. Thank you. Luke On Fri, Feb 4, 2022 at 11:43 PM David Jacot wrote: > Hi Luke, > > Thanks for updating the KIP. I just have a minor request. > Could you fully describe the changes to the Subscription

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2022-02-04 Thread David Jacot
Hi Luke, Thanks for updating the KIP. I just have a minor request. Could you fully describe the changes to the Subscription public class in the KIP? I think that it would be better than just saying that the generation will be added to id. Otherwise, the KIP LGTM. Thanks, David On Mon, Nov 29, 2

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-28 Thread Luke Chen
Hi devs, Welcome to provide feedback. If there are no other comments, I'll start a vote tomorrow. Thank you. Luke On Mon, Nov 22, 2021 at 4:16 PM Luke Chen wrote: > Hello David, > > For (3): > > > > * I suppose that we could add a `generation` field to the JoinGroupRequest > instead to do the

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-22 Thread Luke Chen
Hello David, For (3): * I suppose that we could add a `generation` field to the JoinGroupRequest instead to do the fencing that you describe while handling the sentinel in the assignor directly. If we would add the `generation` to the request itself, would we need the `generation` in the subscr

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-18 Thread Luke Chen
Hi David, Thanks for your feedback. I've updated the KIP for your comments (1)(2). For (3), it's a good point! Yes, we didn't deserialize the subscription metadata on broker side, and it's not necessary to add overhead on broker side. And, yes, I think we can fix the original issue by adding a "ge

Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-15 Thread David Jacot
Hi Luke, Thanks for the KIP. Overall, I think that the motivation makes sense. I have a couple of comments/questions: 1. In the Public Interfaces section, it would be great if you could put the end state not the current one. 2. Do we need to update the Subscription class to expose the generation

[DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-11 Thread Luke Chen
Hi all, I'd like to start the discussion for KIP-792: Add "generation" field into consumer protocol. The goal of this KIP is to allow assignor/consumer coordinator/group coordinator to have a way to identify the out-of-date members/assignments. Detailed description can be found here: https://cwi