Skip to content

Commit 7388205

Browse files
pandeydivyansh1803Divyansh Pandey
pandeydivyansh1803
and
Divyansh Pandey
authored
Update validator for index update request (#17529)
Signed-off-by: Divyansh Pandey <dpaandey@amazon.com> Co-authored-by: Divyansh Pandey <dpaandey@amazon.com>
1 parent 342c645 commit 7388205

File tree

3 files changed

+103
-3
lines changed

3 files changed

+103
-3
lines changed

‎server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/ShardsLimitAllocationDeciderRemoteStoreEnabledIT.java

+71
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.opensearch.cluster.routing.allocation.decider;
1010

11+
import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest;
12+
import org.opensearch.action.support.clustermanager.AcknowledgedResponse;
1113
import org.opensearch.cluster.ClusterState;
1214
import org.opensearch.cluster.routing.IndexShardRoutingTable;
1315
import org.opensearch.cluster.routing.ShardRouting;
@@ -99,6 +101,75 @@ public void testIndexPrimaryShardLimit() throws Exception {
99101
});
100102
}
101103

104+
public void testUpdatingIndexPrimaryShardLimit() throws Exception {
105+
// Create first index with primary shard limit
106+
Settings firstIndexSettings = Settings.builder()
107+
.put(remoteStoreIndexSettings(0, 4)) // 4 shards, 0 replicas
108+
.put(INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey(), 1)
109+
.build();
110+
111+
// Create first index
112+
createIndex("test1", firstIndexSettings);
113+
114+
// Update the index settings to set INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING
115+
UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest("test1");
116+
Settings updatedSettings = Settings.builder().put(INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey(), 1).build();
117+
updateSettingsRequest.settings(updatedSettings);
118+
119+
AcknowledgedResponse response = client().admin().indices().updateSettings(updateSettingsRequest).actionGet();
120+
121+
assertTrue(response.isAcknowledged());
122+
123+
// Create second index
124+
createIndex("test2", remoteStoreIndexSettings(0, 4));
125+
126+
assertBusy(() -> {
127+
ClusterState state = client().admin().cluster().prepareState().get().getState();
128+
129+
// Check total number of shards (8 total: 4 from each index)
130+
assertEquals("Total shards should be 8", 8, state.getRoutingTable().allShards().size());
131+
132+
// Count assigned and unassigned shards for test1
133+
int test1AssignedShards = 0;
134+
int test1UnassignedShards = 0;
135+
Map<String, Integer> nodePrimaryCount = new HashMap<>();
136+
137+
// Check test1 shard distribution
138+
for (IndexShardRoutingTable shardRouting : state.routingTable().index("test1")) {
139+
for (ShardRouting shard : shardRouting) {
140+
if (shard.assignedToNode()) {
141+
test1AssignedShards++;
142+
// Count primaries per node for test1
143+
String nodeId = shard.currentNodeId();
144+
nodePrimaryCount.merge(nodeId, 1, Integer::sum);
145+
} else {
146+
test1UnassignedShards++;
147+
}
148+
}
149+
}
150+
151+
// Check test2 shard assignment
152+
int test2UnassignedShards = 0;
153+
for (IndexShardRoutingTable shardRouting : state.routingTable().index("test2")) {
154+
for (ShardRouting shard : shardRouting) {
155+
if (!shard.assignedToNode()) {
156+
test2UnassignedShards++;
157+
}
158+
}
159+
}
160+
161+
// Assertions
162+
assertEquals("test1 should have 3 assigned shards", 3, test1AssignedShards);
163+
assertEquals("test1 should have 1 unassigned shard", 1, test1UnassignedShards);
164+
assertEquals("test2 should have no unassigned shards", 0, test2UnassignedShards);
165+
166+
// Verify no node has more than one primary shard of test1
167+
for (Integer count : nodePrimaryCount.values()) {
168+
assertTrue("No node should have more than 1 primary shard of test1", count <= 1);
169+
}
170+
});
171+
}
172+
102173
public void testClusterPrimaryShardLimitss() throws Exception {
103174
// Update cluster setting to limit primary shards per node
104175
updateClusterSetting(CLUSTER_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey(), 1);

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -1847,7 +1847,8 @@ public static void validateRefreshIntervalSettings(Settings requestSettings, Clu
18471847
}
18481848

18491849
/**
1850-
* Validates {@code index.routing.allocation.total_primary_shards_per_node} is only set for remote store enabled cluster
1850+
* Validates the {@code index.routing.allocation.total_primary_shards_per_node} setting during index creation.
1851+
* Ensures this setting is only specified for remote store enabled clusters.
18511852
*/
18521853
// TODO : Update this check for SegRep to DocRep migration on need basis
18531854
public static void validateIndexTotalPrimaryShardsPerNodeSetting(Settings indexSettings) {

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java

+30-2
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
4444
import org.opensearch.cluster.block.ClusterBlock;
4545
import org.opensearch.cluster.block.ClusterBlocks;
46+
import org.opensearch.cluster.node.DiscoveryNode;
4647
import org.opensearch.cluster.routing.RoutingTable;
4748
import org.opensearch.cluster.routing.allocation.AllocationService;
4849
import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance;
@@ -78,12 +79,12 @@
7879
import static org.opensearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
7980
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
8081
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
81-
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateIndexTotalPrimaryShardsPerNodeSetting;
8282
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap;
8383
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
8484
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings;
8585
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex;
8686
import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findComponentTemplate;
87+
import static org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider.INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING;
8788
import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
8889
import static org.opensearch.index.IndexSettings.same;
8990

@@ -140,7 +141,7 @@ public void updateSettings(
140141

141142
validateRefreshIntervalSettings(normalizedSettings, clusterService.getClusterSettings());
142143
validateTranslogDurabilitySettings(normalizedSettings, clusterService.getClusterSettings(), clusterService.getSettings());
143-
validateIndexTotalPrimaryShardsPerNodeSetting(normalizedSettings);
144+
validateIndexTotalPrimaryShardsPerNodeSetting(normalizedSettings, clusterService);
144145
final int defaultReplicaCount = clusterService.getClusterSettings().get(Metadata.DEFAULT_REPLICA_COUNT_SETTING);
145146

146147
Settings.Builder settingsForClosedIndices = Settings.builder();
@@ -549,4 +550,31 @@ private void validateSearchReplicaCountSettings(Settings requestSettings, Index[
549550
}
550551
}
551552
}
553+
554+
/**
555+
* Validates the 'index.routing.allocation.total_primary_shards_per_node' setting during index settings update.
556+
* Ensures this setting can only be modified for existing indices in remote store enabled clusters.
557+
*/
558+
public static void validateIndexTotalPrimaryShardsPerNodeSetting(Settings indexSettings, ClusterService clusterService) {
559+
// Get the setting value
560+
int indexPrimaryShardsPerNode = INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.get(indexSettings);
561+
562+
// If default value (-1), no validation needed
563+
if (indexPrimaryShardsPerNode == -1) {
564+
return;
565+
}
566+
567+
// Check if remote store is enabled
568+
boolean isRemoteStoreEnabled = clusterService.state()
569+
.nodes()
570+
.getNodes()
571+
.values()
572+
.stream()
573+
.allMatch(DiscoveryNode::isRemoteStoreNode);
574+
if (!isRemoteStoreEnabled) {
575+
throw new IllegalArgumentException(
576+
"Setting [" + INDEX_TOTAL_PRIMARY_SHARDS_PER_NODE_SETTING.getKey() + "] can only be used with remote store enabled clusters"
577+
);
578+
}
579+
}
552580
}

0 commit comments

Comments
 (0)