[Remote Store] Add support to disable flush based on translog reader …

…count (opensearch-project#14027) (opensearch-project#14044) Signed-off-by: Shourya Dutta Biswas <[email protected]> Signed-off-by: kkewwei <[email protected]>
kkewwei · Jul 24, 2024 · a9ce600 · a9ce600
1 parent 5069dbb
commit a9ce600
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Remote Store] Add dynamic cluster settings to set timeout for segments upload to Remote Store ([#13679](https://github.com/opensearch-project/OpenSearch/pull/13679))
 - Add getMetadataFields to MapperService ([#13819](https://github.com/opensearch-project/OpenSearch/pull/13819))
 - Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776))
+- [Remote Store] Add support to disable flush based on translog reader count ([#14027](https://github.com/opensearch-project/OpenSearch/pull/14027))
 
 ### Dependencies
 - Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559))

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java
@@ -852,7 +852,9 @@ public void testFlushOnTooManyRemoteTranslogFiles() throws Exception {
 
         ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
         updateSettingsRequest.persistentSettings(
-            Settings.builder().put(RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS.getKey(), "100")
+            Settings.builder()
+                .put(RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS.getKey(), "100")
+                .put(CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "0ms")
         );
         assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
 
@@ -883,5 +885,27 @@ public void testFlushOnTooManyRemoteTranslogFiles() throws Exception {
                 assertEquals(totalFiles, 1L);
             }
         }, 30, TimeUnit.SECONDS);
+
+        // Disabling max translog readers
+        assertAcked(
+            internalCluster().client()
+                .admin()
+                .cluster()
+                .prepareUpdateSettings()
+                .setPersistentSettings(Settings.builder().put(RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS.getKey(), "-1"))
+                .get()
+        );
+
+        // Indexing 500 more docs
+        for (int i = 0; i < 500; i++) {
+            indexBulk(INDEX_NAME, 1);
+        }
+
+        // No flush is triggered since max_translog_readers is set to -1
+        // Total tlog files would be incremented by 500
+        try (Stream<Path> files = Files.list(translogLocation)) {
+            long totalFiles = files.filter(f -> f.getFileName().toString().endsWith(Translog.TRANSLOG_FILE_SUFFIX)).count();
+            assertEquals(totalFiles, 501L);
+        }
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java b/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java
@@ -706,6 +706,10 @@ int availablePermits() {
      */
     @Override
     protected boolean shouldFlush() {
-        return readers.size() >= translogTransferManager.getMaxRemoteTranslogReadersSettings();
+        int maxRemoteTlogReaders = translogTransferManager.getMaxRemoteTranslogReadersSettings();
+        if (maxRemoteTlogReaders == -1) {
+            return false;
+        }
+        return readers.size() >= maxRemoteTlogReaders;
     }
 }
diff --git a/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java b/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java
@@ -25,6 +25,7 @@
  */
 @PublicApi(since = "2.14.0")
 public class RemoteStoreSettings {
+    private static final int MIN_CLUSTER_REMOTE_MAX_TRANSLOG_READERS = 100;
 
     /**
      * Used to specify the default translog buffer interval for remote store backed indexes.
@@ -112,7 +113,12 @@ public class RemoteStoreSettings {
     public static final Setting<Integer> CLUSTER_REMOTE_MAX_TRANSLOG_READERS = Setting.intSetting(
         "cluster.remote_store.translog.max_readers",
         1000,
-        100,
+        -1,
+        v -> {
+            if (v != -1 && v < MIN_CLUSTER_REMOTE_MAX_TRANSLOG_READERS) {
+                throw new IllegalArgumentException("Cannot set value lower than " + MIN_CLUSTER_REMOTE_MAX_TRANSLOG_READERS);
+            }
+        },
         Property.Dynamic,
         Property.NodeScope
     );

diff --git a/server/src/test/java/org/opensearch/indices/RemoteStoreSettingsDynamicUpdateTests.java b/server/src/test/java/org/opensearch/indices/RemoteStoreSettingsDynamicUpdateTests.java
@@ -116,4 +116,15 @@ public void testMaxRemoteReferencedTranslogFiles() {
         );
         assertEquals(500, remoteStoreSettings.getMaxRemoteTranslogReaders());
     }
+
+    public void testDisableMaxRemoteReferencedTranslogFiles() {
+        // Test default value
+        assertEquals(1000, remoteStoreSettings.getMaxRemoteTranslogReaders());
+
+        // Test override with valid value
+        clusterSettings.applySettings(
+            Settings.builder().put(RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS.getKey(), "-1").build()
+        );
+        assertEquals(-1, remoteStoreSettings.getMaxRemoteTranslogReaders());
+    }
 }