Skip to content

Commit

Permalink
Adding feature to exclude indexes starting with dot from shard valida…
Browse files Browse the repository at this point in the history 
…tor (#4695)

* Adding feature to exclude indexes starting with dot from shard validator

Signed-off-by: Nishchay Malhotra <[email protected]>
  • Loading branch information
@nishchay21
nishchay21 authored Oct 19, 2022
1 parent a282d39 commit 04eb817
Show file tree
Hide file tree
Showing 7 changed files with 601 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Addition of GeoShape ValueSource level code interfaces for accessing the DocValues.
- Addition of Missing Value feature in the GeoShape Aggregations.
- Install and configure Log4j JUL Adapter for Lucene 9.4 ([#4754](https:/opensearch-project/OpenSearch/pull/4754))
- Added feature to ignore indexes starting with dot during shard limit validation.([#4695](https:/opensearch-project/OpenSearch/pull/4695))
### Changed
### Deprecated
### Removed
Expand Down
306 changes: 305 additions & 1 deletion server/src/internalClusterTest/java/org/opensearch/cluster/shards/ClusterShardLimitIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,28 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.common.Priority;
import org.opensearch.common.network.NetworkModule;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.ByteSizeUnit;
import org.opensearch.core.internal.io.IOUtils;
import org.opensearch.indices.ShardLimitValidator;
import org.opensearch.snapshots.SnapshotInfo;
import org.opensearch.snapshots.SnapshotState;
import org.opensearch.snapshots.mockstore.MockRepository;
import org.opensearch.test.InternalSettingsPlugin;
import org.opensearch.test.InternalTestCluster;
import org.opensearch.test.MockHttpTransport;
import org.opensearch.test.NodeConfigurationSource;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.transport.MockTransportService;
import org.opensearch.transport.nio.MockNioTransportPlugin;

import java.io.IOException;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
Expand All @@ -63,12 +76,18 @@
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
public class ClusterShardLimitIT extends OpenSearchIntegTestCase {
private static final String shardsPerNodeKey = ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey();
private static final String ignoreDotIndexKey = ShardLimitValidator.SETTING_CLUSTER_IGNORE_DOT_INDEXES.getKey();

public void testSettingClusterMaxShards() {
int shardsPerNode = between(1, 500_000);
setShardsPerNode(shardsPerNode);
}

public void testSettingIgnoreDotIndexes() {
boolean ignoreDotIndexes = randomBoolean();
setIgnoreDotIndex(ignoreDotIndexes);
}

public void testMinimumPerNode() {
int negativeShardsPerNode = between(-50_000, 0);
try {
Expand Down Expand Up @@ -100,7 +119,6 @@ public void testIndexCreationOverLimit() {
ShardCounts counts = ShardCounts.forDataNodeCount(dataNodes);

setShardsPerNode(counts.getShardsPerNode());

// Create an index that will bring us up to the limit
createIndex(
"test",
Expand All @@ -127,6 +145,164 @@ public void testIndexCreationOverLimit() {
assertFalse(clusterState.getMetadata().hasIndex("should-fail"));
}

/**
* The test checks if the index starting with the dot can be created if the node has
* number of shards equivalent to the cluster.max_shards_per_node and the cluster.ignore_Dot_indexes
* setting is set to true. If the cluster.ignore_Dot_indexes is set to true index creation of
* indexes starting with dot would succeed.
*/
public void testIndexCreationOverLimitForDotIndexesSucceeds() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();

// Setting the cluster.max_shards_per_node setting according to the data node count.
setShardsPerNode(dataNodes);
setIgnoreDotIndex(true);

/*
Create an index that will bring us up to the limit. It would create index with primary equal to the
dataNodes * dataNodes so that cluster.max_shards_per_node setting is reached.
*/
createIndex(
"test",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, dataNodes * dataNodes)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
.build()
);

// Getting total active shards in the cluster.
int currentActiveShards = client().admin().cluster().prepareHealth().get().getActiveShards();

// Getting cluster.max_shards_per_node setting
ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
String maxShardsPerNode = clusterState.getMetadata()
.settings()
.get(ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey());

// Checking if the total shards created are equivalent to dataNodes * cluster.max_shards_per_node
assertEquals(dataNodes * Integer.parseInt(maxShardsPerNode), currentActiveShards);

createIndex(
".test-index",
Settings.builder().put(indexSettings()).put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0).build()
);

clusterState = client().admin().cluster().prepareState().get().getState();
assertTrue(clusterState.getMetadata().hasIndex(".test-index"));
}

/**
* The test checks if the index starting with the dot should not be created if the node has
* number of shards equivalent to the cluster.max_shards_per_node and the cluster.ignore_Dot_indexes
* setting is set to false. If the cluster.ignore_Dot_indexes is set to false index creation of
* indexes starting with dot would fail as well.
*/
public void testIndexCreationOverLimitForDotIndexesFail() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();
int maxAllowedShards = dataNodes * dataNodes;

// Setting the cluster.max_shards_per_node setting according to the data node count.
setShardsPerNode(dataNodes);

/*
Create an index that will bring us up to the limit. It would create index with primary equal to the
dataNodes * dataNodes so that cluster.max_shards_per_node setting is reached.
*/
createIndex(
"test",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, maxAllowedShards)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
.build()
);

// Getting total active shards in the cluster.
int currentActiveShards = client().admin().cluster().prepareHealth().get().getActiveShards();

// Getting cluster.max_shards_per_node setting
ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
String maxShardsPerNode = clusterState.getMetadata()
.settings()
.get(ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey());

// Checking if the total shards created are equivalent to dataNodes * cluster.max_shards_per_node
assertEquals(dataNodes * Integer.parseInt(maxShardsPerNode), currentActiveShards);

int extraShardCount = 1;
try {
createIndex(
".test-index",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, extraShardCount)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
.build()
);
} catch (IllegalArgumentException e) {
verifyException(maxAllowedShards, currentActiveShards, extraShardCount, e);
}
clusterState = client().admin().cluster().prepareState().get().getState();
assertFalse(clusterState.getMetadata().hasIndex(".test-index"));
}

/**
* The test checks if the index starting with the .ds- can be created if the node has
* number of shards equivalent to the cluster.max_shards_per_node and the cluster.ignore_Dot_indexes
* setting is set to true. If the cluster.ignore_Dot_indexes is set to true index creation of
* indexes starting with dot would only succeed and dataStream indexes would still have validation applied.
*/
public void testIndexCreationOverLimitForDataStreamIndexes() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();
int maxAllowedShards = dataNodes * dataNodes;

// Setting the cluster.max_shards_per_node setting according to the data node count.
setShardsPerNode(dataNodes);
setIgnoreDotIndex(true);

/*
Create an index that will bring us up to the limit. It would create index with primary equal to the
dataNodes * dataNodes so that cluster.max_shards_per_node setting is reached.
*/
createIndex(
"test",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, maxAllowedShards)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
.build()
);

// Getting total active shards in the cluster.
int currentActiveShards = client().admin().cluster().prepareHealth().get().getActiveShards();

// Getting cluster.max_shards_per_node setting
ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
String maxShardsPerNode = clusterState.getMetadata()
.settings()
.get(ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey());

// Checking if the total shards created are equivalent to dataNodes * cluster.max_shards_per_node
assertEquals(dataNodes * Integer.parseInt(maxShardsPerNode), currentActiveShards);

int extraShardCount = 1;
try {
createIndex(
".ds-test-index",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, extraShardCount)
.put(SETTING_NUMBER_OF_REPLICAS, 0)
.build()
);
} catch (IllegalArgumentException e) {
verifyException(maxAllowedShards, currentActiveShards, extraShardCount, e);
}
clusterState = client().admin().cluster().prepareState().get().getState();
assertFalse(clusterState.getMetadata().hasIndex(".ds-test-index"));
}

public void testIndexCreationOverLimitFromTemplate() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();

Expand Down Expand Up @@ -414,6 +590,100 @@ public void testOpenIndexOverLimit() {
assertFalse(clusterState.getMetadata().hasIndex("snapshot-index"));
}

public void testIgnoreDotSettingOnMultipleNodes() throws IOException, InterruptedException {
int maxAllowedShardsPerNode = 10, indexPrimaryShards = 11, indexReplicaShards = 1;

InternalTestCluster cluster = new InternalTestCluster(
randomLong(),
createTempDir(),
true,
true,
0,
0,
"cluster",
new NodeConfigurationSource() {
@Override
public Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(ClusterShardLimitIT.this.nodeSettings(nodeOrdinal))
.put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType())
.build();
}

@Override
public Path nodeConfigPath(int nodeOrdinal) {
return null;
}
},
0,
"cluster-",
Arrays.asList(
TestSeedPlugin.class,
MockHttpTransport.TestPlugin.class,
MockTransportService.TestPlugin.class,
MockNioTransportPlugin.class,
InternalSettingsPlugin.class,
MockRepository.Plugin.class
),
Function.identity()
);
cluster.beforeTest(random());

