Fix delay allocation #95921
Fix delay allocation #95921
Conversation
idegtiarenko added >bug :Distributed Coordination/Allocation 
| Pinging @elastic/es-distributed (Team:Distributed) |
| Hi @idegtiarenko, I've created a changelog YAML for you. |
| * @param assignments a set of the (persistent) node IDs to which each {@link ShardId} should be allocated | ||
| */ | ||
| public record DesiredBalance(long lastConvergedIndex, Map<ShardId, ShardAssignment> assignments) { | ||
| public record DesiredBalance(long lastConvergedIndex, long currentTimeNano, Map<ShardId, ShardAssignment> assignments) { |
There was a problem hiding this comment.
Time is captured here
| @FunctionalInterface | ||
| public interface DesiredBalanceReconcilerAction { | ||
| ClusterState apply(ClusterState clusterState, Consumer<RoutingAllocation> routingAllocationAction); | ||
| ClusterState apply(ClusterState clusterState, long currentTimeNano, Consumer<RoutingAllocation> routingAllocationAction); |
There was a problem hiding this comment.
And passed to the reconciler via this interface. The rest are mostly changes to accommodate new parameter.
There was a problem hiding this comment.
I would rather we fixed this as I suggested in #93993 (comment): remove the delay markers only in a full reroute and not in a reconcile.
This reverts commit 50d1d59.
| Updated to use a flag. Though I am not sure if it is a good idea. May be we should have a dedicated reconcile method on allocation service? |
| if (routingAllocationAction.removeDelayMarkers()) { | ||
| removeDelayMarkers(allocation); | ||
| } |
There was a problem hiding this comment.
I think I'd rather have another method on RoutingAllocationAction which consumes the RoutingAllocation (and which by default removes the delay markers) instead of one which just returns a boolean.
| } | ||
| | ||
| @FunctionalInterface | ||
| public interface RoutingAllocationAction { |
There was a problem hiding this comment.
Naming nit/suggestion - "Action" is overloaded and this isn't really an action, it's more like a strategy. How about RerouteStrategy?
3 participants
As pointed in #93993 (comment) it is possible that desired balance is computed before delayed allocations are expired but reconciled after that. As a result delay markers are removed but shards are not allocated as their desired locations are not computed yet and follow up allocation is not scheduled as all markers might be expired and removed.
This change attempts to fix this bug by injecting time into reconciliation representing the time of the latest balance computed.
Close: #93993
Close: #93470