dongxiaoman commented on a change in pull request #8272:
URL: https://github.com/apache/pinot/pull/8272#discussion_r817416806



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/StreamingReduceService.java
##########
@@ -106,15 +108,17 @@ public BrokerResponseNative 
reduceOnStreamResponse(BrokerRequest brokerRequest,
     return brokerResponseNative;
   }
 
-  private static void processIterativeServerResponse(StreamingReducer reducer, 
ExecutorService executorService,
+  @VisibleForTesting
+  static void processIterativeServerResponse(StreamingReducer reducer, 
ExecutorService executorService,
       Map<ServerRoutingInstance, Iterator<Server.ServerResponse>> 
serverResponseMap, long reduceTimeOutMs,
-      ExecutionStatsAggregator aggregator) throws Exception {
+      ExecutionStatsAggregator aggregator) throws ExecutionException {
     int cnt = 0;
-    Future[] futures = new Future[serverResponseMap.size()];
-    CountDownLatch countDownLatch = new 
CountDownLatch(serverResponseMap.size());
-
+    Future<Void>[] futures = new Future[serverResponseMap.size()];
+    // based on ideas from on 
https://stackoverflow.com/questions/19348248/waiting-on-a-list-of-future
+    // mostly because we need to handle and transfer the exception in threads 
into caller thread.
+    ExecutorCompletionService<Void> countDownHelper = new 
ExecutorCompletionService<>(executorService);

Review comment:
       Thought about this, at first thought gRPC calls returns a `Future<Void>` 
and it is painful to convert to `CompletableFuture`.
   But I found 
https://stackoverflow.com/questions/23301598/transform-java-future-into-a-completablefuture
 , because we are creating Future by `ExecutorService.submit()` we can switch 
to use `CompletableFuture.supplyAsync()` to create CompletableFuture "natively".
   
   Tried it out and it works. Now the code is shorter and cleaner.




-- 
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: commits-unsubscr...@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to