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 class 'NoMasterBlockService' and 'MasterService' (#4006)

Deprecates public methods and variables that contain 'master' terminology
in class NoMasterBlockService and MasterService. Unit tests are also
added to ensure consistency with the new naming.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit e65aaca)
  • Loading branch information
@github-actions
Tianli Feng authored and github-actions[bot] committed Jul 28, 2022
1 parent 5d1dc07 commit 4528ddb
Show file tree
Hide file tree
Showing 24 changed files with 159 additions and 93 deletions.
2 changes: 1 addition & 1 deletion modules/rank-eval/src/test/java/org/opensearch/index/rankeval/RankEvalResponseTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
public class RankEvalResponseTests extends OpenSearchTestCase {

private static final Exception[] RANDOM_EXCEPTIONS = new Exception[] {
new ClusterBlockException(singleton(NoClusterManagerBlockService.NO_MASTER_BLOCK_WRITES)),
new ClusterBlockException(singleton(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_WRITES)),
new CircuitBreakingException("Data too large", 123, 456, CircuitBreaker.Durability.PERMANENT),
new SearchParseException(SHARD_TARGET, "Parse failure", new XContentLocation(12, 98)),
new IllegalArgumentException("Closed resource", new RuntimeException("Resource")),
Expand Down
4 changes: 2 additions & 2 deletions server/src/internalClusterTest/java/org/opensearch/cluster/ClusterStateDiffIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,9 @@ private ClusterState.Builder randomBlocks(ClusterState clusterState) {
private ClusterBlock randomGlobalBlock() {
switch (randomInt(2)) {
case 0:
return NoClusterManagerBlockService.NO_MASTER_BLOCK_ALL;
return NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ALL;
case 1:
return NoClusterManagerBlockService.NO_MASTER_BLOCK_WRITES;
return NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_WRITES;
default:
return GatewayService.STATE_NOT_RECOVERED_BLOCK;
}
Expand Down
24 changes: 12 additions & 12 deletions server/src/internalClusterTest/java/org/opensearch/cluster/MinimumClusterManagerNodesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {

logger.info("--> should be blocked, no cluster-manager...");
ClusterState state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(true));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(true));
assertThat(state.nodes().getSize(), equalTo(1)); // verify that we still see the local node in the cluster state

logger.info("--> start second node, cluster should be formed");
Expand All @@ -109,9 +109,9 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {
assertThat(clusterHealthResponse.isTimedOut(), equalTo(false));

state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));
state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));

state = client().admin().cluster().prepareState().execute().actionGet().getState();
assertThat(state.nodes().getSize(), equalTo(2));
Expand Down Expand Up @@ -161,11 +161,11 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {

assertBusy(() -> {
ClusterState clusterState = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertTrue(clusterState.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(clusterState.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
});

state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(true));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(true));
// verify that both nodes are still in the cluster state but there is no cluster-manager
assertThat(state.nodes().getSize(), equalTo(2));
assertThat(state.nodes().getClusterManagerNode(), equalTo(null));
Expand All @@ -184,9 +184,9 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {
assertThat(clusterHealthResponse.isTimedOut(), equalTo(false));

state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));
state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));

state = client().admin().cluster().prepareState().execute().actionGet().getState();
assertThat(state.nodes().getSize(), equalTo(2));
Expand Down Expand Up @@ -214,7 +214,7 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {

assertBusy(() -> {
ClusterState state1 = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state1.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(true));
assertThat(state1.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(true));
});

logger.info("--> starting the previous cluster-manager node again...");
Expand All @@ -232,9 +232,9 @@ public void testTwoNodesNoClusterManagerBlock() throws Exception {
assertThat(clusterHealthResponse.isTimedOut(), equalTo(false));

state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));
state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(false));
assertThat(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(false));

