diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequest.java index 0ffc3d5567aa3..50f89b073c532 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequest.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.io.stream.StreamInput; @@ -69,10 +70,10 @@ public AddVotingTombstonesRequest(StreamInput in) throws IOException { timeout = in.readTimeValue(); } - Set resolveNodes(ClusterState currentState) { + Set resolveVotingTombstones(ClusterState currentState) { final DiscoveryNodes allNodes = currentState.nodes(); - final Set resolvedNodes = Arrays.stream(allNodes.resolveNodes(nodeDescriptions)) - .map(allNodes::get).filter(DiscoveryNode::isMasterNode).collect(Collectors.toSet()); + final Set resolvedNodes = Arrays.stream(allNodes.resolveNodes(nodeDescriptions)) + .map(allNodes::get).filter(DiscoveryNode::isMasterNode).map(VotingTombstone::new).collect(Collectors.toSet()); if (resolvedNodes.isEmpty()) { throw new IllegalArgumentException("add voting tombstones request for " + Arrays.asList(nodeDescriptions) @@ -83,8 +84,9 @@ Set resolveNodes(ClusterState currentState) { return resolvedNodes; } - Set resolveNodesAndCheckMaximum(ClusterState currentState, int maxTombstoneCount, String maximumSettingKey) { - final Set resolvedNodes = resolveNodes(currentState); + Set resolveVotingTombstonesAndCheckMaximum(ClusterState currentState, int maxTombstoneCount, + String maximumSettingKey) { + final Set resolvedNodes = resolveVotingTombstones(currentState); final int oldTombstoneCount = currentState.getVotingTombstones().size(); final int newTombstoneCount = resolvedNodes.size(); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesAction.java index 852ae6839c14a..3a571777c1c04 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesAction.java @@ -30,9 +30,9 @@ import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.coordination.CoordinationMetaData; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.inject.Inject; @@ -80,16 +80,16 @@ protected AddVotingTombstonesResponse read(StreamInput in) throws IOException { protected void masterOperation(AddVotingTombstonesRequest request, ClusterState state, ActionListener listener) throws Exception { - resolveNodesAndCheckMaximum(request, state); // throws IllegalArgumentException if no nodes matched or maximum exceeded + resolveVotingTombstonesAndCheckMaximum(request, state); // throws IllegalArgumentException if no nodes matched or maximum exceeded clusterService.submitStateUpdateTask("add-voting-tombstones", new ClusterStateUpdateTask(Priority.URGENT) { - private Set resolvedNodes; + private Set resolvedNodes; @Override public ClusterState execute(ClusterState currentState) { assert resolvedNodes == null : resolvedNodes; - resolvedNodes = resolveNodesAndCheckMaximum(request, currentState); + resolvedNodes = resolveVotingTombstonesAndCheckMaximum(request, currentState); final CoordinationMetaData.Builder builder = CoordinationMetaData.builder(currentState.coordinationMetaData()); resolvedNodes.forEach(builder::addVotingTombstone); @@ -110,7 +110,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS final ClusterStateObserver observer = new ClusterStateObserver(clusterService, request.getTimeout(), logger, threadPool.getThreadContext()); - final Set resolvedNodeIds = resolvedNodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet()); + final Set resolvedNodeIds = resolvedNodes.stream().map(VotingTombstone::getNodeId).collect(Collectors.toSet()); final Predicate allNodesRemoved = clusterState -> { final Set votingNodeIds = clusterState.getLastCommittedConfiguration().getNodeIds(); @@ -145,8 +145,8 @@ public void onTimeout(TimeValue timeout) { }); } - private static Set resolveNodesAndCheckMaximum(AddVotingTombstonesRequest request, ClusterState state) { - return request.resolveNodesAndCheckMaximum(state, + private static Set resolveVotingTombstonesAndCheckMaximum(AddVotingTombstonesRequest request, ClusterState state) { + return request.resolveVotingTombstonesAndCheckMaximum(state, MAXIMUM_VOTING_TOMBSTONES_SETTING.get(state.metaData().settings()), MAXIMUM_VOTING_TOMBSTONES_SETTING.getKey()); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesAction.java index dfaf93bd44a0c..dfc602a83f1b9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesAction.java @@ -30,9 +30,9 @@ import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.coordination.CoordinationMetaData; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.inject.Inject; @@ -77,10 +77,9 @@ protected void masterOperation(ClearVotingTombstonesRequest request, ClusterStat final long startTimeMillis = threadPool.relativeTimeInMillis(); final Predicate allTombstonedNodesRemoved = newState -> { - for (DiscoveryNode tombstone : initialState.getVotingTombstones()) { + for (VotingTombstone tombstone : initialState.getVotingTombstones()) { // NB checking for the existence of any node with this persistent ID, because persistent IDs are how votes are counted. - // Calling nodeExists(tombstone) is insufficient because this compares on the ephemeral ID. - if (newState.nodes().nodeExists(tombstone.getId())) { + if (newState.nodes().nodeExists(tombstone.getNodeId())) { return false; } } diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java index 535aed7c60a39..bcd04209c27b6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -28,6 +28,7 @@ import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.coordination.CoordinationMetaData; import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; @@ -277,7 +278,7 @@ public VotingConfiguration getLastCommittedConfiguration() { return coordinationMetaData().getLastCommittedConfiguration(); } - public Set getVotingTombstones() { + public Set getVotingTombstones() { return coordinationMetaData().getVotingTombstones(); } diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationMetaData.java b/server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationMetaData.java index 072a31b1790fb..7c57ccc908c12 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationMetaData.java @@ -49,40 +49,48 @@ public class CoordinationMetaData implements Writeable, ToXContentFragment { private final VotingConfiguration lastAcceptedConfiguration; - private final Set votingTombstones; + private final Set votingTombstones; private static final ParseField TERM_PARSE_FIELD = new ParseField("term"); private static final ParseField LAST_COMMITTED_CONFIGURATION_FIELD = new ParseField("last_committed_config"); private static final ParseField LAST_ACCEPTED_CONFIGURATION_FIELD = new ParseField("last_accepted_config"); + private static final ParseField VOTING_TOMBSTONES_FIELD = new ParseField("voting_tombstones"); private static long term(Object[] termAndConfigs) { return (long)termAndConfigs[0]; } @SuppressWarnings("unchecked") - private static VotingConfiguration lastCommittedConfig(Object[] termAndConfig) { - List nodeIds = (List) termAndConfig[1]; + private static VotingConfiguration lastCommittedConfig(Object[] fields) { + List nodeIds = (List) fields[1]; return new VotingConfiguration(new HashSet<>(nodeIds)); } @SuppressWarnings("unchecked") - private static VotingConfiguration lastAcceptedConfig(Object[] termAndConfig) { - List nodeIds = (List) termAndConfig[2]; + private static VotingConfiguration lastAcceptedConfig(Object[] fields) { + List nodeIds = (List) fields[2]; return new VotingConfiguration(new HashSet<>(nodeIds)); } + @SuppressWarnings("unchecked") + private static Set votingTombstones(Object[] fields) { + Set votingTombstones = new HashSet<>((List) fields[3]); + return votingTombstones; + } + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "coordination_metadata", - termAndConfigs -> new CoordinationMetaData(term(termAndConfigs), lastCommittedConfig(termAndConfigs), - lastAcceptedConfig(termAndConfigs), Collections.emptySet())); + fields -> new CoordinationMetaData(term(fields), lastCommittedConfig(fields), + lastAcceptedConfig(fields), votingTombstones(fields))); static { PARSER.declareLong(ConstructingObjectParser.constructorArg(), TERM_PARSE_FIELD); PARSER.declareStringArray(ConstructingObjectParser.constructorArg(), LAST_COMMITTED_CONFIGURATION_FIELD); PARSER.declareStringArray(ConstructingObjectParser.constructorArg(), LAST_ACCEPTED_CONFIGURATION_FIELD); + PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), VotingTombstone.PARSER, VOTING_TOMBSTONES_FIELD); } public CoordinationMetaData(long term, VotingConfiguration lastCommittedConfiguration, VotingConfiguration lastAcceptedConfiguration, - Set votingTombstones) { + Set votingTombstones) { this.term = term; this.lastCommittedConfiguration = lastCommittedConfiguration; this.lastAcceptedConfiguration = lastAcceptedConfiguration; @@ -93,7 +101,7 @@ public CoordinationMetaData(StreamInput in) throws IOException { term = in.readLong(); lastCommittedConfiguration = new VotingConfiguration(in); lastAcceptedConfiguration = new VotingConfiguration(in); - votingTombstones = Collections.unmodifiableSet(in.readSet(DiscoveryNode::new)); + votingTombstones = Collections.unmodifiableSet(in.readSet(VotingTombstone::new)); } public static Builder builder() { @@ -117,8 +125,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder .field(TERM_PARSE_FIELD.getPreferredName(), term) .field(LAST_COMMITTED_CONFIGURATION_FIELD.getPreferredName(), lastCommittedConfiguration) - .field(LAST_ACCEPTED_CONFIGURATION_FIELD.getPreferredName(), lastAcceptedConfiguration); - // TODO include voting tombstones here + .field(LAST_ACCEPTED_CONFIGURATION_FIELD.getPreferredName(), lastAcceptedConfiguration) + .field(VOTING_TOMBSTONES_FIELD.getPreferredName(), votingTombstones); } public static CoordinationMetaData fromXContent(XContentParser parser) throws IOException { @@ -137,7 +145,7 @@ public VotingConfiguration getLastCommittedConfiguration() { return lastCommittedConfiguration; } - public Set getVotingTombstones() { + public Set getVotingTombstones() { return votingTombstones; } @@ -177,7 +185,7 @@ public static class Builder { private long term = 0; private VotingConfiguration lastCommittedConfiguration = VotingConfiguration.EMPTY_CONFIG; private VotingConfiguration lastAcceptedConfiguration = VotingConfiguration.EMPTY_CONFIG; - private final Set votingTombstones = new HashSet<>(); + private final Set votingTombstones = new HashSet<>(); public Builder() { @@ -205,7 +213,7 @@ public Builder lastAcceptedConfiguration(VotingConfiguration config) { return this; } - public Builder addVotingTombstone(DiscoveryNode tombstone) { + public Builder addVotingTombstone(VotingTombstone tombstone) { votingTombstones.add(tombstone); return this; } @@ -220,6 +228,97 @@ public CoordinationMetaData build() { } } + public static class VotingTombstone implements Writeable, ToXContentFragment { + private final String nodeId; + private final String nodeName; + + public VotingTombstone(DiscoveryNode node) { + this(node.getId(), node.getName()); + } + + public VotingTombstone(StreamInput in) throws IOException { + this.nodeId = in.readString(); + this.nodeName = in.readString(); + } + + public VotingTombstone(String nodeId, String nodeName) { + this.nodeId = nodeId; + this.nodeName = nodeName; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(nodeId); + out.writeString(nodeName); + } + + public String getNodeId() { + return nodeId; + } + + public String getNodeName() { + return nodeName; + } + + private static final ParseField NODE_ID_PARSE_FIELD = new ParseField("node_id"); + private static final ParseField NODE_NAME_PARSE_FIELD = new ParseField("node_name"); + + private static String nodeId(Object[] nodeIdAndName) { + return (String) nodeIdAndName[0]; + } + + private static String nodeName(Object[] nodeIdAndName) { + return (String) nodeIdAndName[1]; + } + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "voting_tombstone", + nodeIdAndName -> new VotingTombstone(nodeId(nodeIdAndName), nodeName(nodeIdAndName)) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), NODE_ID_PARSE_FIELD); + PARSER.declareString(ConstructingObjectParser.constructorArg(), NODE_NAME_PARSE_FIELD); + } + + public static VotingTombstone fromXContent(XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.startObject() + .field(NODE_ID_PARSE_FIELD.getPreferredName(), nodeId) + .field(NODE_NAME_PARSE_FIELD.getPreferredName(), nodeName) + .endObject(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + VotingTombstone that = (VotingTombstone) o; + return Objects.equals(nodeId, that.nodeId) && + Objects.equals(nodeName, that.nodeName); + } + + @Override + public int hashCode() { + return Objects.hash(nodeId, nodeName); + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + if (nodeName.length() > 0) { + sb.append('{').append(nodeName).append('}'); + } + sb.append('{').append(nodeId).append('}'); + return sb.toString(); + } + + } + /** * A collection of persistent node ids, denoting the voting configuration for cluster state changes. */ 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 23ee7b0cd6e25..972cf28ecc12e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -33,6 +33,7 @@ import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.coordination.FollowersChecker.FollowerCheckRequest; import org.elasticsearch.cluster.coordination.JoinHelper.InitialJoinAccumulator; import org.elasticsearch.cluster.metadata.MetaData; @@ -654,7 +655,7 @@ ClusterState improveConfiguration(ClusterState clusterState) { final Set liveNodes = StreamSupport.stream(clusterState.nodes().spliterator(), false) .filter(this::hasJoinVoteFrom).collect(Collectors.toSet()); final VotingConfiguration newConfig = reconfigurator.reconfigure(liveNodes, - clusterState.getVotingTombstones().stream().map(DiscoveryNode::getId).collect(Collectors.toSet()), + clusterState.getVotingTombstones().stream().map(VotingTombstone::getNodeId).collect(Collectors.toSet()), clusterState.getLastAcceptedConfiguration()); if (newConfig.equals(clusterState.getLastAcceptedConfiguration()) == false) { assert coordinationState.get().joinVotesHaveQuorumFor(newConfig); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequestTests.java index 514700818305a..cd761e5dae507 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingTombstonesRequestTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.coordination.CoordinationMetaData; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode.Role; @@ -55,49 +56,61 @@ public void testSerialization() throws IOException { public void testResolve() { final DiscoveryNode localNode = new DiscoveryNode("local", "local", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone localNodeTombstone = new VotingTombstone(localNode); final DiscoveryNode otherNode1 = new DiscoveryNode("other1", "other1", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone otherNode1Tombstone = new VotingTombstone(otherNode1); final DiscoveryNode otherNode2 = new DiscoveryNode("other2", "other2", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone otherNode2Tombstone = new VotingTombstone(otherNode2); final DiscoveryNode otherDataNode = new DiscoveryNode("data", "data", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder() .add(localNode).add(otherNode1).add(otherNode2).add(otherDataNode).localNodeId(localNode.getId())).build(); - assertThat(makeRequest().resolveNodes(clusterState), containsInAnyOrder(localNode, otherNode1, otherNode2)); - assertThat(makeRequest("_all").resolveNodes(clusterState), containsInAnyOrder(localNode, otherNode1, otherNode2)); - assertThat(makeRequest("_local").resolveNodes(clusterState), contains(localNode)); - assertThat(makeRequest("other*").resolveNodes(clusterState), containsInAnyOrder(otherNode1, otherNode2)); + assertThat(makeRequest().resolveVotingTombstones(clusterState), + containsInAnyOrder(localNodeTombstone, otherNode1Tombstone, otherNode2Tombstone)); + assertThat(makeRequest("_all").resolveVotingTombstones(clusterState), + containsInAnyOrder(localNodeTombstone, otherNode1Tombstone, otherNode2Tombstone)); + assertThat(makeRequest("_local").resolveVotingTombstones(clusterState), + contains(localNodeTombstone)); + assertThat(makeRequest("other*").resolveVotingTombstones(clusterState), + containsInAnyOrder(otherNode1Tombstone, otherNode2Tombstone)); - assertThat(expectThrows(IllegalArgumentException.class, () -> makeRequest("not-a-node").resolveNodes(clusterState)).getMessage(), - equalTo("add voting tombstones request for [not-a-node] matched no master-eligible nodes")); + assertThat(expectThrows(IllegalArgumentException.class, + () -> makeRequest("not-a-node").resolveVotingTombstones(clusterState)).getMessage(), + equalTo("add voting tombstones request for [not-a-node] matched no master-eligible nodes")); } public void testResolveAndCheckMaximum() { final DiscoveryNode localNode = new DiscoveryNode("local", "local", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone localNodeTombstone = new VotingTombstone(localNode); final DiscoveryNode otherNode1 = new DiscoveryNode("other1", "other1", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone otherNode1Tombstone = new VotingTombstone(otherNode1); final DiscoveryNode otherNode2 = new DiscoveryNode("other2", "other2", buildNewFakeTransportAddress(), emptyMap(), singleton(Role.MASTER), Version.CURRENT); + final VotingTombstone otherNode2Tombstone = new VotingTombstone(otherNode2); final ClusterState.Builder builder = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder() .add(localNode).add(otherNode1).add(otherNode2).localNodeId(localNode.getId())); - builder.metaData(MetaData.builder().coordinationMetaData(CoordinationMetaData.builder().addVotingTombstone(otherNode1).build())); + builder.metaData(MetaData.builder() + .coordinationMetaData(CoordinationMetaData.builder().addVotingTombstone(otherNode1Tombstone).build())); final ClusterState clusterState = builder.build(); - assertThat(makeRequest().resolveNodesAndCheckMaximum(clusterState, 3, "setting.name"), - containsInAnyOrder(localNode, otherNode2)); - assertThat(makeRequest("_local").resolveNodesAndCheckMaximum(clusterState, 2, "setting.name"), - contains(localNode)); + assertThat(makeRequest().resolveVotingTombstonesAndCheckMaximum(clusterState, 3, "setting.name"), + containsInAnyOrder(localNodeTombstone, otherNode2Tombstone)); + assertThat(makeRequest("_local").resolveVotingTombstonesAndCheckMaximum(clusterState, 2, "setting.name"), + contains(localNodeTombstone)); assertThat(expectThrows(IllegalArgumentException.class, - () -> makeRequest().resolveNodesAndCheckMaximum(clusterState, 2, "setting.name")).getMessage(), + () -> makeRequest().resolveVotingTombstonesAndCheckMaximum(clusterState, 2, "setting.name")).getMessage(), equalTo("add voting tombstones request for [] would add [2] voting tombstones to the existing [1] which would exceed the " + "maximum of [2] set by [setting.name]")); assertThat(expectThrows(IllegalArgumentException.class, - () -> makeRequest("_local").resolveNodesAndCheckMaximum(clusterState, 1, "setting.name")).getMessage(), + () -> makeRequest("_local").resolveVotingTombstonesAndCheckMaximum(clusterState, 1, "setting.name")).getMessage(), equalTo("add voting tombstones request for [_local] would add [1] voting tombstones to the existing [1] which would exceed " + "the maximum of [1] set by [setting.name]")); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesActionTests.java index 4153ce373ce39..0efbb4e6f93d8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingTombstonesActionTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.coordination.CoordinationMetaData; import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; @@ -76,6 +77,7 @@ public class TransportAddVotingTombstonesActionTests extends ESTestCase { private static ThreadPool threadPool; private static ClusterService clusterService; private static DiscoveryNode localNode, otherNode1, otherNode2, otherDataNode; + private static VotingTombstone localNodeTombstone, otherNode1Tombstone, otherNode2Tombstone; private TransportService transportService; private ClusterStateObserver clusterStateObserver; @@ -84,8 +86,11 @@ public class TransportAddVotingTombstonesActionTests extends ESTestCase { public static void createThreadPoolAndClusterService() { threadPool = new TestThreadPool("test", Settings.EMPTY); localNode = makeDiscoveryNode("local"); + localNodeTombstone = new VotingTombstone(localNode); otherNode1 = makeDiscoveryNode("other1"); + otherNode1Tombstone = new VotingTombstone(otherNode1); otherNode2 = makeDiscoveryNode("other2"); + otherNode2Tombstone = new VotingTombstone(otherNode2); otherDataNode = new DiscoveryNode("data", "data", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); clusterService = createClusterService(threadPool, localNode); } @@ -137,7 +142,7 @@ public void testWithdrawsVoteFromANode() throws InterruptedException { ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), contains(otherNode1)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), contains(otherNode1Tombstone)); } public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException { @@ -153,7 +158,8 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException { ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), containsInAnyOrder(otherNode1, otherNode2)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + containsInAnyOrder(otherNode1Tombstone, otherNode2Tombstone)); } public void testWithdrawsVotesFromNodesMatchingWildcard() throws InterruptedException { @@ -169,7 +175,8 @@ public void testWithdrawsVotesFromNodesMatchingWildcard() throws InterruptedExce ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), containsInAnyOrder(otherNode1, otherNode2)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + containsInAnyOrder(otherNode1Tombstone, otherNode2Tombstone)); } public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedException { @@ -186,7 +193,7 @@ public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedExc assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), - containsInAnyOrder(localNode, otherNode1, otherNode2)); + containsInAnyOrder(localNodeTombstone, otherNode1Tombstone, otherNode2Tombstone)); } public void testWithdrawsVoteFromLocalNode() throws InterruptedException { @@ -202,7 +209,8 @@ public void testWithdrawsVoteFromLocalNode() throws InterruptedException { ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), contains(localNode)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + contains(localNodeTombstone)); } public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedException { @@ -226,7 +234,8 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), contains(otherNode1)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + contains(otherNode1Tombstone)); } public void testReturnsErrorIfNoMatchingNodes() throws InterruptedException { @@ -272,7 +281,7 @@ public void testSucceedsEvenIfAllTombstonesAlreadyAdded() throws InterruptedExce builder.metaData(MetaData.builder(state.metaData()). coordinationMetaData( CoordinationMetaData.builder(state.coordinationMetaData()) - .addVotingTombstone(otherNode1). + .addVotingTombstone(otherNode1Tombstone). build())); setState(clusterService, builder); @@ -287,7 +296,8 @@ public void testSucceedsEvenIfAllTombstonesAlreadyAdded() throws InterruptedExce ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), contains(otherNode1)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + contains(otherNode1Tombstone)); } public void testReturnsErrorIfMaximumTombstoneCountExceeded() throws InterruptedException { @@ -295,11 +305,12 @@ public void testReturnsErrorIfMaximumTombstoneCountExceeded() throws Interrupted Settings.builder().put(clusterService.state().metaData().persistentSettings()) .put(MAXIMUM_VOTING_TOMBSTONES_SETTING.getKey(), 2).build()); CoordinationMetaData.Builder coordinationMetaDataBuilder = - CoordinationMetaData.builder(clusterService.state().coordinationMetaData()).addVotingTombstone(localNode); + CoordinationMetaData.builder(clusterService.state().coordinationMetaData()) + .addVotingTombstone(localNodeTombstone); final int existingCount, newCount; if (randomBoolean()) { - coordinationMetaDataBuilder.addVotingTombstone(otherNode1); + coordinationMetaDataBuilder.addVotingTombstone(otherNode1Tombstone); existingCount = 2; newCount = 1; } else { @@ -395,7 +406,7 @@ public ClusterState execute(ClusterState currentState) { assertThat(currentState, sameInstance(state)); final Set votingNodeIds = new HashSet<>(); currentState.nodes().forEach(n -> votingNodeIds.add(n.getId())); - currentState.getVotingTombstones().forEach(t -> votingNodeIds.remove(t.getId())); + currentState.getVotingTombstones().forEach(t -> votingNodeIds.remove(t.getNodeId())); final VotingConfiguration votingConfiguration = new VotingConfiguration(votingNodeIds); return builder(currentState) .metaData(MetaData.builder(currentState.metaData()) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesActionTests.java index 19794a3cc55b8..78f5514403402 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportClearVotingTombstonesActionTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.coordination.CoordinationMetaData; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; @@ -66,6 +67,7 @@ public class TransportClearVotingTombstonesActionTests extends ESTestCase { private static ThreadPool threadPool; private static ClusterService clusterService; private static DiscoveryNode localNode, otherNode1, otherNode2; + private static VotingTombstone otherNode1Tombstone, otherNode2Tombstone; private TransportService transportService; @@ -74,7 +76,9 @@ public static void createThreadPoolAndClusterService() { threadPool = new TestThreadPool("test", Settings.EMPTY); localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); otherNode1 = new DiscoveryNode("other1", "other1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); + otherNode1Tombstone = new VotingTombstone(otherNode1); otherNode2 = new DiscoveryNode("other2", "other2", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); + otherNode2Tombstone = new VotingTombstone(otherNode2); clusterService = createClusterService(threadPool, localNode); } @@ -101,8 +105,8 @@ public void setupForTest() { .localNodeId(localNode.getId()).masterNodeId(localNode.getId())); builder.metaData(MetaData.builder() .coordinationMetaData(CoordinationMetaData.builder() - .addVotingTombstone(otherNode1) - .addVotingTombstone(otherNode2) + .addVotingTombstone(otherNode1Tombstone) + .addVotingTombstone(otherNode2Tombstone) .build())); setState(clusterService, builder); } @@ -141,7 +145,8 @@ public void testTimesOutIfWaitingForNodesThatAreNotRemoved() throws InterruptedE ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), containsInAnyOrder(otherNode1, otherNode2)); + assertThat(clusterService.getClusterApplierService().state().getVotingTombstones(), + containsInAnyOrder(otherNode1Tombstone, otherNode2Tombstone)); final Throwable rootCause = responseHolder.get().getRootCause(); assertThat(rootCause, instanceOf(ElasticsearchTimeoutException.class)); assertThat(rootCause.getMessage(), diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java index 863ced3c1b306..c8c163a37f799 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java @@ -87,7 +87,8 @@ public void testToXContent() throws IOException { " \"cluster_coordination\" : {\n" + " \"term\" : 0,\n" + " \"last_committed_config\" : [ ],\n" + - " \"last_accepted_config\" : [ ]\n" + + " \"last_accepted_config\" : [ ],\n" + + " \"voting_tombstones\" : [ ]\n" + " },\n" + " \"templates\" : { },\n" + " \"indices\" : {\n" + @@ -181,7 +182,8 @@ public void testToXContent() throws IOException { " \"cluster_coordination\" : {\n" + " \"term\" : 0,\n" + " \"last_committed_config\" : [ ],\n" + - " \"last_accepted_config\" : [ ]\n" + + " \"last_accepted_config\" : [ ],\n" + + " \"voting_tombstones\" : [ ]\n" + " },\n" + " \"templates\" : { },\n" + " \"indices\" : {\n" + diff --git a/server/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java b/server/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java index aa7f07c1bc027..b0d39776f2b0c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java +++ b/server/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java @@ -23,6 +23,7 @@ import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.coordination.CoordinationMetaData; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexGraveyard; import org.elasticsearch.cluster.metadata.IndexGraveyardTests; @@ -207,7 +208,7 @@ private ClusterState.Builder randomCoordinationMetaData(ClusterState clusterStat new CoordinationMetaData.VotingConfiguration(Sets.newHashSet(generateRandomStringArray(10, 10, false)))); } if (randomBoolean()) { - metaBuilder.addVotingTombstone(randomNode("node-" + randomAlphaOfLength(10))); + metaBuilder.addVotingTombstone(new VotingTombstone(randomNode("node-" + randomAlphaOfLength(10)))); } return builder; } diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinationMetaDataTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinationMetaDataTests.java index 58e755d4e6f4a..698ee18efc2a3 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinationMetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinationMetaDataTests.java @@ -18,10 +18,8 @@ */ package org.elasticsearch.cluster.coordination; -import org.elasticsearch.Version; import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfiguration; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.UUIDs; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.util.set.Sets; @@ -37,8 +35,6 @@ import java.util.HashSet; import java.util.Set; -import static java.util.Collections.emptyMap; -import static java.util.Collections.singleton; import static org.hamcrest.Matchers.equalTo; public class CoordinationMetaDataTests extends ESTestCase { @@ -97,6 +93,33 @@ private static VotingConfiguration randomVotingConfig() { return new VotingConfiguration(Sets.newHashSet(generateRandomStringArray(randomInt(10), 20, false))); } + public void testVotingTombstoneSerializationEqualsHashCode() { + VotingTombstone tombstone = new VotingTombstone(randomAlphaOfLength(10), randomAlphaOfLength(10)); + EqualsHashCodeTestUtils.checkEqualsAndHashCode(tombstone, + orig -> ESTestCase.copyWriteable(orig, new NamedWriteableRegistry(Collections.emptyList()), VotingTombstone::new), + orig -> randomlyChangeVotingTombstone(orig)); + } + + public void testVotingTombstoneXContent() throws IOException { + VotingTombstone originalTombstone = new VotingTombstone(randomAlphaOfLength(10), randomAlphaOfLength(10)); + + final XContentBuilder builder = JsonXContent.contentBuilder(); + originalTombstone.toXContent(builder, ToXContent.EMPTY_PARAMS); + + try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { + final VotingTombstone fromXContentTombstone = VotingTombstone.fromXContent(parser); + assertThat(originalTombstone, equalTo(fromXContentTombstone)); + } + } + + private VotingTombstone randomlyChangeVotingTombstone(VotingTombstone tombstone) { + if (randomBoolean()) { + return new VotingTombstone(randomAlphaOfLength(10), tombstone.getNodeName()); + } else { + return new VotingTombstone(tombstone.getNodeId(), randomAlphaOfLength(10)); + } + } + private VotingConfiguration randomlyChangeVotingConfiguration(VotingConfiguration cfg) { Set newNodeIds = new HashSet<>(cfg.getNodeIds()); if (cfg.isEmpty() == false && randomBoolean()) { @@ -113,20 +136,18 @@ private VotingConfiguration randomlyChangeVotingConfiguration(VotingConfiguratio return new VotingConfiguration(newNodeIds); } - private Set randomDiscoveryNodeSet() { + private Set randomVotingTombstones() { final int size = randomIntBetween(1, 10); - final Set nodes = new HashSet<>(size); + 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(DiscoveryNode.Role.MASTER), Version.CURRENT))); + assertTrue(nodes.add(new VotingTombstone(randomAlphaOfLength(10), randomAlphaOfLength(10)))); } return nodes; } public void testCoordinationMetaDataSerializationEqualsHashCode() { CoordinationMetaData initialMetaData = new CoordinationMetaData(randomNonNegativeLong(), randomVotingConfig(), randomVotingConfig(), - randomDiscoveryNodeSet()); + randomVotingTombstones()); EqualsHashCodeTestUtils.checkEqualsAndHashCode(initialMetaData, orig -> ESTestCase.copyWriteable(orig, new NamedWriteableRegistry(Collections.emptyList()), CoordinationMetaData::new), meta -> { @@ -145,7 +166,7 @@ public void testCoordinationMetaDataSerializationEqualsHashCode() { if (meta.getVotingTombstones().isEmpty() == false && randomBoolean()) { builder.clearVotingTombstones(); } else { - randomDiscoveryNodeSet().forEach(dn -> builder.addVotingTombstone(dn)); + randomVotingTombstones().forEach(dn -> builder.addVotingTombstone(dn)); } break; } @@ -155,7 +176,7 @@ public void testCoordinationMetaDataSerializationEqualsHashCode() { public void testXContent() throws IOException { CoordinationMetaData originalMeta = new CoordinationMetaData(randomNonNegativeLong(), randomVotingConfig(), randomVotingConfig(), - Collections.emptySet()); //TODO use non-empty tombstones set once toXContent for tombstones is implemented + randomVotingTombstones()); final XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java index b9c015f22dc16..b2f437c00e003 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java @@ -23,7 +23,7 @@ import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.coordination.CoordinationMetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingTombstone; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.bytes.BytesReference; @@ -44,15 +44,12 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import static java.util.Collections.emptyMap; -import static java.util.Collections.singleton; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; @@ -418,20 +415,18 @@ private static CoordinationMetaData.VotingConfiguration randomVotingConfig() { return new CoordinationMetaData.VotingConfiguration(Sets.newHashSet(generateRandomStringArray(randomInt(10), 20, false))); } - private Set randomDiscoveryNodeSet() { - final int size = randomIntBetween(1, 10); - final Set nodes = new HashSet<>(size); + private Set randomVotingTombstones() { + final int size = randomIntBetween(0, 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(DiscoveryNode.Role.MASTER), Version.CURRENT))); + assertTrue(nodes.add(new VotingTombstone(randomAlphaOfLength(10), randomAlphaOfLength(10)))); } return nodes; } public void testXContentWithCoordinationMetaData() throws IOException { CoordinationMetaData originalMeta = new CoordinationMetaData(randomNonNegativeLong(), randomVotingConfig(), randomVotingConfig(), - Collections.emptySet()); //TODO use non-empty tombstones set once toXContent for tombstones is implemented + randomVotingTombstones()); MetaData metaData = MetaData.builder().coordinationMetaData(originalMeta).build(); @@ -448,10 +443,10 @@ public void testXContentWithCoordinationMetaData() throws IOException { public void testGlobalStateEqualsCoordinationMetaData() { CoordinationMetaData coordinationMetaData1 = new CoordinationMetaData(randomNonNegativeLong(), randomVotingConfig(), - randomVotingConfig(), randomDiscoveryNodeSet()); + randomVotingConfig(), randomVotingTombstones()); MetaData metaData1 = MetaData.builder().coordinationMetaData(coordinationMetaData1).build(); CoordinationMetaData coordinationMetaData2 = new CoordinationMetaData(randomNonNegativeLong(), randomVotingConfig(), - randomVotingConfig(), randomDiscoveryNodeSet()); + randomVotingConfig(), randomVotingTombstones()); MetaData metaData2 = MetaData.builder().coordinationMetaData(coordinationMetaData2).build(); assertTrue(MetaData.isGlobalStateEquals(metaData1, metaData1));