Skip to content

Commit eb206b3

Browse files
rakshit98Gagan Singh Saini
rakshit98
authored and
Gagan Singh Saini
committed
Disabling _close API invocation during remote migration. (opensearch-project#18327)
Signed-off-by: Rakshit Goyal <irakshg@amazon.com>
1 parent ce87487 commit eb206b3

File tree

4 files changed

+253
-0
lines changed

4 files changed

+253
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1919
- [Security Manager Replacement] Enhance Java Agent to intercept newByteChannel ([#17989](https://github.com/opensearch-project/OpenSearch/pull/17989))
2020
- Enabled Async Shard Batch Fetch by default ([#18139](https://github.com/opensearch-project/OpenSearch/pull/18139))
2121
- Allow to get the search request from the QueryCoordinatorContext ([#17818](https://github.com/opensearch-project/OpenSearch/pull/17818))
22+
- Reject close index requests, while remote store migration is in progress.([#18327](https://github.com/opensearch-project/OpenSearch/pull/18327))
2223
- Improve sort-query performance by retaining the default `totalHitsThreshold` for approximated `match_all` queries ([#18189](https://github.com/opensearch-project/OpenSearch/pull/18189))
2324
- Enable testing for ExtensiblePlugins using classpath plugins ([#16908](https://github.com/opensearch-project/OpenSearch/pull/16908))
2425
- Introduce system generated ingest pipeline ([#17817](https://github.com/opensearch-project/OpenSearch/pull/17817)))

‎server/src/internalClusterTest/java/org/opensearch/remotemigration/CloseIndexMigrationTestCase.java‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.remotemigration;
10+
11+
import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
12+
import org.opensearch.action.admin.indices.close.CloseIndexRequest;
13+
import org.opensearch.action.support.ActiveShardCount;
14+
import org.opensearch.cluster.ClusterState;
15+
import org.opensearch.cluster.metadata.IndexMetadata;
16+
import org.opensearch.cluster.metadata.MetadataIndexStateService;
17+
import org.opensearch.common.settings.Settings;
18+
import org.opensearch.indices.replication.common.ReplicationType;
19+
import org.opensearch.test.OpenSearchIntegTestCase;
20+
21+
import java.util.concurrent.ExecutionException;
22+
23+
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
24+
import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
25+
import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
26+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
27+
28+
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
29+
public class CloseIndexMigrationTestCase extends MigrationBaseTestCase {
30+
private static final String TEST_INDEX = "ind";
31+
private final static String REMOTE_STORE_DIRECTION = "remote_store";
32+
private final static String MIXED_MODE = "mixed";
33+
34+
/*
35+
* This test will verify the close request failure, when cluster mode is mixed
36+
* and migration to remote store is in progress.
37+
* */
38+
public void testFailCloseIndexWhileDocRepToRemoteStoreMigration() {
39+
setAddRemote(false);
40+
// create a docrep cluster
41+
internalCluster().startClusterManagerOnlyNode();
42+
internalCluster().validateClusterFormed();
43+
44+
// add a non-remote node
45+
String nonRemoteNodeName = internalCluster().startDataOnlyNode();
46+
internalCluster().validateClusterFormed();
47+
48+
// create index in cluster
49+
Settings.Builder builder = Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
50+
internalCluster().client()
51+
.admin()
52+
.indices()
53+
.prepareCreate(TEST_INDEX)
54+
.setSettings(
55+
builder.put("index.number_of_shards", 2)
56+
.put("index.number_of_replicas", 0)
57+
.put("index.routing.allocation.include._name", nonRemoteNodeName)
58+
)
59+
.setWaitForActiveShards(ActiveShardCount.ALL)
60+
.execute()
61+
.actionGet();
62+
63+
// set mixed mode
64+
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
65+
updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE));
66+
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
67+
68+
// add a remote node
69+
addRemote = true;
70+
internalCluster().startDataOnlyNode();
71+
internalCluster().validateClusterFormed();
72+
73+
// set remote store migration direction
74+
updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE_DIRECTION));
75+
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
76+
77+
ensureGreen(TEST_INDEX);
78+
79+
// Try closing the index, expecting failure.
80+
ExecutionException ex = expectThrows(
81+
ExecutionException.class,
82+
() -> internalCluster().client().admin().indices().close(new CloseIndexRequest(TEST_INDEX)).get()
83+
84+
);
85+
assertEquals("Cannot close index while remote migration is ongoing", ex.getCause().getMessage());
86+
}
87+
88+
/*
89+
* Verify that index closes if compatibility mode is MIXED, and direction is set to NONE
90+
* */
91+
public void testCloseIndexRequestWithMixedCompatibilityModeAndNoDirection() {
92+
setAddRemote(false);
93+
// create a docrep cluster
94+
internalCluster().startClusterManagerOnlyNode();
95+
internalCluster().validateClusterFormed();
96+
97+
// add a non-remote node
98+
String nonRemoteNodeName = internalCluster().startDataOnlyNode();
99+
internalCluster().validateClusterFormed();
100+
101+
// create index in cluster
102+
Settings.Builder builder = Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT);
103+
internalCluster().client()
104+
.admin()
105+
.indices()
106+
.prepareCreate(TEST_INDEX)
107+
.setSettings(
108+
builder.put("index.number_of_shards", 2)
109+
.put("index.number_of_replicas", 0)
110+
.put("index.routing.allocation.include._name", nonRemoteNodeName)
111+
)
112+
.setWaitForActiveShards(ActiveShardCount.ALL)
113+
.execute()
114+
.actionGet();
115+
116+
// set mixed mode
117+
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
118+
updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE));
119+
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
120+
121+
ensureGreen(TEST_INDEX);
122+
123+
// perform close action
124+
assertAcked(internalCluster().client().admin().indices().close(new CloseIndexRequest(TEST_INDEX)).actionGet());
125+
126+
// verify that index has been closed
127+
final ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
128+
129+
final IndexMetadata indexMetadata = clusterState.metadata().indices().get(TEST_INDEX);
130+
assertEquals(IndexMetadata.State.CLOSE, indexMetadata.getState());
131+
final Settings indexSettings = indexMetadata.getSettings();
132+
assertTrue(indexSettings.hasValue(MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey()));
133+
assertEquals(true, indexSettings.getAsBoolean(MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey(), false));
134+
assertNotNull(clusterState.routingTable().index(TEST_INDEX));
135+
assertTrue(clusterState.blocks().hasIndexBlock(TEST_INDEX, MetadataIndexStateService.INDEX_CLOSED_BLOCK));
136+
137+
}
138+
}

‎server/src/main/java/org/opensearch/action/admin/indices/close/TransportCloseIndexAction.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252
import org.opensearch.core.action.ActionListener;
5353
import org.opensearch.core.common.io.stream.StreamInput;
5454
import org.opensearch.core.index.Index;
55+
import org.opensearch.node.remotestore.RemoteStoreNodeService;
56+
import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode;
57+
import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction;
5558
import org.opensearch.tasks.Task;
5659
import org.opensearch.threadpool.ThreadPool;
5760
import org.opensearch.transport.TransportService;
@@ -130,6 +133,7 @@ protected void doExecute(Task task, CloseIndexRequest request, ActionListener<Cl
130133
+ ": true] to enable it. NOTE: closed indices still consume a significant amount of diskspace"
131134
);
132135
}
136+
validateRemoteMigration();
133137
super.doExecute(task, request, listener);
134138
}
135139

@@ -172,4 +176,17 @@ protected void clusterManagerOperation(
172176
delegatedListener.onFailure(t);
173177
}));
174178
}
179+
180+
/**
181+
* Reject close index request if cluster mode is [MIXED] and migration direction is [RemoteStore]
182+
* @throws IllegalStateException if cluster mode is [MIXED] and migration direction is [RemoteStore]
183+
*/
184+
private void validateRemoteMigration() {
185+
ClusterSettings clusterSettings = clusterService.getClusterSettings();
186+
CompatibilityMode compatibilityMode = clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING);
187+
Direction migrationDirection = clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING);
188+
if (compatibilityMode == CompatibilityMode.MIXED && migrationDirection == Direction.REMOTE_STORE) {
189+
throw new IllegalStateException("Cannot close index while remote migration is ongoing");
190+
}
191+
}
175192
}

