Fix race condition in IndexServiceTests (#19281)

One test method had a race condition when getting an index after
closing. This change extracts the common pattern of polling until the
index is available and uses it everywhere.

Signed-off-by: Andrew Ross <[email protected]>

Author: andrross

server/src/test/java/org/opensearch/index/IndexServiceTests.java

@@ -67,6 +67,7 @@
 import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
@@ -77,7 +78,9 @@
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.core.IsEqual.equalTo;
+import static org.awaitility.Awaitility.await;
 
 /** Unit test(s) for IndexService */
 public class IndexServiceTests extends OpenSearchSingleNodeTestCase {
@@ -155,18 +158,15 @@ protected void runInternal() {
         // now close the index
         final Index index = indexService.index();
         assertAcked(client().admin().indices().prepareClose(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-
-        final IndexService closedIndexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        final IndexService closedIndexService = getIndexService(index);
         assertNotSame(indexService, closedIndexService);
         assertFalse(task.mustReschedule());
         assertFalse(task.isClosed());
         assertEquals(1000000, task.getInterval().millis());
 
         // now reopen the index
         assertAcked(client().admin().indices().prepareOpen(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-        indexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        indexService = getIndexService(index);
         assertNotSame(closedIndexService, indexService);
 
         task = new IndexService.BaseAsyncTask(indexService, TimeValue.timeValueMillis(100000)) {
@@ -245,9 +245,7 @@ public void testRefreshTaskIsUpdated() throws Exception {
         // now close the index
         final Index index = indexService.index();
         assertAcked(client().admin().indices().prepareClose(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-
-        final IndexService closedIndexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        final IndexService closedIndexService = getIndexService(index);
         assertNotSame(indexService, closedIndexService);
         assertNotSame(refreshTask, closedIndexService.getRefreshTask());
         assertFalse(closedIndexService.getRefreshTask().mustReschedule());
@@ -256,8 +254,7 @@ public void testRefreshTaskIsUpdated() throws Exception {
 
         // now reopen the index
         assertAcked(client().admin().indices().prepareOpen(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-        indexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        indexService = getIndexService(index);
         assertNotSame(closedIndexService, indexService);
         refreshTask = indexService.getRefreshTask();
         assertTrue(indexService.getRefreshTask().mustReschedule());
@@ -283,9 +280,7 @@ public void testFsyncTaskIsRunning() throws Exception {
         // now close the index
         final Index index = indexService.index();
         assertAcked(client().admin().indices().prepareClose(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-
-        final IndexService closedIndexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        final IndexService closedIndexService = getIndexService(index);
         assertNotSame(indexService, closedIndexService);
         assertNotSame(fsyncTask, closedIndexService.getFsyncTask());
         assertFalse(closedIndexService.getFsyncTask().mustReschedule());
@@ -294,8 +289,7 @@ public void testFsyncTaskIsRunning() throws Exception {
 
         // now reopen the index
         assertAcked(client().admin().indices().prepareOpen(index.getName()));
-        assertBusy(() -> assertTrue("Index not found: " + index.getName(), getInstanceFromNode(IndicesService.class).hasIndex(index)));
-        indexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(index);
+        indexService = getIndexService(index);
         assertNotSame(closedIndexService, indexService);
         fsyncTask = indexService.getFsyncTask();
         assertTrue(indexService.getRefreshTask().mustReschedule());
@@ -462,16 +456,14 @@ public void testAsyncTranslogTrimTaskOnClosedIndex() throws Exception {
         assertThat(translog.totalOperations(), equalTo(translogOps));
         assertThat(translog.stats().estimatedNumberOfOperations(), equalTo(translogOps));
         assertAcked(client().admin().indices().prepareClose("test").setWaitForActiveShards(ActiveShardCount.DEFAULT));
-
-        indexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(indexService.index());
+        indexService = getIndexService(indexService.index());
         assertTrue(indexService.getTrimTranslogTask().mustReschedule());
 
         final Engine readOnlyEngine = getEngine(indexService.getShard(0));
         assertBusy(() -> assertTrue(isTranslogEmpty(readOnlyEngine)));
 
         assertAcked(client().admin().indices().prepareOpen("test").setWaitForActiveShards(ActiveShardCount.DEFAULT));
-
-        indexService = getInstanceFromNode(IndicesService.class).indexServiceSafe(indexService.index());
+        indexService = getIndexService(indexService.index());
         translog = IndexShardTestCase.getTranslog(indexService.getShard(0));
         assertThat(translog.totalOperations(), equalTo(0));
         assertThat(translog.stats().estimatedNumberOfOperations(), equalTo(0));
@@ -886,6 +878,10 @@ public void testRefreshTaskUpdatesWithDynamicShardLevelRefreshes() throws Except
 
         // OS test case fails if test leaves behind transient cluster setting so need to clear it.
         client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().putNull("*")).get();
+    }
 
+    private IndexService getIndexService(Index index) {
+        return await().atMost(10, TimeUnit.SECONDS)
+            .until(() -> getInstanceFromNode(IndicesService.class).indexService(index), notNullValue());
     }
 }