Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Lakshya Taragi <[email protected]>
  • Loading branch information
@ltaragi
ltaragi committed Apr 12, 2024
1 parent 74e664f commit fe8d1b5
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 48 deletions.
36 changes: 18 additions & 18 deletions ...ClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,46 +39,46 @@ public class RemoteStoreMigrationSettingsUpdateIT extends ShardAllocationBaseTes
// remote store backed index setting tests

public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() {
logger.info(" --> initialize cluster: gives non remote cluster manager");
logger.info("Initialize cluster: gives non remote cluster manager");
initializeCluster(false);

String indexName1 = "test_index_1";
String indexName2 = "test_index_2";

logger.info(" --> add non-remote node");
logger.info("Add non-remote node");
addRemote = false;
String nonRemoteNodeName = internalCluster().startNode();
internalCluster().validateClusterFormed();
assertNodeInCluster(nonRemoteNodeName);

logger.info(" --> create an index");
logger.info("Create an index");
prepareIndexWithoutReplica(Optional.of(indexName1));

logger.info(" --> verify that non remote-backed index is created");
logger.info("Verify that non remote-backed index is created");
assertNonRemoteStoreBackedIndex(indexName1);

logger.info(" --> set mixed cluster compatibility mode and remote_store direction");
logger.info("Set mixed cluster compatibility mode and remote_store direction");
setClusterMode(MIXED.mode);
setDirection(REMOTE_STORE.direction);

logger.info(" --> add remote node");
logger.info("Add remote node");
addRemote = true;
String remoteNodeName = internalCluster().startNode();
internalCluster().validateClusterFormed();
assertNodeInCluster(remoteNodeName);

logger.info(" --> create another index");
logger.info("Create another index");
prepareIndexWithoutReplica(Optional.of(indexName2));

logger.info(" --> verify that remote backed index is created");
logger.info("Verify that remote backed index is created");
assertRemoteStoreBackedIndex(indexName2);
}

public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() throws Exception {
logger.info(" --> initialize cluster: gives non remote cluster manager");
logger.info("Initialize cluster: gives non remote cluster manager");
initializeCluster(false);

logger.info(" --> add remote and non-remote nodes");
logger.info("Add remote and non-remote nodes");
setClusterMode(MIXED.mode);
addRemote = false;
String nonRemoteNodeName = internalCluster().startNode();
Expand All @@ -88,7 +88,7 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
assertNodeInCluster(nonRemoteNodeName);
assertNodeInCluster(remoteNodeName);

logger.info(" --> create a non remote-backed index");
logger.info("Create a non remote-backed index");
client.admin()
.indices()
.prepareCreate(TEST_INDEX)
Expand All @@ -97,10 +97,10 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
)
.get();

logger.info(" --> verify that non remote stored backed index is created");
logger.info("Verify that non remote stored backed index is created");
assertNonRemoteStoreBackedIndex(TEST_INDEX);

logger.info(" --> create repository");
logger.info("Create repository");
String snapshotName = "test-snapshot";
String snapshotRepoName = "test-restore-snapshot-repo";
Path snapshotRepoNameAbsolutePath = randomRepoPath().toAbsolutePath();
Expand All @@ -110,7 +110,7 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
.setSettings(Settings.builder().put("location", snapshotRepoNameAbsolutePath))
);

logger.info(" --> create snapshot of non remote stored backed index");
logger.info("Create snapshot of non remote stored backed index");

SnapshotInfo snapshotInfo = client().admin()
.cluster()
Expand All @@ -124,19 +124,19 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
assertTrue(snapshotInfo.successfulShards() > 0);
assertEquals(0, snapshotInfo.failedShards());

logger.info(" --> restore index from snapshot under NONE direction");
logger.info("Restore index from snapshot under NONE direction");
String restoredIndexName1 = TEST_INDEX + "-restored1";
restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName1);

logger.info(" --> verify that restored index is non remote-backed");
logger.info("Verify that restored index is non remote-backed");
assertNonRemoteStoreBackedIndex(restoredIndexName1);

logger.info(" --> restore index from snapshot under REMOTE_STORE direction");
logger.info("Restore index from snapshot under REMOTE_STORE direction");
setDirection(REMOTE_STORE.direction);
String restoredIndexName2 = TEST_INDEX + "-restored2";
restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName2);

logger.info(" --> verify that restored index is non remote-backed");
logger.info("Verify that restored index is non remote-backed");
assertRemoteStoreBackedIndex(restoredIndexName2);
}

Expand Down
10 changes: 5 additions & 5 deletions server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -922,16 +922,16 @@ private DiscoveryNode newDiscoveryNode(Map<String, String> attributes) {
);
}

public static final String SEGMENT_REPO = "segment-repo";
public static final String TRANSLOG_REPO = "translog-repo";
private static final String SEGMENT_REPO = "segment-repo";
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";

public static Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) {
private Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) {
return remoteStoreNodeAttributes(segmentRepoName, translogRepoName, CLUSTER_STATE_REPO);
}

