From 71b6b311522066324f97549f03055a4dc1e9464b Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 8 Oct 2019 10:35:15 +0200 Subject: [PATCH 1/2] Shrink should not touch max_retries 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. While there is no new node to allocate to and a retry will likely fail again, 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`. --- .../cluster/metadata/MetaDataCreateIndexService.java | 5 +---- .../cluster/metadata/MetaDataCreateIndexServiceTests.java | 3 ++- 2 files changed, 3 insertions(+), 5 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 d746977d8d539..e927e3ceaa4b1 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -766,10 +766,7 @@ static void prepareResizeIndexSettings( 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/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java index 53e7cb17facb1..97898f84236b5 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -68,6 +68,7 @@ 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 { @@ -267,7 +268,7 @@ public void testPrepareResizeIndexSettings() { 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")); From 8e826737b20895558cb7ca0e183dfff6bbf716e8 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 10 Oct 2019 12:40:13 +0200 Subject: [PATCH 2/2] Resize: no longer copy max_retries If max_retries was set on source, it is unlikely to be wanted on target too, instead the new index will rely on the default. --- .../allocation/decider/MaxRetryAllocationDecider.java | 2 +- .../metadata/MetaDataCreateIndexServiceTests.java | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) 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 708482feae7d4..e9619d4141ab4 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 @@ 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 97898f84236b5..436157ae79b54 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -248,16 +248,18 @@ public void testPrepareResizeIndexSettings() { 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(),