From cfb551475e601c6ecb9e6f7c879eef2bcab30e95 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Sun, 9 Jul 2023 09:13:43 +0530 Subject: [PATCH 1/2] Fix flakyness in RemoteStoreRefreshListenerTests Signed-off-by: Ashish Singh --- .../index/shard/RemoteStoreRefreshListenerTests.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java b/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java index 688f29fa1f4bf..80aafcff76f95 100644 --- a/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java +++ b/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java @@ -322,11 +322,12 @@ public void testRefreshSuccessOnThirdAttemptAttempt() throws Exception { assertBusy(() -> assertEquals(0, successLatch.getCount())); RemoteRefreshSegmentPressureService pressureService = tuple.v2(); RemoteRefreshSegmentTracker segmentTracker = pressureService.getRemoteRefreshSegmentTracker(indexShard.shardId()); - assertEquals(0, segmentTracker.getBytesLag()); - assertEquals(0, segmentTracker.getRefreshSeqNoLag()); - assertEquals(0, segmentTracker.getTimeMsLag()); - assertEquals(2, segmentTracker.getTotalUploadsFailed()); - + assertBusy(() -> { + assertEquals(0, segmentTracker.getBytesLag()); + assertEquals(0, segmentTracker.getRefreshSeqNoLag()); + assertEquals(0, segmentTracker.getTimeMsLag()); + assertEquals(2, segmentTracker.getTotalUploadsFailed()); + }); } public void testTrackerData() throws Exception { From 43cc42f813c5c14d3884cc7f65537c240fe976a0 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Sun, 9 Jul 2023 22:00:37 +0530 Subject: [PATCH 2/2] Fix flakyness in RemoteStoreRefreshListenerTests Signed-off-by: Ashish Singh --- .../RemoteStoreRefreshListenerTests.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java b/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java index 80aafcff76f95..21a9393408529 100644 --- a/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java +++ b/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java @@ -21,9 +21,9 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.collect.Tuple; import org.opensearch.common.concurrent.GatedCloseable; +import org.opensearch.common.lease.Releasable; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.lease.Releasable; import org.opensearch.index.engine.InternalEngineFactory; import org.opensearch.index.remote.RemoteRefreshSegmentPressureService; import org.opensearch.index.remote.RemoteRefreshSegmentTracker; @@ -249,10 +249,7 @@ public void testRefreshSuccessOnFirstAttempt() throws Exception { assertBusy(() -> assertEquals(0, successLatch.getCount())); RemoteRefreshSegmentPressureService pressureService = tuple.v2(); RemoteRefreshSegmentTracker segmentTracker = pressureService.getRemoteRefreshSegmentTracker(indexShard.shardId()); - assertEquals(0, segmentTracker.getBytesLag()); - assertEquals(0, segmentTracker.getRefreshSeqNoLag()); - assertEquals(0, segmentTracker.getTimeMsLag()); - assertEquals(0, segmentTracker.getTotalUploadsFailed()); + assertNoLagAndTotalUploadsFailed(segmentTracker, 0); } public void testRefreshSuccessOnSecondAttempt() throws Exception { @@ -273,10 +270,7 @@ public void testRefreshSuccessOnSecondAttempt() throws Exception { assertBusy(() -> assertEquals(0, successLatch.getCount())); RemoteRefreshSegmentPressureService pressureService = tuple.v2(); RemoteRefreshSegmentTracker segmentTracker = pressureService.getRemoteRefreshSegmentTracker(indexShard.shardId()); - assertEquals(0, segmentTracker.getBytesLag()); - assertEquals(0, segmentTracker.getRefreshSeqNoLag()); - assertEquals(0, segmentTracker.getTimeMsLag()); - assertEquals(1, segmentTracker.getTotalUploadsFailed()); + assertNoLagAndTotalUploadsFailed(segmentTracker, 1); } /** @@ -304,7 +298,7 @@ public void testRefreshSuccessAfterFailureInFirstAttemptAfterSnapshotAndMetadata assertBusy(() -> assertEquals(0, reachedCheckpointPublishLatch.getCount())); } - public void testRefreshSuccessOnThirdAttemptAttempt() throws Exception { + public void testRefreshSuccessOnThirdAttempt() throws Exception { // This covers 3 cases - 1) isRetry=false, shouldRetry=true 2) isRetry=true, shouldRetry=false 3) isRetry=True, shouldRetry=true // Succeed on 3rd attempt int succeedOnAttempt = 3; @@ -322,11 +316,15 @@ public void testRefreshSuccessOnThirdAttemptAttempt() throws Exception { assertBusy(() -> assertEquals(0, successLatch.getCount())); RemoteRefreshSegmentPressureService pressureService = tuple.v2(); RemoteRefreshSegmentTracker segmentTracker = pressureService.getRemoteRefreshSegmentTracker(indexShard.shardId()); + assertNoLagAndTotalUploadsFailed(segmentTracker, 2); + } + + private void assertNoLagAndTotalUploadsFailed(RemoteRefreshSegmentTracker segmentTracker, long totalUploadsFailed) throws Exception { assertBusy(() -> { assertEquals(0, segmentTracker.getBytesLag()); assertEquals(0, segmentTracker.getRefreshSeqNoLag()); assertEquals(0, segmentTracker.getTimeMsLag()); - assertEquals(2, segmentTracker.getTotalUploadsFailed()); + assertEquals(totalUploadsFailed, segmentTracker.getTotalUploadsFailed()); }); }