Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use disassociate in preference to deassociate #37704

Merged
merged 1 commit into from
Jan 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ protected ClusterState.Builder becomeMasterAndTrimConflictingNodes(ClusterState
.blocks(currentState.blocks())
.removeGlobalBlock(DiscoverySettings.NO_MASTER_BLOCK_ID)).build();
logger.trace("becomeMasterAndTrimConflictingNodes: {}", tmpState.nodes());
tmpState = PersistentTasksCustomMetaData.deassociateDeadNodes(tmpState);
return ClusterState.builder(allocationService.deassociateDeadNodes(tmpState, false, "removed dead nodes on election"));
tmpState = PersistentTasksCustomMetaData.disassociateDeadNodes(tmpState);
return ClusterState.builder(allocationService.disassociateDeadNodes(tmpState, false, "removed dead nodes on election"));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ public ClusterTasksResult<Task> execute(final ClusterState currentState, final L

protected ClusterTasksResult<Task> getTaskClusterTasksResult(ClusterState currentState, List<Task> tasks,
ClusterState remainingNodesClusterState) {
ClusterState ptasksDeassociatedState = PersistentTasksCustomMetaData.deassociateDeadNodes(remainingNodesClusterState);
ClusterState ptasksDisassociatedState = PersistentTasksCustomMetaData.disassociateDeadNodes(remainingNodesClusterState);
final ClusterTasksResult.Builder<Task> resultBuilder = ClusterTasksResult.<Task>builder().successes(tasks);
return resultBuilder.build(allocationService.deassociateDeadNodes(ptasksDeassociatedState, true, describeTasks(tasks)));
return resultBuilder.build(allocationService.disassociateDeadNodes(ptasksDisassociatedState, true, describeTasks(tasks)));
}

// visible for testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
* the delay marker). These are shards that have become unassigned due to a node leaving
* and which were assigned the delay marker based on the index delay setting
* {@link UnassignedInfo#INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING}
* (see {@link AllocationService#deassociateDeadNodes(RoutingAllocation)}).
* (see {@link AllocationService#disassociateDeadNodes(RoutingAllocation)}.
* This class is responsible for choosing the next (closest) delay expiration of a
* delayed shard to schedule a reroute to remove the delay marker.
* The actual removal of the delay marker happens in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public ShardRouting activePrimary(ShardId shardId) {
*
*/
public ShardRouting activeReplicaWithHighestVersion(ShardId shardId) {
// It's possible for replicaNodeVersion to be null, when deassociating dead nodes
// It's possible for replicaNodeVersion to be null, when disassociating dead nodes
// that have been removed, the shards are failed, and part of the shard failing
// calls this method with an out-of-date RoutingNodes, where the version might not
// be accessible. Therefore, we need to protect against the version being null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,15 @@ public ClusterState applyFailedShards(final ClusterState clusterState, final Lis
* unassigned an shards that are associated with nodes that are no longer part of the cluster, potentially promoting replicas
* if needed.
*/
public ClusterState deassociateDeadNodes(ClusterState clusterState, boolean reroute, String reason) {
public ClusterState disassociateDeadNodes(ClusterState clusterState, boolean reroute, String reason) {
RoutingNodes routingNodes = getMutableRoutingNodes(clusterState);
// shuffle the unassigned nodes, just so we won't have things like poison failed shards
routingNodes.unassigned().shuffle();
RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, routingNodes, clusterState,
clusterInfoService.getClusterInfo(), currentNanoTime());

// first, clear from the shards any node id they used to belong to that is now dead
deassociateDeadNodes(allocation);
disassociateDeadNodes(allocation);

if (allocation.routingNodesChanged()) {
clusterState = buildResult(clusterState, allocation);
Expand Down Expand Up @@ -400,7 +400,7 @@ private boolean hasDeadNodes(RoutingAllocation allocation) {
}

private void reroute(RoutingAllocation allocation) {
assert hasDeadNodes(allocation) == false : "dead nodes should be explicitly cleaned up. See deassociateDeadNodes";
assert hasDeadNodes(allocation) == false : "dead nodes should be explicitly cleaned up. See disassociateDeadNodes";
assert AutoExpandReplicas.getAutoExpandReplicaChanges(allocation.metaData(), allocation.nodes()).isEmpty() :
"auto-expand replicas out of sync with number of nodes in the cluster";

Expand All @@ -414,7 +414,7 @@ private void reroute(RoutingAllocation allocation) {
assert RoutingNodes.assertShardStats(allocation.routingNodes());
}

private void deassociateDeadNodes(RoutingAllocation allocation) {
private void disassociateDeadNodes(RoutingAllocation allocation) {
for (Iterator<RoutingNode> it = allocation.routingNodes().mutableIterator(); it.hasNext(); ) {
RoutingNode node = it.next();
if (allocation.nodes().getDataNodes().containsKey(node.nodeId())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public static <Params extends PersistentTaskParams> PersistentTask<Params> getTa
* @return If no changes the argument {@code clusterState} is returned else
* a copy with the modified tasks
*/
public static ClusterState deassociateDeadNodes(ClusterState clusterState) {
public static ClusterState disassociateDeadNodes(ClusterState clusterState) {
PersistentTasksCustomMetaData tasks = getPersistentTasksCustomMetaData(clusterState);
if (tasks == null) {
return clusterState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void testRemovingNonExistentNodes() throws Exception {

public void testRerouteAfterRemovingNodes() throws Exception {
final AllocationService allocationService = mock(AllocationService.class);
when(allocationService.deassociateDeadNodes(any(ClusterState.class), eq(true), any(String.class)))
when(allocationService.disassociateDeadNodes(any(ClusterState.class), eq(true), any(String.class)))
.thenAnswer(im -> im.getArguments()[0]);

final AtomicReference<ClusterState> remainingNodesClusterState = new AtomicReference<>();
Expand Down Expand Up @@ -102,7 +102,7 @@ protected ClusterState remainingNodesClusterState(ClusterState currentState,
final ClusterStateTaskExecutor.ClusterTasksResult<NodeRemovalClusterStateTaskExecutor.Task> result =
executor.execute(clusterState, tasks);

verify(allocationService).deassociateDeadNodes(eq(remainingNodesClusterState.get()), eq(true), any(String.class));
verify(allocationService).disassociateDeadNodes(eq(remainingNodesClusterState.get()), eq(true), any(String.class));

for (final NodeRemovalClusterStateTaskExecutor.Task task : tasks) {
assertNull(result.resultingState.nodes().get(task.node().getId()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void testNoDelayedUnassigned() throws Exception {
nodes.add(newNode("node3"));
}
clusterState = ClusterState.builder(clusterState).nodes(nodes).build();
clusterState = allocationService.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocationService.disassociateDeadNodes(clusterState, true, "reroute");
ClusterState newState = clusterState;
List<ShardRouting> unassignedShards = newState.getRoutingTable().shardsWithState(ShardRoutingState.UNASSIGNED);
if (nodeAvailableForAllocation) {
Expand Down Expand Up @@ -153,7 +153,7 @@ public void testDelayedUnassignedScheduleReroute() throws Exception {

// remove node that has replica and reroute
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder(clusterState.nodes()).remove(nodeId)).build();
clusterState = allocationService.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocationService.disassociateDeadNodes(clusterState, true, "reroute");
ClusterState stateWithDelayedShard = clusterState;
// make sure the replica is marked as delayed (i.e. not reallocated)
assertEquals(1, UnassignedInfo.getNumberOfDelayedUnassigned(stateWithDelayedShard));
Expand Down Expand Up @@ -264,7 +264,7 @@ public void testDelayedUnassignedScheduleRerouteAfterDelayedReroute() throws Exc
.build();
// make sure both replicas are marked as delayed (i.e. not reallocated)
allocationService.setNanoTimeOverride(baseTimestampNanos);
clusterState = allocationService.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocationService.disassociateDeadNodes(clusterState, true, "reroute");
final ClusterState stateWithDelayedShards = clusterState;
assertEquals(2, UnassignedInfo.getNumberOfDelayedUnassigned(stateWithDelayedShards));
RoutingNodes.UnassignedShards.UnassignedIterator iter = stateWithDelayedShards.getRoutingNodes().unassigned().iterator();
Expand Down Expand Up @@ -401,7 +401,7 @@ public void testDelayedUnassignedScheduleRerouteRescheduledOnShorterDelay() thro
// remove node that has replica and reroute
clusterState = ClusterState.builder(clusterState).nodes(
DiscoveryNodes.builder(clusterState.nodes()).remove(nodeIdOfFooReplica)).build();
clusterState = allocationService.deassociateDeadNodes(clusterState, true, "fake node left");
clusterState = allocationService.disassociateDeadNodes(clusterState, true, "fake node left");
ClusterState stateWithDelayedShard = clusterState;
// make sure the replica is marked as delayed (i.e. not reallocated)
assertEquals(1, UnassignedInfo.getNumberOfDelayedUnassigned(stateWithDelayedShard));
Expand Down Expand Up @@ -444,7 +444,7 @@ public void testDelayedUnassignedScheduleRerouteRescheduledOnShorterDelay() thro
// remove node that has replica and reroute
clusterState = ClusterState.builder(stateWithDelayedShard).nodes(
DiscoveryNodes.builder(stateWithDelayedShard.nodes()).remove(nodeIdOfBarReplica)).build();
ClusterState stateWithShorterDelay = allocationService.deassociateDeadNodes(clusterState, true, "fake node left");
ClusterState stateWithShorterDelay = allocationService.disassociateDeadNodes(clusterState, true, "fake node left");
delayedAllocationService.setNanoTimeOverride(clusterChangeEventTimestampNanos);
delayedAllocationService.clusterChanged(
new ClusterChangedEvent("fake node left", stateWithShorterDelay, stateWithDelayedShard));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public void testNodeLeave() {
assertThat(clusterState.getRoutingNodes().unassigned().size() > 0, equalTo(false));
// remove node2 and reroute
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder(clusterState.nodes()).remove("node2")).build();
clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");
// verify that NODE_LEAVE is the reason for meta
assertThat(clusterState.getRoutingNodes().unassigned().size() > 0, equalTo(true));
assertThat(clusterState.getRoutingNodes().shardsWithState(UNASSIGNED).size(), equalTo(1));
Expand Down Expand Up @@ -332,7 +332,7 @@ public void testNumberOfDelayedUnassigned() throws Exception {
// remove node2 and reroute
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder(clusterState.nodes()).remove("node2")).build();
// make sure both replicas are marked as delayed (i.e. not reallocated)
clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");
assertThat(clusterState.toString(), UnassignedInfo.getNumberOfDelayedUnassigned(clusterState), equalTo(2));
}

Expand Down Expand Up @@ -364,7 +364,7 @@ public void testFindNextDelayedAllocation() {
final long baseTime = System.nanoTime();
allocation.setNanoTimeOverride(baseTime);
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder(clusterState.nodes()).remove("node2")).build();
clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");

final long delta = randomBoolean() ? 0 : randomInt((int) expectMinDelaySettingsNanos - 1);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ private ClusterState removeNodes(ClusterState clusterState, AllocationService se
}

clusterState = ClusterState.builder(clusterState).nodes(nodes.build()).build();
clusterState = service.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = service.disassociateDeadNodes(clusterState, true, "reroute");

logger.info("start all the primary shards, replicas will start initializing");
RoutingNodes routingNodes = clusterState.getRoutingNodes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private ClusterState removeNodes(ClusterState clusterState, AllocationService st

clusterState = ClusterState.builder(clusterState).nodes(nodes.build()).build();
if (removed) {
clusterState = strategy.deassociateDeadNodes(clusterState, randomBoolean(), "removed nodes");
clusterState = strategy.disassociateDeadNodes(clusterState, randomBoolean(), "removed nodes");
}

logger.info("start all the primary shards, replicas will start initializing");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void testSimpleDeadNodeOnStartedPrimaryShard() {
.add(newNode(nodeIdRemaining))
).build();

clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");

assertThat(clusterState.getRoutingNodes().node(nodeIdRemaining).iterator().next().primary(), equalTo(true));
assertThat(clusterState.getRoutingNodes().node(nodeIdRemaining).iterator().next().state(), equalTo(STARTED));
Expand Down Expand Up @@ -154,7 +154,7 @@ public void testDeadNodeWhileRelocatingOnToNode() {
.add(newNode(origPrimaryNodeId))
.add(newNode(origReplicaNodeId))
).build();
clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");

assertThat(clusterState.getRoutingNodes().node(origPrimaryNodeId).iterator().next().state(), equalTo(STARTED));
assertThat(clusterState.getRoutingNodes().node(origReplicaNodeId).iterator().next().state(), equalTo(STARTED));
Expand Down Expand Up @@ -224,7 +224,7 @@ public void testDeadNodeWhileRelocatingOnFromNode() {
.add(newNode("node3"))
.add(newNode(origReplicaNodeId))
).build();
clusterState = allocation.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = allocation.disassociateDeadNodes(clusterState, true, "reroute");

assertThat(clusterState.getRoutingNodes().node(origReplicaNodeId).iterator().next().state(), equalTo(STARTED));
assertThat(clusterState.getRoutingNodes().node("node3").iterator().next().state(), equalTo(INITIALIZING));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void testElectReplicaAsPrimaryDuringRelocation() {
indexShardRoutingTable.primaryShard().currentNodeId());
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder(clusterState.nodes())
.remove(indexShardRoutingTable.primaryShard().currentNodeId())).build();
clusterState = strategy.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = strategy.disassociateDeadNodes(clusterState, true, "reroute");

logger.info("make sure all the primary shards are active");
assertThat(clusterState.routingTable().index("test").shard(0).primaryShard().active(), equalTo(true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testSimpleFailedNodeTest() {
.remove(clusterState.routingTable().index("test2").shard(0).primaryShard().currentNodeId())
)
.build();
clusterState = strategy.deassociateDeadNodes(clusterState, true, "reroute");
clusterState = strategy.disassociateDeadNodes(clusterState, true, "reroute");
routingNodes = clusterState.getRoutingNodes();

for (RoutingNode routingNode : routingNodes) {
Expand Down
Loading