hudi-agent commented on code in PR #20026:
URL: https://github.com/apache/hudi/pull/20026#discussion_r4073006251
##########
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();
Review Comment:
🤖 Since `setHandlers` swallows the failure and keeps consuming, every
remaining record is still fed to a consumer that has already blown up, and
`finish()` then runs over that state. Would it be worth halting the handler (or
short-circuiting `consume`) once `throwable` is set, so we don't keep writing
after the first fatal error?
<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]
