Skip to content

Commit 81d4efa

Browse files
authored
In ARS, correct default number of outstanding requests (#71245)
When computing node’s ARS rank, we use the number of outstanding search requests to the node. If there are no connections to the node, we consider there to be 1 outstanding request. This isn’t accurate, the default should be 0 to indicate no outstanding requests. The ARS rank we return in node stats actually uses 0 instead of 1. This small fix lets us remove a test workaround. It also ensures the ARS ranks we return in node stats match the ranks we use to select shards during search. Follow-up to #70283.
1 parent 1b35100 commit 81d4efa

File tree

2 files changed

+5
-9
lines changed

2 files changed

+5
-9
lines changed

server/src/main/java/org/elasticsearch/cluster/routing/IndexShardRoutingTable.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ private static Map<String, Double> rankNodes(final Map<String, Optional<Response
289289
Optional<ResponseCollectorService.ComputedNodeStats> maybeStats = entry.getValue();
290290
maybeStats.ifPresent(stats -> {
291291
final String nodeId = entry.getKey();
292-
nodeRanks.put(nodeId, stats.rank(nodeSearchCounts.getOrDefault(nodeId, 1L)));
292+
nodeRanks.put(nodeId, stats.rank(nodeSearchCounts.getOrDefault(nodeId, 0L)));
293293
});
294294
}
295295
return nodeRanks;

server/src/test/java/org/elasticsearch/cluster/routing/OperationRoutingTests.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -658,8 +658,6 @@ public void testARSOutstandingRequestTracking() throws Exception {
658658
collector.addNodeStatistics("node_0", 1, TimeValue.timeValueMillis(50).nanos(), TimeValue.timeValueMillis(40).nanos());
659659
collector.addNodeStatistics("node_1", 1, TimeValue.timeValueMillis(51).nanos(), TimeValue.timeValueMillis(40).nanos());
660660
Map<String, Long> outstandingRequests = new HashMap<>();
661-
outstandingRequests.put("node_0", 1L);
662-
outstandingRequests.put("node_1", 1L);
663661

664662
// Check that we choose to search over both nodes
665663
GroupShardsIterator<ShardIterator> groupIterator = opRouting.searchShards(
@@ -669,14 +667,12 @@ public void testARSOutstandingRequestTracking() throws Exception {
669667
nodeIds.add(groupIterator.get(0).nextOrNull().currentNodeId());
670668
nodeIds.add(groupIterator.get(1).nextOrNull().currentNodeId());
671669
assertThat(nodeIds, containsInAnyOrder("node_0", "node_1"));
672-
assertThat(outstandingRequests.get("node_0"), equalTo(2L));
673-
assertThat(outstandingRequests.get("node_1"), equalTo(2L));
670+
assertThat(outstandingRequests.get("node_0"), equalTo(1L));
671+
assertThat(outstandingRequests.get("node_1"), equalTo(1L));
674672

675673
// The first node becomes much more loaded
676-
collector.addNodeStatistics("node_0", 5, TimeValue.timeValueMillis(300).nanos(), TimeValue.timeValueMillis(200).nanos());
674+
collector.addNodeStatistics("node_0", 6, TimeValue.timeValueMillis(300).nanos(), TimeValue.timeValueMillis(200).nanos());
677675
outstandingRequests = new HashMap<>();
678-
outstandingRequests.put("node_0", 1L);
679-
outstandingRequests.put("node_1", 1L);
680676

681677
// Check that we always choose the second node
682678
groupIterator = opRouting.searchShards(
@@ -686,7 +682,7 @@ public void testARSOutstandingRequestTracking() throws Exception {
686682
nodeIds.add(groupIterator.get(0).nextOrNull().currentNodeId());
687683
nodeIds.add(groupIterator.get(1).nextOrNull().currentNodeId());
688684
assertThat(nodeIds, containsInAnyOrder("node_1"));
689-
assertThat(outstandingRequests.get("node_1"), equalTo(3L));
685+
assertThat(outstandingRequests.get("node_1"), equalTo(2L));
690686

691687
IOUtils.close(clusterService);
692688
terminate(threadPool);

0 commit comments

Comments
 (0)