Hi Jun, 90.1 You're right. That's a miss on my side. 90.2 That makes sense. Relying on ConsumerGroupTargetAssignmentMetadataValue.AssignmentEpoch should be enough here.
91. When a client side assignor is used, the assignment is computed asynchronously. While it is computed for the group at epoch X, the group may have already advanced to epoch X+1 due to another event (e.g. new member joined). In this case, we have chosen to install the assignment computed for epoch X and to trigger a new assignment computation right away. So if the group topology changes rapidly, the assignment may lag a bit behind but it will eventually converge. For context, the alternative would have been to cancel the assignment and to trigger a new one immediately but this approach is prone to a live lock. For instance, if a member joins and leaves all the time, there is a chance that a new assignment is never computed. I have extended that phrase to be more explicit. Thanks for the comments. Best, David On Mon, Oct 24, 2022 at 7:28 PM Jun Rao <j...@confluent.io.invalid> wrote: > > Hi, David, > > Thanks for the updated KIP. A few more comments. > > 90. ConsumerGroupTargetAssignmentMemberValue: > 90.1 Do we need to include MemberId here given that it's in the key already? > 90.2 Since there is no new record if the new member assignment is the same, > it seems that AssignmentEpoch doesn't always reflect the correct assignment > epoch? If so, do we still need this field? Could we just depend > on ConsumerGroupTargetAssignmentMetadataValue.AssignmentEpoch? > > 91. "The AssignmentEpoch corresponds to the group epoch used to compute the > assignment. It is not necessarily the last one." Could you explain what "It > is not necessarily the last one." means? > > Thanks, > > Jun > > > On Mon, Oct 24, 2022 at 7:49 AM Magnus Edenhill <mag...@edenhill.se> wrote: > > > Hi, one minor comment on the latest update: > > > > > > Den mån 24 okt. 2022 kl 16:26 skrev David Jacot > > <dja...@confluent.io.invalid > > >: > > > > > * Jason pointed out that the member id handling is a tad weird. The > > > group coordinator generates the member id and then trusts the member > > > when it rejoins the group. This also implies that the client could > > > directly generate its member id and the group coordinator will accept > > > it. It seems better to directly let the client generate id instead of > > > relying on the group coordinator. I have updated the KIP in this > > > direction. Note that the new APIs still use a string for the member id > > > in order to remain consistent with the existing APIs. > > > > > > > We had a similar discussion for id generation in KIP-714 and I'd advise > > against client-side id generation for a couple of reasons: > > - it is much more likely for the client side prng to be poorly seeded, or > > incorrectly implemented, than the server side. > > This risks two different consumer instances generating the same id. > > - it adds an extra dependency on the client, a uuid library/module, which > > brings with it the usual plethora > > of linking conflicts, package availability issues, etc. > > - as for trusting the authenticity of the id; with server-side generation > > we at least have a (future) possibility for verifying the id, would it ever > > become an issue. > > > > > > Regards, > > Magnus > >
