From d5a17eda019297236129aa698fb3ea5c9e11faff Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Fri, 19 Apr 2024 14:36:06 +0530 Subject: [PATCH 01/14] Initial commit for RemoteRoutingTableService setup Signed-off-by: Himshikha Gupta --- .../remote/RemoteRoutingTableService.java | 104 ++++++++++++++++++ .../cluster/routing/remote/package-info.java | 10 ++ .../common/settings/ClusterSettings.java | 6 +- .../common/settings/FeatureFlagSettings.java | 3 +- .../opensearch/common/util/FeatureFlags.java | 15 ++- .../remote/RemoteClusterStateService.java | 14 +++ .../remotestore/RemoteStoreNodeAttribute.java | 20 ++++ 7 files changed, 169 insertions(+), 3 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/remote/package-info.java diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java new file mode 100644 index 0000000000000..7778353a75047 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing.remote; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.node.Node; +import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.Repository; +import org.opensearch.repositories.blobstore.BlobStoreRepository; + +import java.io.Closeable; +import java.io.IOException; +import java.util.List; +import java.util.function.Supplier; + +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; + +/** + * A Service which provides APIs to upload and download routing table from remote store. + * + * @opensearch.internal + */ +public class RemoteRoutingTableService implements Closeable { + + /** + * Cluster setting to specify if routing table should be published to remote store + */ + public static final Setting REMOTE_ROUTING_TABLE_ENABLED_SETTING = Setting.boolSetting( + "cluster.remote_store.routing.enabled", + false, + Setting.Property.NodeScope, + Setting.Property.Final + ); + private static final Logger logger = LogManager.getLogger(RemoteRoutingTableService.class); + private final Settings settings; + private final Supplier repositoriesService; + private final ClusterSettings clusterSettings; + private BlobStoreRepository blobStoreRepository; + + public RemoteRoutingTableService(Supplier repositoriesService, + Settings settings, + ClusterSettings clusterSettings) { + assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; + this.repositoriesService = repositoriesService; + this.settings = settings; + this.clusterSettings = clusterSettings; + } + + public List writeFullRoutingTable(ClusterState clusterState, String previousClusterUUID) { + return null; + } + + public List writeIncrementalMetadata( + ClusterState previousClusterState, + ClusterState clusterState, + ClusterMetadataManifest previousManifest) { + return null; + } + + public RoutingTable getLatestRoutingTable(String clusterName, String clusterUUID) { + return null; + } + + public RoutingTable getIncrementalRoutingTable(ClusterState previousClusterState, ClusterMetadataManifest previousManifest, String clusterName, String clusterUUID) { + return null; + } + + private void deleteStaleRoutingTable(String clusterName, String clusterUUID, int manifestsToRetain) { + } + + @Override + public void close() throws IOException { + if (blobStoreRepository != null) { + IOUtils.close(blobStoreRepository); + } + } + + public void start() { + assert isRemoteRoutingTableEnabled(settings) == true : "Remote routing table is not enabled"; + final String remoteStoreRepo = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + assert remoteStoreRepo != null : "Remote routing table repository is not configured"; + final Repository repository = repositoriesService.get().repository(remoteStoreRepo); + assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; + blobStoreRepository = (BlobStoreRepository) repository; + } + +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/package-info.java b/server/src/main/java/org/opensearch/cluster/routing/remote/package-info.java new file mode 100644 index 0000000000000..9fe016e783f20 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/package-info.java @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** Package containing class to perform operations on remote routing table */ +package org.opensearch.cluster.routing.remote; 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 dab0f6bcf1c85..0b101135a00e2 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -76,6 +76,7 @@ import org.opensearch.cluster.routing.allocation.decider.SameShardAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.cluster.service.ClusterApplierService; import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; @@ -732,7 +733,10 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING, - RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING + RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, + + // Remote Routing table settings + RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_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 985eb40711e16..255c1c87f0d89 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -36,6 +36,7 @@ protected FeatureFlagSettings( FeatureFlags.DATETIME_FORMATTER_CACHING_SETTING, FeatureFlags.WRITEABLE_REMOTE_INDEX_SETTING, FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, - FeatureFlags.PLUGGABLE_CACHE_SETTING + FeatureFlags.PLUGGABLE_CACHE_SETTING, + FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL_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 bdfce72d106d3..abee98470f925 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -67,6 +67,11 @@ public class FeatureFlags { */ public static final String PLUGGABLE_CACHE = "opensearch.experimental.feature.pluggable.caching.enabled"; + /** + * Gates the functionality of remote routing table. + */ + public static final String REMOTE_ROUTING_TABLE_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.routing.enabled"; + public static final Setting REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING = Setting.boolSetting( REMOTE_STORE_MIGRATION_EXPERIMENTAL, false, @@ -93,6 +98,13 @@ public class FeatureFlags { public static final Setting PLUGGABLE_CACHE_SETTING = Setting.boolSetting(PLUGGABLE_CACHE, false, Property.NodeScope); + public static final Setting REMOTE_ROUTING_TABLE_EXPERIMENTAL_SETTING = Setting.boolSetting( + REMOTE_ROUTING_TABLE_EXPERIMENTAL, + false, + Property.NodeScope + ); + + private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, EXTENSIONS_SETTING, @@ -100,7 +112,8 @@ public class FeatureFlags { TELEMETRY_SETTING, DATETIME_FORMATTER_CACHING_SETTING, WRITEABLE_REMOTE_INDEX_SETTING, - PLUGGABLE_CACHE_SETTING + PLUGGABLE_CACHE_SETTING, + REMOTE_ROUTING_TABLE_EXPERIMENTAL_SETTING ); /** * Should store the settings from opensearch.yml. 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 c892b475d71da..8271b1323e207 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -16,6 +16,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.Nullable; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobMetadata; @@ -64,6 +65,7 @@ import java.util.stream.Collectors; import static org.opensearch.gateway.PersistedClusterStateService.SLOW_WRITE_LOGGING_THRESHOLD; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; /** @@ -162,6 +164,7 @@ public class RemoteClusterStateService implements Closeable { private final ThreadPool threadpool; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; + private RemoteRoutingTableService remoteRoutingTableService; private volatile TimeValue slowWriteLoggingThreshold; private volatile TimeValue indexMetadataUploadTimeout; @@ -206,6 +209,11 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, this::setGlobalMetadataUploadTimeout); clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); + + if(isRemoteRoutingTableEnabled(settings)) { + this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, + settings, clusterSettings); + } } private BlobStoreTransferService getBlobStoreTransferService() { @@ -570,6 +578,9 @@ public void close() throws IOException { if (blobStoreRepository != null) { IOUtils.close(blobStoreRepository); } + if(this.remoteRoutingTableService != null) { + this.remoteRoutingTableService.close(); + } } public void start() { @@ -581,6 +592,9 @@ public void start() { final Repository repository = repositoriesService.get().repository(remoteStoreRepo); assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; blobStoreRepository = (BlobStoreRepository) repository; + if(this.remoteRoutingTableService != null) { + this.remoteRoutingTableService.start(); + } } private ClusterMetadataManifest uploadManifest( diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index a3bfe1195d8cc..f1c2ee72a15e0 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -12,7 +12,9 @@ import org.opensearch.cluster.metadata.RepositoriesMetadata; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.node.Node; import org.opensearch.repositories.blobstore.BlobStoreRepository; @@ -27,6 +29,8 @@ import java.util.Set; import java.util.stream.Collectors; +import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; + /** * This is an abstraction for validating and storing information specific to remote backed storage nodes. * @@ -45,6 +49,8 @@ public class RemoteStoreNodeAttribute { + "." + CryptoMetadata.SETTINGS_KEY; public static final String REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX = "remote_store.repository.%s.settings."; + public static final String REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY = "remote_store.routing.repository"; + private final RepositoriesMetadata repositoriesMetadata; /** @@ -151,6 +157,10 @@ private Set getValidatedRepositoryNames(DiscoveryNode node) { } else if (node.getAttributes().containsKey(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY)) { repositoryNames.add(validateAttributeNonNull(node, REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY)); } + if (node.getAttributes().containsKey(REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)){ + repositoryNames.add(validateAttributeNonNull(node, REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)); + } + return repositoryNames; } @@ -181,6 +191,16 @@ public static boolean isRemoteStoreClusterStateEnabled(Settings settings) { && isRemoteClusterStateAttributePresent(settings); } + public static boolean isRemoteRoutingTableAttributePresent(Settings settings) { + return settings.getByPrefix(Node.NODE_ATTRIBUTES.getKey() + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY) + .isEmpty() == false; + } + + public static boolean isRemoteRoutingTableEnabled(Settings settings) { + return FeatureFlags.isEnabled(REMOTE_ROUTING_TABLE_EXPERIMENTAL) && RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.get(settings) + && isRemoteRoutingTableAttributePresent(settings); + } + public RepositoriesMetadata getRepositoriesMetadata() { return this.repositoriesMetadata; } From 9176015df2852ef2cb7994f3d931f06c8d7abd14 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Mon, 22 Apr 2024 16:43:46 +0530 Subject: [PATCH 02/14] Adds unit test for remote routing setup Signed-off-by: Himshikha Gupta --- .../remote/RemoteClusterStateService.java | 5 + .../RemoteRoutingTableServiceTests.java | 92 +++++++++++++++++++ .../RemoteClusterStateServiceTests.java | 34 ++++++- 3 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java 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 8271b1323e207..69214d6b22a61 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -741,6 +741,11 @@ public TimeValue getMetadataManifestUploadTimeout() { return this.metadataManifestUploadTimeout; } + //Package private for unit test + RemoteRoutingTableService getRemoteRoutingTableService() { + return this.remoteRoutingTableService; + } + static String getManifestFileName(long term, long version, boolean committed) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/manifest/manifest______C/P____ return String.join( diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java new file mode 100644 index 0000000000000..4026a36e76600 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -0,0 +1,92 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing.remote; + +import org.junit.After; +import org.junit.Before; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.repositories.FilterRepository; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.RepositoryMissingException; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.function.Supplier; + +import static org.mockito.Mockito.*; +import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; + +public class RemoteRoutingTableServiceTests extends OpenSearchTestCase { + + private RemoteRoutingTableService remoteRoutingTableService; + private ClusterSettings clusterSettings; + private Supplier repositoriesServiceSupplier; + private RepositoriesService repositoriesService; + private BlobStoreRepository blobStoreRepository; + + @Before + public void setup() { + repositoriesServiceSupplier = mock(Supplier.class); + repositoriesService = mock(RepositoriesService.class); + when(repositoriesServiceSupplier.get()).thenReturn(repositoriesService); + + Settings settings = Settings.builder() + .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), true) + .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .build(); + + clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + blobStoreRepository = mock(BlobStoreRepository.class); + when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); + + Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + + remoteRoutingTableService = new RemoteRoutingTableService( + repositoriesServiceSupplier, + settings, + clusterSettings + ); + } + + @After + public void teardown() throws Exception { + super.tearDown(); + remoteRoutingTableService.close(); + } + + + public void testFailInitializationWhenRemoteRoutingDisabled() { + final Settings settings = Settings.builder().build(); + assertThrows( + AssertionError.class, + () -> new RemoteRoutingTableService( + repositoriesServiceSupplier, + settings, + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ) + ); + } + + public void testFailStartWhenRepositoryNotSet() { + doThrow(new RepositoryMissingException("repository missing")).when(repositoriesService).repository("routing_repository"); + assertThrows(RepositoryMissingException.class, () -> remoteRoutingTableService.start()); + } + + public void testFailStartWhenNotBlobRepository() { + final FilterRepository filterRepository = mock(FilterRepository.class); + when(repositoriesService.repository("routing_repository")).thenReturn(filterRepository); + assertThrows(AssertionError.class, () -> remoteRoutingTableService.start()); + } + +} 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 65477051cdb30..78aa1173bea62 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -17,6 +17,7 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobMetadata; @@ -31,6 +32,7 @@ import org.opensearch.common.network.NetworkModule; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.ParseField; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesArray; @@ -77,6 +79,7 @@ import org.mockito.ArgumentMatchers; import static java.util.stream.Collectors.toList; +import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; import static org.opensearch.gateway.remote.RemoteClusterStateService.DELIMITER; import static org.opensearch.gateway.remote.RemoteClusterStateService.FORMAT_PARAMS; import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_CURRENT_CODEC_VERSION; @@ -84,9 +87,6 @@ import static org.opensearch.gateway.remote.RemoteClusterStateService.MANIFEST_FILE_PREFIX; import static org.opensearch.gateway.remote.RemoteClusterStateService.METADATA_FILE_PREFIX; import static org.opensearch.gateway.remote.RemoteClusterStateService.RETAINED_MANIFESTS; -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; @@ -99,6 +99,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.*; public class RemoteClusterStateServiceTests extends OpenSearchTestCase { @@ -1218,6 +1219,33 @@ public void testGlobalMetadataUploadWaitTimeSetting() { assertEquals(globalMetadataUploadTimeout, remoteClusterStateService.getGlobalMetadataUploadTimeout().seconds()); } + public void testRemoteRoutingTableNotInitializedWhenDisabled() { + assertNull(remoteClusterStateService.getRemoteRoutingTableService()); + } + + public void testRemoteRoutingTableInitializedWhenEnabled() { + Settings newSettings = Settings.builder() + .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), true) + .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "remote_store_repository") + .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .build(); + clusterSettings.applySettings(newSettings); + + Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + + remoteClusterStateService = new RemoteClusterStateService( + "test-node-id", + repositoriesServiceSupplier, + newSettings, + clusterSettings, + () -> 0L, + threadPool + ); + assertNotNull(remoteClusterStateService.getRemoteRoutingTableService()); + } + private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { mockObjectsForGettingPreviousClusterUUID(clusterUUIDsPointers, false, Collections.emptyMap()); } From 7d9badde496038c10955bbffd6f0b010123e6db1 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Tue, 7 May 2024 14:22:20 +0530 Subject: [PATCH 03/14] Updating remote routing table setting name Signed-off-by: Himshikha Gupta --- .../cluster/routing/remote/RemoteRoutingTableService.java | 2 +- .../src/main/java/org/opensearch/common/util/FeatureFlags.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 7778353a75047..7755fb5b74e4e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -41,7 +41,7 @@ public class RemoteRoutingTableService implements Closeable { * Cluster setting to specify if routing table should be published to remote store */ public static final Setting REMOTE_ROUTING_TABLE_ENABLED_SETTING = Setting.boolSetting( - "cluster.remote_store.routing.enabled", + "cluster.remote_store.routing_table.enabled", false, Setting.Property.NodeScope, Setting.Property.Final 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 abee98470f925..acce722919d90 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -70,7 +70,7 @@ public class FeatureFlags { /** * Gates the functionality of remote routing table. */ - public static final String REMOTE_ROUTING_TABLE_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.routing.enabled"; + public static final String REMOTE_ROUTING_TABLE_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.routing_table.enabled"; public static final Setting REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING = Setting.boolSetting( REMOTE_STORE_MIGRATION_EXPERIMENTAL, From ad7ecf5d376254756ca3d7fb61c925dac2f7cd22 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Tue, 7 May 2024 14:28:20 +0530 Subject: [PATCH 04/14] Update remote routing table repo setting name Signed-off-by: Himshikha Gupta --- .../opensearch/node/remotestore/RemoteStoreNodeAttribute.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index f1c2ee72a15e0..542f8143a3af6 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -49,7 +49,7 @@ public class RemoteStoreNodeAttribute { + "." + CryptoMetadata.SETTINGS_KEY; public static final String REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX = "remote_store.repository.%s.settings."; - public static final String REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY = "remote_store.routing.repository"; + public static final String REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY = "remote_store.routing_table.repository"; private final RepositoriesMetadata repositoriesMetadata; From 2107372af0bf6e56c5a6fcc1f5964ffa2f76512c Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Wed, 15 May 2024 11:59:42 +0530 Subject: [PATCH 05/14] spotless apply and fixing gradle failures Signed-off-by: Himshikha Gupta --- .../remote/RemoteRoutingTableService.java | 26 +++++++++++++------ .../opensearch/common/util/FeatureFlags.java | 1 - .../remote/RemoteClusterStateService.java | 11 ++++---- .../remotestore/RemoteStoreNodeAttribute.java | 5 ++-- .../RemoteRoutingTableServiceTests.java | 15 +++++------ .../RemoteClusterStateServiceTests.java | 10 ++++--- 6 files changed, 39 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 7755fb5b74e4e..5392a07b8e8b3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -52,23 +52,29 @@ public class RemoteRoutingTableService implements Closeable { private final ClusterSettings clusterSettings; private BlobStoreRepository blobStoreRepository; - public RemoteRoutingTableService(Supplier repositoriesService, - Settings settings, - ClusterSettings clusterSettings) { + public RemoteRoutingTableService( + Supplier repositoriesService, + Settings settings, + ClusterSettings clusterSettings + ) { assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; this.repositoriesService = repositoriesService; this.settings = settings; this.clusterSettings = clusterSettings; } - public List writeFullRoutingTable(ClusterState clusterState, String previousClusterUUID) { + public List writeFullRoutingTable( + ClusterState clusterState, + String previousClusterUUID + ) { return null; } public List writeIncrementalMetadata( ClusterState previousClusterState, ClusterState clusterState, - ClusterMetadataManifest previousManifest) { + ClusterMetadataManifest previousManifest + ) { return null; } @@ -76,12 +82,16 @@ public RoutingTable getLatestRoutingTable(String clusterName, String clusterUUID return null; } - public RoutingTable getIncrementalRoutingTable(ClusterState previousClusterState, ClusterMetadataManifest previousManifest, String clusterName, String clusterUUID) { + public RoutingTable getIncrementalRoutingTable( + ClusterState previousClusterState, + ClusterMetadataManifest previousManifest, + String clusterName, + String clusterUUID + ) { return null; } - private void deleteStaleRoutingTable(String clusterName, String clusterUUID, int manifestsToRetain) { - } + private void deleteStaleRoutingTable(String clusterName, String clusterUUID, int manifestsToRetain) {} @Override public void close() throws IOException { 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 708b1a7ee39ed..c98cd24af3eeb 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -104,7 +104,6 @@ public class FeatureFlags { Property.NodeScope ); - private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, EXTENSIONS_SETTING, 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 3a6c428a27a04..0d2e03788dbcd 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -214,9 +214,8 @@ public RemoteClusterStateService( this.remoteStateStats = new RemotePersistenceStats(); this.indexMetadataUploadListeners = indexMetadataUploadListeners; - if(isRemoteRoutingTableEnabled(settings)) { - this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, - settings, clusterSettings); + if (isRemoteRoutingTableEnabled(settings)) { + this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, settings, clusterSettings); } } @@ -652,7 +651,7 @@ public void close() throws IOException { if (blobStoreRepository != null) { IOUtils.close(blobStoreRepository); } - if(this.remoteRoutingTableService != null) { + if (this.remoteRoutingTableService != null) { this.remoteRoutingTableService.close(); } } @@ -666,7 +665,7 @@ public void start() { final Repository repository = repositoriesService.get().repository(remoteStoreRepo); assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; blobStoreRepository = (BlobStoreRepository) repository; - if(this.remoteRoutingTableService != null) { + if (this.remoteRoutingTableService != null) { this.remoteRoutingTableService.start(); } } @@ -805,7 +804,7 @@ public TimeValue getMetadataManifestUploadTimeout() { return this.metadataManifestUploadTimeout; } - //Package private for unit test + // Package private for unit test RemoteRoutingTableService getRemoteRoutingTableService() { return this.remoteRoutingTableService; } diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index 091b1903ebd4d..0538f4570006d 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -163,7 +163,7 @@ private Set getValidatedRepositoryNames(DiscoveryNode node) { } else if (node.getAttributes().containsKey(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY)) { repositoryNames.add(validateAttributeNonNull(node, REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY)); } - if (node.getAttributes().containsKey(REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)){ + if (node.getAttributes().containsKey(REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)) { repositoryNames.add(validateAttributeNonNull(node, REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)); } @@ -203,7 +203,8 @@ public static boolean isRemoteRoutingTableAttributePresent(Settings settings) { } public static boolean isRemoteRoutingTableEnabled(Settings settings) { - return FeatureFlags.isEnabled(REMOTE_ROUTING_TABLE_EXPERIMENTAL) && RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.get(settings) + return FeatureFlags.isEnabled(REMOTE_ROUTING_TABLE_EXPERIMENTAL) + && RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.get(settings) && isRemoteRoutingTableAttributePresent(settings); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 4026a36e76600..8f02a61ab4ef4 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -8,8 +8,6 @@ package org.opensearch.cluster.routing.remote; -import org.junit.After; -import org.junit.Before; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; @@ -18,12 +16,16 @@ import org.opensearch.repositories.RepositoryMissingException; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.test.OpenSearchTestCase; +import org.junit.After; +import org.junit.Before; import java.util.function.Supplier; -import static org.mockito.Mockito.*; import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RemoteRoutingTableServiceTests extends OpenSearchTestCase { @@ -52,11 +54,7 @@ public void setup() { Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); - remoteRoutingTableService = new RemoteRoutingTableService( - repositoriesServiceSupplier, - settings, - clusterSettings - ); + remoteRoutingTableService = new RemoteRoutingTableService(repositoriesServiceSupplier, settings, clusterSettings); } @After @@ -65,7 +63,6 @@ public void teardown() throws Exception { remoteRoutingTableService.close(); } - public void testFailInitializationWhenRemoteRoutingDisabled() { final Settings settings = Settings.builder().build(); assertThrows( 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 63a986d779141..0b1dbf0267693 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -88,6 +88,10 @@ import static org.opensearch.gateway.remote.RemoteClusterStateService.MANIFEST_FILE_PREFIX; import static org.opensearch.gateway.remote.RemoteClusterStateService.METADATA_FILE_PREFIX; import static org.opensearch.gateway.remote.RemoteClusterStateService.RETAINED_MANIFESTS; +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.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -100,7 +104,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.*; public class RemoteClusterStateServiceTests extends OpenSearchTestCase { @@ -1224,7 +1227,7 @@ public void testGlobalMetadataUploadWaitTimeSetting() { } public void testRemoteRoutingTableNotInitializedWhenDisabled() { - assertNull(remoteClusterStateService.getRemoteRoutingTableService()); + assertNull(remoteClusterStateService.getRemoteRoutingTableService()); } public void testRemoteRoutingTableInitializedWhenEnabled() { @@ -1245,7 +1248,8 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { newSettings, clusterSettings, () -> 0L, - threadPool + threadPool, + List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings)) ); assertNotNull(remoteClusterStateService.getRemoteRoutingTableService()); } From 89c33bc27cc2e0911f741dec71ff5374c8b5f063 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Thu, 16 May 2024 15:52:33 +0530 Subject: [PATCH 06/14] Remove unused methods Signed-off-by: Himshikha Gupta --- .../remote/RemoteRoutingTableService.java | 34 ------------------- .../remote/RemoteClusterStateService.java | 2 +- 2 files changed, 1 insertion(+), 35 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 5392a07b8e8b3..4bf3727682ecc 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -10,13 +10,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; -import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.repositories.RepositoriesService; @@ -25,7 +22,6 @@ import java.io.Closeable; import java.io.IOException; -import java.util.List; import java.util.function.Supplier; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; @@ -63,36 +59,6 @@ public RemoteRoutingTableService( this.clusterSettings = clusterSettings; } - public List writeFullRoutingTable( - ClusterState clusterState, - String previousClusterUUID - ) { - return null; - } - - public List writeIncrementalMetadata( - ClusterState previousClusterState, - ClusterState clusterState, - ClusterMetadataManifest previousManifest - ) { - return null; - } - - public RoutingTable getLatestRoutingTable(String clusterName, String clusterUUID) { - return null; - } - - public RoutingTable getIncrementalRoutingTable( - ClusterState previousClusterState, - ClusterMetadataManifest previousManifest, - String clusterName, - String clusterUUID - ) { - return null; - } - - private void deleteStaleRoutingTable(String clusterName, String clusterUUID, int manifestsToRetain) {} - @Override public void close() throws IOException { if (blobStoreRepository != null) { 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 1cbad7cb41b33..81294a1f9aeba 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -17,8 +17,8 @@ import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.cluster.metadata.TemplatesMetadata; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.Nullable; import org.opensearch.common.blobstore.BlobContainer; From df6410528d220b479e4d21feec46e86c821a2bb4 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Fri, 24 May 2024 14:24:26 +0530 Subject: [PATCH 07/14] Handle routingTable repo check in JoinTaskExecutor Signed-off-by: Himshikha Gupta --- .../coordination/JoinTaskExecutor.java | 20 ++- .../metadata/RepositoriesMetadata.java | 25 ++++ .../remote/RemoteRoutingTableService.java | 5 +- .../remote/RemoteClusterStateService.java | 2 +- .../remotestore/RemoteStoreNodeAttribute.java | 18 ++- .../coordination/JoinTaskExecutorTests.java | 120 ++++++++++++++++++ .../RemoteRoutingTableServiceTests.java | 5 +- .../node/RemoteStoreNodeAttributeTests.java | 67 ++++++++++ 8 files changed, 249 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index 5475470b81b93..3bea8b1655a99 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -511,11 +511,23 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod assert existingNodes.isEmpty() == false; CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); + List reposToSkip = new ArrayList<>(); + // Skip checking for remote routing table repo if feature is not enabled. + if(!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(metadata.settings())) { + String joiningNodeRepoName = joiningNode.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); + String existingNodeRepoName = existingNodes.get(0).getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); + if(joiningNodeRepoName != null){ + reposToSkip.add(joiningNodeRepoName); + } + if(existingNodeRepoName != null) { + reposToSkip.add(existingNodeRepoName); + } + } if (STRICT.equals(remoteStoreCompatibilityMode)) { DiscoveryNode existingNode = existingNodes.get(0); if (joiningNode.isRemoteStoreNode()) { - ensureRemoteStoreNodesCompatibility(joiningNode, existingNode); + ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, reposToSkip); } else { if (existingNode.isRemoteStoreNode()) { throw new IllegalStateException( @@ -538,18 +550,18 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod } if (joiningNode.isRemoteStoreNode()) { Optional remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode)); + remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, reposToSkip)); } } } } - private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode) { + private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode, List reposToSkip) { if (joiningNode.isRemoteStoreNode()) { if (existingNode.isRemoteStoreNode()) { RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode); RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode); - if (existingRemoteStoreNodeAttribute.equals(joiningRemoteStoreNodeAttribute) == false) { + if (existingRemoteStoreNodeAttribute.equalsWithRepoSkip(joiningRemoteStoreNodeAttribute, reposToSkip) == false) { throw new IllegalStateException( "a remote store node [" + joiningNode diff --git a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java index 9b52bdd1b16c5..3646caed9c501 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java @@ -53,6 +53,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; +import java.util.stream.Collectors; import static org.opensearch.repositories.blobstore.BlobStoreRepository.SYSTEM_REPOSITORY_SETTING; @@ -164,6 +165,30 @@ public boolean equalsIgnoreGenerations(@Nullable RepositoriesMetadata other) { return true; } + /** + * Checks if this instance and the give instance share the same repositories, with option to skip checking for a list of repos. + * This will support + * @param other other repositories metadata + * @param reposToSkip list of repos to skip check for equality + * @return {@code true} iff both instances contain the same repositories apart from differences in generations, not including repos provided in reposToSkip. + */ + public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadata other, List reposToSkip) { + if (other == null) { + return false; + } + List currentRepositories = repositories.stream().filter(repo-> !reposToSkip.contains(repo.name())).collect(Collectors.toList()); + List otherRepositories = other.repositories.stream().filter(repo-> !reposToSkip.contains(repo.name())).collect(Collectors.toList()); + if (otherRepositories.size() != currentRepositories.size()) { + return false; + } + for (int i = 0; i < currentRepositories.size(); i++) { + if (currentRepositories.get(i).equalsIgnoreGenerations(otherRepositories.get(i)) == false) { + return false; + } + } + return true; + } + @Override public int hashCode() { return repositories.hashCode(); diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 4bf3727682ecc..b0c220b45c9e4 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -45,18 +45,15 @@ public class RemoteRoutingTableService implements Closeable { private static final Logger logger = LogManager.getLogger(RemoteRoutingTableService.class); private final Settings settings; private final Supplier repositoriesService; - private final ClusterSettings clusterSettings; private BlobStoreRepository blobStoreRepository; public RemoteRoutingTableService( Supplier repositoriesService, - Settings settings, - ClusterSettings clusterSettings + Settings settings ) { assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; this.repositoriesService = repositoriesService; this.settings = settings; - this.clusterSettings = clusterSettings; } @Override 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 81294a1f9aeba..86e608b5eb787 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -258,7 +258,7 @@ public RemoteClusterStateService( this.indexMetadataUploadListeners = indexMetadataUploadListeners; if (isRemoteRoutingTableEnabled(settings)) { - this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, settings, clusterSettings); + this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, settings); } } diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index 0538f4570006d..648f820835554 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -138,6 +138,7 @@ private RepositoryMetadata buildRepositoryMetadata(DiscoveryNode node, String na // Repository metadata built here will always be for a system repository. settings.put(BlobStoreRepository.SYSTEM_REPOSITORY_SETTING.getKey(), true); + settings.put("repositories.fs.location", "./fsdata"); return new RepositoryMetadata(name, type, settings.build(), cryptoMetadata); } @@ -197,7 +198,7 @@ public static boolean isRemoteStoreClusterStateEnabled(Settings settings) { && isRemoteClusterStateAttributePresent(settings); } - public static boolean isRemoteRoutingTableAttributePresent(Settings settings) { + private static boolean isRemoteRoutingTableAttributePresent(Settings settings) { return settings.getByPrefix(Node.NODE_ATTRIBUTES.getKey() + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY) .isEmpty() == false; } @@ -252,6 +253,21 @@ public int hashCode() { return hashCode; } + /** + * Checks if 2 instances are equal, with option to skip check for a list of repos. + * + * @param o other instance + * @param reposToSkip list of repos to skip check for equality + * @return {@code true} iff both instances are equal, not including the repositories in both instances if they are part of reposToSkip. + */ + public boolean equalsWithRepoSkip(Object o, List reposToSkip) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + RemoteStoreNodeAttribute that = (RemoteStoreNodeAttribute) o; + return this.getRepositoriesMetadata().equalsIgnoreGenerationsWithRepoSkip(that.getRepositoriesMetadata(), reposToSkip); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 3e343e95f6c4b..c7a480b5060b5 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -49,10 +49,13 @@ import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RerouteService; import org.opensearch.cluster.routing.allocation.AllocationService; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.SetOnce; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.node.Node; +import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.blobstore.BlobStoreRepository; @@ -72,6 +75,7 @@ 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.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; @@ -944,6 +948,99 @@ public void testNodeJoinInMixedMode() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode2, currentNodes, metadata); } + public void testRemoteRoutingTableDisabledNodeJoin() { + + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO)); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + + public void testRemoteRoutingTableDisabledNodeJoinRepoPresentInJoiningNode() { + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); + attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + DiscoveryNode joiningNode = newDiscoveryNode(attr); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + + public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { + Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); + attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + attr, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + final Settings settings = Settings.builder() + .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") + .put(Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, ROUTING_TABLE_REPO) + .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") + .build(); + final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .metadata(metadata) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO)); + assertThrows( + IllegalStateException.class, + () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()) + ); + } + + public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInBothNode() { + Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); + attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + attr, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + final Settings settings = Settings.builder() + .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") + .put(Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, ROUTING_TABLE_REPO) + .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") + .build(); + final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .metadata(metadata) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(attr); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + private void validateRepositoryMetadata(ClusterState updatedState, DiscoveryNode existingNode, int expectedRepositories) throws Exception { @@ -985,6 +1082,7 @@ private DiscoveryNode newDiscoveryNode(Map attributes) { private static final String TRANSLOG_REPO = "translog-repo"; private static final String CLUSTER_STATE_REPO = "cluster-state-repo"; private static final String COMMON_REPO = "remote-repo"; + private static final String ROUTING_TABLE_REPO = "routing-table-repo"; private Map remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) { return remoteStoreNodeAttributes(segmentRepoName, translogRepoName, CLUSTER_STATE_REPO); @@ -1049,6 +1147,28 @@ private Map remoteStateNodeAttributes(String clusterStateRepo) { }; } + private Map remoteRoutingTableAttributes(String repoName) { + String routingTableRepositoryTypeAttributeKey = String.format( + Locale.getDefault(), + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, + repoName + ); + String routingTableRepositorySettingsAttributeKeyPrefix = String.format( + Locale.getDefault(), + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, + repoName + ); + + return new HashMap<>() { + { + put(REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, repoName); + putIfAbsent(routingTableRepositoryTypeAttributeKey, "s3"); + putIfAbsent(routingTableRepositorySettingsAttributeKeyPrefix + "bucket", "state_bucket"); + putIfAbsent(routingTableRepositorySettingsAttributeKeyPrefix + "base_path", "/state/path"); + } + }; + } + private void validateAttributes(Map remoteStoreNodeAttributes, ClusterState currentState, DiscoveryNode existingNode) { DiscoveryNode joiningNode = newDiscoveryNode(remoteStoreNodeAttributes); Exception e = assertThrows( diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 8f02a61ab4ef4..c2f12a3e518b0 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -54,7 +54,7 @@ public void setup() { Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); - remoteRoutingTableService = new RemoteRoutingTableService(repositoriesServiceSupplier, settings, clusterSettings); + remoteRoutingTableService = new RemoteRoutingTableService(repositoriesServiceSupplier, settings); } @After @@ -69,8 +69,7 @@ public void testFailInitializationWhenRemoteRoutingDisabled() { AssertionError.class, () -> new RemoteRoutingTableService( repositoriesServiceSupplier, - settings, - new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + settings ) ); } diff --git a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java index c4ba271d27ae9..d6a1514723441 100644 --- a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java +++ b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java @@ -19,6 +19,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.Arrays; import java.util.Locale; import java.util.Map; @@ -30,6 +31,7 @@ import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; public class RemoteStoreNodeAttributeTests extends OpenSearchTestCase { @@ -148,4 +150,69 @@ public void testNoCryptoMetadata() throws UnknownHostException { RepositoryMetadata repositoryMetadata = remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().get(0); assertNull(repositoryMetadata.cryptoMetadata()); } + + public void testEqualsWithRepoSkip() throws UnknownHostException { + String repoName = "remote-store-A"; + String repoTypeSettingKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, repoName); + String repoSettingsKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, repoName); + Map attr = Map.of( + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + repoTypeSettingKey, + "s3", + repoSettingsKey, + "abc", + repoSettingsKey + "base_path", + "xyz" + ); + DiscoveryNode node = new DiscoveryNode( + "C", + new TransportAddress(InetAddress.getByName("localhost"), 9876), + attr, + emptySet(), + Version.CURRENT + ); + + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); + + String routingTableRepoName = "remote-store-B"; + String routingTableRepoTypeSettingKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, routingTableRepoName); + String routingTableRepoSettingsKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, routingTableRepoName); + + Map attr2 = Map.of( + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + repoName, + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, + routingTableRepoName, + repoTypeSettingKey, + "s3", + repoSettingsKey, + "abc", + repoSettingsKey + "base_path", + "xyz", + routingTableRepoTypeSettingKey, + "s3", + routingTableRepoSettingsKey, + "xyz" + ); + DiscoveryNode node2 = new DiscoveryNode( + "C", + new TransportAddress(InetAddress.getByName("localhost"), 9876), + attr2, + emptySet(), + Version.CURRENT + ); + RemoteStoreNodeAttribute remoteStoreNodeAttribute2 = new RemoteStoreNodeAttribute(node2); + + assertFalse(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, Arrays.asList())); + assertTrue(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, Arrays.asList(routingTableRepoName))); + } } From 59a0e90f6120619fe160bce9a16a5240525ab84f Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Fri, 24 May 2024 14:42:24 +0530 Subject: [PATCH 08/14] spotless fix Signed-off-by: Himshikha Gupta --- .../coordination/JoinTaskExecutor.java | 19 +++++++++++++------ .../metadata/RepositoriesMetadata.java | 10 +++++++--- .../remote/RemoteRoutingTableService.java | 6 +----- .../coordination/JoinTaskExecutorTests.java | 10 ++++++++-- .../RemoteRoutingTableServiceTests.java | 8 +------- .../node/RemoteStoreNodeAttributeTests.java | 14 +++++++++++--- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index 3bea8b1655a99..a111252be703a 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -513,13 +513,16 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); List reposToSkip = new ArrayList<>(); // Skip checking for remote routing table repo if feature is not enabled. - if(!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(metadata.settings())) { - String joiningNodeRepoName = joiningNode.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); - String existingNodeRepoName = existingNodes.get(0).getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); - if(joiningNodeRepoName != null){ + if (!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(metadata.settings())) { + String joiningNodeRepoName = joiningNode.getAttributes() + .get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); + String existingNodeRepoName = existingNodes.get(0) + .getAttributes() + .get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); + if (joiningNodeRepoName != null) { reposToSkip.add(joiningNodeRepoName); } - if(existingNodeRepoName != null) { + if (existingNodeRepoName != null) { reposToSkip.add(existingNodeRepoName); } } @@ -556,7 +559,11 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod } } - private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode, List reposToSkip) { + private static void ensureRemoteStoreNodesCompatibility( + DiscoveryNode joiningNode, + DiscoveryNode existingNode, + List reposToSkip + ) { if (joiningNode.isRemoteStoreNode()) { if (existingNode.isRemoteStoreNode()) { RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java index 3646caed9c501..7356db2a1ff10 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java @@ -172,12 +172,16 @@ public boolean equalsIgnoreGenerations(@Nullable RepositoriesMetadata other) { * @param reposToSkip list of repos to skip check for equality * @return {@code true} iff both instances contain the same repositories apart from differences in generations, not including repos provided in reposToSkip. */ - public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadata other, List reposToSkip) { + public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadata other, List reposToSkip) { if (other == null) { return false; } - List currentRepositories = repositories.stream().filter(repo-> !reposToSkip.contains(repo.name())).collect(Collectors.toList()); - List otherRepositories = other.repositories.stream().filter(repo-> !reposToSkip.contains(repo.name())).collect(Collectors.toList()); + List currentRepositories = repositories.stream() + .filter(repo -> !reposToSkip.contains(repo.name())) + .collect(Collectors.toList()); + List otherRepositories = other.repositories.stream() + .filter(repo -> !reposToSkip.contains(repo.name())) + .collect(Collectors.toList()); if (otherRepositories.size() != currentRepositories.size()) { return false; } diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index b0c220b45c9e4..5d6392be37ba6 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -10,7 +10,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; @@ -47,10 +46,7 @@ public class RemoteRoutingTableService implements Closeable { private final Supplier repositoriesService; private BlobStoreRepository blobStoreRepository; - public RemoteRoutingTableService( - Supplier repositoriesService, - Settings settings - ) { + public RemoteRoutingTableService(Supplier repositoriesService, Settings settings) { assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; this.repositoriesService = repositoriesService; this.settings = settings; diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index c7a480b5060b5..6dd5508bca589 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -996,7 +996,10 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { ); final Settings settings = Settings.builder() .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") - .put(Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, ROUTING_TABLE_REPO) + .put( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, + ROUTING_TABLE_REPO + ) .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") .build(); final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); @@ -1026,7 +1029,10 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInBothNode() { ); final Settings settings = Settings.builder() .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") - .put(Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, ROUTING_TABLE_REPO) + .put( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, + ROUTING_TABLE_REPO + ) .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") .build(); final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index c2f12a3e518b0..040fde227dc4e 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -65,13 +65,7 @@ public void teardown() throws Exception { public void testFailInitializationWhenRemoteRoutingDisabled() { final Settings settings = Settings.builder().build(); - assertThrows( - AssertionError.class, - () -> new RemoteRoutingTableService( - repositoriesServiceSupplier, - settings - ) - ); + assertThrows(AssertionError.class, () -> new RemoteRoutingTableService(repositoriesServiceSupplier, settings)); } public void testFailStartWhenRepositoryNotSet() { diff --git a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java index d6a1514723441..1ee5b5540f07c 100644 --- a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java +++ b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java @@ -29,9 +29,9 @@ import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_CRYPTO_SETTINGS_PREFIX; 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.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; public class RemoteStoreNodeAttributeTests extends OpenSearchTestCase { @@ -180,8 +180,16 @@ public void testEqualsWithRepoSkip() throws UnknownHostException { RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); String routingTableRepoName = "remote-store-B"; - String routingTableRepoTypeSettingKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, routingTableRepoName); - String routingTableRepoSettingsKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, routingTableRepoName); + String routingTableRepoTypeSettingKey = String.format( + Locale.ROOT, + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, + routingTableRepoName + ); + String routingTableRepoSettingsKey = String.format( + Locale.ROOT, + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, + routingTableRepoName + ); Map attr2 = Map.of( REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, From 51333c9a2e2e466e8eead84d49122f9a16efe9ab Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Fri, 24 May 2024 15:55:00 +0530 Subject: [PATCH 09/14] Reverting accidental commit Signed-off-by: Himshikha Gupta --- .../opensearch/node/remotestore/RemoteStoreNodeAttribute.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index 648f820835554..d579d6ed82ede 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -138,7 +138,6 @@ private RepositoryMetadata buildRepositoryMetadata(DiscoveryNode node, String na // Repository metadata built here will always be for a system repository. settings.put(BlobStoreRepository.SYSTEM_REPOSITORY_SETTING.getKey(), true); - settings.put("repositories.fs.location", "./fsdata"); return new RepositoryMetadata(name, type, settings.build(), cryptoMetadata); } From 802e04f7b37e11f91c0214869adbb6faf177b0de Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Tue, 28 May 2024 23:21:04 +0530 Subject: [PATCH 10/14] Move repo check logic to RemoteStoreNodeAttr Signed-off-by: Himshikha Gupta --- .../coordination/JoinTaskExecutor.java | 39 +++++---------- .../metadata/RepositoriesMetadata.java | 2 +- .../common/settings/FeatureFlagSettings.java | 2 +- .../opensearch/common/util/FeatureFlags.java | 8 ++-- .../remote/RemoteClusterStateService.java | 19 ++++---- .../main/java/org/opensearch/node/Node.java | 2 +- .../remotestore/RemoteStoreNodeAttribute.java | 29 ++++++++---- .../remotestore/RemoteStoreNodeService.java | 14 ++++-- .../coordination/JoinTaskExecutorTests.java | 8 ++-- .../RemoteRoutingTableServiceTests.java | 4 +- .../RemoteClusterStateServiceTests.java | 8 ++-- .../node/RemoteStoreNodeAttributeTests.java | 47 ++++++++++++++----- 12 files changed, 106 insertions(+), 76 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index a111252be703a..cb428eb9c3e45 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -187,7 +187,8 @@ public ClusterTasksResult execute(ClusterState currentState, List jo DiscoveryNode dn = remoteDN.orElseGet(() -> (currentNodes.getNodes().values()).stream().findFirst().get()); RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( dn, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE) + currentState.getMetadata().custom(RepositoriesMetadata.TYPE), + currentState.getMetadata().settings() ); assert nodesBuilder.isLocalNodeElectedClusterManager(); @@ -223,7 +224,8 @@ public ClusterTasksResult execute(ClusterState currentState, List jo logger.info("Updating system repository now for remote store"); repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( node, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE) + currentState.getMetadata().custom(RepositoriesMetadata.TYPE), + currentState.getMetadata().settings() ); } @@ -511,26 +513,11 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod assert existingNodes.isEmpty() == false; CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); - List reposToSkip = new ArrayList<>(); - // Skip checking for remote routing table repo if feature is not enabled. - if (!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(metadata.settings())) { - String joiningNodeRepoName = joiningNode.getAttributes() - .get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); - String existingNodeRepoName = existingNodes.get(0) - .getAttributes() - .get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); - if (joiningNodeRepoName != null) { - reposToSkip.add(joiningNodeRepoName); - } - if (existingNodeRepoName != null) { - reposToSkip.add(existingNodeRepoName); - } - } if (STRICT.equals(remoteStoreCompatibilityMode)) { DiscoveryNode existingNode = existingNodes.get(0); if (joiningNode.isRemoteStoreNode()) { - ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, reposToSkip); + ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, metadata.settings()); } else { if (existingNode.isRemoteStoreNode()) { throw new IllegalStateException( @@ -553,22 +540,20 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod } if (joiningNode.isRemoteStoreNode()) { Optional remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, reposToSkip)); + remoteDN.ifPresent( + discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, metadata.settings()) + ); } } } } - private static void ensureRemoteStoreNodesCompatibility( - DiscoveryNode joiningNode, - DiscoveryNode existingNode, - List reposToSkip - ) { + private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode, Settings settings) { if (joiningNode.isRemoteStoreNode()) { if (existingNode.isRemoteStoreNode()) { - RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode); - RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode); - if (existingRemoteStoreNodeAttribute.equalsWithRepoSkip(joiningRemoteStoreNodeAttribute, reposToSkip) == false) { + RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode, settings); + RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode, settings); + if (existingRemoteStoreNodeAttribute.equalsIgnoreOptionalRepo(joiningRemoteStoreNodeAttribute) == false) { throw new IllegalStateException( "a remote store node [" + joiningNode diff --git a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java index 7356db2a1ff10..2c9eac78112d2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java @@ -172,7 +172,7 @@ public boolean equalsIgnoreGenerations(@Nullable RepositoriesMetadata other) { * @param reposToSkip list of repos to skip check for equality * @return {@code true} iff both instances contain the same repositories apart from differences in generations, not including repos provided in reposToSkip. */ - public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadata other, List reposToSkip) { + public boolean equalsIgnoreGenerationsIgnoreOptionalRepos(@Nullable RepositoriesMetadata other, List reposToSkip) { if (other == null) { return false; } 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 ee995c4649d08..238df1bd90113 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -37,6 +37,6 @@ protected FeatureFlagSettings( FeatureFlags.TIERED_REMOTE_INDEX_SETTING, FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, FeatureFlags.PLUGGABLE_CACHE_SETTING, - FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL_SETTING + FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_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 eeafe736f7dd6..82f43921d2d28 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -70,7 +70,7 @@ public class FeatureFlags { /** * Gates the functionality of remote routing table. */ - public static final String REMOTE_ROUTING_TABLE_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.routing_table.enabled"; + public static final String REMOTE_PUBLICATION_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.publication.enabled"; public static final Setting REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING = Setting.boolSetting( REMOTE_STORE_MIGRATION_EXPERIMENTAL, @@ -94,8 +94,8 @@ public class FeatureFlags { public static final Setting PLUGGABLE_CACHE_SETTING = Setting.boolSetting(PLUGGABLE_CACHE, false, Property.NodeScope); - public static final Setting REMOTE_ROUTING_TABLE_EXPERIMENTAL_SETTING = Setting.boolSetting( - REMOTE_ROUTING_TABLE_EXPERIMENTAL, + public static final Setting REMOTE_PUBLICATION_EXPERIMENTAL_SETTING = Setting.boolSetting( + REMOTE_PUBLICATION_EXPERIMENTAL, false, Property.NodeScope ); @@ -108,7 +108,7 @@ public class FeatureFlags { DATETIME_FORMATTER_CACHING_SETTING, TIERED_REMOTE_INDEX_SETTING, PLUGGABLE_CACHE_SETTING, - REMOTE_ROUTING_TABLE_EXPERIMENTAL_SETTING + REMOTE_PUBLICATION_EXPERIMENTAL_SETTING ); /** * Should store the settings from opensearch.yml. 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 86e608b5eb787..03064de793c9d 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -204,7 +204,7 @@ public class RemoteClusterStateService implements Closeable { private final List indexMetadataUploadListeners; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; - private RemoteRoutingTableService remoteRoutingTableService; + private Optional remoteRoutingTableService; private volatile TimeValue slowWriteLoggingThreshold; private volatile TimeValue indexMetadataUploadTimeout; @@ -256,10 +256,9 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); this.indexMetadataUploadListeners = indexMetadataUploadListeners; - - if (isRemoteRoutingTableEnabled(settings)) { - this.remoteRoutingTableService = new RemoteRoutingTableService(repositoriesService, settings); - } + this.remoteRoutingTableService = isRemoteRoutingTableEnabled(settings) + ? Optional.of(new RemoteRoutingTableService(repositoriesService, settings)) + : Optional.empty(); } private BlobStoreTransferService getBlobStoreTransferService() { @@ -756,8 +755,8 @@ public void close() throws IOException { if (blobStoreRepository != null) { IOUtils.close(blobStoreRepository); } - if (this.remoteRoutingTableService != null) { - this.remoteRoutingTableService.close(); + if (this.remoteRoutingTableService.isPresent()) { + this.remoteRoutingTableService.get().close(); } } @@ -770,8 +769,8 @@ public void start() { final Repository repository = repositoriesService.get().repository(remoteStoreRepo); assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; blobStoreRepository = (BlobStoreRepository) repository; - if (this.remoteRoutingTableService != null) { - this.remoteRoutingTableService.start(); + if (this.remoteRoutingTableService.isPresent()) { + this.remoteRoutingTableService.get().start(); } } @@ -947,7 +946,7 @@ public TimeValue getMetadataManifestUploadTimeout() { } // Package private for unit test - RemoteRoutingTableService getRemoteRoutingTableService() { + Optional getRemoteRoutingTableService() { return this.remoteRoutingTableService; } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 9462aeddbd0e4..cad1772fe7c4d 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1976,7 +1976,7 @@ public DiscoveryNode apply(BoundTransportAddress boundTransportAddress) { ); if (isRemoteStoreAttributePresent(settings)) { - remoteStoreNodeService.createAndVerifyRepositories(discoveryNode); + remoteStoreNodeService.createAndVerifyRepositories(discoveryNode, settings); } localNode.set(discoveryNode); diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index d579d6ed82ede..e35229f02a554 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -30,7 +30,7 @@ import java.util.Set; import java.util.stream.Collectors; -import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; +import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; /** * This is an abstraction for validating and storing information specific to remote backed storage nodes. @@ -53,6 +53,7 @@ public class RemoteStoreNodeAttribute { public static final String REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY = "remote_store.routing_table.repository"; private final RepositoriesMetadata repositoriesMetadata; + private final List optionalRepos; public static List SUPPORTED_DATA_REPO_NAME_ATTRIBUTES = List.of( REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, @@ -62,8 +63,18 @@ public class RemoteStoreNodeAttribute { /** * Creates a new {@link RemoteStoreNodeAttribute} */ - public RemoteStoreNodeAttribute(DiscoveryNode node) { + public RemoteStoreNodeAttribute(DiscoveryNode node, Settings settings) { this.repositoriesMetadata = buildRepositoriesMetadata(node); + // For supporting feature launches where new repos are added, we can mark repos to be optional and ensure node joins are not + // impacted due to diff in repos. + this.optionalRepos = new ArrayList<>(); + if (!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(settings)) { + if (node.getAttributes().containsKey(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)) { + optionalRepos.add( + node.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY) + ); + } + } } private String validateAttributeNonNull(DiscoveryNode node, String attributeKey) { @@ -203,7 +214,7 @@ private static boolean isRemoteRoutingTableAttributePresent(Settings settings) { } public static boolean isRemoteRoutingTableEnabled(Settings settings) { - return FeatureFlags.isEnabled(REMOTE_ROUTING_TABLE_EXPERIMENTAL) + return FeatureFlags.isEnabled(REMOTE_PUBLICATION_EXPERIMENTAL) && RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.get(settings) && isRemoteRoutingTableAttributePresent(settings); } @@ -253,18 +264,20 @@ public int hashCode() { } /** - * Checks if 2 instances are equal, with option to skip check for a list of repos. + * Checks if 2 instances are equal, ignoring optionalRepos for both. * * @param o other instance - * @param reposToSkip list of repos to skip check for equality - * @return {@code true} iff both instances are equal, not including the repositories in both instances if they are part of reposToSkip. + * @return {@code true} iff both instances are equal, not including the repositories in both instances if they are part of optionalRepos. */ - public boolean equalsWithRepoSkip(Object o, List reposToSkip) { + public boolean equalsIgnoreOptionalRepo(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; RemoteStoreNodeAttribute that = (RemoteStoreNodeAttribute) o; - return this.getRepositoriesMetadata().equalsIgnoreGenerationsWithRepoSkip(that.getRepositoriesMetadata(), reposToSkip); + List reposToSkip = new ArrayList<>(); + reposToSkip.addAll(this.optionalRepos); + reposToSkip.addAll(that.optionalRepos); + return this.getRepositoriesMetadata().equalsIgnoreGenerationsIgnoreOptionalRepos(that.getRepositoriesMetadata(), reposToSkip); } @Override diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java index 874c9408de6c5..42d8fcde5cbff 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java @@ -17,6 +17,7 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; @@ -144,8 +145,8 @@ public RemoteStoreNodeService(Supplier repositoriesService, * If the creation or verification fails this will close all the repositories this method created and throw * exception. */ - public void createAndVerifyRepositories(DiscoveryNode localNode) { - RemoteStoreNodeAttribute nodeAttribute = new RemoteStoreNodeAttribute(localNode); + public void createAndVerifyRepositories(DiscoveryNode localNode, Settings settings) { + RemoteStoreNodeAttribute nodeAttribute = new RemoteStoreNodeAttribute(localNode, settings); RepositoriesService reposService = repositoriesService.get(); Map repositories = new HashMap<>(); for (RepositoryMetadata repositoryMetadata : nodeAttribute.getRepositoriesMetadata().repositories()) { @@ -177,10 +178,15 @@ public void createAndVerifyRepositories(DiscoveryNode localNode) { * repository is already present in the cluster state and if it's different then the joining remote store backed * node repository metadata an exception will be thrown and the node will not be allowed to join the cluster. */ - public RepositoriesMetadata updateRepositoriesMetadata(DiscoveryNode joiningNode, RepositoriesMetadata existingRepositories) { + public RepositoriesMetadata updateRepositoriesMetadata( + DiscoveryNode joiningNode, + RepositoriesMetadata existingRepositories, + Settings settings + ) { if (joiningNode.isRemoteStoreNode()) { List updatedRepositoryMetadataList = new ArrayList<>(); - List newRepositoryMetadataList = new RemoteStoreNodeAttribute(joiningNode).getRepositoriesMetadata() + List newRepositoryMetadataList = new RemoteStoreNodeAttribute(joiningNode, settings) + .getRepositoriesMetadata() .repositories(); if (existingRepositories == null) { diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 6dd5508bca589..6980e9e670a19 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -205,7 +205,9 @@ public void testUpdatesNodeWithNewRoles() throws Exception { when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]); final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null); final RemoteStoreNodeService remoteStoreNodeService = mock(RemoteStoreNodeService.class); - when(remoteStoreNodeService.updateRepositoriesMetadata(any(), any())).thenReturn(new RepositoriesMetadata(Collections.emptyList())); + when(remoteStoreNodeService.updateRepositoriesMetadata(any(), any(), any())).thenReturn( + new RepositoriesMetadata(Collections.emptyList()) + ); final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor( Settings.EMPTY, @@ -1002,7 +1004,7 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { ) .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") .build(); - final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); Metadata metadata = Metadata.builder().persistentSettings(settings).build(); ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) @@ -1035,7 +1037,7 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInBothNode() { ) .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") .build(); - final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); Metadata metadata = Metadata.builder().persistentSettings(settings).build(); ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 040fde227dc4e..d80a927914025 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -21,7 +21,7 @@ import java.util.function.Supplier; -import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; +import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -51,7 +51,7 @@ public void setup() { blobStoreRepository = mock(BlobStoreRepository.class); when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); - Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); remoteRoutingTableService = new RemoteRoutingTableService(repositoriesServiceSupplier, settings); 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 e6d6441fd8520..9cfb1318f0f8a 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -88,7 +88,7 @@ import org.mockito.ArgumentMatchers; import static java.util.stream.Collectors.toList; -import static org.opensearch.common.util.FeatureFlags.REMOTE_ROUTING_TABLE_EXPERIMENTAL; +import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.RemoteClusterStateService.COORDINATION_METADATA; import static org.opensearch.gateway.remote.RemoteClusterStateService.DELIMITER; import static org.opensearch.gateway.remote.RemoteClusterStateService.FORMAT_PARAMS; @@ -1501,7 +1501,7 @@ public void testGlobalMetadataUploadWaitTimeSetting() { } public void testRemoteRoutingTableNotInitializedWhenDisabled() { - assertNull(remoteClusterStateService.getRemoteRoutingTableService()); + assertFalse(remoteClusterStateService.getRemoteRoutingTableService().isPresent()); } public void testRemoteRoutingTableInitializedWhenEnabled() { @@ -1513,7 +1513,7 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { .build(); clusterSettings.applySettings(newSettings); - Settings nodeSettings = Settings.builder().put(REMOTE_ROUTING_TABLE_EXPERIMENTAL, "true").build(); + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); remoteClusterStateService = new RemoteClusterStateService( @@ -1525,7 +1525,7 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { threadPool, List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings)) ); - assertNotNull(remoteClusterStateService.getRemoteRoutingTableService()); + assertTrue(remoteClusterStateService.getRemoteRoutingTableService().isPresent()); } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { diff --git a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java index 1ee5b5540f07c..eb293287a36c3 100644 --- a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java +++ b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java @@ -12,14 +12,15 @@ import org.opensearch.cluster.metadata.CryptoMetadata; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.Arrays; import java.util.Locale; import java.util.Map; @@ -74,7 +75,7 @@ public void testCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node, Settings.builder().build()); assertEquals(remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().size(), 1); RepositoryMetadata repositoryMetadata = remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().get(0); Settings.Builder settings = Settings.builder(); @@ -116,7 +117,7 @@ public void testInvalidCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - assertThrows(IllegalStateException.class, () -> new RemoteStoreNodeAttribute(node)); + assertThrows(IllegalStateException.class, () -> new RemoteStoreNodeAttribute(node, Settings.builder().build())); } public void testNoCryptoMetadata() throws UnknownHostException { @@ -145,13 +146,14 @@ public void testNoCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node, Settings.builder().build()); assertEquals(remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().size(), 1); RepositoryMetadata repositoryMetadata = remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().get(0); assertNull(repositoryMetadata.cryptoMetadata()); } - public void testEqualsWithRepoSkip() throws UnknownHostException { + public void testEqualsIgnoreOptionalRepo() throws UnknownHostException { + // Node 1 -> Remote Routing disabled, repo not present, Node 2 -> Remote Routing disabled, repo present -> should succeed String repoName = "remote-store-A"; String repoTypeSettingKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, repoName); String repoSettingsKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, repoName); @@ -169,7 +171,7 @@ public void testEqualsWithRepoSkip() throws UnknownHostException { repoSettingsKey + "base_path", "xyz" ); - DiscoveryNode node = new DiscoveryNode( + DiscoveryNode nodeWithoutRoutingTableAttr = new DiscoveryNode( "C", new TransportAddress(InetAddress.getByName("localhost"), 9876), attr, @@ -177,7 +179,10 @@ public void testEqualsWithRepoSkip() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute( + nodeWithoutRoutingTableAttr, + Settings.builder().build() + ); String routingTableRepoName = "remote-store-B"; String routingTableRepoTypeSettingKey = String.format( @@ -211,16 +216,36 @@ public void testEqualsWithRepoSkip() throws UnknownHostException { routingTableRepoSettingsKey, "xyz" ); - DiscoveryNode node2 = new DiscoveryNode( + DiscoveryNode nodeWithRoutingTableAttr = new DiscoveryNode( "C", new TransportAddress(InetAddress.getByName("localhost"), 9876), attr2, emptySet(), Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute2 = new RemoteStoreNodeAttribute(node2); + RemoteStoreNodeAttribute remoteStoreNodeAttribute2 = new RemoteStoreNodeAttribute( + nodeWithRoutingTableAttr, + Settings.builder().build() + ); + assertTrue(remoteStoreNodeAttribute.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute2)); + + // Node 1 -> Remote Routing enabled, repo present, Node 2 -> Remote Routing enabled, repo present -> should succeed + final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + RemoteStoreNodeAttribute remoteStoreNodeAttribute3 = new RemoteStoreNodeAttribute( + nodeWithRoutingTableAttr, + Settings.builder() + .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") + .put(Node.NODE_ATTRIBUTES.getKey() + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, routingTableRepoName) + .build() + ); + assertTrue(remoteStoreNodeAttribute3.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute2)); - assertFalse(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, Arrays.asList())); - assertTrue(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, Arrays.asList(routingTableRepoName))); + // Node 1 -> Remote Routing enabled, repo present, Node 2 -> Remote Routing enabled, repo not present -> should fail + RemoteStoreNodeAttribute remoteStoreNodeAttribute4 = new RemoteStoreNodeAttribute( + nodeWithoutRoutingTableAttr, + Settings.builder().put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true").build() + ); + assertFalse(remoteStoreNodeAttribute3.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute4)); } } From e486165f703d31de3ba6252e863085859b13eba0 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Mon, 3 Jun 2024 16:50:55 +0530 Subject: [PATCH 11/14] Addressing PR comments Signed-off-by: Himshikha Gupta --- .../routing/remote/RemoteRoutingTableService.java | 12 ++++++++---- .../remote/RemoteRoutingTableServiceTests.java | 4 ---- .../remote/RemoteClusterStateServiceTests.java | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 5d6392be37ba6..373cd4518ad8f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; @@ -19,7 +20,6 @@ import org.opensearch.repositories.Repository; import org.opensearch.repositories.blobstore.BlobStoreRepository; -import java.io.Closeable; import java.io.IOException; import java.util.function.Supplier; @@ -30,7 +30,7 @@ * * @opensearch.internal */ -public class RemoteRoutingTableService implements Closeable { +public class RemoteRoutingTableService extends AbstractLifecycleComponent { /** * Cluster setting to specify if routing table should be published to remote store @@ -53,13 +53,14 @@ public RemoteRoutingTableService(Supplier repositoriesServi } @Override - public void close() throws IOException { + protected void doClose() throws IOException { if (blobStoreRepository != null) { IOUtils.close(blobStoreRepository); } } - public void start() { + @Override + protected void doStart() { assert isRemoteRoutingTableEnabled(settings) == true : "Remote routing table is not enabled"; final String remoteStoreRepo = settings.get( Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY @@ -70,4 +71,7 @@ public void start() { blobStoreRepository = (BlobStoreRepository) repository; } + @Override + protected void doStop() {} + } diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index d80a927914025..c3100b1eb4bd3 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -8,7 +8,6 @@ package org.opensearch.cluster.routing.remote; -import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.FilterRepository; @@ -30,7 +29,6 @@ public class RemoteRoutingTableServiceTests extends OpenSearchTestCase { private RemoteRoutingTableService remoteRoutingTableService; - private ClusterSettings clusterSettings; private Supplier repositoriesServiceSupplier; private RepositoriesService repositoriesService; private BlobStoreRepository blobStoreRepository; @@ -46,8 +44,6 @@ public void setup() { .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") .build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - blobStoreRepository = mock(BlobStoreRepository.class); when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); 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 254d1edfcda90..6b059c8aea516 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -1402,7 +1402,7 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { "test-node-id", repositoriesServiceSupplier, newSettings, - clusterSettings, + clusterService, () -> 0L, threadPool, List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings)) From dfc904207e132b725f190c99da8b2db2b503262f Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Tue, 4 Jun 2024 15:59:16 +0530 Subject: [PATCH 12/14] node join based on routing table repo Signed-off-by: Himshikha Gupta --- .../coordination/JoinTaskExecutor.java | 41 +++++++++++----- .../metadata/RepositoriesMetadata.java | 9 +++- .../remote/RemoteRoutingTableService.java | 10 ---- .../common/settings/ClusterSettings.java | 6 +-- .../main/java/org/opensearch/node/Node.java | 2 +- .../remotestore/RemoteStoreNodeAttribute.java | 32 ++++--------- .../remotestore/RemoteStoreNodeService.java | 14 ++---- .../coordination/JoinTaskExecutorTests.java | 41 +++------------- .../RemoteRoutingTableServiceTests.java | 1 - .../RemoteClusterStateServiceTests.java | 2 - .../node/RemoteStoreNodeAttributeTests.java | 47 +++++-------------- 11 files changed, 69 insertions(+), 136 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index cb428eb9c3e45..fb8ab0c250b81 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -187,8 +187,7 @@ public ClusterTasksResult execute(ClusterState currentState, List jo DiscoveryNode dn = remoteDN.orElseGet(() -> (currentNodes.getNodes().values()).stream().findFirst().get()); RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( dn, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE), - currentState.getMetadata().settings() + currentState.getMetadata().custom(RepositoriesMetadata.TYPE) ); assert nodesBuilder.isLocalNodeElectedClusterManager(); @@ -224,8 +223,7 @@ public ClusterTasksResult execute(ClusterState currentState, List jo logger.info("Updating system repository now for remote store"); repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( node, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE), - currentState.getMetadata().settings() + currentState.getMetadata().custom(RepositoriesMetadata.TYPE) ); } @@ -513,11 +511,28 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod assert existingNodes.isEmpty() == false; CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); + + List reposToSkip = new ArrayList<>(1); + Optional remoteRoutingTableNode = existingNodes.stream() + .filter( + node -> node.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY) != null + ) + .findFirst(); + // If none of the existing nodes have routing table repo, then we skip this repo check if present in joining node. + // This ensures a new node with remote routing table repo is able to join the cluster. + if (remoteRoutingTableNode.isEmpty()) { + String joiningNodeRepoName = joiningNode.getAttributes() + .get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY); + if (joiningNodeRepoName != null) { + reposToSkip.add(joiningNodeRepoName); + } + } + if (STRICT.equals(remoteStoreCompatibilityMode)) { DiscoveryNode existingNode = existingNodes.get(0); if (joiningNode.isRemoteStoreNode()) { - ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, metadata.settings()); + ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, reposToSkip); } else { if (existingNode.isRemoteStoreNode()) { throw new IllegalStateException( @@ -540,20 +555,22 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod } if (joiningNode.isRemoteStoreNode()) { Optional remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - remoteDN.ifPresent( - discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, metadata.settings()) - ); + remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, reposToSkip)); } } } } - private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode, Settings settings) { + private static void ensureRemoteStoreNodesCompatibility( + DiscoveryNode joiningNode, + DiscoveryNode existingNode, + List reposToSkip + ) { if (joiningNode.isRemoteStoreNode()) { if (existingNode.isRemoteStoreNode()) { - RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode, settings); - RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode, settings); - if (existingRemoteStoreNodeAttribute.equalsIgnoreOptionalRepo(joiningRemoteStoreNodeAttribute) == false) { + RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode); + RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode); + if (existingRemoteStoreNodeAttribute.equalsWithRepoSkip(joiningRemoteStoreNodeAttribute, reposToSkip) == false) { throw new IllegalStateException( "a remote store node [" + joiningNode diff --git a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java index 2c9eac78112d2..4b3dc7964a87b 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java @@ -51,6 +51,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.EnumSet; import java.util.List; import java.util.stream.Collectors; @@ -172,7 +173,7 @@ public boolean equalsIgnoreGenerations(@Nullable RepositoriesMetadata other) { * @param reposToSkip list of repos to skip check for equality * @return {@code true} iff both instances contain the same repositories apart from differences in generations, not including repos provided in reposToSkip. */ - public boolean equalsIgnoreGenerationsIgnoreOptionalRepos(@Nullable RepositoriesMetadata other, List reposToSkip) { + public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadata other, List reposToSkip) { if (other == null) { return false; } @@ -182,9 +183,15 @@ public boolean equalsIgnoreGenerationsIgnoreOptionalRepos(@Nullable Repositories List otherRepositories = other.repositories.stream() .filter(repo -> !reposToSkip.contains(repo.name())) .collect(Collectors.toList()); + if (otherRepositories.size() != currentRepositories.size()) { return false; } + // Sort repos by name for ordered comparison + Comparator compareByName = (o1, o2) -> o1.name().compareTo(o2.name()); + currentRepositories.sort(compareByName); + otherRepositories.sort(compareByName); + for (int i = 0; i < currentRepositories.size(); i++) { if (currentRepositories.get(i).equalsIgnoreGenerations(otherRepositories.get(i)) == false) { return false; diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index 373cd4518ad8f..ba2208e17df1f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -11,7 +11,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; -import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.node.Node; @@ -32,15 +31,6 @@ */ public class RemoteRoutingTableService extends AbstractLifecycleComponent { - /** - * Cluster setting to specify if routing table should be published to remote store - */ - public static final Setting REMOTE_ROUTING_TABLE_ENABLED_SETTING = Setting.boolSetting( - "cluster.remote_store.routing_table.enabled", - false, - Setting.Property.NodeScope, - Setting.Property.Final - ); private static final Logger logger = LogManager.getLogger(RemoteRoutingTableService.class); private final Settings settings; private final Supplier repositoriesService; 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 3d45cf5c872dc..297fc98764d07 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -77,7 +77,6 @@ import org.opensearch.cluster.routing.allocation.decider.SameShardAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.cluster.service.ClusterApplierService; import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; @@ -744,10 +743,7 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS, - RemoteStoreSettings.CLUSTER_REMOTE_STORE_TRANSLOG_METADATA, - - // Remote Routing table settings - RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING + RemoteStoreSettings.CLUSTER_REMOTE_STORE_TRANSLOG_METADATA ) ) ); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index dc4f29d07abce..49545fa8a0c8b 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1983,7 +1983,7 @@ public DiscoveryNode apply(BoundTransportAddress boundTransportAddress) { ); if (isRemoteStoreAttributePresent(settings)) { - remoteStoreNodeService.createAndVerifyRepositories(discoveryNode, settings); + remoteStoreNodeService.createAndVerifyRepositories(discoveryNode); } localNode.set(discoveryNode); diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index e35229f02a554..a0f745a4270c4 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -12,7 +12,6 @@ import org.opensearch.cluster.metadata.RepositoriesMetadata; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.gateway.remote.RemoteClusterStateService; @@ -53,7 +52,6 @@ public class RemoteStoreNodeAttribute { public static final String REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY = "remote_store.routing_table.repository"; private final RepositoriesMetadata repositoriesMetadata; - private final List optionalRepos; public static List SUPPORTED_DATA_REPO_NAME_ATTRIBUTES = List.of( REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, @@ -63,18 +61,8 @@ public class RemoteStoreNodeAttribute { /** * Creates a new {@link RemoteStoreNodeAttribute} */ - public RemoteStoreNodeAttribute(DiscoveryNode node, Settings settings) { + public RemoteStoreNodeAttribute(DiscoveryNode node) { this.repositoriesMetadata = buildRepositoriesMetadata(node); - // For supporting feature launches where new repos are added, we can mark repos to be optional and ensure node joins are not - // impacted due to diff in repos. - this.optionalRepos = new ArrayList<>(); - if (!RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(settings)) { - if (node.getAttributes().containsKey(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY)) { - optionalRepos.add( - node.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY) - ); - } - } } private String validateAttributeNonNull(DiscoveryNode node, String attributeKey) { @@ -214,9 +202,7 @@ private static boolean isRemoteRoutingTableAttributePresent(Settings settings) { } public static boolean isRemoteRoutingTableEnabled(Settings settings) { - return FeatureFlags.isEnabled(REMOTE_PUBLICATION_EXPERIMENTAL) - && RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.get(settings) - && isRemoteRoutingTableAttributePresent(settings); + return FeatureFlags.isEnabled(REMOTE_PUBLICATION_EXPERIMENTAL) && isRemoteRoutingTableAttributePresent(settings); } public RepositoriesMetadata getRepositoriesMetadata() { @@ -264,20 +250,18 @@ public int hashCode() { } /** - * Checks if 2 instances are equal, ignoring optionalRepos for both. - * + * Checks if 2 instances are equal, with option to skip check for a list of repos. + * * * @param o other instance - * @return {@code true} iff both instances are equal, not including the repositories in both instances if they are part of optionalRepos. + * @param reposToSkip list of repos to skip check for equality + * @return {@code true} iff both instances are equal, not including the repositories in both instances if they are part of reposToSkip. */ - public boolean equalsIgnoreOptionalRepo(Object o) { + public boolean equalsWithRepoSkip(Object o, List reposToSkip) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; RemoteStoreNodeAttribute that = (RemoteStoreNodeAttribute) o; - List reposToSkip = new ArrayList<>(); - reposToSkip.addAll(this.optionalRepos); - reposToSkip.addAll(that.optionalRepos); - return this.getRepositoriesMetadata().equalsIgnoreGenerationsIgnoreOptionalRepos(that.getRepositoriesMetadata(), reposToSkip); + return this.getRepositoriesMetadata().equalsIgnoreGenerationsWithRepoSkip(that.getRepositoriesMetadata(), reposToSkip); } @Override diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java index 42d8fcde5cbff..874c9408de6c5 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java @@ -17,7 +17,6 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; @@ -145,8 +144,8 @@ public RemoteStoreNodeService(Supplier repositoriesService, * If the creation or verification fails this will close all the repositories this method created and throw * exception. */ - public void createAndVerifyRepositories(DiscoveryNode localNode, Settings settings) { - RemoteStoreNodeAttribute nodeAttribute = new RemoteStoreNodeAttribute(localNode, settings); + public void createAndVerifyRepositories(DiscoveryNode localNode) { + RemoteStoreNodeAttribute nodeAttribute = new RemoteStoreNodeAttribute(localNode); RepositoriesService reposService = repositoriesService.get(); Map repositories = new HashMap<>(); for (RepositoryMetadata repositoryMetadata : nodeAttribute.getRepositoriesMetadata().repositories()) { @@ -178,15 +177,10 @@ public void createAndVerifyRepositories(DiscoveryNode localNode, Settings settin * repository is already present in the cluster state and if it's different then the joining remote store backed * node repository metadata an exception will be thrown and the node will not be allowed to join the cluster. */ - public RepositoriesMetadata updateRepositoriesMetadata( - DiscoveryNode joiningNode, - RepositoriesMetadata existingRepositories, - Settings settings - ) { + public RepositoriesMetadata updateRepositoriesMetadata(DiscoveryNode joiningNode, RepositoriesMetadata existingRepositories) { if (joiningNode.isRemoteStoreNode()) { List updatedRepositoryMetadataList = new ArrayList<>(); - List newRepositoryMetadataList = new RemoteStoreNodeAttribute(joiningNode, settings) - .getRepositoriesMetadata() + List newRepositoryMetadataList = new RemoteStoreNodeAttribute(joiningNode).getRepositoriesMetadata() .repositories(); if (existingRepositories == null) { diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 6980e9e670a19..79751b863c6e4 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -49,13 +49,10 @@ import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RerouteService; import org.opensearch.cluster.routing.allocation.AllocationService; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.SetOnce; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; -import org.opensearch.node.Node; -import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.blobstore.BlobStoreRepository; @@ -205,9 +202,7 @@ public void testUpdatesNodeWithNewRoles() throws Exception { when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]); final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null); final RemoteStoreNodeService remoteStoreNodeService = mock(RemoteStoreNodeService.class); - when(remoteStoreNodeService.updateRepositoriesMetadata(any(), any(), any())).thenReturn( - new RepositoriesMetadata(Collections.emptyList()) - ); + when(remoteStoreNodeService.updateRepositoriesMetadata(any(), any())).thenReturn(new RepositoriesMetadata(Collections.emptyList())); final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor( Settings.EMPTY, @@ -950,7 +945,7 @@ public void testNodeJoinInMixedMode() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode2, currentNodes, metadata); } - public void testRemoteRoutingTableDisabledNodeJoin() { + public void testRemoteRoutingTableRepoAbsentNodeJoin() { final DiscoveryNode existingNode = new DiscoveryNode( UUIDs.base64UUID(), @@ -968,7 +963,7 @@ public void testRemoteRoutingTableDisabledNodeJoin() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); } - public void testRemoteRoutingTableDisabledNodeJoinRepoPresentInJoiningNode() { + public void testRemoteRoutingTableNodeJoinRepoPresentInJoiningNode() { final DiscoveryNode existingNode = new DiscoveryNode( UUIDs.base64UUID(), buildNewFakeTransportAddress(), @@ -986,7 +981,7 @@ public void testRemoteRoutingTableDisabledNodeJoinRepoPresentInJoiningNode() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); } - public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { + public void testRemoteRoutingTableNodeJoinRepoPresentInExistingNode() { Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); final DiscoveryNode existingNode = new DiscoveryNode( @@ -996,20 +991,9 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { DiscoveryNodeRole.BUILT_IN_ROLES, Version.CURRENT ); - final Settings settings = Settings.builder() - .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") - .put( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, - ROUTING_TABLE_REPO - ) - .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") - .build(); - final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); - Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) - .metadata(metadata) .build(); DiscoveryNode joiningNode = newDiscoveryNode(remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO)); @@ -1019,7 +1003,7 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInExistingNode() { ); } - public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInBothNode() { + public void testRemoteRoutingTableNodeJoinRepoPresentInBothNode() { Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); final DiscoveryNode existingNode = new DiscoveryNode( @@ -1029,20 +1013,9 @@ public void testRemoteRoutingTableEnabledNodeJoinRepoPresentInBothNode() { DiscoveryNodeRole.BUILT_IN_ROLES, Version.CURRENT ); - final Settings settings = Settings.builder() - .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") - .put( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, - ROUTING_TABLE_REPO - ) - .put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true") - .build(); - final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); - Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) - .metadata(metadata) .build(); DiscoveryNode joiningNode = newDiscoveryNode(attr); diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index c3100b1eb4bd3..9a9cbfa153259 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -40,7 +40,6 @@ public void setup() { when(repositoriesServiceSupplier.get()).thenReturn(repositoriesService); Settings settings = Settings.builder() - .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), true) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") .build(); 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 6b059c8aea516..324023061ac46 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -19,7 +19,6 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.TemplatesMetadata; import org.opensearch.cluster.node.DiscoveryNodes; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; import org.opensearch.common.blobstore.BlobContainer; @@ -1388,7 +1387,6 @@ public void testRemoteRoutingTableNotInitializedWhenDisabled() { public void testRemoteRoutingTableInitializedWhenEnabled() { Settings newSettings = Settings.builder() - .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), true) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "remote_store_repository") .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) diff --git a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java index eb293287a36c3..de7f8977686a7 100644 --- a/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java +++ b/server/src/test/java/org/opensearch/node/RemoteStoreNodeAttributeTests.java @@ -12,15 +12,14 @@ import org.opensearch.cluster.metadata.CryptoMetadata; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -75,7 +74,7 @@ public void testCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node, Settings.builder().build()); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); assertEquals(remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().size(), 1); RepositoryMetadata repositoryMetadata = remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().get(0); Settings.Builder settings = Settings.builder(); @@ -117,7 +116,7 @@ public void testInvalidCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - assertThrows(IllegalStateException.class, () -> new RemoteStoreNodeAttribute(node, Settings.builder().build())); + assertThrows(IllegalStateException.class, () -> new RemoteStoreNodeAttribute(node)); } public void testNoCryptoMetadata() throws UnknownHostException { @@ -146,14 +145,13 @@ public void testNoCryptoMetadata() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node, Settings.builder().build()); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); assertEquals(remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().size(), 1); RepositoryMetadata repositoryMetadata = remoteStoreNodeAttribute.getRepositoriesMetadata().repositories().get(0); assertNull(repositoryMetadata.cryptoMetadata()); } - public void testEqualsIgnoreOptionalRepo() throws UnknownHostException { - // Node 1 -> Remote Routing disabled, repo not present, Node 2 -> Remote Routing disabled, repo present -> should succeed + public void testEqualsWithRepoSkip() throws UnknownHostException { String repoName = "remote-store-A"; String repoTypeSettingKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, repoName); String repoSettingsKey = String.format(Locale.ROOT, REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX, repoName); @@ -171,7 +169,7 @@ public void testEqualsIgnoreOptionalRepo() throws UnknownHostException { repoSettingsKey + "base_path", "xyz" ); - DiscoveryNode nodeWithoutRoutingTableAttr = new DiscoveryNode( + DiscoveryNode node = new DiscoveryNode( "C", new TransportAddress(InetAddress.getByName("localhost"), 9876), attr, @@ -179,10 +177,7 @@ public void testEqualsIgnoreOptionalRepo() throws UnknownHostException { Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute( - nodeWithoutRoutingTableAttr, - Settings.builder().build() - ); + RemoteStoreNodeAttribute remoteStoreNodeAttribute = new RemoteStoreNodeAttribute(node); String routingTableRepoName = "remote-store-B"; String routingTableRepoTypeSettingKey = String.format( @@ -216,36 +211,16 @@ public void testEqualsIgnoreOptionalRepo() throws UnknownHostException { routingTableRepoSettingsKey, "xyz" ); - DiscoveryNode nodeWithRoutingTableAttr = new DiscoveryNode( + DiscoveryNode node2 = new DiscoveryNode( "C", new TransportAddress(InetAddress.getByName("localhost"), 9876), attr2, emptySet(), Version.CURRENT ); - RemoteStoreNodeAttribute remoteStoreNodeAttribute2 = new RemoteStoreNodeAttribute( - nodeWithRoutingTableAttr, - Settings.builder().build() - ); - assertTrue(remoteStoreNodeAttribute.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute2)); - - // Node 1 -> Remote Routing enabled, repo present, Node 2 -> Remote Routing enabled, repo present -> should succeed - final Settings nodeSettings = Settings.builder().put(FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); - RemoteStoreNodeAttribute remoteStoreNodeAttribute3 = new RemoteStoreNodeAttribute( - nodeWithRoutingTableAttr, - Settings.builder() - .put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true") - .put(Node.NODE_ATTRIBUTES.getKey() + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, routingTableRepoName) - .build() - ); - assertTrue(remoteStoreNodeAttribute3.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute2)); + RemoteStoreNodeAttribute remoteStoreNodeAttribute2 = new RemoteStoreNodeAttribute(node2); - // Node 1 -> Remote Routing enabled, repo present, Node 2 -> Remote Routing enabled, repo not present -> should fail - RemoteStoreNodeAttribute remoteStoreNodeAttribute4 = new RemoteStoreNodeAttribute( - nodeWithoutRoutingTableAttr, - Settings.builder().put(RemoteRoutingTableService.REMOTE_ROUTING_TABLE_ENABLED_SETTING.getKey(), "true").build() - ); - assertFalse(remoteStoreNodeAttribute3.equalsIgnoreOptionalRepo(remoteStoreNodeAttribute4)); + assertFalse(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, List.of())); + assertTrue(remoteStoreNodeAttribute.equalsWithRepoSkip(remoteStoreNodeAttribute2, List.of(routingTableRepoName))); } } From 7ad7d05043cb0fce893592d8a5c0a9288b18ab39 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Wed, 5 Jun 2024 12:19:43 +0530 Subject: [PATCH 13/14] rebasing Signed-off-by: Himshikha Gupta --- CHANGELOG.md | 1 + .../org/opensearch/gateway/remote/RemoteClusterStateService.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce11dabb6e9c..1ffb438172f50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add getMetadataFields to MapperService ([#13819](https://github.com/opensearch-project/OpenSearch/pull/13819)) - [Remote State] Add async remote state deletion task running on an interval, configurable by a setting ([#13131](https://github.com/opensearch-project/OpenSearch/pull/13131)) - Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776)) +- Add remote routing table for remote state publication with experimental feature flag ([#13304](https://github.com/opensearch-project/OpenSearch/pull/13304)) ### Dependencies - Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559)) 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 f8d09c5726d99..1a67f3cf25bbf 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -19,7 +19,6 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.TemplatesMetadata; import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; -import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.Nullable; import org.opensearch.common.blobstore.BlobContainer; From 0e5862dc27fccd96b826fe5fa6c618311fcd705a Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Wed, 5 Jun 2024 13:49:01 +0530 Subject: [PATCH 14/14] handle case in node join Signed-off-by: Himshikha Gupta --- .../coordination/JoinTaskExecutor.java | 7 ++- .../coordination/JoinTaskExecutorTests.java | 62 +++++++++++++++++++ .../RemoteClusterStateServiceTests.java | 2 +- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index fb8ab0c250b81..f77a7ffc8ce8e 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -529,8 +529,7 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod } if (STRICT.equals(remoteStoreCompatibilityMode)) { - - DiscoveryNode existingNode = existingNodes.get(0); + DiscoveryNode existingNode = remoteRoutingTableNode.orElseGet(() -> existingNodes.get(0)); if (joiningNode.isRemoteStoreNode()) { ensureRemoteStoreNodesCompatibility(joiningNode, existingNode, reposToSkip); } else { @@ -554,7 +553,9 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod throw new IllegalStateException(reason); } if (joiningNode.isRemoteStoreNode()) { - Optional remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); + Optional remoteDN = remoteRoutingTableNode.isPresent() + ? remoteRoutingTableNode + : existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode, reposToSkip)); } } diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 79751b863c6e4..9cb1bd0b57132 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -1022,6 +1022,68 @@ public void testRemoteRoutingTableNodeJoinRepoPresentInBothNode() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); } + public void testRemoteRoutingTableNodeJoinNodeWithRemoteAndRoutingRepoDifference() { + Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); + attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + attr, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final DiscoveryNode existingNode2 = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode2).add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(attr); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + + public void testRemoteRoutingTableNodeJoinNodeWithRemoteAndRoutingRepoDifferenceMixedMode() { + Map attr = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); + attr.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + attr, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final DiscoveryNode existingNode2 = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final Settings settings = Settings.builder() + .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") + .build(); + final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode2).add(existingNode).localNodeId(existingNode.getId()).build()) + .metadata(metadata) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(attr); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + private void validateRepositoryMetadata(ClusterState updatedState, DiscoveryNode existingNode, int expectedRepositories) throws Exception { 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 e9f717dd3cf3b..aa5996d734d27 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -1518,7 +1518,7 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { "test-node-id", repositoriesServiceSupplier, newSettings, - clusterService, + clusterSettings, () -> 0L, threadPool, List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings))