AndrewJSchofield commented on code in PR #17980:
URL: https://github.com/apache/kafka/pull/17980#discussion_r1862726162
##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -369,11 +383,23 @@ public CompletableFuture<Map<TopicIdPartition,
ShareAcknowledgeResponseData.Part
CompletableFuture<Void> allFutures = CompletableFuture.allOf(
futuresMap.values().toArray(new CompletableFuture[0]));
- return allFutures.thenApply(v -> {
+ return allFutures.handle((results, exception) -> {
Map<TopicIdPartition, ShareAcknowledgeResponseData.PartitionData>
result = new HashMap<>();
- futuresMap.forEach((topicIdPartition, future) ->
result.put(topicIdPartition, new ShareAcknowledgeResponseData.PartitionData()
- .setPartitionIndex(topicIdPartition.partition())
- .setErrorCode(future.join().code())));
+ futuresMap.forEach((topicIdPartition, future) -> {
+ try {
+ Errors error = future.join();
+ result.put(topicIdPartition, new
ShareAcknowledgeResponseData.PartitionData()
+ .setPartitionIndex(topicIdPartition.partition())
+ .setErrorCode(error.code()));
Review Comment:
I think there are situations (such as UNKNOWN_TOPIC_OR_PARTITION) where the
future completes with a code other than Errors.NONE, but does not complete
exceptionally. I suggest that the default exception string for the error should
be used in this situation.
##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -300,11 +301,24 @@ public CompletableFuture<Map<TopicIdPartition,
ShareAcknowledgeResponseData.Part
CompletableFuture<Void> allFutures = CompletableFuture.allOf(
futures.values().toArray(new CompletableFuture[0]));
- return allFutures.thenApply(v -> {
+ return allFutures.handle((results, exception) -> {
Review Comment:
This is a CompletableFuture<Void> so `results` is not actually useful. Maybe
`unused` would be better so this is clear.
##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -369,11 +383,23 @@ public CompletableFuture<Map<TopicIdPartition,
ShareAcknowledgeResponseData.Part
CompletableFuture<Void> allFutures = CompletableFuture.allOf(
futuresMap.values().toArray(new CompletableFuture[0]));
- return allFutures.thenApply(v -> {
+ return allFutures.handle((results, exception) -> {
Review Comment:
And `unused` here again?
--
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]
