Prioritise recovery of system index shards#62640
Conversation
Closes elastic#61660. When ordering shard for recovery, ensure system index shards are ordered first.
|
Pinging @elastic/es-distributed (:Distributed/Recovery) |
jaymode
left a comment
There was a problem hiding this comment.
LGTM, but let's also get someone from the distributed team to review
|
@elasticmachine update branch |
tlrx
left a comment
There was a problem hiding this comment.
LGTM - I left some minor comments, feel free to use them or not
| } | ||
| // else sometimes just use the defaults | ||
|
|
||
| indices[i] = IndexMetadata.builder("idx_2015_04_" + String.format(Locale.ROOT, "%02d", i)) |
There was a problem hiding this comment.
nit: since i can span to 99 using a prefix like "idx_" makes more sense
There was a problem hiding this comment.
Are you suggesting just "idx_" + i?
I'm not sure what the best thing to do here is. The comparator class just does a compareTo on the index names, so in a sense it doesn't matter what index names we use, so long as the result is sorted the way we expect. However, the intention of the sort is that newer indices, according to dates in the index names, are sorted first. So I wonder if it's worth preserving a date-like pattern in the index names for that reason? What do you think?
There was a problem hiding this comment.
I think that we should test what the code is effectively doing, ie alphabetically sorting by index names. Applying this to index names that contain a date is sort of a workaround as most of the time SETTING_CREATION_DATE should be present. Prioritizing index-0002 over index-0001 is also a thing.
There was a problem hiding this comment.
I swapped it for just "idx_%04d". Any better?
server/src/test/java/org/elasticsearch/gateway/PriorityComparatorTests.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine run elasticsearch-ci/1 |
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.
|
Backported to |
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
PriorityComparatorTeststo useIndexMetadatainstead of its localIndexMetaPOJO.