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