// Starting 3 ClusterManagerOnlyNode nodes
cluster.startClusterManagerOnlyNode(Settings.builder().put("cluster.ignore_dot_indexes", true).build());
cluster.startClusterManagerOnlyNode(Settings.builder().put("cluster.ignore_dot_indexes", false).build());
cluster.startClusterManagerOnlyNode(Settings.builder().put("cluster.ignore_dot_indexes", false).build());

// Starting 2 data nodes
cluster.startDataOnlyNode(Settings.builder().put("cluster.ignore_dot_indexes", false).build());
cluster.startDataOnlyNode(Settings.builder().put("cluster.ignore_dot_indexes", false).build());

// Setting max shards per node to be 10
cluster.client()
.admin()
.cluster()
.prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put(shardsPerNodeKey, maxAllowedShardsPerNode))
.get();

// Creating an index starting with dot having shards greater thn the desired node limit
cluster.client()
.admin()
.indices()
.prepareCreate(".test-index")
.setSettings(
Settings.builder().put(SETTING_NUMBER_OF_SHARDS, indexPrimaryShards).put(SETTING_NUMBER_OF_REPLICAS, indexReplicaShards)
)
.get();

// As active ClusterManagerNode setting takes precedence killing the active one.
// This would be the first one where cluster.ignore_dot_indexes is true because the above calls are blocking.
cluster.stopCurrentClusterManagerNode();

