diff --git a/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java b/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java index 532396ee6095c..a2a964a6c78cd 100644 --- a/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java +++ b/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java @@ -72,9 +72,12 @@ public void onResponse(T element) { @Override public void onFailure(Exception e) { if (failure.compareAndSet(null, e) == false) { - failure.accumulateAndGet(e, (previous, current) -> { - previous.addSuppressed(current); - return previous; + failure.accumulateAndGet(e, (current, update) -> { + // we have to avoid self-suppression! + if (update != current) { + current.addSuppressed(update); + } + return current; }); } if (countDown.countDown()) { diff --git a/server/src/test/java/org/elasticsearch/action/support/GroupedActionListenerTests.java b/server/src/test/java/org/elasticsearch/action/support/GroupedActionListenerTests.java index 9f6454d4e4bcf..1f5b34caa04d1 100644 --- a/server/src/test/java/org/elasticsearch/action/support/GroupedActionListenerTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/GroupedActionListenerTests.java @@ -33,6 +33,9 @@ import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; public class GroupedActionListenerTests extends ESTestCase { @@ -139,4 +142,22 @@ public void testConcurrentFailures() throws InterruptedException { assertThat(exception, instanceOf(IOException.class)); assertEquals(numGroups - 1, exception.getSuppressed().length); } + + /* + * It can happen that the same exception causes a grouped listener to be notified of the failure multiple times. Since we suppress + * additional exceptions into the first exception, we have to guard against suppressing into the same exception, which could occur if we + * are notified of with the same failure multiple times. This test verifies that the guard against self-suppression remains. + */ + public void testRepeatNotificationForTheSameException() { + final AtomicReference finalException = new AtomicReference<>(); + final GroupedActionListener listener = + new GroupedActionListener(ActionListener.wrap(r -> {}, finalException::set), 2, Collections.emptyList()); + final Exception e = new Exception(); + // repeat notification for the same exception + listener.onFailure(e); + listener.onFailure(e); + assertThat(finalException.get(), not(nullValue())); + assertThat(finalException.get(), equalTo(e)); + } + }