Re: [PR] KAFKA-00000 Handle consumer close [kafka]

2023-12-05 Thread via GitHub


philipnee commented on PR #14920:
URL: https://github.com/apache/kafka/pull/14920#issuecomment-1841153219

   Hi @lucasbru - Thanks for the response. unfortunately, the way the mock is 
setup and how other tests are written, close will have dependencies to the 
network thread, which means we are actually testing all related components.   
I'll get the PR in shape and have you review it asap.


-- 
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



Re: [PR] KAFKA-00000 Handle consumer close [kafka]

2023-12-05 Thread via GitHub


lucasbru commented on PR #14920:
URL: https://github.com/apache/kafka/pull/14920#issuecomment-1840841524

   @philipnee I don't see `AsyncKafkaConsumer` failing in CI, so not sure. Have 
you fixed it already? 
   
   I'd suggest to get this PR in shape and test the close functionality 
specifically. If there is something in the existingtest setup that needs to 
change to make the close go through smoothly, we can do that (once we see it 
failing on CI), but ideally close won't be tested. I'm working on a PR to move 
some of those tests out of `AsyncKafkaConsumerTest`. Feel free to ignore that 
problem for now and just clean up this PR.


-- 
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



Re: [PR] KAFKA-00000 Handle consumer close [kafka]

2023-12-05 Thread via GitHub


philipnee commented on PR #14920:
URL: https://github.com/apache/kafka/pull/14920#issuecomment-1840284323

   Hi @lucasbru - I intended to open this for review however, I'm having a hard 
time to get testCommitAsyncLeaderEpochUpdate because it somehow seems to try to 
send a commit request with -1 epoch in it (as well as several other tests).  I 
thought the intention was to just send an async request, i wonder why is the 
epoch = -1...


-- 
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



[PR] KAFKA-00000 Handle consumer close [kafka]

2023-12-04 Thread via GitHub


philipnee opened a new pull request, #14920:
URL: https://github.com/apache/kafka/pull/14920

   When closing the consumer we need to perform a few tasks
   1. If auto-commit is enabled, send an autocommit and block until completed
   2. Invoke all offsetCommitCallbacks and ensure all inflight commits are sent
   3. Invoke partitionsRevoke callbacks
   4. Unsubscribe from all partitions
   5. LeaveGroup


-- 
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