Skip to content

Commit

Permalink
Fix boundary condition in indexing pressure test (#5168)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
github-actions[bot] committed Nov 9, 2022
1 parent 092ca99 commit 02cfb0d
Showing 1 changed file with 21 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.opensearch.index;

import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
import org.opensearch.cluster.service.ClusterService;
Expand All @@ -23,6 +25,10 @@

import java.util.concurrent.atomic.AtomicInteger;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.lessThanOrEqualTo;

public class ShardIndexingPressureConcurrentExecutionTests extends OpenSearchTestCase {

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

final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
final ClusterService clusterService = new ClusterService(settings, clusterSettings, null);
private final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
private final ClusterService clusterService = new ClusterService(settings, clusterSettings, null);

public enum OperationType {
COORDINATING,
Expand Down Expand Up @@ -71,15 +77,11 @@ public void testCoordinatingPrimaryThreadedUpdateToShardLimits() throws Exceptio
NUM_THREADS * 15,
shardIndexingPressure.shardStats().getIndexingPressureShardStats(shardId1).getCurrentCombinedCoordinatingAndPrimaryBytes()
);
assertTrue(
MatcherAssert.assertThat(
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
.getIndexingPressureShardStats(shardId1)
.getCurrentPrimaryAndCoordinatingLimits() < 0.95
);
assertTrue(
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
.getIndexingPressureShardStats(shardId1)
.getCurrentPrimaryAndCoordinatingLimits() > 0.75
.getCurrentPrimaryAndCoordinatingLimits(),
isInOperatingFactorRange()
);

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

assertEquals(NUM_THREADS * 15, shardIndexingPressure.shardStats().getIndexingPressureShardStats(shardId1).getCurrentReplicaBytes());
assertTrue(
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
.getIndexingPressureShardStats(shardId1)
.getCurrentReplicaLimits() < 0.95
);
assertTrue(
MatcherAssert.assertThat(
(double) (NUM_THREADS * 15) / shardIndexingPressure.shardStats()
.getIndexingPressureShardStats(shardId1)
.getCurrentReplicaLimits() > 0.75
.getCurrentReplicaLimits(),
isInOperatingFactorRange()
);

for (int i = 0; i < NUM_THREADS; i++) {
Expand Down Expand Up @@ -1087,4 +1085,11 @@ private void fireConcurrentAndParallelRequestsForUniformThroughPut(
t.join();
}
}

private Matcher<Double> isInOperatingFactorRange() {
return allOf(
greaterThan(ShardIndexingPressureMemoryManager.LOWER_OPERATING_FACTOR.get(settings)),
lessThanOrEqualTo(ShardIndexingPressureMemoryManager.UPPER_OPERATING_FACTOR.get(settings))
);
}
}

0 comments on commit 02cfb0d

Please sign in to comment.