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 "generation" field into `JoinGroupRequest` instead, and also add a field into `JoinGroupResponse` in `JoinGroupResponseMember` field. That way, the broker can identify the old member from `JoinGroupRequest`. And the assignor can also get the "generation" info via the `Subscription` instance.
I'll update the KIP to add "generation" field into `JoinGroupRequest` and `JoinGroupResponse`, if there is no other options. Thank you. Luke On Tue, Nov 16, 2021 at 12:31 AM David Jacot <dja...@confluent.io.invalid> wrote: > 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? If so, it would be great to mention it in the Public > Interfaces section as well. > > 3. You mention that the broker will set the generation if the subscription > contains a sentinel value (-1). As of today, the broker does not parse > the subscription so I am not sure how/why we would do this. 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 subscription protocol as well? > > Best, > David > > On Fri, Nov 12, 2021 at 3:31 AM Luke Chen <show...@gmail.com> wrote: > > > > 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://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614 > > > > Any feedback is welcome. > > > > Thank you. > > Luke >
