From 8f538fc86ca8753734a2b2c1a01fce24ff20550f Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 14 Jan 2019 17:15:02 +0000 Subject: [PATCH 01/23] Simplify ClusterBootstrapService Today, the cluster bootstrapping process involves two (local) transport actions in order to support a flexible bootstrapping API and to make it easily accessible to plugins. However this flexibility is not required for the current design so it is adding a good deal of unnecessary complexity. This commit removes this complexity in favour of a much simpler ClusterBootstrapService implementation that does all the work itself. --- .../elasticsearch/ElasticsearchException.java | 4 +- .../elasticsearch/action/ActionModule.java | 6 - .../bootstrap/BootstrapClusterAction.java | 41 -- .../bootstrap/BootstrapClusterRequest.java | 65 --- .../bootstrap/BootstrapClusterResponse.java | 66 --- .../bootstrap/BootstrapConfiguration.java | 179 ------ .../bootstrap/GetDiscoveredNodesAction.java | 41 -- .../bootstrap/GetDiscoveredNodesRequest.java | 119 ---- .../bootstrap/GetDiscoveredNodesResponse.java | 73 --- .../TransportBootstrapClusterAction.java | 87 --- .../TransportGetDiscoveredNodesAction.java | 178 ------ .../ClusterAlreadyBootstrappedException.java | 38 -- .../coordination/ClusterBootstrapService.java | 250 ++++---- .../cluster/coordination/Coordinator.java | 46 +- .../ExceptionSerializationTests.java | 2 - .../BootstrapClusterRequestTests.java | 39 -- .../BootstrapClusterResponseTests.java | 33 -- .../BootstrapConfigurationTests.java | 182 ------ .../GetDiscoveredNodesRequestTests.java | 64 --- .../GetDiscoveredNodesResponseTests.java | 59 -- .../TransportBootstrapClusterActionTests.java | 233 -------- ...ransportGetDiscoveredNodesActionTests.java | 533 ------------------ .../ClusterBootstrapServiceTests.java | 350 +++++++----- .../coordination/CoordinatorTests.java | 68 --- .../snapshots/SnapshotsServiceTests.java | 14 +- 25 files changed, 336 insertions(+), 2434 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterAction.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequest.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponse.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfiguration.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesAction.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequest.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponse.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterAction.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesAction.java delete mode 100644 server/src/main/java/org/elasticsearch/cluster/coordination/ClusterAlreadyBootstrappedException.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequestTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponseTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfigurationTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequestTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponseTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterActionTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesActionTests.java diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index d18d4d4820f7d..efa1ccaf33322 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -1008,9 +1008,7 @@ private enum ElasticsearchExceptionHandle { TOO_MANY_BUCKETS_EXCEPTION(MultiBucketConsumerService.TooManyBucketsException.class, MultiBucketConsumerService.TooManyBucketsException::new, 149, Version.V_7_0_0), COORDINATION_STATE_REJECTED_EXCEPTION(org.elasticsearch.cluster.coordination.CoordinationStateRejectedException.class, - org.elasticsearch.cluster.coordination.CoordinationStateRejectedException::new, 150, Version.V_7_0_0), - CLUSTER_ALREADY_BOOTSTRAPPED_EXCEPTION(org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException.class, - org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException::new, 151, Version.V_7_0_0); + org.elasticsearch.cluster.coordination.CoordinationStateRejectedException::new, 150, Version.V_7_0_0); final Class exceptionClass; final CheckedFunction constructor; diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index a3d3c615162dc..142aa6bde74b6 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -23,10 +23,6 @@ import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterAction; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesAction; -import org.elasticsearch.action.admin.cluster.bootstrap.TransportBootstrapClusterAction; -import org.elasticsearch.action.admin.cluster.bootstrap.TransportGetDiscoveredNodesAction; import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction; import org.elasticsearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsAction; import org.elasticsearch.action.admin.cluster.configuration.TransportAddVotingConfigExclusionsAction; @@ -433,8 +429,6 @@ public void reg actions.register(GetTaskAction.INSTANCE, TransportGetTaskAction.class); actions.register(CancelTasksAction.INSTANCE, TransportCancelTasksAction.class); - actions.register(GetDiscoveredNodesAction.INSTANCE, TransportGetDiscoveredNodesAction.class); - actions.register(BootstrapClusterAction.INSTANCE, TransportBootstrapClusterAction.class); actions.register(AddVotingConfigExclusionsAction.INSTANCE, TransportAddVotingConfigExclusionsAction.class); actions.register(ClearVotingConfigExclusionsAction.INSTANCE, TransportClearVotingConfigExclusionsAction.class); actions.register(ClusterAllocationExplainAction.INSTANCE, TransportClusterAllocationExplainAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterAction.java deleted file mode 100644 index 28a8e580cedc4..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterAction.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.Action; -import org.elasticsearch.common.io.stream.Writeable.Reader; - -public class BootstrapClusterAction extends Action { - public static final BootstrapClusterAction INSTANCE = new BootstrapClusterAction(); - public static final String NAME = "cluster:admin/bootstrap/set_voting_config"; - - private BootstrapClusterAction() { - super(NAME); - } - - @Override - public BootstrapClusterResponse newResponse() { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public Reader getResponseReader() { - return BootstrapClusterResponse::new; - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequest.java deleted file mode 100644 index f8d0bcb13a58f..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequest.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; - -/** - * Request to set the initial configuration of master-eligible nodes in a cluster so that the very first master election can take place. - */ -public class BootstrapClusterRequest extends ActionRequest { - private final BootstrapConfiguration bootstrapConfiguration; - - public BootstrapClusterRequest(BootstrapConfiguration bootstrapConfiguration) { - this.bootstrapConfiguration = bootstrapConfiguration; - } - - public BootstrapClusterRequest(StreamInput in) throws IOException { - super(in); - bootstrapConfiguration = new BootstrapConfiguration(in); - } - - /** - * @return the bootstrap configuration: the initial set of master-eligible nodes whose votes are counted in elections. - */ - public BootstrapConfiguration getBootstrapConfiguration() { - return bootstrapConfiguration; - } - - @Override - public ActionRequestValidationException validate() { - return null; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - bootstrapConfiguration.writeTo(out); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponse.java deleted file mode 100644 index 2576409a3cef1..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponse.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; - -/** - * Response to a {@link BootstrapClusterRequest} indicating that the cluster has been successfully bootstrapped. - */ -public class BootstrapClusterResponse extends ActionResponse { - private final boolean alreadyBootstrapped; - - public BootstrapClusterResponse(boolean alreadyBootstrapped) { - this.alreadyBootstrapped = alreadyBootstrapped; - } - - public BootstrapClusterResponse(StreamInput in) throws IOException { - super(in); - alreadyBootstrapped = in.readBoolean(); - } - - /** - * @return whether this node already knew that the cluster had been bootstrapped when handling this request. - */ - public boolean getAlreadyBootstrapped() { - return alreadyBootstrapped; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeBoolean(alreadyBootstrapped); - } - - @Override - public String toString() { - return "BootstrapClusterResponse{" + - "alreadyBootstrapped=" + alreadyBootstrapped + - '}'; - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfiguration.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfiguration.java deleted file mode 100644 index 822287af77465..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfiguration.java +++ /dev/null @@ -1,179 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; - -public class BootstrapConfiguration implements Writeable { - - private final List nodeDescriptions; - - public BootstrapConfiguration(List nodeDescriptions) { - if (nodeDescriptions.isEmpty()) { - throw new IllegalArgumentException("cannot create empty bootstrap configuration"); - } - this.nodeDescriptions = Collections.unmodifiableList(new ArrayList<>(nodeDescriptions)); - } - - public BootstrapConfiguration(StreamInput in) throws IOException { - nodeDescriptions = Collections.unmodifiableList(in.readList(NodeDescription::new)); - assert nodeDescriptions.isEmpty() == false; - } - - public List getNodeDescriptions() { - return nodeDescriptions; - } - - public VotingConfiguration resolve(Iterable discoveredNodes) { - final Set selectedNodes = new HashSet<>(); - for (final NodeDescription nodeDescription : nodeDescriptions) { - final DiscoveryNode discoveredNode = nodeDescription.resolve(discoveredNodes); - if (selectedNodes.add(discoveredNode) == false) { - throw new ElasticsearchException("multiple nodes matching {} in {}", discoveredNode, this); - } - } - - final Set nodeIds = selectedNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()); - assert nodeIds.size() == selectedNodes.size() : selectedNodes + " does not contain distinct IDs"; - return new VotingConfiguration(nodeIds); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeList(nodeDescriptions); - } - - @Override - public String toString() { - return "BootstrapConfiguration{" + - "nodeDescriptions=" + nodeDescriptions + - '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - BootstrapConfiguration that = (BootstrapConfiguration) o; - return Objects.equals(nodeDescriptions, that.nodeDescriptions); - } - - @Override - public int hashCode() { - return Objects.hash(nodeDescriptions); - } - - public static class NodeDescription implements Writeable { - - @Nullable - private final String id; - - private final String name; - - @Nullable - public String getId() { - return id; - } - - public String getName() { - return name; - } - - public NodeDescription(@Nullable String id, String name) { - this.id = id; - this.name = Objects.requireNonNull(name); - } - - public NodeDescription(DiscoveryNode discoveryNode) { - this(discoveryNode.getId(), discoveryNode.getName()); - } - - public NodeDescription(StreamInput in) throws IOException { - this(in.readOptionalString(), in.readString()); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeOptionalString(id); - out.writeString(name); - } - - @Override - public String toString() { - return "NodeDescription{" + - "id='" + id + '\'' + - ", name='" + name + '\'' + - '}'; - } - - public DiscoveryNode resolve(Iterable discoveredNodes) { - DiscoveryNode selectedNode = null; - for (final DiscoveryNode discoveredNode : discoveredNodes) { - assert discoveredNode.isMasterNode() : discoveredNode; - if (discoveredNode.getName().equals(name)) { - if (id == null || id.equals(discoveredNode.getId())) { - if (selectedNode != null) { - throw new ElasticsearchException( - "discovered multiple nodes matching {} in {}", this, discoveredNodes); - } - selectedNode = discoveredNode; - } else { - throw new ElasticsearchException("node id mismatch comparing {} to {}", this, discoveredNode); - } - } else if (id != null && id.equals(discoveredNode.getId())) { - throw new ElasticsearchException("node name mismatch comparing {} to {}", this, discoveredNode); - } - } - if (selectedNode == null) { - throw new ElasticsearchException("no node matching {} found in {}", this, discoveredNodes); - } - - return selectedNode; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - NodeDescription that = (NodeDescription) o; - return Objects.equals(id, that.id) && - Objects.equals(name, that.name); - } - - @Override - public int hashCode() { - return Objects.hash(id, name); - } - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesAction.java deleted file mode 100644 index acaef284a5420..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesAction.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.Action; -import org.elasticsearch.common.io.stream.Writeable.Reader; - -public class GetDiscoveredNodesAction extends Action { - public static final GetDiscoveredNodesAction INSTANCE = new GetDiscoveredNodesAction(); - public static final String NAME = "cluster:admin/bootstrap/discover_nodes"; - - private GetDiscoveredNodesAction() { - super(NAME); - } - - @Override - public GetDiscoveredNodesResponse newResponse() { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public Reader getResponseReader() { - return GetDiscoveredNodesResponse::new; - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequest.java deleted file mode 100644 index f91a4de5263be..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequest.java +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.unit.TimeValue; - -import java.io.IOException; -import java.util.Collections; -import java.util.List; - -/** - * Request the set of master-eligible nodes discovered by this node. Most useful in a brand-new cluster as a precursor to setting the - * initial configuration using {@link BootstrapClusterRequest}. - */ -public class GetDiscoveredNodesRequest extends ActionRequest { - - @Nullable // if the request should wait indefinitely - private TimeValue timeout = TimeValue.timeValueSeconds(30); - - private List requiredNodes = Collections.emptyList(); - - public GetDiscoveredNodesRequest() { - } - - public GetDiscoveredNodesRequest(StreamInput in) throws IOException { - super(in); - timeout = in.readOptionalTimeValue(); - requiredNodes = in.readList(StreamInput::readString); - } - - /** - * Sometimes it is useful to wait until enough nodes have been discovered, rather than failing immediately. This parameter controls how - * long to wait, and defaults to 30s. - * - * @param timeout how long to wait to discover sufficiently many nodes to respond successfully. - */ - public void setTimeout(@Nullable TimeValue timeout) { - if (timeout != null && timeout.compareTo(TimeValue.ZERO) < 0) { - throw new IllegalArgumentException("negative timeout of [" + timeout + "] is not allowed"); - } - this.timeout = timeout; - } - - /** - * Sometimes it is useful to wait until enough nodes have been discovered, rather than failing immediately. This parameter controls how - * long to wait, and defaults to 30s. - * - * @return how long to wait to discover sufficiently many nodes to respond successfully. - */ - @Nullable - public TimeValue getTimeout() { - return timeout; - } - - /** - * Sometimes it is useful only to receive a successful response after discovering a certain set of master-eligible nodes. - * This parameter gives the names or transport addresses of the expected nodes. - * - * @return list of expected nodes - */ - public List getRequiredNodes() { - return requiredNodes; - } - - /** - * Sometimes it is useful only to receive a successful response after discovering a certain set of master-eligible nodes. - * This parameter gives the names or transport addresses of the expected nodes. - * - * @param requiredNodes list of expected nodes - */ - public void setRequiredNodes(final List requiredNodes) { - this.requiredNodes = requiredNodes; - } - - @Override - public ActionRequestValidationException validate() { - return null; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeOptionalTimeValue(timeout); - out.writeStringList(requiredNodes); - } - - @Override - public String toString() { - return "GetDiscoveredNodesRequest{" + - "timeout=" + timeout + - ", requiredNodes=" + requiredNodes + "}"; - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponse.java deleted file mode 100644 index f1174002b6998..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponse.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration.NodeDescription; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.Collectors; - -/** - * Response to {@link GetDiscoveredNodesRequest}, containing the set of master-eligible nodes that were discovered. - */ -public class GetDiscoveredNodesResponse extends ActionResponse { - private final Set nodes; - - public GetDiscoveredNodesResponse(Set nodes) { - this.nodes = Collections.unmodifiableSet(new HashSet<>(nodes)); - } - - public GetDiscoveredNodesResponse(StreamInput in) throws IOException { - super(in); - nodes = Collections.unmodifiableSet(in.readSet(DiscoveryNode::new)); - } - - /** - * @return the set of nodes that were discovered. - */ - public Set getNodes() { - return nodes; - } - - /** - * @return a bootstrap configuration constructed from the set of nodes that were discovered, in order to make a - * {@link BootstrapClusterRequest}. - */ - public BootstrapConfiguration getBootstrapConfiguration() { - return new BootstrapConfiguration(nodes.stream().map(NodeDescription::new).collect(Collectors.toList())); - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeCollection(nodes, (o, v) -> v.writeTo(o)); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterAction.java deleted file mode 100644 index 32a9f39cc0db8..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterAction.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.cluster.coordination.Coordinator; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.AbstractRunnable; -import org.elasticsearch.discovery.Discovery; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.transport.TransportService; - -import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_TYPE_SETTING; - -public class TransportBootstrapClusterAction extends HandledTransportAction { - - @Nullable // TODO make this not nullable - private final Coordinator coordinator; - private final TransportService transportService; - private final String discoveryType; - - @Inject - public TransportBootstrapClusterAction(Settings settings, ActionFilters actionFilters, TransportService transportService, - Discovery discovery) { - super(BootstrapClusterAction.NAME, transportService, actionFilters, BootstrapClusterRequest::new); - this.transportService = transportService; - this.discoveryType = DISCOVERY_TYPE_SETTING.get(settings); - if (discovery instanceof Coordinator) { - coordinator = (Coordinator) discovery; - } else { - coordinator = null; - } - } - - @Override - protected void doExecute(Task task, BootstrapClusterRequest request, ActionListener listener) { - if (coordinator == null) { // TODO remove when not nullable - throw new IllegalArgumentException("cluster bootstrapping is not supported by discovery type [" + discoveryType + "]"); - } - - final DiscoveryNode localNode = transportService.getLocalNode(); - assert localNode != null; - if (localNode.isMasterNode() == false) { - throw new IllegalArgumentException( - "this node is not master-eligible, but cluster bootstrapping can only happen on a master-eligible node"); - } - - transportService.getThreadPool().generic().execute(new AbstractRunnable() { - @Override - public void doRun() { - listener.onResponse(new BootstrapClusterResponse( - coordinator.setInitialConfiguration(request.getBootstrapConfiguration()) == false)); - } - - @Override - public void onFailure(Exception e) { - listener.onFailure(e); - } - - @Override - public String toString() { - return "setting initial configuration with " + request; - } - }); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesAction.java deleted file mode 100644 index 6f6336c3bd5f3..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesAction.java +++ /dev/null @@ -1,178 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.ElasticsearchTimeoutException; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException; -import org.elasticsearch.cluster.coordination.Coordinator; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.stream.Writeable.Reader; -import org.elasticsearch.common.lease.Releasable; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.EsExecutors; -import org.elasticsearch.common.util.concurrent.ListenableFuture; -import org.elasticsearch.discovery.Discovery; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.threadpool.ThreadPool.Names; -import org.elasticsearch.transport.TransportService; - -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; - -import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_TYPE_SETTING; - -public class TransportGetDiscoveredNodesAction extends HandledTransportAction { - - @Nullable // TODO make this not nullable - private final Coordinator coordinator; - private final TransportService transportService; - private final String discoveryType; - - @Inject - public TransportGetDiscoveredNodesAction(Settings settings, ActionFilters actionFilters, TransportService transportService, - Discovery discovery) { - super(GetDiscoveredNodesAction.NAME, transportService, actionFilters, - (Reader) GetDiscoveredNodesRequest::new); - - this.discoveryType = DISCOVERY_TYPE_SETTING.get(settings); - this.transportService = transportService; - if (discovery instanceof Coordinator) { - coordinator = (Coordinator) discovery; - } else { - coordinator = null; - } - } - - @Override - protected void doExecute(Task task, GetDiscoveredNodesRequest request, ActionListener listener) { - if (coordinator == null) { // TODO remove when not nullable - throw new IllegalArgumentException("discovered nodes are not exposed by discovery type [" + discoveryType + "]"); - } - - final DiscoveryNode localNode = transportService.getLocalNode(); - assert localNode != null; - if (localNode.isMasterNode() == false) { - throw new IllegalArgumentException( - "this node is not master-eligible, but discovered nodes are only exposed by master-eligible nodes"); - } - final ExecutorService directExecutor = EsExecutors.newDirectExecutorService(); - final AtomicBoolean listenerNotified = new AtomicBoolean(); - final ListenableFuture listenableFuture = new ListenableFuture<>(); - final ThreadPool threadPool = transportService.getThreadPool(); - listenableFuture.addListener(listener, directExecutor, threadPool.getThreadContext()); - // TODO make it so that listenableFuture copes with multiple completions, and then remove listenerNotified - - final ActionListener> respondIfRequestSatisfied = new ActionListener>() { - @Override - public void onResponse(Iterable nodes) { - final Set nodesSet = new LinkedHashSet<>(); - nodesSet.add(localNode); - nodes.forEach(nodesSet::add); - logger.trace("discovered {}", nodesSet); - try { - if (checkWaitRequirements(request, nodesSet)) { - final GetDiscoveredNodesResponse response = new GetDiscoveredNodesResponse(nodesSet); - if (listenerNotified.compareAndSet(false, true)) { - listenableFuture.onResponse(response); - } - } - } catch (Exception e) { - onFailure(e); - } - } - - @Override - public void onFailure(Exception e) { - if (listenerNotified.compareAndSet(false, true)) { - listenableFuture.onFailure(e); - } - } - - @Override - public String toString() { - return "waiting for " + request; - } - }; - - final Releasable releasable = coordinator.withDiscoveryListener(respondIfRequestSatisfied); - listenableFuture.addListener(ActionListener.wrap(releasable::close), directExecutor, threadPool.getThreadContext()); - - if (coordinator.isInitialConfigurationSet()) { - respondIfRequestSatisfied.onFailure(new ClusterAlreadyBootstrappedException()); - } else { - respondIfRequestSatisfied.onResponse(coordinator.getFoundPeers()); - } - - if (request.getTimeout() != null) { - threadPool.schedule(request.getTimeout(), Names.SAME, new Runnable() { - @Override - public void run() { - respondIfRequestSatisfied.onFailure(new ElasticsearchTimeoutException("timed out while waiting for " + request)); - } - - @Override - public String toString() { - return "timeout handler for " + request; - } - }); - } - } - - private static boolean matchesRequirement(DiscoveryNode discoveryNode, String requirement) { - return discoveryNode.getName().equals(requirement) - || discoveryNode.getAddress().toString().equals(requirement) - || discoveryNode.getAddress().getAddress().equals(requirement); - } - - private static boolean checkWaitRequirements(GetDiscoveredNodesRequest request, Set nodes) { - List requirements = request.getRequiredNodes(); - final Set selectedNodes = new HashSet<>(); - for (final String requirement : requirements) { - final Set matchingNodes - = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet()); - - if (matchingNodes.isEmpty()) { - return false; - } - if (matchingNodes.size() > 1) { - throw new IllegalArgumentException("[" + requirement + "] matches " + matchingNodes); - } - - for (final DiscoveryNode matchingNode : matchingNodes) { - if (selectedNodes.add(matchingNode) == false) { - throw new IllegalArgumentException("[" + matchingNode + "] matches " + - requirements.stream().filter(r -> matchesRequirement(matchingNode, requirement)).collect(Collectors.toList())); - } - } - } - - return true; - } -} diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterAlreadyBootstrappedException.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterAlreadyBootstrappedException.java deleted file mode 100644 index cc1c77c88477c..0000000000000 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterAlreadyBootstrappedException.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.cluster.coordination; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.io.stream.StreamInput; - -import java.io.IOException; - -/** - * Exception thrown if trying to discovery nodes in order to perform cluster bootstrapping, but a cluster is formed before all the required - * nodes are discovered. - */ -public class ClusterAlreadyBootstrappedException extends ElasticsearchException { - public ClusterAlreadyBootstrappedException() { - super("node has already joined a bootstrapped cluster, bootstrapping is not required"); - } - - public ClusterAlreadyBootstrappedException(StreamInput in) throws IOException { - super(in); - } -} diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index fc3f4493104fc..e62bc4b8380ee 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -21,39 +21,33 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterAction; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterRequest; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterResponse; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesAction; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesRequest; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesResponse; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.threadpool.ThreadPool.Names; -import org.elasticsearch.transport.TransportException; -import org.elasticsearch.transport.TransportResponseHandler; import org.elasticsearch.transport.TransportService; -import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; public class ClusterBootstrapService { - private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); - public static final Setting> INITIAL_MASTER_NODES_SETTING = Setting.listSetting("cluster.initial_master_nodes", Collections.emptyList(), Function.identity(), Property.NodeScope); @@ -61,16 +55,23 @@ public class ClusterBootstrapService { Setting.timeSetting("discovery.unconfigured_bootstrap_timeout", TimeValue.timeValueSeconds(3), TimeValue.timeValueMillis(1), Property.NodeScope); + private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); private final List initialMasterNodes; - @Nullable + @Nullable // null if discoveryIsConfigured() private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; - private volatile boolean running; + private final Supplier> discoveredNodesSupplier; + private final Consumer votingConfigurationConsumer; + private final AtomicBoolean bootstrappingPermitted = new AtomicBoolean(true); - public ClusterBootstrapService(Settings settings, TransportService transportService) { + public ClusterBootstrapService(Settings settings, TransportService transportService, + Supplier> discoveredNodesSupplier, + Consumer votingConfigurationConsumer) { initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); this.transportService = transportService; + this.discoveredNodesSupplier = discoveredNodesSupplier; + this.votingConfigurationConsumer = votingConfigurationConsumer; } public static boolean discoveryIsConfigured(Settings settings) { @@ -78,157 +79,112 @@ public static boolean discoveryIsConfigured(Settings settings) { .anyMatch(s -> s.exists(settings)); } - public void start() { - assert running == false; - running = true; + void onFoundPeersUpdated() { + final Set nodes = getDiscoveredNodes(); + if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false + && nodes.stream().noneMatch(Coordinator::isZen1Node) && checkWaitRequirements(nodes)) { + + startBootstrap(nodes); + } + } + + void scheduleUnconfiguredBootstrap() { + if (unconfiguredBootstrapTimeout == null) { + return; + } if (transportService.getLocalNode().isMasterNode() == false) { return; } - if (unconfiguredBootstrapTimeout != null) { - logger.info("no discovery configuration found, will perform best-effort cluster bootstrapping after [{}] " + - "unless existing master is discovered", unconfiguredBootstrapTimeout); - final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + logger.info("no discovery configuration found, will perform best-effort cluster bootstrapping after [{}] " + + "unless existing master is discovered", unconfiguredBootstrapTimeout); + + transportService.getThreadPool().scheduleUnlessShuttingDown(unconfiguredBootstrapTimeout, Names.GENERIC, new Runnable() { + @Override + public void run() { + final Set discoveredNodes = getDiscoveredNodes(); + final List zen1Nodes = discoveredNodes.stream().filter(Coordinator::isZen1Node).collect(Collectors.toList()); + if (zen1Nodes.isEmpty()) { + logger.debug("performing best-effort cluster bootstrapping with {}", discoveredNodes); + startBootstrap(discoveredNodes); + } else { + logger.info("avoiding best-effort cluster bootstrapping due to discovery of pre-7.0 nodes {}", zen1Nodes); + } + } + + @Override + public String toString() { + return "unconfigured-discovery delayed bootstrap"; + } + }); + } + + private Set getDiscoveredNodes() { + return Stream.concat(Stream.of(transportService.getLocalNode()), + StreamSupport.stream(discoveredNodesSupplier.get().spliterator(), false)).collect(Collectors.toSet()); + } + + private void startBootstrap(Set discoveryNodes) { + if (bootstrappingPermitted.compareAndSet(true, false)) { + doBootstrap(new VotingConfiguration(discoveryNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()))); + } + } + + private void doBootstrap(VotingConfiguration votingConfiguration) { + assert transportService.getLocalNode().isMasterNode(); - transportService.getThreadPool().scheduleUnlessShuttingDown(unconfiguredBootstrapTimeout, Names.SAME, new Runnable() { + try { + votingConfigurationConsumer.accept(votingConfiguration); + } catch (Exception e) { + logger.warn(new ParameterizedMessage("exception when bootstrapping with {}, rescheduling", votingConfiguration), e); + transportService.getThreadPool().scheduleUnlessShuttingDown(TimeValue.timeValueSeconds(10), Names.GENERIC, + new Runnable() { @Override public void run() { - // TODO: remove the following line once schedule method properly preserves thread context - threadContext.markAsSystemContext(); - final GetDiscoveredNodesRequest request = new GetDiscoveredNodesRequest(); - logger.trace("sending {}", request); - transportService.sendRequest(transportService.getLocalNode(), GetDiscoveredNodesAction.NAME, request, - new TransportResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - logger.debug("discovered {}, starting to bootstrap", response.getNodes()); - awaitBootstrap(response.getBootstrapConfiguration()); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - if (rootCause instanceof ClusterAlreadyBootstrappedException) { - logger.debug(rootCause.getMessage(), rootCause); - } else { - logger.warn("discovery attempt failed", exp); - } - } - - @Override - public String executor() { - return Names.SAME; - } - - @Override - public GetDiscoveredNodesResponse read(StreamInput in) throws IOException { - return new GetDiscoveredNodesResponse(in); - } - }); + doBootstrap(votingConfiguration); } @Override public String toString() { - return "unconfigured-discovery delayed bootstrap"; + return "retry of failed bootstrapping with " + votingConfiguration; } - }); - - } - } else if (initialMasterNodes.isEmpty() == false) { - logger.debug("waiting for discovery of master-eligible nodes matching {}", initialMasterNodes); - - final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); - - final GetDiscoveredNodesRequest request = new GetDiscoveredNodesRequest(); - request.setRequiredNodes(initialMasterNodes); - request.setTimeout(null); - logger.trace("sending {}", request); - transportService.sendRequest(transportService.getLocalNode(), GetDiscoveredNodesAction.NAME, request, - new TransportResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - assert response.getNodes().stream().allMatch(DiscoveryNode::isMasterNode); - logger.debug("discovered {}, starting to bootstrap", response.getNodes()); - awaitBootstrap(response.getBootstrapConfiguration()); - } - - @Override - public void handleException(TransportException exp) { - logger.warn("discovery attempt failed", exp); - } - - @Override - public String executor() { - return Names.SAME; - } - - @Override - public GetDiscoveredNodesResponse read(StreamInput in) throws IOException { - return new GetDiscoveredNodesResponse(in); - } - }); - } + } + ); } } - public void stop() { - running = false; + private static boolean matchesRequirement(DiscoveryNode discoveryNode, String requirement) { + return discoveryNode.getName().equals(requirement) + || discoveryNode.getAddress().toString().equals(requirement) + || discoveryNode.getAddress().getAddress().equals(requirement); } - private void awaitBootstrap(final BootstrapConfiguration bootstrapConfiguration) { - if (running == false) { - logger.debug("awaitBootstrap: not running"); - return; - } + private boolean checkWaitRequirements(Set nodes) { + final Set selectedNodes = new HashSet<>(); + for (final String requirement : initialMasterNodes) { + final Set matchingNodes + = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet()); - BootstrapClusterRequest request = new BootstrapClusterRequest(bootstrapConfiguration); - logger.trace("sending {}", request); - transportService.sendRequest(transportService.getLocalNode(), BootstrapClusterAction.NAME, request, - new TransportResponseHandler() { - @Override - public void handleResponse(BootstrapClusterResponse response) { - logger.debug("automatic cluster bootstrapping successful: received {}", response); - } - - @Override - public void handleException(TransportException exp) { - // log a warning since a failure here indicates a bad problem, such as: - // - bootstrap configuration resolution failed (e.g. discovered nodes no longer match those in the bootstrap config) - // - discovered nodes no longer form a quorum in the bootstrap config - logger.warn(new ParameterizedMessage("automatic cluster bootstrapping failed, retrying [{}]", - bootstrapConfiguration.getNodeDescriptions()), exp); - - // There's not really much else we can do apart from retry and hope that the problem goes away. The retry is delayed - // since a tight loop here is unlikely to help. - transportService.getThreadPool().scheduleUnlessShuttingDown(TimeValue.timeValueSeconds(10), Names.SAME, new Runnable() { - @Override - public void run() { - // TODO: remove the following line once schedule method properly preserves thread context - transportService.getThreadPool().getThreadContext().markAsSystemContext(); - awaitBootstrap(bootstrapConfiguration); - } - - @Override - public String toString() { - return "retry bootstrapping with " + bootstrapConfiguration.getNodeDescriptions(); - } - }); - } + if (matchingNodes.isEmpty()) { + return false; + } + if (matchingNodes.size() > 1) { + bootstrappingPermitted.set(false); + logger.warn("bootstrapping cancelled: [{}] matches {}", requirement, matchingNodes); + return false; + } - @Override - public String executor() { - return Names.SAME; + for (final DiscoveryNode matchingNode : matchingNodes) { + if (selectedNodes.add(matchingNode) == false) { + bootstrappingPermitted.set(false); + logger.warn("bootstrapping cancelled: [{}] matches {}", matchingNode, initialMasterNodes.stream() + .filter(r -> matchesRequirement(matchingNode, requirement)).collect(Collectors.toList())); + return false; } + } + } - @Override - public BootstrapClusterResponse read(StreamInput in) throws IOException { - return new BootstrapClusterResponse(in); - } - }); + return true; } } diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 1d7ed0bdc7c2b..f2c082cb2ea16 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -24,7 +24,6 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; @@ -83,7 +82,6 @@ import java.util.stream.Collectors; import java.util.stream.StreamSupport; -import static org.elasticsearch.common.util.concurrent.ConcurrentCollections.newConcurrentSet; import static org.elasticsearch.discovery.DiscoverySettings.NO_MASTER_BLOCK_ID; import static org.elasticsearch.gateway.ClusterStateUpdaters.hideStateIfNotRecovered; import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK; @@ -139,8 +137,6 @@ public class Coordinator extends AbstractLifecycleComponent implements Discovery private JoinHelper.JoinAccumulator joinAccumulator; private Optional currentPublication = Optional.empty(); - private final Set>> discoveredNodesListeners = newConcurrentSet(); - public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSettings, TransportService transportService, NamedWriteableRegistry namedWriteableRegistry, AllocationService allocationService, MasterService masterService, Supplier persistedStateSupplier, UnicastHostsProvider unicastHostsProvider, @@ -171,7 +167,7 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe this.clusterApplier = clusterApplier; masterService.setClusterStateSupplier(this::getStateForMasterService); this.reconfigurator = new Reconfigurator(settings, clusterSettings); - this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService); + this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, this::setInitialConfiguration); this.discoveryUpgradeService = new DiscoveryUpgradeService(settings, clusterSettings, transportService, this::isInitialConfigurationSet, joinHelper, peerFinder::getFoundPeers, this::setInitialConfiguration); this.lagDetector = new LagDetector(settings, transportService.getThreadPool(), n -> removeNode(n, "lagging"), @@ -297,12 +293,6 @@ PublishWithJoinResponse handlePublishRequest(PublishRequest publishRequest) { becomeFollower("handlePublishRequest", sourceNode); // also updates preVoteCollector } - if (isInitialConfigurationSet()) { - for (final ActionListener> discoveredNodesListener : discoveredNodesListeners) { - discoveredNodesListener.onFailure(new ClusterAlreadyBootstrappedException()); - } - } - return new PublishWithJoinResponse(publishResponse, joinWithDestination(lastJoin, sourceNode, publishRequest.getAcceptedState().term())); } @@ -599,16 +589,12 @@ public void startInitialJoin() { synchronized (mutex) { becomeCandidate("startInitialJoin"); } - - if (isInitialConfigurationSet() == false) { - clusterBootstrapService.start(); - } + clusterBootstrapService.scheduleUnconfiguredBootstrap(); } @Override protected void doStop() { configuredHostsResolver.stop(); - clusterBootstrapService.stop(); } @Override @@ -707,21 +693,6 @@ public boolean isInitialConfigurationSet() { return getStateForMasterService().getLastAcceptedConfiguration().isEmpty() == false; } - /** - * Sets the initial configuration by resolving the given {@link BootstrapConfiguration} to concrete nodes. This method is safe to call - * more than once, as long as each call's bootstrap configuration resolves to the same set of nodes. - * - * @param bootstrapConfiguration A description of the nodes that should form the initial configuration. - * @return whether this call successfully set the initial configuration - if false, the cluster has already been bootstrapped. - */ - public boolean setInitialConfiguration(final BootstrapConfiguration bootstrapConfiguration) { - final List selfAndDiscoveredPeers = new ArrayList<>(); - selfAndDiscoveredPeers.add(getLocalNode()); - getFoundPeers().forEach(selfAndDiscoveredPeers::add); - final VotingConfiguration votingConfiguration = bootstrapConfiguration.resolve(selfAndDiscoveredPeers); - return setInitialConfiguration(votingConfiguration); - } - /** * Sets the initial configuration to the given {@link VotingConfiguration}. This method is safe to call * more than once, as long as the argument to each call is the same. @@ -734,6 +705,7 @@ public boolean setInitialConfiguration(final VotingConfiguration votingConfigura final ClusterState currentState = getStateForMasterService(); if (isInitialConfigurationSet()) { + logger.debug("initial configuration already set, ignoring {}", votingConfiguration); return false; } @@ -1040,9 +1012,7 @@ protected void onFoundPeersUpdated() { } } - for (final ActionListener> discoveredNodesListener : discoveredNodesListeners) { - discoveredNodesListener.onResponse(foundPeers); - } + clusterBootstrapService.onFoundPeersUpdated(); } } @@ -1077,14 +1047,6 @@ public String toString() { }); } - public Releasable withDiscoveryListener(ActionListener> listener) { - discoveredNodesListeners.add(listener); - return () -> { - boolean removed = discoveredNodesListeners.remove(listener); - assert removed : listener; - }; - } - public Iterable getFoundPeers() { // TODO everyone takes this and adds the local node. Maybe just add the local node here? return peerFinder.getFoundPeers(); diff --git a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java index cee57c9f50c47..4bb180c398638 100644 --- a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -32,7 +32,6 @@ import org.elasticsearch.client.AbstractClientHeadersTestCase; import org.elasticsearch.cluster.action.shard.ShardStateAction; import org.elasticsearch.cluster.block.ClusterBlockException; -import org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException; import org.elasticsearch.cluster.coordination.CoordinationStateRejectedException; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.IllegalShardRoutingStateException; @@ -808,7 +807,6 @@ public void testIds() { ids.put(148, UnknownNamedObjectException.class); ids.put(149, MultiBucketConsumerService.TooManyBucketsException.class); ids.put(150, CoordinationStateRejectedException.class); - ids.put(151, ClusterAlreadyBootstrappedException.class); Map, Integer> reverse = new HashMap<>(); for (Map.Entry> entry : ids.entrySet()) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequestTests.java deleted file mode 100644 index ee9c58413b350..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterRequestTests.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration.NodeDescription; -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; -import java.util.Collections; - -import static org.hamcrest.Matchers.equalTo; - -public class BootstrapClusterRequestTests extends ESTestCase { - - public void testSerialization() throws IOException { - final BootstrapConfiguration bootstrapConfiguration - = new BootstrapConfiguration(Collections.singletonList(new NodeDescription(null, randomAlphaOfLength(10)))); - final BootstrapClusterRequest original = new BootstrapClusterRequest(bootstrapConfiguration); - assertNull(original.validate()); - final BootstrapClusterRequest deserialized = copyWriteable(original, writableRegistry(), BootstrapClusterRequest::new); - assertThat(deserialized.getBootstrapConfiguration(), equalTo(bootstrapConfiguration)); - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponseTests.java deleted file mode 100644 index fb33dbc5fcbd6..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapClusterResponseTests.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; - -import static org.hamcrest.Matchers.equalTo; - -public class BootstrapClusterResponseTests extends ESTestCase { - public void testSerialization() throws IOException { - final BootstrapClusterResponse original = new BootstrapClusterResponse(randomBoolean()); - final BootstrapClusterResponse deserialized = copyWriteable(original, writableRegistry(), BootstrapClusterResponse::new); - assertThat(deserialized.getAlreadyBootstrapped(), equalTo(original.getAlreadyBootstrapped())); - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfigurationTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfigurationTests.java deleted file mode 100644 index fc2e24017e76a..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/BootstrapConfigurationTests.java +++ /dev/null @@ -1,182 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration.NodeDescription; -import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNode.Role; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.EqualsHashCodeTestUtils; -import org.elasticsearch.test.EqualsHashCodeTestUtils.CopyFunction; - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - -import static java.util.Collections.emptyMap; -import static java.util.Collections.singleton; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.startsWith; - -public class BootstrapConfigurationTests extends ESTestCase { - - public void testEqualsHashcodeSerialization() { - // Note: the explicit cast of the CopyFunction is needed for some IDE (specifically Eclipse 4.8.0) to infer the right type - EqualsHashCodeTestUtils.checkEqualsAndHashCode(randomBootstrapConfiguration(), - (CopyFunction) bootstrapConfiguration -> copyWriteable(bootstrapConfiguration, writableRegistry(), - BootstrapConfiguration::new), - this::mutate); - } - - public void testNodeDescriptionResolvedByName() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode expectedNode = randomFrom(discoveryNodes); - assertThat(new NodeDescription(null, expectedNode.getName()).resolve(discoveryNodes), equalTo(expectedNode)); - } - - public void testNodeDescriptionResolvedByIdAndName() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode expectedNode = randomFrom(discoveryNodes); - assertThat(new NodeDescription(expectedNode).resolve(discoveryNodes), equalTo(expectedNode)); - } - - public void testRejectsMismatchedId() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode expectedNode = randomFrom(discoveryNodes); - final ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new NodeDescription(randomAlphaOfLength(11), expectedNode.getName()).resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("node id mismatch comparing ")); - } - - public void testRejectsMismatchedName() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode expectedNode = randomFrom(discoveryNodes); - final ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new NodeDescription(expectedNode.getId(), randomAlphaOfLength(11)).resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("node name mismatch comparing ")); - } - - public void testFailsIfNoMatch() { - final List discoveryNodes = randomDiscoveryNodes(); - final ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> randomNodeDescription().resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("no node matching ")); - } - - public void testFailsIfDuplicateMatchOnName() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode discoveryNode = randomFrom(discoveryNodes); - discoveryNodes.add(new DiscoveryNode(discoveryNode.getName(), randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), - singleton(Role.MASTER), Version.CURRENT)); - final ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new NodeDescription(null, discoveryNode.getName()).resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("discovered multiple nodes matching ")); - } - - public void testFailsIfDuplicatedNode() { - final List discoveryNodes = randomDiscoveryNodes(); - final DiscoveryNode discoveryNode = randomFrom(discoveryNodes); - discoveryNodes.add(discoveryNode); - final ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new NodeDescription(discoveryNode).resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("discovered multiple nodes matching ")); - } - - public void testResolvesEntireConfiguration() { - final List discoveryNodes = randomDiscoveryNodes(); - final List selectedNodes = randomSubsetOf(randomIntBetween(1, discoveryNodes.size()), discoveryNodes); - final BootstrapConfiguration bootstrapConfiguration = new BootstrapConfiguration(selectedNodes.stream() - .map(discoveryNode -> randomBoolean() ? new NodeDescription(discoveryNode) : new NodeDescription(null, discoveryNode.getName())) - .collect(Collectors.toList())); - - final VotingConfiguration expectedConfiguration - = new VotingConfiguration(selectedNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet())); - final VotingConfiguration votingConfiguration = bootstrapConfiguration.resolve(discoveryNodes); - assertThat(votingConfiguration, equalTo(expectedConfiguration)); - } - - public void testRejectsDuplicatedDescriptions() { - final List discoveryNodes = randomDiscoveryNodes(); - final List selectedNodes = randomSubsetOf(randomIntBetween(1, discoveryNodes.size()), discoveryNodes); - final List selectedNodeDescriptions = selectedNodes.stream() - .map(discoveryNode -> randomBoolean() ? new NodeDescription(discoveryNode) : new NodeDescription(null, discoveryNode.getName())) - .collect(Collectors.toList()); - final NodeDescription toDuplicate = randomFrom(selectedNodeDescriptions); - selectedNodeDescriptions.add(randomBoolean() ? toDuplicate : new NodeDescription(null, toDuplicate.getName())); - final BootstrapConfiguration bootstrapConfiguration = new BootstrapConfiguration(selectedNodeDescriptions); - - final ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> bootstrapConfiguration.resolve(discoveryNodes)); - assertThat(e.getMessage(), startsWith("multiple nodes matching ")); - } - - private NodeDescription mutate(NodeDescription original) { - if (randomBoolean()) { - return new NodeDescription(original.getId(), randomAlphaOfLength(21 - original.getName().length())); - } else { - if (original.getId() == null) { - return new NodeDescription(randomAlphaOfLength(10), original.getName()); - } else if (randomBoolean()) { - return new NodeDescription(randomAlphaOfLength(21 - original.getId().length()), original.getName()); - } else { - return new NodeDescription(null, original.getName()); - } - } - } - - protected BootstrapConfiguration mutate(BootstrapConfiguration original) { - final List newDescriptions = new ArrayList<>(original.getNodeDescriptions()); - final int mutateElement = randomIntBetween(0, newDescriptions.size()); - if (mutateElement == newDescriptions.size()) { - newDescriptions.add(randomIntBetween(0, newDescriptions.size()), randomNodeDescription()); - } else { - if (newDescriptions.size() > 1 && randomBoolean()) { - newDescriptions.remove(mutateElement); - } else { - newDescriptions.set(mutateElement, mutate(newDescriptions.get(mutateElement))); - } - } - return new BootstrapConfiguration(newDescriptions); - } - - protected NodeDescription randomNodeDescription() { - return new NodeDescription(randomBoolean() ? null : randomAlphaOfLength(10), randomAlphaOfLength(10)); - } - - protected BootstrapConfiguration randomBootstrapConfiguration() { - final int size = randomIntBetween(1, 5); - final List nodeDescriptions = new ArrayList<>(size); - while (nodeDescriptions.size() <= size) { - nodeDescriptions.add(randomNodeDescription()); - } - return new BootstrapConfiguration(nodeDescriptions); - } - - protected List randomDiscoveryNodes() { - final int size = randomIntBetween(1, 5); - final List nodes = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - nodes.add(new DiscoveryNode(randomAlphaOfLength(10), randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), - singleton(Role.MASTER), Version.CURRENT)); - } - return nodes; - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequestTests.java deleted file mode 100644 index 676e2e958cac7..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesRequestTests.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; - -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.startsWith; -import static org.hamcrest.core.Is.is; - -public class GetDiscoveredNodesRequestTests extends ESTestCase { - - public void testTimeoutValidation() { - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - assertThat("default value is 30s", getDiscoveredNodesRequest.getTimeout(), is(TimeValue.timeValueSeconds(30))); - - final TimeValue newTimeout = TimeValue.parseTimeValue(randomTimeValue(), "timeout"); - getDiscoveredNodesRequest.setTimeout(newTimeout); - assertThat("value updated", getDiscoveredNodesRequest.getTimeout(), equalTo(newTimeout)); - - final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, - () -> getDiscoveredNodesRequest.setTimeout(TimeValue.timeValueNanos(randomLongBetween(-10, -1)))); - assertThat(exception.getMessage(), startsWith("negative timeout of ")); - assertThat(exception.getMessage(), endsWith(" is not allowed")); - - getDiscoveredNodesRequest.setTimeout(null); - assertThat("value updated", getDiscoveredNodesRequest.getTimeout(), nullValue()); - } - - public void testSerialization() throws IOException { - final GetDiscoveredNodesRequest originalRequest = new GetDiscoveredNodesRequest(); - - if (randomBoolean()) { - originalRequest.setTimeout(TimeValue.parseTimeValue(randomTimeValue(), "timeout")); - } else if (randomBoolean()) { - originalRequest.setTimeout(null); - } - - final GetDiscoveredNodesRequest deserialized = copyWriteable(originalRequest, writableRegistry(), GetDiscoveredNodesRequest::new); - - assertThat(deserialized.getTimeout(), equalTo(originalRequest.getTimeout())); - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponseTests.java deleted file mode 100644 index 7d2fc602e66c6..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/GetDiscoveredNodesResponseTests.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.Version; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNode.Role; -import org.elasticsearch.common.UUIDs; -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.Collectors; - -import static java.util.Collections.emptyMap; -import static java.util.Collections.singleton; -import static org.hamcrest.Matchers.equalTo; - -public class GetDiscoveredNodesResponseTests extends ESTestCase { - public void testSerialization() throws IOException { - final GetDiscoveredNodesResponse original = new GetDiscoveredNodesResponse(randomDiscoveryNodeSet()); - final GetDiscoveredNodesResponse deserialized = copyWriteable(original, writableRegistry(), GetDiscoveredNodesResponse::new); - assertThat(deserialized.getNodes(), equalTo(original.getNodes())); - } - - private Set randomDiscoveryNodeSet() { - final int size = randomIntBetween(1, 10); - final Set nodes = new HashSet<>(size); - while (nodes.size() < size) { - assertTrue(nodes.add(new DiscoveryNode(randomAlphaOfLength(10), randomAlphaOfLength(10), - UUIDs.randomBase64UUID(random()), randomAlphaOfLength(10), randomAlphaOfLength(10), buildNewFakeTransportAddress(), - emptyMap(), singleton(Role.MASTER), Version.CURRENT))); - } - return nodes; - } - - public void testConversionToBootstrapConfiguration() { - final Set nodes = randomDiscoveryNodeSet(); - assertThat(new GetDiscoveredNodesResponse(nodes).getBootstrapConfiguration().resolve(nodes).getNodeIds(), - equalTo(nodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()))); - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterActionTests.java deleted file mode 100644 index 31486a52bd08f..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportBootstrapClusterActionTests.java +++ /dev/null @@ -1,233 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.Version; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration.NodeDescription; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ESAllocationTestCase; -import org.elasticsearch.cluster.coordination.Coordinator; -import org.elasticsearch.cluster.coordination.InMemoryPersistedState; -import org.elasticsearch.cluster.coordination.NoOpClusterApplier; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.service.MasterService; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.discovery.Discovery; -import org.elasticsearch.discovery.DiscoveryModule; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.transport.MockTransport; -import org.elasticsearch.threadpool.TestThreadPool; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.threadpool.ThreadPool.Names; -import org.elasticsearch.transport.TransportException; -import org.elasticsearch.transport.TransportResponseHandler; -import org.elasticsearch.transport.TransportService; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; - -import java.io.IOException; -import java.util.Collections; -import java.util.Random; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; - -import static java.util.Collections.emptyList; -import static java.util.Collections.emptyMap; -import static java.util.Collections.emptySet; -import static java.util.Collections.singletonList; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyZeroInteractions; - -public class TransportBootstrapClusterActionTests extends ESTestCase { - - private static final ActionFilters EMPTY_FILTERS = new ActionFilters(emptySet()); - - private DiscoveryNode discoveryNode; - private static ThreadPool threadPool; - private TransportService transportService; - private Coordinator coordinator; - - private static BootstrapClusterRequest exampleRequest() { - return new BootstrapClusterRequest(new BootstrapConfiguration(singletonList(new NodeDescription("id", "name")))); - } - - @BeforeClass - public static void createThreadPool() { - threadPool = new TestThreadPool("test", Settings.EMPTY); - } - - @AfterClass - public static void shutdownThreadPool() { - threadPool.shutdown(); - } - - @Before - public void setupTest() { - discoveryNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); - final MockTransport transport = new MockTransport(); - transportService = transport.createTransportService(Settings.EMPTY, threadPool, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> discoveryNode, null, emptySet()); - - final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - coordinator = new Coordinator("local", Settings.EMPTY, clusterSettings, transportService, writableRegistry(), - ESAllocationTestCase.createAllocationService(Settings.EMPTY), - new MasterService("local", Settings.EMPTY, threadPool), - () -> new InMemoryPersistedState(0, ClusterState.builder(new ClusterName("cluster")).build()), r -> emptyList(), - new NoOpClusterApplier(), Collections.emptyList(), new Random(random().nextLong())); - } - - public void testHandlesNonstandardDiscoveryImplementation() throws InterruptedException { - final Discovery discovery = mock(Discovery.class); - verifyZeroInteractions(discovery); - - final String nonstandardDiscoveryType = randomFrom(DiscoveryModule.ZEN_DISCOVERY_TYPE, "single-node", "unknown"); - new TransportBootstrapClusterAction( - Settings.builder().put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), nonstandardDiscoveryType).build(), - EMPTY_FILTERS, transportService, discovery); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(discoveryNode, BootstrapClusterAction.NAME, exampleRequest(), new ResponseHandler() { - @Override - public void handleResponse(BootstrapClusterResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - assertThat(rootCause, instanceOf(IllegalArgumentException.class)); - assertThat(rootCause.getMessage(), equalTo("cluster bootstrapping is not supported by discovery type [" + - nonstandardDiscoveryType + "]")); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testFailsOnNonMasterEligibleNodes() throws InterruptedException { - discoveryNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); - // transport service only picks up local node when started, so we can change it here ^ - - new TransportBootstrapClusterAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(discoveryNode, BootstrapClusterAction.NAME, exampleRequest(), new ResponseHandler() { - @Override - public void handleResponse(BootstrapClusterResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - assertThat(rootCause, instanceOf(IllegalArgumentException.class)); - assertThat(rootCause.getMessage(), - equalTo("this node is not master-eligible, but cluster bootstrapping can only happen on a master-eligible node")); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testSetsInitialConfiguration() throws InterruptedException { - new TransportBootstrapClusterAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - - assertFalse(coordinator.isInitialConfigurationSet()); - - final BootstrapClusterRequest request - = new BootstrapClusterRequest(new BootstrapConfiguration(singletonList(new NodeDescription(discoveryNode)))); - - { - final int parallelRequests = 10; - final CountDownLatch countDownLatch = new CountDownLatch(parallelRequests); - final AtomicInteger successes = new AtomicInteger(); - - for (int i = 0; i < parallelRequests; i++) { - transportService.sendRequest(discoveryNode, BootstrapClusterAction.NAME, request, new ResponseHandler() { - @Override - public void handleResponse(BootstrapClusterResponse response) { - if (response.getAlreadyBootstrapped() == false) { - successes.incrementAndGet(); - } - countDownLatch.countDown(); - } - - @Override - public void handleException(TransportException exp) { - throw new AssertionError("should not be called", exp); - } - }); - } - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - assertThat(successes.get(), equalTo(1)); - } - - assertTrue(coordinator.isInitialConfigurationSet()); - - { - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(discoveryNode, BootstrapClusterAction.NAME, request, new ResponseHandler() { - @Override - public void handleResponse(BootstrapClusterResponse response) { - assertTrue(response.getAlreadyBootstrapped()); - countDownLatch.countDown(); - } - - @Override - public void handleException(TransportException exp) { - throw new AssertionError("should not be called", exp); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - } - - private abstract class ResponseHandler implements TransportResponseHandler { - @Override - public String executor() { - return Names.SAME; - } - - @Override - public BootstrapClusterResponse read(StreamInput in) throws IOException { - return new BootstrapClusterResponse(in); - } - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesActionTests.java deleted file mode 100644 index 6d94dcf6eca14..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/bootstrap/TransportGetDiscoveredNodesActionTests.java +++ /dev/null @@ -1,533 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.bootstrap; - -import org.elasticsearch.ElasticsearchTimeoutException; -import org.elasticsearch.Version; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ESAllocationTestCase; -import org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException; -import org.elasticsearch.cluster.coordination.ClusterBootstrapService; -import org.elasticsearch.cluster.coordination.CoordinationMetaData; -import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; -import org.elasticsearch.cluster.coordination.Coordinator; -import org.elasticsearch.cluster.coordination.InMemoryPersistedState; -import org.elasticsearch.cluster.coordination.NoOpClusterApplier; -import org.elasticsearch.cluster.coordination.PeersResponse; -import org.elasticsearch.cluster.coordination.PublicationTransportHandler; -import org.elasticsearch.cluster.coordination.PublishWithJoinResponse; -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.cluster.service.MasterService; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.discovery.Discovery; -import org.elasticsearch.discovery.DiscoveryModule; -import org.elasticsearch.discovery.PeersRequest; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.transport.MockTransport; -import org.elasticsearch.threadpool.TestThreadPool; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.threadpool.ThreadPool.Names; -import org.elasticsearch.transport.BytesTransportRequest; -import org.elasticsearch.transport.TransportException; -import org.elasticsearch.transport.TransportRequest; -import org.elasticsearch.transport.TransportResponseHandler; -import org.elasticsearch.transport.TransportService; -import org.elasticsearch.transport.TransportService.HandshakeResponse; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.EnumSet; -import java.util.Random; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; - -import static java.util.Collections.emptyList; -import static java.util.Collections.emptyMap; -import static java.util.Collections.emptySet; -import static java.util.Collections.singleton; -import static java.util.Collections.singletonList; -import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; -import static org.elasticsearch.discovery.PeerFinder.REQUEST_PEERS_ACTION_NAME; -import static org.elasticsearch.transport.TransportService.HANDSHAKE_ACTION_NAME; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.startsWith; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyZeroInteractions; - -public class TransportGetDiscoveredNodesActionTests extends ESTestCase { - - private static final ActionFilters EMPTY_FILTERS = new ActionFilters(emptySet()); - - private static ThreadPool threadPool; - private DiscoveryNode localNode; - private String clusterName; - private TransportService transportService; - private Coordinator coordinator; - private DiscoveryNode otherNode; - - @BeforeClass - public static void createThreadPool() { - threadPool = new TestThreadPool("test", Settings.EMPTY); - } - - @AfterClass - public static void shutdownThreadPool() { - threadPool.shutdown(); - } - - @Before - public void setupTest() { - clusterName = randomAlphaOfLength(10); - localNode = new DiscoveryNode( - "node1", "local", buildNewFakeTransportAddress(), emptyMap(), EnumSet.allOf(DiscoveryNode.Role.class), Version.CURRENT); - otherNode = new DiscoveryNode( - "node2", "other", buildNewFakeTransportAddress(), emptyMap(), EnumSet.allOf(DiscoveryNode.Role.class), Version.CURRENT); - - final MockTransport transport = new MockTransport() { - @Override - protected void onSendRequest(long requestId, String action, TransportRequest request, DiscoveryNode node) { - if (action.equals(HANDSHAKE_ACTION_NAME) && node.getAddress().equals(otherNode.getAddress())) { - handleResponse(requestId, new HandshakeResponse(otherNode, new ClusterName(clusterName), Version.CURRENT)); - } - } - }; - transportService = transport.createTransportService( - Settings.builder().put(CLUSTER_NAME_SETTING.getKey(), clusterName).build(), threadPool, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> localNode, null, emptySet()); - - final Settings settings = Settings.builder() - .putList(ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.getKey(), - ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY)).build(); // suppress auto-bootstrap - - final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - coordinator = new Coordinator("local", settings, clusterSettings, transportService, writableRegistry(), - ESAllocationTestCase.createAllocationService(settings), - new MasterService("local", settings, threadPool), - () -> new InMemoryPersistedState(0, ClusterState.builder(new ClusterName(clusterName)).build()), r -> emptyList(), - new NoOpClusterApplier(), Collections.emptyList(), new Random(random().nextLong())); - } - - public void testHandlesNonstandardDiscoveryImplementation() throws InterruptedException { - final Discovery discovery = mock(Discovery.class); - verifyZeroInteractions(discovery); - - final String nonstandardDiscoveryType = randomFrom(DiscoveryModule.ZEN_DISCOVERY_TYPE, "single-node", "unknown"); - new TransportGetDiscoveredNodesAction( - Settings.builder().put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), nonstandardDiscoveryType).build(), - EMPTY_FILTERS, transportService, discovery); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, new GetDiscoveredNodesRequest(), new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - assertThat(rootCause, instanceOf(IllegalArgumentException.class)); - assertThat(rootCause.getMessage(), equalTo("discovered nodes are not exposed by discovery type [" + - nonstandardDiscoveryType + "]")); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testFailsOnMasterIneligibleNodes() throws InterruptedException { - localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); - // transport service only picks up local node when started, so we can change it here ^ - - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, new GetDiscoveredNodesRequest(), new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - assertThat(rootCause, instanceOf(IllegalArgumentException.class)); - assertThat(rootCause.getMessage(), - equalTo("this node is not master-eligible, but discovered nodes are only exposed by master-eligible nodes")); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testFailsQuicklyWithZeroTimeoutAndAcceptsNullTimeout() throws InterruptedException { - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - - { - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setTimeout(null); - getDiscoveredNodesRequest.setRequiredNodes(singletonList("not-a-node")); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - throw new AssertionError("should not be called", exp); - } - }); - } - - { - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - getDiscoveredNodesRequest.setRequiredNodes(singletonList("not-a-node")); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - final Throwable rootCause = exp.getRootCause(); - assertThat(rootCause, instanceOf(ElasticsearchTimeoutException.class)); - assertThat(rootCause.getMessage(), startsWith("timed out while waiting for GetDiscoveredNodesRequest{")); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - } - - public void testFailsIfAlreadyBootstrapped() throws InterruptedException { - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - coordinator.setInitialConfiguration(new VotingConfiguration(singleton(localNode.getId()))); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setTimeout(null); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - if (exp.getRootCause() instanceof ClusterAlreadyBootstrappedException) { - countDownLatch.countDown(); - } else { - throw new AssertionError("should not be called", exp); - } - } - }); - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testFailsIfAcceptsClusterStateWithNonemptyConfiguration() throws InterruptedException, IOException { - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setTimeout(null); - getDiscoveredNodesRequest.setRequiredNodes(singletonList("not-a-node")); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - if (exp.getRootCause() instanceof ClusterAlreadyBootstrappedException) { - countDownLatch.countDown(); - } else { - throw new AssertionError("should not be called", exp); - } - } - }); - - ClusterState.Builder publishedClusterState = ClusterState.builder(ClusterName.DEFAULT); - publishedClusterState.incrementVersion(); - publishedClusterState.nodes(DiscoveryNodes.builder() - .add(localNode).add(otherNode).localNodeId(localNode.getId()).masterNodeId(otherNode.getId())); - publishedClusterState.metaData(MetaData.builder().coordinationMetaData(CoordinationMetaData.builder() - .term(1) - .lastAcceptedConfiguration(new VotingConfiguration(singleton(otherNode.getId()))) - .lastCommittedConfiguration(new VotingConfiguration(singleton(otherNode.getId()))) - .build())); - - transportService.sendRequest(localNode, PublicationTransportHandler.PUBLISH_STATE_ACTION_NAME, - new BytesTransportRequest(PublicationTransportHandler.serializeFullClusterState(publishedClusterState.build(), Version.CURRENT), - Version.CURRENT), - new TransportResponseHandler() { - @Override - public void handleResponse(PublishWithJoinResponse response) { - // do nothing - } - - @Override - public void handleException(TransportException exp) { - throw new AssertionError("should not be called", exp); - } - - @Override - public String executor() { - return Names.SAME; - } - - @Override - public PublishWithJoinResponse read(StreamInput in) throws IOException { - return new PublishWithJoinResponse(in); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - public void testGetsDiscoveredNodesWithZeroTimeout() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionMet(getDiscoveredNodesRequest); - } - - public void testGetsDiscoveredNodesByAddress() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(localNode.getAddress().toString(), otherNode.getAddress().toString())); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionMet(getDiscoveredNodesRequest); - } - - public void testGetsDiscoveredNodesByName() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(localNode.getName(), otherNode.getName())); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionMet(getDiscoveredNodesRequest); - } - - public void testGetsDiscoveredNodesByIP() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - String ip = localNode.getAddress().getAddress(); - getDiscoveredNodesRequest.setRequiredNodes(Collections.singletonList(ip)); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionFailedOnDuplicate(getDiscoveredNodesRequest, '[' + ip + "] matches ["); - } - - public void testGetsDiscoveredNodesDuplicateName() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - String name = localNode.getName(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(name, name)); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionFailedOnDuplicate(getDiscoveredNodesRequest, "[" + localNode + "] matches [" + name + ", " + name + ']'); - } - - public void testGetsDiscoveredNodesWithDuplicateMatchNameAndAddress() throws InterruptedException { - setupGetDiscoveredNodesAction(); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(localNode.getAddress().toString(), localNode.getName())); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - assertWaitConditionFailedOnDuplicate(getDiscoveredNodesRequest, "[" + localNode + "] matches ["); - } - - public void testGetsDiscoveredNodesTimeoutOnMissing() throws InterruptedException { - setupGetDiscoveredNodesAction(); - - final CountDownLatch latch = new CountDownLatch(1); - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(localNode.getAddress().toString(), "_missing")); - getDiscoveredNodesRequest.setTimeout(TimeValue.ZERO); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - assertThat(exp.getRootCause(), instanceOf(ElasticsearchTimeoutException.class)); - latch.countDown(); - } - }); - - latch.await(10L, TimeUnit.SECONDS); - } - - public void testThrowsExceptionIfDuplicateDiscoveredLater() throws InterruptedException { - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - final String ip = localNode.getAddress().getAddress(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(ip, "not-a-node")); - - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - Throwable t = exp.getRootCause(); - assertThat(t, instanceOf(IllegalArgumentException.class)); - assertThat(t.getMessage(), startsWith('[' + ip + "] matches [")); - countDownLatch.countDown(); - } - }); - - executeRequestPeersAction(); - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - private void executeRequestPeersAction() { - threadPool.generic().execute(() -> - transportService.sendRequest(localNode, REQUEST_PEERS_ACTION_NAME, new PeersRequest(otherNode, emptyList()), - new TransportResponseHandler() { - @Override - public PeersResponse read(StreamInput in) throws IOException { - return new PeersResponse(in); - } - - @Override - public void handleResponse(PeersResponse response) { - } - - @Override - public void handleException(TransportException exp) { - } - - @Override - public String executor() { - return Names.SAME; - } - })); - } - - private void setupGetDiscoveredNodesAction() throws InterruptedException { - new TransportGetDiscoveredNodesAction(Settings.EMPTY, EMPTY_FILTERS, transportService, coordinator); // registers action - transportService.start(); - transportService.acceptIncomingRequests(); - coordinator.start(); - coordinator.startInitialJoin(); - - executeRequestPeersAction(); - - final GetDiscoveredNodesRequest getDiscoveredNodesRequest = new GetDiscoveredNodesRequest(); - getDiscoveredNodesRequest.setRequiredNodes(Arrays.asList(localNode.getName(), otherNode.getName())); - assertWaitConditionMet(getDiscoveredNodesRequest); - } - - private void assertWaitConditionMet(GetDiscoveredNodesRequest getDiscoveredNodesRequest) throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - assertThat(response.getNodes(), containsInAnyOrder(localNode, otherNode)); - countDownLatch.countDown(); - } - - @Override - public void handleException(TransportException exp) { - throw new AssertionError("should not be called", exp); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - private void assertWaitConditionFailedOnDuplicate(GetDiscoveredNodesRequest getDiscoveredNodesRequest, String message) - throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(1); - transportService.sendRequest(localNode, GetDiscoveredNodesAction.NAME, getDiscoveredNodesRequest, new ResponseHandler() { - @Override - public void handleResponse(GetDiscoveredNodesResponse response) { - throw new AssertionError("should not be called"); - } - - @Override - public void handleException(TransportException exp) { - Throwable t = exp.getRootCause(); - assertThat(t, instanceOf(IllegalArgumentException.class)); - assertThat(t.getMessage(), startsWith(message)); - countDownLatch.countDown(); - } - }); - - assertTrue(countDownLatch.await(10, TimeUnit.SECONDS)); - } - - private abstract class ResponseHandler implements TransportResponseHandler { - @Override - public String executor() { - return Names.SAME; - } - - @Override - public GetDiscoveredNodesResponse read(StreamInput in) throws IOException { - return new GetDiscoveredNodesResponse(in); - } - } -} diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 542247c058861..0eef2cb49278f 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -20,49 +20,43 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterAction; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterRequest; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapClusterResponse; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration.NodeDescription; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesAction; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesRequest; -import org.elasticsearch.action.admin.cluster.bootstrap.GetDiscoveredNodesResponse; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode.Role; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.Settings.Builder; -import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.transport.MockTransport; -import org.elasticsearch.threadpool.ThreadPool.Names; -import org.elasticsearch.transport.TransportChannel; import org.elasticsearch.transport.TransportRequest; -import org.elasticsearch.transport.TransportRequestHandler; import org.elasticsearch.transport.TransportService; import org.junit.Before; -import java.util.Set; +import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; +import static java.util.Collections.singletonMap; import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; +import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; import static org.elasticsearch.common.settings.Settings.builder; import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; import static org.elasticsearch.node.Node.NODE_NAME_SETTING; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; public class ClusterBootstrapServiceTests extends ESTestCase { private DiscoveryNode localNode, otherNode1, otherNode2; private DeterministicTaskQueue deterministicTaskQueue; private TransportService transportService; - private ClusterBootstrapService clusterBootstrapService; @Before public void createServices() { @@ -81,10 +75,6 @@ protected void onSendRequest(long requestId, String action, TransportRequest req transportService = transport.createTransportService(Settings.EMPTY, deterministicTaskQueue.getThreadPool(), TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> localNode, null, emptySet()); - - clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), - localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService); } private DiscoveryNode newDiscoveryNode(String nodeName) { @@ -92,152 +82,258 @@ private DiscoveryNode newDiscoveryNode(String nodeName) { Version.CURRENT); } - private void startServices() { - transportService.start(); - transportService.acceptIncomingRequests(); - clusterBootstrapService.start(); - } + public void testBootstrapsAutomaticallyWithDefaultConfiguration() { + final Settings.Builder settings = Settings.builder(); + final long timeout; + if (randomBoolean()) { + timeout = UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(Settings.EMPTY).millis(); + } else { + timeout = randomLongBetween(1, 10000); + settings.put(UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.getKey(), timeout + "ms"); + } + + final AtomicReference>> discoveredNodesSupplier = new AtomicReference<>(() -> { + throw new AssertionError("should not be called yet"); + }); + + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService + = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), + equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); + }); - public void testDoesNothingOnNonMasterNodes() { - localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> { - throw new AssertionError("should not make a discovery request"); - }); + deterministicTaskQueue.scheduleAt(timeout - 1, + () -> discoveredNodesSupplier.set(() -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()))); - startServices(); - deterministicTaskQueue.runAllTasks(); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + deterministicTaskQueue.runAllTasksInTimeOrder(); + assertTrue(bootstrapped.get()); } public void testDoesNothingByDefaultIfHostsProviderConfigured() { - testConfiguredIfSettingSet(builder().putList(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey())); + testDoesNothingWithSettings(builder().putList(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey())); } public void testDoesNothingByDefaultIfUnicastHostsConfigured() { - testConfiguredIfSettingSet(builder().putList(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey())); + testDoesNothingWithSettings(builder().putList(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey())); } public void testDoesNothingByDefaultIfMasterNodesConfigured() { - testConfiguredIfSettingSet(builder().putList(INITIAL_MASTER_NODES_SETTING.getKey())); + testDoesNothingWithSettings(builder().putList(INITIAL_MASTER_NODES_SETTING.getKey())); } - private void testConfiguredIfSettingSet(Builder builder) { - clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> { - throw new AssertionError("should not make a discovery request"); - }); - startServices(); - deterministicTaskQueue.runAllTasks(); + public void testDoesNothingByDefaultOnMasterIneligibleNodes() { + localNode = new DiscoveryNode("local", randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), emptySet(), + Version.CURRENT); + testDoesNothingWithSettings(Settings.builder()); } - public void testBootstrapsAutomaticallyWithDefaultConfiguration() { - clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService); + private void testDoesNothingWithSettings(Settings.Builder builder) { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, () -> { + throw new AssertionError("should not be called"); + }, vc -> { + throw new AssertionError("should not be called"); + }); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + deterministicTaskQueue.runAllTasks(); + } - final Set discoveredNodes = Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> channel.sendResponse(new GetDiscoveredNodesResponse(discoveredNodes))); + public void testDoesNothingByDefaultIfZen1NodesDiscovered() { + final DiscoveryNode zen1Node = new DiscoveryNode("zen1", buildNewFakeTransportAddress(), singletonMap("zen1", "true"), + singleton(Role.MASTER), Version.CURRENT); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, () -> + Stream.of(localNode, zen1Node).collect(Collectors.toSet()), vc -> { + throw new AssertionError("should not be called"); + }); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + deterministicTaskQueue.runAllTasks(); + } + public void testBootstrapsOnDiscoverySuccess() { final AtomicBoolean bootstrapped = new AtomicBoolean(); - transportService.registerRequestHandler(BootstrapClusterAction.NAME, Names.SAME, BootstrapClusterRequest::new, - (request, channel, task) -> { - assertThat(request.getBootstrapConfiguration().getNodeDescriptions().stream() - .map(NodeDescription::getId).collect(Collectors.toSet()), - equalTo(discoveredNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()))); - channel.sendResponse(new BootstrapClusterResponse(randomBoolean())); - assertTrue(bootstrapped.compareAndSet(false, true)); - }); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), hasSize(3)); + }); - startServices(); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testDoesNotBootstrapsOnNonMasterNode() { + localNode = new DiscoveryNode("local", randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), emptySet(), + Version.CURRENT); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> + Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + throw new AssertionError("should not be called"); + }); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); } - public void testDoesNotRetryOnDiscoveryFailure() { - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - new TransportRequestHandler() { - private boolean called = false; + public void testDoesNotBootstrapsIfNotConfigured() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, + () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + throw new AssertionError("should not be called"); + }); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapsIfZen1NodesDiscovered() { + final DiscoveryNode zen1Node = new DiscoveryNode("zen1", buildNewFakeTransportAddress(), singletonMap("zen1", "true"), + singleton(Role.MASTER), Version.CURRENT); - @Override - public void messageReceived(GetDiscoveredNodesRequest request, TransportChannel channel, Task task) { - assert called == false; - called = true; - throw new IllegalArgumentException("simulate failure of discovery request"); - } - }); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), vc -> { + throw new AssertionError("should not be called"); + }); - startServices(); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); } - public void testBootstrapsOnDiscoverySuccess() { - final AtomicBoolean discoveryAttempted = new AtomicBoolean(); - final Set discoveredNodes = Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> { - assertTrue(discoveryAttempted.compareAndSet(false, true)); - channel.sendResponse(new GetDiscoveredNodesResponse(discoveredNodes)); - }); - - final AtomicBoolean bootstrapAttempted = new AtomicBoolean(); - transportService.registerRequestHandler(BootstrapClusterAction.NAME, Names.SAME, BootstrapClusterRequest::new, - (request, channel, task) -> { - assertTrue(bootstrapAttempted.compareAndSet(false, true)); - channel.sendResponse(new BootstrapClusterResponse(false)); - }); - - startServices(); - deterministicTaskQueue.runAllTasks(); - - assertTrue(discoveryAttempted.get()); - assertTrue(bootstrapAttempted.get()); - } - - public void testRetriesOnBootstrapFailure() { - final Set discoveredNodes = Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> channel.sendResponse(new GetDiscoveredNodesResponse(discoveredNodes))); - - AtomicLong callCount = new AtomicLong(); - transportService.registerRequestHandler(BootstrapClusterAction.NAME, Names.SAME, BootstrapClusterRequest::new, - (request, channel, task) -> { - callCount.incrementAndGet(); - channel.sendResponse(new ElasticsearchException("simulated exception")); - }); - - startServices(); - while (callCount.get() < 5) { - if (deterministicTaskQueue.hasDeferredTasks()) { - deterministicTaskQueue.advanceTime(); + public void testRetriesBootstrappingOnException() { + + final AtomicLong bootstrappingAttempts = new AtomicLong(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + bootstrappingAttempts.incrementAndGet(); + if (bootstrappingAttempts.get() < 5L) { + throw new ElasticsearchException("test"); } - deterministicTaskQueue.runAllRunnableTasks(); - } + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertThat(bootstrappingAttempts.get(), greaterThanOrEqualTo(5L)); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); } - public void testStopsRetryingBootstrapWhenStopped() { - final Set discoveredNodes = Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()); - transportService.registerRequestHandler(GetDiscoveredNodesAction.NAME, Names.SAME, GetDiscoveredNodesRequest::new, - (request, channel, task) -> channel.sendResponse(new GetDiscoveredNodesResponse(discoveredNodes))); + public void testDoesNotBootstrapIfRequirementNotMet() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node").build(), + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + throw new AssertionError("should not be called"); + }); - transportService.registerRequestHandler(BootstrapClusterAction.NAME, Names.SAME, BootstrapClusterRequest::new, - (request, channel, task) -> channel.sendResponse(new ElasticsearchException("simulated exception"))); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } - deterministicTaskQueue.scheduleAt(deterministicTaskQueue.getCurrentTimeMillis() + 200000, new Runnable() { - @Override - public void run() { - clusterBootstrapService.stop(); - } + public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { + AtomicReference> discoveredNodes + = new AtomicReference<>(Stream.of(otherNode1, otherNode2).collect(Collectors.toList())); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), + transportService, discoveredNodes::get, vc -> { + throw new AssertionError("should not be called"); + }); - @Override - public String toString() { - return "stop cluster bootstrap service"; - } + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + + discoveredNodes.set(emptyList()); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { + AtomicReference> discoveredNodes + = new AtomicReference<>(Stream.of(otherNode1, otherNode2).collect(Collectors.toList())); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) + .build(), + transportService, discoveredNodes::get, vc -> { + throw new AssertionError("should not be called"); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + + discoveredNodes.set(Stream.of(new DiscoveryNode(otherNode1.getName(), randomAlphaOfLength(10), buildNewFakeTransportAddress(), + emptyMap(), singleton(Role.MASTER), Version.CURRENT), + new DiscoveryNode("yet-another-node", randomAlphaOfLength(10), otherNode1.getAddress(), emptyMap(), singleton(Role.MASTER), + Version.CURRENT)).collect(Collectors.toList())); + + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testMatchesOnNodeName() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, + Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testMatchesOnNodeAddress() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), transportService, + Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testMatchesOnNodeHostAddress() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), + transportService, Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testDoesNotJustMatchEverything() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, + Collections::emptyList, vc -> { + throw new AssertionError("should not be called"); }); - startServices(); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); - // termination means success } } diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java index 98e21581d6f87..b0ac2f5a5482c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java @@ -26,7 +26,6 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; -import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; @@ -48,7 +47,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; @@ -79,7 +77,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; @@ -122,7 +119,6 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -713,70 +709,6 @@ public void testAckListenerReceivesNacksFromFollowerInHigherTerm() { // assertTrue("expected ack from " + follower1, ackCollector.hasAckedSuccessfully(follower1)); } - public void testDiscoveryOfPeersTriggersNotification() { - final Cluster cluster = new Cluster(randomIntBetween(2, 5)); - - // register a listener and then deregister it again to show that it is not called after deregistration - try (Releasable ignored = cluster.getAnyNode().coordinator.withDiscoveryListener(ActionListener.wrap(() -> { - throw new AssertionError("should not be called"); - }))) { - // do nothing - } - - final long startTimeMillis = cluster.deterministicTaskQueue.getCurrentTimeMillis(); - final ClusterNode bootstrapNode = cluster.getAnyNode(); - final AtomicBoolean hasDiscoveredAllPeers = new AtomicBoolean(); - assertFalse(bootstrapNode.coordinator.getFoundPeers().iterator().hasNext()); - try (Releasable ignored = bootstrapNode.coordinator.withDiscoveryListener( - new ActionListener>() { - @Override - public void onResponse(Iterable discoveryNodes) { - int peerCount = 0; - for (final DiscoveryNode discoveryNode : discoveryNodes) { - peerCount++; - } - assertThat(peerCount, lessThan(cluster.size())); - if (peerCount == cluster.size() - 1 && hasDiscoveredAllPeers.get() == false) { - hasDiscoveredAllPeers.set(true); - final long elapsedTimeMillis = cluster.deterministicTaskQueue.getCurrentTimeMillis() - startTimeMillis; - logger.info("--> {} discovered {} peers in {}ms", bootstrapNode.getId(), cluster.size() - 1, elapsedTimeMillis); - assertThat(elapsedTimeMillis, lessThanOrEqualTo(defaultMillis(DISCOVERY_FIND_PEERS_INTERVAL_SETTING) * 2)); - } - } - - @Override - public void onFailure(Exception e) { - throw new AssertionError("unexpected", e); - } - })) { - cluster.runFor(defaultMillis(DISCOVERY_FIND_PEERS_INTERVAL_SETTING) * 2 + randomLongBetween(0, 60000), "discovery phase"); - } - - assertTrue(hasDiscoveredAllPeers.get()); - - final AtomicBoolean receivedAlreadyBootstrappedException = new AtomicBoolean(); - try (Releasable ignored = bootstrapNode.coordinator.withDiscoveryListener( - new ActionListener>() { - @Override - public void onResponse(Iterable discoveryNodes) { - // ignore - } - - @Override - public void onFailure(Exception e) { - if (e instanceof ClusterAlreadyBootstrappedException) { - receivedAlreadyBootstrappedException.set(true); - } else { - throw new AssertionError("unexpected", e); - } - } - })) { - - cluster.stabilise(); - } - assertTrue(receivedAlreadyBootstrappedException.get()); - } - public void testSettingInitialConfigurationTriggersElection() { final Cluster cluster = new Cluster(randomIntBetween(1, 5)); cluster.runFor(defaultMillis(DISCOVERY_FIND_PEERS_INTERVAL_SETTING) * 2 + randomLongBetween(0, 60000), "initial discovery phase"); diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotsServiceTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotsServiceTests.java index b4dbbef65cce2..d1a5a8ddcd89a 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SnapshotsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotsServiceTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.cluster.bootstrap.BootstrapConfiguration; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutRepositoryAction; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotAction; @@ -45,13 +44,14 @@ import org.elasticsearch.cluster.SnapshotsInProgress; import org.elasticsearch.cluster.action.index.NodeMappingRefreshAction; import org.elasticsearch.cluster.action.shard.ShardStateAction; -import org.elasticsearch.cluster.coordination.ClusterBootstrapService; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; import org.elasticsearch.cluster.coordination.CoordinationState; import org.elasticsearch.cluster.coordination.Coordinator; import org.elasticsearch.cluster.coordination.CoordinatorTests; import org.elasticsearch.cluster.coordination.DeterministicTaskQueue; import org.elasticsearch.cluster.coordination.InMemoryPersistedState; import org.elasticsearch.cluster.coordination.MockSinglePrioritizingExecutor; +import org.elasticsearch.cluster.coordination.ClusterBootstrapService; import org.elasticsearch.cluster.metadata.AliasValidator; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; @@ -204,14 +204,10 @@ private void startCluster() { deterministicTaskQueue.advanceTime(); deterministicTaskQueue.runAllRunnableTasks(); - final BootstrapConfiguration bootstrapConfiguration = new BootstrapConfiguration( - testClusterNodes.nodes.values().stream().filter(n -> n.node.isMasterNode()) - .map(node -> new BootstrapConfiguration.NodeDescription(node.node)) - .distinct() - .collect(Collectors.toList())); + final VotingConfiguration votingConfiguration = new VotingConfiguration(testClusterNodes.nodes.values().stream().map(n -> n.node) + .filter(DiscoveryNode::isMasterNode).map(DiscoveryNode::getId).collect(Collectors.toSet())); testClusterNodes.nodes.values().stream().filter(n -> n.node.isMasterNode()).forEach( - testClusterNode -> testClusterNode.coordinator.setInitialConfiguration(bootstrapConfiguration) - ); + testClusterNode -> testClusterNode.coordinator.setInitialConfiguration(votingConfiguration)); runUntil( () -> { From 9742709d1629cd893a490328a1946b29b9901604 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 15 Jan 2019 11:21:17 +0000 Subject: [PATCH 02/23] Line length --- .../org/elasticsearch/cluster/coordination/Coordinator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index f2c082cb2ea16..8df66401db2c0 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -167,7 +167,8 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe this.clusterApplier = clusterApplier; masterService.setClusterStateSupplier(this::getStateForMasterService); this.reconfigurator = new Reconfigurator(settings, clusterSettings); - this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, this::setInitialConfiguration); + this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, + this::setInitialConfiguration); this.discoveryUpgradeService = new DiscoveryUpgradeService(settings, clusterSettings, transportService, this::isInitialConfigurationSet, joinHelper, peerFinder::getFoundPeers, this::setInitialConfiguration); this.lagDetector = new LagDetector(settings, transportService.getThreadPool(), n -> removeNode(n, "lagging"), From c4555ac16a880be1c6f493dfd6590a89ceed0749 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 15 Jan 2019 17:12:09 +0000 Subject: [PATCH 03/23] Assert that the bootstrap configuration contains only Zen2 master-eligible nodes --- .../cluster/coordination/ClusterBootstrapService.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index e62bc4b8380ee..9ae14852b16da 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -126,6 +126,8 @@ private Set getDiscoveredNodes() { } private void startBootstrap(Set discoveryNodes) { + assert discoveryNodes.stream().allMatch(DiscoveryNode::isMasterNode) : discoveryNodes; + assert discoveryNodes.stream().noneMatch(Coordinator::isZen1Node) : discoveryNodes; if (bootstrappingPermitted.compareAndSet(true, false)) { doBootstrap(new VotingConfiguration(discoveryNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()))); } From 61af11d0b8cea0051ca97c5e9172b23cda16ee88 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 15 Jan 2019 17:20:08 +0000 Subject: [PATCH 04/23] Clarify why bootstrapping was cancelled --- .../cluster/coordination/ClusterBootstrapService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 9ae14852b16da..f3653586a5ae6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -173,15 +173,15 @@ private boolean checkWaitRequirements(Set nodes) { } if (matchingNodes.size() > 1) { bootstrappingPermitted.set(false); - logger.warn("bootstrapping cancelled: [{}] matches {}", requirement, matchingNodes); + logger.warn("bootstrapping cancelled: requirement [{}] matches multiple nodes: {}", requirement, matchingNodes); return false; } for (final DiscoveryNode matchingNode : matchingNodes) { if (selectedNodes.add(matchingNode) == false) { bootstrappingPermitted.set(false); - logger.warn("bootstrapping cancelled: [{}] matches {}", matchingNode, initialMasterNodes.stream() - .filter(r -> matchesRequirement(matchingNode, requirement)).collect(Collectors.toList())); + logger.warn("bootstrapping cancelled: node [{}] matches multiple requirements: {}", matchingNode, + initialMasterNodes.stream().filter(r -> matchesRequirement(matchingNode, r)).collect(Collectors.toList())); return false; } } From 7d70a292ea2a189dd8f8b88d962a4ff42cb1562d Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 15 Jan 2019 17:28:47 +0000 Subject: [PATCH 05/23] Throw exception if requirements match multiple nodes or v.v. --- .../coordination/ClusterBootstrapService.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index f3653586a5ae6..e078549de3416 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -82,9 +82,20 @@ public static boolean discoveryIsConfigured(Settings settings) { void onFoundPeersUpdated() { final Set nodes = getDiscoveredNodes(); if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false - && nodes.stream().noneMatch(Coordinator::isZen1Node) && checkWaitRequirements(nodes)) { + && nodes.stream().noneMatch(Coordinator::isZen1Node)) { - startBootstrap(nodes); + final boolean waitRequirementsPassed; + try { + waitRequirementsPassed = checkWaitRequirements(nodes); + } catch (IllegalStateException e) { + logger.warn("bootstrapping cancelled", e); + bootstrappingPermitted.set(false); + return; + } + + if (waitRequirementsPassed) { + startBootstrap(nodes); + } } } @@ -172,17 +183,13 @@ private boolean checkWaitRequirements(Set nodes) { return false; } if (matchingNodes.size() > 1) { - bootstrappingPermitted.set(false); - logger.warn("bootstrapping cancelled: requirement [{}] matches multiple nodes: {}", requirement, matchingNodes); - return false; + throw new IllegalStateException("requirement [" + requirement + "] matches multiple nodes: " + matchingNodes); } for (final DiscoveryNode matchingNode : matchingNodes) { if (selectedNodes.add(matchingNode) == false) { - bootstrappingPermitted.set(false); - logger.warn("bootstrapping cancelled: node [{}] matches multiple requirements: {}", matchingNode, + throw new IllegalStateException("node [" + matchingNode + "] matches multiple requirements: " + initialMasterNodes.stream().filter(r -> matchesRequirement(matchingNode, r)).collect(Collectors.toList())); - return false; } } } From 2ff94a06747d034cebef26f1eb27d33754c43bd0 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 17 Jan 2019 11:54:04 +0000 Subject: [PATCH 06/23] Do not retry if bootstrapping throws an exception --- .../coordination/ClusterBootstrapService.java | 15 +-------------- .../ClusterBootstrapServiceTests.java | 15 ++++----------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index e078549de3416..618dca46cb3a7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -150,20 +150,7 @@ private void doBootstrap(VotingConfiguration votingConfiguration) { try { votingConfigurationConsumer.accept(votingConfiguration); } catch (Exception e) { - logger.warn(new ParameterizedMessage("exception when bootstrapping with {}, rescheduling", votingConfiguration), e); - transportService.getThreadPool().scheduleUnlessShuttingDown(TimeValue.timeValueSeconds(10), Names.GENERIC, - new Runnable() { - @Override - public void run() { - doBootstrap(votingConfiguration); - } - - @Override - public String toString() { - return "retry of failed bootstrapping with " + votingConfiguration; - } - } - ); + logger.warn(new ParameterizedMessage("failed to bootstrap with {}", votingConfiguration), e); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 0eef2cb49278f..d9004a8d6fc80 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -18,7 +18,6 @@ */ package org.elasticsearch.cluster.coordination; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode.Role; @@ -31,7 +30,6 @@ import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -217,23 +215,18 @@ public void testDoesNotBootstrapsIfZen1NodesDiscovered() { deterministicTaskQueue.runAllTasks(); } - public void testRetriesBootstrappingOnException() { - - final AtomicLong bootstrappingAttempts = new AtomicLong(); + public void testDoesNotRetryBootstrappingOnException() { + final AtomicBoolean bootstrappingAttempted = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { - bootstrappingAttempts.incrementAndGet(); - if (bootstrappingAttempts.get() < 5L) { - throw new ElasticsearchException("test"); - } + assertTrue(bootstrappingAttempted.compareAndSet(false, true)); }); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); - assertThat(bootstrappingAttempts.get(), greaterThanOrEqualTo(5L)); - assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); + assertTrue(bootstrappingAttempted.get()); } public void testDoesNotBootstrapIfRequirementNotMet() { From 526acbaf7b994759e84e6240eeae78d46759e2b5 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 17 Jan 2019 12:03:01 +0000 Subject: [PATCH 07/23] Do not include any extra nodes in the bootstrap configuration --- .../coordination/ClusterBootstrapService.java | 15 +++++++-------- .../ClusterBootstrapServiceTests.java | 19 ++++++++++++++++++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 618dca46cb3a7..90d462fda5e02 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -34,6 +34,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; @@ -84,18 +85,16 @@ void onFoundPeersUpdated() { if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false && nodes.stream().noneMatch(Coordinator::isZen1Node)) { - final boolean waitRequirementsPassed; + final Optional> nodesMatchingRequirements; try { - waitRequirementsPassed = checkWaitRequirements(nodes); + nodesMatchingRequirements = getNodesMatchingRequirements(nodes); } catch (IllegalStateException e) { logger.warn("bootstrapping cancelled", e); bootstrappingPermitted.set(false); return; } - if (waitRequirementsPassed) { - startBootstrap(nodes); - } + nodesMatchingRequirements.ifPresent(this::startBootstrap); } } @@ -160,14 +159,14 @@ private static boolean matchesRequirement(DiscoveryNode discoveryNode, String re || discoveryNode.getAddress().getAddress().equals(requirement); } - private boolean checkWaitRequirements(Set nodes) { + private Optional> getNodesMatchingRequirements(Set nodes) { final Set selectedNodes = new HashSet<>(); for (final String requirement : initialMasterNodes) { final Set matchingNodes = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet()); if (matchingNodes.isEmpty()) { - return false; + return Optional.empty(); } if (matchingNodes.size() > 1) { throw new IllegalStateException("requirement [" + requirement + "] matches multiple nodes: " + matchingNodes); @@ -181,6 +180,6 @@ private boolean checkWaitRequirements(Set nodes) { } } - return true; + return Optional.of(Collections.unmodifiableSet(selectedNodes)); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index d9004a8d6fc80..c04b69f9d1763 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -48,7 +48,9 @@ import static org.elasticsearch.node.Node.NODE_NAME_SETTING; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; public class ClusterBootstrapServiceTests extends ESTestCase { @@ -329,4 +331,19 @@ public void testDoesNotJustMatchEverything() { clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); } -} + + public void testDoesNotIncludeExtraNodes() { + final DiscoveryNode extraNode = newDiscoveryNode("extra-node"); + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + }} From 9d2bc207a07951a5f968f8d34a9ddfe027a9b705 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 17 Jan 2019 15:40:38 +0000 Subject: [PATCH 08/23] Revert "Do not retry if bootstrapping throws an exception" This reverts commit 2ff94a06747d034cebef26f1eb27d33754c43bd0. --- .../coordination/ClusterBootstrapService.java | 15 ++++++++++++++- .../ClusterBootstrapServiceTests.java | 15 +++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 90d462fda5e02..3627161fc4661 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -149,7 +149,20 @@ private void doBootstrap(VotingConfiguration votingConfiguration) { try { votingConfigurationConsumer.accept(votingConfiguration); } catch (Exception e) { - logger.warn(new ParameterizedMessage("failed to bootstrap with {}", votingConfiguration), e); + logger.warn(new ParameterizedMessage("exception when bootstrapping with {}, rescheduling", votingConfiguration), e); + transportService.getThreadPool().scheduleUnlessShuttingDown(TimeValue.timeValueSeconds(10), Names.GENERIC, + new Runnable() { + @Override + public void run() { + doBootstrap(votingConfiguration); + } + + @Override + public String toString() { + return "retry of failed bootstrapping with " + votingConfiguration; + } + } + ); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index c04b69f9d1763..1ab901431d35e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.cluster.coordination; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode.Role; @@ -30,6 +31,7 @@ import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -217,18 +219,23 @@ public void testDoesNotBootstrapsIfZen1NodesDiscovered() { deterministicTaskQueue.runAllTasks(); } - public void testDoesNotRetryBootstrappingOnException() { - final AtomicBoolean bootstrappingAttempted = new AtomicBoolean(); + public void testRetriesBootstrappingOnException() { + + final AtomicLong bootstrappingAttempts = new AtomicLong(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { - assertTrue(bootstrappingAttempted.compareAndSet(false, true)); + bootstrappingAttempts.incrementAndGet(); + if (bootstrappingAttempts.get() < 5L) { + throw new ElasticsearchException("test"); + } }); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrappingAttempted.get()); + assertThat(bootstrappingAttempts.get(), greaterThanOrEqualTo(5L)); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); } public void testDoesNotBootstrapIfRequirementNotMet() { From 8df9f666796176aa61e480572ecd0f6f6e443624 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 17 Jan 2019 16:02:43 +0000 Subject: [PATCH 09/23] Best-effort avoidance of bootstrapping if already bootstrapped --- .../coordination/ClusterBootstrapService.java | 7 ++- .../cluster/coordination/Coordinator.java | 5 +- .../ClusterBootstrapServiceTests.java | 47 ++++++++++++------- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 3627161fc4661..5e5367600f846 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -37,6 +37,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BooleanSupplier; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -62,16 +63,18 @@ public class ClusterBootstrapService { private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; private final Supplier> discoveredNodesSupplier; + private final BooleanSupplier isBootstrappedSupplier; private final Consumer votingConfigurationConsumer; private final AtomicBoolean bootstrappingPermitted = new AtomicBoolean(true); public ClusterBootstrapService(Settings settings, TransportService transportService, - Supplier> discoveredNodesSupplier, + Supplier> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier, Consumer votingConfigurationConsumer) { initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); this.transportService = transportService; this.discoveredNodesSupplier = discoveredNodesSupplier; + this.isBootstrappedSupplier = isBootstrappedSupplier; this.votingConfigurationConsumer = votingConfigurationConsumer; } @@ -83,7 +86,7 @@ public static boolean discoveryIsConfigured(Settings settings) { void onFoundPeersUpdated() { final Set nodes = getDiscoveredNodes(); if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false - && nodes.stream().noneMatch(Coordinator::isZen1Node)) { + && isBootstrappedSupplier.getAsBoolean() == false && nodes.stream().noneMatch(Coordinator::isZen1Node)) { final Optional> nodesMatchingRequirements; try { diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 8df66401db2c0..77abb33a0b333 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -168,7 +168,7 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe masterService.setClusterStateSupplier(this::getStateForMasterService); this.reconfigurator = new Reconfigurator(settings, clusterSettings); this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, - this::setInitialConfiguration); + this::isInitialConfigurationSet, this::setInitialConfiguration); this.discoveryUpgradeService = new DiscoveryUpgradeService(settings, clusterSettings, transportService, this::isInitialConfigurationSet, joinHelper, peerFinder::getFoundPeers, this::setInitialConfiguration); this.lagDetector = new LagDetector(settings, transportService.getThreadPool(), n -> removeNode(n, "lagging"), @@ -993,9 +993,8 @@ protected void onActiveMasterFound(DiscoveryNode masterNode, long term) { @Override protected void onFoundPeersUpdated() { - final Iterable foundPeers; synchronized (mutex) { - foundPeers = getFoundPeers(); + final Iterable foundPeers = getFoundPeers(); if (mode == Mode.CANDIDATE) { final CoordinationState.VoteCollection expectedVotes = new CoordinationState.VoteCollection(); foundPeers.forEach(expectedVotes::addVote); diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 1ab901431d35e..7cca05ab21443 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -100,7 +100,7 @@ public void testBootstrapsAutomaticallyWithDefaultConfiguration() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService - = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), vc -> { + = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); @@ -137,7 +137,7 @@ public void testDoesNothingByDefaultOnMasterIneligibleNodes() { private void testDoesNothingWithSettings(Settings.Builder builder) { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, () -> { throw new AssertionError("should not be called"); - }, vc -> { + }, () -> false, vc -> { throw new AssertionError("should not be called"); }); transportService.start(); @@ -149,7 +149,7 @@ public void testDoesNothingByDefaultIfZen1NodesDiscovered() { final DiscoveryNode zen1Node = new DiscoveryNode("zen1", buildNewFakeTransportAddress(), singletonMap("zen1", "true"), singleton(Role.MASTER), Version.CURRENT); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, () -> - Stream.of(localNode, zen1Node).collect(Collectors.toSet()), vc -> { + Stream.of(localNode, zen1Node).collect(Collectors.toSet()), () -> false, vc -> { throw new AssertionError("should not be called"); }); transportService.start(); @@ -162,7 +162,7 @@ public void testBootstrapsOnDiscoverySuccess() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), hasSize(3)); }); @@ -178,13 +178,25 @@ public void testBootstrapsOnDiscoverySuccess() { assertFalse(bootstrapped.get()); // should only bootstrap once } + public void testDoesNotBootstrapIfAlreadyBootstrapped() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> true, vc -> { + throw new AssertionError("should not be called"); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + public void testDoesNotBootstrapsOnNonMasterNode() { localNode = new DiscoveryNode("local", randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), transportService, () -> - Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); transportService.start(); @@ -195,7 +207,7 @@ public void testDoesNotBootstrapsOnNonMasterNode() { public void testDoesNotBootstrapsIfNotConfigured() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, - () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); transportService.start(); @@ -210,7 +222,7 @@ public void testDoesNotBootstrapsIfZen1NodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -224,7 +236,7 @@ public void testRetriesBootstrappingOnException() { final AtomicLong bootstrappingAttempts = new AtomicLong(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { bootstrappingAttempts.incrementAndGet(); if (bootstrappingAttempts.get() < 5L) { throw new ElasticsearchException("test"); @@ -241,7 +253,7 @@ public void testRetriesBootstrappingOnException() { public void testDoesNotBootstrapIfRequirementNotMet() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node").build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -255,7 +267,7 @@ public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { = new AtomicReference<>(Stream.of(otherNode1, otherNode2).collect(Collectors.toList())); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, discoveredNodes::get, vc -> { + transportService, discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -274,7 +286,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) .build(), - transportService, discoveredNodes::get, vc -> { + transportService, discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -295,7 +307,7 @@ public void testMatchesOnNodeName() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, - Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -307,7 +319,7 @@ public void testMatchesOnNodeAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), transportService, - Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -319,7 +331,7 @@ public void testMatchesOnNodeHostAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, Collections::emptyList, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + transportService, Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -330,7 +342,7 @@ public void testMatchesOnNodeHostAddress() { public void testDoesNotJustMatchEverything() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, - Collections::emptyList, vc -> { + Collections::emptyList, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -344,7 +356,7 @@ public void testDoesNotIncludeExtraNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); }); @@ -353,4 +365,5 @@ public void testDoesNotIncludeExtraNodes() { clusterBootstrapService.onFoundPeersUpdated(); deterministicTaskQueue.runAllTasks(); assertTrue(bootstrapped.get()); - }} + } +} From 7a75c0b53f321cc0c31bbac11eb03c31d32b41ff Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 18 Jan 2019 09:51:14 +0000 Subject: [PATCH 10/23] Line length --- .../coordination/ClusterBootstrapServiceTests.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 7cca05ab21443..57973e39baf02 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -100,12 +100,13 @@ public void testBootstrapsAutomaticallyWithDefaultConfiguration() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService - = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), () -> false, vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), - equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); - assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); - }); + = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), + equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); + }); deterministicTaskQueue.scheduleAt(timeout - 1, () -> discoveredNodesSupplier.set(() -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()))); From 1c105742e98356b62e05e8f868e5e23891d5fe10 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 18 Jan 2019 13:21:06 +0000 Subject: [PATCH 11/23] Bootstrap a Zen2 cluster once quorum is discovered Today when bootstrapping a Zen2 cluster we wait for every node in the `initial_master_nodes` setting to be discovered, so that we can map the node names or addresses in the `initial_master_nodes` list to their IDs for inclusion in the initial voting configuration. This means that if any of the expected master-eligible nodes fails to start then bootstrapping will not occur and the cluster will not form. This is not ideal, and we would prefer the cluster to bootstrap even if some of the master-eligible nodes do not start. Safe bootstrapping requires that all pairs of quorums of all initial configurations overlap, and this is particularly troublesome to ensure given that nodes may be concurrently and independently attempting to bootstrap the cluster. The solution is to bootstrap using an initial configuration whose size matches the size of the expected set of master-eligible nodes, but with the unknown IDs replaced by "placeholder" IDs that can never belong to any node. Any quorum of received votes in any of these placeholder-laden initial configurations is also a quorum of the "true" initial set of master-eligible nodes, giving the guarantee that it intersects all other quorums that we require. Note that this change means that the initial configuration is not necessarily robust to any node failures. Normally the cluster will form and then auto-reconfigure to a more robust configuration in which the placeholder IDs are replaced by the IDs of genuine nodes as they join the cluster; however if a node fails between bootstrapping and this auto-reconfiguration then the cluster may become unavailable. This we feel to be less likely than a node failing to start at all. --- .../coordination/ClusterBootstrapService.java | 31 ++-- .../ClusterFormationFailureHelper.java | 17 +- .../cluster/coordination/Coordinator.java | 2 +- .../ClusterBootstrapServiceTests.java | 162 ++++++++++++++---- .../ClusterFormationFailureHelperTests.java | 22 +++ 5 files changed, 183 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 5e5367600f846..c6cef6163cdcb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -24,6 +24,7 @@ import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -34,7 +35,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Optional; +import java.util.Random; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BooleanSupplier; @@ -57,22 +58,26 @@ public class ClusterBootstrapService { Setting.timeSetting("discovery.unconfigured_bootstrap_timeout", TimeValue.timeValueSeconds(3), TimeValue.timeValueMillis(1), Property.NodeScope); + static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}"; + private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); private final List initialMasterNodes; @Nullable // null if discoveryIsConfigured() private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; + private final Random random; private final Supplier> discoveredNodesSupplier; private final BooleanSupplier isBootstrappedSupplier; private final Consumer votingConfigurationConsumer; private final AtomicBoolean bootstrappingPermitted = new AtomicBoolean(true); - public ClusterBootstrapService(Settings settings, TransportService transportService, + public ClusterBootstrapService(Settings settings, TransportService transportService, Random random, Supplier> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier, Consumer votingConfigurationConsumer) { initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); this.transportService = transportService; + this.random = random; this.discoveredNodesSupplier = discoveredNodesSupplier; this.isBootstrappedSupplier = isBootstrappedSupplier; this.votingConfigurationConsumer = votingConfigurationConsumer; @@ -88,7 +93,7 @@ void onFoundPeersUpdated() { if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false && isBootstrappedSupplier.getAsBoolean() == false && nodes.stream().noneMatch(Coordinator::isZen1Node)) { - final Optional> nodesMatchingRequirements; + final Set nodesMatchingRequirements; try { nodesMatchingRequirements = getNodesMatchingRequirements(nodes); } catch (IllegalStateException e) { @@ -97,7 +102,9 @@ void onFoundPeersUpdated() { return; } - nodesMatchingRequirements.ifPresent(this::startBootstrap); + if (nodesMatchingRequirements.size() * 2 > initialMasterNodes.size()) { + startBootstrap(nodesMatchingRequirements, initialMasterNodes.size() - nodesMatchingRequirements.size()); + } } } @@ -120,7 +127,7 @@ public void run() { final List zen1Nodes = discoveredNodes.stream().filter(Coordinator::isZen1Node).collect(Collectors.toList()); if (zen1Nodes.isEmpty()) { logger.debug("performing best-effort cluster bootstrapping with {}", discoveredNodes); - startBootstrap(discoveredNodes); + startBootstrap(discoveredNodes, 0); } else { logger.info("avoiding best-effort cluster bootstrapping due to discovery of pre-7.0 nodes {}", zen1Nodes); } @@ -138,11 +145,14 @@ private Set getDiscoveredNodes() { StreamSupport.stream(discoveredNodesSupplier.get().spliterator(), false)).collect(Collectors.toSet()); } - private void startBootstrap(Set discoveryNodes) { + private void startBootstrap(Set discoveryNodes, int placeholderCount) { assert discoveryNodes.stream().allMatch(DiscoveryNode::isMasterNode) : discoveryNodes; assert discoveryNodes.stream().noneMatch(Coordinator::isZen1Node) : discoveryNodes; + assert placeholderCount < discoveryNodes.size() : discoveryNodes.size() + " <= " + placeholderCount; if (bootstrappingPermitted.compareAndSet(true, false)) { - doBootstrap(new VotingConfiguration(discoveryNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()))); + doBootstrap(new VotingConfiguration(Stream.concat(discoveryNodes.stream().map(DiscoveryNode::getId), + Stream.generate(() -> BOOTSTRAP_PLACEHOLDER_PREFIX + UUIDs.randomBase64UUID(random)).limit(placeholderCount)) + .collect(Collectors.toSet()))); } } @@ -175,15 +185,12 @@ private static boolean matchesRequirement(DiscoveryNode discoveryNode, String re || discoveryNode.getAddress().getAddress().equals(requirement); } - private Optional> getNodesMatchingRequirements(Set nodes) { + private Set getNodesMatchingRequirements(Set nodes) { final Set selectedNodes = new HashSet<>(); for (final String requirement : initialMasterNodes) { final Set matchingNodes = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet()); - if (matchingNodes.isEmpty()) { - return Optional.empty(); - } if (matchingNodes.size() > 1) { throw new IllegalStateException("requirement [" + requirement + "] matches multiple nodes: " + matchingNodes); } @@ -196,6 +203,6 @@ private Optional> getNodesMatchingRequirements(Set nodeIds = votingConfiguration.getNodeIds(); assert nodeIds.isEmpty() == false; + final int requiredNodes = nodeIds.size() / 2 + 1; + + final Set realNodeIds = new HashSet<>(nodeIds); + realNodeIds.removeIf(s -> s.startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX)); + assert requiredNodes <= realNodeIds.size() : nodeIds; if (nodeIds.size() == 1) { - return "a node with id " + nodeIds; + return "a node with id " + realNodeIds; } else if (nodeIds.size() == 2) { - return "two nodes with ids " + nodeIds; + return "two nodes with ids " + realNodeIds; } else { - return "at least " + (nodeIds.size() / 2 + 1) + " nodes with ids from " + nodeIds; + if (requiredNodes < realNodeIds.size()) { + return "at least " + requiredNodes + " nodes with ids from " + realNodeIds; + } else { + return requiredNodes + " nodes with ids " + realNodeIds; + } } } } diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 757fd61d99393..21c20167915d2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -166,7 +166,7 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe this.clusterApplier = clusterApplier; masterService.setClusterStateSupplier(this::getStateForMasterService); this.reconfigurator = new Reconfigurator(settings, clusterSettings); - this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, + this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, random, this::getFoundPeers, this::isInitialConfigurationSet, this::setInitialConfiguration); this.discoveryUpgradeService = new DiscoveryUpgradeService(settings, clusterSettings, transportService, this::isInitialConfigurationSet, joinHelper, peerFinder::getFoundPeers, this::setInitialConfiguration); diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 57973e39baf02..47b07cf64f28b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -30,6 +30,7 @@ import org.junit.Before; import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -41,18 +42,23 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; +import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; import static org.elasticsearch.common.settings.Settings.builder; import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; import static org.elasticsearch.node.Node.NODE_NAME_SETTING; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; public class ClusterBootstrapServiceTests extends ESTestCase { @@ -100,13 +106,13 @@ public void testBootstrapsAutomaticallyWithDefaultConfiguration() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService - = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), () -> false, - vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), - equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); - assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); - }); + = new ClusterBootstrapService(settings.build(), transportService, random(), () -> discoveredNodesSupplier.get().get(), + () -> false, vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), + equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet()))); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); + }); deterministicTaskQueue.scheduleAt(timeout - 1, () -> discoveredNodesSupplier.set(() -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet()))); @@ -136,7 +142,7 @@ public void testDoesNothingByDefaultOnMasterIneligibleNodes() { } private void testDoesNothingWithSettings(Settings.Builder builder) { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, () -> { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, random(), () -> { throw new AssertionError("should not be called"); }, () -> false, vc -> { throw new AssertionError("should not be called"); @@ -149,7 +155,7 @@ private void testDoesNothingWithSettings(Settings.Builder builder) { public void testDoesNothingByDefaultIfZen1NodesDiscovered() { final DiscoveryNode zen1Node = new DiscoveryNode("zen1", buildNewFakeTransportAddress(), singletonMap("zen1", "true"), singleton(Role.MASTER), Version.CURRENT); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, () -> + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, random(), () -> Stream.of(localNode, zen1Node).collect(Collectors.toSet()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -158,14 +164,42 @@ public void testDoesNothingByDefaultIfZen1NodesDiscovered() { deterministicTaskQueue.runAllTasks(); } - public void testBootstrapsOnDiscoverySuccess() { + public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), containsInAnyOrder(localNode.getId(), otherNode1.getId(), otherNode2.getId())); + assertThat(vc.getNodeIds(), not(hasItem(containsString("placeholder")))); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testBootstrapsOnDiscoveryOfTwoOfThreeRequiredNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, random(), () -> singletonList(otherNode1), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), hasSize(3)); + assertThat(vc.getNodeIds(), hasItem(localNode.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); + assertThat(vc.getNodeIds(), hasItem(startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX))); + assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + assertFalse(vc.hasQuorum(singletonList(localNode.getId()))); + assertFalse(vc.hasQuorum(singletonList(otherNode1.getId()))); }); transportService.start(); @@ -179,10 +213,80 @@ public void testBootstrapsOnDiscoverySuccess() { assertFalse(bootstrapped.get()); // should only bootstrap once } + public void testBootstrapsOnDiscoveryOfThreeOfFiveRequiredNodes() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), + "missing-node-1", "missing-node-2").build(), + transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), hasSize(5)); + assertThat(vc.getNodeIds(), hasItem(localNode.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode2.getId())); + final List placeholders + = vc.getNodeIds().stream().filter(s -> s.startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX)).collect(Collectors.toList()); + assertThat(placeholders.size(), equalTo(2)); + assertNotEquals(placeholders.get(0), placeholders.get(1)); + assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toList()))); + assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testDoesNotBootstrapIfNoNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, random(), Collections::emptyList, () -> true, vc -> { + throw new AssertionError("should not be called"); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), + localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node-1", "not-a-node-2").build(), + transportService, random(), () -> Stream.of(otherNode1).collect(Collectors.toList()), () -> false, vc -> { + throw new AssertionError("should not be called"); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapIfThreeOfSixNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), + localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node-1", "not-a-node-2", "not-a-node-3").build(), + transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + throw new AssertionError("should not be called"); + }); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + public void testDoesNotBootstrapIfAlreadyBootstrapped() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> true, vc -> { + transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> true, vc -> { throw new AssertionError("should not be called"); }); @@ -196,7 +300,7 @@ public void testDoesNotBootstrapsOnNonMasterNode() { Version.CURRENT); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> + transportService, random(), () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -207,7 +311,7 @@ public void testDoesNotBootstrapsOnNonMasterNode() { public void testDoesNotBootstrapsIfNotConfigured() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, random(), () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -223,7 +327,7 @@ public void testDoesNotBootstrapsIfZen1NodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), () -> false, vc -> { + transportService, random(), () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -237,7 +341,7 @@ public void testRetriesBootstrappingOnException() { final AtomicLong bootstrappingAttempts = new AtomicLong(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { bootstrappingAttempts.incrementAndGet(); if (bootstrappingAttempts.get() < 5L) { throw new ElasticsearchException("test"); @@ -251,24 +355,12 @@ public void testRetriesBootstrappingOnException() { assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); } - public void testDoesNotBootstrapIfRequirementNotMet() { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( - INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node").build(), - transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { - throw new AssertionError("should not be called"); - }); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { AtomicReference> discoveredNodes = new AtomicReference<>(Stream.of(otherNode1, otherNode2).collect(Collectors.toList())); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, discoveredNodes::get, () -> false, vc -> { + transportService, random(), discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -287,7 +379,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) .build(), - transportService, discoveredNodes::get, () -> false, vc -> { + transportService, random(), discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -307,7 +399,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { public void testMatchesOnNodeName() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, random(), Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); @@ -320,7 +412,7 @@ public void testMatchesOnNodeAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), transportService, - Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + random(), Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -332,7 +424,7 @@ public void testMatchesOnNodeHostAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + transportService, random(), Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -342,7 +434,7 @@ public void testMatchesOnNodeHostAddress() { public void testDoesNotJustMatchEverything() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, random(), Collections::emptyList, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -357,7 +449,7 @@ public void testDoesNotIncludeExtraNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, vc -> { + transportService, random(), () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); }); diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelperTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelperTests.java index 129b29e1f21e5..cf8e1737a7708 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelperTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelperTests.java @@ -38,6 +38,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; +import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.elasticsearch.node.Node.NODE_NAME_SETTING; import static org.hamcrest.Matchers.equalTo; @@ -245,6 +246,13 @@ public void testDescriptionAfterBootstrapping() { "discovery will continue using [] from hosts providers and [" + localNode + "] from last-known cluster state; node term 0, last-accepted version 0 in term 0")); + assertThat(new ClusterFormationState(Settings.EMPTY, state(localNode, "n1", "n2", BOOTSTRAP_PLACEHOLDER_PREFIX + "n3"), + emptyList(), emptyList(), 0L).getDescription(), + is("master not discovered or elected yet, an election requires 2 nodes with ids [n1, n2], " + + "have discovered [] which is not a quorum; " + + "discovery will continue using [] from hosts providers and [" + localNode + + "] from last-known cluster state; node term 0, last-accepted version 0 in term 0")); + assertThat(new ClusterFormationState(Settings.EMPTY, state(localNode, "n1", "n2", "n3", "n4"), emptyList(), emptyList(), 0L) .getDescription(), is("master not discovered or elected yet, an election requires at least 3 nodes with ids from [n1, n2, n3, n4], " + @@ -259,6 +267,20 @@ public void testDescriptionAfterBootstrapping() { "discovery will continue using [] from hosts providers and [" + localNode + "] from last-known cluster state; node term 0, last-accepted version 0 in term 0")); + assertThat(new ClusterFormationState(Settings.EMPTY, state(localNode, "n1", "n2", "n3", "n4", BOOTSTRAP_PLACEHOLDER_PREFIX + "n5"), + emptyList(), emptyList(), 0L).getDescription(), + is("master not discovered or elected yet, an election requires at least 3 nodes with ids from [n1, n2, n3, n4], " + + "have discovered [] which is not a quorum; " + + "discovery will continue using [] from hosts providers and [" + localNode + + "] from last-known cluster state; node term 0, last-accepted version 0 in term 0")); + + assertThat(new ClusterFormationState(Settings.EMPTY, state(localNode, "n1", "n2", "n3", + BOOTSTRAP_PLACEHOLDER_PREFIX + "n4", BOOTSTRAP_PLACEHOLDER_PREFIX + "n5"), emptyList(), emptyList(), 0L).getDescription(), + is("master not discovered or elected yet, an election requires 3 nodes with ids [n1, n2, n3], " + + "have discovered [] which is not a quorum; " + + "discovery will continue using [] from hosts providers and [" + localNode + + "] from last-known cluster state; node term 0, last-accepted version 0 in term 0")); + assertThat(new ClusterFormationState(Settings.EMPTY, state(localNode, new String[]{"n1"}, new String[]{"n1"}), emptyList(), emptyList(), 0L).getDescription(), is("master not discovered or elected yet, an election requires a node with id [n1], " + From 28b9d46b13bd8fef0472e91f8e277e375cc014f2 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 18 Jan 2019 15:40:34 +0000 Subject: [PATCH 12/23] One blasted character over the limit --- .../coordination/ClusterBootstrapServiceTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 47b07cf64f28b..c2dc4d5fdece4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -449,10 +449,11 @@ public void testDoesNotIncludeExtraNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); - }); + transportService, random(), () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); + }); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); From 8185e13444fe2d7b68b5f402a770c558338025e7 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 12:52:27 +0000 Subject: [PATCH 13/23] Add ClusterBootstrapService::isBootstrapPlaceholder --- .../cluster/coordination/ClusterBootstrapService.java | 4 ++++ .../cluster/coordination/ClusterFormationFailureHelper.java | 3 +-- .../cluster/coordination/ClusterBootstrapServiceTests.java | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index c6cef6163cdcb..34ccea2e33aaf 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -156,6 +156,10 @@ private void startBootstrap(Set discoveryNodes, int placeholderCo } } + public static boolean isBootstrapPlaceholder(String nodeId) { + return nodeId.startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX); + } + private void doBootstrap(VotingConfiguration votingConfiguration) { assert transportService.getLocalNode().isMasterNode(); diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java index fa0d804d7c10f..cc58628b53893 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java @@ -42,7 +42,6 @@ import java.util.stream.Collectors; import java.util.stream.StreamSupport; -import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; public class ClusterFormationFailureHelper { @@ -193,7 +192,7 @@ private String describeQuorum(VotingConfiguration votingConfiguration) { final int requiredNodes = nodeIds.size() / 2 + 1; final Set realNodeIds = new HashSet<>(nodeIds); - realNodeIds.removeIf(s -> s.startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX)); + realNodeIds.removeIf(ClusterBootstrapService::isBootstrapPlaceholder); assert requiredNodes <= realNodeIds.size() : nodeIds; if (nodeIds.size() == 1) { diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index c2dc4d5fdece4..a9db05a509887 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -226,7 +226,7 @@ transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Coll assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); assertThat(vc.getNodeIds(), hasItem(otherNode2.getId())); final List placeholders - = vc.getNodeIds().stream().filter(s -> s.startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX)).collect(Collectors.toList()); + = vc.getNodeIds().stream().filter(ClusterBootstrapService::isBootstrapPlaceholder).collect(Collectors.toList()); assertThat(placeholders.size(), equalTo(2)); assertNotEquals(placeholders.get(0), placeholders.get(1)); assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toList()))); From dfaa7de4495097e3f5e6fca2724adc2e78a7f1be Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 12:52:49 +0000 Subject: [PATCH 14/23] Unnecessary assert --- .../org/elasticsearch/cluster/coordination/Coordinator.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 21c20167915d2..8f71f444dc684 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -872,8 +872,6 @@ private ClusterState clusterStateWithNoMasterBlock(ClusterState clusterState) { public void publish(ClusterChangedEvent clusterChangedEvent, ActionListener publishListener, AckListener ackListener) { try { synchronized (mutex) { - assert Thread.holdsLock(mutex) : "Coordinator mutex not held"; - if (mode != Mode.LEADER) { logger.debug(() -> new ParameterizedMessage("[{}] failed publication as not currently leading", clusterChangedEvent.source())); From c0d62afad419a0f5613590eaa83a893a8a67af5e Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 12:59:14 +0000 Subject: [PATCH 15/23] Use clusterNode.applyInitialConfiguration --- .../cluster/coordination/CoordinatorTests.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java index 3f2ea6b2e0934..54ef9e43b448b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java @@ -1203,12 +1203,8 @@ public String toString() { } } else if (rarely()) { final ClusterNode clusterNode = getAnyNode(); - clusterNode.onNode( - () -> { - logger.debug("----> [runRandomly {}] applying initial configuration {} to {}", - thisStep, initialConfiguration, clusterNode.getId()); - clusterNode.coordinator.setInitialConfiguration(initialConfiguration); - }).run(); + logger.debug("----> [runRandomly {}] applying initial configuration on {}", step, clusterNode.getId()); + clusterNode.applyInitialConfiguration(); } else { if (deterministicTaskQueue.hasDeferredTasks() && randomBoolean()) { deterministicTaskQueue.advanceTime(); From eadc79797854da78bae49381cb8b571ea64270de Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 15:55:27 +0000 Subject: [PATCH 16/23] Reject duplicate requirements early --- .../coordination/ClusterBootstrapService.java | 11 +++++++++-- .../coordination/ClusterBootstrapServiceTests.java | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 34ccea2e33aaf..aa1083e05e6b8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -34,6 +34,7 @@ import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Random; import java.util.Set; @@ -61,7 +62,7 @@ public class ClusterBootstrapService { static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}"; private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); - private final List initialMasterNodes; + private final Set initialMasterNodes; @Nullable // null if discoveryIsConfigured() private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; @@ -74,7 +75,13 @@ public class ClusterBootstrapService { public ClusterBootstrapService(Settings settings, TransportService transportService, Random random, Supplier> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier, Consumer votingConfigurationConsumer) { - initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); + final List initialMasterNodesList = INITIAL_MASTER_NODES_SETTING.get(settings); + this.initialMasterNodes = new LinkedHashSet<>(initialMasterNodesList); + if (this.initialMasterNodes.size() != initialMasterNodesList.size()) { + throw new IllegalArgumentException( + "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodesList); + } + unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); this.transportService = transportService; this.random = random; diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index a9db05a509887..261f4ce214c47 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -164,6 +164,20 @@ public void testDoesNothingByDefaultIfZen1NodesDiscovered() { deterministicTaskQueue.runAllTasks(); } + + public void testThrowsExceptionOnDuplicates() { + final IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, () -> { + new ClusterBootstrapService(builder().putList( + INITIAL_MASTER_NODES_SETTING.getKey(), "duplicate-requirement", "duplicate-requirement").build(), + transportService, random(), Collections::emptyList, () -> false, vc -> { + throw new AssertionError("should not be called"); + }); + }); + + assertThat(illegalArgumentException.getMessage(), containsString(INITIAL_MASTER_NODES_SETTING.getKey())); + assertThat(illegalArgumentException.getMessage(), containsString("duplicate-requirement")); + } + public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); From f5b75f4f2476c4bd84cf2fa998ae187d471ffdb3 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 15:57:17 +0000 Subject: [PATCH 17/23] Use placeholders in coordinator tests --- .../cluster/coordination/CoordinatorTests.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java index 54ef9e43b448b..a9ca7d917b9d8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java @@ -86,8 +86,10 @@ import java.util.function.Supplier; import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.emptySet; +import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.elasticsearch.cluster.coordination.CoordinationStateTests.clusterState; import static org.elasticsearch.cluster.coordination.CoordinationStateTests.setValue; import static org.elasticsearch.cluster.coordination.CoordinationStateTests.value; @@ -1731,11 +1733,18 @@ ClusterState getLastAppliedClusterState() { void applyInitialConfiguration() { onNode(() -> { + final Set nodeIdsWithPlaceholders = new HashSet<>(initialConfiguration.getNodeIds()); + Stream.generate(() -> BOOTSTRAP_PLACEHOLDER_PREFIX + UUIDs.randomBase64UUID(random())) + .limit((Math.max(initialConfiguration.getNodeIds().size(), 2) - 1) / 2) + .forEach(nodeIdsWithPlaceholders::add); + final VotingConfiguration configurationWithPlaceholders = new VotingConfiguration(new HashSet<>( + randomSubsetOf(initialConfiguration.getNodeIds().size(), nodeIdsWithPlaceholders))); try { - coordinator.setInitialConfiguration(initialConfiguration); - logger.info("successfully set initial configuration to {}", initialConfiguration); + coordinator.setInitialConfiguration(configurationWithPlaceholders); + logger.info("successfully set initial configuration to {}", configurationWithPlaceholders); } catch (CoordinationStateRejectedException e) { - logger.info(new ParameterizedMessage("failed to set initial configuration to {}", initialConfiguration), e); + logger.info(new ParameterizedMessage("failed to set initial configuration to {}", + configurationWithPlaceholders), e); } }).run(); } From a83dff787d2be19ad6497c2e25782ce2ac602c0b Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 15:59:31 +0000 Subject: [PATCH 18/23] Rename initialMasterNodes to bootstrapRequirements --- .../coordination/ClusterBootstrapService.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index aa1083e05e6b8..51abee80aeea1 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -62,7 +62,7 @@ public class ClusterBootstrapService { static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}"; private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); - private final Set initialMasterNodes; + private final Set bootstrapRequirements; @Nullable // null if discoveryIsConfigured() private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; @@ -75,11 +75,12 @@ public class ClusterBootstrapService { public ClusterBootstrapService(Settings settings, TransportService transportService, Random random, Supplier> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier, Consumer votingConfigurationConsumer) { - final List initialMasterNodesList = INITIAL_MASTER_NODES_SETTING.get(settings); - this.initialMasterNodes = new LinkedHashSet<>(initialMasterNodesList); - if (this.initialMasterNodes.size() != initialMasterNodesList.size()) { + + final List initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); + bootstrapRequirements = new LinkedHashSet<>(initialMasterNodes); + if (bootstrapRequirements.size() != initialMasterNodes.size()) { throw new IllegalArgumentException( - "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodesList); + "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodes); } unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); @@ -97,7 +98,7 @@ public static boolean discoveryIsConfigured(Settings settings) { void onFoundPeersUpdated() { final Set nodes = getDiscoveredNodes(); - if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false + if (transportService.getLocalNode().isMasterNode() && bootstrapRequirements.isEmpty() == false && isBootstrappedSupplier.getAsBoolean() == false && nodes.stream().noneMatch(Coordinator::isZen1Node)) { final Set nodesMatchingRequirements; @@ -109,8 +110,8 @@ void onFoundPeersUpdated() { return; } - if (nodesMatchingRequirements.size() * 2 > initialMasterNodes.size()) { - startBootstrap(nodesMatchingRequirements, initialMasterNodes.size() - nodesMatchingRequirements.size()); + if (nodesMatchingRequirements.size() * 2 > bootstrapRequirements.size()) { + startBootstrap(nodesMatchingRequirements, bootstrapRequirements.size() - nodesMatchingRequirements.size()); } } } @@ -198,18 +199,18 @@ private static boolean matchesRequirement(DiscoveryNode discoveryNode, String re private Set getNodesMatchingRequirements(Set nodes) { final Set selectedNodes = new HashSet<>(); - for (final String requirement : initialMasterNodes) { + for (final String bootstrapRequirement : bootstrapRequirements) { final Set matchingNodes - = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet()); + = nodes.stream().filter(n -> matchesRequirement(n, bootstrapRequirement)).collect(Collectors.toSet()); if (matchingNodes.size() > 1) { - throw new IllegalStateException("requirement [" + requirement + "] matches multiple nodes: " + matchingNodes); + throw new IllegalStateException("requirement [" + bootstrapRequirement + "] matches multiple nodes: " + matchingNodes); } for (final DiscoveryNode matchingNode : matchingNodes) { if (selectedNodes.add(matchingNode) == false) { throw new IllegalStateException("node [" + matchingNode + "] matches multiple requirements: " + - initialMasterNodes.stream().filter(r -> matchesRequirement(matchingNode, r)).collect(Collectors.toList())); + bootstrapRequirements.stream().filter(r -> matchesRequirement(matchingNode, r)).collect(Collectors.toList())); } } } From c7167340a30ae878c094f6a63865f257f4d0a94e Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 16:11:07 +0000 Subject: [PATCH 19/23] Include requirement in the bootstrap placeholder --- .../coordination/ClusterBootstrapService.java | 37 ++++++++------ .../cluster/coordination/Coordinator.java | 2 +- .../ClusterBootstrapServiceTests.java | 51 ++++++++++--------- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 51abee80aeea1..e06452a6005d1 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -24,7 +24,7 @@ import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -32,11 +32,10 @@ import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.transport.TransportService; -import java.util.Collections; +import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Random; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BooleanSupplier; @@ -47,13 +46,14 @@ import java.util.stream.Stream; import java.util.stream.StreamSupport; +import static java.util.Collections.emptyList; import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; public class ClusterBootstrapService { public static final Setting> INITIAL_MASTER_NODES_SETTING = - Setting.listSetting("cluster.initial_master_nodes", Collections.emptyList(), Function.identity(), Property.NodeScope); + Setting.listSetting("cluster.initial_master_nodes", emptyList(), Function.identity(), Property.NodeScope); public static final Setting UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING = Setting.timeSetting("discovery.unconfigured_bootstrap_timeout", @@ -66,13 +66,12 @@ public class ClusterBootstrapService { @Nullable // null if discoveryIsConfigured() private final TimeValue unconfiguredBootstrapTimeout; private final TransportService transportService; - private final Random random; private final Supplier> discoveredNodesSupplier; private final BooleanSupplier isBootstrappedSupplier; private final Consumer votingConfigurationConsumer; private final AtomicBoolean bootstrappingPermitted = new AtomicBoolean(true); - public ClusterBootstrapService(Settings settings, TransportService transportService, Random random, + public ClusterBootstrapService(Settings settings, TransportService transportService, Supplier> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier, Consumer votingConfigurationConsumer) { @@ -85,7 +84,6 @@ public ClusterBootstrapService(Settings settings, TransportService transportServ unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings); this.transportService = transportService; - this.random = random; this.discoveredNodesSupplier = discoveredNodesSupplier; this.isBootstrappedSupplier = isBootstrappedSupplier; this.votingConfigurationConsumer = votingConfigurationConsumer; @@ -101,17 +99,19 @@ void onFoundPeersUpdated() { if (transportService.getLocalNode().isMasterNode() && bootstrapRequirements.isEmpty() == false && isBootstrappedSupplier.getAsBoolean() == false && nodes.stream().noneMatch(Coordinator::isZen1Node)) { - final Set nodesMatchingRequirements; + final Tuple,List> requirementMatchingResult; try { - nodesMatchingRequirements = getNodesMatchingRequirements(nodes); + requirementMatchingResult = checkRequirements(nodes); } catch (IllegalStateException e) { logger.warn("bootstrapping cancelled", e); bootstrappingPermitted.set(false); return; } + final Set nodesMatchingRequirements = requirementMatchingResult.v1(); + final List unsatisfiedRequirements = requirementMatchingResult.v2(); if (nodesMatchingRequirements.size() * 2 > bootstrapRequirements.size()) { - startBootstrap(nodesMatchingRequirements, bootstrapRequirements.size() - nodesMatchingRequirements.size()); + startBootstrap(nodesMatchingRequirements, unsatisfiedRequirements); } } } @@ -135,7 +135,7 @@ public void run() { final List zen1Nodes = discoveredNodes.stream().filter(Coordinator::isZen1Node).collect(Collectors.toList()); if (zen1Nodes.isEmpty()) { logger.debug("performing best-effort cluster bootstrapping with {}", discoveredNodes); - startBootstrap(discoveredNodes, 0); + startBootstrap(discoveredNodes, emptyList()); } else { logger.info("avoiding best-effort cluster bootstrapping due to discovery of pre-7.0 nodes {}", zen1Nodes); } @@ -153,13 +153,13 @@ private Set getDiscoveredNodes() { StreamSupport.stream(discoveredNodesSupplier.get().spliterator(), false)).collect(Collectors.toSet()); } - private void startBootstrap(Set discoveryNodes, int placeholderCount) { + private void startBootstrap(Set discoveryNodes, List unsatisfiedRequirements) { assert discoveryNodes.stream().allMatch(DiscoveryNode::isMasterNode) : discoveryNodes; assert discoveryNodes.stream().noneMatch(Coordinator::isZen1Node) : discoveryNodes; - assert placeholderCount < discoveryNodes.size() : discoveryNodes.size() + " <= " + placeholderCount; + assert unsatisfiedRequirements.size() < discoveryNodes.size() : discoveryNodes + " smaller than " + unsatisfiedRequirements; if (bootstrappingPermitted.compareAndSet(true, false)) { doBootstrap(new VotingConfiguration(Stream.concat(discoveryNodes.stream().map(DiscoveryNode::getId), - Stream.generate(() -> BOOTSTRAP_PLACEHOLDER_PREFIX + UUIDs.randomBase64UUID(random)).limit(placeholderCount)) + unsatisfiedRequirements.stream().map(s -> BOOTSTRAP_PLACEHOLDER_PREFIX + s)) .collect(Collectors.toSet()))); } } @@ -197,12 +197,17 @@ private static boolean matchesRequirement(DiscoveryNode discoveryNode, String re || discoveryNode.getAddress().getAddress().equals(requirement); } - private Set getNodesMatchingRequirements(Set nodes) { + private Tuple,List> checkRequirements(Set nodes) { final Set selectedNodes = new HashSet<>(); + final List unmatchedRequirements = new ArrayList<>(); for (final String bootstrapRequirement : bootstrapRequirements) { final Set matchingNodes = nodes.stream().filter(n -> matchesRequirement(n, bootstrapRequirement)).collect(Collectors.toSet()); + if (matchingNodes.size() == 0) { + unmatchedRequirements.add(bootstrapRequirement); + } + if (matchingNodes.size() > 1) { throw new IllegalStateException("requirement [" + bootstrapRequirement + "] matches multiple nodes: " + matchingNodes); } @@ -215,6 +220,6 @@ private Set getNodesMatchingRequirements(Set nodes } } - return selectedNodes; + return Tuple.tuple(selectedNodes, unmatchedRequirements); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 8f71f444dc684..c9199599811c5 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -166,7 +166,7 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe this.clusterApplier = clusterApplier; masterService.setClusterStateSupplier(this::getStateForMasterService); this.reconfigurator = new Reconfigurator(settings, clusterSettings); - this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, random, this::getFoundPeers, + this.clusterBootstrapService = new ClusterBootstrapService(settings, transportService, this::getFoundPeers, this::isInitialConfigurationSet, this::setInitialConfiguration); this.discoveryUpgradeService = new DiscoveryUpgradeService(settings, clusterSettings, transportService, this::isInitialConfigurationSet, joinHelper, peerFinder::getFoundPeers, this::setInitialConfiguration); diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java index 261f4ce214c47..46a43afa53897 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -51,6 +51,7 @@ import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; import static org.elasticsearch.node.Node.NODE_NAME_SETTING; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -106,7 +107,7 @@ public void testBootstrapsAutomaticallyWithDefaultConfiguration() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService - = new ClusterBootstrapService(settings.build(), transportService, random(), () -> discoveredNodesSupplier.get().get(), + = new ClusterBootstrapService(settings.build(), transportService, () -> discoveredNodesSupplier.get().get(), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), @@ -142,7 +143,7 @@ public void testDoesNothingByDefaultOnMasterIneligibleNodes() { } private void testDoesNothingWithSettings(Settings.Builder builder) { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, random(), () -> { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(builder.build(), transportService, () -> { throw new AssertionError("should not be called"); }, () -> false, vc -> { throw new AssertionError("should not be called"); @@ -155,7 +156,7 @@ private void testDoesNothingWithSettings(Settings.Builder builder) { public void testDoesNothingByDefaultIfZen1NodesDiscovered() { final DiscoveryNode zen1Node = new DiscoveryNode("zen1", buildNewFakeTransportAddress(), singletonMap("zen1", "true"), singleton(Role.MASTER), Version.CURRENT); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, random(), () -> + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.EMPTY, transportService, () -> Stream.of(localNode, zen1Node).collect(Collectors.toSet()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -169,7 +170,7 @@ public void testThrowsExceptionOnDuplicates() { final IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, () -> { new ClusterBootstrapService(builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), "duplicate-requirement", "duplicate-requirement").build(), - transportService, random(), Collections::emptyList, () -> false, vc -> { + transportService, Collections::emptyList, () -> false, vc -> { throw new AssertionError("should not be called"); }); }); @@ -183,7 +184,7 @@ public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), containsInAnyOrder(localNode.getId(), otherNode1.getId(), otherNode2.getId())); assertThat(vc.getNodeIds(), not(hasItem(containsString("placeholder")))); @@ -205,12 +206,12 @@ public void testBootstrapsOnDiscoveryOfTwoOfThreeRequiredNodes() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> singletonList(otherNode1), () -> false, vc -> { + transportService, () -> singletonList(otherNode1), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), hasSize(3)); assertThat(vc.getNodeIds(), hasItem(localNode.getId())); assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); - assertThat(vc.getNodeIds(), hasItem(startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX))); + assertThat(vc.getNodeIds(), hasItem(allOf(startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX), containsString(otherNode2.getName())))); assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); assertFalse(vc.hasQuorum(singletonList(localNode.getId()))); assertFalse(vc.hasQuorum(singletonList(otherNode1.getId()))); @@ -233,16 +234,20 @@ public void testBootstrapsOnDiscoveryOfThreeOfFiveRequiredNodes() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "missing-node-1", "missing-node-2").build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), hasSize(5)); assertThat(vc.getNodeIds(), hasItem(localNode.getId())); assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); assertThat(vc.getNodeIds(), hasItem(otherNode2.getId())); + final List placeholders = vc.getNodeIds().stream().filter(ClusterBootstrapService::isBootstrapPlaceholder).collect(Collectors.toList()); assertThat(placeholders.size(), equalTo(2)); assertNotEquals(placeholders.get(0), placeholders.get(1)); + assertThat(placeholders, hasItem(containsString("missing-node-1"))); + assertThat(placeholders, hasItem(containsString("missing-node-2"))); + assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toList()))); assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); @@ -262,7 +267,7 @@ transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Coll public void testDoesNotBootstrapIfNoNodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), Collections::emptyList, () -> true, vc -> { + transportService, Collections::emptyList, () -> true, vc -> { throw new AssertionError("should not be called"); }); @@ -275,7 +280,7 @@ public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node-1", "not-a-node-2").build(), - transportService, random(), () -> Stream.of(otherNode1).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -288,7 +293,7 @@ public void testDoesNotBootstrapIfThreeOfSixNodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName(), "not-a-node-1", "not-a-node-2", "not-a-node-3").build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -300,7 +305,7 @@ transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Coll public void testDoesNotBootstrapIfAlreadyBootstrapped() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> true, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> true, vc -> { throw new AssertionError("should not be called"); }); @@ -314,7 +319,7 @@ public void testDoesNotBootstrapsOnNonMasterNode() { Version.CURRENT); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> + transportService, () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -325,7 +330,7 @@ transportService, random(), () -> public void testDoesNotBootstrapsIfNotConfigured() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, random(), + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), transportService, () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -341,7 +346,7 @@ public void testDoesNotBootstrapsIfZen1NodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2, zen1Node).collect(Collectors.toList()), () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -355,7 +360,7 @@ public void testRetriesBootstrappingOnException() { final AtomicLong bootstrappingAttempts = new AtomicLong(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { + transportService, () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), () -> false, vc -> { bootstrappingAttempts.incrementAndGet(); if (bootstrappingAttempts.get() < 5L) { throw new ElasticsearchException("test"); @@ -374,7 +379,7 @@ public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { = new AtomicReference<>(Stream.of(otherNode1, otherNode2).collect(Collectors.toList())); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, random(), discoveredNodes::get, () -> false, vc -> { + transportService, discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -393,7 +398,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) .build(), - transportService, random(), discoveredNodes::get, () -> false, vc -> { + transportService, discoveredNodes::get, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -413,7 +418,7 @@ transportService, random(), discoveredNodes::get, () -> false, vc -> { public void testMatchesOnNodeName() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, random(), + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), transportService, Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); @@ -426,7 +431,7 @@ public void testMatchesOnNodeAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), transportService, - random(), Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -438,7 +443,7 @@ public void testMatchesOnNodeHostAddress() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, random(), Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); + transportService, Collections::emptyList, () -> false, vc -> assertTrue(bootstrapped.compareAndSet(false, true))); transportService.start(); clusterBootstrapService.onFoundPeersUpdated(); @@ -448,7 +453,7 @@ public void testMatchesOnNodeHostAddress() { public void testDoesNotJustMatchEverything() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, random(), + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), randomAlphaOfLength(10)).build(), transportService, Collections::emptyList, () -> false, vc -> { throw new AssertionError("should not be called"); }); @@ -463,7 +468,7 @@ public void testDoesNotIncludeExtraNodes() { final AtomicBoolean bootstrapped = new AtomicBoolean(); ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList( INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(), - transportService, random(), () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, + transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), () -> false, vc -> { assertTrue(bootstrapped.compareAndSet(false, true)); assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); From f30ab6e6b8c0551b66a63a7ce7e2f3a63e9f942e Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 16:29:06 +0000 Subject: [PATCH 20/23] Permit bootstrapping in any mode --- .../org/elasticsearch/cluster/coordination/Coordinator.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index c9199599811c5..968a9a5f01f7a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -709,10 +709,6 @@ public boolean setInitialConfiguration(final VotingConfiguration votingConfigura return false; } - if (mode != Mode.CANDIDATE) { - throw new CoordinationStateRejectedException("Cannot set initial configuration in mode " + mode); - } - final List knownNodes = new ArrayList<>(); knownNodes.add(getLocalNode()); peerFinder.getFoundPeers().forEach(knownNodes::add); From b40144f4313da037b11e903bbc127f0450de5d1b Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 18:38:46 +0000 Subject: [PATCH 21/23] Add trailing hyphen to placeholder prefix --- .../cluster/coordination/ClusterBootstrapService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index e06452a6005d1..7505b7ba6c341 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -59,7 +59,7 @@ public class ClusterBootstrapService { Setting.timeSetting("discovery.unconfigured_bootstrap_timeout", TimeValue.timeValueSeconds(3), TimeValue.timeValueMillis(1), Property.NodeScope); - static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}"; + static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}-"; private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class); private final Set bootstrapRequirements; From 2878bd6bc0aa725c6ed1735bc455448e3274c9b5 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 18:40:47 +0000 Subject: [PATCH 22/23] Trace logging --- .../cluster/coordination/ClusterBootstrapService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 7505b7ba6c341..7a001dbeac05e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -110,6 +110,9 @@ void onFoundPeersUpdated() { final Set nodesMatchingRequirements = requirementMatchingResult.v1(); final List unsatisfiedRequirements = requirementMatchingResult.v2(); + logger.trace("nodesMatchingRequirements={}, unsatisfiedRequirements={}, bootstrapRequirements={}", + nodesMatchingRequirements, unsatisfiedRequirements, bootstrapRequirements); + if (nodesMatchingRequirements.size() * 2 > bootstrapRequirements.size()) { startBootstrap(nodesMatchingRequirements, unsatisfiedRequirements); } From b757444a23f4574f963646bf27c11da45193bbc0 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 21 Jan 2019 18:40:56 +0000 Subject: [PATCH 23/23] Make requirements unmodifiable --- .../cluster/coordination/ClusterBootstrapService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java index 7a001dbeac05e..d21c54c03e4e5 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java @@ -47,6 +47,7 @@ import java.util.stream.StreamSupport; import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableSet; import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING; @@ -76,7 +77,7 @@ public ClusterBootstrapService(Settings settings, TransportService transportServ Consumer votingConfigurationConsumer) { final List initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings); - bootstrapRequirements = new LinkedHashSet<>(initialMasterNodes); + bootstrapRequirements = unmodifiableSet(new LinkedHashSet<>(initialMasterNodes)); if (bootstrapRequirements.size() != initialMasterNodes.size()) { throw new IllegalArgumentException( "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodes);