From 0eed4e7d7f0f4c2d9145f52aebae7e47d04fdf12 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 11 Sep 2024 23:15:25 +1000 Subject: [PATCH 1/2] Use a dedicated test executor in MockTransportService Instead of using the generic executor for delayed transport actions, this PR adds a new executor to schedule these actions. It helps avoid sharing executors with the node which may lead to unexpected CI failures due to unsafe future assertion. --- .../test/transport/MockTransportService.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java index da478cbf1cb26..4f30ce26ed056 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java @@ -29,6 +29,8 @@ import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.MockPageCacheRecycler; import org.elasticsearch.common.util.concurrent.AbstractRunnable; +import org.elasticsearch.common.util.concurrent.EsExecutors; +import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor; import org.elasticsearch.common.util.concurrent.RunOnce; import org.elasticsearch.core.AbstractRefCounted; import org.elasticsearch.core.IOUtils; @@ -206,6 +208,7 @@ public static MockTransportService getInstance(String nodeName) { } private final Transport original; + private final EsThreadPoolExecutor testExecutor; /** * Build the service. @@ -302,6 +305,16 @@ private MockTransportService( Tracer.NOOP ); this.original = transport.getDelegate(); + this.testExecutor = EsExecutors.newScaling( + "mock-transport", + 0, + 4, + 30, + TimeUnit.SECONDS, + false, + EsExecutors.daemonThreadFactory("mock-transport"), + threadPool.getThreadContext() + ); } private static TransportAddress[] extractTransportAddresses(TransportService transportService) { @@ -617,7 +630,7 @@ protected void doRun() throws IOException { delay ) ); - threadPool.schedule(runnable, delay, threadPool.generic()); + threadPool.schedule(runnable, delay, testExecutor); } } } @@ -795,6 +808,8 @@ protected void doClose() throws IOException { } } catch (InterruptedException e) { throw new IllegalStateException(e); + } finally { + ThreadPool.terminate(testExecutor, 10, TimeUnit.SECONDS); } } From 63a461498b049ba8ae0b39b924b9bc514f0484da Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 12 Sep 2024 00:23:04 +1000 Subject: [PATCH 2/2] review comments --- .../elasticsearch/test/transport/MockTransportService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java index 4f30ce26ed056..8e10fd08c9d42 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java @@ -83,6 +83,7 @@ import java.util.function.Supplier; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.spy; /** @@ -311,7 +312,7 @@ private MockTransportService( 4, 30, TimeUnit.SECONDS, - false, + true, EsExecutors.daemonThreadFactory("mock-transport"), threadPool.getThreadContext() ); @@ -809,7 +810,7 @@ protected void doClose() throws IOException { } catch (InterruptedException e) { throw new IllegalStateException(e); } finally { - ThreadPool.terminate(testExecutor, 10, TimeUnit.SECONDS); + assertTrue(ThreadPool.terminate(testExecutor, 10, TimeUnit.SECONDS)); } }