lianetm commented on code in PR #18737: URL: https://github.com/apache/kafka/pull/18737#discussion_r1954732895
########## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java: ########## @@ -818,6 +826,8 @@ void maybeReconcile() { return; } + if (autoCommitEnabled && !canCommit) return; Review Comment: > we could apply the same approach we use for auto-commit and auto-commit on revocation. (offsetsReady) Could it be simpler here? The commit event has a future that only completes when the request gets a response, so we couldn't wait for that on the app thread. But the `PollEvent` is very different, it doesn't even have a future at the moment (it only performs actions locally). So wouldn't it be enough to make the `PollEvent` completable and wait for it (meaning it would wait for local actions that need to happen on the background, no network-io at all). This is a key point btw, we're not performing any network IO when processing a `PollEvent`, even if we wait for it's completion in the app thread, because the `PollEvent` only generates async commits. No other request involved, never waits for a response. I could be missing something, but seems to me that we wouldn't be breaking the separation of responsabilities (background thread is still the one in charge of the network IO in this case, because the network IO will happen when we poll the `commitRequestManager`, that will send the async commit that the `PollEvent` generated. The app thread just waited for the generation, to ensure that it could move into the fetching phase) All that being said, the fetching happening in the app thread is probably the elephant in the room here, but that is definitely food for thought for after 4.0 :) -- 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