frankvicky commented on code in PR #20324: URL: https://github.com/apache/kafka/pull/20324#discussion_r2279388884
########## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java: ########## @@ -243,8 +237,11 @@ public CompletableFuture<Boolean> updateFetchPositions(long deadlineMs) { if (subscriptionState.hasAllFetchPositions()) { // All positions are already available + commitOffsetsSharedState.setSubscriptionHasAllFetchPositions(true); Review Comment: > What about continuing relying on the source of truth (subscriptionState.hasAllFetchPositions), but piggyback on the PollEvent to get it safely (we need that event anyways). Agree. The new class has introduced additional complexity; it would be nice if we could leverage the current event. IIUC, `CommitOffsetsSharedState` appears to act as a signal between the app thread and the network thread. If so, it will serve the same purpose as the `ApplicationEvent`. ########## clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java: ########## @@ -243,8 +237,11 @@ public CompletableFuture<Boolean> updateFetchPositions(long deadlineMs) { if (subscriptionState.hasAllFetchPositions()) { // All positions are already available + commitOffsetsSharedState.setSubscriptionHasAllFetchPositions(true); Review Comment: > What about continuing relying on the source of truth (subscriptionState.hasAllFetchPositions), but piggyback on the PollEvent to get it safely (we need that event anyways). Agree. The new class has introduced additional complexity; it would be nice if we could leverage the current event. IIUC, `CommitOffsetsSharedState` appears to act as a signal between the app thread and the network thread. If so, it will serve the same purpose as the `ApplicationEvent`. -- 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