From ffcf46e1da0b5dc731b93e28f62b295a16851187 Mon Sep 17 00:00:00 2001 From: jimczi Date: Wed, 13 May 2020 12:20:54 +0200 Subject: [PATCH 1/3] Disable search context release checks after every test This change adds a way to disable the checks that search contexts are all released after each test in ESTestCase. This is used by ESIntegTestCase to disable these checks if the cluster is shared across all methods (SUITE scope). In this case, the contexts are checked at the end when the cluster is cleared. This fixes integration tests that starts components that perform search requests internally with a fixed delay (e.g.: async search maintenance service). Fixes #55988 --- .../java/org/elasticsearch/test/ESIntegTestCase.java | 8 ++++++++ .../main/java/org/elasticsearch/test/ESTestCase.java | 11 ++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 9c7d779d8c163..18d272b8e7e8e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -348,6 +348,14 @@ protected final boolean enableWarningsCheck() { return false; } + @Override + protected boolean enableSearchContextAfterCheck() { + // We don't want to check the release of search contexts between tests if the cluster is shared. + // The release of search contexts is always checked at the end, when the cluster is cleared so + // we're just skipping this operation between tests. + return getCurrentClusterScope() == Scope.TEST; + } + protected final void beforeInternal() throws Exception { final Scope currentClusterScope = getCurrentClusterScope(); Callable setup = () -> { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 2eb761af73ce5..199fe2ca15c05 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -363,6 +363,13 @@ protected boolean enableWarningsCheck() { return true; } + /** + * Whether or not we check after each test whether it has left search contexts behind. + */ + protected boolean enableSearchContextAfterCheck() { + return true; + } + @After public final void after() throws Exception { checkStaticState(false); @@ -375,7 +382,9 @@ public final void after() throws Exception { DeprecationLogger.removeThreadContext(threadContext); threadContext = null; } - ensureAllSearchContextsReleased(); + if (enableSearchContextAfterCheck()) { + ensureAllSearchContextsReleased(); + } ensureCheckIndexPassed(); logger.info("{}after test", getTestParamsForLogging()); } From 59603cbd7e2866cf598570d5c7a14a6f6a71b92e Mon Sep 17 00:00:00 2001 From: jimczi Date: Wed, 13 May 2020 21:45:10 +0200 Subject: [PATCH 2/3] start/stop the maintenance service between each test --- .../elasticsearch/test/ESIntegTestCase.java | 8 ------- .../org/elasticsearch/test/ESTestCase.java | 11 +-------- .../search/AsyncSearchIntegTestCase.java | 23 +++++++++++++++++++ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 18d272b8e7e8e..9c7d779d8c163 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -348,14 +348,6 @@ protected final boolean enableWarningsCheck() { return false; } - @Override - protected boolean enableSearchContextAfterCheck() { - // We don't want to check the release of search contexts between tests if the cluster is shared. - // The release of search contexts is always checked at the end, when the cluster is cleared so - // we're just skipping this operation between tests. - return getCurrentClusterScope() == Scope.TEST; - } - protected final void beforeInternal() throws Exception { final Scope currentClusterScope = getCurrentClusterScope(); Callable setup = () -> { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 199fe2ca15c05..2eb761af73ce5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -363,13 +363,6 @@ protected boolean enableWarningsCheck() { return true; } - /** - * Whether or not we check after each test whether it has left search contexts behind. - */ - protected boolean enableSearchContextAfterCheck() { - return true; - } - @After public final void after() throws Exception { checkStaticState(false); @@ -382,9 +375,7 @@ public final void after() throws Exception { DeprecationLogger.removeThreadContext(threadContext); threadContext = null; } - if (enableSearchContextAfterCheck()) { - ensureAllSearchContextsReleased(); - } + ensureAllSearchContextsReleased(); ensureCheckIndexPassed(); logger.info("{}after test", getTestParamsForLogging()); } diff --git a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java index f579b834b723e..38415c5f5ea86 100644 --- a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java +++ b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java @@ -12,7 +12,10 @@ import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ContextParser; @@ -34,6 +37,7 @@ import org.elasticsearch.xpack.core.search.action.SubmitAsyncSearchRequest; import org.elasticsearch.xpack.ilm.IndexLifecycle; import org.junit.After; +import org.junit.Before; import java.io.Closeable; import java.util.Arrays; @@ -71,6 +75,25 @@ public List getAggregations() { } } + @Before + public void startMaitenanceService() { + for (AsyncSearchMaintenanceService service : internalCluster().getDataNodeInstances(AsyncSearchMaintenanceService.class)) { + if (service.lifecycleState() == Lifecycle.State.STOPPED) { + // force the service to start again + service.start(); + ClusterState state = internalCluster().clusterService().state(); + service.clusterChanged(new ClusterChangedEvent("noop", state, state)); + } + } + } + + @After + public void stopMaitenanceService() { + for (AsyncSearchMaintenanceService service : internalCluster().getDataNodeInstances(AsyncSearchMaintenanceService.class)) { + service.stop(); + } + } + @After public void releaseQueryLatch() { BlockingQueryBuilder.releaseQueryLatch(); From 86a17aa450e57f464df7d57929079642aae1b571 Mon Sep 17 00:00:00 2001 From: jimczi Date: Wed, 13 May 2020 22:57:11 +0200 Subject: [PATCH 3/3] fix typo --- .../elasticsearch/xpack/search/AsyncSearchIntegTestCase.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java index 38415c5f5ea86..1b215068faa09 100644 --- a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java +++ b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchIntegTestCase.java @@ -76,7 +76,7 @@ public List getAggregations() { } @Before - public void startMaitenanceService() { + public void startMaintenanceService() { for (AsyncSearchMaintenanceService service : internalCluster().getDataNodeInstances(AsyncSearchMaintenanceService.class)) { if (service.lifecycleState() == Lifecycle.State.STOPPED) { // force the service to start again @@ -88,7 +88,7 @@ public void startMaitenanceService() { } @After - public void stopMaitenanceService() { + public void stopMaintenanceService() { for (AsyncSearchMaintenanceService service : internalCluster().getDataNodeInstances(AsyncSearchMaintenanceService.class)) { service.stop(); }