From 3013bac75e4b8f278cb0e87ab96051f9c9000f20 Mon Sep 17 00:00:00 2001 From: Shubh Sahu Date: Wed, 5 Jun 2024 17:22:05 +0530 Subject: [PATCH] Making Recovery Chunk Size setting dynamic Signed-off-by: Shubh Sahu --- .../indices/recovery/IndexRecoveryIT.java | 2 +- .../common/settings/ClusterSettings.java | 1 + .../indices/recovery/RecoverySettings.java | 19 ++++++++++++------- .../RecoverySettingsDynamicUpdateTests.java | 9 +++++++++ .../index/shard/IndexShardTestCase.java | 2 +- .../node/RecoverySettingsChunkSizePlugin.java | 2 +- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java index 8ce87f37d77cd..6c41c7a4c9e30 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java @@ -278,7 +278,7 @@ private void restoreRecoverySpeed() { .setTransientSettings( Settings.builder() .put(RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey(), "20mb") - .put(CHUNK_SIZE_SETTING.getKey(), RecoverySettings.DEFAULT_CHUNK_SIZE) + .put(CHUNK_SIZE_SETTING.getKey(), RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getDefault(Settings.EMPTY)) ) .get() .isAcknowledged() diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 297fc98764d07..a53b9e82f31f4 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -306,6 +306,7 @@ public void apply(Settings value, Settings current, Settings previous) { RecoverySettings.INDICES_RECOVERY_MAX_CONCURRENT_OPERATIONS_SETTING, RecoverySettings.INDICES_RECOVERY_MAX_CONCURRENT_REMOTE_STORE_STREAMS_SETTING, RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT, + RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING, ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_PRIMARIES_RECOVERIES_SETTING, ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_REPLICAS_RECOVERIES_SETTING, ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING, diff --git a/server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java b/server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java index 8f9da6babdd99..c34b978464852 100644 --- a/server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java +++ b/server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java @@ -177,7 +177,14 @@ public class RecoverySettings { ); // choose 512KB-16B to ensure that the resulting byte[] is not a humongous allocation in G1. - public static final ByteSizeValue DEFAULT_CHUNK_SIZE = new ByteSizeValue(512 * 1024 - 16, ByteSizeUnit.BYTES); + public static final Setting INDICES_RECOVERY_CHUNK_SIZE_SETTING = Setting.byteSizeSetting( + "indices.recovery.chunk_size", + new ByteSizeValue(512 * 1024 - 16, ByteSizeUnit.BYTES), + new ByteSizeValue(0, ByteSizeUnit.BYTES), + new ByteSizeValue(100 * 1024 * 1024, ByteSizeUnit.BYTES), + Property.Dynamic, + Property.NodeScope + ); private volatile ByteSizeValue recoveryMaxBytesPerSec; private volatile ByteSizeValue replicationMaxBytesPerSec; @@ -193,7 +200,7 @@ public class RecoverySettings { private volatile TimeValue internalActionRetryTimeout; private volatile TimeValue internalActionLongTimeout; - private volatile ByteSizeValue chunkSize = DEFAULT_CHUNK_SIZE; + private volatile ByteSizeValue chunkSize; private volatile TimeValue internalRemoteUploadTimeout; public RecoverySettings(Settings settings, ClusterSettings clusterSettings) { @@ -221,6 +228,7 @@ public RecoverySettings(Settings settings, ClusterSettings clusterSettings) { logger.debug("using recovery max_bytes_per_sec[{}]", recoveryMaxBytesPerSec); this.internalRemoteUploadTimeout = INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.get(settings); + this.chunkSize = INDICES_RECOVERY_CHUNK_SIZE_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING, this::setRecoveryMaxBytesPerSec); clusterSettings.addSettingsUpdateConsumer(INDICES_REPLICATION_MAX_BYTES_PER_SEC_SETTING, this::setReplicationMaxBytesPerSec); @@ -239,7 +247,7 @@ public RecoverySettings(Settings settings, ClusterSettings clusterSettings) { ); clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_ACTIVITY_TIMEOUT_SETTING, this::setActivityTimeout); clusterSettings.addSettingsUpdateConsumer(INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT, this::setInternalRemoteUploadTimeout); - + clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_CHUNK_SIZE_SETTING, this::setChunkSize); } public RateLimiter recoveryRateLimiter() { @@ -282,10 +290,7 @@ public ByteSizeValue getChunkSize() { return chunkSize; } - public void setChunkSize(ByteSizeValue chunkSize) { // only settable for tests - if (chunkSize.bytesAsInt() <= 0) { - throw new IllegalArgumentException("chunkSize must be > 0"); - } + public void setChunkSize(ByteSizeValue chunkSize) { this.chunkSize = chunkSize; } diff --git a/server/src/test/java/org/opensearch/indices/recovery/RecoverySettingsDynamicUpdateTests.java b/server/src/test/java/org/opensearch/indices/recovery/RecoverySettingsDynamicUpdateTests.java index 2793d446d66c8..7991100a21856 100644 --- a/server/src/test/java/org/opensearch/indices/recovery/RecoverySettingsDynamicUpdateTests.java +++ b/server/src/test/java/org/opensearch/indices/recovery/RecoverySettingsDynamicUpdateTests.java @@ -118,4 +118,13 @@ public void testInternalLongActionTimeout() { ); assertEquals(new TimeValue(duration, timeUnit), recoverySettings.internalActionLongTimeout()); } + + public void testChunkSize() { + ByteSizeValue chunkSize = new ByteSizeValue(between(1, 1000), ByteSizeUnit.BYTES); + clusterSettings.applySettings( + Settings.builder().put(RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getKey(), chunkSize).build() + ); + assertEquals(chunkSize, recoverySettings.getChunkSize()); + } + } diff --git a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java index 6b609d8af62a1..655a9eb7d5d38 100644 --- a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java +++ b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java @@ -1148,7 +1148,7 @@ public final void recoverUnstartedReplica( startingSeqNo ); long fileChunkSizeInBytes = randomBoolean() - ? RecoverySettings.DEFAULT_CHUNK_SIZE.getBytes() + ? RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getDefault(Settings.EMPTY).getBytes() : randomIntBetween(1, 10 * 1024 * 1024); final Settings settings = Settings.builder() .put("indices.recovery.max_concurrent_file_chunks", Integer.toString(between(1, 4))) diff --git a/test/framework/src/main/java/org/opensearch/node/RecoverySettingsChunkSizePlugin.java b/test/framework/src/main/java/org/opensearch/node/RecoverySettingsChunkSizePlugin.java index dabf23ce08263..49a18b03799ec 100644 --- a/test/framework/src/main/java/org/opensearch/node/RecoverySettingsChunkSizePlugin.java +++ b/test/framework/src/main/java/org/opensearch/node/RecoverySettingsChunkSizePlugin.java @@ -51,7 +51,7 @@ public class RecoverySettingsChunkSizePlugin extends Plugin { */ public static final Setting CHUNK_SIZE_SETTING = Setting.byteSizeSetting( "indices.recovery.chunk_size", - RecoverySettings.DEFAULT_CHUNK_SIZE, + RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING, Property.Dynamic, Property.NodeScope );