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.
This commit is contained in:
parent
3da91d5f7a
commit
a0d0866f59
|
@ -785,10 +785,7 @@ public class MetaDataCreateIndexService {
|
||||||
if (type == ResizeType.SHRINK) {
|
if (type == ResizeType.SHRINK) {
|
||||||
final List<String> nodesToAllocateOn = validateShrinkIndex(currentState, resizeSourceIndex.getName(),
|
final List<String> nodesToAllocateOn = validateShrinkIndex(currentState, resizeSourceIndex.getName(),
|
||||||
mappingKeys, resizeIntoName, indexSettingsBuilder.build());
|
mappingKeys, resizeIntoName, indexSettingsBuilder.build());
|
||||||
indexSettingsBuilder
|
indexSettingsBuilder.put(initialRecoveryIdFilter, Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray()));
|
||||||
.put(initialRecoveryIdFilter, Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray()))
|
|
||||||
// we only try once and then give up with a shrink index
|
|
||||||
.put("index.allocation.max_retries", 1);
|
|
||||||
} else if (type == ResizeType.SPLIT) {
|
} else if (type == ResizeType.SPLIT) {
|
||||||
validateSplitIndex(currentState, resizeSourceIndex.getName(), mappingKeys, resizeIntoName, indexSettingsBuilder.build());
|
validateSplitIndex(currentState, resizeSourceIndex.getName(), mappingKeys, resizeIntoName, indexSettingsBuilder.build());
|
||||||
indexSettingsBuilder.putNull(initialRecoveryIdFilter);
|
indexSettingsBuilder.putNull(initialRecoveryIdFilter);
|
||||||
|
|
|
@ -37,7 +37,7 @@ import org.elasticsearch.common.settings.Setting;
|
||||||
public class MaxRetryAllocationDecider extends AllocationDecider {
|
public class MaxRetryAllocationDecider extends AllocationDecider {
|
||||||
|
|
||||||
public static final Setting<Integer> SETTING_ALLOCATION_MAX_RETRY = Setting.intSetting("index.allocation.max_retries", 5, 0,
|
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";
|
public static final String NAME = "max_retry";
|
||||||
|
|
||||||
|
|
|
@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.containsString;
|
||||||
import static org.hamcrest.Matchers.endsWith;
|
import static org.hamcrest.Matchers.endsWith;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.hasToString;
|
import static org.hamcrest.Matchers.hasToString;
|
||||||
|
import static org.hamcrest.Matchers.nullValue;
|
||||||
|
|
||||||
public class MetaDataCreateIndexServiceTests extends ESTestCase {
|
public class MetaDataCreateIndexServiceTests extends ESTestCase {
|
||||||
|
|
||||||
|
@ -263,16 +264,18 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
|
||||||
versions.sort(Comparator.comparingLong(l -> l.id));
|
versions.sort(Comparator.comparingLong(l -> l.id));
|
||||||
final Version version = versions.get(0);
|
final Version version = versions.get(0);
|
||||||
final Version upgraded = versions.get(1);
|
final Version upgraded = versions.get(1);
|
||||||
final Settings indexSettings =
|
final Settings.Builder indexSettingsBuilder =
|
||||||
Settings.builder()
|
Settings.builder()
|
||||||
.put("index.version.created", version)
|
.put("index.version.created", version)
|
||||||
.put("index.version.upgraded", upgraded)
|
.put("index.version.upgraded", upgraded)
|
||||||
.put("index.similarity.default.type", "BM25")
|
.put("index.similarity.default.type", "BM25")
|
||||||
.put("index.analysis.analyzer.default.tokenizer", "keyword")
|
.put("index.analysis.analyzer.default.tokenizer", "keyword")
|
||||||
.put("index.soft_deletes.enabled", "true")
|
.put("index.soft_deletes.enabled", "true");
|
||||||
.build();
|
if (randomBoolean()) {
|
||||||
|
indexSettingsBuilder.put("index.allocation.max_retries", randomIntBetween(1, 1000));
|
||||||
|
}
|
||||||
runPrepareResizeIndexSettingsTest(
|
runPrepareResizeIndexSettingsTest(
|
||||||
indexSettings,
|
indexSettingsBuilder.build(),
|
||||||
Settings.EMPTY,
|
Settings.EMPTY,
|
||||||
Collections.emptyList(),
|
Collections.emptyList(),
|
||||||
randomBoolean(),
|
randomBoolean(),
|
||||||
|
@ -283,7 +286,7 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
|
||||||
settings.get("index.analysis.analyzer.default.tokenizer"),
|
settings.get("index.analysis.analyzer.default.tokenizer"),
|
||||||
equalTo("keyword"));
|
equalTo("keyword"));
|
||||||
assertThat(settings.get("index.routing.allocation.initial_recovery._id"), equalTo("node1"));
|
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.created", null), equalTo(version));
|
||||||
assertThat(settings.getAsVersion("index.version.upgraded", null), equalTo(upgraded));
|
assertThat(settings.getAsVersion("index.version.upgraded", null), equalTo(upgraded));
|
||||||
assertThat(settings.get("index.soft_deletes.enabled"), equalTo("true"));
|
assertThat(settings.get("index.soft_deletes.enabled"), equalTo("true"));
|
||||||
|
|
Loading…
Reference in New Issue