From 44666b76e7e805753794ad7d6b5b9bca1f9102aa Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Fri, 19 May 2023 15:43:34 +0530 Subject: [PATCH] Fix failing test & incorporate comments Signed-off-by: Ashish Singh --- ...emoteStoreMockRepositoryIntegTestCase.java | 6 +-- .../remotestore/CreateRemoteIndexIT.java | 6 +-- ...teRefreshSegmentPressureSettingsTests.java | 4 +- .../RemoteStoreRefreshListenerTests.java | 44 +++++++++++++++++-- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java index 316a4a68a081a..dc312ffa6676d 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java @@ -38,11 +38,7 @@ public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends Abs @Override protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .put(FeatureFlags.REMOTE_STORE, "true") - .build(); + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); } @Before diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java index f8b56857168b8..8cc983b991d52 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java @@ -58,11 +58,7 @@ protected Settings nodeSettings(int nodeOriginal) { @Override protected Settings featureFlagSettings() { - return Settings.builder() - .put(super.featureFlagSettings()) - .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true") - .put(FeatureFlags.REMOTE_STORE, "true") - .build(); + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); } @Before diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteRefreshSegmentPressureSettingsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteRefreshSegmentPressureSettingsTests.java index b3c05a50ac881..75b5b946e8bf8 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteRefreshSegmentPressureSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteRefreshSegmentPressureSettingsTests.java @@ -55,10 +55,10 @@ public void testGetDefaultSettings() { assertFalse(pressureSettings.isRemoteRefreshSegmentPressureEnabled()); // Check bytes lag variance threshold default value - assertEquals(2.0, pressureSettings.getBytesLagVarianceFactor(), 0.0d); + assertEquals(10.0, pressureSettings.getBytesLagVarianceFactor(), 0.0d); // Check time lag variance threshold default value - assertEquals(2.0, pressureSettings.getUploadTimeLagVarianceFactor(), 0.0d); + assertEquals(10.0, pressureSettings.getUploadTimeLagVarianceFactor(), 0.0d); // Check minimum consecutive failures limit default value assertEquals(5, pressureSettings.getMinConsecutiveFailuresLimit()); 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 9fbe2143da196..365fb0237f80f 100644 --- a/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java +++ b/server/src/test/java/org/opensearch/index/shard/RemoteStoreRefreshListenerTests.java @@ -30,6 +30,7 @@ import org.opensearch.index.store.RemoteSegmentStoreDirectory; import org.opensearch.index.store.Store; import org.opensearch.indices.replication.checkpoint.SegmentReplicationCheckpointPublisher; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.threadpool.ThreadPool; import java.io.IOException; @@ -42,6 +43,7 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.index.shard.RemoteStoreRefreshListener.SEGMENT_INFO_SNAPSHOT_FILENAME_PREFIX; public class RemoteStoreRefreshListenerTests extends IndexShardTestCase { @@ -237,9 +239,19 @@ public void testRefreshSuccessOnFirstAttempt() throws Exception { // We spy on IndexShard.getEngine() to validate that we have successfully hit the terminal code for ascertaining successful upload. // Value has been set as 3 as during a successful upload IndexShard.getEngine() is hit thrice and with mockito we are counting down CountDownLatch successLatch = new CountDownLatch(3); - mockIndexShardWithRetryAndScheduleRefresh(succeedOnAttempt, refreshCountLatch, successLatch); + Tuple tuple = mockIndexShardWithRetryAndScheduleRefresh( + succeedOnAttempt, + refreshCountLatch, + successLatch + ); assertBusy(() -> assertEquals(0, refreshCountLatch.getCount())); 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()); } public void testRefreshSuccessOnSecondAttempt() throws Exception { @@ -251,9 +263,19 @@ public void testRefreshSuccessOnSecondAttempt() throws Exception { // We spy on IndexShard.getEngine() to validate that we have successfully hit the terminal code for ascertaining successful upload. // Value has been set as 3 as during a successful upload IndexShard.getEngine() is hit thrice and with mockito we are counting down CountDownLatch successLatch = new CountDownLatch(3); - mockIndexShardWithRetryAndScheduleRefresh(succeedOnAttempt, refreshCountLatch, successLatch); + Tuple tuple = mockIndexShardWithRetryAndScheduleRefresh( + succeedOnAttempt, + refreshCountLatch, + successLatch + ); assertBusy(() -> assertEquals(0, refreshCountLatch.getCount())); 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()); } public void testRefreshSuccessOnThirdAttemptAttempt() throws Exception { @@ -265,9 +287,20 @@ public void testRefreshSuccessOnThirdAttemptAttempt() throws Exception { // We spy on IndexShard.getEngine() to validate that we have successfully hit the terminal code for ascertaining successful upload. // Value has been set as 3 as during a successful upload IndexShard.getEngine() is hit thrice and with mockito we are counting down CountDownLatch successLatch = new CountDownLatch(3); - mockIndexShardWithRetryAndScheduleRefresh(succeedOnAttempt, refreshCountLatch, successLatch); + Tuple tuple = mockIndexShardWithRetryAndScheduleRefresh( + succeedOnAttempt, + refreshCountLatch, + successLatch + ); assertBusy(() -> assertEquals(0, refreshCountLatch.getCount())); 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()); + } public void testTrackerData() throws Exception { @@ -310,7 +343,10 @@ private Tuple m // Create index shard that we will be using to mock different methods in IndexShard for the unit test indexShard = newStartedShard( true, - Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true).build(), + Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true) + .put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .build(), new InternalEngineFactory() );