From d0f1a756d995cfe49ce61bf1dfcc198dd5d2af35 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 10 Jul 2019 13:23:40 +0100 Subject: [PATCH] Comment on the extra reroute after failing shards (#44152) The `ShardFailedClusterStateTaskExecutor` fails some shards, which performs a reroute, but then sometimes schedules a followup reroute. It's not clear from the code why this followup is necessary, so this commit adds a short comment describing why it's necessary. --- .../cluster/action/shard/ShardStateAction.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/action/shard/ShardStateAction.java b/server/src/main/java/org/elasticsearch/cluster/action/shard/ShardStateAction.java index d73527a27c4..b3354e30c2f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/action/shard/ShardStateAction.java +++ b/server/src/main/java/org/elasticsearch/cluster/action/shard/ShardStateAction.java @@ -378,10 +378,11 @@ public class ShardStateAction { public void clusterStatePublished(ClusterChangedEvent clusterChangedEvent) { int numberOfUnassignedShards = clusterChangedEvent.state().getRoutingNodes().unassigned().size(); if (numberOfUnassignedShards > 0) { - String reason = String.format(Locale.ROOT, "[%d] unassigned shards after failing shards", numberOfUnassignedShards); - if (logger.isTraceEnabled()) { - logger.trace("{}, scheduling a reroute", reason); - } + // The reroute called after failing some shards will not assign any shard back to the node on which it failed. If there were + // no other options for a failed shard then it is left unassigned. However, absent other options it's better to try and + // assign it again, even if that means putting it back on the node on which it previously failed: + final String reason = String.format(Locale.ROOT, "[%d] unassigned shards after failing shards", numberOfUnassignedShards); + logger.trace("{}, scheduling a reroute", reason); rerouteService.reroute(reason, ActionListener.wrap( r -> logger.trace("{}, reroute completed", reason), e -> logger.debug(new ParameterizedMessage("{}, reroute failed", reason), e)));