Avoid extra reroutes of delayed shards in RoutingService #12532
Conversation
|
@kimchy can you take a look? |
| @@ -57,6 +57,7 @@ | |||
| private AtomicBoolean rerouting = new AtomicBoolean(); | |||
| private volatile long registeredNextDelaySetting = Long.MAX_VALUE; | |||
| private volatile ScheduledFuture registeredNextDelayFuture; | |||
| private volatile long lastAllocateUnassignedShardsRun = System.currentTimeMillis(); | |||
There was a problem hiding this comment.
Would it be useful to have 0 have the special "I've not even tried yet" meaning? I think setting it to now is kind-of funky because you aren't actually doing an allocation when you build the object.
There was a problem hiding this comment.
I could set this to 0, but either way, gateway allocator should always run at least once before these does. So this is just because I wanted a non-null value.
| @@ -167,12 +167,12 @@ public long getAllocationDelayTimeoutSetting(Settings settings, Settings indexSe | |||
| /** | |||
| * The time in millisecond until this unassigned shard can be reassigned. | |||
| */ | |||
| public long getDelayAllocationExpirationIn(Settings settings, Settings indexSettings) { | |||
| public long getDelayAllocationExpirationIn(long lastUnassignedShardsAllocated, Settings settings, Settings indexSettings) { | |||
There was a problem hiding this comment.
can we call this timestamp? I think the "lastXXX" part depends on the context of calling it. If we do, would be good to rename the other ones in this change.
There was a problem hiding this comment.
Sure, I'll rename to unassignedShardsAllocatedTimestamp
|
left a few comments, LGTM otherwise. For master forward patching, the code changed a bit, so it will be slightly different. I think we should in GatewayAllocator take a timestamp, and pass it to |
In order to avoid extra reroutes, `RoutingService` should avoid scheduling a reroute of any shards where the delay is negative. To make sure that we don't encounter a race condition between the GatewayAllocator thinking a shard is delayed and RoutingService thinking it is not, the GatewayAllocator will update the RoutingService with the last time it checked in order to use a consistent "view" of the delay. Resolves elastic#12456 Relates to elastic#12515 and elastic#12456
dakrone
force-pushed
the
avoid-extra-reroutes
branch
from
7a61318
to
93ac27a
Compare
|
Merged this, but for the 2.0 forward port I am going to open another PR. |
lcawl
added
:Distributed/Distributed
clintongormley
added
:Distributed/Allocation
In order to avoid extra reroutes,
RoutingServiceshould avoidscheduling a reroute of any shards where the delay is negative. To make
sure that we don't encounter a race condition between the
GatewayAllocator thinking a shard is delayed and RoutingService thinking
it is not, the GatewayAllocator will update the RoutingService with the
last time it checked in order to use a consistent "view" of the delay.
Resolves #12456
Relates to #12515 and #12456
This is a PR against 1.7 and will be forward-ported