test fixes

Signed-off-by: Marc Handalian <[email protected]>
opensearch-project · Aug 29, 2024 · c5d29a9 · c5d29a9
1 parent a42af7e
commit c5d29a9
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 31 deletions.
diff --git a/...ernalClusterTest/java/org/opensearch/indices/settings/SearchOnlyReplicaFeatureFlagIT.java b/...ernalClusterTest/java/org/opensearch/indices/settings/SearchOnlyReplicaFeatureFlagIT.java
@@ -17,7 +17,6 @@
 
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
-import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 1)
 public class SearchOnlyReplicaFeatureFlagIT extends OpenSearchIntegTestCase {
@@ -32,27 +31,15 @@ protected Settings featureFlagSettings() {
             .build();
     }
 
-    @Override
-    public Settings indexSettings() {
-        return Settings.builder()
-            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS, 1)
-            .put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
-            .build();
-    }
-
     public void testCreateFeatureFlagDisabled() {
         Settings settings = Settings.builder().put(indexSettings()).put(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL, false).build();
-
         SettingsException settingsException = expectThrows(SettingsException.class, () -> createIndex(TEST_INDEX, settings));
         assertTrue(settingsException.getMessage().contains("unknown setting"));
     }
 
     public void testUpdateFeatureFlagDisabled() {
         Settings settings = Settings.builder()
             .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "0ms") // so that after we punt a node we can immediately try to
-            // reallocate after node left.
             .put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
             .build();
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/settings/SearchOnlyReplicaIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/settings/SearchOnlyReplicaIT.java
@@ -8,6 +8,7 @@
 
 package org.opensearch.indices.settings;
 
+import org.opensearch.action.support.WriteRequest;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.Metadata;
@@ -24,6 +25,7 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
 import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING;
+import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class SearchOnlyReplicaIT extends OpenSearchIntegTestCase {
@@ -79,9 +81,9 @@ public void testUpdateDocRepFails() {
         assertEquals(expectedFailureMessage, illegalArgumentException.getMessage());
     }
 
-    public void testSearchReplicasAreNotPrimaryEligible() throws IOException {
-        int numSearchReplicas = randomIntBetween(0, 3);
-        int numWriterReplicas = randomIntBetween(0, 3);
+    public void testFailoverWithSearchReplica_WithWriterReplicas() throws IOException {
+        int numSearchReplicas = 1;
+        int numWriterReplicas = 1;
         internalCluster().startClusterManagerOnlyNode();
         String primaryNodeName = internalCluster().startDataOnlyNode();
         createIndex(
@@ -93,29 +95,51 @@ public void testSearchReplicasAreNotPrimaryEligible() throws IOException {
                 .build()
         );
         ensureYellow(TEST_INDEX);
-        for (int i = 0; i < numSearchReplicas + numWriterReplicas; i++) {
-            internalCluster().startDataOnlyNode();
-        }
+        // add 2 nodes for the replicas
+        internalCluster().startDataOnlyNodes(2);
         ensureGreen(TEST_INDEX);
 
         // assert shards are on separate nodes & all active
         assertActiveShardCounts(numSearchReplicas, numWriterReplicas);
 
         // stop the primary and ensure search shard is not promoted:
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNodeName));
-        ensureRed(TEST_INDEX);
+        ensureYellowAndNoInitializingShards(TEST_INDEX);
+
+        assertActiveShardCounts(numSearchReplicas, 0); // 1 repl is inactive that was promoted to primary
+        // add back a node
+        internalCluster().startDataOnlyNode();
+        ensureGreen(TEST_INDEX);
+
+    }
+
+    public void testFailoverWithSearchReplica_WithoutWriterReplicas() throws IOException {
+        int numSearchReplicas = 1;
+        int numWriterReplicas = 0;
+        internalCluster().startClusterManagerOnlyNode();
+        String primaryNodeName = internalCluster().startDataOnlyNode();
+        createIndex(
+            TEST_INDEX,
+            Settings.builder()
+                .put(indexSettings())
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numWriterReplicas)
+                .put(IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS, numSearchReplicas)
+                .build()
+        );
+        ensureYellow(TEST_INDEX);
+        client().prepareIndex(TEST_INDEX).setId("1").setSource("foo", "bar").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get();
+        // start a node for our search replica
+        String replica = internalCluster().startDataOnlyNode();
+        ensureGreen(TEST_INDEX);
+        assertActiveSearchShards(numSearchReplicas);
+        assertHitCount(client(replica).prepareSearch(TEST_INDEX).setSize(0).setPreference("_only_local").get(), 1);
 
-        if (numWriterReplicas > 0) {
-            assertActiveShardCounts(numSearchReplicas, numWriterReplicas - 1); // 1 repl is inactive that was promoted to primary
-            // add back a node
-            internalCluster().startDataOnlyNode();
-            ensureGreen(TEST_INDEX);
-        } else {
-            // index falls red and does not recover
-            // Without any writer replica with n2n replication this is an unrecoverable scenario and snapshot restore is required.
-            ensureRed(TEST_INDEX);
-            assertActiveSearchShards(numSearchReplicas);
-        }
+        // stop the primary and ensure search shard is not promoted:
+        internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNodeName));
+        ensureRed(TEST_INDEX);
+        assertActiveSearchShards(numSearchReplicas);
+        // while red our search shard is still searchable
+        assertHitCount(client(replica).prepareSearch(TEST_INDEX).setSize(0).setPreference("_only_local").get(), 1);
     }
 
     public void testSearchReplicaScaling() {