From d2e9904925637ca14ce75f72a459a18d69e470c1 Mon Sep 17 00:00:00 2001 From: Rakshit Goyal Date: Sun, 18 May 2025 08:11:49 +0530 Subject: [PATCH 1/3] Disabling _close API invocation during remote migration. Signed-off-by: Rakshit Goyal --- .../CloseIndexMigrationTestCase.java | 90 +++++++++++++++++++ .../close/TransportCloseIndexAction.java | 20 +++++ 2 files changed, 110 insertions(+) create mode 100644 server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java new file mode 100644 index 0000000000000..7c7d2d0a91cdc --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java @@ -0,0 +1,90 @@ +/* + * 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.remotemigration; + +import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.admin.indices.close.CloseIndexRequest; +import org.opensearch.action.support.ActiveShardCount; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.indices.replication.common.ReplicationType; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.util.concurrent.ExecutionException; + +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class CloseIndexMigrationTestCase extends MigrationBaseTestCase { + private static final String TEST_INDEX = "ind"; + private final static String REMOTE_STORE_DIRECTION = "remote_store"; + private final static String MIXED_MODE = "mixed"; + + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + } + + /* + * This test will verify the close request failure, when cluster mode is mixed + * and migration to remote store is in progress. + * */ + public void testFailCloseIndexWhileDocRepToRemoteStoreMigration() throws IllegalStateException { + setAddRemote(false); + // create a docrep cluster + internalCluster().startClusterManagerOnlyNode(); + internalCluster().validateClusterFormed(); + + // add a non-remote node + String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + + // create index in cluster + Settings.Builder builder = Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT); + internalCluster().client() + .admin() + .indices() + .prepareCreate(TEST_INDEX) + .setSettings( + builder.put("index.number_of_shards", 2) + .put("index.number_of_replicas", 0) + .put("index.routing.allocation.include._name", nonRemoteNodeName) + ) + .setWaitForActiveShards(ActiveShardCount.ALL) + .execute() + .actionGet(); + + // set mixed mode + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); + updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE)); + assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + // add a remote node + addRemote = true; + internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + + // set remote store migration direction + updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE_DIRECTION)); + assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + ensureGreen(TEST_INDEX); + + // Try closing the index, expecting failure. + ExecutionException ex = expectThrows( + ExecutionException.class, + () -> internalCluster().client().admin().indices().close(new CloseIndexRequest(TEST_INDEX)).get() + + ); + assertEquals("Cannot close index while remote migration is ongoing", ex.getCause().getMessage()); + } +} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java b/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java index 4e56eacb298d3..3118ebae06584 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java @@ -52,6 +52,9 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.index.Index; +import org.opensearch.node.remotestore.RemoteStoreNodeService; +import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode; +import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; @@ -130,6 +133,7 @@ protected void doExecute(Task task, CloseIndexRequest request, ActionListener Date: Sun, 18 May 2025 08:34:02 +0530 Subject: [PATCH 2/3] Adding issue to CHANGELOG.md Signed-off-by: Rakshit Goyal --- CHANGELOG.md | 1 + .../CloseIndexMigrationTestCase.java | 62 ++++++++++-- .../close/TransportCloseIndexAction.java | 7 +- .../close/TransportCloseIndexActionTests.java | 97 +++++++++++++++++++ 4 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 server/src/test/java/org/opensearch/action/admin/indices/close/TransportCloseIndexActionTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index efe350ea83764..36d6dc58238b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Decouple the init of Crypto Plugin and KeyProvider in CryptoRegistry ([18270](https://github.com/opensearch-project/OpenSearch/pull18270))) ### Changed +- Reject close index requests, while remote store migration is in progress.([#18327](https://github.com/opensearch-project/OpenSearch/pull/18327)) ### Dependencies - Bump `com.google.code.gson:gson` from 2.12.1 to 2.13.1 ([#17923](https://github.com/opensearch-project/OpenSearch/pull/17923), [#18266](https://github.com/opensearch-project/OpenSearch/pull/18266)) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java index 7c7d2d0a91cdc..276887a558339 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java @@ -11,8 +11,10 @@ import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.indices.close.CloseIndexRequest; import org.opensearch.action.support.ActiveShardCount; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.MetadataIndexStateService; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchIntegTestCase; @@ -29,16 +31,11 @@ public class CloseIndexMigrationTestCase extends MigrationBaseTestCase { private final static String REMOTE_STORE_DIRECTION = "remote_store"; private final static String MIXED_MODE = "mixed"; - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - } - /* * This test will verify the close request failure, when cluster mode is mixed * and migration to remote store is in progress. * */ - public void testFailCloseIndexWhileDocRepToRemoteStoreMigration() throws IllegalStateException { + public void testFailCloseIndexWhileDocRepToRemoteStoreMigration() { setAddRemote(false); // create a docrep cluster internalCluster().startClusterManagerOnlyNode(); @@ -87,4 +84,55 @@ public void testFailCloseIndexWhileDocRepToRemoteStoreMigration() throws Illegal ); assertEquals("Cannot close index while remote migration is ongoing", ex.getCause().getMessage()); } + + /* + * Verify that index closes if compatibility mode is MIXED, and direction is set to NONE + * */ + public void testCloseIndexRequestWithMixedCompatibilityModeAndNoDirection() { + setAddRemote(false); + // create a docrep cluster + internalCluster().startClusterManagerOnlyNode(); + internalCluster().validateClusterFormed(); + + // add a non-remote node + String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + + // create index in cluster + Settings.Builder builder = Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT); + internalCluster().client() + .admin() + .indices() + .prepareCreate(TEST_INDEX) + .setSettings( + builder.put("index.number_of_shards", 2) + .put("index.number_of_replicas", 0) + .put("index.routing.allocation.include._name", nonRemoteNodeName) + ) + .setWaitForActiveShards(ActiveShardCount.ALL) + .execute() + .actionGet(); + + // set mixed mode + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); + updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE)); + assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + ensureGreen(TEST_INDEX); + + // perform close action + assertAcked(internalCluster().client().admin().indices().close(new CloseIndexRequest(TEST_INDEX)).actionGet()); + + // verify that index has been closed + final ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); + + final IndexMetadata indexMetadata = clusterState.metadata().indices().get(TEST_INDEX); + assertEquals(IndexMetadata.State.CLOSE, indexMetadata.getState()); + final Settings indexSettings = indexMetadata.getSettings(); + assertTrue(indexSettings.hasValue(MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey())); + assertEquals(true, indexSettings.getAsBoolean(MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey(), false)); + assertNotNull(clusterState.routingTable().index(TEST_INDEX)); + assertTrue(clusterState.blocks().hasIndexBlock(TEST_INDEX, MetadataIndexStateService.INDEX_CLOSED_BLOCK)); + + } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java b/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java index 3118ebae06584..4a0822a9bb754 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java @@ -185,11 +185,8 @@ private void validateRemoteMigration() { ClusterSettings clusterSettings = clusterService.getClusterSettings(); CompatibilityMode compatibilityMode = clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING); Direction migrationDirection = clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING); - if (compatibilityMode == CompatibilityMode.MIXED) { - boolean isRemoteMigration = migrationDirection == Direction.REMOTE_STORE; - if (isRemoteMigration) { - throw new IllegalStateException("Cannot close index while remote migration is ongoing"); - } + if (compatibilityMode == CompatibilityMode.MIXED && migrationDirection == Direction.REMOTE_STORE) { + throw new IllegalStateException("Cannot close index while remote migration is ongoing"); } } } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/close/TransportCloseIndexActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/close/TransportCloseIndexActionTests.java new file mode 100644 index 0000000000000..7279f8ead79aa --- /dev/null +++ b/server/src/test/java/org/opensearch/action/admin/indices/close/TransportCloseIndexActionTests.java @@ -0,0 +1,97 @@ +/* + * 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.action.admin.indices.close; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.DestructiveOperations; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.metadata.MetadataIndexStateService; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; + +import java.util.concurrent.TimeUnit; + +import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TransportCloseIndexActionTests extends OpenSearchTestCase { + private static ThreadPool threadPool; + private ClusterService clusterService; + private final static String MIXED_MODE = "mixed"; + private final static String REMOTE_STORE_DIRECTION = "remote_store"; + private ClusterSettings clusterSettings; + private final static String TEST_IND = "ind"; + + @BeforeClass + public static void beforeClass() { + threadPool = new TestThreadPool(getTestClass().getName()); + } + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + + clusterService = mock(ClusterService.class); + Settings settings = Settings.builder() + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE) + .put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE_DIRECTION) + .build(); + ClusterSettings clusSet = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + when(clusterService.getClusterSettings()).thenReturn(clusSet); + clusterSettings = clusterService.getClusterSettings(); + } + + @Override + @After + public void tearDown() throws Exception { + super.tearDown(); + clusterService.close(); + } + + @AfterClass + public static void afterClass() { + ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + threadPool = null; + } + + private TransportCloseIndexAction createAction() { + return new TransportCloseIndexAction( + Settings.EMPTY, + mock(TransportService.class), + clusterService, + threadPool, + mock(MetadataIndexStateService.class), + clusterSettings, + mock(ActionFilters.class), + mock(IndexNameExpressionResolver.class), + new DestructiveOperations(Settings.EMPTY, clusterSettings) + ); + } + + // Test if validateRemoteMigration throws illegal exception when compatibility mode is MIXED and migration Direction is REMOTE_STORE + public void testRemoteValidation() { + TransportCloseIndexAction action = createAction(); + + Exception e = expectThrows(IllegalStateException.class, () -> action.doExecute(null, new CloseIndexRequest(TEST_IND), null)); + + assertEquals("Cannot close index while remote migration is ongoing", e.getMessage()); + } +} From da42ccb8dae12d33e219aa1cdee16b4763274227 Mon Sep 17 00:00:00 2001 From: Gaurav Bafna <85113518+gbbafna@users.noreply.github.com> Date: Tue, 20 May 2025 20:14:06 +0530 Subject: [PATCH 3/3] Update CHANGELOG.md Signed-off-by: Gaurav Bafna <85113518+gbbafna@users.noreply.github.com> --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fca6a4018e09..d75141e99547d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Upgrade crypto kms plugin dependencies for AWS SDK v2.x. ([#18268](https://github.com/opensearch-project/OpenSearch/pull/18268)) - Add support for `matched_fields` with the unified highlighter ([#18164](https://github.com/opensearch-project/OpenSearch/issues/18164)) +### Changed +- Create generic DocRequest to better categorize ActionRequests ([#18269](https://github.com/opensearch-project/OpenSearch/pull/18269))) + ### Dependencies - Update Apache Lucene from 10.1.0 to 10.2.1 ([#17961](https://github.com/opensearch-project/OpenSearch/pull/17961)) - Bump `com.google.code.gson:gson` from 2.12.1 to 2.13.1 ([#17923](https://github.com/opensearch-project/OpenSearch/pull/17923), [#18266](https://github.com/opensearch-project/OpenSearch/pull/18266))