Skip to content

Commit

Permalink
Use disassociate in preference to deassociate (#37704)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidkyle committed Jan 23, 2019
1 parent 1b187fe commit 17ffdd4
Show file tree
Hide file tree
Showing 25 changed files with 50 additions and 50 deletions.
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 @@ -510,8 +510,8 @@ private ClusterState.Builder becomeMasterAndTrimConflictingNodes(ClusterState cu
ClusterState tmpState = ClusterState.builder(currentState).nodes(nodesBuilder).blocks(ClusterBlocks.builder()
.blocks(currentState.blocks())
.removeGlobalBlock(DiscoverySettings.NO_MASTER_BLOCK_ID)).build();
tmpState = PersistentTasksCustomMetaData.deassociateDeadNodes(tmpState);
return ClusterState.builder(allocationService.deassociateDeadNodes(tmpState, false,
tmpState = PersistentTasksCustomMetaData.disassociateDeadNodes(tmpState);
return ClusterState.builder(allocationService.disassociateDeadNodes(tmpState, false,
"removed dead nodes on election"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,8 @@ public ClusterTasksResult<Task> execute(final ClusterState currentState, final L
masterNodes, electMasterService.minimumMasterNodes()));
return resultBuilder.build(currentState);
} else {
ClusterState ptasksDisassociatedState = PersistentTasksCustomMetaData.deassociateDeadNodes(remainingNodesClusterState);
return resultBuilder.build(allocationService.deassociateDeadNodes(ptasksDisassociatedState, true, describeTasks(tasks)));
ClusterState ptasksDisassociatedState = PersistentTasksCustomMetaData.disassociateDeadNodes(remainingNodesClusterState);
return resultBuilder.build(allocationService.disassociateDeadNodes(ptasksDisassociatedState, true, describeTasks(tasks)));
}
}

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 @@ -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 @@ -110,7 +110,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

0 comments on commit 17ffdd4

Please sign in to comment.