Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Remote Store] fail index template creation with refresh_interval less than cluster minimum #11340

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
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
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
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());
linuxpi marked this conversation as resolved.
Show resolved Hide resolved
}

if (indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@
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 @@
if (metadata.templates() != null) {
// TODO: Should all existing templates be deleted first?
for (final IndexTemplateMetadata cursor : metadata.templates().values()) {
MetadataCreateIndexService.validateRefreshIntervalSettings(cursor.settings(), clusterSettings);

Check warning on line 585 in server/src/main/java/org/opensearch/snapshots/RestoreService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/snapshots/RestoreService.java#L585

Added line #L585 was not covered by tests
linuxpi marked this conversation as resolved.
Show resolved Hide resolved
mdBuilder.put(cursor);
}
}
Expand Down
Loading