Simplify since we don't have node locks
@@ -113,15 +113,19 @@ public class Rebalancer extends Maintainer {
return bestMove;
}
- private boolean markWantToRetire(Node node, boolean wantToRetire, Mutex lock) {
+ /** Returns true only if this operation changes the state of the wantToRetire flag */
+ private boolean markWantToRetire(Node node, boolean wantToRetire) {
+ try (Mutex lock = nodeRepository().lock(node)) {
Optional<Node> nodeToMove = nodeRepository().getNode(node.hostname());
if (nodeToMove.isEmpty()) return false;
if (nodeToMove.get().state() != Node.State.active) return false;
- if (node.status().wantToRetire() != wantToRetire)
+ if (node.status().wantToRetire() == wantToRetire) return false;
+
nodeRepository().write(nodeToMove.get().withWantToRetire(wantToRetire, Agent.system, clock.instant()), lock);
return true;
}
+ }
/**
* Try a redeployment to effect the chosen move.
@@ -133,9 +137,10 @@ public class Rebalancer extends Maintainer {
ApplicationId application = move.node.allocation().get().owner();
try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) {
if ( ! deployment.isValid()) return false;
- try (Mutex lock = nodeRepository().lock(move.node)) {
- boolean couldMarkRetired = markWantToRetire(move.node, true, lock);
- if ( ! couldMarkRetired) return false;
+
+ boolean couldMarkRetiredNow = markWantToRetire(move.node, true);
+ if ( ! couldMarkRetiredNow) return false;
+
try {
if ( ! deployment.prepare()) return false;
if (nodeRepository().getNodes(application, Node.State.reserved).stream().noneMatch(node -> node.hasParent(move.toHost.hostname())))
@@ -146,8 +151,7 @@ public class Rebalancer extends Maintainer {
return true;
}
finally {
- markWantToRetire(move.node, false, lock); // In case this failed, noop otherwise
- }
+ markWantToRetire(move.node, false); // Necessary if this failed, no-op otherwise
}
}
}
