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.

Sign up for GitHub

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

Log [initial_master_nodes] on formation failure #36466

Merged
merged 1 commit into from
Dec 11, 2018
Merged

Log [initial_master_nodes] on formation failure #36466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ...r/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ String getDescription() {

if (INITIAL_MASTER_NODE_COUNT_SETTING.get(Settings.EMPTY).equals(INITIAL_MASTER_NODE_COUNT_SETTING.get(settings))
&& INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY).equals(INITIAL_MASTER_NODES_SETTING.get(settings))) {
bootstrappingDescription = "cluster bootstrapping is disabled on this node";
bootstrappingDescription = "[" + INITIAL_MASTER_NODES_SETTING.getKey() + "] is empty on this node";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think technically we check for is not set on this node, shall we just use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We check if it's set here:

elasticsearch/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java

Lines 84 to 85 in 6e6e63d

return Stream.of(DISCOVERY_HOSTS_PROVIDER_SETTING, DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING,
INITIAL_MASTER_NODE_COUNT_SETTING, INITIAL_MASTER_NODES_SETTING).anyMatch(s -> s.exists(settings));

If it's not set, and nor are any of the other settings listed here, then we auto-bootstrap after a few seconds which leads to a different log message that starts master not discovered or elected yet and doesn't mention bootstrapping.

However if bootstrapping hasn't occurred by the time we write this message then this can't have happened, so the problem is actually that it's empty:

elasticsearch/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java

Line 142 in 6e6e63d

} else if (initialMasterNodeCount > 0 || initialMasterNodes.isEmpty() == false) {

Copy link
Contributor

Choose a reason for hiding this comment

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

My comment was just targeted at the if clause:

INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY).equals(INITIAL_MASTER_NODES_SETTING.get(settings))```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also checks it's not empty, since the default for this setting is empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

It technically checks it's not set or set to the default (which is empty). As I said this was just a suggestion. I'm good anyway.

} else if (INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY).equals(INITIAL_MASTER_NODES_SETTING.get(settings))) {
bootstrappingDescription = String.format(Locale.ROOT,
"this node must discover at least [%d] master-eligible nodes to bootstrap a cluster",
Expand Down
6 changes: 3 additions & 3 deletions .../test/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,20 @@ public void testDescriptionBeforeBootstrapping() {

assertThat(new ClusterFormationState(Settings.EMPTY, clusterState, emptyList(), emptyList()).getDescription(),
is("master not discovered yet, this node has not previously joined a bootstrapped (v7+) cluster, and " +
"cluster bootstrapping is disabled on this node: have discovered []; " +
"[cluster.initial_master_nodes] is empty on this node: have discovered []; " +
"discovery will continue using [] from hosts providers and [" + localNode + "] from last-known cluster state"));

final TransportAddress otherAddress = buildNewFakeTransportAddress();
assertThat(new ClusterFormationState(Settings.EMPTY, clusterState, singletonList(otherAddress), emptyList()).getDescription(),
is("master not discovered yet, this node has not previously joined a bootstrapped (v7+) cluster, and " +
"cluster bootstrapping is disabled on this node: have discovered []; " +
"[cluster.initial_master_nodes] is empty on this node: have discovered []; " +
"discovery will continue using [" + otherAddress + "] from hosts providers and [" + localNode +
"] from last-known cluster state"));

final DiscoveryNode otherNode = new DiscoveryNode("other", buildNewFakeTransportAddress(), Version.CURRENT);
assertThat(new ClusterFormationState(Settings.EMPTY, clusterState, emptyList(), singletonList(otherNode)).getDescription(),
is("master not discovered yet, this node has not previously joined a bootstrapped (v7+) cluster, and " +
"cluster bootstrapping is disabled on this node: have discovered [" + otherNode + "]; " +
"[cluster.initial_master_nodes] is empty on this node: have discovered [" + otherNode + "]; " +
"discovery will continue using [] from hosts providers and [" + localNode + "] from last-known cluster state"));

assertThat(new ClusterFormationState(Settings.builder().put(INITIAL_MASTER_NODE_COUNT_SETTING.getKey(), 2).build(),
Expand Down