kirktrue commented on code in PR #18089:
URL: https://github.com/apache/kafka/pull/18089#discussion_r1884678941
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java:
##########
@@ -1175,10 +1175,17 @@ private CompletableFuture<Void> assignPartitions(
// Invoke user call back.
CompletableFuture<Void> result =
signalPartitionsAssigned(addedPartitions);
+ // Enable newly added partitions to start fetching and updating
positions for them.
result.whenComplete((__, exception) -> {
if (exception == null) {
- // Enable newly added partitions to start fetching and
updating positions for them.
-
subscriptions.enablePartitionsAwaitingCallback(addedPartitions);
+ // Enable assigned partitions to start fetching and updating
positions for them.
+ // We use assignedPartitions here instead of addedPartitions
because there's a chance that the callback
+ // might throw an exception, leaving addedPartitions empty.
This would result in the poll operation
+ // returning no records, as no topic partitions are marked as
fetchable. In contrast, with the classic consumer,
+ // if the first callback fails but the next one succeeds,
polling can still retrieve data. To align with
+ // this behavior, we rely on assignedPartitions to avoid such
scenarios.
+ subscriptions.enablePartitionsAwaitingCallback(
+
assignedPartitions.stream().map(TopicIdPartition::topicPartition).collect(Collectors.toSet()));
Review Comment:
Can this be simplified to:
```suggestion
subscriptions.enablePartitionsAwaitingCallback(toTopicPartitionSet(assignedPartitions));
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
