Skip to content

Commit

Permalink
[Remote Store] Fail index template creation with refresh_interval les…
Browse files Browse the repository at this point in the history 
…s than cluster minimum (#11340)

---------

Signed-off-by: bansvaru <bansvaru@amazon.com>
  • Loading branch information
@linuxpi
linuxpi committed Nov 29, 2023
1 parent 530a93b commit 6bc0cd6
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 9 deletions.
50 changes: 42 additions & 8 deletions ...ternalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,27 @@
import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest;
import org.opensearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.index.Index;
import org.opensearch.index.IndexService;
import org.opensearch.index.IndexSettings;
import org.opensearch.indices.IndicesService;
import org.opensearch.snapshots.AbstractSnapshotIntegTestCase;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.junit.Before;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;

import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING;

@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
public class ClusterIndexRefreshIntervalIT extends OpenSearchIntegTestCase {
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 2)
public class ClusterIndexRefreshIntervalIT extends AbstractSnapshotIntegTestCase {

public static final String INDEX_NAME = "test-index";

Expand All @@ -69,9 +73,39 @@ public void setUp() throws Exception {
internalCluster().startClusterManagerOnlyNode();
}

static void putIndexTemplate(String refreshInterval) {
PutIndexTemplateRequest request = new PutIndexTemplateRequest("my-template"); // <1>
request.patterns(Arrays.asList("pattern-1", "log-*")); // <2>

request.settings(
Settings.builder() // <1>
.put("index.number_of_shards", 1)
.put("index.number_of_replicas", 1)
.put("index.refresh_interval", refreshInterval)
);
assertTrue(client().admin().indices().putTemplate(request).actionGet().isAcknowledged());
}

public void testIndexTemplateCreationSucceedsWhenNoMinimumRefreshInterval() throws ExecutionException, InterruptedException {
String clusterManagerName = internalCluster().getClusterManagerName();
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
putIndexTemplate("2s");

// Test index creation using template with valid refresh interval
String indexName = "log-myindex-1";
createIndex(indexName);
ensureYellowAndNoInitializingShards(indexName);
ensureGreen(indexName);
GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
String uuid = getIndexResponse.getSettings().get(indexName).get(IndexMetadata.SETTING_INDEX_UUID);
IndexService indexService = indicesService.indexService(new Index(indexName, uuid));
assertEquals(TimeValue.timeValueSeconds(2), indexService.getRefreshTaskInterval());
}

public void testDefaultRefreshIntervalWithUpdateClusterAndIndexSettings() throws Exception {
String clusterManagerName = internalCluster().getClusterManagerName();
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
createIndex(INDEX_NAME);
ensureYellowAndNoInitializingShards(INDEX_NAME);
ensureGreen(INDEX_NAME);
Expand All @@ -90,7 +124,7 @@ public void testDefaultRefreshIntervalWithUpdateClusterAndIndexSettings() throws
.get();
assertEquals(refreshInterval, indexService.getRefreshTaskInterval());

// Update of cluster.minimum.index.refresh_interval setting to value less than refreshInterval above will fail
// Update of cluster.minimum.index.refresh_interval setting to value more than default refreshInterval above will fail
TimeValue invalidMinimumRefreshInterval = TimeValue.timeValueMillis(refreshInterval.millis() + randomIntBetween(1, 1000));
IllegalArgumentException exceptionDuringMinUpdate = assertThrows(
IllegalArgumentException.class,
Expand Down Expand Up @@ -205,7 +239,7 @@ public void testRefreshIntervalDisabled() throws ExecutionException, Interrupted
.getAsTime(IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.MINUS_ONE);
boolean createIndexSuccess = clusterMinimumRefreshInterval.equals(TimeValue.MINUS_ONE);
String clusterManagerName = internalCluster().getClusterManagerName();
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
Settings settings = Settings.builder()
.put(indexSettings())
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), IndexSettings.MINIMUM_REFRESH_INTERVAL)
Expand Down Expand Up @@ -236,7 +270,7 @@ protected TimeValue getMinRefreshIntervalForRefreshDisabled() {

public void testInvalidRefreshInterval() {
String invalidRefreshInterval = "-10s";
internalCluster().startDataOnlyNodes(2);
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
Settings settings = Settings.builder()
.put(indexSettings())
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), invalidRefreshInterval)
Expand All @@ -251,7 +285,7 @@ public void testInvalidRefreshInterval() {
}

public void testCreateIndexWithExplicitNullRefreshInterval() throws ExecutionException, InterruptedException {
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
Settings indexSettings = Settings.builder()
.put(indexSettings())
.putNull(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey())
Expand All @@ -278,7 +312,7 @@ public void testCreateIndexWithExplicitNullRefreshInterval() throws ExecutionExc
* the index setting. The underlying index should continue to use the same refresh interval as earlier.
*/
public void testClusterMinimumChangeOnIndexWithCustomRefreshInterval() throws ExecutionException, InterruptedException {
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
TimeValue customRefreshInterval = TimeValue.timeValueSeconds(getDefaultRefreshInterval().getSeconds() + randomIntBetween(1, 5));
Settings indexSettings = Settings.builder()
.put(indexSettings())
Expand Down
178 changes: 178 additions & 0 deletions ...t/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalWithNodeSettingsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,27 @@

package org.opensearch.cluster.metadata;

import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
import org.opensearch.action.admin.indices.template.get.GetIndexTemplatesResponse;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.indices.IndicesService;
import org.opensearch.snapshots.SnapshotInfo;
import org.opensearch.snapshots.SnapshotState;

import java.util.Locale;
import java.util.concurrent.ExecutionException;

import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.opensearch.index.IndexSettings.INDEX_REFRESH_INTERVAL_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertIndexTemplateExists;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertIndexTemplateMissing;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

public class ClusterIndexRefreshIntervalWithNodeSettingsIT extends ClusterIndexRefreshIntervalIT {

Expand All @@ -26,6 +44,166 @@ protected Settings nodeSettings(int nodeOrdinal) {
.build();
}

public void testIndexTemplateCreationFailsWithLessThanMinimumRefreshInterval() throws ExecutionException, InterruptedException {
Throwable throwable = assertThrows(IllegalArgumentException.class, () -> putIndexTemplate("0s"));
assertEquals(
throwable.getMessage(),
String.format(
Locale.ROOT,
"invalid index.refresh_interval [%s]: cannot be smaller than cluster.minimum.index.refresh_interval [%s]",
"0s",
getMinRefreshIntervalForRefreshDisabled()
)
);
}

public void testIndexTemplateSnapshotRestoreWithLessThanMinimumRefreshInterval() throws ExecutionException, InterruptedException {
putIndexTemplate("2s");
createRepository("test-repo", "fs");

final SnapshotInfo snapshotInfo = clusterAdmin().prepareCreateSnapshot("test-repo", "test-snap")
.setIndices()
.setWaitForCompletion(true)
.execute()
.get()
.getSnapshotInfo();
assertThat(snapshotInfo.state(), is(SnapshotState.SUCCESS));

assertThat(snapshotInfo.totalShards(), equalTo(0));
assertThat(snapshotInfo.successfulShards(), equalTo(0));

assertThat(client().admin().indices().prepareDeleteTemplate("my-template").get().isAcknowledged(), equalTo(true));

GetIndexTemplatesResponse getIndexTemplatesResponse = client().admin().indices().prepareGetTemplates().get();
assertIndexTemplateMissing(getIndexTemplatesResponse, "my-template");

String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "4s")
)
.get();

logger.info("--> try restore cluster state -- should fail");
Throwable throwable = assertThrows(
IllegalArgumentException.class,
() -> clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap")
.setWaitForCompletion(true)
.setRestoreGlobalState(true)
.execute()
.actionGet()
);
assertEquals(
throwable.getMessage(),
"invalid index.refresh_interval [2s]: cannot be smaller than cluster.minimum.index.refresh_interval [4s]"
);

client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "1s")
)
.get();

logger.info("--> restore cluster state");
RestoreSnapshotResponse restoreSnapshotResponse = clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap")
.setWaitForCompletion(true)
.setRestoreGlobalState(true)
.execute()
.actionGet();
assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), equalTo(0));

