Skip to content

Commit 5e3b6ab

Browse files
DaveCTurnerDaveCTurner
committed
Use VotingConfiguration#of where possible (#54507)
This resolves a longstanding TODO in the cluster coordination subsystem. Relates #32006
1 parent 325b8ec commit 5e3b6ab

File tree

6 files changed

+66
-74
lines changed

6 files changed

+66
-74
lines changed

Diff for: ‎server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationMetadata.java

-1
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
385385
}
386386

387387
public static VotingConfiguration of(DiscoveryNode... nodes) {
388-
// this could be used in many more places - TODO use this where appropriate
389388
return new VotingConfiguration(Arrays.stream(nodes).map(DiscoveryNode::getId).collect(Collectors.toSet()));
390389
}
391390
}

Diff for: ‎server/src/test/java/org/elasticsearch/cluster/coordination/CoordinationStateTests.java

+49-50
Large diffs are not rendered by default.

Diff for: ‎server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848

4949
import java.io.IOException;
5050
import java.util.Arrays;
51-
import java.util.Collections;
5251
import java.util.HashSet;
5352
import java.util.List;
5453
import java.util.Map;
@@ -775,8 +774,7 @@ public void testCannotSetInitialConfigurationWithoutQuorum() {
775774
assertThat(exceptionMessage, containsString(coordinator.getLocalNode().toString()));
776775

777776
// This is VERY BAD: setting a _different_ initial configuration. Yet it works if the first attempt will never be a quorum.
778-
assertTrue(coordinator.setInitialConfiguration(
779-
new VotingConfiguration(Collections.singleton(coordinator.getLocalNode().getId()))));
777+
assertTrue(coordinator.setInitialConfiguration(VotingConfiguration.of(coordinator.getLocalNode())));
780778
cluster.stabilise();
781779
}
782780
}

Diff for: ‎server/src/test/java/org/elasticsearch/cluster/coordination/NodeJoinTests.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ public void testJoinWithHigherTermElectsLeader() {
276276
long initialTerm = randomLongBetween(1, 10);
277277
long initialVersion = randomLongBetween(1, 10);
278278
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
279-
new VotingConfiguration(Collections.singleton(randomFrom(node0, node1).getId()))));
279+
VotingConfiguration.of(randomFrom(node0, node1))));
280280
assertFalse(isLocalNodeElectedMaster());
281281
assertNull(coordinator.getStateForMasterService().nodes().getMasterNodeId());
282282
long newTerm = initialTerm + randomLongBetween(1, 10);
@@ -296,7 +296,7 @@ public void testJoinWithHigherTermButBetterStateGetsRejected() {
296296
long initialTerm = randomLongBetween(1, 10);
297297
long initialVersion = randomLongBetween(1, 10);
298298
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
299-
new VotingConfiguration(Collections.singleton(node1.getId()))));
299+
VotingConfiguration.of(node1)));
300300
assertFalse(isLocalNodeElectedMaster());
301301
long newTerm = initialTerm + randomLongBetween(1, 10);
302302
long higherVersion = initialVersion + randomLongBetween(1, 10);
@@ -312,7 +312,7 @@ public void testJoinWithHigherTermButBetterStateStillElectsMasterThroughSelfJoin
312312
long initialTerm = randomLongBetween(1, 10);
313313
long initialVersion = randomLongBetween(1, 10);
314314
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
315-
new VotingConfiguration(Collections.singleton(node0.getId()))));
315+
VotingConfiguration.of(node0)));
316316
assertFalse(isLocalNodeElectedMaster());
317317
long newTerm = initialTerm + randomLongBetween(1, 10);
318318
long higherVersion = initialVersion + randomLongBetween(1, 10);
@@ -326,7 +326,7 @@ public void testJoinElectedLeader() {
326326
long initialTerm = randomLongBetween(1, 10);
327327
long initialVersion = randomLongBetween(1, 10);
328328
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
329-
new VotingConfiguration(Collections.singleton(node0.getId()))));
329+
VotingConfiguration.of(node0)));
330330
assertFalse(isLocalNodeElectedMaster());
331331
long newTerm = initialTerm + randomLongBetween(1, 10);
332332
joinNodeAndRun(new JoinRequest(node0, newTerm, Optional.of(new Join(node0, node0, newTerm, initialTerm, initialVersion))));
@@ -343,7 +343,7 @@ public void testJoinElectedLeaderWithHigherTerm() {
343343
long initialTerm = randomLongBetween(1, 10);
344344
long initialVersion = randomLongBetween(1, 10);
345345
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
346-
new VotingConfiguration(Collections.singleton(node0.getId()))));
346+
VotingConfiguration.of(node0)));
347347
long newTerm = initialTerm + randomLongBetween(1, 10);
348348

349349
joinNodeAndRun(new JoinRequest(node0, newTerm, Optional.of(new Join(node0, node0, newTerm, initialTerm, initialVersion))));
@@ -362,7 +362,7 @@ public void testJoinAccumulation() {
362362
long initialTerm = randomLongBetween(1, 10);
363363
long initialVersion = randomLongBetween(1, 10);
364364
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
365-
new VotingConfiguration(Collections.singleton(node2.getId()))));
365+
VotingConfiguration.of(node2)));
366366
assertFalse(isLocalNodeElectedMaster());
367367
long newTerm = initialTerm + randomLongBetween(1, 10);
368368
SimpleFuture futNode0 = joinNodeAsync(new JoinRequest(node0, newTerm, Optional.of(
@@ -389,7 +389,7 @@ public void testJoinFollowerWithHigherTerm() throws Exception {
389389
long initialTerm = randomLongBetween(1, 10);
390390
long initialVersion = randomLongBetween(1, 10);
391391
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
392-
new VotingConfiguration(Collections.singleton(node0.getId()))));
392+
VotingConfiguration.of(node0)));
393393
long newTerm = initialTerm + randomLongBetween(1, 10);
394394
handleStartJoinFrom(node1, newTerm);
395395
handleFollowerCheckFrom(node1, newTerm);
@@ -464,7 +464,7 @@ public void testJoinFollowerFails() throws Exception {
464464
long initialTerm = randomLongBetween(1, 10);
465465
long initialVersion = randomLongBetween(1, 10);
466466
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
467-
new VotingConfiguration(Collections.singleton(node0.getId()))));
467+
VotingConfiguration.of(node0)));
468468
long newTerm = initialTerm + randomLongBetween(1, 10);
469469
handleStartJoinFrom(node1, newTerm);
470470
handleFollowerCheckFrom(node1, newTerm);
@@ -480,7 +480,7 @@ public void testBecomeFollowerFailsPendingJoin() throws Exception {
480480
long initialTerm = randomLongBetween(1, 10);
481481
long initialVersion = randomLongBetween(1, 10);
482482
setupFakeMasterServiceAndCoordinator(initialTerm, initialState(node0, initialTerm, initialVersion,
483-
new VotingConfiguration(Collections.singleton(node1.getId()))));
483+
VotingConfiguration.of(node1)));
484484
long newTerm = initialTerm + randomLongBetween(1, 10);
485485
SimpleFuture fut = joinNodeAsync(new JoinRequest(node0, newTerm,
486486
Optional.of(new Join(node0, node0, newTerm, initialTerm, initialVersion))));

Diff for: ‎server/src/test/java/org/elasticsearch/cluster/coordination/PreVoteCollectorTests.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,12 @@
3737
import org.junit.Before;
3838

3939
import java.io.IOException;
40-
import java.util.Arrays;
4140
import java.util.HashMap;
4241
import java.util.HashSet;
4342
import java.util.Map;
4443
import java.util.Objects;
4544
import java.util.Set;
4645
import java.util.concurrent.atomic.AtomicReference;
47-
import java.util.stream.Collectors;
4846

4947
import static java.util.Collections.emptySet;
5048
import static org.elasticsearch.cluster.coordination.PreVoteCollector.REQUEST_PRE_VOTE_ACTION_NAME;
@@ -135,8 +133,7 @@ private void runCollector() {
135133
}
136134

