[GitHub] [kafka] erikvanoosten commented on a diff in pull request #13678: KAFKA-10337: await async commits in commitSync even if no offsets given

["erikvanoosten (via GitHub)" <gi...@apache.org>]

erikvanoosten commented on code in PR #13678:
URL: https://github.com/apache/kafka/pull/13678#discussion_r1190730213


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java:
##########
@@ -1223,6 +1233,26 @@ public void maybeAutoCommitOffsetsAsync(long now) {
         }
     }
 
+    private boolean waitForPendingAsyncCommits(Timer timer) {

Review Comment:
   I tried to apply @philipnee's suggestion but it will be very tricky; logic will spread around and understandability will suffer a lot. Method commitSync must either return false, or wait until the pending async commits have been handled. I discovered that the test covers this aspect well. When we use the if statement and then use the rest of the code as is, it will return true (because we gave it an empty offsets) even though there are still inflight async commits. To fix this, more logic is needed. How to do this exactly is beyond me at the moment. I would prefer to not change this aspect.



-- 
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-unsubscribe@kafka.apache.org

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