From 8d321530de539bfa3e09d10e4fd15d3ab05b0c05 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 16 Dec 2013 11:20:07 +0100 Subject: [PATCH] Reset source shards to `started` if canceling relocation. Currently we miss to reset the source shards status to ACTIVE if we cancel a relocation. If the shard is RELOCATING we need to reset to state ACTIVE. Closes #4457 --- .../command/CancelAllocationCommand.java | 4 +--- .../allocation/AllocationCommandsTests.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/elasticsearch/cluster/routing/allocation/command/CancelAllocationCommand.java b/src/main/java/org/elasticsearch/cluster/routing/allocation/command/CancelAllocationCommand.java index 54ddbbbb455..abefc4ce206 100644 --- a/src/main/java/org/elasticsearch/cluster/routing/allocation/command/CancelAllocationCommand.java +++ b/src/main/java/org/elasticsearch/cluster/routing/allocation/command/CancelAllocationCommand.java @@ -176,9 +176,7 @@ public class CancelAllocationCommand implements AllocationCommand { RoutingNode relocatingFromNode = allocation.routingNodes().node(shardRouting.relocatingNodeId()); if (relocatingFromNode != null) { for (MutableShardRouting fromShardRouting : relocatingFromNode) { - assert shardRouting.state() != RELOCATING; - if (fromShardRouting.shardId().equals(shardRouting.shardId()) && shardRouting.state() == RELOCATING) { - // NOCOMMIT @Shay the check on the shardRouting.state() == RELOCATING can never be true right this should be fromShardRouting? + if (fromShardRouting.shardId().equals(shardRouting.shardId()) && fromShardRouting.state() == RELOCATING) { allocation.routingNodes().cancelRelocation(fromShardRouting); break; } diff --git a/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java b/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java index 76a66055949..bb8b0da5d15 100644 --- a/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java +++ b/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Test; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; +import static org.elasticsearch.cluster.routing.ShardRoutingState.RELOCATING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; import static org.elasticsearch.cluster.routing.allocation.RoutingAllocationTests.newNode; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; @@ -301,6 +302,25 @@ public class AllocationCommandsTests extends ElasticsearchTestCase { assertThat(clusterState.routingNodes().node("node2").size(), equalTo(1)); assertThat(clusterState.routingNodes().node("node2").shardsWithState(STARTED).size(), equalTo(1)); + logger.info("--> move the replica shard"); + rerouteResult = allocation.reroute(clusterState, new AllocationCommands(new MoveAllocationCommand(new ShardId("test", 0), "node2", "node3"))); + clusterState = ClusterState.builder(clusterState).routingTable(rerouteResult.routingTable()).build(); + assertThat(clusterState.routingNodes().node("node1").size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node1").shardsWithState(STARTED).size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node2").size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node2").shardsWithState(RELOCATING).size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node3").size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node3").shardsWithState(INITIALIZING).size(), equalTo(1)); + + logger.info("--> cancel the move of the replica shard"); + rerouteResult = allocation.reroute(clusterState, new AllocationCommands(new CancelAllocationCommand(new ShardId("test", 0), "node3", false))); + clusterState = ClusterState.builder(clusterState).routingTable(rerouteResult.routingTable()).build(); + assertThat(clusterState.routingNodes().node("node1").size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node1").shardsWithState(STARTED).size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node2").size(), equalTo(1)); + assertThat(clusterState.routingNodes().node("node2").shardsWithState(STARTED).size(), equalTo(1)); + + logger.info("--> cancel the primary allocation (with allow_primary set to true)"); rerouteResult = allocation.reroute(clusterState, new AllocationCommands(new CancelAllocationCommand(new ShardId("test", 0), "node1", true))); clusterState = ClusterState.builder(clusterState).routingTable(rerouteResult.routingTable()).build();