Addressing comments

Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
opensearch-project · Jan 5, 2023 · 3a1afc0 · 3a1afc0
1 parent 099a7d1
commit 3a1afc0
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 88 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased 3.0]
 ### Added
-- Add support of default replica count change using total awareness attribute([#5610](https://github.com/opensearch-project/OpenSearch/pull/5610))
+- Add support of default replica count cluster setting ([#5610](https://github.com/opensearch-project/OpenSearch/pull/5610))
 - Hardened token permissions in GitHub workflows ([#4587](https://github.com/opensearch-project/OpenSearch/pull/4587))
 - Support for HTTP/2 (server-side) ([#3847](https://github.com/opensearch-project/OpenSearch/pull/3847))
 - Add getter for path field in NestedQueryBuilder ([#4636](https://github.com/opensearch-project/OpenSearch/pull/4636))

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java
@@ -162,7 +162,7 @@ public interface Custom extends NamedDiffable<Custom>, ToXContentFragment, Clust
     }
 
     public static final Setting<Integer> DEFAULT_REPLICA_COUNT_SETTING = Setting.intSetting(
-        "cluster.default_replica_count",
+        "cluster.default_number_of_replicas",
         1,
         Property.Dynamic,
         Property.NodeScope

diff --git a/.../test/java/org/opensearch/action/support/replication/TransportReplicationActionTests.java b/.../test/java/org/opensearch/action/support/replication/TransportReplicationActionTests.java
@@ -201,6 +201,13 @@ public void setUp() throws Exception {
         transportService.acceptIncomingRequests();
         shardStateAction = new ShardStateAction(clusterService, transportService, null, null, threadPool);
         action = new TestAction(Settings.EMPTY, "internal:testAction", transportService, clusterService, shardStateAction, threadPool);
+        Metadata metadata = Metadata.builder()
+            .transientSettings(Settings.builder().put(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey(), 1).build())
+            .build();
+        ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
+            .metadata(metadata)
+            .build();
+        setState(clusterService, clusterState);
     }
 
     @Override

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -1072,6 +1072,13 @@ public void testParseMappingsWithTypelessTemplate() throws Exception {
 
     public void testvalidateIndexSettings() {
         ClusterService clusterService = mock(ClusterService.class);
+        Metadata metadata = Metadata.builder()
+            .transientSettings(Settings.builder().put(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey(), 1).build())
+            .build();
+        ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
+            .metadata(metadata)
+            .build();
+
         ThreadPool threadPool = new TestThreadPool(getTestName());
         Settings settings = Settings.builder()
             .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone, rack")
@@ -1082,6 +1089,8 @@ public void testvalidateIndexSettings() {
         ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
         when(clusterService.getSettings()).thenReturn(settings);
         when(clusterService.getClusterSettings()).thenReturn(clusterSettings);
+        when(clusterService.state()).thenReturn(clusterState);
+
         MetadataCreateIndexService checkerService = new MetadataCreateIndexService(
             settings,
             clusterService,
@@ -1113,37 +1122,6 @@ public void testvalidateIndexSettings() {
         validationErrors = checkerService.getIndexSettingsValidationErrors(settings, false, Optional.empty());
         assertThat(validationErrors.size(), is(0));
 
-        settings = Settings.builder()
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone, rack")
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "zone.values", "a, b")
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "rack.values", "c, d, e")
-            .put(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey(), true)
-            .put(AwarenessReplicaBalance.FORCE_AWARENESS_REPLICA_SETTING.getKey(), true)
-            .build();
-
-        clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
-        when(clusterService.getSettings()).thenReturn(settings);
-        when(clusterService.getClusterSettings()).thenReturn(clusterSettings);
-
-        checkerService = new MetadataCreateIndexService(
-            settings,
-            clusterService,
-            null,
-            null,
-            null,
-            createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
-            new Environment(Settings.builder().put("path.home", "dummy").build(), null),
-            IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
-            threadPool,
-            null,
-            new SystemIndices(Collections.emptyMap()),
-            true,
-            new AwarenessReplicaBalance(settings, clusterService.getClusterSettings())
-        );
-
-        validationErrors = checkerService.getIndexSettingsValidationErrors(settings, false, Optional.empty());
-        assertThat(validationErrors.size(), is(0));
-
         threadPool.shutdown();
     }
 

diff --git a/...src/test/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalanceTests.java b/...src/test/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalanceTests.java
@@ -39,30 +39,6 @@ public void testNoForcedAwarenessAttribute() {
         assertEquals(awarenessReplicaBalance.validate(1, autoExpandReplica), Optional.empty());
     }
 
-    public void testStaticMaxAwarenessAttributes() {
-        Settings settings = Settings.builder()
-            .put("cluster.routing.allocation.awareness.attributes", "rack_id")
-            .put(SETTING_AUTO_EXPAND_REPLICAS, "0-1")
-            .build();
-        assertThat(AwarenessReplicaBalance.maxAwarenessAttributes(settings), equalTo(1));
-
-        settings = Settings.builder()
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone, rack")
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "zone.values", "a, b")
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "rack.values", "c, d, e")
-            .put(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey(), true)
-            .put(SETTING_AUTO_EXPAND_REPLICAS, "0-2")
-            .build();
-        assertThat(AwarenessReplicaBalance.maxAwarenessAttributes(settings), equalTo(3));
-
-        settings = Settings.builder()
-            .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone, rack")
-            .put(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey(), true)
-            .put(SETTING_AUTO_EXPAND_REPLICAS, "0-1")
-            .build();
-        assertThat(AwarenessReplicaBalance.maxAwarenessAttributes(settings), equalTo(1));
-    }
-
     public void testForcedAwarenessAttribute() {
         // When auto expand replica settings is as per zone awareness
         Settings settings = Settings.builder()
@@ -155,33 +131,4 @@ public void testForcedAwarenessAttributeDisabled() {
         assertEquals(awarenessReplicaBalance.validate(1, autoExpandReplica), Optional.empty());
     }
 
-    public void testSetUseForceZoneForReplicaWhenAllocationAwarenessIsDefault() {
-        Settings settings = Settings.builder().put(AwarenessReplicaBalance.FORCE_AWARENESS_REPLICA_SETTING.getKey(), true).build();
-
-        IllegalArgumentException iae = expectThrows(
-            IllegalArgumentException.class,
-            () -> new AwarenessReplicaBalance(settings, EMPTY_CLUSTER_SETTINGS)
-        );
-        assertEquals(
-            "To enable cluster.routing.allocation.awareness.force_replica, cluster.routing.allocation.awareness.balance should be enabled",
-            iae.getMessage()
-        );
-    }
-
-    public void testSetUseForceZoneForReplicaWhenAllocationAwarenessIsFalse() {
-        Settings settings = Settings.builder()
-            .put(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey(), false)
-            .put(AwarenessReplicaBalance.FORCE_AWARENESS_REPLICA_SETTING.getKey(), true)
-            .build();
-
-        IllegalArgumentException iae = expectThrows(
-            IllegalArgumentException.class,
-            () -> new AwarenessReplicaBalance(settings, EMPTY_CLUSTER_SETTINGS)
-        );
-        assertEquals(
-            "To enable cluster.routing.allocation.awareness.force_replica, cluster.routing.allocation.awareness.balance should be enabled",
-            iae.getMessage()
-        );
-    }
-
 }
diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java
@@ -2413,14 +2413,14 @@ public void manageUseZoneForReplicaSetting(boolean apply) {
                     "a, b, c"
                 )
                 .put(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey(), true)
-                .put(AwarenessReplicaBalance.FORCE_AWARENESS_REPLICA_SETTING.getKey(), true)
+                .put(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey(), 2)
                 .build();
         } else {
             settings = Settings.builder()
                 .putNull(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey())
                 .putNull(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "zone.values")
                 .putNull(AwarenessReplicaBalance.CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.getKey())
-                .putNull(AwarenessReplicaBalance.FORCE_AWARENESS_REPLICA_SETTING.getKey())
+                .putNull(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey())
                 .build();
         }
         ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();