From 5d9f2cd33d5a1ba507812e904518c61e2db99a57 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 7 Aug 2025 09:10:04 +0100 Subject: [PATCH 1/2] Clean up `awaitClusterState` overloads The `logger` parameter is unused, it doesn't throw any checked exceptions, and there's no need for `protected` instance methods when they're also available as `public static`. --- .../http/snapshots/RestGetSnapshotsIT.java | 2 +- ...TransportClusterStateActionDisruptionIT.java | 1 - .../DedicatedClusterSnapshotRestoreIT.java | 1 - .../AbstractSnapshotIntegTestCase.java | 9 ++++----- .../elasticsearch/test/ClusterServiceUtils.java | 4 +--- .../org/elasticsearch/test/ESIntegTestCase.java | 17 ++++------------- .../elasticsearch/test/InternalTestCluster.java | 2 +- .../xpack/ilm/IndexLifecycleRunnerTests.java | 1 - .../xpack/ml/MlSingleNodeTestCase.java | 6 +----- .../xpack/ml/support/BaseMlIntegTestCase.java | 4 ++-- .../xpack/monitoring/MultiNodesStatsTests.java | 2 +- ...napshotsCanMatchOnCoordinatorIntegTests.java | 2 +- 12 files changed, 16 insertions(+), 35 deletions(-) diff --git a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java index 683990d51d4a8..eb3b95ff27595 100644 --- a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java +++ b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java @@ -204,7 +204,7 @@ public void testSortAndPaginateWithInProgress() throws Exception { inProgressSnapshots.add(AbstractSnapshotIntegTestCase.startFullSnapshot(logger, repoName, snapshotName, false)); } AbstractSnapshotIntegTestCase.awaitNumberOfSnapshotsInProgress(logger, inProgressCount); - AbstractSnapshotIntegTestCase.awaitClusterState(logger, state -> { + AbstractSnapshotIntegTestCase.awaitClusterState(state -> { final var snapshotsInProgress = SnapshotsInProgress.get(state); boolean firstIndexSuccessfullySnapshot = snapshotsInProgress.asStream() .flatMap(s -> s.shards().entrySet().stream()) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateActionDisruptionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateActionDisruptionIT.java index b94c8e84a9f99..c9ccb7216d85b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateActionDisruptionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateActionDisruptionIT.java @@ -157,7 +157,6 @@ public void runRepeatedlyWhileChangingMaster(Runnable runnable) throws Exception final String nonMasterNode = randomValueOtherThan(masterName, () -> randomFrom(internalCluster().getNodeNames())); awaitClusterState( - logger, nonMasterNode, state -> Optional.ofNullable(state.nodes().getMasterNode()).map(m -> m.getName().equals(masterName) == false).orElse(false) ); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index f052af8b8a651..57104119af59f 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -1073,7 +1073,6 @@ public void onRequestSent( final ActionFuture deleteResponse = startDeleteSnapshot(repoName, snapshotName); awaitClusterState( - logger, otherDataNode, state -> SnapshotsInProgress.get(state) .forRepo(repoName) diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index ead163eaf26a4..438d152a4abfe 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -560,7 +560,7 @@ protected void addBwCFailedSnapshot(String repoName, String snapshotName, Map wait for [{}] deletions to show up in the cluster state", count); awaitClusterState(state -> SnapshotDeletionsInProgress.get(state).getEntries().size() == count); } @@ -572,7 +572,6 @@ protected void awaitNoMoreRunningOperations() throws Exception { protected void awaitNoMoreRunningOperations(String viaNode) throws Exception { logger.info("--> verify no more operations in the cluster state"); awaitClusterState( - logger, viaNode, state -> SnapshotsInProgress.get(state).isEmpty() && SnapshotDeletionsInProgress.get(state).hasDeletionsInProgress() == false ); @@ -607,13 +606,13 @@ public static ActionFuture startFullSnapshot( .execute(); } - protected void awaitNumberOfSnapshotsInProgress(int count) throws Exception { + protected void awaitNumberOfSnapshotsInProgress(int count) { awaitNumberOfSnapshotsInProgress(logger, count); } - public static void awaitNumberOfSnapshotsInProgress(Logger logger, int count) throws Exception { + public static void awaitNumberOfSnapshotsInProgress(Logger logger, int count) { logger.info("--> wait for [{}] snapshots to show up in the cluster state", count); - awaitClusterState(logger, state -> SnapshotsInProgress.get(state).count() == count); + awaitClusterState(state -> SnapshotsInProgress.get(state).count() == count); } protected SnapshotInfo assertSuccessful(ActionFuture future) throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java b/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java index 7cdb01af8b965..90a2070aea520 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java @@ -8,7 +8,6 @@ */ package org.elasticsearch.test; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.util.Throwables; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; @@ -239,8 +238,7 @@ public static void setAllElapsedMillis(ClusterStatePublicationEvent clusterState clusterStatePublicationEvent.setMasterApplyElapsedMillis(0L); } - public static void awaitClusterState(Logger logger, Predicate statePredicate, ClusterService clusterService) - throws Exception { + public static void awaitClusterState(Predicate statePredicate, ClusterService clusterService) { final var listener = addTemporaryStateListener(clusterService, statePredicate, ESTestCase.TEST_REQUEST_TIMEOUT); ESTestCase.safeAwait(listener, ESTestCase.TEST_REQUEST_TIMEOUT); } 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 5c2f9646f03b3..bab5fe8d64b78 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -17,7 +17,6 @@ import com.carrotsearch.randomizedtesting.generators.RandomPicks; import org.apache.http.HttpHost; -import org.apache.logging.log4j.Logger; import org.apache.lucene.search.Sort; import org.apache.lucene.search.TotalHits; import org.apache.lucene.tests.util.LuceneTestCase; @@ -1215,20 +1214,12 @@ public static PendingClusterTasksResponse getClusterPendingTasks(Client client) } } - protected void awaitClusterState(Predicate statePredicate) throws Exception { - awaitClusterState(logger, internalCluster().getMasterName(), statePredicate); + public static void awaitClusterState(Predicate statePredicate) { + awaitClusterState(internalCluster().getMasterName(), statePredicate); } - protected void awaitClusterState(String viaNode, Predicate statePredicate) throws Exception { - ClusterServiceUtils.awaitClusterState(logger, statePredicate, internalCluster().getInstance(ClusterService.class, viaNode)); - } - - public static void awaitClusterState(Logger logger, Predicate statePredicate) throws Exception { - awaitClusterState(logger, internalCluster().getMasterName(), statePredicate); - } - - public static void awaitClusterState(Logger logger, String viaNode, Predicate statePredicate) throws Exception { - ClusterServiceUtils.awaitClusterState(logger, statePredicate, internalCluster().getInstance(ClusterService.class, viaNode)); + public static void awaitClusterState(String viaNode, Predicate statePredicate) { + ClusterServiceUtils.awaitClusterState(statePredicate, internalCluster().getInstance(ClusterService.class, viaNode)); } public static String getNodeId(String nodeName) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 77d7bc5d5f7c5..e99abe49cb596 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -2064,7 +2064,7 @@ public String getMasterName(@Nullable String viaNode) { throw new AssertionError("Unable to get master name, no node found"); } try { - ClusterServiceUtils.awaitClusterState(logger, state -> state.nodes().getMasterNode() != null, clusterService(viaNode)); + ClusterServiceUtils.awaitClusterState(state -> state.nodes().getMasterNode() != null, clusterService(viaNode)); final ClusterState state = client(viaNode).admin().cluster().prepareState(TEST_REQUEST_TIMEOUT).get().getState(); final DiscoveryNode masterNode = state.nodes().getMasterNode(); if (masterNode == null) { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunnerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunnerTests.java index 41cda5ebf4913..43d15233cb7a8 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunnerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunnerTests.java @@ -490,7 +490,6 @@ public void doTestRunPolicyWithFailureToReadPolicy(boolean asyncAction, boolean // The cluster state can take a few extra milliseconds to update after the steps are executed ClusterServiceUtils.awaitClusterState( - logger, s -> s.metadata().getProject(state.projectId()).index(indexMetadata.getIndex()).getLifecycleExecutionState().stepInfo() != null, clusterService ); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java index 8898cac495706..c8c78e13f5b09 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java @@ -121,11 +121,7 @@ public void tearDown() throws Exception { protected void waitForMlTemplates() throws Exception { // block until the templates are installed - ClusterServiceUtils.awaitClusterState( - logger, - MachineLearning::criticalTemplatesInstalled, - getInstanceFromNode(ClusterService.class) - ); + ClusterServiceUtils.awaitClusterState(MachineLearning::criticalTemplatesInstalled, getInstanceFromNode(ClusterService.class)); } protected void blockingCall(Consumer> function, AtomicReference response, AtomicReference error) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java index 4473919130c83..ee966ec951826 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java @@ -169,9 +169,9 @@ protected Collection> getMockPlugins() { } @Before - public void ensureTemplatesArePresent() throws Exception { + public void ensureTemplatesArePresent() { if (cluster().size() > 0) { - awaitClusterState(logger, MachineLearning::criticalTemplatesInstalled); + awaitClusterState(MachineLearning::criticalTemplatesInstalled); } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MultiNodesStatsTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MultiNodesStatsTests.java index 5e5300f6b41c7..457cefbc9c703 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MultiNodesStatsTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MultiNodesStatsTests.java @@ -99,7 +99,7 @@ public void testMultipleNodes() throws Exception { }); } - private void waitForMonitoringIndices() throws Exception { + private void waitForMonitoringIndices() { final var indexNameExpressionResolver = internalCluster().getCurrentMasterNodeInstance(IndexNameExpressionResolver.class); final var indicesOptions = IndicesOptions.builder() .wildcardOptions(IndicesOptions.WildcardOptions.builder().allowEmptyExpressions(true)) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java index 50e58befc4a0e..2a9ae3035ae9e 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java @@ -1323,7 +1323,7 @@ private static IndexMetadata getIndexMetadata(String indexName) { .index(indexName); } - private void waitUntilAllShardsAreUnassigned(Index index) throws Exception { + private void waitUntilAllShardsAreUnassigned(Index index) { awaitClusterState(state -> state.getRoutingTable().index(index).allPrimaryShardsUnassigned()); } From 5d2ad879327c6647dc20a4b3ae5a14f4be6a0e7a Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 7 Aug 2025 09:34:03 +0100 Subject: [PATCH 2/2] Inline listener and TEST_REQUEST_TIMEOUT -- this isn't what TEST_REQUEST_TIMEOUT is for --- .../main/java/org/elasticsearch/test/ClusterServiceUtils.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java b/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java index 90a2070aea520..9773bea8b5e3c 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java @@ -239,8 +239,7 @@ public static void setAllElapsedMillis(ClusterStatePublicationEvent clusterState } public static void awaitClusterState(Predicate statePredicate, ClusterService clusterService) { - final var listener = addTemporaryStateListener(clusterService, statePredicate, ESTestCase.TEST_REQUEST_TIMEOUT); - ESTestCase.safeAwait(listener, ESTestCase.TEST_REQUEST_TIMEOUT); + ESTestCase.safeAwait(addTemporaryStateListener(clusterService, statePredicate, TimeValue.THIRTY_SECONDS), TimeValue.THIRTY_SECONDS); } public static void awaitNoPendingTasks(ClusterService clusterService) {