Skip to content

Commit 04e4394

Browse files
authored
Make DiscoveryNodes#mastersFirstStream deterministic (#94948)
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 `DiscoveryNodes#mastersFirstStream` yield the nodes in a deterministic order, which addresses one such area of unreproducibility. 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
1 parent 1421e2e commit 04e4394

File tree

2 files changed

+16
-5
lines changed

2 files changed

+16
-5
lines changed

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.AbstractCollection;
2525
import java.util.ArrayList;
2626
import java.util.Collections;
27+
import java.util.Comparator;
2728
import java.util.HashMap;
2829
import java.util.Iterator;
2930
import java.util.List;
@@ -185,11 +186,15 @@ public Map<String, DiscoveryNode> getCoordinatingOnlyNodes() {
185186
return filteredNodes(nodes, n -> n.canContainData() == false && n.isMasterNode() == false && n.isIngestNode() == false);
186187
}
187188

189+
private static final Comparator<DiscoveryNode> MASTERS_FIRST_COMPARATOR
190+
// Ugly hack: when https://github.com/elastic/elasticsearch/issues/94946 is fixed, remove the sorting by ephemeral ID here
191+
= Comparator.<DiscoveryNode>comparingInt(n -> n.isMasterNode() ? 0 : 1).thenComparing(DiscoveryNode::getEphemeralId);
192+
188193
/**
189194
* Returns a stream of all nodes, with master nodes at the front
190195
*/
191196
public Stream<DiscoveryNode> mastersFirstStream() {
192-
return Stream.concat(masterNodes.values().stream(), stream().filter(n -> n.isMasterNode() == false));
197+
return nodes.values().stream().sorted(MASTERS_FIRST_COMPARATOR);
193198
}
194199

195200
/**

server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import java.util.ArrayList;
1818
import java.util.Arrays;
1919
import java.util.Collections;
20-
import java.util.Comparator;
2120
import java.util.HashMap;
2221
import java.util.HashSet;
2322
import java.util.List;
@@ -160,9 +159,16 @@ public void testMastersFirst() {
160159
final List<DiscoveryNode> returnedNodes = discoBuilder.build().mastersFirstStream().toList();
161160
assertEquals(returnedNodes.size(), inputNodes.size());
162161
assertEquals(new HashSet<>(returnedNodes), new HashSet<>(inputNodes));
163-
final List<DiscoveryNode> sortedNodes = new ArrayList<>(returnedNodes);
164-
Collections.sort(sortedNodes, Comparator.comparing(n -> n.isMasterNode() == false));
165-
assertEquals(sortedNodes, returnedNodes);
162+
163+
boolean mastersOk = true;
164+
final var message = returnedNodes.toString();
165+
for (final var discoveryNode : returnedNodes) {
166+
if (discoveryNode.isMasterNode()) {
167+
assertTrue(message, mastersOk);
168+
} else {
169+
mastersOk = false;
170+
}
171+
}
166172
}
167173

168174
public void testDeltaListsMultipleNodes() {

0 commit comments

Comments
 (0)