Skip to content

Commit

Permalink
Adding more assertions to ITs and removing unintended changes
Browse files Browse the repository at this point in the history
  • Loading branch information
shourya035 committed Mar 21, 2024
1 parent 7f36b5d commit 163635b
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,17 @@ public void testRemotePrimaryDocRepAndRemoteReplica() throws Exception {
assertReplicaAndPrimaryConsistencyMultiCopy(shardStatsMap, firstBatch, secondBatch, nodes);
}

/*
Scenario:
- Starts 1 docrep backed data node
- Creates an index with 0 replica
- Starts 1 remote backed data node
- Move primary copy from docrep to remote through _cluster/reroute
- Expands index to 1 replica
- Stops remote enabled node
- Ensure doc count is same after failover
- Index some more docs to ensure working of failed-over primary
*/
public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
internalCluster().setBootstrapClusterManagerNodeIndex(0);
internalCluster().startClusterManagerOnlyNode();
Expand Down Expand Up @@ -268,6 +279,7 @@ public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
long initialPrimaryDocCount = 0;
for (ShardRouting shardRouting : shardStatsMap.keySet()) {
if (shardRouting.primary()) {
assertTrue(nodes.get(shardRouting.currentNodeId()).isRemoteStoreNode());
initialPrimaryDocCount = shardStatsMap.get(shardRouting).getStats().getDocs().getCount();
}
}
Expand All @@ -279,10 +291,11 @@ public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
ensureYellow(FAILOVER_REMOTE_TO_DOCREP);

shardStatsMap = internalCluster().client().admin().indices().prepareStats(FAILOVER_REMOTE_TO_DOCREP).setDocs(true).get().asMap();
DiscoveryNodes discoveryNodes = internalCluster().client().admin().cluster().prepareState().get().getState().getNodes();
nodes = internalCluster().client().admin().cluster().prepareState().get().getState().getNodes();
long primaryDocCountAfterFailover = 0;
for (ShardRouting shardRouting : shardStatsMap.keySet()) {
if (shardRouting.primary()) {
assertFalse(nodes.get(shardRouting.currentNodeId()).isRemoteStoreNode());
primaryDocCountAfterFailover = shardStatsMap.get(shardRouting).getStats().getDocs().getCount();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,14 @@

package org.opensearch.index.remote;

import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.collect.Tuple;
import org.opensearch.node.remotestore.RemoteStoreNodeService;

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;

import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;

/**
* Utils for remote store
*
Expand Down Expand Up @@ -105,19 +100,4 @@ public static void verifyNoMultipleWriters(List<String> mdFiles, Function<String
}
});
}

/**
* Helper method to check the values for the following cluster settings:
* - `remote_store.compatibility_mode` (should be `mixed`)
* - `migration.direction` (should NOT be `none`)
* Used as a source of truth to confirm if a remote store migration is in progress
* @param clusterService Current clusterService ref to fetch cluster settings
*/
public static boolean isMigrationDirectionSet(ClusterService clusterService) {
RemoteStoreNodeService.Direction migrationDirection = clusterService.getClusterSettings().get(MIGRATION_DIRECTION_SETTING);
RemoteStoreNodeService.CompatibilityMode currentCompatiblityMode = clusterService.getClusterSettings()
.get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING);
return currentCompatiblityMode.equals(RemoteStoreNodeService.CompatibilityMode.MIXED) == true
&& migrationDirection.equals(RemoteStoreNodeService.Direction.NONE) == false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ protected Logger getLogger() {
return LOGGER;
}

private final ClusterService clusterService;

@Inject
public RetentionLeaseBackgroundSyncAction(
final Settings settings,
Expand All @@ -113,7 +111,6 @@ public RetentionLeaseBackgroundSyncAction(
Request::new,
ThreadPool.Names.MANAGEMENT
);
this.clusterService = clusterService;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ protected Logger getLogger() {
return LOGGER;
}

private final ClusterService clusterService;

@Inject
public RetentionLeaseSyncAction(
final Settings settings,
Expand Down Expand Up @@ -122,7 +120,6 @@ public RetentionLeaseSyncAction(
systemIndices,
tracer
);
this.clusterService = clusterService;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public class PublishCheckpointAction extends TransportReplicationAction<
protected static Logger logger = LogManager.getLogger(PublishCheckpointAction.class);

private final SegmentReplicationTargetService replicationService;
private final ClusterService clusterService;

@Inject
public PublishCheckpointAction(
Expand All @@ -85,7 +84,6 @@ public PublishCheckpointAction(
ThreadPool.Names.REFRESH
);
this.replicationService = targetService;
this.clusterService = clusterService;
}

@Override
Expand Down

0 comments on commit 163635b

Please sign in to comment.