‎server/src/test/java/org/opensearch/action/admin/indices/close/TransportCloseIndexActionTests.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.action.admin.indices.close;
10+
11+
import org.opensearch.action.support.ActionFilters;
12+
import org.opensearch.action.support.DestructiveOperations;
13+
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
14+
import org.opensearch.cluster.metadata.MetadataIndexStateService;
15+
import org.opensearch.cluster.service.ClusterService;
16+
import org.opensearch.common.settings.ClusterSettings;
17+
import org.opensearch.common.settings.Settings;
18+
import org.opensearch.test.OpenSearchTestCase;
19+
import org.opensearch.threadpool.TestThreadPool;
20+
import org.opensearch.threadpool.ThreadPool;
21+
import org.opensearch.transport.TransportService;
22+
import org.junit.After;
23+
import org.junit.AfterClass;
24+
import org.junit.Before;
25+
import org.junit.BeforeClass;
26+
27+
import java.util.concurrent.TimeUnit;
28+
29+
import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
30+
import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
31+
import static org.mockito.Mockito.mock;
32+
import static org.mockito.Mockito.when;
33+
34+
public class TransportCloseIndexActionTests extends OpenSearchTestCase {
35+
private static ThreadPool threadPool;
36+
private ClusterService clusterService;
37+
private final static String MIXED_MODE = "mixed";
38+
private final static String REMOTE_STORE_DIRECTION = "remote_store";
39+
private ClusterSettings clusterSettings;
40+
private final static String TEST_IND = "ind";
41+
42+
@BeforeClass
43+
public static void beforeClass() {
44+
threadPool = new TestThreadPool(getTestClass().getName());
45+
}
46+
47+
@Override
48+
@Before
49+
public void setUp() throws Exception {
50+
super.setUp();
51+
52+
clusterService = mock(ClusterService.class);
53+
Settings settings = Settings.builder()
54+
.put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE)
55+
.put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE_DIRECTION)
56+
.build();
57+
ClusterSettings clusSet = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
58+
when(clusterService.getClusterSettings()).thenReturn(clusSet);
59+
clusterSettings = clusterService.getClusterSettings();
60+
}
61+
62+
@Override
63+
@After
64+
public void tearDown() throws Exception {
65+
super.tearDown();
66+
clusterService.close();
67+
}
68+
69+
@AfterClass
70+
public static void afterClass() {
71+
ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS);
72+
threadPool = null;
73+
}
74+
75+
private TransportCloseIndexAction createAction() {
76+
return new TransportCloseIndexAction(
77+
Settings.EMPTY,
78+
mock(TransportService.class),
79+
clusterService,
80+
threadPool,
81+
mock(MetadataIndexStateService.class),
82+
clusterSettings,
83+
mock(ActionFilters.class),
84+
mock(IndexNameExpressionResolver.class),
85+
new DestructiveOperations(Settings.EMPTY, clusterSettings)
86+
);
87+
}
88+
89+
// Test if validateRemoteMigration throws illegal exception when compatibility mode is MIXED and migration Direction is REMOTE_STORE
90+
public void testRemoteValidation() {
91+
TransportCloseIndexAction action = createAction();
92+
93+
Exception e = expectThrows(IllegalStateException.class, () -> action.doExecute(null, new CloseIndexRequest(TEST_IND), null));
94+
95+
assertEquals("Cannot close index while remote migration is ongoing", e.getMessage());
96+
}
97+
}

0 commit comments

Comments
 (0)