From 1f7caf2652cd4e1595a1b824a1591ab2eaeba8ae Mon Sep 17 00:00:00 2001 From: Varun Bansal Date: Fri, 3 May 2024 14:11:55 +0530 Subject: [PATCH] Support wildcard/regex for indices param in _remotestore/_restore (#8922) * Support wildcard/regex for indices param in _remotestore/_restore Signed-off-by: bansvaru --- CHANGELOG.md | 1 + .../remotestore/RemoteStoreRestoreIT.java | 21 ++- .../opensearch/common/util/IndexUtils.java | 128 ++++++++++++++++++ .../recovery/RemoteStoreRestoreService.java | 9 +- .../opensearch/snapshots/RestoreService.java | 2 +- .../opensearch/snapshots/SnapshotUtils.java | 105 -------------- .../snapshots/SnapshotsService.java | 7 +- .../snapshots/SnapshotUtilsTests.java | 3 +- 8 files changed, 152 insertions(+), 124 deletions(-) create mode 100644 server/src/main/java/org/opensearch/common/util/IndexUtils.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f2b92c76689f..9f013cedd82dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Tiered Caching] Expose new cache stats API ([#13237](https://github.com/opensearch-project/OpenSearch/pull/13237)) - [Streaming Indexing] Ensure support of the new transport by security plugin ([#13174](https://github.com/opensearch-project/OpenSearch/pull/13174)) - Add cluster setting to dynamically configure the buckets for filter rewrite optimization. ([#13179](https://github.com/opensearch-project/OpenSearch/pull/13179)) +- Support wildcard/regex for indices param in _remotestore/_restore ([#8922](https://github.com/opensearch-project/OpenSearch/pull/8922)) - [Tiered Caching] Gate new stats logic behind FeatureFlags.PLUGGABLE_CACHE ([#13238](https://github.com/opensearch-project/OpenSearch/pull/13238)) - [Tiered Caching] Add a dynamic setting to disable/enable disk cache. ([#13373](https://github.com/opensearch-project/OpenSearch/pull/13373)) - [Remote Store] Add capability of doing refresh as determined by the translog ([#12992](https://github.com/opensearch-project/OpenSearch/pull/12992)) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRestoreIT.java index 94acf2b1dbb27..195b2d18df7cc 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRestoreIT.java @@ -42,7 +42,7 @@ import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; import static org.hamcrest.Matchers.greaterThan; -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 0) +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class RemoteStoreRestoreIT extends BaseRemoteStoreRestoreIT { /** @@ -295,7 +295,6 @@ public void testRestoreFlowNoRedIndex() throws Exception { * for multiple indices matching a wildcard name pattern. * @throws IOException IO Exception. */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8480") public void testRTSRestoreWithCommittedDataMultipleIndicesPatterns() throws Exception { testRestoreFlowMultipleIndices(2, true, randomIntBetween(1, 5)); } @@ -306,16 +305,16 @@ public void testRTSRestoreWithCommittedDataMultipleIndicesPatterns() throws Exce * with all remote-enabled red indices considered for the restore by default. * @throws IOException IO Exception. */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8480") public void testRTSRestoreWithCommittedDataDefaultAllIndices() throws Exception { int shardCount = randomIntBetween(1, 5); - prepareCluster(1, 3, INDEX_NAMES, 1, shardCount); + int replicaCount = 1; + prepareCluster(1, 3, INDEX_NAMES, replicaCount, shardCount); String[] indices = INDEX_NAMES.split(","); Map> indicesStats = new HashMap<>(); for (String index : indices) { Map indexStats = indexData(2, true, index); indicesStats.put(index, indexStats); - assertEquals(shardCount, getNumShards(index).totalNumShards); + assertEquals(shardCount * (replicaCount + 1), getNumShards(index).totalNumShards); } for (String index : indices) { @@ -337,7 +336,7 @@ public void testRTSRestoreWithCommittedDataDefaultAllIndices() throws Exception ensureGreen(indices); for (String index : indices) { - assertEquals(shardCount, getNumShards(index).totalNumShards); + assertEquals(shardCount * (replicaCount + 1), getNumShards(index).totalNumShards); verifyRestoredData(indicesStats.get(index), index); } } @@ -395,16 +394,16 @@ public void testRTSRestoreWithCommittedDataNotAllRedRemoteIndices() throws Excep * except those matching the specified exclusion pattern. * @throws IOException IO Exception. */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8480") public void testRTSRestoreWithCommittedDataExcludeIndicesPatterns() throws Exception { int shardCount = randomIntBetween(1, 5); - prepareCluster(1, 3, INDEX_NAMES, 1, shardCount); + int replicaCount = 1; + prepareCluster(1, 3, INDEX_NAMES, replicaCount, shardCount); String[] indices = INDEX_NAMES.split(","); Map> indicesStats = new HashMap<>(); for (String index : indices) { Map indexStats = indexData(2, true, index); indicesStats.put(index, indexStats); - assertEquals(shardCount, getNumShards(index).totalNumShards); + assertEquals(shardCount * (replicaCount + 1), getNumShards(index).totalNumShards); } for (String index : indices) { @@ -433,9 +432,9 @@ public void testRTSRestoreWithCommittedDataExcludeIndicesPatterns() throws Excep PlainActionFuture.newFuture() ); ensureGreen(indices[0], indices[1]); - assertEquals(shardCount, getNumShards(indices[0]).totalNumShards); + assertEquals(shardCount * (replicaCount + 1), getNumShards(indices[0]).totalNumShards); verifyRestoredData(indicesStats.get(indices[0]), indices[0]); - assertEquals(shardCount, getNumShards(indices[1]).totalNumShards); + assertEquals(shardCount * (replicaCount + 1), getNumShards(indices[1]).totalNumShards); verifyRestoredData(indicesStats.get(indices[1]), indices[1]); ensureRed(indices[2], indices[3]); } diff --git a/server/src/main/java/org/opensearch/common/util/IndexUtils.java b/server/src/main/java/org/opensearch/common/util/IndexUtils.java new file mode 100644 index 0000000000000..b4f049617c4fc --- /dev/null +++ b/server/src/main/java/org/opensearch/common/util/IndexUtils.java @@ -0,0 +1,128 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.util; + +import org.opensearch.action.support.IndicesOptions; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.common.regex.Regex; +import org.opensearch.index.IndexNotFoundException; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Common Utility methods for Indices. + * + * @opensearch.internal + */ +public class IndexUtils { + + /** + * Filters out list of available indices based on the list of selected indices. + * + * @param availableIndices list of available indices + * @param selectedIndices list of selected indices + * @param indicesOptions ignore indices flag + * @return filtered out indices + */ + public static List filterIndices(List availableIndices, String[] selectedIndices, IndicesOptions indicesOptions) { + if (IndexNameExpressionResolver.isAllIndices(Arrays.asList(selectedIndices))) { + return availableIndices; + } + + // Move the exclusions to end of list to ensure they are processed + // after explicitly selected indices are chosen. + final List excludesAtEndSelectedIndices = Stream.concat( + Arrays.stream(selectedIndices).filter(s -> s.isEmpty() || s.charAt(0) != '-'), + Arrays.stream(selectedIndices).filter(s -> !s.isEmpty() && s.charAt(0) == '-') + ).collect(Collectors.toUnmodifiableList()); + + Set result = null; + for (int i = 0; i < excludesAtEndSelectedIndices.size(); i++) { + String indexOrPattern = excludesAtEndSelectedIndices.get(i); + boolean add = true; + if (!indexOrPattern.isEmpty()) { + if (availableIndices.contains(indexOrPattern)) { + if (result == null) { + result = new HashSet<>(); + } + result.add(indexOrPattern); + continue; + } + if (indexOrPattern.charAt(0) == '+') { + add = true; + indexOrPattern = indexOrPattern.substring(1); + // if its the first, add empty set + if (i == 0) { + result = new HashSet<>(); + } + } else if (indexOrPattern.charAt(0) == '-') { + // If the first index pattern is an exclusion, then all patterns are exclusions due to the + // reordering logic above. In this case, the request is interpreted as "include all indexes except + // those matching the exclusions" so we add all indices here and then remove the ones that match the exclusion patterns. + if (i == 0) { + result = new HashSet<>(availableIndices); + } + add = false; + indexOrPattern = indexOrPattern.substring(1); + } + } + if (indexOrPattern.isEmpty() || !Regex.isSimpleMatchPattern(indexOrPattern)) { + if (!availableIndices.contains(indexOrPattern)) { + if (!indicesOptions.ignoreUnavailable()) { + throw new IndexNotFoundException(indexOrPattern); + } else { + if (result == null) { + // add all the previous ones... + result = new HashSet<>(availableIndices.subList(0, i)); + } + } + } else { + if (result != null) { + if (add) { + result.add(indexOrPattern); + } else { + result.remove(indexOrPattern); + } + } + } + continue; + } + if (result == null) { + // add all the previous ones... + result = new HashSet<>(availableIndices.subList(0, i)); + } + boolean found = false; + for (String index : availableIndices) { + if (Regex.simpleMatch(indexOrPattern, index)) { + found = true; + if (add) { + result.add(index); + } else { + result.remove(index); + } + } + } + if (!found && !indicesOptions.allowNoIndices()) { + throw new IndexNotFoundException(indexOrPattern); + } + } + if (result == null) { + return Collections.unmodifiableList(new ArrayList<>(Arrays.asList(selectedIndices))); + } + return Collections.unmodifiableList(new ArrayList<>(result)); + } + +} diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java index 23bb4cea17a20..fe90f24b0f544 100644 --- a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java +++ b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.block.ClusterBlocks; @@ -48,6 +49,7 @@ import java.util.stream.Collectors; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; +import static org.opensearch.common.util.IndexUtils.filterIndices; import static org.opensearch.repositories.blobstore.BlobStoreRepository.SYSTEM_REPOSITORY_SETTING; /** @@ -158,7 +160,12 @@ public RemoteRestoreResult restore( throw new IllegalStateException("Unable to restore remote index metadata", e); } } else { - for (String indexName : indexNames) { + List filteredIndices = filterIndices( + List.of(currentState.metadata().getConcreteAllIndices()), + indexNames, + IndicesOptions.fromOptions(true, true, true, true) + ); + for (String indexName : filteredIndices) { IndexMetadata indexMetadata = currentState.metadata().index(indexName); if (indexMetadata == null) { logger.warn("Index restore is not supported for non-existent index. Skipping: {}", indexName); diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index e6a6b747c2baf..5883a8a37be71 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -125,12 +125,12 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; +import static org.opensearch.common.util.IndexUtils.filterIndices; import static org.opensearch.common.util.set.Sets.newHashSet; import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; import static org.opensearch.index.store.remote.filecache.FileCache.DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING; import static org.opensearch.node.Node.NODE_SEARCH_CACHE_SIZE_SETTING; -import static org.opensearch.snapshots.SnapshotUtils.filterIndices; /** * Service responsible for restoring snapshots diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index e7338a29cafeb..8cce5e1c98254 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -31,24 +31,15 @@ package org.opensearch.snapshots; -import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.metadata.IndexNameExpressionResolver; -import org.opensearch.common.regex.Regex; import org.opensearch.index.IndexModule; -import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexSettings; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Snapshot utilities @@ -57,102 +48,6 @@ */ public class SnapshotUtils { - /** - * Filters out list of available indices based on the list of selected indices. - * - * @param availableIndices list of available indices - * @param selectedIndices list of selected indices - * @param indicesOptions ignore indices flag - * @return filtered out indices - */ - public static List filterIndices(List availableIndices, String[] selectedIndices, IndicesOptions indicesOptions) { - if (IndexNameExpressionResolver.isAllIndices(Arrays.asList(selectedIndices))) { - return availableIndices; - } - - // Move the exclusions to end of list to ensure they are processed - // after explicitly selected indices are chosen. - final List excludesAtEndSelectedIndices = Stream.concat( - Arrays.stream(selectedIndices).filter(s -> s.isEmpty() || s.charAt(0) != '-'), - Arrays.stream(selectedIndices).filter(s -> !s.isEmpty() && s.charAt(0) == '-') - ).collect(Collectors.toUnmodifiableList()); - - Set result = null; - for (int i = 0; i < excludesAtEndSelectedIndices.size(); i++) { - String indexOrPattern = excludesAtEndSelectedIndices.get(i); - boolean add = true; - if (!indexOrPattern.isEmpty()) { - if (availableIndices.contains(indexOrPattern)) { - if (result == null) { - result = new HashSet<>(); - } - result.add(indexOrPattern); - continue; - } - if (indexOrPattern.charAt(0) == '+') { - add = true; - indexOrPattern = indexOrPattern.substring(1); - // if its the first, add empty set - if (i == 0) { - result = new HashSet<>(); - } - } else if (indexOrPattern.charAt(0) == '-') { - // If the first index pattern is an exclusion, then all patterns are exclusions due to the - // reordering logic above. In this case, the request is interpreted as "include all indexes except - // those matching the exclusions" so we add all indices here and then remove the ones that match the exclusion patterns. - if (i == 0) { - result = new HashSet<>(availableIndices); - } - add = false; - indexOrPattern = indexOrPattern.substring(1); - } - } - if (indexOrPattern.isEmpty() || !Regex.isSimpleMatchPattern(indexOrPattern)) { - if (!availableIndices.contains(indexOrPattern)) { - if (!indicesOptions.ignoreUnavailable()) { - throw new IndexNotFoundException(indexOrPattern); - } else { - if (result == null) { - // add all the previous ones... - result = new HashSet<>(availableIndices.subList(0, i)); - } - } - } else { - if (result != null) { - if (add) { - result.add(indexOrPattern); - } else { - result.remove(indexOrPattern); - } - } - } - continue; - } - if (result == null) { - // add all the previous ones... - result = new HashSet<>(availableIndices.subList(0, i)); - } - boolean found = false; - for (String index : availableIndices) { - if (Regex.simpleMatch(indexOrPattern, index)) { - found = true; - if (add) { - result.add(index); - } else { - result.remove(index); - } - } - } - if (!found && !indicesOptions.allowNoIndices()) { - throw new IndexNotFoundException(indexOrPattern); - } - } - if (result == null) { - return Collections.unmodifiableList(new ArrayList<>(Arrays.asList(selectedIndices))); - } - return Collections.unmodifiableList(new ArrayList<>(result)); - } - /** * Validates if there are any remote snapshots backing an index * @param metadata index metadata from cluster state diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java index 22b640963e896..acc2dc83749cd 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java @@ -131,6 +131,7 @@ import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableList; import static org.opensearch.cluster.SnapshotsInProgress.completed; +import static org.opensearch.common.util.IndexUtils.filterIndices; 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; @@ -475,11 +476,7 @@ public ClusterState execute(ClusterState currentState) { indicesForSnapshot.add(indexId.getName()); } } - final List matchingIndices = SnapshotUtils.filterIndices( - indicesForSnapshot, - request.indices(), - request.indicesOptions() - ); + final List matchingIndices = filterIndices(indicesForSnapshot, request.indices(), request.indicesOptions()); if (matchingIndices.isEmpty()) { throw new SnapshotException( new Snapshot(repositoryName, sourceSnapshotId), diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java index 14e711e03a345..871f2cdd88ea1 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java @@ -46,6 +46,7 @@ import java.util.Map; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; +import static org.opensearch.common.util.IndexUtils.filterIndices; import static org.hamcrest.Matchers.containsInAnyOrder; public class SnapshotUtilsTests extends OpenSearchTestCase { @@ -94,7 +95,7 @@ private void assertIndexNameFiltering(String[] indices, String[] filter, String[ private void assertIndexNameFiltering(String[] indices, String[] filter, IndicesOptions indicesOptions, String[] expected) { List indicesList = Arrays.asList(indices); - List actual = SnapshotUtils.filterIndices(indicesList, filter, indicesOptions); + List actual = filterIndices(indicesList, filter, indicesOptions); assertThat(actual, containsInAnyOrder(expected)); }