Skip to content

Commit

Permalink
Incorporate feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Ashish Singh <[email protected]>
  • Loading branch information
@ashking94
ashking94 committed Jul 10, 2023
1 parent 59fe10a commit 3e582ec
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 33 deletions.
43 changes: 11 additions & 32 deletions server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private void assertRecoveryState(
int shardId,
RecoverySource type,
boolean primary,
RecoveryState.Stage stage,
Stage stage,
String sourceNode,
String targetNode
) {
Expand Down Expand Up @@ -288,7 +288,6 @@ private void restoreRecoverySpeed() {
public void testGatewayRecovery() throws Exception {
logger.info("--> start nodes");
String node = internalCluster().startNode();
afterFirstStartNode();

createAndPopulateIndex(INDEX_NAME, 1, SHARD_COUNT, REPLICA_COUNT);

Expand All @@ -311,14 +310,9 @@ public void testGatewayRecovery() throws Exception {
validateIndexRecoveryState(recoveryState.getIndex());
}

protected void afterFirstStartNode() {
// No-op
}

public void testGatewayRecoveryTestActiveOnly() throws Exception {
logger.info("--> start nodes");
internalCluster().startNode();
afterFirstStartNode();

createAndPopulateIndex(INDEX_NAME, 1, SHARD_COUNT, REPLICA_COUNT);

Expand All @@ -335,7 +329,6 @@ public void testGatewayRecoveryTestActiveOnly() throws Exception {

public void testReplicaRecovery() throws Exception {
final String nodeA = internalCluster().startNode();
afterFirstStartNode();
createIndex(
INDEX_NAME,
Settings.builder()
Expand Down Expand Up @@ -407,7 +400,6 @@ public void testReplicaRecovery() throws Exception {
public void testCancelNewShardRecoveryAndUsesExistingShardCopy() throws Exception {
logger.info("--> start node A");
final String nodeA = internalCluster().startNode();
afterFirstStartNode();

logger.info("--> create index on node: {}", nodeA);
createIndex(
Expand Down Expand Up @@ -506,7 +498,6 @@ public Settings onNodeStopped(String nodeName) throws Exception {
public void testRerouteRecovery() throws Exception {
logger.info("--> start node A");
final String nodeA = internalCluster().startNode();
afterFirstStartNode();

logger.info("--> create index on node: {}", nodeA);
ByteSizeValue shardSize = createAndPopulateIndex(INDEX_NAME, 1, SHARD_COUNT, REPLICA_COUNT).getShards()[0].getStats()
Expand Down Expand Up @@ -587,8 +578,12 @@ public void testRerouteRecovery() throws Exception {
.clear()
.setIndices(new CommonStatsFlags(CommonStatsFlags.Flag.Recovery))
.get();
assertThat(statsResponse1.getNodes(), hasSize(2));
for (NodeStats nodeStats : statsResponse1.getNodes()) {
List<NodeStats> dataNodeStats = statsResponse1.getNodes()
.stream()
.filter(nodeStats -> nodeStats.getNode().isDataNode())
.collect(Collectors.toList());
assertThat(dataNodeStats, hasSize(2));
for (NodeStats nodeStats : dataNodeStats) {
final RecoveryStats recoveryStats = nodeStats.getIndices().getRecoveryStats();
if (nodeStats.getNode().getName().equals(nodeA)) {
assertThat(
Expand Down Expand Up @@ -654,7 +649,8 @@ public void testRerouteRecovery() throws Exception {

logger.info("--> start node C");
String nodeC = internalCluster().startNode();
assertFalse(client().admin().cluster().prepareHealth().setWaitForNodes("3").get().isTimedOut());
int nodeCount = internalCluster().getNodeNames().length;
assertFalse(client().admin().cluster().prepareHealth().setWaitForNodes(String.valueOf(nodeCount)).get().isTimedOut());

logger.info("--> slowing down recoveries");
slowDownRecovery(shardSize);
Expand Down Expand Up @@ -743,7 +739,6 @@ protected Matcher<Long> getMatcherForThrottling(long value) {
public void testSnapshotRecovery() throws Exception {
logger.info("--> start node A");
String nodeA = internalCluster().startNode();
afterFirstStartNode();

logger.info("--> create repository");
assertAcked(
Expand Down Expand Up @@ -878,7 +873,6 @@ public void testTransientErrorsDuringRecoveryAreRetried() throws Exception {
.build();
// start a cluster-manager node
internalCluster().startNode(nodeSettings);
afterFirstStartNode();

final String blueNodeName = internalCluster().startNode(
Settings.builder().put("node.attr.color", "blue").put(nodeSettings).build()
Expand Down Expand Up @@ -1081,7 +1075,6 @@ public void testDisconnectsWhileRecovering() throws Exception {
.build();
// start a cluster-manager node
internalCluster().startNode(nodeSettings);
afterFirstStartNode();

final String blueNodeName = internalCluster().startNode(
Settings.builder().put("node.attr.color", "blue").put(nodeSettings).build()
Expand Down Expand Up @@ -1239,7 +1232,6 @@ public void testDisconnectsDuringRecovery() throws Exception {
TimeValue disconnectAfterDelay = TimeValue.timeValueMillis(randomIntBetween(0, 100));
// start a cluster-manager node
String clusterManagerNodeName = internalCluster().startClusterManagerOnlyNode(nodeSettings);
afterFirstStartNode();

final String blueNodeName = internalCluster().startNode(
Settings.builder().put("node.attr.color", "blue").put(nodeSettings).build()
Expand Down Expand Up @@ -1385,7 +1377,6 @@ public void sendRequest(

public void testHistoryRetention() throws Exception {
internalCluster().startNodes(3);
afterFirstStartNode();

final String indexName = "test";
client().admin()
Expand All @@ -1411,10 +1402,10 @@ public void testHistoryRetention() throws Exception {
flush(indexName);
}

String firstNodeToStop = randomFrom(internalCluster().getNodeNames());
String firstNodeToStop = randomFrom(internalCluster().getDataNodeNames());
Settings firstNodeToStopDataPathSettings = internalCluster().dataPathSettings(firstNodeToStop);
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(firstNodeToStop));
String secondNodeToStop = randomFrom(internalCluster().getNodeNames());
String secondNodeToStop = randomFrom(internalCluster().getDataNodeNames());
Settings secondNodeToStopDataPathSettings = internalCluster().dataPathSettings(secondNodeToStop);
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(secondNodeToStop));

Expand Down Expand Up @@ -1452,7 +1443,6 @@ public void testHistoryRetention() throws Exception {

public void testDoNotInfinitelyWaitForMapping() {
internalCluster().ensureAtLeastNumDataNodes(3);
afterFirstStartNode();
createIndex(
"test",
Settings.builder()
Expand Down Expand Up @@ -1499,7 +1489,6 @@ public void testDoNotInfinitelyWaitForMapping() {
public void testOngoingRecoveryAndClusterManagerFailOver() throws Exception {
String indexName = "test";
internalCluster().startNodes(2);
afterFirstStartNode();
String nodeWithPrimary = internalCluster().startDataOnlyNode();
assertAcked(
client().admin()
Expand Down Expand Up @@ -1564,7 +1553,6 @@ public void testOngoingRecoveryAndClusterManagerFailOver() throws Exception {

public void testRecoverLocallyUpToGlobalCheckpoint() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();
List<String> nodes = randomSubsetOf(
2,
StreamSupport.stream(Spliterators.spliterator(clusterService().state().nodes().getDataNodes().values(), 0), false)
Expand Down Expand Up @@ -1672,7 +1660,6 @@ public void testRecoverLocallyUpToGlobalCheckpoint() throws Exception {

public void testUsesFileBasedRecoveryIfRetentionLeaseMissing() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();

String indexName = "test-index";
createIndex(
Expand Down Expand Up @@ -1744,7 +1731,6 @@ public Settings onNodeStopped(String nodeName) throws Exception {

public void testUsesFileBasedRecoveryIfRetentionLeaseAheadOfGlobalCheckpoint() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();

String indexName = "test-index";
createIndex(
Expand Down Expand Up @@ -1830,7 +1816,6 @@ public Settings onNodeStopped(String nodeName) throws Exception {

public void testUsesFileBasedRecoveryIfOperationsBasedRecoveryWouldBeUnreasonable() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();

String indexName = "test-index";
final Settings.Builder settings = Settings.builder()
Expand Down Expand Up @@ -1976,7 +1961,6 @@ public Settings onNodeStopped(String nodeName) throws Exception {

public void testDoesNotCopyOperationsInSafeCommit() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();

String indexName = "test-index";
createIndex(
Expand Down Expand Up @@ -2061,7 +2045,6 @@ public TokenStream create(TokenStream tokenStream) {

public void testRepeatedRecovery() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
afterFirstStartNode();

// Ensures that you can remove a replica and then add it back again without any ill effects, even if it's allocated back to the
// node that held it previously, in case that node hasn't completely cleared it up.
Expand Down Expand Up @@ -2126,7 +2109,6 @@ public void testRepeatedRecovery() throws Exception {
public void testAllocateEmptyPrimaryResetsGlobalCheckpoint() throws Exception {
internalCluster().startClusterManagerOnlyNode(Settings.EMPTY);
final List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
afterFirstStartNode();
final Settings randomNodeDataPathSettings = internalCluster().dataPathSettings(randomFrom(dataNodes));
final String indexName = "test";
assertAcked(
Expand Down Expand Up @@ -2167,7 +2149,6 @@ public void testAllocateEmptyPrimaryResetsGlobalCheckpoint() throws Exception {

public void testPeerRecoveryTrimsLocalTranslog() throws Exception {
internalCluster().startNode();
afterFirstStartNode();
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
String indexName = "test-index";
createIndex(
Expand Down Expand Up @@ -2227,7 +2208,6 @@ public void testPeerRecoveryTrimsLocalTranslog() throws Exception {

public void testCancelRecoveryWithAutoExpandReplicas() throws Exception {
internalCluster().startClusterManagerOnlyNode();
afterFirstStartNode();
assertAcked(
client().admin()
.indices()
Expand All @@ -2248,7 +2228,6 @@ public void testCancelRecoveryWithAutoExpandReplicas() throws Exception {

public void testReservesBytesDuringPeerRecoveryPhaseOne() throws Exception {
internalCluster().startNode();
afterFirstStartNode();
List<String> dataNodes = internalCluster().startDataOnlyNodes(2);
String indexName = "test-index";
createIndex(
Expand Down
6 changes: 5 additions & 1 deletion server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
Expand Down Expand Up @@ -40,8 +41,11 @@ protected Settings featureFlagSettings() {
.build();
}

@Before
@Override
protected void afterFirstStartNode() {
public void setUp() throws Exception {
super.setUp();
internalCluster().startClusterManagerOnlyNode();
absolutePath = randomRepoPath().toAbsolutePath();
assertAcked(
clusterAdmin().preparePutRepository(REPOSITORY_NAME).setType("fs").setSettings(Settings.builder().put("location", absolutePath))
Expand Down
4 changes: 4 additions & 0 deletions test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,10 @@ synchronized Set<String> allDataNodesButN(int count) {
return set;
}

public Set<String> getDataNodeNames() {
return allDataNodesButN(0);
}

/**
* Returns a set of nodes that have at least one shard of the given index.
*/
Expand Down

0 comments on commit 3e582ec

Please sign in to comment.