gianm commented on a change in pull request #10027:
URL: https://github.com/apache/druid/pull/10027#discussion_r441779534



##########
File path: core/src/main/java/org/apache/druid/common/guava/GuavaUtils.java
##########
@@ -77,4 +79,24 @@ public static Long tryParseLong(@Nullable String string)
     }
     return arg1;
   }
+
+  /**
+   * Cancel futures manually, because sometime we can't cancel all futures in 
{@link com.google.common.util.concurrent.Futures.CombinedFuture}
+   * automatically. Especially when we call {@link  
com.google.common.util.concurrent.Futures#allAsList(Iterable)} to create a 
batch of
+   * future.
+   * @param futures The futures that we want to cancel
+   * @param <T>   The result type returned by this Future's {@code get} method
+   */
+  public static <T, F  extends Future<T>> void cancelAll(List<F> futures){
+    if(futures == null || futures.isEmpty()){
+      return;
+    }
+    futures.forEach(f -> {
+      try {
+        f.cancel(true);
+      } catch (Throwable t){
+        //do nothing and continue the loop.

Review comment:
       We shouldn't chomp Throwables, because they're generally bad things that 
should interrupt execution. It'd be better to either assume no exceptions, i.e.:
   
   ```java
   futures.forEach(f -> f.cancel(true));
   ```
   
   Or surface Errors, but suppress and log exceptions:
   
   ```java
   futures.forEach(f -> {
     try {
       f.cancel(true);
     } catch (Exception e) {
       log.warn(e, "Error while canceling future.");
     }
   });
   ```
   
   IMO, the first one is best if we can assure ourselves that 
`ListenableFuture.cancel` isn't going to throw exceptions. Looking at 
`AbstractFuture`, it seems like it won't, so the first option looks safe to me.




----------------------------------------------------------------
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.

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



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

Reply via email to