Skip to content

Replace assertBusy of indexExists #126501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 10, 2025
Merged

Replace assertBusy of indexExists #126501

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
123e9c9
Replace assertBusy of indexExists
ywangd Apr 9, 2025
9c1d4c0
Incorporate review comments
ywangd Apr 9, 2025
a3338b3
Merge remote-tracking branch 'origin/main' into replace-more-index-ex…
ywangd Apr 9, 2025
7ea2eab
drop timeout
ywangd Apr 9, 2025
59c6473
drop
ywangd Apr 9, 2025
780d0e7
Merge branch 'main' into replace-more-index-exists
elasticmachine Apr 9, 2025
75c1464
handle wildcard
ywangd Apr 9, 2025
5ef316e
Merge remote-tracking branch 'origin/main' into replace-more-index-ex…
ywangd Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .../internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ public void testDeletionOfFailingToRecoverIndexShouldStopRestore() throws Except

logger.info("--> wait for the index to appear");
// that would mean that recovery process started and failing
safeGet(clusterAdmin().prepareHealth(SAFE_AWAIT_TIMEOUT, "test-idx").execute());
awaitIndexExists("test-idx");

logger.info("--> delete index");
cluster().wipeIndices("test-idx");
Expand Down
14 changes: 14 additions & 0 deletions test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,20 @@ public static boolean indexExists(String index, Client client) {
return getIndexResponse.getIndices().length > 0;
}

public static void awaitIndexExists(String index) {
awaitIndexExists(index, client());
}

public static void awaitIndexExists(String index, Client client) {
safeGet(
client.admin()
.cluster()
.prepareHealth(SAFE_AWAIT_TIMEOUT, index)
.setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED)
.execute()
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we're not using something like ESIntegTestCase#awaitClusterState here? We're not waiting for a specific health (i.e. GREEN) here, just for the index to exist. Couldn't (/shouldn't) we do that with a predicate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really just that this is replacing pre-existing client-based calls. It's the same either way really, the health API waits with a ClusterStateObserver.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One difference is that this method is also used to check a remote cluster's indices, i.e. it needs a client.

The other difference that I just noticed from the CI failure is that index name can be a wildcard like this usage. It requires expansion. Unfortunately, this means we have to use assertBusy in this case. I pushed 75c1464

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remote cluster in question is still running in the test JVM so no this doesn't need a client, you could use awaitClusterState with the ClusterService of the elected master of the follower cluster obtained with getFollowerCluster().getCurrentMasterNodeInstance(ClusterService.class).

I don't think we should support wildcards in this utility, especially if it requires an assertBusy like that. The only test that uses it is pretty odd (and very old). I'd rather we did something like #126582 there.

}

/**
* Syntactic sugar for enabling allocation for <code>indices</code>
*/
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ccr/src/internalClusterTest/java/org/elasticsearch/xpack/ccr/AutoFollowIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void testAutoFollow() throws Exception {
createLeaderIndex("metrics-201901", leaderIndexSettings);

createLeaderIndex("logs-201901", leaderIndexSettings);
assertLongBusy(() -> { assertTrue(ESIntegTestCase.indexExists("copy-logs-201901", followerClient())); });
ESIntegTestCase.awaitIndexExists("copy-logs-201901", followerClient());
createLeaderIndex("transactions-201901", leaderIndexSettings);
assertLongBusy(() -> {
AutoFollowStats autoFollowStats = getAutoFollowStats();
Expand Down
2 changes: 1 addition & 1 deletion ...nalClusterTest/java/org/elasticsearch/xpack/ilm/ClusterStateWaitThresholdBreachTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public void onFailure(Exception e) {
// the shrink index generated in the first attempt must've been deleted!
assertBusy(() -> assertFalse(indexExists(firstAttemptShrinkIndexName[0])));

assertBusy(() -> assertTrue(indexExists(secondCycleShrinkIndexName[0])), 30, TimeUnit.SECONDS);
awaitIndexExists(secondCycleShrinkIndexName[0]);

// at this point, the second shrink attempt was executed and the manged index is looping into the `shrunk-shards-allocated` step as
// waiting for the huge numbers of replicas for the shrunk index to allocate. this will never happen, so let's unblock this
Expand Down
4 changes: 2 additions & 2 deletions ...ternalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public void testSingleNodeCluster() throws Exception {
ClusterState clusterState = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT).get().getState();
RoutingNode routingNodeEntry1 = clusterState.getRoutingNodes().node(node1);
assertThat(routingNodeEntry1.numberOfShardsWithState(STARTED), equalTo(1));
assertBusy(() -> { assertTrue(indexExists("test")); });
awaitIndexExists("test");
IndexLifecycleService indexLifecycleService = internalCluster().getInstance(IndexLifecycleService.class, server_1);
assertThat(indexLifecycleService.getScheduler().jobCount(), equalTo(1));
assertNotNull(indexLifecycleService.getScheduledJob());
Expand Down Expand Up @@ -420,7 +420,7 @@ public void testMasterDedicatedDataDedicated() throws Exception {
RoutingNode routingNodeEntry1 = clusterState.getRoutingNodes().node(node2);
assertThat(routingNodeEntry1.numberOfShardsWithState(STARTED), equalTo(1));

assertBusy(() -> assertTrue(indexExists("test")));
awaitIndexExists("test");
assertBusy(() -> {
LifecycleExecutionState lifecycleState = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT)
.get()
Expand Down
2 changes: 1 addition & 1 deletion .../java/org/elasticsearch/xpack/ml/integration/MlNativeDataFrameAnalyticsIntegTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ protected static void assertThatAuditMessagesMatch(String configId, String... ex
// Make sure we wrote to the audit
// Since calls to write the AbstractAuditor are sent and forgot (async) we could have returned from the start,
// finished the job (as this is a very short analytics job), all without the audit being fully written.
assertBusy(() -> assertTrue(indexExists(NotificationsIndex.NOTIFICATIONS_INDEX)));
awaitIndexExists(NotificationsIndex.NOTIFICATIONS_INDEX);

@SuppressWarnings("unchecked")
Matcher<String>[] itemMatchers = Arrays.stream(expectedAuditMessagePrefixes).map(Matchers::startsWith).toArray(Matcher[]::new);
Expand Down
13 changes: 0 additions & 13 deletions ...toring/src/test/java/org/elasticsearch/xpack/monitoring/test/MonitoringIntegTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.elasticsearch.analysis.common.CommonAnalysisPlugin;
import org.elasticsearch.cluster.metadata.IndexTemplateMetadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.CountDown;
Expand All @@ -33,7 +32,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
Expand Down Expand Up @@ -157,17 +155,6 @@ protected void waitForMonitoringIndices() throws Exception {
assertBusy(this::ensureMonitoringIndicesYellow);
}

protected void awaitIndexExists(final String index) throws Exception {
assertBusy(() -> assertIndicesExists(index), 30, TimeUnit.SECONDS);
}

private void assertIndicesExists(String... indices) {
logger.trace("checking if index exists [{}]", Strings.arrayToCommaDelimitedString(indices));
for (String index : indices) {
assertThat(indexExists(index), is(true));
}
}

protected void enableMonitoringCollection() {
updateClusterSettings(Settings.builder().put(MonitoringService.ENABLED.getKey(), true));
}
Expand Down