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

Reply via email to