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

Replace internal usages of 'master' term in 'server/src/main' directory #2519

Merged
merged 20 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
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 @@ -785,7 +785,7 @@ private enum OpenSearchExceptionHandle {
2,
UNKNOWN_VERSION_ADDED
),
MASTER_NOT_DISCOVERED_EXCEPTION(
CLUSTER_MANAGER_NOT_DISCOVERED_EXCEPTION(
org.opensearch.discovery.MasterNotDiscoveredException.class,
org.opensearch.discovery.MasterNotDiscoveredException::new,
3,
Expand Down Expand Up @@ -1496,7 +1496,7 @@ private enum OpenSearchExceptionHandle {
143,
UNKNOWN_VERSION_ADDED
),
NOT_MASTER_EXCEPTION(
NOT_CLUSTER_MANAGER_EXCEPTION(
org.opensearch.cluster.NotMasterException.class,
org.opensearch.cluster.NotMasterException::new,
144,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
// ClusterStateHealth fields
int numberOfNodes = (int) parsedObjects[i++];
int numberOfDataNodes = (int) parsedObjects[i++];
boolean hasDiscoveredMaster = (boolean) parsedObjects[i++];
boolean hasDiscoveredClusterManager = (boolean) parsedObjects[i++];
int activeShards = (int) parsedObjects[i++];
int relocatingShards = (int) parsedObjects[i++];
int activePrimaryShards = (int) parsedObjects[i++];
Expand Down Expand Up @@ -118,7 +118,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
unassignedShards,
numberOfNodes,
numberOfDataNodes,
hasDiscoveredMaster,
hasDiscoveredClusterManager,
activeShardsPercent,
status,
indices
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,20 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
ClusterStateResponse response = (ClusterStateResponse) o;
return waitForTimedOut == response.waitForTimedOut && Objects.equals(clusterName, response.clusterName) &&
// Best effort. Only compare cluster state version and master node id,
// Best effort. Only compare cluster state version and cluster-manager node id,
// because cluster state doesn't implement equals()
Objects.equals(getVersion(clusterState), getVersion(response.clusterState))
&& Objects.equals(getMasterNodeId(clusterState), getMasterNodeId(response.clusterState));
&& Objects.equals(getClusterManagerNodeId(clusterState), getClusterManagerNodeId(response.clusterState));
}

@Override
public int hashCode() {
// Best effort. Only use cluster state version and master node id,
// Best effort. Only use cluster state version and cluster-manager node id,
// because cluster state doesn't implement hashcode()
return Objects.hash(clusterName, getVersion(clusterState), getMasterNodeId(clusterState), waitForTimedOut);
return Objects.hash(clusterName, getVersion(clusterState), getClusterManagerNodeId(clusterState), waitForTimedOut);
}

private static String getMasterNodeId(ClusterState clusterState) {
private static String getClusterManagerNodeId(ClusterState clusterState) {
if (clusterState == null) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the getMasterNodeId below in line 130 need to be changed to getClusterManagerNodeId?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @xuezhou25 , thanks a lot for your time in reviewing my PR! 👍👍.
the method getMasterNodeId() in line 130 is published to Maven as Java API. Please see my comment above: #2519 (comment).
Although the normal way to rename the Java API is to deprecate old method and create new alternative method, there are too many public methods contain name "master", and it will introduce too many extra codes to deprecate them all. So I plan to rename them all in a future major version, and give the other plugins and clients enough notice before renaming.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import java.io.IOException;

/**
* A based request for master based operation.
* A based request for cluster-manager based operation.
*/
public abstract class MasterNodeRequest<Request extends MasterNodeRequest<Request>> extends ActionRequest {

Expand All @@ -62,7 +62,7 @@ public void writeTo(StreamOutput out) throws IOException {
}

/**
* A timeout value in case the master has not been discovered yet or disconnected.
* A timeout value in case the cluster-manager has not been discovered yet or disconnected.
*/
@SuppressWarnings("unchecked")
public final Request masterNodeTimeout(TimeValue timeout) {
Expand All @@ -71,7 +71,7 @@ public final Request masterNodeTimeout(TimeValue timeout) {
}

/**
* A timeout value in case the master has not been discovered yet or disconnected.
* A timeout value in case the cluster-manager has not been discovered yet or disconnected.
*/
public final Request masterNodeTimeout(String timeout) {
return masterNodeTimeout(TimeValue.parseTimeValue(timeout, null, getClass().getSimpleName() + ".masterNodeTimeout"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ protected void doStart(ClusterState clusterState) {
logger.debug("no known master node, scheduling a retry");
Copy link
Contributor

@xuezhou25 xuezhou25 Mar 23, 2022

Choose a reason for hiding this comment

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

line 70
line 185
line 201
line 219
line 282
master->cluster-manager

Copy link
Collaborator Author

@tlfeng tlfeng Mar 23, 2022

Choose a reason for hiding this comment

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

Good catch! Thank you, you are so carefully. Those usages are all missed... 😅
I will change them.

retryOnMasterChange(clusterState, null);
} else {
DiscoveryNode masterNode = nodes.getMasterNode();
final String actionName = getMasterActionName(masterNode);
DiscoveryNode clusterManagerNode = nodes.getMasterNode();
final String actionName = getMasterActionName(clusterManagerNode);
transportService.sendRequest(
masterNode,
clusterManagerNode,
actionName,
request,
new ActionListenerResponseHandler<Response>(listener, TransportMasterNodeAction.this::read) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ private MasterNodeChangePredicate() {
*/
public static Predicate<ClusterState> build(ClusterState currentState) {
final long currentVersion = currentState.version();
final DiscoveryNode masterNode = currentState.nodes().getMasterNode();
final String currentMasterId = masterNode == null ? null : masterNode.getEphemeralId();
final DiscoveryNode clusterManagerNode = currentState.nodes().getMasterNode();
final String currentMasterId = clusterManagerNode == null ? null : clusterManagerNode.getEphemeralId();
return newState -> {
final DiscoveryNode newMaster = newState.nodes().getMasterNode();
final boolean accept;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ public void setClient(Client client) {
}

/**
* Update mappings on the master node, waiting for the change to be committed,
* Update mappings on the cluster-manager node, waiting for the change to be committed,
* but not for the mapping update to be applied on all nodes. The timeout specified by
* {@code timeout} is the master node timeout ({@link MasterNodeRequest#masterNodeTimeout()}),
* potentially waiting for a master node to be available.
* {@code timeout} is the cluster-manager node timeout ({@link MasterNodeRequest#masterNodeTimeout()}),
* potentially waiting for a cluster-manager node to be available.
*/
public void updateMappingOnMaster(Index index, Mapping mappingUpdate, ActionListener<Void> listener) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ public NodeMappingRefreshAction(TransportService transportService, MetadataMappi
);
}

public void nodeMappingRefresh(final DiscoveryNode masterNode, final NodeMappingRefreshRequest request) {
if (masterNode == null) {
public void nodeMappingRefresh(final DiscoveryNode clusterManagerNode, final NodeMappingRefreshRequest request) {
if (clusterManagerNode == null) {
logger.warn("can't send mapping refresh for [{}], no master known.", request.index());
return;
}
transportService.sendRequest(masterNode, ACTION_NAME, request, EmptyTransportResponseHandler.INSTANCE_SAME);
transportService.sendRequest(clusterManagerNode, ACTION_NAME, request, EmptyTransportResponseHandler.INSTANCE_SAME);
}

private class NodeMappingRefreshTransportHandler implements TransportRequestHandler<NodeMappingRefreshRequest> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,14 @@ private void sendShardAction(
final ActionListener<Void> listener
) {
ClusterStateObserver observer = new ClusterStateObserver(currentState, clusterService, null, logger, threadPool.getThreadContext());
DiscoveryNode masterNode = currentState.nodes().getMasterNode();
DiscoveryNode clusterManagerNode = currentState.nodes().getMasterNode();
Predicate<ClusterState> changePredicate = MasterNodeChangePredicate.build(currentState);
if (masterNode == null) {
if (clusterManagerNode == null) {
logger.warn("no master 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);
transportService.sendRequest(masterNode, actionName, request, new EmptyTransportResponseHandler(ThreadPool.Names.SAME) {
logger.debug("sending [{}] to [{}] for shard entry [{}]", actionName, clusterManagerNode.getId(), request);
transportService.sendRequest(clusterManagerNode, actionName, request, new EmptyTransportResponseHandler(ThreadPool.Names.SAME) {
@Override
public void handleResponse(TransportResponse.Empty response) {
listener.onResponse(null);
Expand All @@ -199,7 +199,7 @@ public void handleException(TransportException exp) {
new ParameterizedMessage(
"unexpected failure while sending request [{}]" + " to [{}] for shard entry [{}]",
actionName,
masterNode,
clusterManagerNode,
request
),
exp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import java.io.IOException;

/**
* A master node sends this request to its peers to inform them that it could commit the
* A cluster-manager node sends this request to its peers to inform them that it could commit the
* cluster state with the given term and version. Peers that have accepted the given cluster
* state will then consider it as committed and proceed to apply the state locally.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ public ClusterBootstrapService(
bootstrapRequirements = Collections.singleton(Node.NODE_NAME_SETTING.get(settings));
unconfiguredBootstrapTimeout = null;
} else {
final List<String> initialMasterNodes = INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings);
bootstrapRequirements = unmodifiableSet(new LinkedHashSet<>(initialMasterNodes));
if (bootstrapRequirements.size() != initialMasterNodes.size()) {
final List<String> initialClusterManagerNodes = INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings);
bootstrapRequirements = unmodifiableSet(new LinkedHashSet<>(initialClusterManagerNodes));
if (bootstrapRequirements.size() != initialClusterManagerNodes.size()) {
throw new IllegalArgumentException(
"setting [" + INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodes
"setting [" + INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey() + "] contains duplicates: " + initialClusterManagerNodes
);
}
unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings);
Expand Down
Loading