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

Rename reserved node id '_must_join_elected_master_' to '_must_join_elected_cluster_manager_' that used by in DetachClusterCommand #3116

Merged
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -218,7 +218,7 @@ String getDescription() {

assert clusterState.getLastCommittedConfiguration().isEmpty() == false;

if (clusterState.getLastCommittedConfiguration().equals(VotingConfiguration.MUST_JOIN_ELECTED_MASTER)) {
if (clusterState.getLastCommittedConfiguration().equals(VotingConfiguration.MUST_JOIN_ELECTED_CLUSTER_MANAGER)) {
return String.format(
Locale.ROOT,
"cluster-manager not discovered yet and this node was detached from its previous cluster, have discovered %s; %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,8 @@ public String toString() {
public static class VotingConfiguration implements Writeable, ToXContentFragment {

public static final VotingConfiguration EMPTY_CONFIG = new VotingConfiguration(Collections.emptySet());
public static final VotingConfiguration MUST_JOIN_ELECTED_MASTER = new VotingConfiguration(
Collections.singleton("_must_join_elected_master_")
public static final VotingConfiguration MUST_JOIN_ELECTED_CLUSTER_MANAGER = new VotingConfiguration(
Collections.singleton("_must_join_elected_cluster_manager_")
Copy link
Member

@dblock dblock May 2, 2022

Choose a reason for hiding this comment

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

I assume this name is never used in a distributed manner where 1 node calls it _must_join_elected_cluster_manager_ and the other _must_join_elected_master_ and it's ok?

Copy link
Collaborator Author

@tlfeng tlfeng May 2, 2022

Choose a reason for hiding this comment

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

Hi @dblock , thanks for your review!
In my opinion, the preserved node id _must_join_elected_master_ is only read by ClusterFormationFailureHelper, which result a line of log generated. The log says "master not discovered yet and this node was detached from its previous cluster", so it looks like not used in a distributed manner.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, to make it safe, it's better for me to deprecate MUST_JOIN_ELECTED_MASTER first in version 2.x, and accepting the both _must_join_elected_cluster_manager_ and _must_join_elected_master_ in ClusterFormationFailureHelper.
Otherwise, maybe doing a rolling upgrade from 2.x to 3.x after running ./bin/opensearch-node detach-cluster will be a problem to identify the old identifier _must_join_elected_master_. Thank you! 😄

);

private final Set<String> nodeIds;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ protected void processNodePaths(Terminal terminal, Path[] dataPaths, int nodeLoc
// package-private for tests
static Metadata updateMetadata(Metadata oldMetadata) {
final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder()
.lastAcceptedConfiguration(CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_MASTER)
.lastCommittedConfiguration(CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_MASTER)
.lastAcceptedConfiguration(CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_CLUSTER_MANAGER)
.lastCommittedConfiguration(CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_CLUSTER_MANAGER)
.term(0)
.build();
return Metadata.builder(oldMetadata).coordinationMetadata(coordinationMetadata).clusterUUIDCommitted(false).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ public void testDescriptionAfterDetachCluster() {

final ClusterState clusterState = state(
localNode,
VotingConfiguration.MUST_JOIN_ELECTED_MASTER.getNodeIds().toArray(new String[0])
VotingConfiguration.MUST_JOIN_ELECTED_CLUSTER_MANAGER.getNodeIds().toArray(new String[0])
);

assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void reboot() {
.getLastAcceptedConfiguration()
.isEmpty()
? CoordinationMetadata.VotingConfiguration.EMPTY_CONFIG
: CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_MASTER;
: CoordinationMetadata.VotingConfiguration.MUST_JOIN_ELECTED_CLUSTER_MANAGER;
persistedState = new InMemoryPersistedState(
0L,
clusterState(0L, 0L, localNode, votingConfiguration, votingConfiguration, 0L)
Expand Down