state = client().admin().cluster().prepareState().execute().actionGet().getState();
assertThat(state.nodes().getSize(), equalTo(2));
Expand Down Expand Up @@ -262,7 +262,7 @@ public void testThreeNodesNoClusterManagerBlock() throws Exception {
assertBusy(() -> {
for (Client client : clients()) {
ClusterState state1 = client.admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(state1.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(true));
assertThat(state1.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(true));
}
});

Expand Down Expand Up @@ -324,7 +324,7 @@ public void testThreeNodesNoClusterManagerBlock() throws Exception {
// spin here to wait till the state is set
assertBusy(() -> {
ClusterState st = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertThat(st.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID), equalTo(true));
assertThat(st.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID), equalTo(true));
});

logger.info("--> start back the 2 nodes ");
Expand Down
6 changes: 3 additions & 3 deletions server/src/internalClusterTest/java/org/opensearch/cluster/NoClusterManagerNodeIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void testNoClusterManagerActions() throws Exception {
.execute()
.actionGet()
.getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
});

assertRequestBuilderThrows(
Expand Down Expand Up @@ -288,7 +288,7 @@ public void testNoClusterManagerActionsWriteClusterManagerBlock() throws Excepti

assertBusy(() -> {
ClusterState state = clientToClusterManagerlessNode.admin().cluster().prepareState().setLocal(true).get().getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
});

GetResponse getResponse = clientToClusterManagerlessNode.prepareGet("test1", "1").get();
Expand Down Expand Up @@ -387,7 +387,7 @@ public void testNoClusterManagerActionsMetadataWriteClusterManagerBlock() throws
assertBusy(() -> {
for (String node : nodesWithShards) {
ClusterState state = client(node).admin().cluster().prepareState().setLocal(true).get().getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
}
});

Expand Down
8 changes: 4 additions & 4 deletions ...usterTest/java/org/opensearch/cluster/coordination/UnsafeBootstrapAndDetachCommandIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public void testBootstrapNotBootstrappedCluster() throws Exception {
);
assertBusy(() -> {
ClusterState state = client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
});

Settings dataPathSettings = internalCluster().dataPathSettings(node);
Expand Down Expand Up @@ -338,7 +338,7 @@ public void test3ClusterManagerNodes2Failed() throws Exception {
.execute()
.actionGet()
.getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
});

logger.info("--> try to unsafely bootstrap 1st cluster-manager-eligible node, while node lock is held");
Expand Down Expand Up @@ -394,7 +394,7 @@ public void test3ClusterManagerNodes2Failed() throws Exception {
.execute()
.actionGet()
.getState();
assertFalse(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertFalse(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));
assertTrue(
state.metadata().persistentSettings().getAsBoolean(UnsafeBootstrapClusterManagerCommand.UNSAFE_BOOTSTRAP.getKey(), false)
);
Expand Down Expand Up @@ -508,7 +508,7 @@ public void testNoInitialBootstrapAfterDetach() throws Exception {
);

ClusterState state = internalCluster().client().admin().cluster().prepareState().setLocal(true).execute().actionGet().getState();
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID));
assertTrue(state.blocks().hasGlobalBlockWithId(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID));

