Skip to content

Commit aff2b1f

Browse files
Fix boundary condition in indexing pressure test (#5168) (#5179)
This updates the boundary condition in an assertion in two tests in ShardIndexingPressureConcurrentExecutionTests. I could reliably reproduce errors here by running: ``` ./gradlew ':server:test' -Dtests.iters=10000 --tests "org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testReplicaThreadedUpdateToShardLimits" ``` On every error the value that failed was exactly 0.95 and failed the less than check. The change here is to accept 0.95, and also refactor the test to give a better error message on failure. Signed-off-by: Andrew Ross <andrross@amazon.com> Signed-off-by: Andrew Ross <andrross@amazon.com> (cherry picked from commit 3423f44) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: Andrew Ross <andrross@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 73af856 commit aff2b1f

1 file changed

Lines changed: 21 additions & 16 deletions

File tree

server/src/test/java/org/opensearch/index/ShardIndexingPressureConcurrentExecutionTests.java

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.opensearch.index;
1010

11+
import org.hamcrest.Matcher;
12+
import org.hamcrest.MatcherAssert;
1113
import org.hamcrest.Matchers;
1214
import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
1315
import org.opensearch.cluster.service.ClusterService;
@@ -23,6 +25,10 @@
2325

2426
import java.util.concurrent.atomic.AtomicInteger;
2527

28+
import static org.hamcrest.Matchers.allOf;
29+
import static org.hamcrest.Matchers.greaterThan;
30+
import static org.hamcrest.Matchers.lessThanOrEqualTo;
31+
2632
public class ShardIndexingPressureConcurrentExecutionTests extends OpenSearchTestCase {
2733

2834
private final Settings settings = Settings.builder()
@@ -34,8 +40,8 @@ public class ShardIndexingPressureConcurrentExecutionTests extends OpenSearchTes
3440
.put(ShardIndexingPressureSettings.REQUEST_SIZE_WINDOW.getKey(), 100)
3541
.build();
3642

37-
final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
38-
final ClusterService clusterService = new ClusterService(settings, clusterSettings, null);
43+
private final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
44+
private final ClusterService clusterService = new ClusterService(settings, clusterSettings, null);
3945

4046
public enum OperationType {
4147
COORDINATING,
@@ -71,15 +77,11 @@ public void testCoordinatingPrimaryThreadedUpdateToShardLimits() throws Exceptio
7177
NUM_THREADS * 15,
7278
shardIndexingPressure.shardStats().getIndexingPressureShardStats(shardId1).getCurrentCombinedCoordinatingAndPrimaryBytes()
7379
);
74-
assertTrue(
80+
MatcherAssert.assertThat(
7581
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
7682
.getIndexingPressureShardStats(shardId1)
77-
.getCurrentPrimaryAndCoordinatingLimits() < 0.95
78-
);
79-
assertTrue(
80-
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
81-
.getIndexingPressureShardStats(shardId1)
82-
.getCurrentPrimaryAndCoordinatingLimits() > 0.75
83+
.getCurrentPrimaryAndCoordinatingLimits(),
84+
isInOperatingFactorRange()
8385
);
8486

8587
for (int i = 0; i < NUM_THREADS; i++) {
@@ -112,15 +114,11 @@ public void testReplicaThreadedUpdateToShardLimits() throws Exception {
112114
Releasable[] releasable = fireConcurrentRequests(NUM_THREADS, shardIndexingPressure, shardId1, 15, OperationType.REPLICA);
113115

114116
assertEquals(NUM_THREADS * 15, shardIndexingPressure.shardStats().getIndexingPressureShardStats(shardId1).getCurrentReplicaBytes());
115-
assertTrue(
116-
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
117-
.getIndexingPressureShardStats(shardId1)
118-
.getCurrentReplicaLimits() < 0.95
119-
);
120-
assertTrue(
117+
MatcherAssert.assertThat(
121118
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
122119
.getIndexingPressureShardStats(shardId1)
123-
.getCurrentReplicaLimits() > 0.75
120+
.getCurrentReplicaLimits(),
121+
isInOperatingFactorRange()
124122
);
125123

126124
for (int i = 0; i < NUM_THREADS; i++) {
@@ -1087,4 +1085,11 @@ private void fireConcurrentAndParallelRequestsForUniformThroughPut(
10871085
t.join();
10881086
}
10891087
}
1088+
1089+
private Matcher<Double> isInOperatingFactorRange() {
1090+
return allOf(
1091+
greaterThan(ShardIndexingPressureMemoryManager.LOWER_OPERATING_FACTOR.get(settings)),
1092+
lessThanOrEqualTo(ShardIndexingPressureMemoryManager.UPPER_OPERATING_FACTOR.get(settings))
1093+
);
1094+
}
10901095
}

0 commit comments

Comments
 (0)