This is an automated email from the ASF dual-hosted git repository.
danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new 45cec1a3bc13 fix(common): propagate stored disruptor consumer failures
(#20026)
45cec1a3bc13 is described below
commit 45cec1a3bc139d1e9d6db9bb75efa20451575cea
Author: Burak Kalaycı <[email protected]>
AuthorDate: Tue Oct 13 06:02:57 2026 +0300
fix(common): propagate stored disruptor consumer failures (#20026)
* fix(common): propagate stored disruptor consumer failures
Disruptor handlers catch consume exceptions and store them, so execute()
can return the finish() result while a fatal failure is still stored.
Check that stored failure after finish() so the caller sees it.
Closes #20016
Signed-off-by: Burak KALAYCI <[email protected]>
* test(common): prefer stored consume failure when finish throws
Add a companion case where consume() and finish() throw different
exceptions, and assert the stored consume failure stays the direct cause.
Signed-off-by: Burak KALAYCI <[email protected]>
---------
Signed-off-by: Burak KALAYCI <[email protected]>
---
.../util/queue/BaseHoodieQueueBasedExecutor.java | 9 +-
.../TestDisruptorExecutorConsumerFailure.java | 113 +++++++++++++++++++++
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/util/queue/BaseHoodieQueueBasedExecutor.java
b/hudi-common/src/main/java/org/apache/hudi/common/util/queue/BaseHoodieQueueBasedExecutor.java
index 6faa59342c67..90a8c8f3bd7a 100644
---
a/hudi-common/src/main/java/org/apache/hudi/common/util/queue/BaseHoodieQueueBasedExecutor.java
+++
b/hudi-common/src/main/java/org/apache/hudi/common/util/queue/BaseHoodieQueueBasedExecutor.java
@@ -201,7 +201,7 @@ public abstract class BaseHoodieQueueBasedExecutor<I, O, E>
implements HoodieExe
// 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();
@@ -209,6 +209,13 @@ public abstract class BaseHoodieQueueBasedExecutor<I, O,
E> implements HoodieExe
.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);
+ }
+ return result;
} catch (Exception e) {
if (e instanceof InterruptedException) {
// In case {@code InterruptedException} was thrown, resetting the
interrupted flag
diff --git
a/hudi-common/src/test/java/org/apache/hudi/common/util/queue/TestDisruptorExecutorConsumerFailure.java
b/hudi-common/src/test/java/org/apache/hudi/common/util/queue/TestDisruptorExecutorConsumerFailure.java
new file mode 100644
index 000000000000..65c34f620574
--- /dev/null
+++
b/hudi-common/src/test/java/org/apache/hudi/common/util/queue/TestDisruptorExecutorConsumerFailure.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.common.util.queue;
+
+import org.apache.hudi.exception.HoodieException;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests that a Disruptor consumer failure reaches {@link
DisruptorExecutor#execute()}
+ * even when production and {@link HoodieConsumer#finish()} complete normally.
+ */
+public class TestDisruptorExecutorConsumerFailure {
+
+ @Test
+ @Timeout(30)
+ public void testExecutePropagatesStoredConsumerFailureAfterFinish() {
+ IllegalStateException consumeFailure = new IllegalStateException("consume
failed");
+ AtomicBoolean finished = new AtomicBoolean(false);
+ HoodieConsumer<String, String> consumer = new HoodieConsumer<String,
String>() {
+ @Override
+ public void consume(String record) {
+ throw consumeFailure;
+ }
+
+ @Override
+ public String finish() {
+ finished.set(true);
+ return "finished";
+ }
+ };
+
+ DisruptorExecutor<String, String, String> executor = new
DisruptorExecutor<>(
+ 8,
+ Collections.singletonList("row").iterator(),
+ consumer,
+ record -> record,
+ WaitStrategyFactory.DEFAULT_STRATEGY,
+ () -> { });
+
+ try {
+ HoodieException thrown = assertThrows(HoodieException.class,
executor::execute);
+ assertSame(consumeFailure, thrown.getCause());
+ assertTrue(finished.get(), "finish() must still release consumer
resources");
+ } finally {
+ executor.shutdownNow();
+ }
+ }
+
+ @Test
+ @Timeout(30)
+ public void testExecutePrefersStoredConsumerFailureWhenFinishThrows() {
+ IllegalStateException consumeFailure = new IllegalStateException("consume
failed");
+ IllegalArgumentException finishFailure = new
IllegalArgumentException("finish failed");
+ // shutdownNow() calls finish() again; only the execute() call should fail.
+ AtomicBoolean finishThrew = new AtomicBoolean(false);
+ HoodieConsumer<String, String> consumer = new HoodieConsumer<String,
String>() {
+ @Override
+ public void consume(String record) {
+ throw consumeFailure;
+ }
+
+ @Override
+ public String finish() {
+ if (finishThrew.compareAndSet(false, true)) {
+ throw finishFailure;
+ }
+ return "closed";
+ }
+ };
+
+ DisruptorExecutor<String, String, String> executor = new
DisruptorExecutor<>(
+ 8,
+ Collections.singletonList("row").iterator(),
+ consumer,
+ record -> record,
+ WaitStrategyFactory.DEFAULT_STRATEGY,
+ () -> { });
+
+ try {
+ HoodieException thrown = assertThrows(HoodieException.class,
executor::execute);
+ assertSame(consumeFailure, thrown.getCause());
+ assertTrue(finishThrew.get(), "finish() must run so the catch path can
prefer the stored failure");
+ } finally {
+ executor.shutdownNow();
+ }
+ }
+}