Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.0] Replace 'master' terminology with 'cluster manager' in log messages in 'server/src/main' directory - Part 2 #3174

Merged
merged 2 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {

String masterNode = internalCluster().getMasterName();
String otherNode = node1Name.equals(masterNode) ? node2Name : node1Name;
logger.info("--> add voting config exclusion for non-master node, to be sure it's not elected");
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();
logger.info("--> stop master node, no master block should appear");
Settings masterDataPathSettings = internalCluster().dataPathSettings(masterNode);
Expand Down Expand Up @@ -208,7 +208,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {
otherNode = node1Name.equals(masterNode) ? node2Name : node1Name;
logger.info("--> add voting config exclusion for master node, to be sure it's not elected");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still a few master references on L#209, L#158, L#173

Copy link
Collaborator Author

@tlfeng tlfeng May 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your careful review!
Ah, I didn't realized there are still lots of log messages contain master in server/src/internalClusterTest directory and is shown in this PR. 😂
It definitely doesn't look good. Thanks for pointing out!
To minimize the change for 2.0 version, I propose to limit the scope of this PR to server/src/main directory. For the master references in other directories, I will create PR to main branch first and then backport to 2.x branch.
So I will revert those changes in server/src/internalClusterTest directory. Is that fine?

Copy link
Collaborator Author

@tlfeng tlfeng May 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes for this PR is actually a subset of #2519. I didn't plan to backport that PR to 2.0 branch since the feature for 2.0 version is frozen and I want to reduce the code change, but when I saw there are still log messages contain master terminology, I think it's worth to replace all master reference in log message in 2.0 version for consistency.
I will make sure all the log message in server/src/main don't have master terminology, since it will be exposed to the user.
For the master terminology in other directories, I will handle in different PRs ( tracked in issue #1548)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 😄

client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(masterNode)).get();
logger.info("--> stop non-master node, no master block should appear");
logger.info("--> stop non-cluster-manager node, no master block should appear");
Settings otherNodeDataPathSettings = internalCluster().dataPathSettings(otherNode);
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(otherNode));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public void clusterChanged(ClusterChangedEvent event) {
assertNotNull(sne);
assertThat(
sne.getMessage(),
either(endsWith(" Failed to update cluster state during snapshot finalization")).or(endsWith(" no longer master"))
either(endsWith(" Failed to update cluster state during snapshot finalization")).or(endsWith(" no longer cluster-manager"))
);
assertThat(sne.getSnapshotName(), is(snapshot));
}
Expand Down Expand Up @@ -248,7 +248,7 @@ public void testMasterFailOverDuringShardSnapshots() throws Exception {

blockDataNode(repoName, dataNode);

logger.info("--> create snapshot via master node client");
logger.info("--> create snapshot via cluster-manager node client");
final ActionFuture<CreateSnapshotResponse> snapshotResponse = internalCluster().masterClient()
.admin()
.cluster()
Expand All @@ -272,7 +272,7 @@ public void testMasterFailOverDuringShardSnapshots() throws Exception {
SnapshotException.class,
() -> snapshotResponse.actionGet(TimeValue.timeValueSeconds(30L))
);
assertThat(sne.getMessage(), endsWith("no longer master"));
assertThat(sne.getMessage(), endsWith("no longer cluster-manager"));
}

private void assertSnapshotExists(String repository, String snapshot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,11 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
@Override
public void onNoLongerMaster(String source) {
logger.trace(
"stopped being master while waiting for events with priority [{}]. retrying.",
"stopped being cluster-manager while waiting for events with priority [{}]. retrying.",
request.waitForEvents()
);
// TransportMasterNodeAction implements the retry logic, which is triggered by passing a NotMasterException
listener.onFailure(new NotMasterException("no longer master. source: [" + source + "]"));
listener.onFailure(new NotMasterException("no longer cluster-manager. source: [" + source + "]"));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private static void addClusterStateApplier(ClusterService clusterService) {
return;
}
clusterService.submitStateUpdateTask(
"clean up repository cleanup task after master failover",
"clean up repository cleanup task after cluster-manager failover",
new ClusterStateUpdateTask() {
@Override
public ClusterState execute(ClusterState currentState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private void reroute(final boolean updateSettingsAcked) {
// For example the minimum_master_node could have been breached and we're no longer elected master,
// so we should *not* execute the reroute.
if (!clusterService.state().nodes().isLocalNodeElectedMaster()) {
logger.debug("Skipping reroute after cluster update settings, because node is no longer master");
logger.debug("Skipping reroute after cluster update settings, because node is no longer cluster-manager");
listener.onResponse(
new ClusterUpdateSettingsResponse(
updateSettingsAcked,
Expand Down Expand Up @@ -198,7 +198,7 @@ protected ClusterUpdateSettingsResponse newResponse(boolean acknowledged) {
@Override
public void onNoLongerMaster(String source) {
logger.debug(
"failed to preform reroute after cluster settings were updated - current node is no longer a master"
"failed to preform reroute after cluster settings were updated - current node is no longer a cluster-manager"
);
listener.onResponse(
new ClusterUpdateSettingsResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void onNewClusterState(ClusterState newState) {
} else {
listener.onFailure(
new NotMasterException(
"master stepped down waiting for metadata version " + request.waitForMetadataVersion()
"cluster-manager stepped down waiting for metadata version " + request.waitForMetadataVersion()
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ protected void doStart(ClusterState clusterState) {
}
} else {
if (nodes.getMasterNode() == null) {
logger.debug("no known master node, scheduling a retry");
logger.debug("no known cluster-manager node, scheduling a retry");
retryOnMasterChange(clusterState, null);
} else {
DiscoveryNode masterNode = nodes.getMasterNode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public interface ClusterStateTaskListener {
* Used only for tasks submitted to {@link MasterService}.
*/
default void onNoLongerMaster(String source) {
onFailure(source, new NotMasterException("no longer master. source: [" + source + "]"));
onFailure(source, new NotMasterException("no longer cluster-manager. source: [" + source + "]"));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public NodeMappingRefreshAction(TransportService transportService, MetadataMappi

public void nodeMappingRefresh(final DiscoveryNode masterNode, final NodeMappingRefreshRequest request) {
if (masterNode == null) {
logger.warn("can't send mapping refresh for [{}], no master known.", request.index());
logger.warn("can't send mapping refresh for [{}], no cluster-manager known.", request.index());
return;
}
transportService.sendRequest(masterNode, ACTION_NAME, request, EmptyTransportResponseHandler.INSTANCE_SAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private void sendShardAction(
DiscoveryNode masterNode = currentState.nodes().getMasterNode();
Predicate<ClusterState> changePredicate = MasterNodeChangePredicate.build(currentState);
if (masterNode == null) {
logger.warn("no master known for action [{}] for shard entry [{}]", actionName, request);
logger.warn("no cluster-manager known for action [{}] for shard entry [{}]", actionName, request);
waitForNewMasterAndRetry(actionName, observer, request, listener, changePredicate);
} else {
logger.debug("sending [{}] to [{}] for shard entry [{}]", actionName, masterNode.getId(), request);
Expand Down Expand Up @@ -305,7 +305,7 @@ protected void waitForNewMasterAndRetry(
@Override
public void onNewClusterState(ClusterState state) {
if (logger.isTraceEnabled()) {
logger.trace("new cluster state [{}] after waiting for master election for shard entry [{}]", state, request);
logger.trace("new cluster state [{}] after waiting for cluster-manager election for shard entry [{}]", state, request);
}
sendShardAction(actionName, state, request, listener);
}
Expand Down Expand Up @@ -376,13 +376,13 @@ public void onFailure(String source, Exception e) {

@Override
public void onNoLongerMaster(String source) {
logger.error("{} no longer master while failing shard [{}]", request.shardId, request);
logger.error("{} no longer cluster-manager while failing shard [{}]", request.shardId, request);
try {
channel.sendResponse(new NotMasterException(source));
} catch (Exception channelException) {
logger.warn(
() -> new ParameterizedMessage(
"{} failed to send no longer master while failing shard [{}]",
"{} failed to send no longer cluster-manager while failing shard [{}]",
request.shardId,
request
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1233,14 +1233,16 @@ public void publish(
if (mode != Mode.LEADER || getCurrentTerm() != clusterChangedEvent.state().term()) {
logger.debug(
() -> new ParameterizedMessage(
"[{}] failed publication as node is no longer master for term {}",
"[{}] failed publication as node is no longer cluster-manager for term {}",
clusterChangedEvent.source(),
clusterChangedEvent.state().term()
)
);
publishListener.onFailure(
new FailedToCommitClusterStateException(
"node is no longer master for term " + clusterChangedEvent.state().term() + " while handling publication"
"node is no longer cluster-manager for term "
+ clusterChangedEvent.state().term()
+ " while handling publication"
)
);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,23 @@ public ClusterTasksResult<JoinTaskExecutor.Task> execute(ClusterState currentSta
// Stop processing the current cluster state update, as there's no point in continuing to compute it as
// it will later be rejected by Coordinator.publish(...) anyhow
if (currentState.term() > term) {
logger.trace("encountered higher term {} than current {}, there is a newer master", currentState.term(), term);
logger.trace("encountered higher term {} than current {}, there is a newer cluster-manager", currentState.term(), term);
throw new NotMasterException(
"Higher term encountered (current: " + currentState.term() + " > used: " + term + "), there is a newer master"
"Higher term encountered (current: "
+ currentState.term()
+ " > used: "
+ term
+ "), there is a newer cluster-manager"
);
} else if (currentState.nodes().getMasterNodeId() == null && joiningTasks.stream().anyMatch(Task::isBecomeMasterTask)) {
assert currentState.term() < term : "there should be at most one become master task per election (= by term)";
assert currentState.term() < term : "there should be at most one become cluster-manager task per election (= by term)";
final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder(currentState.coordinationMetadata())
.term(term)
.build();
final Metadata metadata = Metadata.builder(currentState.metadata()).coordinationMetadata(coordinationMetadata).build();
currentState = ClusterState.builder(currentState).metadata(metadata).build();
} else if (currentState.nodes().isLocalNodeElectedMaster()) {
assert currentState.term() == term : "term should be stable for the same master";
assert currentState.term() == term : "term should be stable for the same cluster-manager";
}
return super.execute(currentState, joiningTasks);
}
Expand Down Expand Up @@ -297,7 +301,7 @@ void logLastFailedJoinAttempt() {
}

public void sendJoinRequest(DiscoveryNode destination, long term, Optional<Join> optionalJoin, Runnable onCompletion) {
assert destination.isMasterNode() : "trying to join master-ineligible " + destination;
assert destination.isMasterNode() : "trying to join cluster-manager-ineligible " + destination;
final StatusInfo statusInfo = nodeHealthService.getHealth();
if (statusInfo.getStatus() == UNHEALTHY) {
logger.debug("dropping join request to [{}]: [{}]", destination, statusInfo.getInfo());
Expand Down Expand Up @@ -348,7 +352,7 @@ public String executor() {
}

public void sendStartJoinRequest(final StartJoinRequest startJoinRequest, final DiscoveryNode destination) {
assert startJoinRequest.getSourceNode().isMasterNode() : "sending start-join request for master-ineligible "
assert startJoinRequest.getSourceNode().isMasterNode() : "sending start-join request for cluster-manager-ineligible "
+ startJoinRequest.getSourceNode();
transportService.sendRequest(destination, START_JOIN_ACTION_NAME, startJoinRequest, new TransportResponseHandler<Empty>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,19 @@ public ClusterTasksResult<Task> execute(ClusterState currentState, List<Task> jo
if (joiningNodes.size() == 1 && joiningNodes.get(0).isFinishElectionTask()) {
return results.successes(joiningNodes).build(currentState);
} else if (currentNodes.getMasterNode() == null && joiningNodes.stream().anyMatch(Task::isBecomeMasterTask)) {
assert joiningNodes.stream().anyMatch(Task::isFinishElectionTask) : "becoming a master but election is not finished "
assert joiningNodes.stream().anyMatch(Task::isFinishElectionTask) : "becoming a cluster-manager but election is not finished "
+ joiningNodes;
// use these joins to try and become the master.
// Note that we don't have to do any validation of the amount of joining nodes - the commit
// during the cluster state publishing guarantees that we have enough
newState = becomeMasterAndTrimConflictingNodes(currentState, joiningNodes);
nodesChanged = true;
} else if (currentNodes.isLocalNodeElectedMaster() == false) {
logger.trace("processing node joins, but we are not the master. current master: {}", currentNodes.getMasterNode());
throw new NotMasterException("Node [" + currentNodes.getLocalNode() + "] not master for join request");
logger.trace(
"processing node joins, but we are not the cluster-manager. current cluster-manager: {}",
currentNodes.getMasterNode()
);
throw new NotMasterException("Node [" + currentNodes.getLocalNode() + "] not cluster-manager for join request");
} else {
newState = ClusterState.builder(currentState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ private void handleLeaderCheck(LeaderCheckRequest request) {
logger.debug(message);
throw new NodeHealthCheckFailureException(message);
} else if (discoveryNodes.isLocalNodeElectedMaster() == false) {
logger.debug("rejecting leader check on non-master {}", request);
logger.debug("rejecting leader check on non-cluster-manager {}", request);
throw new CoordinationStateRejectedException(
"rejecting leader check from [" + request.getSender() + "] sent to a node that is no longer the master"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ private static ClusterBlock parseNoMasterBlock(String value) {
case "metadata_write":
return NO_MASTER_BLOCK_METADATA_WRITES;
default:
throw new IllegalArgumentException("invalid no-master block [" + value + "], must be one of [all, write, metadata_write]");
throw new IllegalArgumentException(
"invalid no-cluster-manager block [" + value + "], must be one of [all, write, metadata_write]"
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void onFailure(final String source, final Exception e) {

@Override
public void onNoLongerMaster(String source) {
logger.debug("no longer master while processing node removal [{}]", source);
logger.debug("no longer cluster-manager while processing node removal [{}]", source);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public VotingConfiguration reconfigure(
) {
assert liveNodes.contains(currentMaster) : "liveNodes = " + liveNodes + " master = " + currentMaster;
logger.trace(
"{} reconfiguring {} based on liveNodes={}, retiredNodeIds={}, currentMaster={}",
"{} reconfiguring {} based on liveNodes={}, retiredNodeIds={}, currentClusterManager={}",
this,
currentConfig,
liveNodes,
Expand Down
Loading