private static Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName, String clusterStateRepo) {
private Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName, String clusterStateRepo) {
String segmentRepositoryTypeAttributeKey = String.format(
Locale.getDefault(),
REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
Expand Down Expand Up @@ -968,7 +968,7 @@ private static Map<String, String> remoteStoreNodeAttributes(String segmentRepoN
};
}

private static Map<String, String> remoteStateNodeAttributes(String clusterStateRepo) {
private Map<String, String> remoteStateNodeAttributes(String clusterStateRepo) {
String clusterStateRepositoryTypeAttributeKey = String.format(
Locale.getDefault(),
REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
Expand Down
47 changes: 28 additions & 19 deletions server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.SEGMENT_REPO;
import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.TRANSLOG_REPO;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
Expand All @@ -141,8 +139,6 @@
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.getIndexNumberOfRoutingShards;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.parseV1Mappings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.resolveAndValidateAliases;
import static org.opensearch.cluster.routing.allocation.RemoteStoreMigrationAllocationDeciderTests.getNonRemoteNode;
import static org.opensearch.cluster.routing.allocation.RemoteStoreMigrationAllocationDeciderTests.getRemoteNode;
import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL;
import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
import static org.opensearch.index.IndexSettings.INDEX_REFRESH_INTERVAL_SETTING;
Expand Down Expand Up @@ -1364,8 +1360,8 @@ public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettin
.build();
Settings settings = Settings.builder()
.put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT)
.put(segmentRepositoryNameAttributeKey, SEGMENT_REPO)
.put(translogRepositoryNameAttributeKey, TRANSLOG_REPO)
.put(segmentRepositoryNameAttributeKey, "my-segment-repo-1")
.put(translogRepositoryNameAttributeKey, "my-translog-repo-1")
.build();
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
Expand All @@ -1385,8 +1381,8 @@ public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettin
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
SEGMENT_REPO,
TRANSLOG_REPO,
"my-segment-repo-1",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);
Expand All @@ -1397,8 +1393,8 @@ public void testRemoteStoreImplicitOverrideReplicationTypeToSegmentForRemoteStor
.nodes(DiscoveryNodes.builder().add(getRemoteNode()).build())
.build();
Settings settings = Settings.builder()
.put(segmentRepositoryNameAttributeKey, SEGMENT_REPO)
.put(translogRepositoryNameAttributeKey, TRANSLOG_REPO)
.put(segmentRepositoryNameAttributeKey, "my-segment-repo-1")
.put(translogRepositoryNameAttributeKey, "my-translog-repo-1")
.build();
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
Expand All @@ -1417,8 +1413,8 @@ public void testRemoteStoreImplicitOverrideReplicationTypeToSegmentForRemoteStor
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
SEGMENT_REPO,
TRANSLOG_REPO,
"my-segment-repo-1",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);
Expand All @@ -1430,8 +1426,8 @@ public void testRemoteStoreNoUserOverrideIndexSettings() {
.build();
Settings settings = Settings.builder()
.put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT)
.put(segmentRepositoryNameAttributeKey, SEGMENT_REPO)
.put(translogRepositoryNameAttributeKey, TRANSLOG_REPO)
.put(segmentRepositoryNameAttributeKey, "my-segment-repo-1")
.put(translogRepositoryNameAttributeKey, "my-translog-repo-1")
.build();
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
Settings indexSettings = aggregateIndexSettings(
Expand All @@ -1448,8 +1444,8 @@ public void testRemoteStoreNoUserOverrideIndexSettings() {
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
SEGMENT_REPO,
TRANSLOG_REPO,
"my-segment-repo-1",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);
Expand Down Expand Up @@ -1571,7 +1567,7 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode()
FeatureFlags.initializeFeatureFlags(Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build());

// non-remote cluster manager node
DiscoveryNode nonRemoteClusterManagerNode = getNonRemoteNode();
DiscoveryNode nonRemoteClusterManagerNode = new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT);

DiscoveryNodes discoveryNodes = DiscoveryNodes.builder()
.add(nonRemoteClusterManagerNode)
Expand Down Expand Up @@ -1634,8 +1630,8 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode()
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
SEGMENT_REPO,
TRANSLOG_REPO,
"my-segment-repo-1",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);
Expand Down Expand Up @@ -2248,6 +2244,19 @@ private void verifyRemoteStoreIndexSettings(
assertEquals(translogBufferInterval, INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(indexSettings));
}

private DiscoveryNode getRemoteNode() {
Map<String, String> attributes = new HashMap<>();
attributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1");
attributes.put(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-translog-repo-1");
return new DiscoveryNode(
UUIDs.base64UUID(),
buildNewFakeTransportAddress(),
attributes,
DiscoveryNodeRole.BUILT_IN_ROLES,
Version.CURRENT
);
}

@After
public void shutdown() throws Exception {
clusterSettings = null;
Expand Down
17 changes: 11 additions & 6 deletions ...org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,16 @@
import org.opensearch.node.remotestore.RemoteStoreNodeService;

import java.util.Collections;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.SEGMENT_REPO;
import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.TRANSLOG_REPO;
import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.remoteStoreNodeAttributes;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
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.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL;
import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY;
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.hamcrest.core.Is.is;
Expand Down Expand Up @@ -659,16 +659,21 @@ private ClusterState getInitialClusterState(
}

// get a dummy non-remote node
public static DiscoveryNode getNonRemoteNode() {
private DiscoveryNode getNonRemoteNode() {
return new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT);
}

// get a dummy remote node
public static DiscoveryNode getRemoteNode() {
private DiscoveryNode getRemoteNode() {
Map<String, String> attributes = new HashMap<>();
attributes.put(
REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY,
"REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_VALUE"
);
return new DiscoveryNode(
UUIDs.base64UUID(),
buildNewFakeTransportAddress(),
remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO),
attributes,
DiscoveryNodeRole.BUILT_IN_ROLES,
Version.CURRENT
);
Expand Down

0 comments on commit fe8d1b5

Please sign in to comment.