getIndexTemplatesResponse = client().admin().indices().prepareGetTemplates().get();
assertIndexTemplateExists(getIndexTemplatesResponse, "my-template");

}

public void testIndexSnapshotRestoreWithLessThanMinimumRefreshInterval() throws ExecutionException, InterruptedException {
createIndex(
"my-index",
Settings.builder()
.put(indexSettings())
.put(INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
.put(INDEX_REFRESH_INTERVAL_SETTING.getKey(), "2s")
.build()
);

createRepository("test-repo", "fs");

final SnapshotInfo snapshotInfo = clusterAdmin().prepareCreateSnapshot("test-repo", "test-snap")
.setIndices()
.setWaitForCompletion(true)
.execute()
.get()
.getSnapshotInfo();
assertThat(snapshotInfo.state(), is(SnapshotState.SUCCESS));

assertThat(snapshotInfo.totalShards(), equalTo(1));
assertThat(snapshotInfo.successfulShards(), equalTo(1));

GetIndexResponse getIndexResponse = client().admin().indices().getIndex(new GetIndexRequest().indices("my-index")).get();
assertEquals(1, getIndexResponse.indices().length);
assertEquals("2s", getIndexResponse.getSetting("my-index", INDEX_REFRESH_INTERVAL_SETTING.getKey()));

assertThat(client().admin().indices().prepareDelete("my-index").get().isAcknowledged(), equalTo(true));

getIndexResponse = client().admin().indices().getIndex(new GetIndexRequest()).get();
assertEquals(getIndexResponse.indices().length, 0);

String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "4s")
)
.get();

