Skip to content

Commit

Permalink
Deprecate public methods and variables that contain 'master' terminol…
Browse files Browse the repository at this point in the history
…ogy in 'test/framework' directory (#3978) (#3987)

* Deprecate methods with 'master' name mainly in class 'InternalTestCluster', 'NodeRoles', and 'AbstractSnapshotIntegTestCase'

* Deprecate variables contain 'master' in the name in class 'InternalTestCluster'

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit 5db75c1)

Co-authored-by: Tianli Feng <[email protected]>

To support inclusive language, the `master` terminology is going to be replaced by `cluster manager` in the code base.
This PR deprecate public and protected methods/variable that contains `master` terminology in the name in `test/framework` directory, and create alternatives.

List of public methods in `test/framework` directory that renamed in this PR:
```
public static String blockMasterFromFinalizingSnapshotOnIndexFile(final String repositoryName) {
public static String blockMasterOnWriteIndexFile(final String repositoryName) {
public static void blockMasterFromDeletingIndexNFile(String repositoryName) {
public static String blockMasterFromFinalizingSnapshotOnSnapFile(final String repositoryName) {
public Client masterClient() {
public Client nonMasterClient() {
public boolean isMasterEligible() {
public synchronized <T> T getCurrentMasterNodeInstance(Class<T> clazz) {
public <T> Iterable<T> getDataOrMasterNodeInstances(Class<T> clazz) {
public <T> T getMasterNodeInstance(Class<T> clazz) {
public synchronized void stopCurrentMasterNode() throws IOException {
public synchronized void stopRandomNonMasterNode() throws IOException {
public String getMasterName() {
public String getMasterName(@nullable String viaNode) {
public List<String> startMasterOnlyNodes(int numNodes) {
public List<String> startMasterOnlyNodes(int numNodes, Settings settings) {
public int numMasterNodes() {
public static Settings masterNode()
public static Settings masterNode(final Settings settings)
public static Settings masterOnlyNode()
public static Settings masterOnlyNode(final Settings settings)
public static Settings nonMasterNode()
public static Settings nonMasterNode(final Settings settings)
public Version masterVersion()
```

List of public variables in `test/framework` directory that renamed in this PR:
(all in `org.opensearch.test.InternalTestCluster`)
```
public static final int DEFAULT_LOW_NUM_MASTER_NODES
public static final int DEFAULT_HIGH_NUM_MASTER_NODES
public static final int REMOVED_MINIMUM_MASTER_NODES
```
 
public that not renamed in this PR:
```
public static MasterService createMasterService(ThreadPool threadPool, ClusterState initialClusterState)
public static MasterService createMasterService(ThreadPool threadPool, DiscoveryNode localNode)
public abstract int numDataAndMasterNodes();
```
Reason for the above are not deprecated and renamed:
1. The class `MasterService` is not deprecated, and needs to find a solution to deal with deprecating this class in the return value.
2. abstract methods will be deprecated in a separate PR.
  • Loading branch information
