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
