Relocation targets are assigned shards too #43276
Conversation
Adds relocation targets to the output of `IndexShardRoutingTable#assignedShards`.
DaveCTurner
added
>non-issue
:Distributed Coordination/Allocation
|
Pinging @elastic/es-distributed |
ywelsch
left a comment
ywelsch
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have just one small ask.
| // trim entries that have no corresponding shard routing in the cluster state (i.e. trim unavailable copies) | ||
| List<ShardRouting> assignedShards = newShardRoutingTable.assignedShards(); | ||
| assert assignedShards.size() <= maxActiveShards : | ||
| assert assignedShards.stream().filter(s -> s.isRelocationTarget() == false).count() <= maxActiveShards : |
There was a problem hiding this comment.
This only filters the relocation targets from the assertion here, but leaves them in for the trimming logic later. As (initializing) relocation targets are never in the in-sync set (same as for initializing replicas), this should be fine, but I find it a bit dangerous to leave the code like this. I would prefer to have the sorting further down below adapted so that we prefer to keep in-sync IDs of active shards, and then those of regular initializing shards, and then those of initializing relocation targets.
Alternatively, filter out the relocation targets again from the assignedShards list here.
There was a problem hiding this comment.
Ok, I added an assertion of your statement, and moved the filter, in c49e275.
DaveCTurner
deleted the
2019-06-17-relocation-targets-are-assigned-shards
branch
Adds relocation targets to the output of `IndexShardRoutingTable#assignedShards`.
4 participants
Adds relocation targets to the output of
IndexShardRoutingTable#assignedShards.