internalCluster().stopRandomNode(InternalTestCluster.nameFilter(node));
}
Expand Down
8 changes: 6 additions & 2 deletions server/src/internalClusterTest/java/org/opensearch/discovery/ClusterManagerDisruptionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ public void testVerifyApiBlocksDuringPartition() throws Exception {
// continuously ping until network failures have been resolved. However
// It may a take a bit before the node detects it has been cut off from the elected cluster-manager
logger.info("waiting for isolated node [{}] to have no cluster-manager", isolatedNode);
assertNoClusterManager(isolatedNode, NoClusterManagerBlockService.NO_MASTER_BLOCK_METADATA_WRITES, TimeValue.timeValueSeconds(30));
assertNoClusterManager(
isolatedNode,
NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_METADATA_WRITES,
TimeValue.timeValueSeconds(30)
);

logger.info("wait until elected cluster-manager has been removed and a new 2 node cluster was from (via [{}])", isolatedNode);
ensureStableCluster(2, nonIsolatedNode);
Expand Down Expand Up @@ -280,7 +284,7 @@ public void testVerifyApiBlocksDuringPartition() throws Exception {
// continuously ping until network failures have been resolved. However
// It may a take a bit before the node detects it has been cut off from the elected cluster-manager
logger.info("waiting for isolated node [{}] to have no cluster-manager", isolatedNode);
assertNoClusterManager(isolatedNode, NoClusterManagerBlockService.NO_MASTER_BLOCK_ALL, TimeValue.timeValueSeconds(30));
assertNoClusterManager(isolatedNode, NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ALL, TimeValue.timeValueSeconds(30));

// make sure we have stable cluster & cross partition recoveries are canceled by the removal of the missing node
// the unresponsive partition causes recoveries to only time out after 15m (default) and these will cause
Expand Down
10 changes: 5 additions & 5 deletions server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import static org.opensearch.cluster.coordination.NoClusterManagerBlockService.NO_MASTER_BLOCK_ID;
import static org.opensearch.cluster.coordination.NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID;
import static org.opensearch.gateway.ClusterStateUpdaters.hideStateIfNotRecovered;
import static org.opensearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
import static org.opensearch.monitor.StatusInfo.Status.UNHEALTHY;
Expand Down Expand Up @@ -839,7 +839,7 @@ protected void doStart() {
.blocks(
ClusterBlocks.builder()
.addGlobalBlock(STATE_NOT_RECOVERED_BLOCK)
.addGlobalBlock(noClusterManagerBlockService.getNoMasterBlock())
.addGlobalBlock(noClusterManagerBlockService.getNoClusterManagerBlock())
)
.nodes(DiscoveryNodes.builder().add(getLocalNode()).localNodeId(getLocalNode().getId()))
.build();
Expand Down Expand Up @@ -886,7 +886,7 @@ public void invariant() {
assert followersChecker.getFastResponseState().term == getCurrentTerm() : followersChecker.getFastResponseState();
assert followersChecker.getFastResponseState().mode == getMode() : followersChecker.getFastResponseState();
assert (applierState.nodes().getClusterManagerNodeId() == null) == applierState.blocks()
.hasGlobalBlockWithId(NO_MASTER_BLOCK_ID);
.hasGlobalBlockWithId(NO_CLUSTER_MANAGER_BLOCK_ID);
assert preVoteCollector.getPreVoteResponse().equals(getPreVoteResponse()) : preVoteCollector + " vs " + getPreVoteResponse();

assert lagDetector.getTrackedNodes().contains(getLocalNode()) == false : lagDetector.getTrackedNodes();
Expand Down Expand Up @@ -1221,11 +1221,11 @@ ClusterState getStateForClusterManagerService() {
private ClusterState clusterStateWithNoClusterManagerBlock(ClusterState clusterState) {
if (clusterState.nodes().getClusterManagerNodeId() != null) {
// remove block if it already exists before adding new one
assert clusterState.blocks().hasGlobalBlockWithId(NO_MASTER_BLOCK_ID) == false
assert clusterState.blocks().hasGlobalBlockWithId(NO_CLUSTER_MANAGER_BLOCK_ID) == false
: "NO_MASTER_BLOCK should only be added by Coordinator";
final ClusterBlocks clusterBlocks = ClusterBlocks.builder()
.blocks(clusterState.blocks())
.addGlobalBlock(noClusterManagerBlockService.getNoMasterBlock())
.addGlobalBlock(noClusterManagerBlockService.getNoClusterManagerBlock())
.build();
final DiscoveryNodes discoveryNodes = new DiscoveryNodes.Builder(clusterState.nodes()).clusterManagerNodeId(null).build();
return ClusterState.builder(clusterState).blocks(clusterBlocks).nodes(discoveryNodes).build();
Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,9 @@ protected ClusterState.Builder becomeClusterManagerAndTrimConflictingNodes(Clust
ClusterState tmpState = ClusterState.builder(currentState)
.nodes(nodesBuilder)
.blocks(
ClusterBlocks.builder().blocks(currentState.blocks()).removeGlobalBlock(NoClusterManagerBlockService.NO_MASTER_BLOCK_ID)
ClusterBlocks.builder()
.blocks(currentState.blocks())
.removeGlobalBlock(NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_ID)
)
.build();
logger.trace("becomeClusterManagerAndTrimConflictingNodes: {}", tmpState.nodes());
Expand Down
Loading

0 comments on commit 4528ddb

Please sign in to comment.