kbuci commented on code in PR #18456:
URL: https://github.com/apache/hudi/pull/18456#discussion_r3031689835
##########
hudi-common/src/main/java/org/apache/hudi/common/util/FutureUtils.java:
##########
@@ -42,24 +44,37 @@ public class FutureUtils {
*/
public static <T> CompletableFuture<List<T>>
allOf(List<CompletableFuture<T>> futures) {
CompletableFuture<Void> union =
CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
+ AtomicReference<Throwable> firstFailure = new AtomicReference<>();
futures.forEach(future -> {
// NOTE: We add a callback to every future, to cancel all the other not
yet completed futures,
// which will be providing for an early termination semantic:
whenever any of the futures
// fail other futures will be cancelled and the exception will be
returned as a result
future.whenComplete((ignored, throwable) -> {
if (throwable != null) {
+ firstFailure.compareAndSet(null, throwable);
+ // Note that {@link CompletableFuture#cancel} does not interrupt the
other underlying tasks;
+ // it only marks their futures as cancelled. The tasks will still
run to completion.
futures.forEach(f -> f.cancel(true));
- union.completeExceptionally(throwable);
Review Comment:
I think swapping this and the preceding line would also fix this issue. But
I didn't want to go this route since I felt it would be a bit harder for users
not familiar with Java future semantics (like myself) to follow
That being said, if we think there's a more idiomatic approach for this I
can refactor this PR to that - this was just the approach that I thought made
sense 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
