Allocate primary shards based on allocation IDs #15281
Conversation
|
|
||
| NodesAndVersions nodesAndVersions = buildNodesAndVersions(shard, recoverOnAnyNode(indexSettings), allocation.getIgnoreNodes(shard.shardId()), shardState); | ||
| logger.debug("[{}][{}] found {} allocations of {}, highest version: [{}]", shard.index(), shard.id(), nodesAndVersions.allocationsFound, shard, nodesAndVersions.highestVersion); | ||
| Set<String> allocationIds = indexMetaData.activeAllocationIds(shard.id()); |
There was a problem hiding this comment.
call this lastActiveAllocationIds?
|
Pushed changes related to our discussion:
|
| @@ -115,9 +116,10 @@ private void writeTo(StreamOutput out) throws IOException { | |||
| private StoreStatus() { | |||
| } | |||
|
|
|||
| public StoreStatus(DiscoveryNode node, long version, Allocation allocation, Throwable storeException) { | |||
| public StoreStatus(DiscoveryNode node, long version, String allocationId, Allocation allocation, Throwable storeException) { | |||
There was a problem hiding this comment.
That Allocation enum is really confusing in this context (I stumbled on this 3 times now). This is unrelated to this change, so I'm not suggesting we change it here - but do you have suggestions for a better name? maybe AllocatedAs ?
There was a problem hiding this comment.
What about AllocationStatus? The JSON field should also be adapted.
There was a problem hiding this comment.
I liked allocatedAs better :) (note that the name of the java JSON field is ALLOCATED :))
|
@bleskes I pushed a new set of changes that address your comments. |
| continue; | ||
| } | ||
|
|
||
| if (recoverOnAnyNode(indexSettings)) { |
There was a problem hiding this comment.
can we only skip allocation if we don't find any copy?
|
This looks great. Left some minor comment and one important one about the recover on any node settings. Also let's have another discussion with @dakrone about the failing test. |
|
Pushed another set of changes, dealing with recover_on_any_node. |
| // Note that once the shard has been active, lastActiveAllocationIds will be non-empty | ||
| nodesAndVersions = buildNodesAndVersions(shard, snapshotRestore || recoverOnAnyNode, allocation.getIgnoreNodes(shard.shardId()), shardState); | ||
| if (snapshotRestore || recoverOnAnyNode) { | ||
| enoughAllocationsFound = nodesAndVersions.nodes.isEmpty() == false; |
There was a problem hiding this comment.
nit: can we be consistent and use allocationsFound > 0?
|
LGTM. Left some extremely minor comments. No need for another review. Just merge after they are addressed. Thanks @ywelsch ! I can't tell you how happy I am for having this. |
ywelsch
force-pushed
the
feature/alloc-ids-primary
branch
from
dc3479a
to
ae5e9e3
Compare
ywelsch
force-pushed
the
feature/alloc-ids-primary
branch
from
ae5e9e3
to
3a442db
Compare
Allocate primary shards based on allocation IDs
lcawl
added
:Distributed/Distributed
clintongormley
added
:Distributed/Allocation
TransportNodesListGatewayStartedShardsaction./some_index/_shard_stores)Relates to #14739