Skip to content

Commit 3013bac

Browse files
author
Shubh Sahu
committed
Making Recovery Chunk Size setting dynamic
Signed-off-by: Shubh Sahu <shubhvs@amazon.com>
1 parent 581fcd2 commit 3013bac

File tree

6 files changed

+25
-10
lines changed

6 files changed

+25
-10
lines changed

server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ private void restoreRecoverySpeed() {
278278
.setTransientSettings(
279279
Settings.builder()
280280
.put(RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey(), "20mb")
281-
.put(CHUNK_SIZE_SETTING.getKey(), RecoverySettings.DEFAULT_CHUNK_SIZE)
281+
.put(CHUNK_SIZE_SETTING.getKey(), RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getDefault(Settings.EMPTY))
282282
)
283283
.get()
284284
.isAcknowledged()

server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

+1
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ public void apply(Settings value, Settings current, Settings previous) {
306306
RecoverySettings.INDICES_RECOVERY_MAX_CONCURRENT_OPERATIONS_SETTING,
307307
RecoverySettings.INDICES_RECOVERY_MAX_CONCURRENT_REMOTE_STORE_STREAMS_SETTING,
308308
RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT,
309+
RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING,
309310
ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_PRIMARIES_RECOVERIES_SETTING,
310311
ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_REPLICAS_RECOVERIES_SETTING,
311312
ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING,

server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java

+12-7
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,14 @@ public class RecoverySettings {
177177
);
178178

179179
// choose 512KB-16B to ensure that the resulting byte[] is not a humongous allocation in G1.
180-
public static final ByteSizeValue DEFAULT_CHUNK_SIZE = new ByteSizeValue(512 * 1024 - 16, ByteSizeUnit.BYTES);
180+
public static final Setting<ByteSizeValue> INDICES_RECOVERY_CHUNK_SIZE_SETTING = Setting.byteSizeSetting(
181+
"indices.recovery.chunk_size",
182+
new ByteSizeValue(512 * 1024 - 16, ByteSizeUnit.BYTES),
183+
new ByteSizeValue(0, ByteSizeUnit.BYTES),
184+
new ByteSizeValue(100 * 1024 * 1024, ByteSizeUnit.BYTES),
185+
Property.Dynamic,
186+
Property.NodeScope
187+
);
181188

182189
private volatile ByteSizeValue recoveryMaxBytesPerSec;
183190
private volatile ByteSizeValue replicationMaxBytesPerSec;
@@ -193,7 +200,7 @@ public class RecoverySettings {
193200
private volatile TimeValue internalActionRetryTimeout;
194201
private volatile TimeValue internalActionLongTimeout;
195202

196-
private volatile ByteSizeValue chunkSize = DEFAULT_CHUNK_SIZE;
203+
private volatile ByteSizeValue chunkSize;
197204
private volatile TimeValue internalRemoteUploadTimeout;
198205

199206
public RecoverySettings(Settings settings, ClusterSettings clusterSettings) {
@@ -221,6 +228,7 @@ public RecoverySettings(Settings settings, ClusterSettings clusterSettings) {
221228

222229
logger.debug("using recovery max_bytes_per_sec[{}]", recoveryMaxBytesPerSec);
223230
this.internalRemoteUploadTimeout = INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.get(settings);
231+
this.chunkSize = INDICES_RECOVERY_CHUNK_SIZE_SETTING.get(settings);
224232

225233
clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING, this::setRecoveryMaxBytesPerSec);
226234
clusterSettings.addSettingsUpdateConsumer(INDICES_REPLICATION_MAX_BYTES_PER_SEC_SETTING, this::setReplicationMaxBytesPerSec);
@@ -239,7 +247,7 @@ public RecoverySettings(Settings settings, ClusterSettings clusterSettings) {
239247
);
240248
clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_ACTIVITY_TIMEOUT_SETTING, this::setActivityTimeout);
241249
clusterSettings.addSettingsUpdateConsumer(INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT, this::setInternalRemoteUploadTimeout);
242-
250+
clusterSettings.addSettingsUpdateConsumer(INDICES_RECOVERY_CHUNK_SIZE_SETTING, this::setChunkSize);
243251
}
244252

245253
public RateLimiter recoveryRateLimiter() {
@@ -282,10 +290,7 @@ public ByteSizeValue getChunkSize() {
282290
return chunkSize;
283291
}
284292

285-
public void setChunkSize(ByteSizeValue chunkSize) { // only settable for tests
286-
if (chunkSize.bytesAsInt() <= 0) {
287-
throw new IllegalArgumentException("chunkSize must be > 0");
288-
}
293+
public void setChunkSize(ByteSizeValue chunkSize) {
289294
this.chunkSize = chunkSize;
290295
}
291296

server/src/test/java/org/opensearch/indices/recovery/RecoverySettingsDynamicUpdateTests.java

+9
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,13 @@ public void testInternalLongActionTimeout() {
118118
);
119119
assertEquals(new TimeValue(duration, timeUnit), recoverySettings.internalActionLongTimeout());
120120
}
121+
122+
public void testChunkSize() {
123+
ByteSizeValue chunkSize = new ByteSizeValue(between(1, 1000), ByteSizeUnit.BYTES);
124+
clusterSettings.applySettings(
125+
Settings.builder().put(RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getKey(), chunkSize).build()
126+
);
127+
assertEquals(chunkSize, recoverySettings.getChunkSize());
128+
}
129+
121130
}

test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1148,7 +1148,7 @@ public final void recoverUnstartedReplica(
11481148
startingSeqNo
11491149
);
11501150
long fileChunkSizeInBytes = randomBoolean()
1151-
? RecoverySettings.DEFAULT_CHUNK_SIZE.getBytes()
1151+
? RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING.getDefault(Settings.EMPTY).getBytes()
11521152
: randomIntBetween(1, 10 * 1024 * 1024);
11531153
final Settings settings = Settings.builder()
11541154
.put("indices.recovery.max_concurrent_file_chunks", Integer.toString(between(1, 4)))

test/framework/src/main/java/org/opensearch/node/RecoverySettingsChunkSizePlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public class RecoverySettingsChunkSizePlugin extends Plugin {
5151
*/
5252
public static final Setting<ByteSizeValue> CHUNK_SIZE_SETTING = Setting.byteSizeSetting(
5353
"indices.recovery.chunk_size",
54-
RecoverySettings.DEFAULT_CHUNK_SIZE,
54+
RecoverySettings.INDICES_RECOVERY_CHUNK_SIZE_SETTING,
5555
Property.Dynamic,
5656
Property.NodeScope
5757
);

0 commit comments

Comments
 (0)