From bdf6f1d09702aebd8b1e94a2cb722c14a6825297 Mon Sep 17 00:00:00 2001 From: Sooraj Sinha <81695996+soosinha@users.noreply.github.com> Date: Thu, 7 Sep 2023 20:01:04 +0530 Subject: [PATCH 1/6] Add logic to fetch previousClusterUUID (#9746) Signed-off-by: Sooraj Sinha --- .../remote/ClusterMetadataManifest.java | 46 +++++- .../remote/RemoteClusterStateService.java | 146 ++++++++++++++++-- .../coordination/CoordinationStateTests.java | 6 +- .../remote/ClusterMetadataManifestTests.java | 22 ++- .../RemoteClusterStateServiceTests.java | 110 +++++++++++++ 5 files changed, 305 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java index cac77f9996438..0ebbdc81661ad 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java @@ -41,6 +41,7 @@ public class ClusterMetadataManifest implements Writeable, ToXContentFragment { private static final ParseField NODE_ID_FIELD = new ParseField("node_id"); private static final ParseField COMMITTED_FIELD = new ParseField("committed"); private static final ParseField INDICES_FIELD = new ParseField("indices"); + private static final ParseField PREVIOUS_CLUSTER_UUID = new ParseField("previous_cluster_uuid"); private static long term(Object[] fields) { return (long) fields[0]; @@ -74,6 +75,10 @@ private static List indices(Object[] fields) { return (List) fields[7]; } + private static String previousClusterUUID(Object[] fields) { + return (String) fields[8]; + } + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "cluster_metadata_manifest", fields -> new ClusterMetadataManifest( @@ -84,7 +89,8 @@ private static List indices(Object[] fields) { opensearchVersion(fields), nodeId(fields), committed(fields), - indices(fields) + indices(fields), + previousClusterUUID(fields) ) ); @@ -101,6 +107,7 @@ private static List indices(Object[] fields) { (p, c) -> UploadedIndexMetadata.fromXContent(p), INDICES_FIELD ); + PARSER.declareString(ConstructingObjectParser.constructorArg(), PREVIOUS_CLUSTER_UUID); } private final List indices; @@ -111,6 +118,7 @@ private static List indices(Object[] fields) { private final Version opensearchVersion; private final String nodeId; private final boolean committed; + private final String previousClusterUUID; public List getIndices() { return indices; @@ -144,6 +152,10 @@ public boolean isCommitted() { return committed; } + public String getPreviousClusterUUID() { + return previousClusterUUID; + } + public ClusterMetadataManifest( long clusterTerm, long version, @@ -152,7 +164,8 @@ public ClusterMetadataManifest( Version opensearchVersion, String nodeId, boolean committed, - List indices + List indices, + String previousClusterUUID ) { this.clusterTerm = clusterTerm; this.stateVersion = version; @@ -162,6 +175,7 @@ public ClusterMetadataManifest( this.nodeId = nodeId; this.committed = committed; this.indices = Collections.unmodifiableList(indices); + this.previousClusterUUID = previousClusterUUID; } public ClusterMetadataManifest(StreamInput in) throws IOException { @@ -173,6 +187,7 @@ public ClusterMetadataManifest(StreamInput in) throws IOException { this.nodeId = in.readString(); this.committed = in.readBoolean(); this.indices = Collections.unmodifiableList(in.readList(UploadedIndexMetadata::new)); + this.previousClusterUUID = in.readString(); } public static Builder builder() { @@ -199,6 +214,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } } builder.endArray(); + builder.field(PREVIOUS_CLUSTER_UUID.getPreferredName(), getPreviousClusterUUID()); return builder; } @@ -212,6 +228,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(nodeId); out.writeBoolean(committed); out.writeCollection(indices); + out.writeString(previousClusterUUID); } @Override @@ -230,12 +247,23 @@ public boolean equals(Object o) { && Objects.equals(stateUUID, that.stateUUID) && Objects.equals(opensearchVersion, that.opensearchVersion) && Objects.equals(nodeId, that.nodeId) - && Objects.equals(committed, that.committed); + && Objects.equals(committed, that.committed) + && Objects.equals(previousClusterUUID, that.previousClusterUUID); } @Override public int hashCode() { - return Objects.hash(indices, clusterTerm, stateVersion, clusterUUID, stateUUID, opensearchVersion, nodeId, committed); + return Objects.hash( + indices, + clusterTerm, + stateVersion, + clusterUUID, + stateUUID, + opensearchVersion, + nodeId, + committed, + previousClusterUUID + ); } @Override @@ -261,6 +289,7 @@ public static class Builder { private String stateUUID; private Version opensearchVersion; private String nodeId; + private String previousClusterUUID; private boolean committed; public Builder indices(List indices) { @@ -307,6 +336,11 @@ public List getIndices() { return indices; } + public Builder previousClusterUUID(String previousClusterUUID) { + this.previousClusterUUID = previousClusterUUID; + return this; + } + public Builder() { indices = new ArrayList<>(); } @@ -320,6 +354,7 @@ public Builder(ClusterMetadataManifest manifest) { this.nodeId = manifest.nodeId; this.committed = manifest.committed; this.indices = new ArrayList<>(manifest.indices); + this.previousClusterUUID = manifest.previousClusterUUID; } public ClusterMetadataManifest build() { @@ -331,7 +366,8 @@ public ClusterMetadataManifest build() { opensearchVersion, nodeId, committed, - indices + indices, + previousClusterUUID ); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index d2f6fd8ebe228..990fc20a7e95d 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -46,6 +46,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -93,6 +94,11 @@ public class RemoteClusterStateService implements Closeable { private static final Logger logger = LogManager.getLogger(RemoteClusterStateService.class); public static final String DELIMITER = "__"; + private static final String CLUSTER_STATE_PATH_TOKEN = "cluster-state"; + private static final String INDEX_PATH_TOKEN = "index"; + private static final String MANIFEST_PATH_TOKEN = "manifest"; + private static final String MANIFEST_FILE_PREFIX = "manifest"; + private static final String INDEX_METADATA_FILE_PREFIX = "metadata"; private final String nodeId; private final Supplier repositoriesService; @@ -159,8 +165,8 @@ public ClusterMetadataManifest writeFullMetadata(ClusterState clusterState) thro /** * This method uploads the diff between the previous cluster state and the current cluster state. The previous manifest file is needed to create the new - * manifest. The new manifest file is created by using the unchanged metadata from the previous manifest and the new metadata changes from the current cluster - * state. + * manifest. The new manifest file is created by using the unchanged metadata from the previous manifest and the new metadata changes from the current + * cluster state. * * @return The uploaded ClusterMetadataManifest file */ @@ -238,10 +244,10 @@ public ClusterMetadataManifest writeIncrementalMetadata( /** * Uploads provided IndexMetadata's to remote store in parallel. The call is blocking so the method waits for upload to finish and then return. + * * @param clusterState current ClusterState * @param toUpload list of IndexMetadata to upload * @return {@code List} list of IndexMetadata uploaded to remote - * @throws IOException */ private List writeIndexMetadataParallel(ClusterState clusterState, List toUpload) throws IOException { @@ -312,10 +318,10 @@ private List writeIndexMetadataParallel(ClusterState clus /** * Allows async Upload of IndexMetadata to remote + * * @param clusterState current ClusterState * @param indexMetadata {@link IndexMetadata} to upload * @param latchedActionListener listener to respond back on after upload finishes - * @throws IOException */ private void writeIndexMetadataAsync( ClusterState clusterState, @@ -360,11 +366,6 @@ public ClusterMetadataManifest markLastStateAsCommitted(ClusterState clusterStat return uploadManifest(clusterState, previousManifest.getIndices(), true); } - public ClusterState getLatestClusterState(String clusterUUID) { - // todo - return null; - } - @Override public void close() throws IOException { if (blobStoreRepository != null) { @@ -402,7 +403,9 @@ private ClusterMetadataManifest uploadManifest( Version.CURRENT, nodeId, committed, - uploadedIndexMetadata + uploadedIndexMetadata, + // todo Change this to proper cluster UUID + ClusterState.UNKNOWN_UUID ); writeMetadataManifest(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID(), manifest, manifestFileName); return manifest; @@ -421,9 +424,9 @@ private BlobContainer indexMetadataContainer(String clusterName, String clusterU .blobContainer( blobStoreRepository.basePath() .add(encodeString(clusterName)) - .add("cluster-state") + .add(CLUSTER_STATE_PATH_TOKEN) .add(clusterUUID) - .add("index") + .add(INDEX_PATH_TOKEN) .add(indexUUID) ); } @@ -432,7 +435,20 @@ private BlobContainer manifestContainer(String clusterName, String clusterUUID) // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest return blobStoreRepository.blobStore() .blobContainer( - blobStoreRepository.basePath().add(encodeString(clusterName)).add("cluster-state").add(clusterUUID).add("manifest") + blobStoreRepository.basePath() + .add(encodeString(clusterName)) + .add(CLUSTER_STATE_PATH_TOKEN) + .add(clusterUUID) + .add(MANIFEST_PATH_TOKEN) + ); + } + + private BlobContainer clusterUUIDContainer(String clusterName) { + return blobStoreRepository.blobStore() + .blobContainer( + blobStoreRepository.basePath() + .add(Base64.getUrlEncoder().withoutPadding().encodeToString(clusterName.getBytes(StandardCharsets.UTF_8))) + .add(CLUSTER_STATE_PATH_TOKEN) ); } @@ -444,7 +460,7 @@ private static String getManifestFileName(long term, long version) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest_2147483642_2147483637_456536447 return String.join( DELIMITER, - "manifest", + MANIFEST_FILE_PREFIX, RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version), RemoteStoreUtils.invertLong(System.currentTimeMillis()) @@ -452,11 +468,17 @@ private static String getManifestFileName(long term, long version) { } private static String indexMetadataFileName(IndexMetadata indexMetadata) { - return String.join(DELIMITER, "metadata", String.valueOf(indexMetadata.getVersion()), String.valueOf(System.currentTimeMillis())); + return String.join( + DELIMITER, + INDEX_METADATA_FILE_PREFIX, + String.valueOf(indexMetadata.getVersion()), + String.valueOf(System.currentTimeMillis()) + ); } /** * Fetch latest index metadata from remote cluster state + * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @return {@code Map} latest IndexUUID to IndexMetadata map @@ -476,6 +498,7 @@ public Map getLatestIndexMetadata(String clusterName, Str /** * Fetch index metadata from remote cluster state + * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @param uploadedIndexMetadata {@link UploadedIndexMetadata} contains details about remote location of index metadata @@ -499,6 +522,7 @@ private IndexMetadata getIndexMetadata(String clusterName, String clusterUUID, U /** * Fetch latest ClusterMetadataManifest from remote state store + * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @return ClusterMetadataManifest @@ -508,8 +532,97 @@ public ClusterMetadataManifest getLatestClusterMetadataManifest(String clusterNa return fetchRemoteClusterMetadataManifest(clusterName, clusterUUID, latestManifestFileName); } + /** + * Fetch the previous cluster UUIDs from remote state store and return the most recent valid cluster UUID + * + * @param clusterName The cluster name for which previous cluster UUID is to be fetched + * @return Last valid cluster UUID + */ + public String getLatestClusterUUID(String clusterName) { + try { + Set clusterUUIDs = getAllClusterUUIDs(clusterName); + Map latestManifests = getLatestManifestForAllClusterUUIDs(clusterName, clusterUUIDs); + List validChain = createClusterChain(latestManifests); + if (validChain.isEmpty()) { + return ClusterState.UNKNOWN_UUID; + } + return validChain.get(0); + } catch (IOException e) { + throw new IllegalStateException( + String.format(Locale.ROOT, "Error while fetching previous UUIDs from remote store for cluster name: %s", clusterName) + ); + } + } + + private Set getAllClusterUUIDs(String clusterName) throws IOException { + Map clusterUUIDMetadata = clusterUUIDContainer(clusterName).children(); + if (clusterUUIDMetadata == null) { + return Collections.emptySet(); + } + return Collections.unmodifiableSet(clusterUUIDMetadata.keySet()); + } + + private Map getLatestManifestForAllClusterUUIDs(String clusterName, Set clusterUUIDs) { + Map manifestsByClusterUUID = new HashMap<>(); + for (String clusterUUID : clusterUUIDs) { + try { + ClusterMetadataManifest manifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); + manifestsByClusterUUID.put(clusterUUID, manifest); + } catch (Exception e) { + throw new IllegalStateException( + String.format(Locale.ROOT, "Exception in fetching manifest for clusterUUID: %s", clusterUUID) + ); + } + } + return manifestsByClusterUUID; + } + + /** + * This method creates a valid cluster UUID chain. + * + * @param manifestsByClusterUUID Map of latest ClusterMetadataManifest for every cluster UUID + * @return List of cluster UUIDs. The first element is the most recent cluster UUID in the chain + */ + private List createClusterChain(final Map manifestsByClusterUUID) { + final Map clusterUUIDGraph = manifestsByClusterUUID.values() + .stream() + .collect(Collectors.toMap(ClusterMetadataManifest::getClusterUUID, ClusterMetadataManifest::getPreviousClusterUUID)); + final List validClusterUUIDs = manifestsByClusterUUID.values() + .stream() + .filter(m -> !isInvalidClusterUUID(m) && !clusterUUIDGraph.containsValue(m.getClusterUUID())) + .map(ClusterMetadataManifest::getClusterUUID) + .collect(Collectors.toList()); + if (validClusterUUIDs.isEmpty()) { + logger.info("There is no valid previous cluster UUID"); + return Collections.emptyList(); + } + if (validClusterUUIDs.size() > 1) { + throw new IllegalStateException( + String.format( + Locale.ROOT, + "The system has ended into multiple valid cluster states in the remote store. " + + "Please check their latest manifest to decide which one you want to keep. Valid Cluster UUIDs: - %s", + validClusterUUIDs + ) + ); + } + final List validChain = new ArrayList<>(); + String currentUUID = validClusterUUIDs.get(0); + while (!ClusterState.UNKNOWN_UUID.equals(currentUUID)) { + validChain.add(currentUUID); + // Getting the previous cluster UUID of a cluster UUID from the clusterUUID Graph + currentUUID = clusterUUIDGraph.get(currentUUID); + } + return validChain; + } + + private boolean isInvalidClusterUUID(ClusterMetadataManifest manifest) { + return !manifest.isCommitted() && manifest.getIndices().isEmpty(); + } + /** * Fetch latest ClusterMetadataManifest file from remote state store + * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @return latest ClusterMetadataManifest filename @@ -522,7 +635,7 @@ private String getLatestManifestFileName(String clusterName, String clusterUUID) * when sorted in LEXICOGRAPHIC order the latest uploaded manifest file comes on top. */ List manifestFilesMetadata = manifestContainer(clusterName, clusterUUID).listBlobsByPrefixInSortedOrder( - "manifest" + DELIMITER, + MANIFEST_FILE_PREFIX + DELIMITER, 1, BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC ); @@ -538,6 +651,7 @@ private String getLatestManifestFileName(String clusterName, String clusterUUID) /** * Fetch ClusterMetadataManifest from remote state store + * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster * @return ClusterMetadataManifest diff --git a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java index ef56d70d6153c..aa4472c4fcec5 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java @@ -935,7 +935,8 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep Version.CURRENT, randomAlphaOfLength(10), false, - Collections.emptyList() + Collections.emptyList(), + randomAlphaOfLength(10) ) ); @@ -976,7 +977,8 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep Version.CURRENT, randomAlphaOfLength(10), false, - Collections.emptyList() + Collections.emptyList(), + randomAlphaOfLength(10) ); Mockito.when(remoteClusterStateService.writeFullMetadata(clusterState2)).thenReturn(manifest2); coordinationState.handlePrePublish(clusterState2); diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index 449af91c34531..9f8dde5ba9d45 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -36,7 +36,8 @@ public void testClusterMetadataManifestXContent() throws IOException { Version.CURRENT, "test-node-id", false, - Collections.singletonList(uploadedIndexMetadata) + Collections.singletonList(uploadedIndexMetadata), + "prev-cluster-uuid" ); final XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); @@ -58,7 +59,8 @@ public void testClusterMetadataManifestSerializationEqualsHashCode() { Version.CURRENT, "B10RX1f5RJenMQvYccCgSQ", true, - randomUploadedIndexMetadataList() + randomUploadedIndexMetadataList(), + "yfObdx8KSMKKrXf8UyHhM" ); { // Mutate Cluster Term EqualsHashCodeTestUtils.checkEqualsAndHashCode( @@ -165,6 +167,22 @@ public void testClusterMetadataManifestSerializationEqualsHashCode() { } ); } + { // Mutate Previous cluster UUID + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.previousClusterUUID("vZX62DCQEOzGXlxXCrEu"); + return builder.build(); + } + ); + + } } private List randomUploadedIndexMetadataList() { diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index da1f97f7042f1..156138340db10 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -54,13 +54,16 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -163,6 +166,7 @@ public void testWriteFullMetadataSuccess() throws IOException { .stateVersion(1L) .stateUUID("state-uuid") .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") .build(); assertThat(manifest.getIndices().size(), is(1)); @@ -290,6 +294,7 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { .stateVersion(1L) .stateUUID("state-uuid") .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") .build(); assertThat(manifest.getIndices().size(), is(1)); @@ -383,6 +388,7 @@ public void testReadLatestMetadataManifestSuccessButNoIndexMetadata() throws IOE .clusterUUID("cluster-uuid") .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") .build(); BlobContainer blobContainer = mockBlobStoreObjects(); @@ -408,6 +414,7 @@ public void testReadLatestMetadataManifestSuccessButIndexMetadataFetchIOExceptio .clusterUUID("cluster-uuid") .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") .build(); BlobContainer blobContainer = mockBlobStoreObjects(); @@ -438,6 +445,7 @@ public void testReadLatestMetadataManifestSuccess() throws IOException { .clusterUUID("cluster-uuid") .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") .build(); mockBlobContainer(mockBlobStoreObjects(), expectedManifest, new HashMap<>()); @@ -482,6 +490,7 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { .clusterUUID("cluster-uuid") .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") .build(); mockBlobContainer(mockBlobStoreObjects(), expectedManifest, Map.of(index.getUUID(), indexMetadata)); @@ -514,6 +523,7 @@ public void testMarkLastStateAsCommittedSuccess() throws IOException { .stateUUID("state-uuid") .clusterUUID("cluster-uuid") .nodeId("nodeA") + .previousClusterUUID("prev-cluster-uuid") .build(); assertThat(manifest.getIndices().size(), is(1)); @@ -526,6 +536,98 @@ public void testMarkLastStateAsCommittedSuccess() throws IOException { assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); } + public void testGetValidPreviousClusterUUID() throws IOException { + Map clusterUUIDsPointers = Map.of( + "cluster-uuid1", + ClusterState.UNKNOWN_UUID, + "cluster-uuid2", + "cluster-uuid1", + "cluster-uuid3", + "cluster-uuid2" + ); + mockObjectsForGettingPreviousClusterUUID(clusterUUIDsPointers); + + remoteClusterStateService.ensureRepositorySet(); + String previousClusterUUID = remoteClusterStateService.getLatestClusterUUID("test-cluster"); + assertThat(previousClusterUUID, equalTo("cluster-uuid3")); + } + + public void testGetValidPreviousClusterUUIDForInvalidChain() throws IOException { + Map clusterUUIDsPointers = Map.of( + "cluster-uuid1", + ClusterState.UNKNOWN_UUID, + "cluster-uuid2", + ClusterState.UNKNOWN_UUID, + "cluster-uuid3", + "cluster-uuid2" + ); + mockObjectsForGettingPreviousClusterUUID(clusterUUIDsPointers); + + remoteClusterStateService.ensureRepositorySet(); + assertThrows(IllegalStateException.class, () -> remoteClusterStateService.getLatestClusterUUID("test-cluster")); + } + + private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { + final BlobPath blobPath = mock(BlobPath.class); + when((blobStoreRepository.basePath())).thenReturn(blobPath); + when(blobPath.add(anyString())).thenReturn(blobPath); + when(blobPath.buildAsString()).thenReturn("/blob/path/"); + BlobContainer blobContainer1 = mock(BlobContainer.class); + BlobContainer blobContainer2 = mock(BlobContainer.class); + BlobContainer blobContainer3 = mock(BlobContainer.class); + BlobContainer uuidBlobContainer = mock(BlobContainer.class); + when(blobContainer1.path()).thenReturn(blobPath); + when(blobContainer2.path()).thenReturn(blobPath); + when(blobContainer3.path()).thenReturn(blobPath); + + mockBlobContainerForClusterUUIDs(uuidBlobContainer, clusterUUIDsPointers.keySet()); + final ClusterMetadataManifest clusterManifest1 = generateClusterMetadataManifest( + "cluster-uuid1", + clusterUUIDsPointers.get("cluster-uuid1"), + randomAlphaOfLength(10) + ); + mockBlobContainer(blobContainer1, clusterManifest1, Map.of()); + + final ClusterMetadataManifest clusterManifest2 = generateClusterMetadataManifest( + "cluster-uuid2", + clusterUUIDsPointers.get("cluster-uuid2"), + randomAlphaOfLength(10) + ); + mockBlobContainer(blobContainer2, clusterManifest2, Map.of()); + + final ClusterMetadataManifest clusterManifest3 = generateClusterMetadataManifest( + "cluster-uuid3", + clusterUUIDsPointers.get("cluster-uuid3"), + randomAlphaOfLength(10) + ); + mockBlobContainer(blobContainer3, clusterManifest3, Map.of()); + + when(blobStore.blobContainer(ArgumentMatchers.any())).thenReturn( + uuidBlobContainer, + blobContainer1, + blobContainer1, + blobContainer2, + blobContainer2, + blobContainer3, + blobContainer3 + ); + when(blobStoreRepository.getCompressor()).thenReturn(new DeflateCompressor()); + } + + private ClusterMetadataManifest generateClusterMetadataManifest(String clusterUUID, String previousClusterUUID, String stateUUID) { + return ClusterMetadataManifest.builder() + .indices(List.of()) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID(stateUUID) + .clusterUUID(clusterUUID) + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID(previousClusterUUID) + .committed(true) + .build(); + } + private BlobContainer mockBlobStoreObjects() { return mockBlobStoreObjects(BlobContainer.class); } @@ -542,6 +644,14 @@ private BlobContainer mockBlobStoreObjects(Class blobCo return blobContainer; } + private void mockBlobContainerForClusterUUIDs(BlobContainer blobContainer, Set clusterUUIDs) throws IOException { + Map blobContainerMap = new HashMap<>(); + for (String clusterUUID : clusterUUIDs) { + blobContainerMap.put(clusterUUID, mockBlobStoreObjects()); + } + when(blobContainer.children()).thenReturn(blobContainerMap); + } + private void mockBlobContainer( BlobContainer blobContainer, ClusterMetadataManifest clusterMetadataManifest, From 6a0c50e9f6936e99184dfebf788f769815827f11 Mon Sep 17 00:00:00 2001 From: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> Date: Thu, 7 Sep 2023 20:02:32 +0530 Subject: [PATCH 2/6] [Remote Store] Removing feature flag to mark feature GA (#9761) Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> --- CHANGELOG.md | 1 + distribution/src/config/opensearch.yml | 8 --- .../index/SegmentReplicationPressureIT.java | 8 +-- .../replication/SegmentReplicationBaseIT.java | 4 +- ...emoteStoreMockRepositoryIntegTestCase.java | 14 ---- .../RemoteIndexPrimaryRelocationIT.java | 10 --- .../remotestore/RemoteIndexRecoveryIT.java | 10 --- .../remotestore/RemoteRestoreSnapshotIT.java | 2 - .../RemoteStoreBaseIntegTestCase.java | 17 ----- .../SegmentReplicationUsingRemoteStoreIT.java | 13 ---- ...tReplicationWithRemoteStorePressureIT.java | 12 ---- .../opensearch/snapshots/CloneSnapshotIT.java | 6 -- .../snapshots/DeleteSnapshotIT.java | 12 ---- .../RemoteIndexSnapshotStatusApiIT.java | 3 - .../snapshots/RestoreSnapshotIT.java | 6 -- .../snapshots/SnapshotStatusApisIT.java | 2 - .../org/opensearch/action/ActionModule.java | 8 +-- .../restore/RestoreSnapshotRequest.java | 65 ++++++------------- .../action/bulk/TransportShardBulkAction.java | 3 +- .../common/settings/ClusterSettings.java | 5 +- .../common/settings/FeatureFlagSettings.java | 1 - .../common/settings/IndexScopedSettings.java | 11 ++-- .../opensearch/common/util/FeatureFlags.java | 9 --- .../org/opensearch/index/IndexSettings.java | 4 +- .../opensearch/index/shard/IndexShard.java | 3 +- .../org/opensearch/indices/IndicesModule.java | 5 +- .../opensearch/indices/IndicesService.java | 10 +-- .../cluster/IndicesClusterStateService.java | 6 +- .../repositories/RepositoriesService.java | 7 -- .../TransportRemoteStoreStatsActionTests.java | 5 -- .../MetadataCreateIndexServiceTests.java | 23 ++++--- .../index/shard/RemoteIndexShardTests.java | 10 --- ...overyWithRemoteTranslogOnPrimaryTests.java | 10 --- .../indices/IndicesServiceTests.java | 4 +- ...teStorePeerRecoverySourceHandlerTests.java | 10 --- .../BlobStoreRepositoryRemoteIndexTests.java | 9 --- .../blobstore/BlobStoreRepositoryTests.java | 6 -- 37 files changed, 60 insertions(+), 282 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe90492f3d309..37cdc4ddaafac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote Store] Add Segment download stats to remotestore stats API ([#8718](https://github.com/opensearch-project/OpenSearch/pull/8718)) - [Remote Store] Add remote segment transfer stats on NodesStats API ([#9168](https://github.com/opensearch-project/OpenSearch/pull/9168) [#9393](https://github.com/opensearch-project/OpenSearch/pull/9393) [#9454](https://github.com/opensearch-project/OpenSearch/pull/9454)) - Return 409 Conflict HTTP status instead of 503 on failure to concurrently execute snapshots ([#8986](https://github.com/opensearch-project/OpenSearch/pull/5855)) +- [Remote Store] Removing feature flag to mark feature GA ([#9761](https://github.com/opensearch-project/OpenSearch/pull/9761)) ### Deprecated diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml index de2d0e023a200..1d2cfe7eccae6 100644 --- a/distribution/src/config/opensearch.yml +++ b/distribution/src/config/opensearch.yml @@ -98,18 +98,10 @@ ${path.logs} # node.attr.remote_store.translog.repository: my-repo-1 # # ---------------------------------- Experimental Features ----------------------------------- -# # Gates the visibility of the experimental segment replication features until they are production ready. # #opensearch.experimental.feature.segment_replication_experimental.enabled: false # -# -# Gates the visibility of the index setting that allows persisting data to remote store along with local disk. -# Once the feature is ready for production release, this feature flag can be removed. -# -#opensearch.experimental.feature.remote_store.enabled: false -# -# # Gates the functionality of a new parameter to the snapshot restore API # that allows for creation of a new index type that searches a snapshot # directly in a remote repository without restoring all index data to disk diff --git a/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java b/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java index 2fbad9099c056..033ea75b68958 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java @@ -15,7 +15,6 @@ import org.opensearch.common.lease.Releasable; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; import org.opensearch.core.rest.RestStatus; import org.opensearch.index.shard.IndexShard; @@ -31,7 +30,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -264,9 +262,9 @@ public void testFailStaleReplica() throws Exception { } public void testWithDocumentReplicationEnabledIndex() throws Exception { - assumeTrue( - "Can't create DocRep index with remote store enabled. Skipping.", - Objects.equals(featureFlagSettings().get(FeatureFlags.REMOTE_STORE, "false"), "false") + assumeFalse( + "Skipping the test as its not compatible with segment replication with remote store. Cannot create DocRep indices with Remote store enabled", + segmentReplicationWithRemoteEnabled() ); Settings settings = Settings.builder() .put(MAX_REPLICATION_TIME_BACKPRESSURE_SETTING.getKey(), TimeValue.timeValueMillis(500)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java index a16167c8ad0c4..8e68a8bde39d5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java @@ -18,7 +18,6 @@ import org.opensearch.common.Nullable; import org.opensearch.common.lease.Releasable; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.index.Index; import org.opensearch.core.index.shard.ShardId; import org.opensearch.index.IndexModule; @@ -204,8 +203,7 @@ protected IndexShard getIndexShard(String node, String indexName) { } protected boolean segmentReplicationWithRemoteEnabled() { - return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue() - && "true".equalsIgnoreCase(featureFlagSettings().get(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL)); + return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue(); } protected Releasable blockReplication(List nodes, CountDownLatch latch) { diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java index a5017144ce890..bc55f6cc2cbcb 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java @@ -12,15 +12,12 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.util.FileSystemUtils; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.store.RemoteSegmentStoreDirectory; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.snapshots.AbstractSnapshotIntegTestCase; -import org.opensearch.test.FeatureFlagSetter; -import org.junit.Before; import java.io.IOException; import java.nio.file.Path; @@ -44,17 +41,6 @@ public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends Abs protected static final String TRANSLOG_REPOSITORY_NAME = "my-translog-repo-1"; protected static final String INDEX_NAME = "remote-store-test-idx-1"; - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); - } - - @Before - public void setup() { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); - FeatureFlagSetter.set(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL); - } - @Override public Settings indexSettings() { return remoteStoreIndexSettings(0); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java index 66259869a5d28..d8b7718a55377 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java @@ -10,7 +10,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.indices.recovery.IndexPrimaryRelocationIT; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchIntegTestCase; @@ -50,15 +49,6 @@ public Settings indexSettings() { .build(); } - @Override - protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .build(); - } - public void testPrimaryRelocationWhileIndexing() throws Exception { internalCluster().startClusterManagerOnlyNode(); super.testPrimaryRelocationWhileIndexing(); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java index d11c09928a08f..4eb1cc7703735 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java @@ -10,7 +10,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.indices.recovery.IndexRecoveryIT; @@ -46,15 +45,6 @@ protected Settings nodeSettings(int nodeOrdinal) { .build(); } - @Override - protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .build(); - } - @Override public Settings indexSettings() { return Settings.builder() diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 16264253b4000..4ebccb9b9e551 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -20,7 +20,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.io.PathUtils; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.rest.RestStatus; import org.opensearch.index.IndexSettings; import org.opensearch.indices.replication.common.ReplicationType; @@ -65,7 +64,6 @@ public void teardown() { protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) - .put(FeatureFlags.REMOTE_STORE, "true") .put(remoteStoreClusterSettings(BASE_REMOTE_REPO, remoteRepoPath)) .build(); } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java index 1660201dcb307..9a684ce0a1482 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java @@ -22,7 +22,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; @@ -48,7 +47,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; -import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT; @@ -136,15 +134,6 @@ protected Settings nodeSettings(int nodeOrdinal) { } } - @Override - protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .build(); - } - public Settings indexSettings() { return defaultIndexSettings(); } @@ -186,13 +175,7 @@ public static Settings remoteStoreClusterSettings( Path translogRepoPath ) { Settings.Builder settingsBuilder = Settings.builder(); - - if (randomBoolean()) { - settingsBuilder.put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT); - } - settingsBuilder.put(buildRemoteStoreNodeAttributes(segmentRepoName, segmentRepoPath, translogRepoName, translogRepoPath, false)); - return settingsBuilder.build(); } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java index 1b817408596ab..45c3ef7f5bae5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java @@ -9,7 +9,6 @@ package org.opensearch.remotestore; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.indices.replication.SegmentReplicationIT; import org.opensearch.test.OpenSearchIntegTestCase; import org.junit.After; @@ -22,9 +21,6 @@ /** * This class runs Segment Replication Integ test suite with remote store enabled. - * Setup is similar to SegmentReplicationRemoteStoreIT but this also enables the segment replication using remote store which - * is behind SEGMENT_REPLICATION_EXPERIMENTAL flag. After this is moved out of experimental, we can combine and keep only one - * test suite for Segment and Remote store integration tests. */ @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class SegmentReplicationUsingRemoteStoreIT extends SegmentReplicationIT { @@ -47,15 +43,6 @@ protected boolean segmentReplicationWithRemoteEnabled() { return true; } - @Override - protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .build(); - } - @Before public void setup() { internalCluster().startClusterManagerOnlyNode(); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java index fa0944e5bfee0..0da4d81a8871e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java @@ -9,7 +9,6 @@ package org.opensearch.remotestore; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.SegmentReplicationPressureIT; import org.opensearch.test.OpenSearchIntegTestCase; import org.junit.After; @@ -22,8 +21,6 @@ /** * This class executes the SegmentReplicationPressureIT suite with remote store integration enabled. - * Setup is similar to SegmentReplicationPressureIT but this also enables the segment replication using remote store which - * is behind SEGMENT_REPLICATION_EXPERIMENTAL flag. */ @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class SegmentReplicationWithRemoteStorePressureIT extends SegmentReplicationPressureIT { @@ -36,15 +33,6 @@ protected boolean segmentReplicationWithRemoteEnabled() { return true; } - @Override - protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .build(); - } - @Override protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java index 9ec8cee2685fe..066d82483ae91 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java @@ -44,7 +44,6 @@ import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot; @@ -57,7 +56,6 @@ import org.opensearch.repositories.RepositoryShardId; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.snapshots.mockstore.MockRepository; -import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchIntegTestCase; import java.nio.file.Path; @@ -159,7 +157,6 @@ public void testCloneSnapshotIndex() throws Exception { public void testCloneShallowSnapshotIndex() throws Exception { disableRepoConsistencyCheck("This test uses remote store repository"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final String remoteStoreRepoName = "remote-store-repo-name"; final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath)); @@ -204,7 +201,6 @@ public void testCloneShallowSnapshotIndex() throws Exception { public void testShallowCloneNameAvailability() throws Exception { disableRepoConsistencyCheck("This test uses remote store repository"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final String remoteStoreRepoName = "remote-store-repo-name"; final Path remoteStorePath = randomRepoPath().toAbsolutePath(); internalCluster().startClusterManagerOnlyNode( @@ -245,7 +241,6 @@ public void testShallowCloneNameAvailability() throws Exception { public void testCloneAfterRepoShallowSettingEnabled() throws Exception { disableRepoConsistencyCheck("This test uses remote store repository"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final String remoteStoreRepoName = "remote-store-repo-name"; final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath)); @@ -280,7 +275,6 @@ public void testCloneAfterRepoShallowSettingEnabled() throws Exception { public void testCloneAfterRepoShallowSettingDisabled() throws Exception { disableRepoConsistencyCheck("This test uses remote store repository"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final String remoteStoreRepoName = "remote-store-repo-name"; final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath)); diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java index 448b860683668..e79bf1c16b586 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java @@ -16,9 +16,7 @@ import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase; -import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchIntegTestCase; import java.nio.file.Path; @@ -41,7 +39,6 @@ public class DeleteSnapshotIT extends AbstractSnapshotIntegTestCase { public void testDeleteSnapshot() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); @@ -69,7 +66,6 @@ public void testDeleteSnapshot() throws Exception { public void testDeleteShallowCopySnapshot() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); @@ -98,8 +94,6 @@ public void testDeleteShallowCopySnapshot() throws Exception { // Deleting multiple shallow copy snapshots as part of single delete call with repo having only shallow copy snapshots. public void testDeleteMultipleShallowCopySnapshotsCase1() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); - final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); @@ -142,8 +136,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase1() throws Exception { @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8610") public void testDeleteMultipleShallowCopySnapshotsCase2() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); - final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); final String dataNode = internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); @@ -228,8 +220,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase2() throws Exception { @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8610") public void testDeleteMultipleShallowCopySnapshotsCase3() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); - final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); @@ -288,8 +278,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase3() throws Exception { public void testRemoteStoreCleanupForDeletedIndex() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used in the test"); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); - final Path remoteStoreRepoPath = randomRepoPath(); internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath)); diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java index 979df95547d06..8e2580aba1745 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java @@ -39,7 +39,6 @@ import org.opensearch.cluster.SnapshotsInProgress; import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.test.OpenSearchIntegTestCase; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; @@ -67,8 +66,6 @@ protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) .put(ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING.getKey(), 0) // We have tests that check by-timestamp order - .put(FeatureFlags.REMOTE_STORE, "true") - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") .put(remoteStoreClusterSettings(remoteStoreRepoName, absolutePath)) .build(); } diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java index 7327f1127eab2..7117818451e14 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java @@ -45,7 +45,6 @@ import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.rest.RestStatus; @@ -82,11 +81,6 @@ import static org.hamcrest.Matchers.nullValue; public class RestoreSnapshotIT extends AbstractSnapshotIntegTestCase { - @Override - protected Settings nodeSettings(int nodeOrdinal) { - return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(FeatureFlags.REMOTE_STORE, "true").build(); - } - public void testParallelRestoreOperations() { String indexName1 = "testindex1"; String indexName2 = "testindex2"; diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index 30394611ac48f..c574233d25051 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -47,7 +47,6 @@ import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.Strings; import org.opensearch.core.common.unit.ByteSizeUnit; @@ -77,7 +76,6 @@ protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) .put(ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING.getKey(), 0) // We have tests that check by-timestamp order - .put(FeatureFlags.REMOTE_STORE, "true") .build(); } diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index e9fe10b15e817..46775466aa615 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -972,12 +972,8 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestCatAction(catActions)); registerHandler.accept(new RestDecommissionAction()); registerHandler.accept(new RestGetDecommissionStateAction()); - - // Remote Store APIs - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { - registerHandler.accept(new RestRemoteStoreStatsAction()); - registerHandler.accept(new RestRestoreRemoteStoreAction()); - } + registerHandler.accept(new RestRemoteStoreStatsAction()); + registerHandler.accept(new RestRestoreRemoteStoreAction()); } @Override 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 6f58a532b48a1..fcc48b973d49b 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 @@ -39,7 +39,6 @@ import org.opensearch.common.Nullable; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; @@ -152,7 +151,7 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_7_0)) { storageType = in.readEnum(StorageType.class); } - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && in.getVersion().onOrAfter(Version.V_2_9_0)) { + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { sourceRemoteStoreRepository = in.readOptionalString(); } } @@ -176,7 +175,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeEnum(storageType); } - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && out.getVersion().onOrAfter(Version.V_2_9_0)) { + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { out.writeOptionalString(sourceRemoteStoreRepository); } } @@ -616,11 +615,6 @@ 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 { @@ -671,7 +665,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (storageType != null) { storageType.toXContent(builder); } - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && sourceRemoteStoreRepository != null) { + if (sourceRemoteStoreRepository != null) { builder.field("source_remote_store_repository", sourceRemoteStoreRepository); } builder.endObject(); @@ -701,48 +695,29 @@ 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); - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { - equals = Objects.equals(sourceRemoteStoreRepository, that.sourceRemoteStoreRepository); - } + && Objects.equals(storageType, that.storageType) + && Objects.equals(sourceRemoteStoreRepository, that.sourceRemoteStoreRepository); return equals; } @Override public int hashCode() { 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 = Objects.hash( + snapshot, + repository, + indicesOptions, + renamePattern, + renameReplacement, + waitForCompletion, + includeGlobalState, + partial, + includeAliases, + indexSettings, + snapshotUuid, + storageType, + sourceRemoteStoreRepository + ); result = 31 * result + Arrays.hashCode(indices); result = 31 * result + Arrays.hashCode(ignoreIndexSettings); return result; diff --git a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java index efc21b2c03808..fddda0ef1f9a7 100644 --- a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java +++ b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java @@ -72,7 +72,6 @@ import org.opensearch.common.lease.Releasable; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.action.ActionListener; @@ -539,7 +538,7 @@ protected Releasable checkPrimaryLimits(BulkShardRequest request, boolean rerout } // TODO - While removing remote store flag, this can be encapsulated to single class with common interface for backpressure // service - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE) && remoteStorePressureService.isSegmentsUploadBackpressureEnabled()) { + if (remoteStorePressureService.isSegmentsUploadBackpressureEnabled()) { remoteStorePressureService.validateSegmentsUploadLag(request.shardId()); } } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 8c04e01a75fb4..74224d66400da 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -670,7 +670,8 @@ public void apply(Settings value, Settings current, Settings previous) { // Remote cluster state settings RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING, - RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING + RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING, + IndicesService.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING ) ) ); @@ -683,8 +684,6 @@ public void apply(Settings value, Settings current, Settings previous) { * setting should be moved to {@link #BUILT_IN_CLUSTER_SETTINGS}. */ public static final Map, List> FEATURE_FLAGGED_CLUSTER_SETTINGS = Map.of( - List.of(FeatureFlags.REMOTE_STORE), - List.of(IndicesService.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING), List.of(FeatureFlags.CONCURRENT_SEGMENT_SEARCH), List.of( SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING, diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index e109d2a871cef..90abc0a0765c1 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -36,7 +36,6 @@ protected FeatureFlagSettings( new HashSet<>( Arrays.asList( FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING, - FeatureFlags.REMOTE_STORE_SETTING, FeatureFlags.EXTENSIONS_SETTING, FeatureFlags.IDENTITY_SETTING, FeatureFlags.CONCURRENT_SEGMENT_SEARCH_SETTING, diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 30a3550d62c8a..5b2afc44600bd 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -213,6 +213,11 @@ public final class IndexScopedSettings extends AbstractScopedSettings { // Settings for remote translog IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, + // Settings for remote store enablement + IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING, + IndexMetadata.INDEX_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING, + IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING, + // validate that built-in similarities don't get redefined Setting.groupSetting("index.similarity.", (s) -> { Map groups = s.getAsGroups(); @@ -236,12 +241,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings { * setting should be moved to {@link #BUILT_IN_INDEX_SETTINGS}. */ public static final Map> FEATURE_FLAGGED_INDEX_SETTINGS = Map.of( - FeatureFlags.REMOTE_STORE, - List.of( - IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING, - IndexMetadata.INDEX_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING, - IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING - ), FeatureFlags.CONCURRENT_SEGMENT_SEARCH, List.of(IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING) ); diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index e2663b56c5cca..b89d2d0549823 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -20,19 +20,12 @@ * @opensearch.internal */ public class FeatureFlags { - /** * Gates the visibility of the segment replication experimental features that allows users to test unreleased beta features. */ public static final String SEGMENT_REPLICATION_EXPERIMENTAL = "opensearch.experimental.feature.segment_replication_experimental.enabled"; - /** - * Gates the visibility of the index setting that allows persisting data to remote store along with local disk. - * Once the feature is ready for production release, this feature flag can be removed. - */ - public static final String REMOTE_STORE = "opensearch.experimental.feature.remote_store.enabled"; - /** * Gates the ability for Searchable Snapshots to read snapshots that are older than the * guaranteed backward compatibility for OpenSearch (one prior major version) on a best effort basis. @@ -96,8 +89,6 @@ public static boolean isEnabled(String featureFlagName) { Property.NodeScope ); - public static final Setting REMOTE_STORE_SETTING = Setting.boolSetting(REMOTE_STORE, false, Property.NodeScope); - public static final Setting EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope); public static final Setting IDENTITY_SETTING = Setting.boolSetting(IDENTITY, false, Property.NodeScope); diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index ff2a67e99a94d..03c71351294d5 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -1063,11 +1063,11 @@ public boolean isSegRepEnabled() { } public boolean isSegRepLocalEnabled() { - return isSegRepEnabled() && !isSegRepWithRemoteEnabled(); + return isSegRepEnabled() && !isRemoteStoreEnabled(); } public boolean isSegRepWithRemoteEnabled() { - return isSegRepEnabled() && isRemoteStoreEnabled() && FeatureFlags.isEnabled(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL); + return isSegRepEnabled() && isRemoteStoreEnabled(); } /** diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 3cd2fb231e284..8ed75330f938e 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -3729,8 +3729,7 @@ private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) thro internalRefreshListener.add( new RemoteStoreRefreshListener( this, - // Add the checkpoint publisher if the Segment Replciation via remote store is enabled. - indexSettings.isSegRepWithRemoteEnabled() ? this.checkpointPublisher : SegmentReplicationCheckpointPublisher.EMPTY, + this.checkpointPublisher, remoteStoreStatsTrackerFactory.getRemoteSegmentTransferTracker(shardId()) ) ); diff --git a/server/src/main/java/org/opensearch/indices/IndicesModule.java b/server/src/main/java/org/opensearch/indices/IndicesModule.java index c6ae8b988aed0..5c2137ec742a4 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesModule.java +++ b/server/src/main/java/org/opensearch/indices/IndicesModule.java @@ -38,7 +38,6 @@ import org.opensearch.action.admin.indices.rollover.MaxSizeCondition; import org.opensearch.action.resync.TransportResyncReplicationAction; import org.opensearch.common.inject.AbstractModule; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.ParseField; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.common.io.stream.NamedWriteableRegistry.Entry; @@ -288,9 +287,7 @@ protected void configure() { bind(RetentionLeaseSyncer.class).asEagerSingleton(); bind(SegmentReplicationCheckpointPublisher.class).asEagerSingleton(); bind(SegmentReplicationPressureService.class).asEagerSingleton(); - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { - bind(RemoteStorePressureService.class).asEagerSingleton(); - } + bind(RemoteStorePressureService.class).asEagerSingleton(); } /** diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 2e2a0762ea489..2ed3cb8d9e8ea 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -69,7 +69,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.BigArrays; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.AbstractRefCounted; import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.util.concurrent.OpenSearchExecutors; @@ -457,12 +456,9 @@ protected void closeInternal() { this.clusterDefaultRefreshInterval = CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.get(clusterService.getSettings()); clusterService.getClusterSettings() .addSettingsUpdateConsumer(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING, this::onRefreshIntervalUpdate); - - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { - this.clusterRemoteTranslogBufferInterval = CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(clusterService.getSettings()); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, this::setClusterRemoteTranslogBufferInterval); - } + this.clusterRemoteTranslogBufferInterval = CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(clusterService.getSettings()); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, this::setClusterRemoteTranslogBufferInterval); } /** diff --git a/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java b/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java index 0904dc5fa18a4..dc538a03de595 100644 --- a/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java +++ b/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java @@ -55,7 +55,6 @@ import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.util.concurrent.ConcurrentCollections; import org.opensearch.core.action.ActionListener; @@ -223,10 +222,7 @@ public IndicesClusterStateService( ); indexEventListeners.add(segmentReplicationTargetService); indexEventListeners.add(segmentReplicationSourceService); - // if remote store feature is not enabled, do not wire the remote upload pressure service as an IndexEventListener. - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE)) { - indexEventListeners.add(remoteStoreStatsTrackerFactory); - } + indexEventListeners.add(remoteStoreStatsTrackerFactory); this.segmentReplicationTargetService = segmentReplicationTargetService; this.builtInIndexListener = Collections.unmodifiableList(indexEventListeners); this.indicesService = indicesService; diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index ab343aabd3a82..3d6679b3ef80e 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -64,7 +64,6 @@ 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.core.action.ActionListener; @@ -683,12 +682,6 @@ 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/test/java/org/opensearch/action/admin/cluster/remotestore/stats/TransportRemoteStoreStatsActionTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/TransportRemoteStoreStatsActionTests.java index 75707f2a7853a..1695e3dea59c6 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/TransportRemoteStoreStatsActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/TransportRemoteStoreStatsActionTests.java @@ -23,7 +23,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.index.Index; import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; @@ -32,7 +31,6 @@ import org.opensearch.index.shard.IndexShardTestCase; import org.opensearch.indices.IndicesService; import org.opensearch.indices.replication.common.ReplicationType; -import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.transport.MockTransport; import org.opensearch.transport.TransportService; @@ -113,7 +111,6 @@ public void tearDown() throws Exception { } public void testAllShardCopies() throws Exception { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); RoutingTable routingTable = RoutingTable.builder().addAsNew(remoteStoreIndexMetadata).build(); Metadata metadata = Metadata.builder().put(remoteStoreIndexMetadata, false).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).routingTable(routingTable).build(); @@ -133,7 +130,6 @@ public void testAllShardCopies() throws Exception { } public void testOnlyLocalShards() throws Exception { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); String[] concreteIndices = new String[] { INDEX.getName() }; RoutingTable routingTable = spy(RoutingTable.builder().addAsNew(remoteStoreIndexMetadata).build()); doReturn(new PlainShardsIterator(routingTable.allShards(INDEX.getName()).stream().map(Mockito::spy).collect(Collectors.toList()))) @@ -161,7 +157,6 @@ public void testOnlyLocalShards() throws Exception { } public void testOnlyRemoteStoreEnabledShardCopies() throws Exception { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); Index NEW_INDEX = new Index("newIndex", "newUUID"); IndexMetadata indexMetadataWithoutRemoteStore = IndexMetadata.builder(NEW_INDEX.getName()) .settings( diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 86e154c547e07..8adaae6d230cd 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -61,7 +61,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.BigArrays; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -79,7 +78,6 @@ import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.snapshots.EmptySnapshotsInfoService; import org.opensearch.test.ClusterServiceUtils; -import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; import org.opensearch.test.gateway.TestGatewayAllocator; @@ -1221,7 +1219,6 @@ public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettin .put(segmentRepositoryNameAttributeKey, "my-segment-repo-1") .put(translogRepositoryNameAttributeKey, "my-translog-repo-1") .build(); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); final Settings.Builder requestSettings = Settings.builder(); @@ -1253,7 +1250,6 @@ public void testRemoteStoreImplicitOverrideReplicationTypeToSegmentForRemoteStor .put(segmentRepositoryNameAttributeKey, "my-segment-repo-1") .put(translogRepositoryNameAttributeKey, "my-translog-repo-1") .build(); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); final Settings.Builder requestSettings = Settings.builder(); @@ -1285,7 +1281,6 @@ public void testRemoteStoreNoUserOverrideIndexSettings() { .put(segmentRepositoryNameAttributeKey, "my-segment-repo-1") .put(translogRepositoryNameAttributeKey, "my-translog-repo-1") .build(); - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); Settings indexSettings = aggregateIndexSettings( @@ -1337,7 +1332,7 @@ public void testRemoteStoreDisabledByUserIndexSettings() { assertThat(validationErrors.size(), is(1)); assertThat( validationErrors.get(0), - is(String.format(Locale.ROOT, "expected [%s] to be private but it was not", SETTING_REMOTE_STORE_ENABLED)) + is(String.format(Locale.ROOT, "private index setting [%s] can not be set explicitly", SETTING_REMOTE_STORE_ENABLED)) ); })); } @@ -1371,7 +1366,13 @@ public void testRemoteStoreOverrideSegmentRepoIndexSettings() { assertThat(validationErrors.size(), is(1)); assertThat( validationErrors.get(0), - is(String.format(Locale.ROOT, "expected [%s] to be private but it was not", SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)) + is( + String.format( + Locale.ROOT, + "private index setting [%s] can not be set explicitly", + SETTING_REMOTE_SEGMENT_STORE_REPOSITORY + ) + ) ); })); } @@ -1404,7 +1405,13 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() { assertThat(validationErrors.size(), is(1)); assertThat( validationErrors.get(0), - is(String.format(Locale.ROOT, "expected [%s] to be private but it was not", SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)) + is( + String.format( + Locale.ROOT, + "private index setting [%s] can not be set explicitly", + SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY + ) + ) ); })); } diff --git a/server/src/test/java/org/opensearch/index/shard/RemoteIndexShardTests.java b/server/src/test/java/org/opensearch/index/shard/RemoteIndexShardTests.java index bcda7b941aaee..6a99063d11353 100644 --- a/server/src/test/java/org/opensearch/index/shard/RemoteIndexShardTests.java +++ b/server/src/test/java/org/opensearch/index/shard/RemoteIndexShardTests.java @@ -14,7 +14,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.concurrent.GatedCloseable; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.action.ActionListener; import org.opensearch.index.engine.DocIdSeqNoAndSource; import org.opensearch.index.engine.Engine; @@ -34,7 +33,6 @@ import org.opensearch.indices.replication.common.ReplicationType; import org.hamcrest.MatcherAssert; import org.junit.Assert; -import org.junit.Before; import java.io.IOException; import java.nio.file.Path; @@ -65,14 +63,6 @@ public class RemoteIndexShardTests extends SegmentReplicationIndexShardTests { .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME) .build(); - @Before - public void setup() { - // Todo: Remove feature flag once remote store integration with segrep goes GA - FeatureFlags.initializeFeatureFlags( - Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build() - ); - } - protected Settings getIndexSettings() { return settings; } diff --git a/server/src/test/java/org/opensearch/index/shard/ReplicaRecoveryWithRemoteTranslogOnPrimaryTests.java b/server/src/test/java/org/opensearch/index/shard/ReplicaRecoveryWithRemoteTranslogOnPrimaryTests.java index 2a4c572ee2875..4f5cad70fd643 100644 --- a/server/src/test/java/org/opensearch/index/shard/ReplicaRecoveryWithRemoteTranslogOnPrimaryTests.java +++ b/server/src/test/java/org/opensearch/index/shard/ReplicaRecoveryWithRemoteTranslogOnPrimaryTests.java @@ -13,7 +13,6 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexSettings; import org.opensearch.index.engine.DocIdSeqNoAndSource; import org.opensearch.index.engine.NRTReplicationEngine; @@ -25,7 +24,6 @@ import org.opensearch.indices.recovery.RecoveryTarget; import org.opensearch.indices.replication.common.ReplicationType; import org.junit.Assert; -import org.junit.Before; import java.io.IOException; import java.nio.file.Path; @@ -43,14 +41,6 @@ public class ReplicaRecoveryWithRemoteTranslogOnPrimaryTests extends OpenSearchI .put(IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "100ms") .build(); - @Before - public void setup() { - // Todo: Remove feature flag once remote store integration with segrep goes GA - FeatureFlags.initializeFeatureFlags( - Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build() - ); - } - public void testStartSequenceForReplicaRecovery() throws Exception { final Path remoteDir = createTempDir(); final String indexMapping = "{ \"" + MapperService.SINGLE_MAPPING_NAME + "\": {} }"; diff --git a/server/src/test/java/org/opensearch/indices/IndicesServiceTests.java b/server/src/test/java/org/opensearch/indices/IndicesServiceTests.java index d22ad06245687..742dbdeba8c5b 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesServiceTests.java @@ -620,8 +620,8 @@ public void testConflictingEngineFactories() { assertThat(e, hasToString(new RegexMatcher(pattern))); } - public void testClusterRemoteTranslogBufferIntervalNull() { + public void testClusterRemoteTranslogBufferIntervalDefault() { IndicesService indicesService = getIndicesService(); - assertNull(indicesService.getClusterRemoteTranslogBufferInterval()); + assertEquals(IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL, indicesService.getClusterRemoteTranslogBufferInterval()); } } diff --git a/server/src/test/java/org/opensearch/indices/recovery/RemoteStorePeerRecoverySourceHandlerTests.java b/server/src/test/java/org/opensearch/indices/recovery/RemoteStorePeerRecoverySourceHandlerTests.java index 920a3f2946884..131514eb019b3 100644 --- a/server/src/test/java/org/opensearch/indices/recovery/RemoteStorePeerRecoverySourceHandlerTests.java +++ b/server/src/test/java/org/opensearch/indices/recovery/RemoteStorePeerRecoverySourceHandlerTests.java @@ -10,7 +10,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexSettings; import org.opensearch.index.engine.NRTReplicationEngineFactory; import org.opensearch.index.mapper.MapperService; @@ -18,7 +17,6 @@ import org.opensearch.index.seqno.ReplicationTracker; import org.opensearch.index.shard.IndexShard; import org.opensearch.indices.replication.common.ReplicationType; -import org.junit.Before; import java.nio.file.Path; @@ -31,14 +29,6 @@ public class RemoteStorePeerRecoverySourceHandlerTests extends OpenSearchIndexLe .put(IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "100ms") .build(); - @Before - public void setup() { - // Todo: Remove feature flag once remote store integration with segrep goes GA - FeatureFlags.initializeFeatureFlags( - Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build() - ); - } - public void testReplicaShardRecoveryUptoLastFlushedCommit() throws Exception { final Path remoteDir = createTempDir(); final String indexMapping = "{ \"" + MapperService.SINGLE_MAPPING_NAME + "\": {} }"; diff --git a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java index a820c17d188d6..7091e88892d09 100644 --- a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java +++ b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java @@ -36,7 +36,6 @@ import org.opensearch.client.Client; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.index.shard.ShardId; import org.opensearch.env.Environment; import org.opensearch.gateway.remote.RemoteClusterStateService; @@ -49,7 +48,6 @@ import org.opensearch.repositories.fs.FsRepository; import org.opensearch.snapshots.SnapshotId; import org.opensearch.snapshots.SnapshotInfo; -import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchIntegTestCase; import java.io.IOException; @@ -72,10 +70,6 @@ * Tests for the {@link BlobStoreRepository} and its subclasses. */ public class BlobStoreRepositoryRemoteIndexTests extends BlobStoreRepositoryHelperTests { - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); - } @Override protected Settings nodeSettings() { @@ -118,7 +112,6 @@ private Settings buildRemoteStoreNodeAttributes(String repoName, Path repoPath) // Validate Scenario Normal Snapshot -> remoteStoreShallowCopy Snapshot -> normal Snapshot public void testRetrieveShallowCopySnapshotCase1() throws IOException { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final Client client = client(); final String snapshotRepositoryName = "test-repo"; final String remoteStoreRepositoryName = "test-rs-repo"; @@ -208,7 +201,6 @@ public void testRetrieveShallowCopySnapshotCase1() throws IOException { } public void testGetRemoteStoreShallowCopyShardMetadata() throws IOException { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final Client client = client(); final String snapshotRepositoryName = "test-repo"; final String remoteStoreRepositoryName = "test-rs-repo"; @@ -259,7 +251,6 @@ public void testGetRemoteStoreShallowCopyShardMetadata() throws IOException { // Validate Scenario remoteStoreShallowCopy Snapshot -> remoteStoreShallowCopy Snapshot // -> remoteStoreShallowCopy Snapshot -> normal snapshot public void testRetrieveShallowCopySnapshotCase2() throws IOException { - FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); final Client client = client(); final String snapshotRepositoryName = "test-repo"; final String remoteStoreRepositoryName = "test-rs-repo"; diff --git a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java index ea9f96ff925cd..e097c7025e4fe 100644 --- a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java +++ b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java @@ -41,7 +41,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; @@ -105,11 +104,6 @@ protected void assertSnapshotOrGenericThread() { } } - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); - } - public void testRetrieveSnapshots() throws Exception { final Client client = client(); final Path location = OpenSearchIntegTestCase.randomRepoPath(node().settings()); From 731b13fe5830ec98022f650db52d8c011f7c56e7 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Thu, 7 Sep 2023 21:23:03 +0530 Subject: [PATCH 3/6] Fix flaky codec tests (#9889) Signed-off-by: Sarthak Aggarwal --- .../org/opensearch/index/codec/MultiCodecReindexIT.java | 4 ++-- .../opensearch/index/codec/CodecCompressionLevelIT.java | 8 ++++---- .../org/opensearch/index/codec/MultiCodecMergeIT.java | 5 ++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java b/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java index 60afe44babc26..3f5df7cf57897 100644 --- a/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java +++ b/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java @@ -135,7 +135,7 @@ private void assertReindexingWithMultipleCodecs(String destCodec, String destCod } private void useCodec(String index, String codec) throws ExecutionException, InterruptedException { - assertAcked(client().admin().indices().prepareClose(index)); + assertAcked(client().admin().indices().prepareClose(index).setWaitForActiveShards(1)); assertAcked( client().admin() @@ -144,7 +144,7 @@ private void useCodec(String index, String codec) throws ExecutionException, Int .get() ); - assertAcked(client().admin().indices().prepareOpen(index)); + assertAcked(client().admin().indices().prepareOpen(index).setWaitForActiveShards(1)); } private void flushAndRefreshIndex(String index) { diff --git a/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/CodecCompressionLevelIT.java b/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/CodecCompressionLevelIT.java index 9bef3e2d3d235..7810b5810fffb 100644 --- a/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/CodecCompressionLevelIT.java +++ b/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/CodecCompressionLevelIT.java @@ -97,7 +97,7 @@ public void testZStandardToLuceneCodecsWithCompressionLevel() throws ExecutionEx ); ensureGreen(index); - assertAcked(client().admin().indices().prepareClose(index)); + assertAcked(client().admin().indices().prepareClose(index).setWaitForActiveShards(1)); Throwable executionException = expectThrows( ExecutionException.class, @@ -128,7 +128,7 @@ public void testZStandardToLuceneCodecsWithCompressionLevel() throws ExecutionEx .get() ); - assertAcked(client().admin().indices().prepareOpen(index)); + assertAcked(client().admin().indices().prepareOpen(index).setWaitForActiveShards(1)); ensureGreen(index); } @@ -148,7 +148,7 @@ public void testLuceneToZStandardCodecsWithCompressionLevel() throws ExecutionEx ); ensureGreen(index); - assertAcked(client().admin().indices().prepareClose(index)); + assertAcked(client().admin().indices().prepareClose(index).setWaitForActiveShards(1)); Throwable executionException = expectThrows( ExecutionException.class, @@ -181,7 +181,7 @@ public void testLuceneToZStandardCodecsWithCompressionLevel() throws ExecutionEx .get() ); - assertAcked(client().admin().indices().prepareOpen(index)); + assertAcked(client().admin().indices().prepareOpen(index).setWaitForActiveShards(1)); ensureGreen(index); } diff --git a/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java b/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java index fda9327c79dc1..bb508282e1952 100644 --- a/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java +++ b/plugins/custom-codecs/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java @@ -49,7 +49,6 @@ protected Collection> nodePlugins() { return Collections.singletonList(CustomCodecPlugin.class); } - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/9872") public void testForceMergeMultipleCodecs() throws ExecutionException, InterruptedException { Map codecMap = Map.of( @@ -119,7 +118,7 @@ private void forceMergeMultipleCodecs(String finalCodec, String finalCodecMode, } private void useCodec(String index, String codec) throws ExecutionException, InterruptedException { - assertAcked(client().admin().indices().prepareClose(index)); + assertAcked(client().admin().indices().prepareClose(index).setWaitForActiveShards(1)); assertAcked( client().admin() @@ -128,7 +127,7 @@ private void useCodec(String index, String codec) throws ExecutionException, Int .get() ); - assertAcked(client().admin().indices().prepareOpen(index)); + assertAcked(client().admin().indices().prepareOpen(index).setWaitForActiveShards(1)); } private void ingestDocs(String index) throws InterruptedException { From 0bc1d5b48c5f9369e44a935026370e6896adcead Mon Sep 17 00:00:00 2001 From: Sooraj Sinha <81695996+soosinha@users.noreply.github.com> Date: Thu, 7 Sep 2023 21:40:45 +0530 Subject: [PATCH 4/6] Restore cluster metadata during bootstrap (#9831) Signed-off-by: Sooraj Sinha --- CHANGELOG.md | 1 + .../opensearch/gateway/GatewayMetaState.java | 42 ++-- .../remote/RemoteClusterStateService.java | 82 ++++--- .../main/java/org/opensearch/node/Node.java | 7 +- .../coordination/CoordinationStateTests.java | 46 ++-- .../GatewayMetaStatePersistedStateTests.java | 201 ++++++++++++++++-- .../RemoteClusterStateServiceTests.java | 52 ++--- .../BlobStoreRepositoryRemoteIndexTests.java | 2 +- .../gateway/MockGatewayMetaState.java | 46 ++-- 9 files changed, 336 insertions(+), 143 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37cdc4ddaafac..eabea882750f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote state] Integrate remote cluster state in publish/commit flow ([#9665](https://github.com/opensearch-project/OpenSearch/pull/9665)) - [Segment Replication] Adding segment replication statistics rolled up at index, node and cluster level ([#9709](https://github.com/opensearch-project/OpenSearch/pull/9709)) - [Remote Store] Changes to introduce repository registration during bootstrap via node attributes. ([#9105](https://github.com/opensearch-project/OpenSearch/pull/9105)) +- [Remote state] Auto restore index metadata from last known cluster state ([#9831](https://github.com/opensearch-project/OpenSearch/pull/9831)) ### Dependencies - Bump `org.apache.logging.log4j:log4j-core` from 2.17.1 to 2.20.0 ([#8307](https://github.com/opensearch-project/OpenSearch/pull/8307)) diff --git a/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java b/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java index ac6297598ecf5..e42ac8daa3b1c 100644 --- a/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java +++ b/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java @@ -63,6 +63,8 @@ import org.opensearch.env.NodeMetadata; import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.index.recovery.RemoteStoreRestoreService; +import org.opensearch.index.recovery.RemoteStoreRestoreService.RemoteRestoreResult; import org.opensearch.node.Node; import org.opensearch.plugins.MetadataUpgrader; import org.opensearch.repositories.RepositoryMissingException; @@ -126,7 +128,8 @@ public void start( MetadataUpgrader metadataUpgrader, PersistedClusterStateService persistedClusterStateService, RemoteClusterStateService remoteClusterStateService, - PersistedStateRegistry persistedStateRegistry + PersistedStateRegistry persistedStateRegistry, + RemoteStoreRestoreService remoteStoreRestoreService ) { assert this.persistedStateRegistry == null : "Persisted state registry should only be set once"; this.persistedStateRegistry = persistedStateRegistry; @@ -154,7 +157,7 @@ public void start( PersistedState remotePersistedState = null; boolean success = false; try { - final ClusterState clusterState = prepareInitialClusterState( + ClusterState clusterState = prepareInitialClusterState( transportService, clusterService, ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(settings)) @@ -164,10 +167,28 @@ public void start( ); if (DiscoveryNode.isClusterManagerNode(settings)) { - persistedState = new LucenePersistedState(persistedClusterStateService, currentTerm, clusterState); if (isRemoteStoreClusterStateEnabled(settings)) { + // If the cluster UUID loaded from local is unknown (_na_) then fetch the best state from remote + // If there is no valid state on remote, continue with initial empty state + // If there is a valid state, then restore index metadata using this state + if (ClusterState.UNKNOWN_UUID.equals(clusterState.metadata().clusterUUID())) { + String lastKnownClusterUUID = remoteClusterStateService.getLastKnownUUIDFromRemote( + clusterState.getClusterName().value() + ); + if (!ClusterState.UNKNOWN_UUID.equals(lastKnownClusterUUID)) { + // Load state from remote + final RemoteRestoreResult remoteRestoreResult = remoteStoreRestoreService.restore( + clusterState, + lastKnownClusterUUID, + false, + new String[] {} + ); + clusterState = remoteRestoreResult.getClusterState(); + } + } remotePersistedState = new RemotePersistedState(remoteClusterStateService); } + persistedState = new LucenePersistedState(persistedClusterStateService, currentTerm, clusterState); } else { persistedState = new AsyncLucenePersistedState( settings, @@ -651,12 +672,6 @@ public void setCurrentTerm(long currentTerm) { @Override public void setLastAcceptedState(ClusterState clusterState) { try { - if (lastAcceptedState == null || lastAcceptedState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) { - // On the initial bootstrap, repository will not be available. So we do not persist the cluster state and bail out. - logger.info("Cluster is not yet ready to publish state to remote store"); - lastAcceptedState = clusterState; - return; - } final ClusterMetadataManifest manifest; if (shouldWriteFullClusterState(clusterState)) { manifest = remoteClusterStateService.writeFullMetadata(clusterState); @@ -706,13 +721,8 @@ private boolean shouldWriteFullClusterState(ClusterState clusterState) { @Override public void markLastAcceptedStateAsCommitted() { try { - if (lastAcceptedState == null - || lastAcceptedManifest == null - || lastAcceptedState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) { - // On the initial bootstrap, repository will not be available. So we do not persist the cluster state and bail out. - logger.trace("Cluster is not yet ready to publish state to remote store"); - return; - } + assert lastAcceptedState != null : "Last accepted state is not present"; + assert lastAcceptedManifest != null : "Last accepted manifest is not present"; final ClusterMetadataManifest committedManifest = remoteClusterStateService.markLastStateAsCommitted( lastAcceptedState, lastAcceptedManifest diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 990fc20a7e95d..2aa3384b0f33a 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -46,6 +46,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -136,14 +137,20 @@ public ClusterMetadataManifest writeFullMetadata(ClusterState clusterState) thro logger.error("Local node is not elected cluster manager. Exiting"); return null; } - ensureRepositorySet(); + + // should fetch the previous cluster UUID before writing full cluster state. + // Whenever a new election happens, a new leader will be elected and it might have stale previous UUID + final String previousClusterUUID = fetchPreviousClusterUUID( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID() + ); // any validations before/after upload ? final List allUploadedIndexMetadata = writeIndexMetadataParallel( clusterState, new ArrayList<>(clusterState.metadata().indices().values()) ); - final ClusterMetadataManifest manifest = uploadManifest(clusterState, allUploadedIndexMetadata, false); + final ClusterMetadataManifest manifest = uploadManifest(clusterState, allUploadedIndexMetadata, previousClusterUUID, false); final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); if (durationMillis >= slowWriteLoggingThreshold.getMillis()) { logger.warn( @@ -220,7 +227,12 @@ public ClusterMetadataManifest writeIncrementalMetadata( for (String removedIndexName : previousStateIndexMetadataVersionByName.keySet()) { allUploadedIndexMetadata.remove(removedIndexName); } - final ClusterMetadataManifest manifest = uploadManifest(clusterState, new ArrayList<>(allUploadedIndexMetadata.values()), false); + final ClusterMetadataManifest manifest = uploadManifest( + clusterState, + new ArrayList<>(allUploadedIndexMetadata.values()), + previousManifest.getPreviousClusterUUID(), + false + ); final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); if (durationMillis >= slowWriteLoggingThreshold.getMillis()) { logger.warn( @@ -333,13 +345,13 @@ private void writeIndexMetadataAsync( clusterState.metadata().clusterUUID(), indexMetadata.getIndexUUID() ); - + final String indexMetadataFilename = indexMetadataFileName(indexMetadata); ActionListener completionListener = ActionListener.wrap( resp -> latchedActionListener.onResponse( new UploadedIndexMetadata( indexMetadata.getIndex().getName(), indexMetadata.getIndexUUID(), - indexMetadataContainer.path().buildAsString() + indexMetadataFileName(indexMetadata) + indexMetadataContainer.path().buildAsString() + indexMetadataFilename ) ), ex -> latchedActionListener.onFailure(new IndexMetadataTransferException(indexMetadata.getIndex().toString(), ex)) @@ -348,7 +360,7 @@ private void writeIndexMetadataAsync( INDEX_METADATA_FORMAT.writeAsync( indexMetadata, indexMetadataContainer, - indexMetadataFileName(indexMetadata), + indexMetadataFilename, blobStoreRepository.getCompressor(), completionListener ); @@ -363,7 +375,7 @@ public ClusterMetadataManifest markLastStateAsCommitted(ClusterState clusterStat } assert clusterState != null : "Last accepted cluster state is not set"; assert previousManifest != null : "Last cluster metadata manifest is not set"; - return uploadManifest(clusterState, previousManifest.getIndices(), true); + return uploadManifest(clusterState, previousManifest.getIndices(), previousManifest.getPreviousClusterUUID(), true); } @Override @@ -373,12 +385,8 @@ public void close() throws IOException { } } - // Visible for testing - void ensureRepositorySet() { - if (blobStoreRepository != null) { - return; - } - assert isRemoteStoreClusterStateEnabled(settings) : "Remote cluster state is not enabled"; + public void start() { + assert isRemoteStoreClusterStateEnabled(settings) == true : "Remote cluster state is not enabled"; final String remoteStoreRepo = settings.get( Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY ); @@ -391,6 +399,7 @@ void ensureRepositorySet() { private ClusterMetadataManifest uploadManifest( ClusterState clusterState, List uploadedIndexMetadata, + String previousClusterUUID, boolean committed ) throws IOException { synchronized (this) { @@ -404,8 +413,7 @@ private ClusterMetadataManifest uploadManifest( nodeId, committed, uploadedIndexMetadata, - // todo Change this to proper cluster UUID - ClusterState.UNKNOWN_UUID + previousClusterUUID ); writeMetadataManifest(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID(), manifest, manifestFileName); return manifest; @@ -418,6 +426,16 @@ private void writeMetadataManifest(String clusterName, String clusterUUID, Clust CLUSTER_METADATA_MANIFEST_FORMAT.write(uploadManifest, metadataManifestContainer, fileName, blobStoreRepository.getCompressor()); } + private String fetchPreviousClusterUUID(String clusterName, String clusterUUID) { + final Optional latestManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); + if (!latestManifest.isPresent()) { + final String previousClusterUUID = getLastKnownUUIDFromRemote(clusterName); + assert !clusterUUID.equals(previousClusterUUID) : "Last cluster UUID is same current cluster UUID"; + return previousClusterUUID; + } + return latestManifest.get().getPreviousClusterUUID(); + } + private BlobContainer indexMetadataContainer(String clusterName, String clusterUUID, String indexUUID) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX return blobStoreRepository.blobStore() @@ -484,12 +502,15 @@ private static String indexMetadataFileName(IndexMetadata indexMetadata) { * @return {@code Map} latest IndexUUID to IndexMetadata map */ public Map getLatestIndexMetadata(String clusterName, String clusterUUID) throws IOException { - ensureRepositorySet(); + start(); Map remoteIndexMetadata = new HashMap<>(); - ClusterMetadataManifest clusterMetadataManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); - assert Objects.equals(clusterUUID, clusterMetadataManifest.getClusterUUID()) + Optional clusterMetadataManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); + if (!clusterMetadataManifest.isPresent()) { + throw new IllegalStateException("Latest index metadata is not present for the provided clusterUUID"); + } + assert Objects.equals(clusterUUID, clusterMetadataManifest.get().getClusterUUID()) : "Corrupt ClusterMetadataManifest found. Cluster UUID mismatch."; - for (UploadedIndexMetadata uploadedIndexMetadata : clusterMetadataManifest.getIndices()) { + for (UploadedIndexMetadata uploadedIndexMetadata : clusterMetadataManifest.get().getIndices()) { IndexMetadata indexMetadata = getIndexMetadata(clusterName, clusterUUID, uploadedIndexMetadata); remoteIndexMetadata.put(uploadedIndexMetadata.getIndexUUID(), indexMetadata); } @@ -527,9 +548,12 @@ private IndexMetadata getIndexMetadata(String clusterName, String clusterUUID, U * @param clusterName name of the cluster * @return ClusterMetadataManifest */ - public ClusterMetadataManifest getLatestClusterMetadataManifest(String clusterName, String clusterUUID) { - String latestManifestFileName = getLatestManifestFileName(clusterName, clusterUUID); - return fetchRemoteClusterMetadataManifest(clusterName, clusterUUID, latestManifestFileName); + public Optional getLatestClusterMetadataManifest(String clusterName, String clusterUUID) { + Optional latestManifestFileName = getLatestManifestFileName(clusterName, clusterUUID); + if (latestManifestFileName.isPresent()) { + return Optional.of(fetchRemoteClusterMetadataManifest(clusterName, clusterUUID, latestManifestFileName.get())); + } + return Optional.empty(); } /** @@ -538,7 +562,7 @@ public ClusterMetadataManifest getLatestClusterMetadataManifest(String clusterNa * @param clusterName The cluster name for which previous cluster UUID is to be fetched * @return Last valid cluster UUID */ - public String getLatestClusterUUID(String clusterName) { + public String getLastKnownUUIDFromRemote(String clusterName) { try { Set clusterUUIDs = getAllClusterUUIDs(clusterName); Map latestManifests = getLatestManifestForAllClusterUUIDs(clusterName, clusterUUIDs); @@ -566,8 +590,8 @@ private Map getLatestManifestForAllClusterUUIDs Map manifestsByClusterUUID = new HashMap<>(); for (String clusterUUID : clusterUUIDs) { try { - ClusterMetadataManifest manifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); - manifestsByClusterUUID.put(clusterUUID, manifest); + Optional manifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); + manifestsByClusterUUID.put(clusterUUID, manifest.get()); } catch (Exception e) { throw new IllegalStateException( String.format(Locale.ROOT, "Exception in fetching manifest for clusterUUID: %s", clusterUUID) @@ -627,7 +651,7 @@ private boolean isInvalidClusterUUID(ClusterMetadataManifest manifest) { * @param clusterName name of the cluster * @return latest ClusterMetadataManifest filename */ - private String getLatestManifestFileName(String clusterName, String clusterUUID) throws IllegalStateException { + private Optional getLatestManifestFileName(String clusterName, String clusterUUID) throws IllegalStateException { try { /** * {@link BlobContainer#listBlobsByPrefixInSortedOrder} will get the latest manifest file @@ -640,13 +664,13 @@ private String getLatestManifestFileName(String clusterName, String clusterUUID) BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC ); if (manifestFilesMetadata != null && !manifestFilesMetadata.isEmpty()) { - return manifestFilesMetadata.get(0).name(); + return Optional.of(manifestFilesMetadata.get(0).name()); } } catch (IOException e) { throw new IllegalStateException("Error while fetching latest manifest file for remote cluster state", e); } - - throw new IllegalStateException(String.format(Locale.ROOT, "Remote Cluster State not found - %s", clusterUUID)); + logger.info("No manifest file present in remote store for cluster name: {}, cluster UUID: {}", clusterName, clusterUUID); + return Optional.empty(); } /** diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index d34d4ed3bed3e..e06207a0c7bff 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1335,6 +1335,10 @@ public Node start() throws NodeValidationException { injector.getInstance(PeerRecoverySourceService.class).start(); injector.getInstance(SegmentReplicationSourceService.class).start(); + final RemoteClusterStateService remoteClusterStateService = injector.getInstance(RemoteClusterStateService.class); + if (remoteClusterStateService != null) { + remoteClusterStateService.start(); + } // Load (and maybe upgrade) the metadata stored on disk final GatewayMetaState gatewayMetaState = injector.getInstance(GatewayMetaState.class); gatewayMetaState.start( @@ -1346,7 +1350,8 @@ public Node start() throws NodeValidationException { injector.getInstance(MetadataUpgrader.class), injector.getInstance(PersistedClusterStateService.class), injector.getInstance(RemoteClusterStateService.class), - injector.getInstance(PersistedStateRegistry.class) + injector.getInstance(PersistedStateRegistry.class), + injector.getInstance(RemoteStoreRestoreService.class) ); if (Assertions.ENABLED) { try { diff --git a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java index aa4472c4fcec5..d1c2dda615992 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java @@ -925,20 +925,18 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep final RemoteClusterStateService remoteClusterStateService = Mockito.mock(RemoteClusterStateService.class); final VotingConfiguration initialConfig = VotingConfiguration.of(node1); final ClusterState clusterState = clusterState(0L, 0L, node1, initialConfig, initialConfig, 42L); - Mockito.when(remoteClusterStateService.writeFullMetadata(clusterState)) - .thenReturn( - new ClusterMetadataManifest( - 0L, - 0L, - randomAlphaOfLength(10), - randomAlphaOfLength(10), - Version.CURRENT, - randomAlphaOfLength(10), - false, - Collections.emptyList(), - randomAlphaOfLength(10) - ) - ); + final ClusterMetadataManifest manifest = new ClusterMetadataManifest( + 0L, + 0L, + randomAlphaOfLength(10), + randomAlphaOfLength(10), + Version.CURRENT, + randomAlphaOfLength(10), + false, + Collections.emptyList(), + randomAlphaOfLength(10) + ); + Mockito.when(remoteClusterStateService.writeFullMetadata(clusterState)).thenReturn(manifest); final PersistedStateRegistry persistedStateRegistry = persistedStateRegistry(); persistedStateRegistry.addPersistedState(PersistedStateType.LOCAL, ps1); @@ -965,27 +963,11 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep final CoordinationState coordinationState = createCoordinationState(persistedStateRegistry, node1, settings); coordinationState.handlePrePublish(clusterState); - Mockito.verifyNoInteractions(remoteClusterStateService); + Mockito.verify(remoteClusterStateService, Mockito.times(1)).writeFullMetadata(clusterState); assertThat(persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE).getLastAcceptedState(), equalTo(clusterState)); - final ClusterState clusterState2 = clusterState(0L, 1L, node1, initialConfig, initialConfig, 42L); - final ClusterMetadataManifest manifest2 = new ClusterMetadataManifest( - 0L, - 1L, - randomAlphaOfLength(10), - randomAlphaOfLength(10), - Version.CURRENT, - randomAlphaOfLength(10), - false, - Collections.emptyList(), - randomAlphaOfLength(10) - ); - Mockito.when(remoteClusterStateService.writeFullMetadata(clusterState2)).thenReturn(manifest2); - coordinationState.handlePrePublish(clusterState2); - Mockito.verify(remoteClusterStateService, Mockito.times(1)).writeFullMetadata(clusterState2); - coordinationState.handlePreCommit(); - Mockito.verify(remoteClusterStateService, Mockito.times(1)).markLastStateAsCommitted(clusterState2, manifest2); + Mockito.verify(remoteClusterStateService, Mockito.times(1)).markLastStateAsCommitted(clusterState, manifest); } public static CoordinationState createCoordinationState( diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 98a9c0ded2d9d..6a2f4cd0ab300 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -42,6 +42,7 @@ import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.coordination.CoordinationMetadata.VotingConfigExclusion; import org.opensearch.cluster.coordination.CoordinationState; +import org.opensearch.cluster.coordination.CoordinationState.PersistedState; import org.opensearch.cluster.coordination.PersistedStateRegistry; import org.opensearch.cluster.coordination.PersistedStateRegistry.PersistedStateType; import org.opensearch.cluster.metadata.IndexMetadata; @@ -51,6 +52,7 @@ import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.BigArrays; @@ -63,8 +65,11 @@ import org.opensearch.env.NodeEnvironment; import org.opensearch.env.TestEnvironment; import org.opensearch.gateway.GatewayMetaState.RemotePersistedState; +import org.opensearch.gateway.PersistedClusterStateService.Writer; import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.index.recovery.RemoteStoreRestoreService; +import org.opensearch.index.recovery.RemoteStoreRestoreService.RemoteRestoreResult; import org.opensearch.node.Node; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.fs.FsRepository; @@ -87,6 +92,7 @@ import org.mockito.Mockito; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT; @@ -96,8 +102,12 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class GatewayMetaStatePersistedStateTests extends OpenSearchTestCase { @@ -479,7 +489,8 @@ public void testDataOnlyNodePersistence() throws Exception { null, persistedClusterStateService, remoteClusterStateServiceSupplier.get(), - new PersistedStateRegistry() + new PersistedStateRegistry(), + null ); final CoordinationState.PersistedState persistedState = gateway.getPersistedState(); assertThat(persistedState, instanceOf(GatewayMetaState.AsyncLucenePersistedState.class)); @@ -712,7 +723,7 @@ public void testRemotePersistedState() throws IOException { ); remotePersistedState.setLastAcceptedState(clusterState); - Mockito.verify(remoteClusterStateService, times(0)).writeFullMetadata(Mockito.any()); + Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState); assertThat(remotePersistedState.getLastAcceptedState(), equalTo(clusterState)); assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm)); @@ -723,7 +734,7 @@ public void testRemotePersistedState() throws IOException { ); remotePersistedState.setLastAcceptedState(secondClusterState); - Mockito.verify(remoteClusterStateService, times(1)).writeFullMetadata(Mockito.any()); + Mockito.verify(remoteClusterStateService, times(1)).writeFullMetadata(secondClusterState); assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState)); assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm)); @@ -748,20 +759,19 @@ public void testRemotePersistedStateExceptionOnFullStateUpload() throws IOExcept Metadata.builder().coordinationMetadata(CoordinationMetadata.builder().term(clusterTerm).build()).build() ); - remotePersistedState.setLastAcceptedState(clusterState); - - final ClusterState secondClusterState = createClusterState( - randomNonNegativeLong(), - Metadata.builder().coordinationMetadata(CoordinationMetadata.builder().term(clusterTerm).build()).build() - ); - - assertThrows(OpenSearchException.class, () -> remotePersistedState.setLastAcceptedState(secondClusterState)); + assertThrows(OpenSearchException.class, () -> remotePersistedState.setLastAcceptedState(clusterState)); } public void testGatewayForRemoteState() throws IOException { MockGatewayMetaState gateway = null; try { - gateway = new MockGatewayMetaState(localNode, bigArrays); + RemoteClusterStateService remoteClusterStateService = mock(RemoteClusterStateService.class); + when(remoteClusterStateService.getLastKnownUUIDFromRemote("test-cluster")).thenReturn("test-cluster-uuid"); + RemoteStoreRestoreService remoteStoreRestoreService = mock(RemoteStoreRestoreService.class); + when(remoteStoreRestoreService.restore(any(), any(), anyBoolean(), any())).thenReturn( + RemoteRestoreResult.build("test-cluster-uuid", null, ClusterState.EMPTY_STATE) + ); + gateway = new MockGatewayMetaState(localNode, bigArrays, remoteClusterStateService, remoteStoreRestoreService); final PersistedStateRegistry persistedStateRegistry = persistedStateRegistry(); String stateRepoTypeAttributeKey = String.format( @@ -798,6 +808,173 @@ public void testGatewayForRemoteState() throws IOException { } } + public void testGatewayForRemoteStateForInitialBootstrap() throws IOException { + MockGatewayMetaState gateway = null; + try { + final RemoteClusterStateService remoteClusterStateService = mock(RemoteClusterStateService.class); + when(remoteClusterStateService.getLastKnownUUIDFromRemote(clusterName.value())).thenReturn(ClusterState.UNKNOWN_UUID); + + final RemoteStoreRestoreService remoteStoreRestoreService = mock(RemoteStoreRestoreService.class); + when(remoteStoreRestoreService.restore(any(), any(), anyBoolean(), any())).thenReturn( + RemoteRestoreResult.build("test-cluster-uuid", null, ClusterState.EMPTY_STATE) + ); + final PersistedStateRegistry persistedStateRegistry = persistedStateRegistry(); + gateway = newGatewayForRemoteState( + remoteClusterStateService, + remoteStoreRestoreService, + persistedStateRegistry, + ClusterState.EMPTY_STATE + ); + final CoordinationState.PersistedState lucenePersistedState = gateway.getPersistedState(); + PersistedState remotePersistedState = persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE); + verify(remoteClusterStateService).getLastKnownUUIDFromRemote(Mockito.any()); // change this + verifyNoInteractions(remoteStoreRestoreService); + assertThat(remotePersistedState.getLastAcceptedState(), nullValue()); + assertThat(lucenePersistedState.getLastAcceptedState().metadata(), equalTo(ClusterState.EMPTY_STATE.metadata())); + } finally { + IOUtils.close(gateway); + } + } + + public void testGatewayForRemoteStateForNodeReplacement() throws IOException { + MockGatewayMetaState gateway = null; + try { + final RemoteClusterStateService remoteClusterStateService = mock(RemoteClusterStateService.class); + when(remoteClusterStateService.getLastKnownUUIDFromRemote("test-cluster")).thenReturn("test-cluster-uuid"); + final ClusterState previousState = createClusterState( + randomNonNegativeLong(), + Metadata.builder() + .coordinationMetadata(CoordinationMetadata.builder().term(randomLong()).build()) + .put( + IndexMetadata.builder("test-index1") + .settings(settings(Version.CURRENT).put(SETTING_INDEX_UUID, randomAlphaOfLength(10))) + .numberOfShards(5) + .numberOfReplicas(1) + .build(), + false + ) + .clusterUUID(randomAlphaOfLength(10)) + .build() + ); + when(remoteClusterStateService.getLastKnownUUIDFromRemote(clusterName.value())).thenReturn( + previousState.metadata().clusterUUID() + ); + + final RemoteStoreRestoreService remoteStoreRestoreService = mock(RemoteStoreRestoreService.class); + when(remoteStoreRestoreService.restore(any(), any(), anyBoolean(), any())).thenReturn( + RemoteRestoreResult.build("test-cluster-uuid", null, previousState) + ); + final PersistedStateRegistry persistedStateRegistry = persistedStateRegistry(); + gateway = newGatewayForRemoteState( + remoteClusterStateService, + remoteStoreRestoreService, + persistedStateRegistry, + ClusterState.EMPTY_STATE + ); + final CoordinationState.PersistedState lucenePersistedState = gateway.getPersistedState(); + PersistedState remotePersistedState = persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE); + verify(remoteClusterStateService).getLastKnownUUIDFromRemote(Mockito.any()); + verify(remoteStoreRestoreService).restore(any(), any(), anyBoolean(), any()); + assertThat(remotePersistedState.getLastAcceptedState(), nullValue()); + assertThat(lucenePersistedState.getLastAcceptedState().metadata(), equalTo(previousState.metadata())); + } finally { + IOUtils.close(gateway); + } + } + + public void testGatewayForRemoteStateForNodeReboot() throws IOException { + MockGatewayMetaState gateway = null; + try { + final RemoteClusterStateService remoteClusterStateService = mock(RemoteClusterStateService.class); + final RemoteStoreRestoreService remoteStoreRestoreService = mock(RemoteStoreRestoreService.class); + final PersistedStateRegistry persistedStateRegistry = persistedStateRegistry(); + final IndexMetadata indexMetadata = IndexMetadata.builder("test-index1") + .settings(settings(Version.CURRENT).put(SETTING_INDEX_UUID, randomAlphaOfLength(10))) + .numberOfShards(5) + .numberOfReplicas(1) + .build(); + final ClusterState clusterState = createClusterState( + randomNonNegativeLong(), + Metadata.builder() + .coordinationMetadata(CoordinationMetadata.builder().term(randomLong()).build()) + .put(indexMetadata, false) + .clusterUUID(randomAlphaOfLength(10)) + .build() + ); + gateway = newGatewayForRemoteState(remoteClusterStateService, remoteStoreRestoreService, persistedStateRegistry, clusterState); + final CoordinationState.PersistedState lucenePersistedState = gateway.getPersistedState(); + PersistedState remotePersistedState = persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE); + verifyNoInteractions(remoteClusterStateService); + verifyNoInteractions(remoteStoreRestoreService); + assertThat(remotePersistedState.getLastAcceptedState(), nullValue()); + logger.info("lucene state metadata: {}", lucenePersistedState.getLastAcceptedState().toString()); + logger.info("initial metadata: {}", clusterState.toString()); + assertThat(lucenePersistedState.getLastAcceptedState().metadata().indices().size(), equalTo(1)); + assertThat(lucenePersistedState.getLastAcceptedState().metadata().indices().get("test-index1"), equalTo(indexMetadata)); + } finally { + IOUtils.close(gateway); + } + } + + private MockGatewayMetaState newGatewayForRemoteState( + RemoteClusterStateService remoteClusterStateService, + RemoteStoreRestoreService remoteStoreRestoreService, + PersistedStateRegistry persistedStateRegistry, + ClusterState currentState + ) throws IOException { + MockGatewayMetaState gateway = new MockGatewayMetaState(localNode, bigArrays); + String randomRepoName = "randomRepoName"; + String stateRepoTypeAttributeKey = String.format( + Locale.getDefault(), + "node.attr." + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, + randomRepoName + ); + String stateRepoSettingsAttributeKeyPrefix = String.format( + Locale.getDefault(), + "node.attr." + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, + randomRepoName + ); + Settings settingWithRemoteStateEnabled = Settings.builder() + .put(settings) + .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, randomRepoName) + .put(stateRepoTypeAttributeKey, FsRepository.TYPE) + .put(stateRepoSettingsAttributeKeyPrefix + "location", "randomRepoPath") + .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .build(); + final TransportService transportService = mock(TransportService.class); + ClusterService clusterService = mock(ClusterService.class); + when(clusterService.getClusterSettings()).thenReturn( + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + final PersistedClusterStateService persistedClusterStateService = new PersistedClusterStateService( + nodeEnvironment, + xContentRegistry(), + getBigArrays(), + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), + () -> 0L + ); + if (!ClusterState.EMPTY_STATE.equals(currentState)) { + Writer writer = persistedClusterStateService.createWriter(); + writer.writeFullStateAndCommit(currentState.term(), currentState); + writer.close(); + } + final MetaStateService metaStateService = mock(MetaStateService.class); + when(metaStateService.loadFullState()).thenReturn(new Tuple<>(Manifest.empty(), ClusterState.EMPTY_STATE.metadata())); + gateway.start( + settingWithRemoteStateEnabled, + transportService, + clusterService, + metaStateService, + null, + null, + persistedClusterStateService, + remoteClusterStateService, + persistedStateRegistry, + remoteStoreRestoreService + ); + return gateway; + } + private static BigArrays getBigArrays() { return usually() ? BigArrays.NON_RECYCLING_INSTANCE diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 156138340db10..8c6ccea940816 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -54,6 +54,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.Supplier; @@ -140,22 +141,21 @@ public void testFailInitializationWhenRemoteStateDisabled() throws IOException { ); } - public void testFailWriteFullMetadataWhenRepositoryNotSet() { - final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + public void testFailInitializeWhenRepositoryNotSet() { doThrow(new RepositoryMissingException("repository missing")).when(repositoriesService).repository("remote_store_repository"); - assertThrows(RepositoryMissingException.class, () -> remoteClusterStateService.writeFullMetadata(clusterState)); + assertThrows(RepositoryMissingException.class, () -> remoteClusterStateService.start()); } public void testFailWriteFullMetadataWhenNotBlobRepository() { final FilterRepository filterRepository = mock(FilterRepository.class); when(repositoriesService.repository("remote_store_repository")).thenReturn(filterRepository); - final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); - assertThrows(AssertionError.class, () -> remoteClusterStateService.writeFullMetadata(clusterState)); + assertThrows(AssertionError.class, () -> remoteClusterStateService.start()); } public void testWriteFullMetadataSuccess() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); mockBlobStoreObjects(); + remoteClusterStateService.start(); final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); List indices = List.of(uploadedIndexMetadata); @@ -191,6 +191,7 @@ public void testWriteFullMetadataInParallelSuccess() throws IOException { return null; }).when(container).asyncBlobUpload(writeContextArgumentCaptor.capture(), actionListenerArgumentCaptor.capture()); + remoteClusterStateService.start(); final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); @@ -245,6 +246,7 @@ public void testWriteFullMetadataInParallelFailure() throws IOException { return null; }).when(container).asyncBlobUpload(any(WriteContext.class), actionListenerArgumentCaptor.capture()); + remoteClusterStateService.start(); assertThrows( RemoteClusterStateService.IndexMetadataTransferException.class, () -> remoteClusterStateService.writeFullMetadata(clusterState) @@ -253,6 +255,7 @@ public void testWriteFullMetadataInParallelFailure() throws IOException { public void testFailWriteIncrementalMetadataNonClusterManagerNode() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().build(); + remoteClusterStateService.start(); final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata(clusterState, clusterState, null); Assert.assertThat(manifest, nullValue()); } @@ -279,7 +282,7 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata( previousClusterState, clusterState, @@ -319,7 +322,7 @@ public void testReadLatestMetadataManifestFailedIOException() throws IOException ) ).thenThrow(IOException.class); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, () -> remoteClusterStateService.getLatestClusterMetadataManifest( @@ -342,15 +345,12 @@ public void testReadLatestMetadataManifestFailedNoManifestFileInRemote() throws ) ).thenReturn(List.of()); - remoteClusterStateService.ensureRepositorySet(); - Exception e = assertThrows( - IllegalStateException.class, - () -> remoteClusterStateService.getLatestClusterMetadataManifest( - clusterState.getClusterName().value(), - clusterState.metadata().clusterUUID() - ) + remoteClusterStateService.start(); + Optional manifest = remoteClusterStateService.getLatestClusterMetadataManifest( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID() ); - assertEquals(e.getMessage(), "Remote Cluster State not found - " + clusterState.metadata().clusterUUID()); + assertEquals(manifest, Optional.empty()); } public void testReadLatestMetadataManifestFailedManifestFileRemoveAfterFetchInRemote() throws IOException { @@ -367,7 +367,7 @@ public void testReadLatestMetadataManifestFailedManifestFileRemoveAfterFetchInRe ).thenReturn(Arrays.asList(blobMetadata)); when(blobContainer.readBlob("manifestFileName")).thenThrow(FileNotFoundException.class); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, () -> remoteClusterStateService.getLatestClusterMetadataManifest( @@ -394,7 +394,7 @@ public void testReadLatestMetadataManifestSuccessButNoIndexMetadata() throws IOE BlobContainer blobContainer = mockBlobStoreObjects(); mockBlobContainer(blobContainer, expectedManifest, Map.of()); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); assertEquals( remoteClusterStateService.getLatestIndexMetadata(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) .size(), @@ -421,7 +421,7 @@ public void testReadLatestMetadataManifestSuccessButIndexMetadataFetchIOExceptio mockBlobContainer(blobContainer, expectedManifest, Map.of()); when(blobContainer.readBlob(uploadedIndexMetadata.getUploadedFilename() + ".dat")).thenThrow(FileNotFoundException.class); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); Exception e = assertThrows( IllegalStateException.class, () -> remoteClusterStateService.getLatestIndexMetadata( @@ -449,11 +449,11 @@ public void testReadLatestMetadataManifestSuccess() throws IOException { .build(); mockBlobContainer(mockBlobStoreObjects(), expectedManifest, new HashMap<>()); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); final ClusterMetadataManifest manifest = remoteClusterStateService.getLatestClusterMetadataManifest( clusterState.getClusterName().value(), clusterState.metadata().clusterUUID() - ); + ).get(); assertThat(manifest.getIndices().size(), is(1)); assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); @@ -467,7 +467,7 @@ public void testReadLatestMetadataManifestSuccess() throws IOException { public void testReadLatestIndexMetadataSuccess() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); final Index index = new Index("test-index", "index-uuid"); String fileName = "metadata-" + index.getUUID(); @@ -509,7 +509,7 @@ public void testReadLatestIndexMetadataSuccess() throws IOException { public void testMarkLastStateAsCommittedSuccess() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); mockBlobStoreObjects(); - remoteClusterStateService.ensureRepositorySet(); + remoteClusterStateService.start(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); List indices = List.of(uploadedIndexMetadata); final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(indices).build(); @@ -547,8 +547,8 @@ public void testGetValidPreviousClusterUUID() throws IOException { ); mockObjectsForGettingPreviousClusterUUID(clusterUUIDsPointers); - remoteClusterStateService.ensureRepositorySet(); - String previousClusterUUID = remoteClusterStateService.getLatestClusterUUID("test-cluster"); + remoteClusterStateService.start(); + String previousClusterUUID = remoteClusterStateService.getLastKnownUUIDFromRemote("test-cluster"); assertThat(previousClusterUUID, equalTo("cluster-uuid3")); } @@ -563,8 +563,8 @@ public void testGetValidPreviousClusterUUIDForInvalidChain() throws IOException ); mockObjectsForGettingPreviousClusterUUID(clusterUUIDsPointers); - remoteClusterStateService.ensureRepositorySet(); - assertThrows(IllegalStateException.class, () -> remoteClusterStateService.getLatestClusterUUID("test-cluster")); + remoteClusterStateService.start(); + assertThrows(IllegalStateException.class, () -> remoteClusterStateService.getLastKnownUUIDFromRemote("test-cluster")); } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { diff --git a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java index 7091e88892d09..e3e1bf31e82dc 100644 --- a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java +++ b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryRemoteIndexTests.java @@ -106,7 +106,7 @@ private Settings buildRemoteStoreNodeAttributes(String repoName, Path repoPath) .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, repoName) .put(repoTypeAttributeKey, FsRepository.TYPE) .put(repoSettingsAttributeKeyPrefix + "location", repoPath) - .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), false) .build(); } diff --git a/test/framework/src/main/java/org/opensearch/gateway/MockGatewayMetaState.java b/test/framework/src/main/java/org/opensearch/gateway/MockGatewayMetaState.java index bf59e9ff06cbe..dea205619ce95 100644 --- a/test/framework/src/main/java/org/opensearch/gateway/MockGatewayMetaState.java +++ b/test/framework/src/main/java/org/opensearch/gateway/MockGatewayMetaState.java @@ -46,16 +46,13 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.NodeEnvironment; import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.index.recovery.RemoteStoreRestoreService; import org.opensearch.plugins.MetadataUpgrader; -import org.opensearch.repositories.RepositoriesService; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; import java.io.IOException; -import java.util.Collections; -import java.util.function.Supplier; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -68,10 +65,26 @@ public class MockGatewayMetaState extends GatewayMetaState { private final DiscoveryNode localNode; private final BigArrays bigArrays; + private final RemoteClusterStateService remoteClusterStateService; + private final RemoteStoreRestoreService remoteStoreRestoreService; public MockGatewayMetaState(DiscoveryNode localNode, BigArrays bigArrays) { this.localNode = localNode; this.bigArrays = bigArrays; + this.remoteClusterStateService = mock(RemoteClusterStateService.class); + this.remoteStoreRestoreService = mock(RemoteStoreRestoreService.class); + } + + public MockGatewayMetaState( + DiscoveryNode localNode, + BigArrays bigArrays, + RemoteClusterStateService remoteClusterStateService, + RemoteStoreRestoreService remoteStoreRestoreService + ) { + this.localNode = localNode; + this.bigArrays = bigArrays; + this.remoteClusterStateService = remoteClusterStateService; + this.remoteStoreRestoreService = remoteStoreRestoreService; } @Override @@ -108,26 +121,6 @@ public void start( } catch (IOException e) { throw new AssertionError(e); } - Supplier remoteClusterStateServiceSupplier = () -> { - if (isRemoteStoreClusterStateEnabled(settings)) { - return new RemoteClusterStateService( - nodeEnvironment.nodeId(), - () -> new RepositoriesService( - settings, - clusterService, - transportService, - Collections.emptyMap(), - Collections.emptyMap(), - transportService.getThreadPool() - ), - settings, - new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), - () -> 0L - ); - } else { - return null; - } - }; start( settings, transportService, @@ -142,8 +135,9 @@ public void start( new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), () -> 0L ), - remoteClusterStateServiceSupplier.get(), - persistedStateRegistry + remoteClusterStateService, + persistedStateRegistry, + remoteStoreRestoreService ); } } From 224accfcb3cd874fe3887a37cde9d88881427136 Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Thu, 7 Sep 2023 09:59:39 -0700 Subject: [PATCH 5/6] Unmute testIndexingWithPrimaryOnBwcNodes (#9742) Signed-off-by: Suraj Singh --- .../src/test/java/org/opensearch/backwards/IndexingIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java index 75083a929b491..686fc78dcec8a 100644 --- a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java +++ b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java @@ -114,7 +114,6 @@ private void printClusterRouting() throws IOException, ParseException { * This test verifies that segment replication does not break when primary shards are on lower OS version. It does this * by verifying replica shards contains same number of documents as primary's. */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/9685") public void testIndexingWithPrimaryOnBwcNodes() throws Exception { if (UPGRADE_FROM_VERSION.before(Version.V_2_4_0)) { logger.info("--> Skip test for version {} where segment replication feature is not available", UPGRADE_FROM_VERSION); From 4ccecd6958e02287549bfcfd30bcc4088d18bf4b Mon Sep 17 00:00:00 2001 From: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> Date: Thu, 7 Sep 2023 23:21:20 +0530 Subject: [PATCH 6/6] changing compat version back to 2.10 (#9899) Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> --- .../cluster/snapshots/restore/RestoreSnapshotRequest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 fcc48b973d49b..256850da0d503 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 @@ -151,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_3_0_0)) { + if (in.getVersion().onOrAfter(Version.V_2_10_0)) { sourceRemoteStoreRepository = in.readOptionalString(); } } @@ -175,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_3_0_0)) { + if (out.getVersion().onOrAfter(Version.V_2_10_0)) { out.writeOptionalString(sourceRemoteStoreRepository); } }