hudi-agent commented on code in PR #20026:
URL: https://github.com/apache/hudi/pull/20026#discussion_r4079415095
##########
hudi-common/src/main/java/org/apache/hudi/common/util/queue/BaseHoodieQueueBasedExecutor.java:
##########
@@ -201,14 +201,21 @@ public E execute() {
// NOTE: To properly support mode when there's no consumer, we have to
fall back
// to producing future as the trigger for us to shut down the queue
- return allOf(Arrays.asList(producingFuture, consumingFuture))
+ E result = allOf(Arrays.asList(producingFuture, consumingFuture))
.whenComplete((ignored, throwable) -> {
// Close the queue to release the resources
queue.close();
})
.thenApply(ignored -> consumer.get().finish())
// Block until producing and consuming both finish
.get();
+ // Disruptor handlers store consumer failures and return normally, so
production
+ // and finish() can both succeed while a fatal consume error is still
stored.
+ Throwable consumerFailure = queue.getThrowable();
+ if (consumerFailure != null) {
+ throw new HoodieException(consumerFailure);
Review Comment:
🤖 When `finish()` also throws, that exception is discarded entirely (the
catch below prefers the stored failure) and never logged — the new test pins
that behavior in. Would it be worth attaching it via `addSuppressed` so a
failing `finish()` (e.g. a write handle failing to close) still shows up in the
stack trace?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
