Skip to content

Commit

Permalink
Shrink should not touch max_retries (elastic#47719)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
henningandersen committed Oct 11, 2019
1 parent 3da91d5 commit a0d0866
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -785,10 +785,7 @@ static void prepareResizeIndexSettings(
if (type == ResizeType.SHRINK) {
final List<String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
public class MaxRetryAllocationDecider extends AllocationDecider {

public static final Setting<Integer> 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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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 {

Expand Down Expand Up @@ -263,16 +264,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(),
Expand All @@ -283,7 +286,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"));
Expand Down

0 comments on commit a0d0866

Please sign in to comment.