Fix `ThreadedActionListenerTests#testRejectionHandling` by DaveCTurner · Pull Request #144795 · elastic/elasticsearch

DaveCTurner · 2026-03-23T17:59:22Z

elasticsearchmachine · 2026-03-23T17:59:47Z

Pinging @elastic/es-distributed (Team:Distributed)

DaveCTurner

This test had weak coverage because it would usually shut the threadpool down before doing anything else. Introduced another latch to delay the shutdown a bit, and also fixed the actual problem.

DaveCTurner · 2026-03-23T17:59:54Z

server/src/test/java/org/elasticsearch/action/support/ThreadedActionListenerTests.java

+                                    if (pool.equals("fixed-bounded-queue") == false || forceExecution) {
+                                        assertTrue(esRejectedExecutionException.isExecutorShutdown());


This is the key fix, we sometimes reject these tasks on the bounded queue before the executor is shut down.

nit David just to understand, would they be rejected on the bounded queue because it's bounded? I.e., it happens that more than 10 listeners are submitted to it before they are actully triggered?

If so, I think a code comment above this fix would be nice as well.

…ejectionHandling-130129

kingherc

LGTM @DaveCTurner , just some nits for potentially improve readability, feel free to pick any of them as you see fit.

kingherc · 2026-03-24T08:54:41Z

server/src/test/java/org/elasticsearch/action/support/ThreadedActionListenerTests.java

+                                    if (pool.equals("fixed-bounded-queue") == false || forceExecution) {
+                                        assertTrue(esRejectedExecutionException.isExecutorShutdown());


nit David just to understand, would they be rejected on the bounded queue because it's bounded? I.e., it happens that more than 10 listeners are submitted to it before they are actully triggered?

If so, I think a code comment above this fix would be nice as well.

kingherc · 2026-03-24T08:57:08Z

server/src/test/java/org/elasticsearch/action/support/ThreadedActionListenerTests.java

    public void testRejectionHandling() throws InterruptedException {
        final var listenerCount = between(1, 1000);
-        final var countdownLatch = new CountDownLatch(listenerCount);
+        final var startLatch = new CountDownLatch(between(1, listenerCount));


nit

Suggested change

final var startLatch = new CountDownLatch(between(1, listenerCount));

final var latchMinListenersToStartBeforeShutdown = new CountDownLatch(between(1, listenerCount));

I've added a comment describing startLatch but I find such long symbol names to be somewhat hostile to readers so I'd rather keep it as-is.

kingherc · 2026-03-24T08:57:34Z

server/src/test/java/org/elasticsearch/action/support/ThreadedActionListenerTests.java

        final var listenerCount = between(1, 1000);
-        final var countdownLatch = new CountDownLatch(listenerCount);
+        final var startLatch = new CountDownLatch(between(1, listenerCount));
+        final var finishLatch = new CountDownLatch(listenerCount);


nit

Suggested change

final var finishLatch = new CountDownLatch(listenerCount);

final var latchAllListenersCompleted = new CountDownLatch(listenerCount);

I've added a comment describing finishLatch but I find such long symbol names to be somewhat hostile to readers so I'd rather keep it as-is.

I would prefer the more descriptive version, to be honest.

This test is generally very hard to understand. I wonder if there's a way to reveal the intention of particular sections anyhow... and e.g. separate the "setup" "run" and "verification" parts of the test.

Just a loose idea.

…ejectionHandling-130129

elasticsearchmachine · 2026-03-26T10:33:02Z

💔 Backport failed

Status	Branch	Result
❌	9.3	Commit could not be cherrypicked due to conflicts
❌	8.19	Commit could not be cherrypicked due to conflicts
❌	9.2	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 144795

* upstream/main: (146 commits) Revert "[Native] Gradle-related tweaks to improve handling of the simdvec native library (elastic#144539)" Fix ArrayIndexOutOfBoundsException in fetch phase with partial results (elastic#144385) ESQL: Correctly manage NULL data type for SUM (elastic#144942) [ESQL] Fixes GroupedTopNBenchmark not executing (elastic#144944) Fix reader context leak when query response serialization fails (elastic#144708) Validate individual offset values in BULK_OFFSETS bounds checks (elastic#144643) Merge main21 source set into main in simdvec (elastic#144921) [TEST] Unmute TsidExtractingIdFieldMapperTests (elastic#144848) [Native] Gradle-related tweaks to improve handling of the simdvec native library (elastic#144539) Fix `ThreadedActionListenerTests#testRejectionHandling` (elastic#144795) Add new DLM Frozen Tier Transition execution plugin and service (elastic#144595) Prometheus: execute query_range via parsed EsqlStatement plan (elastic#144416) Investigate `testBulkIndexingRequestSplitting` failure (elastic#144766) Add test utility for wrapping directories in FilterDirectory layer (elastic#143563) Fix ES|QL decay tests with negative scale (elastic#144657) Fix circuit breaker leak in percolator query construction (elastic#144827) Use XPerFieldDocValuesFormat in AbstractTSDBSyntheticIdCodec (elastic#144744) [DOCS] Document how reindex work in CPS (elastic#144016) Fix Int4 vector library tests failing on Java 21 (elastic#144830) [DiskBBQ] Fix index sorting on flush (elastic#144938) ...

Closes #130129

DaveCTurner · 2026-03-26T15:39:14Z

Backports:

Closes elastic#130129

Fix ThreadedActionListenerTests#testRejectionHandling

a054a11

Closes elastic#130129

DaveCTurner requested a review from kingherc March 23, 2026 17:59

DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. branch:9.2 branch:8.19 v9.4.0 branch:9.3 labels Mar 23, 2026

elasticsearchmachine added Team:Distributed Meta label for distributed team. v9.3.3 v8.19.14 labels Mar 23, 2026

elasticsearchmachine added v9.2.8 and removed branch:9.2 branch:8.19 branch:9.3 labels Mar 23, 2026

DaveCTurner commented Mar 23, 2026

View reviewed changes

DaveCTurner added 2 commits March 23, 2026 18:02

Force more often

a26c97f

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

2d99828

…ejectionHandling-130129

kingherc approved these changes Mar 24, 2026

View reviewed changes

DaveCTurner added 2 commits March 24, 2026 09:10

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

3f74759

…ejectionHandling-130129

Comments

bd9015f

DaveCTurner enabled auto-merge (squash) March 24, 2026 09:10

DaveCTurner added the auto-backport Automatically create backport pull requests when merged label Mar 24, 2026

DaveCTurner added 6 commits March 24, 2026 13:49

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

c8c6c45

…ejectionHandling-130129

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

17547c4

…ejectionHandling-130129

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

4ba4ca5

…ejectionHandling-130129

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

661a37a

…ejectionHandling-130129

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

d0e82d9

…ejectionHandling-130129

Merge branch 'main' into 2026/03/23/ThreadedActionListenerTests-testR…

776524d

…ejectionHandling-130129

DaveCTurner merged commit b50a0e6 into elastic:main Mar 26, 2026
36 checks passed

elasticsearchmachine added the backport pending label Mar 26, 2026

DaveCTurner deleted the 2026/03/23/ThreadedActionListenerTests-testRejectionHandling-130129 branch March 26, 2026 12:44

DaveCTurner added a commit that referenced this pull request Mar 26, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (#144795)

e1a0b32

Closes #130129

DaveCTurner added a commit that referenced this pull request Mar 26, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (#144795)

c7c1b8a

Closes #130129

DaveCTurner added a commit that referenced this pull request Mar 26, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (#144795)

4c74987

Closes #130129

DaveCTurner removed the backport pending label Mar 26, 2026

seanzatzdev pushed a commit to seanzatzdev/elasticsearch that referenced this pull request Mar 26, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (elastic#144795)

104ce9f

Closes elastic#130129

seanzatzdev pushed a commit to seanzatzdev/elasticsearch that referenced this pull request Mar 27, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (elastic#144795)

46ba8f1

Closes elastic#130129

mamazzol pushed a commit to mamazzol/elasticsearch that referenced this pull request Mar 30, 2026

Fix ThreadedActionListenerTests#testRejectionHandling (elastic#144795)

083e363

Closes elastic#130129

		if (pool.equals("fixed-bounded-queue") == false \|\| forceExecution) {
		assertTrue(esRejectedExecutionException.isExecutorShutdown());

	final var startLatch = new CountDownLatch(between(1, listenerCount));
	final var latchMinListenersToStartBeforeShutdown = new CountDownLatch(between(1, listenerCount));

	final var finishLatch = new CountDownLatch(listenerCount);
	final var latchAllListenersCompleted = new CountDownLatch(listenerCount);

Conversation

DaveCTurner commented Mar 23, 2026

Uh oh!

elasticsearchmachine commented Mar 23, 2026

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kingherc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Mar 26, 2026

💔 Backport failed

Uh oh!

DaveCTurner commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants