Skip to content

Commit

Permalink
Make NodeConnectionsService#connectToNodes deterministic (#94949)
Browse files Browse the repository at this point in the history
Today the `CoordinatorTests` test suite is not totally deterministic
because its behaviour depends on the iteration order of the JDK's
unordered collections which are not under the control of our test seed.

This commit makes `NodeConnectionsService#connectToNodes` iterate
through the nodes using `DiscoveryNodes#mastersFirstStream`, which is
made deterministic by #94948. It's an ugly hack to do some extra work in
production just for the sake of tests, but we're only sorting at most a
few hundred elements here so it's not a huge deal.

Relates #94946
  • Loading branch information
DaveCTurner committed Mar 31, 2023
1 parent 964e309 commit ff2591d
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -99,7 +100,9 @@ public void connectToNodes(DiscoveryNodes discoveryNodes, Runnable onCompletion)
final List<Runnable> runnables = new ArrayList<>(discoveryNodes.getSize());
try (var refs = new RefCountingRunnable(onCompletion)) {
synchronized (mutex) {
for (final DiscoveryNode discoveryNode : discoveryNodes) {
// Ugly hack: when https://github.com/elastic/elasticsearch/issues/94946 is fixed, just iterate over discoveryNodes here
for (final Iterator<DiscoveryNode> iterator = discoveryNodes.mastersFirstStream().iterator(); iterator.hasNext();) {
final DiscoveryNode discoveryNode = iterator.next();
ConnectionTarget connectionTarget = targetsByNode.get(discoveryNode);
final boolean isNewNode = connectionTarget == null;
if (isNewNode) {
Expand Down

0 comments on commit ff2591d

Please sign in to comment.