logger.info("--> try restore cluster state -- should fail");
Throwable throwable = assertThrows(
IllegalArgumentException.class,
() -> clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap")
.setWaitForCompletion(true)
.setRestoreGlobalState(true)
.execute()
.actionGet()
);
assertEquals(
throwable.getMessage(),
"invalid index.refresh_interval [2s]: cannot be smaller than cluster.minimum.index.refresh_interval [4s]"
);

client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "1s")
)
.get();

logger.info("--> try restore cluster state -- should pass");
RestoreSnapshotResponse restoreSnapshotResponse = clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap")
.setWaitForCompletion(true)
.setRestoreGlobalState(true)
.execute()
.actionGet();
assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), equalTo(1));

getIndexResponse = client().admin().indices().getIndex(new GetIndexRequest().indices("my-index")).get();
assertEquals(getIndexResponse.indices().length, 1);
}

@Override
protected TimeValue getMinRefreshIntervalForRefreshDisabled() {
return TimeValue.timeValueSeconds(1);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,7 @@ public static void validateTranslogRetentionSettings(Settings indexSettings) {
* @param requestSettings settings passed in during index create/update request
* @param clusterSettings cluster setting
*/
static void validateRefreshIntervalSettings(Settings requestSettings, ClusterSettings clusterSettings) {
public static void validateRefreshIntervalSettings(Settings requestSettings, ClusterSettings clusterSettings) {
if (IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.exists(requestSettings) == false) {
return;
}
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
import java.util.stream.Collectors;

import static org.opensearch.cluster.metadata.MetadataCreateDataStreamService.validateTimestampFieldMapping;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
import static org.opensearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED;

/**
Expand Down Expand Up @@ -1529,6 +1530,9 @@ private void validate(String name, @Nullable Settings settings, List<String> ind
Optional.empty()
);
validationErrors.addAll(indexSettingsValidation);

// validate index refresh interval settings
validateRefreshIntervalSettings(settings, clusterService.getClusterSettings());
}

if (indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/snapshots/RestoreService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ public ClusterState execute(ClusterState currentState) {
createIndexService.validateIndexName(renamedIndexName, currentState);
createIndexService.validateDotIndex(renamedIndexName, isHidden);
createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
MetadataCreateIndexService.validateRefreshIntervalSettings(
snapshotIndexMetadata.getSettings(),
clusterSettings
);
IndexMetadata.Builder indexMdBuilder = IndexMetadata.builder(snapshotIndexMetadata)
.state(IndexMetadata.State.OPEN)
.index(renamedIndexName);
Expand Down Expand Up @@ -578,6 +582,7 @@ public ClusterState execute(ClusterState currentState) {
if (metadata.templates() != null) {
// TODO: Should all existing templates be deleted first?
for (final IndexTemplateMetadata cursor : metadata.templates().values()) {
MetadataCreateIndexService.validateRefreshIntervalSettings(cursor.settings(), clusterSettings);
mdBuilder.put(cursor);
}
}
Expand Down

0 comments on commit 6bc0cd6

Please sign in to comment.