hi TengYao Could you please consider adding a "default" behavior for leaveGroup? As I previously mentioned, leaveGroup=true is not ideal as the default for static members, and similarly, leaveGroup=false is not suitable as the default for dynamic members.
Maybe we could change the type of leaveGroup to Optional<Boolean>, allowing it to represent three distinct behaviors. Best, Chia-Ping TengYao Chi <kiting...@gmail.com> 於 2024年9月29日 週日 上午12:51寫道： > 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 > >> > >> > >> > > > >> > > >> > > >
