From 6bc0cd68e25d63d31d735634b9edfc76ca4f1749 Mon Sep 17 00:00:00 2001 From: Varun Bansal Date: Wed, 29 Nov 2023 21:00:45 +0530 Subject: [PATCH] [Remote Store] Fail index template creation with refresh_interval less than cluster minimum (#11340) --------- Signed-off-by: bansvaru --- .../ClusterIndexRefreshIntervalIT.java | 50 ++++- ...ndexRefreshIntervalWithNodeSettingsIT.java | 178 ++++++++++++++++++ .../metadata/MetadataCreateIndexService.java | 2 +- .../MetadataIndexTemplateService.java | 4 + .../opensearch/snapshots/RestoreService.java | 5 + 5 files changed, 230 insertions(+), 9 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java index 54824b67b7abc..25fa7ae7eb8eb 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java @@ -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"; @@ -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 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 dataNodes = internalCluster().startDataOnlyNodes(2); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); createIndex(INDEX_NAME); ensureYellowAndNoInitializingShards(INDEX_NAME); ensureGreen(INDEX_NAME); @@ -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, @@ -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 dataNodes = internalCluster().startDataOnlyNodes(2); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); Settings settings = Settings.builder() .put(indexSettings()) .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), IndexSettings.MINIMUM_REFRESH_INTERVAL) @@ -236,7 +270,7 @@ protected TimeValue getMinRefreshIntervalForRefreshDisabled() { public void testInvalidRefreshInterval() { String invalidRefreshInterval = "-10s"; - internalCluster().startDataOnlyNodes(2); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); Settings settings = Settings.builder() .put(indexSettings()) .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), invalidRefreshInterval) @@ -251,7 +285,7 @@ public void testInvalidRefreshInterval() { } public void testCreateIndexWithExplicitNullRefreshInterval() throws ExecutionException, InterruptedException { - List dataNodes = internalCluster().startDataOnlyNodes(2); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); Settings indexSettings = Settings.builder() .put(indexSettings()) .putNull(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()) @@ -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 dataNodes = internalCluster().startDataOnlyNodes(2); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); TimeValue customRefreshInterval = TimeValue.timeValueSeconds(getDefaultRefreshInterval().getSeconds() + randomIntBetween(1, 5)); Settings indexSettings = Settings.builder() .put(indexSettings()) diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalWithNodeSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalWithNodeSettingsIT.java index 5fc7bfcbcd442..2817d0e6a5951 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalWithNodeSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalWithNodeSettingsIT.java @@ -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 { @@ -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); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index 8d76a39712ee3..2704dccd46076 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -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; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index 1093ac09777e7..5b03d3f7b19ce 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -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; /** @@ -1529,6 +1530,9 @@ private void validate(String name, @Nullable Settings settings, List ind Optional.empty() ); validationErrors.addAll(indexSettingsValidation); + + // validate index refresh interval settings + validateRefreshIntervalSettings(settings, clusterService.getClusterSettings()); } if (indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) { diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 955ec71f30360..9d2c7eb882fa1 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -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); @@ -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); } }