Primary shard allocator observes limits in forcing allocation

[abeyad]

Previously, during primary shards allocation of shards
with prior allocation IDs, if all nodes returned a
NO decision for allocation (e.g. the settings blocked
allocation on that node), we would chose one of those
nodes and force the primary shard to be allocated to it.

However, this meant that primary shard allocation
would not adhere to the decision of the MaxRetryAllocationDecider,
which would lead to attempting to allocate a shard
which has failed N number of times already (presumably
due to some configuration issue).

This commit solves this issue by introducing the
notion of force allocating a primary shard to a node
and each decider implementation must implement whether
this is allowed or not. In the case of MaxRetryAllocationDecider,
it just forwards the request to canAllocate.

Closes #19446

[abeyad]

@ywelsch FYI, initial pass on this, would welcome your feedback.

[ywelsch]

this means that throttling is just ignored.
Assume scenario where FilterAllocationDecider says NO and ThrottlingAllocationDecider says THROTTLE. The implementation here would just forcefully allocate the shard, ignoring the throttling.

[abeyad]

@ywelsch I pushed 397e374 to address your feedback

[ywelsch]

"This method should only be called on nodes for which previous allocations exist for the primary shard." should be more like "This method should only be called for unassigned primary shards where the node has a shard copy on disk."
Can you also assert shardRouting.unassigned() ?

[abeyad]

Done

[ywelsch]

@abeyad I've left some comments. Can you also check if there are tests that FilterAllocationDecider and other deciders are indeed allowing force-allocating primaries?

I would also like to see some docs where we can explain the implemented behavior. This could be useful to understand why primary shards are / are not allocated.

[abeyad]

@ywelsch I pushed 9e47cd1, which addresses your review comments, and adds PrimaryAllocationIT test that covers filter allocation decider force allocating primaries.

As we discussed, I augmented the PrimaryShardAllocator with some more javadocs to explain the behavior. Docs explaining shard allocation in general (including primary shard allocation) will come in a separate PR.

[ywelsch]

I think I prefer the way it was before with the default being

decision = canAllocate(...)
if (decision == Decision.NO) {
    decision = Decision.single(Type.YES, "force override of " + decision.label, ...)
}

This keeps the override logic out of AllocationDeciders.

[abeyad]

My concern with your suggestion (which was the initial approach) is that for any decider that overrides canForceAllocatePrimary, it is easy for it to forget to take into account the decision of canAllocate. That's why I figured it would be better to put the invocation of canAllocate in AllocationDeciders, then if the decision is NO (as opposed to throttle, for example), then call the deciders canForceAllocatePrimary implementation. The only downside to this that I can think of is that its rigid in that no canForceAllocatePrimary method would be able to override a non-NO canAllocate decision - but I don't see any reason why we would want that?

[abeyad]

I pushed 5c25b15 to address this

[abeyad]

@ywelsch I pushed 9446267 to remove ForcePrimaryDecider in favor of anonymous class creation that extends TestAllocateDecision

[abeyad]

@elasticmachine retest this please

[ywelsch]

print shardRouting, not only shardRouting.shardId().

[ywelsch]

@abeyad Left minor comments about docs/tests, change looks good o.w.

[abeyad]

@ywelsch I pushed 5b536be

[ywelsch]

LGTM. Thanks @abeyad!

[abeyad]

Thanks for the review @ywelsch !