Hi, David, Thanks for the reply.
The KIP mentioned downgrade support in a future KIP. So, with this KIP, once the new records have been generated on the coordinator, there is no path to downgrade the broker, is that correct? Thanks, Jun On Tue, Oct 25, 2022 at 7:10 AM David Jacot <dja...@confluent.io.invalid> wrote: > 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 > > > >
