refactoring changes

Signed-off-by: Divyansh Pandey <[email protected]>
opensearch-project · Feb 14, 2025 · da1d789 · da1d789
1 parent f00903e
commit da1d789
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 26 deletions.
diff --git a/...va/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/...va/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java
@@ -258,14 +258,7 @@ public void onFailure(String source, Exception e) {
 
                 @Override
                 public ClusterState execute(final ClusterState currentState) {
-                    if (request.transientSettings().hasValue(CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey())
-                        || request.persistentSettings().hasValue(CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey())) {
-
-                        Settings settings = Settings.builder().put(request.transientSettings()).put(request.persistentSettings()).build();
-
-                        int newValue = CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.get(settings);
-                        validateClusterTotalPrimaryShardsPerNodeSetting(currentState, newValue);
-                    }
+                    validateClusterTotalPrimaryShardsPerNodeSetting(currentState, request);
                     boolean isCompatibilityModeChanging = validateCompatibilityModeSettingRequest(request, state);
                     ClusterState clusterState = updater.updateSettings(
                         currentState,
@@ -334,23 +327,31 @@ private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) {
         }
     }
 
-    private void validateClusterTotalPrimaryShardsPerNodeSetting(ClusterState currentState, int newValue) {
-        // If default value (-1), no validation needed
-        if (newValue == -1) {
-            return;
-        }
+    private void validateClusterTotalPrimaryShardsPerNodeSetting(ClusterState currentState, ClusterUpdateSettingsRequest request) {
+        if (request.transientSettings().hasValue(CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey())
+            || request.persistentSettings().hasValue(CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey())) {
 
-        // Check current state
-        boolean allNodesRemoteStoreEnabled = currentState.nodes()
-            .getNodes()
-            .values()
-            .stream()
-            .allMatch(discoveryNode -> discoveryNode.isRemoteStoreNode());
+            Settings settings = Settings.builder().put(request.transientSettings()).put(request.persistentSettings()).build();
 
-        if (!allNodesRemoteStoreEnabled) {
-            throw new IllegalArgumentException(
-                "Setting [" + CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey() + "] can only be used with remote store domains"
-            );
+            int newValue = CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.get(settings);
+
+            // If default value (-1), no validation needed
+            if (newValue == -1) {
+                return;
+            }
+
+            // Check current state
+            boolean allNodesRemoteStoreEnabled = currentState.nodes()
+                .getNodes()
+                .values()
+                .stream()
+                .allMatch(discoveryNode -> discoveryNode.isRemoteStoreNode());
+
+            if (!allNodesRemoteStoreEnabled) {
+                throw new IllegalArgumentException(
+                    "Setting [" + CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey() + "] can only be used with remote store enabled clusters"
+                );
+            }
         }
     }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -1862,7 +1862,7 @@ public static void validateIndexTotalPrimaryShardsPerNodeSetting(Settings indexS
         boolean isRemoteStoreEnabled = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings);
         if (!isRemoteStoreEnabled) {
             throw new IllegalArgumentException(
-                "Setting [" + INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey() + "] can only be used with remote store domains"
+                "Setting [" + INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey() + "] can only be used with remote store enabled clusters"
             );
         }
     }

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -2620,7 +2620,7 @@ public void testIndexTotalPrimaryShardsPerNodeSettingValidationWithoutRemoteStor
 
         // Verify error message
         assertEquals(
-            "Setting [index.routing.allocation.total_primary_shards_per_node] can only be used with remote store domains",
+            "Setting [index.routing.allocation.total_primary_shards_per_node] can only be used with remote store enabled clusters",
             exception.getMessage()
         );
     }

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
@@ -2454,7 +2454,7 @@ public void testIndexPrimaryShardsSetting() {
         List<Throwable> throwables = putTemplate(xContentRegistry(), request, clusterSettings);
         assertThat(throwables.get(0), instanceOf(IllegalArgumentException.class));
         assertEquals(
-            "Setting [index.routing.allocation.total_primary_shards_per_node] can only be used with remote store domains",
+            "Setting [index.routing.allocation.total_primary_shards_per_node] can only be used with remote store enabled clusters",
             throwables.get(0).getMessage()
         );
     }