kirktrue commented on PR #16686:
URL: https://github.com/apache/kafka/pull/16686#issuecomment-2311407402
Hi @lianetm!
> > the proposed behavior would be divergent from the existing consumer’s
behavior
>
> uhm why? I could be missing something, but actually I see it as the
proposed behaviour is to make both consumer similar on close+interrupt: send
leave request and do not wait for broker responses.
Agreed 😄
> That's what the classic consumer achieves by throwing the interrupt right
after polling the network client.
I apologize if I've overlooked it, but I haven't seen where the classic
consumer checks the current thread’s interrupted state (or catches
`InterruptException`/`InterruptedException`) and then explicitly (or
implicitly) ignores the provided timeout 🤔
For what it's worth, this is the code path I've followed to understand the
behavior of the classic consumer leaving the group on close:
```
ClassicKafkaConsumer.close() →
ConsumerCoordinator.close() →
AbstractCoordinator.close() →
AbstractCoordinator.maybeLeaveGroup()
```
Please correct me if I'm looking in the wrong place.
> With the current shape of the PR, we don't achieve the same with the new
consumer, and actually, we introduce an unwanted behaviour imo: interrupted
thread + close sits and wait for broker responses...response for the leave
request, or any other in-flight request there may be.
I'm not aware of anywhere in
`AsyncKafkaConsumer.releaseAssignmentAndLeaveGroup()` where we wait for the
response from the server 🤔
Are you referring to the fact that we now process the background events in
`releaseAssignmentAndLeaveGroup()` after the `UnsubscribeEvent`? At present
there's no direct way for the application thread to tell the background thread
to leave the group. Leaving the consumer group is part of the overall process
driven by `UnsubscribeEvent`, isn't it? Even still, the unsubscribe process
needs to invoke the user's `ConsumerRebalanceListener` callbacks, right?
> Is there an issue/concern with the alternative of calling close(0) if
interrupted?
I hate to be stubborn here, but my issue is that I don't see that the
classic consumer behaves this way and/or it's documented as the intention. I'm
happy to be proven otherwise, of course.
> How does the IT behave if we try the change?
I have yet not tested the scenario of ignoring the timeout on interrupt. If
we decide that's the direction we want to go, the tests will be updated in due
course.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
