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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to