Hi Sophie Thanks for feedback. I have updated the Public Interface part accordingly. Please take a look.
Best, TengYao TengYao Chi <kiting...@gmail.com> 於 2024年9月28日 週六 下午1:26寫道： > 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 >> > >> >> > > >> > >> >
