From 4c50ad5a305a68652bc34fafa56ef7c09b39bf65 Mon Sep 17 00:00:00 2001 From: kkewwei Date: Tue, 5 Nov 2024 05:23:32 +0800 Subject: [PATCH] fix rollover alias supports restored searchable snapshot index (#16483) Signed-off-by: kkewwei Signed-off-by: kkewwei (cherry picked from commit 4c35a2b418aa4cf14b8f49d7639d386b96752ae8) --- CHANGELOG.md | 1 + .../alias/TransportIndicesAliasesAction.java | 4 +- .../rollover/TransportRolloverAction.java | 13 +- .../put/TransportUpdateSettingsAction.java | 7 +- .../cluster/block/ClusterBlocks.java | 26 ++- .../cluster/block/ClusterBlocksTests.java | 177 ++++++++++++++++++ 6 files changed, 215 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d52f03637a4..1d5e82b2e14a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Ensure index templates are not applied to system indices ([#16418](https://github.com/opensearch-project/OpenSearch/pull/16418)) - Remove resource usages object from search response headers ([#16532](https://github.com/opensearch-project/OpenSearch/pull/16532)) - Support retrieving doc values of unsigned long field ([#16543](https://github.com/opensearch-project/OpenSearch/pull/16543)) +- Fix rollover alias supports restored searchable snapshot index([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483)) ### Security diff --git a/server/src/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index 81cb3102cfcb9..42e02e9e1aff8 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -41,7 +41,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ack.ClusterStateUpdateResponse; import org.opensearch.cluster.block.ClusterBlockException; -import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.block.ClusterBlocks; import org.opensearch.cluster.metadata.AliasAction; import org.opensearch.cluster.metadata.AliasMetadata; import org.opensearch.cluster.metadata.IndexAbstraction; @@ -123,7 +123,7 @@ protected ClusterBlockException checkBlock(IndicesAliasesRequest request, Cluste for (IndicesAliasesRequest.AliasActions aliasAction : request.aliasActions()) { Collections.addAll(indices, aliasAction.indices()); } - return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indices.toArray(new String[0])); + return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, state); } @Override diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java index 3b11a3d82d707..28d1d14655e3b 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -44,7 +44,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.block.ClusterBlockException; -import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.block.ClusterBlocks; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.Metadata; @@ -62,8 +62,10 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -127,11 +129,10 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState request.indicesOptions().expandWildcardsClosed() ); - return state.blocks() - .indicesBlockedException( - ClusterBlockLevel.METADATA_WRITE, - indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request) - ); + return ClusterBlocks.indicesWithRemoteSnapshotBlockedException( + new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))), + state + ); } @Override diff --git a/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java index 09cceca52ce23..779b136abef5c 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java @@ -42,7 +42,7 @@ import org.opensearch.cluster.ack.ClusterStateUpdateResponse; import org.opensearch.cluster.block.ClusterBlockException; import org.opensearch.cluster.block.ClusterBlockLevel; -import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.block.ClusterBlocks; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.MetadataUpdateSettingsService; import org.opensearch.cluster.service.ClusterService; @@ -118,9 +118,8 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste return globalBlock; } if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock an index - IndexMetadata.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings()) - || IndexMetadata.INDEX_READ_ONLY_SETTING.exists(request.settings()) - || IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) { + ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream() + .anyMatch(booleanSetting -> booleanSetting.exists(request.settings()))) { return null; } diff --git a/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java b/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java index c2c9e2e2bd0d2..aeba57718e6f3 100644 --- a/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java +++ b/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java @@ -33,18 +33,22 @@ package org.opensearch.cluster.block; import org.opensearch.cluster.AbstractDiffable; +import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.Diff; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.MetadataIndexStateService; import org.opensearch.common.Nullable; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.settings.Setting; import org.opensearch.common.util.set.Sets; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.IndexModule; import org.opensearch.index.translog.BufferedChecksumStreamOutput; import java.io.IOException; +import java.util.Collection; import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; @@ -56,6 +60,7 @@ import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.toSet; +import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; /** * Represents current cluster level blocks to block dirty operations done against the cluster. @@ -65,7 +70,11 @@ @PublicApi(since = "1.0.0") public class ClusterBlocks extends AbstractDiffable { public static final ClusterBlocks EMPTY_CLUSTER_BLOCK = new ClusterBlocks(emptySet(), Map.of()); - + public static final Set> INDEX_DATA_READ_ONLY_BLOCK_SETTINGS = Set.of( + IndexMetadata.INDEX_READ_ONLY_SETTING, + IndexMetadata.INDEX_BLOCKS_METADATA_SETTING, + IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING + ); private final Set global; private final Map> indicesBlocks; @@ -275,6 +284,21 @@ public ClusterBlockException indicesAllowReleaseResources(String[] indices) { return new ClusterBlockException(indexLevelBlocks); } + public static ClusterBlockException indicesWithRemoteSnapshotBlockedException(Collection concreteIndices, ClusterState state) { + for (String index : concreteIndices) { + if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index)) { + IndexMetadata indexMeta = state.metadata().index(index); + if (indexMeta != null + && (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMeta.getSettings().get(INDEX_STORE_TYPE_SETTING.getKey())) == false + || ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream() + .anyMatch(booleanSetting -> booleanSetting.exists(indexMeta.getSettings())))) { + return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, concreteIndices.toArray(new String[0])); + } + } + } + return null; + } + @Override public String toString() { if (global.isEmpty() && indices().isEmpty()) { diff --git a/server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java b/server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java index e17cc7c24ffbd..668f3c8ac9a84 100644 --- a/server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java +++ b/server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java @@ -8,12 +8,40 @@ package org.opensearch.cluster.block; +import com.carrotsearch.randomizedtesting.RandomizedTest; + +import org.opensearch.Version; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.AliasMetadata; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.index.IndexModule; +import org.opensearch.index.IndexSettings; import org.opensearch.index.translog.BufferedChecksumStreamOutput; import org.opensearch.test.OpenSearchTestCase; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + import static org.opensearch.cluster.block.ClusterBlockTests.randomClusterBlock; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_METADATA_BLOCK; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_BLOCK; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_WRITE_BLOCK; +import static org.opensearch.cluster.metadata.IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; public class ClusterBlocksTests extends OpenSearchTestCase { @@ -52,4 +80,153 @@ public void testWriteVerifiableTo() throws Exception { clusterBlocks2.writeVerifiableTo(checksumOut2); assertEquals(checksumOut.getChecksum(), checksumOut2.getChecksum()); } + + public void testGlobalBlock() { + String index = "test-000001"; + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + Set indices = new HashSet<>(); + indices.add(index); + + // no global blocks + { + stateBuilder.blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState)); + } + + // has global block + { + for (ClusterBlock block : Arrays.asList( + INDEX_READ_ONLY_BLOCK, + INDEX_READ_BLOCK, + INDEX_WRITE_BLOCK, + INDEX_METADATA_BLOCK, + INDEX_READ_ONLY_ALLOW_DELETE_BLOCK, + REMOTE_READ_ONLY_ALLOW_DELETE + )) { + stateBuilder.blocks(ClusterBlocks.builder().addGlobalBlock(block).build()); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState)); + } + } + } + + public void testIndexWithBlock() { + String index = "test-000001"; + Set indices = new HashSet<>(); + indices.add(index); + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + stateBuilder.blocks(ClusterBlocks.builder().addIndexBlock(index, IndexMetadata.INDEX_METADATA_BLOCK)); + stateBuilder.metadata(Metadata.builder().put(createIndexMetadata(index, false, null, null), false)); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, stateBuilder.build())); + } + + public void testRemoteIndexBlock() { + String remoteIndex = "remote_index"; + Set indices = new HashSet<>(); + indices.add(remoteIndex); + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + + { + IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, null); + stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false)); + stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata)); + + ClusterState clusterState = stateBuilder.build(); + assertTrue(clusterState.blocks().hasIndexBlock(remoteIndex, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE)); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState)); + } + + // searchable snapshot index with block + { + Setting setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS)); + IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, setting); + stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false)); + stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata)); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState)); + } + } + + public void testRemoteIndexWithoutBlock() { + String remoteIndex = "remote_index"; + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + + String alias = "alias1"; + IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null); + String index = "test-000001"; + IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null); + stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false)); + + Set indices = new HashSet<>(); + indices.add(remoteIndex); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState)); + } + + public void testRemoteIndexWithIndexBlock() { + String index = "test-000001"; + String remoteIndex = "remote_index"; + String alias = "alias1"; + { + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null); + IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null); + stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false)) + .blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata)); + ClusterState clusterState = stateBuilder.build(); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState)); + clusterState.blocks(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState)); + } + + { + ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster")); + Setting setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS)); + IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, setting); + IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null); + stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false)) + .blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata)); + ClusterState clusterState = stateBuilder.build(); + assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState)); + assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState)); + } + } + + private IndexMetadata createIndexMetadata(String index, boolean isRemoteIndex, String alias, Setting blockSetting) { + IndexMetadata.Builder builder = IndexMetadata.builder(index).settings(createIndexSettingBuilder(isRemoteIndex, blockSetting)); + if (alias != null) { + AliasMetadata.Builder aliasBuilder = AliasMetadata.builder(alias); + return builder.putAlias(aliasBuilder.build()).build(); + } + return builder.build(); + } + + private Settings.Builder createIndexSettingBuilder(boolean isRemoteIndex, Setting blockSetting) { + Settings.Builder builder = Settings.builder() + .put(IndexMetadata.SETTING_INDEX_UUID, "abc") + .put(SETTING_VERSION_CREATED, Version.CURRENT) + .put(SETTING_CREATION_DATE, System.currentTimeMillis()) + .put(SETTING_NUMBER_OF_SHARDS, 1) + .put(SETTING_NUMBER_OF_REPLICAS, 1); + + if (isRemoteIndex) { + builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), "repo") + .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), "snapshot"); + } + if (blockSetting != null) { + builder.put(blockSetting.getKey(), true); + } + + return builder; + } }