From 1dd08bcf2e559e99afa82d8f16a9778aaf365cfa Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 18:15:33 +0530 Subject: [PATCH 1/2] Take shallow copy snapshot when remote store migration is not in progress (#13309) (#13382) (cherry picked from commit 5bf522cce3dded6ecf6167ce53f1f56e3771edd5) Signed-off-by: Lakshya Taragi Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../RemoteStoreMigrationSettingsUpdateIT.java | 81 ++-------------- ...eMigrationShardAllocationBaseTestCase.java | 96 +++++++++++++++++++ .../RemoteStoreMigrationTestCase.java | 51 ++++++++++ .../snapshots/SnapshotsService.java | 9 ++ 4 files changed, 165 insertions(+), 72 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java index c3720e6fbbd09..b71f7d7cf7e4a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java @@ -8,27 +8,15 @@ package org.opensearch.remotemigration; -import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.client.Client; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsException; -import org.opensearch.core.rest.RestStatus; -import org.opensearch.index.IndexSettings; -import org.opensearch.indices.replication.common.ReplicationType; -import org.opensearch.snapshots.SnapshotInfo; -import org.opensearch.snapshots.SnapshotState; import org.opensearch.test.InternalTestCluster; import org.opensearch.test.OpenSearchIntegTestCase; import java.nio.file.Path; import java.util.Optional; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED; import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT; import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE; @@ -92,13 +80,7 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix assertNodeInCluster(remoteNodeName); logger.info("Create a non remote-backed index"); - client.admin() - .indices() - .prepareCreate(TEST_INDEX) - .setSettings( - Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).build() - ) - .get(); + createIndex(TEST_INDEX, 0); logger.info("Verify that non remote stored backed index is created"); assertNonRemoteStoreBackedIndex(TEST_INDEX); @@ -115,21 +97,12 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix logger.info("Create snapshot of non remote stored backed index"); - SnapshotInfo snapshotInfo = client().admin() - .cluster() - .prepareCreateSnapshot(snapshotRepoName, snapshotName) - .setIndices(TEST_INDEX) - .setWaitForCompletion(true) - .get() - .getSnapshotInfo(); - - assertEquals(SnapshotState.SUCCESS, snapshotInfo.state()); - assertTrue(snapshotInfo.successfulShards() > 0); - assertEquals(0, snapshotInfo.failedShards()); + createSnapshot(snapshotRepoName, snapshotName, TEST_INDEX); logger.info("Restore index from snapshot under NONE direction"); String restoredIndexName1 = TEST_INDEX + "-restored1"; restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName1); + ensureGreen(restoredIndexName1); logger.info("Verify that restored index is non remote-backed"); assertNonRemoteStoreBackedIndex(restoredIndexName1); @@ -138,6 +111,7 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix setDirection(REMOTE_STORE.direction); String restoredIndexName2 = TEST_INDEX + "-restored2"; restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName2); + ensureGreen(restoredIndexName2); logger.info("Verify that restored index is non remote-backed"); assertRemoteStoreBackedIndex(restoredIndexName2); @@ -146,10 +120,10 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix // compatibility mode setting test public void testSwitchToStrictMode() throws Exception { - logger.info(" --> initialize cluster"); + logger.info("Initialize cluster"); initializeCluster(false); - logger.info(" --> create a mixed mode cluster"); + logger.info("Create a mixed mode cluster"); setClusterMode(MIXED.mode); addRemote = true; String remoteNodeName = internalCluster().startNode(); @@ -159,58 +133,21 @@ public void testSwitchToStrictMode() throws Exception { assertNodeInCluster(remoteNodeName); assertNodeInCluster(nonRemoteNodeName); - logger.info(" --> attempt switching to strict mode"); + logger.info("Attempt switching to strict mode"); SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode)); assertEquals( "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes", exception.getMessage() ); - logger.info(" --> stop remote node so that cluster had only non-remote nodes"); + logger.info("Stop remote node so that cluster had only non-remote nodes"); internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName)); ensureStableCluster(2); - logger.info(" --> attempt switching to strict mode"); + logger.info("Attempt switching to strict mode"); setClusterMode(STRICT.mode); } - // restore indices from a snapshot - private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) { - RestoreSnapshotResponse restoreSnapshotResponse = client.admin() - .cluster() - .prepareRestoreSnapshot(snapshotRepoName, snapshotName) - .setWaitForCompletion(false) - .setIndices(TEST_INDEX) - .setRenamePattern(TEST_INDEX) - .setRenameReplacement(restoredIndexName) - .get(); - - assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); - ensureGreen(restoredIndexName); - } - - // verify that the created index is not remote store backed - private void assertNonRemoteStoreBackedIndex(String indexName) { - Settings indexSettings = client.admin().indices().prepareGetIndex().execute().actionGet().getSettings().get(indexName); - assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); - assertNull(indexSettings.get(SETTING_REMOTE_STORE_ENABLED)); - assertNull(indexSettings.get(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)); - assertNull(indexSettings.get(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)); - } - - // verify that the created index is remote store backed - private void assertRemoteStoreBackedIndex(String indexName) { - Settings indexSettings = client.admin().indices().prepareGetIndex().execute().actionGet().getSettings().get(indexName); - assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); - assertEquals("true", indexSettings.get(SETTING_REMOTE_STORE_ENABLED)); - assertEquals(REPOSITORY_NAME, indexSettings.get(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)); - assertEquals(REPOSITORY_2_NAME, indexSettings.get(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)); - assertEquals( - IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL, - INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(indexSettings) - ); - } - // bootstrap a cluster private void initializeCluster(boolean remoteClusterManager) { addRemote = remoteClusterManager; diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java index ad2302d1ab2e1..ffcab9483485d 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java @@ -9,15 +9,27 @@ package org.opensearch.remotemigration; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.common.settings.Settings; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.IndexSettings; +import org.opensearch.indices.replication.common.ReplicationType; +import org.opensearch.snapshots.SnapshotInfo; +import org.opensearch.snapshots.SnapshotState; import java.util.Map; import java.util.Optional; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @@ -98,4 +110,88 @@ protected ShardRouting getShardRouting(boolean isPrimary) { return (isPrimary ? table.primaryShard() : table.replicaShards().get(0)); } + // create a snapshot + public static SnapshotInfo createSnapshot(String snapshotRepoName, String snapshotName, String... indices) { + SnapshotInfo snapshotInfo = internalCluster().client() + .admin() + .cluster() + .prepareCreateSnapshot(snapshotRepoName, snapshotName) + .setIndices(indices) + .setWaitForCompletion(true) + .get() + .getSnapshotInfo(); + + assertEquals(SnapshotState.SUCCESS, snapshotInfo.state()); + assertTrue(snapshotInfo.successfulShards() > 0); + assertEquals(0, snapshotInfo.failedShards()); + return snapshotInfo; + } + + // create new index + public static void createIndex(String indexName, int replicaCount) { + assertAcked( + internalCluster().client() + .admin() + .indices() + .prepareCreate(indexName) + .setSettings( + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, replicaCount) + .build() + ) + .get() + ); + } + + // restore indices from a snapshot + public static RestoreSnapshotResponse restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) { + RestoreSnapshotResponse restoreSnapshotResponse = internalCluster().client() + .admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepoName, snapshotName) + .setWaitForCompletion(false) + .setIndices(TEST_INDEX) + .setRenamePattern(TEST_INDEX) + .setRenameReplacement(restoredIndexName) + .get(); + assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); + return restoreSnapshotResponse; + } + + // verify that the created index is not remote store backed + public static void assertNonRemoteStoreBackedIndex(String indexName) { + Settings indexSettings = internalCluster().client() + .admin() + .indices() + .prepareGetIndex() + .execute() + .actionGet() + .getSettings() + .get(indexName); + assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); + assertNull(indexSettings.get(SETTING_REMOTE_STORE_ENABLED)); + assertNull(indexSettings.get(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)); + assertNull(indexSettings.get(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)); + } + + // verify that the created index is remote store backed + public static void assertRemoteStoreBackedIndex(String indexName) { + Settings indexSettings = internalCluster().client() + .admin() + .indices() + .prepareGetIndex() + .execute() + .actionGet() + .getSettings() + .get(indexName); + assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); + assertEquals("true", indexSettings.get(SETTING_REMOTE_STORE_ENABLED)); + assertEquals(REPOSITORY_NAME, indexSettings.get(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)); + assertEquals(REPOSITORY_2_NAME, indexSettings.get(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)); + assertEquals( + IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL, + INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(indexSettings) + ); + } } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java index 640b83f194c1c..7d816a5e18698 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java @@ -13,8 +13,11 @@ import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.snapshots.SnapshotInfo; import org.opensearch.test.OpenSearchIntegTestCase; +import java.nio.file.Path; import java.util.List; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; @@ -70,4 +73,52 @@ public void testMigrationDirections() { updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), "random")); assertThrows(IllegalArgumentException.class, () -> client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); } + + public void testNoShallowSnapshotInMixedMode() throws Exception { + logger.info("Initialize remote cluster"); + addRemote = true; + internalCluster().setBootstrapClusterManagerNodeIndex(0); + List cmNodes = internalCluster().startNodes(1); + Client client = internalCluster().client(cmNodes.get(0)); + + logger.info("Add remote node"); + internalCluster().startNode(); + internalCluster().validateClusterFormed(); + + logger.info("Create remote backed index"); + RemoteStoreMigrationShardAllocationBaseTestCase.createIndex("test", 0); + RemoteStoreMigrationShardAllocationBaseTestCase.assertRemoteStoreBackedIndex("test"); + + logger.info("Create shallow snapshot setting enabled repo"); + String shallowSnapshotRepoName = "shallow-snapshot-repo-name"; + Path shallowSnapshotRepoPath = randomRepoPath(); + assertAcked( + clusterAdmin().preparePutRepository(shallowSnapshotRepoName) + .setType("fs") + .setSettings( + Settings.builder() + .put("location", shallowSnapshotRepoPath) + .put(BlobStoreRepository.REMOTE_STORE_INDEX_SHALLOW_COPY.getKey(), Boolean.TRUE) + ) + ); + + logger.info("Verify shallow snapshot creation"); + final String snapshot1 = "snapshot1"; + SnapshotInfo snapshotInfo1 = RemoteStoreMigrationShardAllocationBaseTestCase.createSnapshot( + shallowSnapshotRepoName, + snapshot1, + "test" + ); + assertEquals(snapshotInfo1.isRemoteStoreIndexShallowCopyEnabled(), true); + + logger.info("Set MIXED compatibility mode"); + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); + updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed")); + assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + logger.info("Verify that new snapshot is not shallow"); + final String snapshot2 = "snapshot2"; + SnapshotInfo snapshotInfo2 = RemoteStoreMigrationShardAllocationBaseTestCase.createSnapshot(shallowSnapshotRepoName, snapshot2); + assertEquals(snapshotInfo2.isRemoteStoreIndexShallowCopyEnabled(), false); + } } diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java index b567649367b0c..1ef2aa4d75e10 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java @@ -132,6 +132,8 @@ import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableList; import static org.opensearch.cluster.SnapshotsInProgress.completed; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; import static org.opensearch.repositories.blobstore.BlobStoreRepository.REMOTE_STORE_INDEX_SHALLOW_COPY; import static org.opensearch.snapshots.SnapshotUtils.validateSnapshotsBackingAnyIndex; @@ -521,6 +523,13 @@ public ClusterState execute(ClusterState currentState) { } boolean remoteStoreIndexShallowCopy = REMOTE_STORE_INDEX_SHALLOW_COPY.get(repository.getMetadata().settings()); + logger.debug("remote_store_index_shallow_copy setting is set as [{}]", remoteStoreIndexShallowCopy); + if (remoteStoreIndexShallowCopy + && clusterService.getClusterSettings().get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING).equals(CompatibilityMode.MIXED)) { + // don't allow shallow snapshots if compatibility mode is not strict + logger.warn("Shallow snapshots are not supported during migration. Falling back to full snapshot."); + remoteStoreIndexShallowCopy = false; + } newEntry = SnapshotsInProgress.startedEntry( new Snapshot(repositoryName, snapshotId), request.includeGlobalState(), From 17516ff3571a0f92c61af0fdc23111e76412824c Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 18:19:06 +0530 Subject: [PATCH 2/2] Allow index & cluster default refresh interval setting value to be -1 (#11411) (#13380) (cherry picked from commit 42d46a9d048b8f8ac07d88b71ede32f6c0d7a261) Signed-off-by: Ashish Singh Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../ClusterIndexRefreshIntervalIT.java | 169 +++++++++++++++--- .../metadata/MetadataCreateIndexService.java | 7 +- .../opensearch/indices/IndicesService.java | 7 +- 3 files changed, 159 insertions(+), 24 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 25fa7ae7eb8eb..f936b53f52a7b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java @@ -235,33 +235,19 @@ public void testDefaultRefreshIntervalWithUpdateClusterAndIndexSettings() throws } public void testRefreshIntervalDisabled() throws ExecutionException, InterruptedException { - TimeValue clusterMinimumRefreshInterval = client().settings() - .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 = new ArrayList<>(internalCluster().getDataNodeNames()); Settings settings = Settings.builder() .put(indexSettings()) .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), IndexSettings.MINIMUM_REFRESH_INTERVAL) .build(); - if (createIndexSuccess) { - createIndex(INDEX_NAME, settings); - ensureYellowAndNoInitializingShards(INDEX_NAME); - ensureGreen(INDEX_NAME); - GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get(); - IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes)); - String uuid = getIndexResponse.getSettings().get(INDEX_NAME).get(IndexMetadata.SETTING_INDEX_UUID); - IndexService indexService = indicesService.indexService(new Index(INDEX_NAME, uuid)); - assertEquals(IndexSettings.MINIMUM_REFRESH_INTERVAL, indexService.getRefreshTaskInterval()); - } else { - IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> createIndex(INDEX_NAME, settings)); - assertEquals( - "invalid index.refresh_interval [-1]: cannot be smaller than cluster.minimum.index.refresh_interval [" - + getMinRefreshIntervalForRefreshDisabled() - + "]", - exception.getMessage() - ); - } + createIndex(INDEX_NAME, settings); + ensureGreen(INDEX_NAME); + GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get(); + IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes)); + String uuid = getIndexResponse.getSettings().get(INDEX_NAME).get(IndexMetadata.SETTING_INDEX_UUID); + IndexService indexService = indicesService.indexService(new Index(INDEX_NAME, uuid)); + assertEquals(IndexSettings.MINIMUM_REFRESH_INTERVAL, indexService.getRefreshTaskInterval()); } protected TimeValue getMinRefreshIntervalForRefreshDisabled() { @@ -366,6 +352,147 @@ public void testClusterMinimumChangeOnIndexWithCustomRefreshInterval() throws Ex assertEquals(customRefreshInterval, indexService.getRefreshTaskInterval()); } + public void testClusterMinimumRefreshIntervalOfMinusOneFails() { + // This test checks that we can not set cluster minimum refresh interval as -1 (or -1ms). + String clusterManagerName = internalCluster().getClusterManagerName(); + String refreshInterval = randomFrom("-1", "-1ms"); + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), refreshInterval)) + .get() + ); + assertEquals( + "failed to parse value [" + refreshInterval + "] for setting [cluster.minimum.index.refresh_interval], must be >= [0ms]", + ex.getMessage() + ); + } + + public void testClusterMinimumRefreshIntervalOfZero() { + // This test checks that we can set the cluster minimum refresh interval as 0. + String clusterManagerName = internalCluster().getClusterManagerName(); + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "0")) + .get(); + } + + public void testDefaultRefreshIntervalOfMinusOneIrrespectiveOfMinimum() { + // This test checks that we are able to set the cluster default refresh interval to one regardless of what the + // minimum is set to. -1 corresponds to no period background refreshes. + String clusterManagerName = internalCluster().getClusterManagerName(); + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings( + Settings.builder() + .put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("0", "1ms", "1s", "10s")) + .put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms")) + ) + .get(); + } + + public void testCreateIndexWithMinusOneRefreshInterval() throws ExecutionException, InterruptedException { + // This test checks that we are able to create index with -1 refresh interval using index settings and default interval both. + String clusterManagerName = internalCluster().getClusterManagerName(); + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings( + Settings.builder() + .put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s") + .put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s") + ) + .get(); + + Settings indexSettings = Settings.builder() + .put(indexSettings()) + .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms")) + .build(); + createIndex(INDEX_NAME, indexSettings); + ensureGreen(INDEX_NAME); + + IndexService indexService = getIndexServiceFromRandomDataNode(INDEX_NAME); + assertEquals(-1, indexService.getRefreshTaskInterval().millis()); + + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms"))) + .get(); + createIndex(OTHER_INDEX_NAME); + ensureGreen(OTHER_INDEX_NAME); + indexService = getIndexServiceFromRandomDataNode(OTHER_INDEX_NAME); + assertEquals(-1, indexService.getRefreshTaskInterval().millis()); + } + + public void testUpdateIndexWithMinusOneRefreshInterval() throws ExecutionException, InterruptedException { + // This test checks that we are able to update index with -1 refresh interval using index settings and default interval both. + String clusterManagerName = internalCluster().getClusterManagerName(); + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings( + Settings.builder() + .put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s") + .put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s") + ) + .get(); + + createIndex(INDEX_NAME); + ensureGreen(INDEX_NAME); + IndexService indexService = getIndexServiceFromRandomDataNode(INDEX_NAME); + assertEquals(10, indexService.getRefreshTaskInterval().seconds()); + + client(clusterManagerName).admin() + .indices() + .updateSettings( + new UpdateSettingsRequest(INDEX_NAME).settings( + Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms")) + ) + ) + .actionGet(); + assertEquals(-1, indexService.getRefreshTaskInterval().millis()); + + client(clusterManagerName).admin() + .indices() + .updateSettings( + new UpdateSettingsRequest(INDEX_NAME).settings( + Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "100s") + ) + ) + .actionGet(); + assertEquals(100, indexService.getRefreshTaskInterval().seconds()); + + client(clusterManagerName).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms"))) + .get(); + + client(clusterManagerName).admin() + .indices() + .updateSettings( + new UpdateSettingsRequest(INDEX_NAME).settings( + Settings.builder().putNull(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()) + ) + ) + .actionGet(); + assertEquals(-1, indexService.getRefreshTaskInterval().millis()); + } + + private IndexService getIndexServiceFromRandomDataNode(String indexName) throws ExecutionException, InterruptedException { + String clusterManagerName = internalCluster().getClusterManagerName(); + List dataNodes = new ArrayList<>(internalCluster().getDataNodeNames()); + 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); + return indicesService.indexService(new Index(indexName, uuid)); + } + protected TimeValue getDefaultRefreshInterval() { return IndexSettings.DEFAULT_REFRESH_INTERVAL; } 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 a79f7851b801e..1844a0c3b132d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1614,10 +1614,15 @@ public static void validateTranslogRetentionSettings(Settings indexSettings) { * @param clusterSettings cluster setting */ public static void validateRefreshIntervalSettings(Settings requestSettings, ClusterSettings clusterSettings) { - if (IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.exists(requestSettings) == false) { + if (IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.exists(requestSettings) == false + || requestSettings.get(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()) == null) { return; } TimeValue requestRefreshInterval = IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.get(requestSettings); + // If the refresh interval supplied is -1, we allow the index to be created because -1 means no periodic refresh. + if (requestRefreshInterval.millis() == -1) { + return; + } TimeValue clusterMinimumRefreshInterval = clusterSettings.get(IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING); if (requestRefreshInterval.millis() < clusterMinimumRefreshInterval.millis()) { throw new IllegalArgumentException( diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index ae2df1e0668bb..9595a943deb28 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -275,8 +275,8 @@ public class IndicesService extends AbstractLifecycleComponent */ public static final Setting CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING = Setting.timeSetting( "cluster.minimum.index.refresh_interval", - IndexSettings.MINIMUM_REFRESH_INTERVAL, - IndexSettings.MINIMUM_REFRESH_INTERVAL, + TimeValue.ZERO, + TimeValue.ZERO, new ClusterMinimumRefreshIntervalValidator(), Property.NodeScope, Property.Dynamic @@ -2008,6 +2008,9 @@ public Iterator> settings() { * @param defaultRefreshInterval value of cluster default index refresh interval setting */ private static void validateRefreshIntervalSettings(TimeValue minimumRefreshInterval, TimeValue defaultRefreshInterval) { + if (defaultRefreshInterval.millis() < 0) { + return; + } if (minimumRefreshInterval.compareTo(defaultRefreshInterval) > 0) { throw new IllegalArgumentException( "cluster minimum index refresh interval ["