Test automatic task failing on controller leadership loss

3 files changed, 54 insertions, 11 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
index ab16d9a1d1d..7d579d241b7 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
@@ -868,7 +868,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
                 eventLog.add(new ClusterEvent(ClusterEvent.Type.MASTER_ELECTION, "This node is no longer fleetcontroller master.", timer.getCurrentTimeInMillis()));
                 firstAllowedStateBroadcast = Long.MAX_VALUE;
                 metricUpdater.noLongerMaster();
-                failAllVersionDependentTasks(); // TODO test
+                failAllVersionDependentTasks();
             }
             wantedStateChanged = false;
             isMaster = false;
@@ -892,7 +892,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
             System.exit(1);
         } finally {
             running.set(false);
-            failAllVersionDependentTasks(); // TODO test....
+            failAllVersionDependentTasks();
             synchronized (monitor) { monitor.notifyAll(); }
         }
     }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
index b6bc8fc1337..dc79a92e68b 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
@@ -27,7 +27,6 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-@Ignore
 public class MasterElectionTest extends FleetControllerTest {
 
     private static Logger log = Logger.getLogger(MasterElectionTest.class.getName());
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java
index 28d1095f629..959fe4e5afd 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java
@@ -52,7 +52,12 @@ public class StateChangeTest extends FleetControllerTest {
         ctrl = new FleetController(timer, eventLog, cluster, stateGatherer, communicator, null, null, communicator, database, stateGenerator, stateBroadcaster, masterElectionHandler, metricUpdater, options);
 
         ctrl.tick();
+        if (options.fleetControllerCount == 1) {
+            markAllNodesAsUp(options);
+        }
+    }
 
+    private void markAllNodesAsUp(FleetControllerOptions options) throws Exception {
         for (int i = 0; i < options.nodes.size(); ++i) {
             communicator.setNodeState(new Node(NodeType.STORAGE, i), State.UP, "");
             communicator.setNodeState(new Node(NodeType.DISTRIBUTOR, i), State.UP, "");
@@ -1242,11 +1247,17 @@ public class StateChangeTest extends FleetControllerTest {
 
     private static abstract class MockTask extends RemoteClusterControllerTask {
         boolean invoked = false;
+        boolean leadershipLost = false;
 
         boolean isInvoked() { return invoked; }
 
+        boolean isLeadershipLost() { return leadershipLost; }
+
         @Override
         public boolean hasVersionAckDependency() { return true; }
+
+        @Override
+        public void handleLeadershipLost() { this.leadershipLost = true; }
     }
 
     // We create an explicit mock task class instead of using mock() simply because of
@@ -1313,6 +1324,11 @@ public class StateChangeTest extends FleetControllerTest {
             communicator.sendAllDeferredDistributorClusterStateAcks();
             ctrl.tick();
         }
+
+        void processScheduledTask() throws Exception {
+            ctrl.tick(); // Cluster state recompute iteration and send
+            ctrl.tick(); // Iff ACKs were received, process version dependent task(s)
+        }
     }
 
     private static FleetControllerOptions defaultOptions() {
@@ -1349,8 +1365,7 @@ public class StateChangeTest extends FleetControllerTest {
         assertFalse(task.isCompleted());
         communicator.setShouldDeferDistributorClusterStateAcks(true);
 
-        ctrl.tick(); // Cluster state recompute iteration. New state generated, but not ACKed by nodes
-        ctrl.tick(); // Ensure that we're deferring ACKs. Otherwise, this tick would process ACKs and complete tasks.
+        fixture.processScheduledTask(); // New state generated, but not ACKed by nodes since we're deferring.
         assertFalse(task.isCompleted());
 
         fixture.sendPartialDeferredDistributorClusterStateAcks();
@@ -1369,8 +1384,7 @@ public class StateChangeTest extends FleetControllerTest {
         assertTrue(task.isInvoked());
         assertFalse(task.isCompleted());
 
-        ctrl.tick(); // Cluster state recompute iteration. New state _not_ generated
-        ctrl.tick(); // Deferred tasks processing; should complete tasks
+        fixture.processScheduledTask(); // Deferred tasks processing; should complete tasks
         assertTrue(task.isCompleted());
     }
 
@@ -1385,8 +1399,7 @@ public class StateChangeTest extends FleetControllerTest {
         assertTrue(task.isInvoked());
         assertFalse(task.isCompleted());
 
-        ctrl.tick(); // Cluster state recompute iteration. New state _not_ generated
-        ctrl.tick(); // Deferred task processing; version not satisfied yet
+        fixture.processScheduledTask(); // Deferred task processing; version not satisfied yet
         assertFalse(task.isCompleted());
 
         fixture.sendAllDeferredDistributorClusterStateAcks();
@@ -1409,11 +1422,42 @@ public class StateChangeTest extends FleetControllerTest {
         assertTrue(task.isInvoked());
         assertFalse(task.isCompleted());
 
-        ctrl.tick(); // Cluster state recompute iteration. New state _not_ generated
-        ctrl.tick(); // Deferred tasks processing; should complete tasks
+        fixture.processScheduledTask(); // Deferred tasks processing; should complete tasks
         assertTrue(task.isCompleted());
+    }
+
+    @Test
+    public void synchronous_task_immediately_failed_when_leadership_lost() throws Exception {
+        FleetControllerOptions options = optionsWithZeroTransitionTime();
+        options.fleetControllerCount = 3;
+        RemoteTaskFixture fixture = createFixtureWith(options);
 
+        Map<Integer, Integer> leaderVotes = new HashMap<>();
+        leaderVotes.put(0, 0);
+        leaderVotes.put(1, 0);
+        leaderVotes.put(2, 0);
+        ctrl.handleFleetData(leaderVotes);
+        ctrl.tick();
+        markAllNodesAsUp(options);
 
+        MockTask task = fixture.scheduleNonIdempotentVersionDependentTask();
+
+        assertTrue(task.isInvoked());
+        assertFalse(task.isCompleted());
+
+        communicator.setShouldDeferDistributorClusterStateAcks(true);
+        fixture.processScheduledTask();
+        assertFalse(task.isCompleted());
+        assertFalse(task.isLeadershipLost());
+
+        // Receive leadership loss event; other nodes not voting for us anymore.
+        leaderVotes.put(1, 1);
+        leaderVotes.put(2, 1);
+        ctrl.handleFleetData(leaderVotes);
+        ctrl.tick();
+
+        assertTrue(task.isCompleted());
+        assertTrue(task.isLeadershipLost());
     }
 
 }