Skip to content

Commit

Permalink
Prioritise recovery of system index shards (#62640)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@pugnascotia
pugnascotia committed Sep 22, 2020
1 parent c0e611e commit 3f856d1
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 70 deletions.
40 changes: 25 additions & 15 deletions server/src/main/java/org/elasticsearch/gateway/PriorityComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@
import java.util.Comparator;

/**
* A comparator that compares ShardRouting based on it's indexes priority (index.priority),
* it's creation date (index.creation_date), or eventually by it's index name in reverse order.
* We try to recover first shards from an index with the highest priority, if that's the same
* we try to compare the timestamp the index is created and pick the newer first (time-based indices,
* here the newer indices matter more). If even that is the same, we compare the index name which is useful
* if the date is baked into the index name. ie logstash-2015.05.03.
* A comparator that compares {@link ShardRouting} instances based on various properties. Instances
* are ordered as follows.
* <ol>
* <li>First, system indices are ordered before non-system indices</li>
* <li>Then indices are ordered by their priority, in descending order (index.priority)</li>
* <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> {

Expand All @@ -43,13 +47,20 @@ public final int compare(ShardRouting o1, ShardRouting o2) {
final String o2Index = o2.getIndexName();
int cmp = 0;
if (o1Index.equals(o2Index) == false) {
final Settings settingsO1 = getIndexSettings(o1.index());
final Settings settingsO2 = getIndexSettings(o2.index());
cmp = Long.compare(priority(settingsO2), priority(settingsO1));
final IndexMetadata metadata01 = getMetadata(o1.index());
final IndexMetadata metadata02 = getMetadata(o2.index());
cmp = Boolean.compare(metadata02.isSystem(), metadata01.isSystem());

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) {
cmp = o2Index.compareTo(o1Index);
cmp = Long.compare(timeCreated(settingsO2), timeCreated(settingsO1));
if (cmp == 0) {
cmp = o2Index.compareTo(o1Index);
}
}
}
}
Expand All @@ -64,17 +75,16 @@ private static long timeCreated(Settings settings) {
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.
*/
public static PriorityComparator getAllocationComparator(final RoutingAllocation allocation) {
return new PriorityComparator() {
@Override
protected Settings getIndexSettings(Index index) {
IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(index);
return indexMetadata.getSettings();
protected IndexMetadata getMetadata(Index index) {
return allocation.metadata().getIndexSafe(index);
}
};
}
Expand Down
176 changes: 121 additions & 55 deletions server/src/test/java/org/elasticsearch/gateway/PriorityComparatorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.gateway;

import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.routing.RoutingNodes;
import org.elasticsearch.cluster.routing.ShardRouting;
Expand All @@ -35,6 +36,7 @@
import java.util.Locale;
import java.util.Map;

import static org.hamcrest.Matchers.greaterThan;
import static org.mockito.Mockito.mock;

public class PriorityComparatorTests extends ESTestCase {
Expand All @@ -52,15 +54,17 @@ public void testPreferNewIndices() {
}
shards.sort(new PriorityComparator() {
@Override
protected Settings getIndexSettings(Index index) {
protected IndexMetadata getMetadata(Index index) {
Settings settings;
if ("oldest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 10)
.put(IndexMetadata.SETTING_PRIORITY, 1).build();
settings = buildSettings(10, 1);
} else if ("newest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 100)
.put(IndexMetadata.SETTING_PRIORITY, 1).build();
settings = buildSettings(100, 1);
} else {
settings = Settings.EMPTY;
}
return Settings.EMPTY;

return IndexMetadata.builder(index.getName()).settings(settings).build();
}
});
RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator();
Expand All @@ -84,15 +88,53 @@ public void testPreferPriorityIndices() {
}
shards.sort(new PriorityComparator() {
@Override
protected Settings getIndexSettings(Index index) {
protected IndexMetadata getMetadata(Index index) {
Settings settings;
if ("oldest".equals(index.getName())) {
settings = buildSettings(10, 100);
} else if ("newest".equals(index.getName())) {
settings = buildSettings(100, 1);
} else {
settings = 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())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 10)
.put(IndexMetadata.SETTING_PRIORITY, 100).build();
settings = buildSettings(10, 100);
isSystem = true;
} else if ("newest".equals(index.getName())) {
return Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 100)
.put(IndexMetadata.SETTING_PRIORITY, 1).build();
settings = buildSettings(100, 1);
} else {
settings = Settings.EMPTY;
}
return Settings.EMPTY;

return IndexMetadata.builder(index.getName()).system(isSystem).settings(settings).build();
}
});
RoutingNodes.UnassignedShards.UnassignedIterator iterator = shards.iterator();
Expand All @@ -106,75 +148,99 @@ protected Settings getIndexSettings(Index index) {
public void testPriorityComparatorSort() {
RoutingNodes.UnassignedShards shards = new RoutingNodes.UnassignedShards(mock(RoutingNodes.class));
int numIndices = randomIntBetween(3, 99);
IndexMeta[] indices = new IndexMeta[numIndices];
final Map<String, IndexMeta> map = new HashMap<>();
IndexMetadata[] indices = new IndexMetadata[numIndices];
final Map<String, IndexMetadata> map = new HashMap<>();

for (int i = 0; i < indices.length; i++) {
int priority = 0;
int creationDate = 0;
boolean isSystem = false;

if (frequently()) {
indices[i] = new IndexMeta("idx_2015_04_" + String.format(Locale.ROOT, "%02d", i), randomIntBetween(1, 1000),
randomIntBetween(1, 10000));
} else { // sometimes just use defaults
indices[i] = new IndexMeta("idx_2015_04_" + String.format(Locale.ROOT, "%02d", i));
priority = randomIntBetween(1, 1000);
creationDate = randomIntBetween(1, 10000);
}
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);
for (int i = 0; i < numShards; i++) {
IndexMeta indexMeta = randomFrom(indices);
shards.add(TestShardRouting.newShardRouting(indexMeta.name, randomIntBetween(1, 5), null, null,
IndexMetadata indexMeta = randomFrom(indices);
shards.add(TestShardRouting.newShardRouting(indexMeta.getIndex().getName(), randomIntBetween(1, 5), null, null,
randomBoolean(), ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()),
"foobar")));
}
shards.sort(new PriorityComparator() {
@Override
protected Settings getIndexSettings(Index index) {
IndexMeta indexMeta = map.get(index.getName());
return indexMeta.settings;
protected IndexMetadata getMetadata(Index index) {
return map.get(index.getName());
}
});
ShardRouting previous = null;
for (ShardRouting routing : shards) {
if (previous != null) {
IndexMeta prevMeta = map.get(previous.getIndexName());
IndexMeta currentMeta = map.get(routing.getIndexName());
if (prevMeta.priority == currentMeta.priority) {
if (prevMeta.creationDate == currentMeta.creationDate) {
if (prevMeta.name.equals(currentMeta.name) == false) {
assertTrue("indexName mismatch, expected:" + currentMeta.name + " after " + prevMeta.name + " " +
prevMeta.name.compareTo(currentMeta.name), prevMeta.name.compareTo(currentMeta.name) > 0);
IndexMetadata prevMeta = map.get(previous.getIndexName());
IndexMetadata currentMeta = map.get(routing.getIndexName());

if (prevMeta.isSystem() == currentMeta.isSystem()) {
final int prevPriority = prevMeta.getSettings().getAsInt(IndexMetadata.SETTING_PRIORITY, -1);
final int currentPriority = currentMeta.getSettings().getAsInt(IndexMetadata.SETTING_PRIORITY, -1);

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 {
assertTrue("creationDate mismatch, expected:" + currentMeta.creationDate + " after " + prevMeta.creationDate,
prevMeta.creationDate > currentMeta.creationDate);
assertThat(
"priority mismatch, expected:" + currentPriority + " after " + prevPriority,
prevPriority, greaterThan(currentPriority)
);
}
} else {
assertTrue("priority mismatch, expected:" + currentMeta.priority + " after " + prevMeta.priority,
prevMeta.priority > currentMeta.priority);
assertThat(
"system mismatch, expected:" + currentMeta.isSystem() + " after " + prevMeta.isSystem(),
prevMeta.isSystem(),
greaterThan(currentMeta.isSystem())
);
}
}
previous = routing;
}
}

private static class IndexMeta {
final String name;
final int priority;
final long creationDate;
final Settings settings;

private IndexMeta(String name) { // default
this.name = name;
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();
}
private static Settings buildSettings(int creationDate, int priority) {
return Settings.builder()
.put(IndexMetadata.SETTING_CREATION_DATE, creationDate)
.put(IndexMetadata.SETTING_PRIORITY, priority)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.build();
}
}

0 comments on commit 3f856d1

Please sign in to comment.