Hi Matthias, Thanks for the explanation, particularly regarding the important considerations for both the plain consumer and Kafka Streams use cases.
In this case, I think it would be better to stick with my initial proposal. We should give plain consumers the ability to determine whether to send a leave group request or not, with clear documentation highlighting the potential downsides. This could also provide flexibility for future features. Best, TengYao Chia-Ping Tsai <chia7...@gmail.com> 於 2024年9月28日 週六 上午3:27寫道： > hi Matthias > > > 100: Why do we want to distinguish between the classic and the new async > consumer? Should they not have the same (user facing) behavior? Or maybe > I misunderstand something. Can one catch we up what epoch "-1" vs epoch > "-2" means? > > I apologize for any confusion in my earlier explanation. The way a consumer > leaves a group varies between the Classic Consumer and the Async Consumer: > > - The *Classic Consumer* uses a LeaveGroupRequest but does *not* send this > request for static members. > > - In contrast, the *Async Consumer* sends a ConsumerGroupHeartbeatRequest. > If the member is static, this request is sent with an epoch value of -2, > indicating that the static member has temporarily left the group and is > *not* removed. An epoch of -1 in the CONSUMER protocol signifies that the > static member is treated as a dynamic member and will leave the group > completely. > > > > Hence, even if not useful for the plain consumer to disable sending a > leave-group-request, it might be worth to add a generic enable/disable API > for both dynamic and static groups, so KS can use this API (and we could > remove the internal consumer config, which is a workaround anyway). > > I agree that having a generic enable/disable API would be beneficial, > especially if we can provide comprehensive documentation. This > documentation should clearly outline the potential downsides of not sending > a LEAVE_REQUEST for dynamic members, ensuring users are well-informed about > the implications of their choices. > > > Best, > > Chia-Ping > > > > > Matthias J. Sax <mj...@apache.org> 於 2024年9月28日 週六 上午2:07寫道： > > > Thanks for the KIP. Two questions/comments: > > > > > > 100: Why do we want to distinguish between the classic and the new async > > consumer? Should they not have the same (user facing) behavior? Or maybe > > I misunderstand something. Can one catch we up what epoch "-1" vs epoch > > "-2" means? > > > > > > 101: I think we need to distinguish between the plain consumer and the > > KS case. > > > > Plain consumer: for this case, atm user don't have control at all, but > > it's hard coded when a leave group request is sent. If we only consider > > this case, the current KIP to allow sending a leave-group request for > > static members is sufficient. I agree that disabling leave-group request > > for dynamic member is not necessary for the plain consumer case. > > > > However, for the KS case it's different. Because KS uses the internal > > config to disable sending leave group request for dynamic members, we > > lack an user facing API to enable sending a leave group request for this > > case, and if we only allow to enable sending leave group request for > > static members on the consumer, the KIP would fall short to close this > gap. > > > > Hence, even if not useful for the plain consumer to disable sending a > > leave-group-request, it might be worth to add a generic enable/disable > > API for both dynamic and static groups, so KS can use this API (and we > > could remove the internal consumer config, which is a workaround anyway). > > > > > > > > On the other hand, given the light of KIP-1088, maybe there are other > > ways to fix it on the KS side? I think the goal should be to remove the > > internal consumer config (as it's static, and we cannot overwrite it at > > runtime), and to give KS a way to dynamically send a leave-group-request > > on close() -- but maybe we could build this on an internal consumer API, > > and not make it public? For this case, the current KIP would be > sufficient. > > > > > > > > > > -Matthias > > > > > > On 9/26/24 8:19 PM, Sophie Blee-Goldman wrote: > > > Thanks for the KIP! Quick request for readability, can you please > include > > > the exact APIs that you're proposing to add or change under the "Public > > > Interfaces" section? The KIP should display the actual method signature > > and > > > any applicable javadocs for new public APIs. > > > > > > You can look at other KIPs for a clear sense of what it should contain, > > but > > > here's one example you could work from: > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1036%3A+Extend+RecordDeserializationException+exception > > > > > > On Thu, Sep 26, 2024 at 6:22 PM Chia-Ping Tsai <chia7...@gmail.com> > > wrote: > > > > > >>> I think I’m actually OK with leaving it as leaveGroup with a lot of > > >> documentation that warns users away from changing it arbitrarily. > > >> > > >> Pardon me, I just want to ensure we are all on the same page. > > >> > > >> 1. `leaveGroup=true`: `ClassicKafkaConsumer` sends a > > >> `LeaveGroupRequest` for either the dynamic or static member. > > >> 2. `leaveGroup=false`: `ClassicKafkaConsumer` does not send any ` > > >> LeaveGroupRequest` for either the dynamic or static member. > > >> 3. `leaveGroup=default` (current behavior): `ClassicKafkaConsumer` > > sends > > >> a `LeaveGroupRequest` for dynamic member, and it does NOT send any > > >> `ConsumerGroupHeartbeatRequest`for static member > > >> 4. `leaveGroup=true`: `AsyncKafkaConsumer` sends a > > >> `ConsumerGroupHeartbeatRequest` with "-1" epoch for either the > > dynamic > > >> or > > >> static member > > >> 5. `leaveGroup=false`: `AsyncKafkaConsumer` sends a > > >> `ConsumerGroupHeartbeatRequest` with "-2" epoch for the static > > member, > > >> and > > >> it does NOT send any `ConsumerGroupHeartbeatRequest` for dynamic > > member > > >> 6. `leaveGroup=default` (current behavior): `AsyncKafkaConsumer` > > sends a > > >> `ConsumerGroupHeartbeatRequest`with "-1" epoch for dynamic member > > and > > >> "-2" epoch for static member > > >> > > >> Best, > > >> Chia-Ping > > >> > > > > > >