137135
private ClusterState makeClusterState(DiscoveryNode[] votingNodes) {
138-
final VotingConfiguration votingConfiguration
139-
= new VotingConfiguration(Arrays.stream(votingNodes).map(DiscoveryNode::getId).collect(Collectors.toSet()));
136+
final VotingConfiguration votingConfiguration = VotingConfiguration.of(votingNodes);
140137
return CoordinationStateTests.clusterState(lastAcceptedTerm, lastAcceptedVersion, localNode,
141138
votingConfiguration, votingConfiguration, 0);
142139
}

Diff for: ‎server/src/test/java/org/elasticsearch/cluster/coordination/PublicationTests.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ private void initializeCluster(VotingConfiguration initialConfig) {
176176
}
177177

178178
public void testSimpleClusterStatePublishing() throws InterruptedException {
179-
VotingConfiguration singleNodeConfig = new VotingConfiguration(Sets.newHashSet(n1.getId()));
179+
VotingConfiguration singleNodeConfig = VotingConfiguration.of(n1);
180180
initializeCluster(singleNodeConfig);
181181

182182
AssertingAckListener ackListener = new AssertingAckListener(nodes.size());
@@ -252,7 +252,7 @@ public void testSimpleClusterStatePublishing() throws InterruptedException {
252252
}
253253

254254
public void testClusterStatePublishingWithFaultyNodeBeforeCommit() throws InterruptedException {
255-
VotingConfiguration singleNodeConfig = new VotingConfiguration(Sets.newHashSet(n1.getId()));
255+
VotingConfiguration singleNodeConfig = VotingConfiguration.of(n1);
256256
initializeCluster(singleNodeConfig);
257257

258258
AssertingAckListener ackListener = new AssertingAckListener(nodes.size());
@@ -295,7 +295,7 @@ public void testClusterStatePublishingWithFaultyNodeBeforeCommit() throws Interr
295295
}
296296

297297
public void testClusterStatePublishingWithFaultyNodeAfterCommit() throws InterruptedException {
298-
VotingConfiguration singleNodeConfig = new VotingConfiguration(Sets.newHashSet(n1.getId()));
298+
VotingConfiguration singleNodeConfig = VotingConfiguration.of(n1);
299299
initializeCluster(singleNodeConfig);
300300

301301
AssertingAckListener ackListener = new AssertingAckListener(nodes.size());
@@ -348,7 +348,7 @@ public void testClusterStatePublishingWithFaultyNodeAfterCommit() throws Interru
348348
}
349349

350350
public void testClusterStatePublishingFailsOrTimesOutBeforeCommit() throws InterruptedException {
351-
VotingConfiguration config = new VotingConfiguration(Sets.newHashSet(n1.getId(), n2.getId()));
351+
VotingConfiguration config = VotingConfiguration.of(n1, n2);
352352
initializeCluster(config);
353353

354354
AssertingAckListener ackListener = new AssertingAckListener(nodes.size());
@@ -387,7 +387,7 @@ public void testClusterStatePublishingFailsOrTimesOutBeforeCommit() throws Inter
387387
}
388388

389389
public void testPublishingToMastersFirst() {
390-
VotingConfiguration singleNodeConfig = new VotingConfiguration(Sets.newHashSet(n1.getId()));
390+
VotingConfiguration singleNodeConfig = VotingConfiguration.of(n1);
391391
initializeCluster(singleNodeConfig);
392392

393393
DiscoveryNodes.Builder discoNodesBuilder = DiscoveryNodes.builder();
@@ -403,8 +403,7 @@ public void testPublishingToMastersFirst() {
403403
}
404404

405405
public void testClusterStatePublishingTimesOutAfterCommit() throws InterruptedException {
406-
VotingConfiguration config = new VotingConfiguration(randomBoolean() ?
407-
Sets.newHashSet(n1.getId(), n2.getId()) : Sets.newHashSet(n1.getId(), n2.getId(), n3.getId()));
406+
VotingConfiguration config = randomBoolean() ? VotingConfiguration.of(n1, n2): VotingConfiguration.of(n1, n2, n3);
408407
initializeCluster(config);
409408

410409
AssertingAckListener ackListener = new AssertingAckListener(nodes.size());

0 commit comments

Comments
 (0)