// Waiting for all shards to get assigned
cluster.client().admin().cluster().prepareHealth().setWaitForGreenStatus().get();

// Creating an index starting with dot having shards greater thn the desired node limit
try {
cluster.client()
.admin()
.indices()
.prepareCreate(".test-index1")
.setSettings(
Settings.builder().put(SETTING_NUMBER_OF_SHARDS, indexPrimaryShards).put(SETTING_NUMBER_OF_REPLICAS, indexReplicaShards)
)
.get();
} catch (IllegalArgumentException e) {
ClusterHealthResponse clusterHealth = cluster.client().admin().cluster().prepareHealth().get();
int currentActiveShards = clusterHealth.getActiveShards();
int dataNodeCount = clusterHealth.getNumberOfDataNodes();
int extraShardCount = indexPrimaryShards * (1 + indexReplicaShards);
verifyException(maxAllowedShardsPerNode * dataNodeCount, currentActiveShards, extraShardCount, e);
}

IOUtils.close(cluster);
}

private int ensureMultipleDataNodes(int dataNodes) {
if (dataNodes == 1) {
internalCluster().startNode(dataNode());
Expand Down Expand Up @@ -457,6 +727,29 @@ private void setShardsPerNode(int shardsPerNode) {
}
}

private void setIgnoreDotIndex(boolean ignoreDotIndex) {
try {
ClusterUpdateSettingsResponse response;
if (frequently()) {
response = client().admin()
.cluster()
.prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put(ignoreDotIndexKey, ignoreDotIndex).build())
.get();
assertEquals(ignoreDotIndex, response.getPersistentSettings().getAsBoolean(ignoreDotIndexKey, true));
} else {
response = client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(ignoreDotIndexKey, ignoreDotIndex).build())
.get();
assertEquals(ignoreDotIndex, response.getTransientSettings().getAsBoolean(ignoreDotIndexKey, true));
}
} catch (IllegalArgumentException ex) {
fail(ex.getMessage());
}
}

private void verifyException(int dataNodes, ShardCounts counts, IllegalArgumentException e) {
int totalShards = counts.getFailingIndexShards() * (1 + counts.getFailingIndexReplicas());
int currentShards = counts.getFirstIndexShards() * (1 + counts.getFirstIndexReplicas());
Expand All @@ -471,4 +764,15 @@ private void verifyException(int dataNodes, ShardCounts counts, IllegalArgumentE
assertEquals(expectedError, e.getMessage());
}

private void verifyException(int maxShards, int currentShards, int extraShards, IllegalArgumentException e) {
String expectedError = "Validation Failed: 1: this action would add ["
+ extraShards
+ "] total shards, but this cluster currently has ["
+ currentShards
+ "]/["
+ maxShards
+ "] maximum shards open;";
assertEquals(expectedError, e.getMessage());
}

}
1 change: 1 addition & 0 deletions server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ public void apply(Settings value, Settings current, Settings previous) {
Metadata.SETTING_READ_ONLY_SETTING,
Metadata.SETTING_READ_ONLY_ALLOW_DELETE_SETTING,
ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE,
ShardLimitValidator.SETTING_CLUSTER_IGNORE_DOT_INDEXES,
RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING,
RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_STATE_SYNC_SETTING,
RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_NETWORK_SETTING,
Expand Down
Loading

0 comments on commit 04eb817

Please sign in to comment.