Skip to content

Prioritise recovery of system index shards #62640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pugnascotia merged 5 commits into from
Sep 22, 2020
Merged

Prioritise recovery of system index shards #62640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
445bf67
Prioritise recovery of system index shards
pugnascotia Sep 18, 2020
15d2b69
Checkstyle
pugnascotia Sep 18, 2020
67f7711
Merge branch 'master' into 61660-prioritize-recovery-of-system-indices
elasticmachine Sep 22, 2020
1906210
Test tweaks
pugnascotia Sep 22, 2020
e7f3082
Drop date part of index name in PriorityComparatorTests
pugnascotia Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions server/src/main/java/org/elasticsearch/gateway/PriorityComparator.java
View file
Open in desktop
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
Open in desktop
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();
}
}