lianetm commented on PR #16686: URL: https://github.com/apache/kafka/pull/16686#issuecomment-2305216018
One high level thought. With this PR we're ignoring the interrupted status temporarily, to allow the close to do what it needs (and ensure it cleanly leaves the group), makes sense. But in that case, we're allowing the close to run with whatever timeout was provided in the api call to close (or the default api timeout). Wouldn't make more sense for this interrupt+close(timeout) to behave exactly like close(0)? (instead of behaving like close(timeout) like it does now). This would be the only change: `final Timer closeTimer = wasInterrupted ? time.timer(Duration.ZERO) : time.timer(timeout);` Seems like a good compromise between respecting the interrupted status, while also attempting to close cleanly (assuming that close(0) does send the leave, this is why I was suggesting we also add an IT for it). Thoughts? -- 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
