From a0d0866f5965912ee77bc95812429abad624b11f Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Fri, 11 Oct 2019 14:22:04 +0200 Subject: [PATCH] Shrink should not touch max_retries (#47719) Shrink would set `max_retries=1` in order to avoid retrying. This however sticks to the shrunk index afterwards, causing issues when a shard copy later fails to allocate just once. Avoiding a retry of a shrink makes sense since there is no new node to allocate to and a retry will likely fail again. However, the downside of having max_retries=1 afterwards outweigh the benefit of not retrying the failed shrink a few times. This change ensures shrink no longer sets max_retries and also makes all resize operations (shrink, clone, split) leave the setting at default value rather than copy it from source. --- .../metadata/MetaDataCreateIndexService.java | 5 +---- .../decider/MaxRetryAllocationDecider.java | 2 +- .../metadata/MetaDataCreateIndexServiceTests.java | 13 ++++++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 142837fe5fc..c914b9a95f8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -785,10 +785,7 @@ public class MetaDataCreateIndexService { if (type == ResizeType.SHRINK) { final List nodesToAllocateOn = validateShrinkIndex(currentState, resizeSourceIndex.getName(), mappingKeys, resizeIntoName, indexSettingsBuilder.build()); - indexSettingsBuilder - .put(initialRecoveryIdFilter, Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray())) - // we only try once and then give up with a shrink index - .put("index.allocation.max_retries", 1); + indexSettingsBuilder.put(initialRecoveryIdFilter, Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray())); } else if (type == ResizeType.SPLIT) { validateSplitIndex(currentState, resizeSourceIndex.getName(), mappingKeys, resizeIntoName, indexSettingsBuilder.build()); indexSettingsBuilder.putNull(initialRecoveryIdFilter); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java index 708482feae7..e9619d4141a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java @@ -37,7 +37,7 @@ import org.elasticsearch.common.settings.Setting; public class MaxRetryAllocationDecider extends AllocationDecider { public static final Setting SETTING_ALLOCATION_MAX_RETRY = Setting.intSetting("index.allocation.max_retries", 5, 0, - Setting.Property.Dynamic, Setting.Property.IndexScope); + Setting.Property.Dynamic, Setting.Property.IndexScope, Setting.Property.NotCopyableOnResize); public static final String NAME = "max_retry"; diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java index e5bcac3f41f..9880a2ca784 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasToString; +import static org.hamcrest.Matchers.nullValue; public class MetaDataCreateIndexServiceTests extends ESTestCase { @@ -263,16 +264,18 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase { versions.sort(Comparator.comparingLong(l -> l.id)); final Version version = versions.get(0); final Version upgraded = versions.get(1); - final Settings indexSettings = + final Settings.Builder indexSettingsBuilder = Settings.builder() .put("index.version.created", version) .put("index.version.upgraded", upgraded) .put("index.similarity.default.type", "BM25") .put("index.analysis.analyzer.default.tokenizer", "keyword") - .put("index.soft_deletes.enabled", "true") - .build(); + .put("index.soft_deletes.enabled", "true"); + if (randomBoolean()) { + indexSettingsBuilder.put("index.allocation.max_retries", randomIntBetween(1, 1000)); + } runPrepareResizeIndexSettingsTest( - indexSettings, + indexSettingsBuilder.build(), Settings.EMPTY, Collections.emptyList(), randomBoolean(), @@ -283,7 +286,7 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase { settings.get("index.analysis.analyzer.default.tokenizer"), equalTo("keyword")); assertThat(settings.get("index.routing.allocation.initial_recovery._id"), equalTo("node1")); - assertThat(settings.get("index.allocation.max_retries"), equalTo("1")); + assertThat(settings.get("index.allocation.max_retries"), nullValue()); assertThat(settings.getAsVersion("index.version.created", null), equalTo(version)); assertThat(settings.getAsVersion("index.version.upgraded", null), equalTo(upgraded)); assertThat(settings.get("index.soft_deletes.enabled"), equalTo("true"));