Prioritise recovery of system index shards (#62640)

Closes #61660. When ordering shard for recovery, ensure system index shards are
ordered first so that their recovery will be started first.

Note that I rewrote PriorityComparatorTests to use IndexMetadata instead of its
local IndexMeta POJO.
This commit is contained in:
Rory Hunter 2020-09-22 15:47:41 +01:00 committed by Rory Hunter
parent c0e611e0a7
commit 3f856d1c81
2 changed files with 146 additions and 70 deletions

View File

@ -28,12 +28,16 @@ import org.elasticsearch.index.Index;
import java.util.Comparator; import java.util.Comparator;
/** /**
* A comparator that compares ShardRouting based on it's indexes priority (index.priority), * A comparator that compares {@link ShardRouting} instances based on various properties. Instances
* it's creation date (index.creation_date), or eventually by it's index name in reverse order. * are ordered as follows.
* We try to recover first shards from an index with the highest priority, if that's the same * <ol>
* we try to compare the timestamp the index is created and pick the newer first (time-based indices, * <li>First, system indices are ordered before non-system indices</li>
* here the newer indices matter more). If even that is the same, we compare the index name which is useful * <li>Then indices are ordered by their priority, in descending order (index.priority)</li>
* if the date is baked into the index name. ie logstash-2015.05.03. * <li>Then newer indices are ordered before older indices, based on their creation date. This benefits
* time-series indices, where newer indices are considered more urgent (index.creation_date)</li>
* <li>Lastly the index names are compared, which is useful when a date is baked into the index
* name, e.g. <code>logstash-2015.05.03</code></li>
* </ol>
*/ */
public abstract class PriorityComparator implements Comparator<ShardRouting> { public abstract class PriorityComparator implements Comparator<ShardRouting> {
@ -43,13 +47,20 @@ public abstract class PriorityComparator implements Comparator<ShardRouting> {
final String o2Index = o2.getIndexName(); final String o2Index = o2.getIndexName();
int cmp = 0; int cmp = 0;
if (o1Index.equals(o2Index) == false) { if (o1Index.equals(o2Index) == false) {
final Settings settingsO1 = getIndexSettings(o1.index()); final IndexMetadata metadata01 = getMetadata(o1.index());
final Settings settingsO2 = getIndexSettings(o2.index()); final IndexMetadata metadata02 = getMetadata(o2.index());
cmp = Long.compare(priority(settingsO2), priority(settingsO1)); cmp = Boolean.compare(metadata02.isSystem(), metadata01.isSystem());
if (cmp == 0) { if (cmp == 0) {
cmp = Long.compare(timeCreated(settingsO2), timeCreated(settingsO1)); final Settings settingsO1 = metadata01.getSettings();
final Settings settingsO2 = metadata02.getSettings();
cmp = Long.compare(priority(settingsO2), priority(settingsO1));
if (cmp == 0) { if (cmp == 0) {
cmp = o2Index.compareTo(o1Index); cmp = Long.compare(timeCreated(settingsO2), timeCreated(settingsO1));
if (cmp == 0) {
cmp = o2Index.compareTo(o1Index);
}
} }
} }
} }
@ -64,7 +75,7 @@ public abstract class PriorityComparator implements Comparator<ShardRouting> {
return settings.getAsLong(IndexMetadata.SETTING_CREATION_DATE, -1L); return settings.getAsLong(IndexMetadata.SETTING_CREATION_DATE, -1L);
} }
protected abstract Settings getIndexSettings(Index index); protected abstract IndexMetadata getMetadata(Index index);
/** /**
* Returns a PriorityComparator that uses the RoutingAllocation index metadata to access the index setting per index. * Returns a PriorityComparator that uses the RoutingAllocation index metadata to access the index setting per index.
@ -72,9 +83,8 @@ public abstract class PriorityComparator implements Comparator<ShardRouting> {
public static PriorityComparator getAllocationComparator(final RoutingAllocation allocation) { public static PriorityComparator getAllocationComparator(final RoutingAllocation allocation) {
return new PriorityComparator() { return new PriorityComparator() {
@Override @Override
protected Settings getIndexSettings(Index index) { protected IndexMetadata getMetadata(Index index) {
IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(index); return allocation.metadata().getIndexSafe(index);
return indexMetadata.getSettings();
} }
}; };
} }

View File

@ -18,6 +18,7 @@
*/ */
package org.elasticsearch.gateway; package org.elasticsearch.gateway;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.routing.RoutingNodes; import org.elasticsearch.cluster.routing.RoutingNodes;
import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRouting;
@ -35,6 +36,7 @@ import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import static org.hamcrest.Matchers.greaterThan;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
public class PriorityComparatorTests extends ESTestCase { public class PriorityComparatorTests extends ESTestCase {
@ -52,15 +54,17 @@ public class PriorityComparatorTests extends ESTestCase {
} }
shards.sort(new PriorityComparator() { shards.sort(new PriorityComparator() {
@Override @Override
protected Settings getIndexSettings(Index index) { protected IndexMetadata getMetadata(Index index) {
Settings settings;
if ("oldest".equals(index.getName())) { if ("oldest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 10) settings = buildSettings(10, 1);
.put(IndexMetadata.SETTING_PRIORITY, 1).build();
} else if ("newest".equals(index.getName())) { } else if ("newest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 100) settings = buildSettings(100, 1);
.put(IndexMetadata.SETTING_PRIORITY, 1).build(); } else {
settings = Settings.EMPTY;
} }
return Settings.EMPTY;
return IndexMetadata.builder(index.getName()).settings(settings).build();
} }
}); });
RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator(); RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator();
@ -84,15 +88,53 @@ public class PriorityComparatorTests extends ESTestCase {
} }
shards.sort(new PriorityComparator() { shards.sort(new PriorityComparator() {
@Override @Override
protected Settings getIndexSettings(Index index) { protected IndexMetadata getMetadata(Index index) {
Settings settings;
if ("oldest".equals(index.getName())) { if ("oldest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 10) settings = buildSettings(10, 100);
.put(IndexMetadata.SETTING_PRIORITY, 100).build();
} else if ("newest".equals(index.getName())) { } else if ("newest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 100) settings = buildSettings(100, 1);
.put(IndexMetadata.SETTING_PRIORITY, 1).build(); } else {
settings = Settings.EMPTY;
} }
return Settings.EMPTY;
return IndexMetadata.builder(index.getName()).settings(settings).build();
}
});
RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator();
ShardRouting next = iterator.next();
assertEquals("oldest", next.getIndexName());
next = iterator.next();
assertEquals("newest", next.getIndexName());
assertFalse(iterator.hasNext());
}
public void testPreferSystemIndices() {
RoutingNodes.UnassignedShards shards = new RoutingNodes.UnassignedShards(mock(RoutingNodes.class));
List<ShardRouting> shardRoutings = Arrays.asList(
TestShardRouting.newShardRouting("oldest", 0, null, null,
randomBoolean(), ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()), "foobar")),
TestShardRouting.newShardRouting("newest", 0, null, null,
randomBoolean(), ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()), "foobar")));
Collections.shuffle(shardRoutings, random());
for (ShardRouting routing : shardRoutings) {
shards.add(routing);
}
shards.sort(new PriorityComparator() {
@Override
protected IndexMetadata getMetadata(Index index) {
Settings settings;
boolean isSystem = false;
if ("oldest".equals(index.getName())) {
settings = buildSettings(10, 100);
isSystem = true;
} else if ("newest".equals(index.getName())) {
settings = buildSettings(100, 1);
} else {
settings = Settings.EMPTY;
}
return IndexMetadata.builder(index.getName()).system(isSystem).settings(settings).build();
} }
}); });
RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator(); RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator();
@ -106,75 +148,99 @@ public class PriorityComparatorTests extends ESTestCase {
public void testPriorityComparatorSort() { public void testPriorityComparatorSort() {
RoutingNodes.UnassignedShards shards = new RoutingNodes.UnassignedShards(mock(RoutingNodes.class)); RoutingNodes.UnassignedShards shards = new RoutingNodes.UnassignedShards(mock(RoutingNodes.class));
int numIndices = randomIntBetween(3, 99); int numIndices = randomIntBetween(3, 99);
IndexMeta[] indices = new IndexMeta[numIndices]; IndexMetadata[] indices = new IndexMetadata[numIndices];
final Map<String, IndexMeta> map = new HashMap<>(); final Map<String, IndexMetadata> map = new HashMap<>();
for (int i = 0; i < indices.length; i++) { for (int i = 0; i < indices.length; i++) {
int priority = 0;
int creationDate = 0;
boolean isSystem = false;
if (frequently()) { if (frequently()) {
indices[i] = new IndexMeta("idx_2015_04_" + String.format(Locale.ROOT, "%02d", i), randomIntBetween(1, 1000), priority = randomIntBetween(1, 1000);
randomIntBetween(1, 10000)); creationDate = randomIntBetween(1, 10000);
} else { // sometimes just use defaults
indices[i] = new IndexMeta("idx_2015_04_" + String.format(Locale.ROOT, "%02d", i));
} }
map.put(indices[i].name, indices[i]); if (rarely()) {
isSystem = true;
}
// else sometimes just use the defaults
indices[i] = IndexMetadata.builder(String.format(Locale.ROOT, "idx_%04d", i))
.system(isSystem)
.settings(buildSettings(creationDate, priority))
.build();
map.put(indices[i].getIndex().getName(), indices[i]);
} }
int numShards = randomIntBetween(10, 100); int numShards = randomIntBetween(10, 100);
for (int i = 0; i < numShards; i++) { for (int i = 0; i < numShards; i++) {
IndexMeta indexMeta = randomFrom(indices); IndexMetadata indexMeta = randomFrom(indices);
shards.add(TestShardRouting.newShardRouting(indexMeta.name, randomIntBetween(1, 5), null, null, shards.add(TestShardRouting.newShardRouting(indexMeta.getIndex().getName(), randomIntBetween(1, 5), null, null,
randomBoolean(), ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()), randomBoolean(), ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()),
"foobar"))); "foobar")));
} }
shards.sort(new PriorityComparator() { shards.sort(new PriorityComparator() {
@Override @Override
protected Settings getIndexSettings(Index index) { protected IndexMetadata getMetadata(Index index) {
IndexMeta indexMeta = map.get(index.getName()); return map.get(index.getName());
return indexMeta.settings;
} }
}); });
ShardRouting previous = null; ShardRouting previous = null;
for (ShardRouting routing : shards) { for (ShardRouting routing : shards) {
if (previous != null) { if (previous != null) {
IndexMeta prevMeta = map.get(previous.getIndexName()); IndexMetadata prevMeta = map.get(previous.getIndexName());
IndexMeta currentMeta = map.get(routing.getIndexName()); IndexMetadata currentMeta = map.get(routing.getIndexName());
if (prevMeta.priority == currentMeta.priority) {
if (prevMeta.creationDate == currentMeta.creationDate) { if (prevMeta.isSystem() == currentMeta.isSystem()) {
if (prevMeta.name.equals(currentMeta.name) == false) { final int prevPriority = prevMeta.getSettings().getAsInt(IndexMetadata.SETTING_PRIORITY, -1);
assertTrue("indexName mismatch, expected:" + currentMeta.name + " after " + prevMeta.name + " " + final int currentPriority = currentMeta.getSettings().getAsInt(IndexMetadata.SETTING_PRIORITY, -1);
prevMeta.name.compareTo(currentMeta.name), prevMeta.name.compareTo(currentMeta.name) > 0);
if (prevPriority == currentPriority) {
final int prevCreationDate = prevMeta.getSettings().getAsInt(IndexMetadata.SETTING_CREATION_DATE, -1);
final int currentCreationDate = currentMeta.getSettings().getAsInt(IndexMetadata.SETTING_CREATION_DATE, -1);
if (prevCreationDate == currentCreationDate) {
final String prevName = prevMeta.getIndex().getName();
final String currentName = currentMeta.getIndex().getName();
if (prevName.equals(currentName) == false) {
assertThat(
"indexName mismatch, expected:" + currentName + " after " + prevName,
prevName,
greaterThan(currentName)
);
}
} else {
assertThat(
"creationDate mismatch, expected:" + currentCreationDate + " after " + prevCreationDate,
prevCreationDate, greaterThan(currentCreationDate)
);
} }
} else { } else {
assertTrue("creationDate mismatch, expected:" + currentMeta.creationDate + " after " + prevMeta.creationDate, assertThat(
prevMeta.creationDate > currentMeta.creationDate); "priority mismatch, expected:" + currentPriority + " after " + prevPriority,
prevPriority, greaterThan(currentPriority)
);
} }
} else { } else {
assertTrue("priority mismatch, expected:" + currentMeta.priority + " after " + prevMeta.priority, assertThat(
prevMeta.priority > currentMeta.priority); "system mismatch, expected:" + currentMeta.isSystem() + " after " + prevMeta.isSystem(),
prevMeta.isSystem(),
greaterThan(currentMeta.isSystem())
);
} }
} }
previous = routing; previous = routing;
} }
} }
private static class IndexMeta { private static Settings buildSettings(int creationDate, int priority) {
final String name; return Settings.builder()
final int priority; .put(IndexMetadata.SETTING_CREATION_DATE, creationDate)
final long creationDate; .put(IndexMetadata.SETTING_PRIORITY, priority)
final Settings settings; .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
private IndexMeta(String name) { // default .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
this.name = name; .build();
this.priority = 1;
this.creationDate = -1;
this.settings = Settings.EMPTY;
}
private IndexMeta(String name, int priority, long creationDate) {
this.name = name;
this.priority = priority;
this.creationDate = creationDate;
this.settings = Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, creationDate)
.put(IndexMetadata.SETTING_PRIORITY, priority).build();
}
} }
} }