From 98004f824535b20e28f8ce3a5bcc376b93e08314 Mon Sep 17 00:00:00 2001 From: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> Date: Thu, 25 Apr 2024 11:09:28 +0530 Subject: [PATCH] Addressing comments Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com> --- .../routing/allocation/IndexMetadataUpdater.java | 9 +-------- .../remote/RemoteMigrationIndexMetadataUpdater.java | 13 +++++++------ .../RemoteMigrationIndexMetadataUpdaterTests.java | 12 +++++------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/IndexMetadataUpdater.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/IndexMetadataUpdater.java index b5ee8d1f464c5..25be7f92fffb9 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/IndexMetadataUpdater.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/IndexMetadataUpdater.java @@ -60,8 +60,6 @@ import java.util.stream.Collectors; import static org.opensearch.index.remote.RemoteStoreUtils.getRemoteStoreRepoName; -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; /** * Observer that tracks changes made to RoutingNodes in order to update the primary terms and in-sync allocation ids in @@ -185,12 +183,7 @@ public Metadata applyChanges(Metadata oldMetadata, RoutingTable newRoutingTable, logger ); migrationImdUpdater.maybeUpdateRemoteStorePathStrategy(indexMetadataBuilder, index.getName()); - migrationImdUpdater.maybeAddRemoteIndexSettings( - indexMetadataBuilder, - index.getName(), - remoteRepoNames.get(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY), - remoteRepoNames.get(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY) - ); + migrationImdUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, index.getName()); } } diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdater.java b/server/src/main/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdater.java index 3feb919f7506e..fd4dc42df28d6 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdater.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdater.java @@ -32,8 +32,11 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.index.remote.RemoteStoreUtils.getRemoteStoreRepoName; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +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; /** * Utils for checking and mutating cluster state during remote migration @@ -68,18 +71,16 @@ public RemoteMigrationIndexMetadataUpdater( *
* Also appends the requisite Remote Store Path based custom metadata to the existing index metadata */ - public void maybeAddRemoteIndexSettings( - IndexMetadata.Builder indexMetadataBuilder, - String index, - String segmentRepoName, - String tlogRepoName - ) { + public void maybeAddRemoteIndexSettings(IndexMetadata.Builder indexMetadataBuilder, String index) { Settings currentIndexSettings = indexMetadata.getSettings(); if (needsRemoteIndexSettingsUpdate(routingTable.indicesRouting().get(index), discoveryNodes, currentIndexSettings)) { logger.info( "Index {} does not have remote store based index settings but all primary shards and STARTED replica shards have moved to remote enabled nodes. Applying remote store settings to the index", index ); + Map remoteRepoNames = getRemoteStoreRepoName(discoveryNodes); + String segmentRepoName = remoteRepoNames.get(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY); + String tlogRepoName = remoteRepoNames.get(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY); assert Objects.nonNull(segmentRepoName) && Objects.nonNull(tlogRepoName) : "Remote repo names cannot be null"; Settings.Builder indexSettingsBuilder = Settings.builder().put(currentIndexSettings); updateRemoteStoreSettings(indexSettingsBuilder, segmentRepoName, tlogRepoName); diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdaterTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdaterTests.java index 2f89e90a5bc10..d8220c93e4eeb 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdaterTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteMigrationIndexMetadataUpdaterTests.java @@ -38,8 +38,6 @@ import static org.mockito.Mockito.mock; public class RemoteMigrationIndexMetadataUpdaterTests extends OpenSearchTestCase { - private final String tlogRepoName = "test-tlog-repo"; - private final String segmentRepoName = "test-segment-repo"; private final String indexName = "test-index"; public void testMaybeAddRemoteIndexSettingsAllPrimariesAndReplicasOnRemote() throws IOException { @@ -58,7 +56,7 @@ public void testMaybeAddRemoteIndexSettingsAllPrimariesAndReplicasOnRemote() thr metadata.settings(), logger ); - migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName, segmentRepoName, tlogRepoName); + migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName); assertTrue(currentSettingsVersion < indexMetadataBuilder.settingsVersion()); assertRemoteSettingsApplied(indexMetadataBuilder.build()); } @@ -79,7 +77,7 @@ public void testMaybeAddRemoteIndexSettingsDoesNotRunWhenSettingsAlreadyPresent( metadata.settings(), logger ); - migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName, segmentRepoName, tlogRepoName); + migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName); assertEquals(currentSettingsVersion, indexMetadataBuilder.settingsVersion()); } @@ -99,7 +97,7 @@ public void testMaybeAddRemoteIndexSettingsDoesNotUpdateSettingsWhenAllShardsInD metadata.settings(), logger ); - migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName, segmentRepoName, tlogRepoName); + migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName); assertEquals(currentSettingsVersion, indexMetadataBuilder.settingsVersion()); assertDocrepSettingsApplied(indexMetadataBuilder.build()); } @@ -120,7 +118,7 @@ public void testMaybeAddRemoteIndexSettingsUpdatesIndexSettingsWithUnassignedRep metadata.settings(), logger ); - migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName, segmentRepoName, tlogRepoName); + migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName); assertTrue(currentSettingsVersion < indexMetadataBuilder.settingsVersion()); assertRemoteSettingsApplied(indexMetadataBuilder.build()); } @@ -142,7 +140,7 @@ public void testMaybeAddRemoteIndexSettingsDoesNotUpdateIndexSettingsWithRelocat metadata.settings(), logger ); - migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName, segmentRepoName, tlogRepoName); + migrationIndexMetadataUpdater.maybeAddRemoteIndexSettings(indexMetadataBuilder, indexName); assertEquals(currentSettingsVersion, indexMetadataBuilder.settingsVersion()); assertDocrepSettingsApplied(indexMetadataBuilder.build()); }