Skip to content

Commit ef51c81

Browse files
authored
Adjust the formula for "adaptive replica selection" (#144562)
Makes a change to the C3 ranking formula used for adaptive replica selection, gated behind a feature flag. This change is motivated by inconsistencies between the intention of the formula and the current state of search. The first term of the C3 ranking formula is (R - S), meant to isolate network overhead and queue time. Inconsistency 1: Since inter-segment search concurrency was introduced to the query phase (QUERY_PHASE_PARALLEL_COLLECTION_ENABLED), search threads may now only process a portion of the shard query. That means the value of S may be significantly smaller (it's an exponentially weighted moving average), and the (R - S) term far overestimates network latency and queueing. Inconsistency 2: Batched query execution (BATCHED_QUERY_PHASE) complicates this further. If multiple shards are batched into the same transport request, the total response time R will be for the entire batch, making (R - S) further overestimate network latency and queueing.
1 parent 3825623 commit ef51c81

File tree

4 files changed

+59
-14
lines changed

4 files changed

+59
-14
lines changed

docs/changelog/144562.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
area: Search
2+
issues: []
3+
pr: 144562
4+
summary: Adjust the formula for "adaptive replica selection"
5+
type: bug

server/src/main/java/org/elasticsearch/node/ResponseCollectorService.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.common.io.stream.StreamInput;
1818
import org.elasticsearch.common.io.stream.StreamOutput;
1919
import org.elasticsearch.common.io.stream.Writeable;
20+
import org.elasticsearch.common.util.FeatureFlag;
2021
import org.elasticsearch.common.util.Maps;
2122
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
2223

@@ -38,6 +39,8 @@ public final class ResponseCollectorService implements ClusterStateListener {
3839
*/
3940
public static final double ALPHA = 0.3;
4041

42+
public static final FeatureFlag ARS_FORMULA_ADJUSTMENT_FEATURE_FLAG = new FeatureFlag("ars_formula_adjustment");
43+
4144
private final ConcurrentMap<String, NodeStatistics> nodeIdToStats = ConcurrentCollections.newConcurrentMap();
4245

4346
public ResponseCollectorService(ClusterService clusterService) {
@@ -172,8 +175,12 @@ private double innerRank(long outstandingRequests) {
172175
// defines service time as the inverse of service rate (muBarS).
173176
double muBarSInverse = serviceTime / FACTOR;
174177

175-
// The final formula
176-
return rS - muBarSInverse + Math.pow(qHatS, queueAdjustmentFactor) * muBarSInverse;
178+
double innerRank = Math.pow(qHatS, queueAdjustmentFactor) * muBarSInverse;
179+
// When the feature flag is enabled, the rS - muBarSInverse term is dropped.
180+
if (ARS_FORMULA_ADJUSTMENT_FEATURE_FLAG.isEnabled() == false) {
181+
innerRank += rS - muBarSInverse;
182+
}
183+
return innerRank;
177184
}
178185

179186
public double rank(long outstandingRequests) {

server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/ComputedNodeStatsTests.java

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.action.admin.cluster.node.stats;
1111

12+
import org.elasticsearch.node.ResponseCollectorService;
1213
import org.elasticsearch.node.ResponseCollectorService.ComputedNodeStats;
1314
import org.elasticsearch.test.ESTestCase;
1415

@@ -19,13 +20,26 @@
1920
*/
2021
public class ComputedNodeStatsTests extends ESTestCase {
2122

23+
private static final boolean ARS_ADJUSTMENT = ResponseCollectorService.ARS_FORMULA_ADJUSTMENT_FEATURE_FLAG.isEnabled();
24+
2225
public void testBasicInvariants() {
23-
// When queue size estimate is 0, the rank should equal response time.
24-
ComputedNodeStats stats = createStats(0, 150, 100);
25-
assertThat(stats.rank(0), equalTo(150.0));
26+
if (ARS_ADJUSTMENT) {
27+
// With the adjustment, rank = qHatS^3 * muBarSInverse.
28+
// When queueSize=0 and outstandingRequests=0: qHatS = 1, so rank = muBarSInverse = serviceTime.
29+
ComputedNodeStats stats = createStats(0, 150, 100);
30+
assertThat(stats.rank(0), equalTo(100.0));
31+
32+
stats = createStats(0, 20, 19);
33+
assertThat(stats.rank(0), equalTo(19.0));
34+
} else {
35+
// Without the adjustment, rank = rS - muBarSInverse + qHatS^3 * muBarSInverse.
36+
// When queueSize=0 and outstandingRequests=0: qHatS = 1, so rank = rS.
37+
ComputedNodeStats stats = createStats(0, 150, 100);
38+
assertThat(stats.rank(0), equalTo(150.0));
2639

27-
stats = createStats(0, 20, 19);
28-
assertThat(stats.rank(0), equalTo(20.0));
40+
stats = createStats(0, 20, 19);
41+
assertThat(stats.rank(0), equalTo(20.0));
42+
}
2943
}
3044

3145
public void testParameterScaling() {
@@ -38,14 +52,16 @@ public void testParameterScaling() {
3852
second = createStats(1, 200, 200);
3953
assertTrue(first.rank(3) < second.rank(3));
4054

41-
// A larger response time should always result in a larger rank.
42-
first = createStats(2, 150, 100);
43-
second = createStats(2, 200, 100);
44-
assertTrue(first.rank(1) < second.rank(1));
55+
if (ARS_ADJUSTMENT == false) {
56+
// A larger response time should always result in a larger rank (only when the response time term is included).
57+
first = createStats(2, 150, 100);
58+
second = createStats(2, 200, 100);
59+
assertTrue(first.rank(1) < second.rank(1));
4560

46-
first = createStats(2, 150, 150);
47-
second = createStats(2, 200, 150);
48-
assertTrue(first.rank(1) < second.rank(1));
61+
first = createStats(2, 150, 150);
62+
second = createStats(2, 200, 150);
63+
assertTrue(first.rank(1) < second.rank(1));
64+
}
4965

5066
// More queued requests should always result in a larger rank.
5167
first = createStats(2, 150, 100);

server/src/test/java/org/elasticsearch/node/ResponseCollectorServiceTests.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,21 @@ public void testNodeRemoval() throws Exception {
139139
assertTrue(nodeStats.containsKey("node1"));
140140
assertFalse(nodeStats.containsKey("node2"));
141141
}
142+
143+
public void testArsFormulaAdjustmentFeatureFlag() {
144+
// 100ms response time, 10ms service time
145+
collector.addNodeStatistics("node1", 1, 100 * 1_000_000L, 10 * 1_000_000L);
146+
double rank = collector.getAllNodeStatistics().get("node1").rank(1);
147+
148+
if (ResponseCollectorService.ARS_FORMULA_ADJUSTMENT_FEATURE_FLAG.isEnabled()) {
149+
// With the adjustment enabled, the response time component (rS - muBarSInverse) is dropped,
150+
// so rank should equal just the queue-based term: qHatS^3 * muBarSInverse
151+
// qHatS = 1 + 1*1 + 1 = 3, muBarSInverse = 10ms, so rank = 27 * 10 = 270
152+
assertThat(rank, equalTo(270.0));
153+
} else {
154+
// Without the adjustment, rank = (rS - muBarSInverse) + qHatS^3 * muBarSInverse
155+
// = (100 - 10) + 270 = 360
156+
assertThat(rank, equalTo(360.0));
157+
}
158+
}
142159
}

0 commit comments

Comments
 (0)