opensearch-trigger-bot[bot] authored Jul 22, 2022
1 parent c0b1b59 commit fb4f96f
Show file tree
Hide file tree
Showing 71 changed files with 539 additions and 299 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void testScriptDisabled() throws Exception {
checkPipelineExists.accept(pipelineIdWithScript);
checkPipelineExists.accept(pipelineIdWithoutScript);

internalCluster().restartNode(internalCluster().getMasterName(), new InternalTestCluster.RestartCallback() {
internalCluster().restartNode(internalCluster().getClusterManagerName(), new InternalTestCluster.RestartCallback() {

@Override
public Settings onNodeStopped(String nodeName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,9 @@ public void testDeleteByQueryOnReadOnlyAllowDeleteIndex() throws Exception {
InternalTestCluster internalTestCluster = internalCluster();
InternalClusterInfoService infoService = (InternalClusterInfoService) internalTestCluster.getInstance(
ClusterInfoService.class,
internalTestCluster.getMasterName()
internalTestCluster.getClusterManagerName()
);
ThreadPool threadPool = internalTestCluster.getInstance(ThreadPool.class, internalTestCluster.getMasterName());
ThreadPool threadPool = internalTestCluster.getInstance(ThreadPool.class, internalTestCluster.getClusterManagerName());
// Refresh the cluster info after a random delay to check the disk threshold and release the block on the index
threadPool.schedule(infoService::refresh, TimeValue.timeValueMillis(randomIntBetween(1, 100)), ThreadPool.Names.MANAGEMENT);
// The delete by query request will be executed successfully because the block will be released
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private void testCase(
assertFalse(initialBulkResponse.buildFailureMessage(), initialBulkResponse.hasFailures());
client().admin().indices().prepareRefresh("source").get();

AbstractBulkByScrollRequestBuilder<?, ?> builder = request.apply(internalCluster().masterClient());
AbstractBulkByScrollRequestBuilder<?, ?> builder = request.apply(internalCluster().clusterManagerClient());
// Make sure we use more than one batch so we have to scroll
builder.source().setSize(DOC_COUNT / randomIntBetween(2, 10));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ protected Settings nodeSettings(int nodeOrdinal) {

public void testDeleteSingleItem() {
final String repoName = createRepository(randomName());
final RepositoriesService repositoriesService = internalCluster().getMasterNodeInstance(RepositoriesService.class);
final RepositoriesService repositoriesService = internalCluster().getClusterManagerNodeInstance(RepositoriesService.class);
final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(repoName);
PlainActionFuture.get(
f -> repository.threadPool()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public void testEnforcedCooldownPeriod() throws IOException {
.get()
.getSnapshotInfo()
.snapshotId();
final RepositoriesService repositoriesService = internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class);
final RepositoriesService repositoriesService = internalCluster().getCurrentClusterManagerNodeInstance(RepositoriesService.class);
final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(repoName);
final RepositoryData repositoryData = getRepositoryData(repository);
final RepositoryData modifiedRepositoryData = repositoryData.withVersions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,14 @@ public void testClusterManagerNodeOperationTasks() {
registerTaskManagerListeners(ClusterHealthAction.NAME);

// First run the health on the cluster-manager node - should produce only one task on the cluster-manager node
internalCluster().masterClient().admin().cluster().prepareHealth().get();
internalCluster().clusterManagerClient().admin().cluster().prepareHealth().get();
assertEquals(1, numberOfEvents(ClusterHealthAction.NAME, Tuple::v1)); // counting only registration events
assertEquals(1, numberOfEvents(ClusterHealthAction.NAME, event -> event.v1() == false)); // counting only unregistration events

resetTaskManagerListeners(ClusterHealthAction.NAME);

// Now run the health on a non-cluster-manager node - should produce one task on cluster-manager and one task on another node
internalCluster().nonMasterClient().admin().cluster().prepareHealth().get();
internalCluster().nonClusterManagerClient().admin().cluster().prepareHealth().get();
assertEquals(2, numberOfEvents(ClusterHealthAction.NAME, Tuple::v1)); // counting only registration events
assertEquals(2, numberOfEvents(ClusterHealthAction.NAME, event -> event.v1() == false)); // counting only unregistration events
List<TaskInfo> tasks = findEvents(ClusterHealthAction.NAME, Tuple::v1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public void runRepeatedlyWhileChangingClusterManager(Runnable runnable) throws E
)
);

final String clusterManagerName = internalCluster().getMasterName();
final String clusterManagerName = internalCluster().getClusterManagerName();

final AtomicBoolean shutdown = new AtomicBoolean();
final Thread assertingThread = new Thread(() -> {
Expand Down Expand Up @@ -245,7 +245,7 @@ public void runRepeatedlyWhileChangingClusterManager(Runnable runnable) throws E
clusterManagerName,
() -> randomFrom(internalCluster().getNodeNames())
);
final String claimedClusterManagerName = internalCluster().getMasterName(nonClusterManagerNode);
final String claimedClusterManagerName = internalCluster().getClusterManagerName(nonClusterManagerNode);
assertThat(claimedClusterManagerName, not(equalTo(clusterManagerName)));
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public void testFailureToCreateIndexCleansUpIndicesService() {
IllegalStateException.class
);

IndicesService indicesService = internalCluster().getInstance(IndicesService.class, internalCluster().getMasterName());
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, internalCluster().getClusterManagerName());
for (IndexService indexService : indicesService) {
assertThat(indexService.index().getName(), not("test-idx-2"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ public void testCreateShrinkIndexFails() throws Exception {

final InternalClusterInfoService infoService = (InternalClusterInfoService) internalCluster().getInstance(
ClusterInfoService.class,
internalCluster().getMasterName()
internalCluster().getClusterManagerName()
);
infoService.refresh();
// kick off a retry and wait until it's done!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testClusterManagerFailoverDuringIndexingWithMappingChanges() throws

internalCluster().setBootstrapClusterManagerNodeIndex(2);

internalCluster().startMasterOnlyNodes(3, Settings.EMPTY);
internalCluster().startClusterManagerOnlyNodes(3, Settings.EMPTY);

String dataNode = internalCluster().startDataOnlyNode(Settings.EMPTY);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public <T extends TransportResponse> void sendRequest(
}

public void testRetryOnStoppedTransportService() throws Exception {
internalCluster().startMasterOnlyNodes(2);
internalCluster().startClusterManagerOnlyNodes(2);
String primary = internalCluster().startDataOnlyNode();
assertAcked(
prepareCreate("test").setSettings(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@ public void testWaitForEventsRetriesIfOtherConditionsNotMet() {
.execute();

final AtomicBoolean keepSubmittingTasks = new AtomicBoolean(true);
final ClusterService clusterService = internalCluster().getInstance(ClusterService.class, internalCluster().getMasterName());
final ClusterService clusterService = internalCluster().getInstance(
ClusterService.class,
internalCluster().getClusterManagerName()
);
final PlainActionFuture<Void> completionFuture = new PlainActionFuture<>();
clusterService.submitStateUpdateTask("looping task", new ClusterStateUpdateTask(Priority.LOW) {
@Override
Expand Down Expand Up @@ -377,7 +380,7 @@ public void testHealthOnClusterManagerFailover() throws Exception {
.setClusterManagerNodeTimeout(TimeValue.timeValueMinutes(2))
.execute()
);
internalCluster().restartNode(internalCluster().getMasterName(), InternalTestCluster.EMPTY_CALLBACK);
internalCluster().restartNode(internalCluster().getClusterManagerName(), InternalTestCluster.EMPTY_CALLBACK);
}
if (withIndex) {
assertAcked(
Expand All @@ -396,7 +399,10 @@ public void testHealthOnClusterManagerFailover() throws Exception {

public void testWaitForEventsTimesOutIfClusterManagerBusy() {
final AtomicBoolean keepSubmittingTasks = new AtomicBoolean(true);
final ClusterService clusterService = internalCluster().getInstance(ClusterService.class, internalCluster().getMasterName());
final ClusterService clusterService = internalCluster().getInstance(
ClusterService.class,
internalCluster().getClusterManagerName()
);
final PlainActionFuture<Void> completionFuture = new PlainActionFuture<>();
clusterService.submitStateUpdateTask("looping task", new ClusterStateUpdateTask(Priority.LOW) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public void testClusterInfoServiceCollectsInformation() {
// Get the cluster info service on the cluster-manager node
final InternalClusterInfoService infoService = (InternalClusterInfoService) internalTestCluster.getInstance(
ClusterInfoService.class,
internalTestCluster.getMasterName()
internalTestCluster.getClusterManagerName()
);
infoService.setUpdateFrequency(TimeValue.timeValueMillis(200));
ClusterInfo info = infoService.refresh();
Expand All @@ -193,7 +193,7 @@ public void testClusterInfoServiceCollectsInformation() {
logger.info("--> shard size: {}", size.value);
assertThat("shard size is greater than 0", size.value, greaterThanOrEqualTo(0L));
}
ClusterService clusterService = internalTestCluster.getInstance(ClusterService.class, internalTestCluster.getMasterName());
ClusterService clusterService = internalTestCluster.getInstance(ClusterService.class, internalTestCluster.getClusterManagerName());
ClusterState state = clusterService.state();
for (ShardRouting shard : state.routingTable().allShards()) {
String dataPath = info.getDataPath(shard);
Expand Down Expand Up @@ -221,7 +221,7 @@ public void testClusterInfoServiceInformationClearOnError() {
InternalTestCluster internalTestCluster = internalCluster();
InternalClusterInfoService infoService = (InternalClusterInfoService) internalTestCluster.getInstance(
ClusterInfoService.class,
internalTestCluster.getMasterName()
internalTestCluster.getClusterManagerName()
);
// get one healthy sample
ClusterInfo info = infoService.refresh();
Expand All @@ -231,7 +231,7 @@ public void testClusterInfoServiceInformationClearOnError() {

MockTransportService mockTransportService = (MockTransportService) internalCluster().getInstance(
TransportService.class,
internalTestCluster.getMasterName()
internalTestCluster.getClusterManagerName()
);

final AtomicBoolean timeout = new AtomicBoolean(false);
Expand Down Expand Up @@ -272,7 +272,7 @@ public void testClusterInfoServiceInformationClearOnError() {

// now we cause an exception
timeout.set(false);
ActionFilters actionFilters = internalTestCluster.getInstance(ActionFilters.class, internalTestCluster.getMasterName());
ActionFilters actionFilters = internalTestCluster.getInstance(ActionFilters.class, internalTestCluster.getClusterManagerName());
BlockingActionFilter blockingActionFilter = null;
for (ActionFilter filter : actionFilters.filters()) {
if (filter instanceof BlockingActionFilter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {
);
}

String clusterManagerNode = internalCluster().getMasterName();
String clusterManagerNode = internalCluster().getClusterManagerName();
String otherNode = node1Name.equals(clusterManagerNode) ? node2Name : node1Name;
logger.info("--> add voting config exclusion for non-cluster-manager node, to be sure it's not elected");
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(otherNode)).get();
Expand Down Expand Up @@ -204,7 +204,7 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {
clearRequest.setWaitForRemoval(false);
client().execute(ClearVotingConfigExclusionsAction.INSTANCE, clearRequest).get();

clusterManagerNode = internalCluster().getMasterName();
clusterManagerNode = internalCluster().getClusterManagerName();
otherNode = node1Name.equals(clusterManagerNode) ? node2Name : node1Name;
logger.info("--> add voting config exclusion for cluster-manager node, to be sure it's not elected");
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(clusterManagerNode)).get();
Expand Down Expand Up @@ -310,12 +310,15 @@ public void testThreeNodesNoClusterManagerBlock() throws Exception {
}

List<String> nonClusterManagerNodes = new ArrayList<>(
Sets.difference(Sets.newHashSet(internalCluster().getNodeNames()), Collections.singleton(internalCluster().getMasterName()))
Sets.difference(
Sets.newHashSet(internalCluster().getNodeNames()),
Collections.singleton(internalCluster().getClusterManagerName())
)
);
Settings nonClusterManagerDataPathSettings1 = internalCluster().dataPathSettings(nonClusterManagerNodes.get(0));
Settings nonClusterManagerDataPathSettings2 = internalCluster().dataPathSettings(nonClusterManagerNodes.get(1));
internalCluster().stopRandomNonMasterNode();
internalCluster().stopRandomNonMasterNode();
internalCluster().stopRandomNonClusterManagerNode();
internalCluster().stopRandomNonClusterManagerNode();

logger.info("--> verify that there is no cluster-manager anymore on remaining node");
// spin here to wait till the state is set
Expand Down Expand Up @@ -347,7 +350,7 @@ public void testCannotCommitStateThreeNodes() throws Exception {
internalCluster().startNodes(3, settings);
ensureStableCluster(3);

final String clusterManager = internalCluster().getMasterName();
final String clusterManager = internalCluster().getClusterManagerName();
Set<String> otherNodes = new HashSet<>(Arrays.asList(internalCluster().getNodeNames()));
otherNodes.remove(clusterManager);
NetworkDisruption partition = isolateClusterManagerDisruption(NetworkDisruption.DISCONNECT);
Expand Down
Loading

0 comments on commit fb4f96f

Please sign in to comment.