From 302bee649b26d8af8b66e090eaa4dc60486fe2c2 Mon Sep 17 00:00:00 2001 From: Harish Bhakuni Date: Tue, 11 Jul 2023 00:45:22 -0700 Subject: [PATCH] [Snapshot Interop] Keep API parameters behind remote store experimental flag. (#8594) Signed-off-by: Harish Bhakuni Signed-off-by: Shivansh Arora --- .../restore/RestoreSnapshotRequest.java | 69 +++++++++++++------ .../repositories/RepositoriesService.java | 7 ++ .../opensearch/snapshots/RestoreService.java | 6 ++ 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 9ab66d726854e..cddb338310c4f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -41,6 +41,7 @@ import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; @@ -150,7 +151,7 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_7_0)) { storageType = in.readEnum(StorageType.class); } - if (in.getVersion().onOrAfter(Version.V_2_9_0)) { + if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && in.getVersion().onOrAfter(Version.V_2_9_0)) { sourceRemoteStoreRepository = in.readOptionalString(); } } @@ -174,7 +175,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeEnum(storageType); } - if (out.getVersion().onOrAfter(Version.V_2_9_0)) { + if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && out.getVersion().onOrAfter(Version.V_2_9_0)) { out.writeOptionalString(sourceRemoteStoreRepository); } } @@ -614,6 +615,11 @@ public RestoreSnapshotRequest source(Map source) { } } else if (name.equals("source_remote_store_repository")) { + if (!FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { + throw new IllegalArgumentException( + "Unsupported parameter " + name + ". Please enable remote store feature flag for this experimental feature" + ); + } if (entry.getValue() instanceof String) { setSourceRemoteStoreRepository((String) entry.getValue()); } else { @@ -664,7 +670,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (storageType != null) { storageType.toXContent(builder); } - if (sourceRemoteStoreRepository != null) { + if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && sourceRemoteStoreRepository != null) { builder.field("source_remote_store_repository", sourceRemoteStoreRepository); } builder.endObject(); @@ -681,7 +687,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; RestoreSnapshotRequest that = (RestoreSnapshotRequest) o; - return waitForCompletion == that.waitForCompletion + boolean equals = waitForCompletion == that.waitForCompletion && includeGlobalState == that.includeGlobalState && partial == that.partial && includeAliases == that.includeAliases @@ -694,27 +700,48 @@ public boolean equals(Object o) { && Objects.equals(indexSettings, that.indexSettings) && Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) && Objects.equals(snapshotUuid, that.snapshotUuid) - && Objects.equals(storageType, that.storageType) - && Objects.equals(sourceRemoteStoreRepository, that.sourceRemoteStoreRepository); + && Objects.equals(storageType, that.storageType); + if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { + equals = Objects.equals(sourceRemoteStoreRepository, that.sourceRemoteStoreRepository); + } + return equals; } @Override public int hashCode() { - int result = Objects.hash( - snapshot, - repository, - indicesOptions, - renamePattern, - renameReplacement, - waitForCompletion, - includeGlobalState, - partial, - includeAliases, - indexSettings, - snapshotUuid, - storageType, - sourceRemoteStoreRepository - ); + int result; + if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { + result = Objects.hash( + snapshot, + repository, + indicesOptions, + renamePattern, + renameReplacement, + waitForCompletion, + includeGlobalState, + partial, + includeAliases, + indexSettings, + snapshotUuid, + storageType, + sourceRemoteStoreRepository + ); + } else { + result = Objects.hash( + snapshot, + repository, + indicesOptions, + renamePattern, + renameReplacement, + waitForCompletion, + includeGlobalState, + partial, + includeAliases, + indexSettings, + snapshotUuid, + storageType + ); + } result = 31 * result + Arrays.hashCode(indices); result = 31 * result + Arrays.hashCode(ignoreIndexSettings); return result; diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index 9c56d172f2ea1..e7f7a1d9c0554 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -63,6 +63,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.ConcurrentCollections; import org.opensearch.common.util.io.IOUtils; import org.opensearch.repositories.blobstore.MeteredBlobStoreRepository; @@ -627,6 +628,12 @@ public static void validateRepositoryMetadataSettings( + minVersionInCluster ); } + if (REMOTE_STORE_INDEX_SHALLOW_COPY.get(repositoryMetadataSettings) && !FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { + throw new RepositoryException( + repositoryName, + "setting " + REMOTE_STORE_INDEX_SHALLOW_COPY.getKey() + " cannot be enabled, as remote store feature is not enabled." + ); + } } private static void ensureRepositoryNotInUse(ClusterState clusterState, String repository) { diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index ebd0e59599c21..7df24e5357555 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -453,6 +453,12 @@ public ClusterState execute(ClusterState currentState) { final boolean isRemoteStoreShallowCopy = Boolean.TRUE.equals( snapshotInfo.isRemoteStoreIndexShallowCopyEnabled() ) && metadata.index(index).getSettings().getAsBoolean(SETTING_REMOTE_STORE_ENABLED, false); + if (isSearchableSnapshot && isRemoteStoreShallowCopy) { + throw new SnapshotRestoreException( + snapshot, + "Shallow copy snapshot cannot be restored as searchable snapshot." + ); + } if (isRemoteStoreShallowCopy && !currentState.getNodes().getMinNodeVersion().onOrAfter(Version.V_2_9_0)) { throw new SnapshotRestoreException( snapshot,