From 179c1e7274423b1dcdeaab1b4f6e70bfd4399f4c Mon Sep 17 00:00:00 2001 From: Andrew Ross Date: Tue, 8 Nov 2022 16:13:38 -0800 Subject: [PATCH] Fix boundary condition in indexing pressure test 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 --- ...exingPressureConcurrentExecutionTests.java | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/ShardIndexingPressureConcurrentExecutionTests.java b/server/src/test/java/org/opensearch/index/ShardIndexingPressureConcurrentExecutionTests.java index 72ca8bff4087d..faab2f405010a 100644 --- a/server/src/test/java/org/opensearch/index/ShardIndexingPressureConcurrentExecutionTests.java +++ b/server/src/test/java/org/opensearch/index/ShardIndexingPressureConcurrentExecutionTests.java @@ -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; @@ -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() @@ -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, @@ -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++) { @@ -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++) { @@ -1087,4 +1085,11 @@ private void fireConcurrentAndParallelRequestsForUniformThroughPut( t.join(); } } + + private Matcher isInOperatingFactorRange() { + return allOf( + greaterThan(ShardIndexingPressureMemoryManager.LOWER_OPERATING_FACTOR.get(settings)), + lessThanOrEqualTo(ShardIndexingPressureMemoryManager.UPPER_OPERATING_FACTOR.get(settings)) + ); + } }