Merge branch 'master' into mpolden/ignore-stale-job-data

9 files changed, 43 insertions, 48 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
index e0d18f2b571..06b50145fc1 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
@@ -322,10 +322,6 @@ public class FastHit extends Hit {
             needXmlEscape = ! (fieldType instanceof XMLField);
             this.contents = contents;
         }
-        public RawField(byte [] contents) {
-            needXmlEscape = true;
-            this.contents = contents;
-        }
 
         public byte [] getUtf8() { return contents; }
         public boolean needXmlEscape() { return needXmlEscape; }
@@ -359,10 +355,6 @@ public class FastHit extends Hit {
         return queryPacketData;
     }
 
-    public void clearQueryPacketData() {
-        queryPacketData = null;
-    }
-
     CacheKey getCacheKey() {
         return cacheKey;
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 2e81ef19f5e..9d2198cedcc 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -180,7 +180,9 @@ public class NodeAgentImpl implements NodeAgent {
 
     @Override
     public void start(int intervalMillis) {
-        addDebugMessage("Starting with interval " + intervalMillis + "ms");
+        String message = "Starting with interval " + intervalMillis + " ms";
+        logger.info(message);
+        addDebugMessage(message);
         delaysBetweenEachConvergeMillis = intervalMillis;
         if (loopThread != null) {
             throw new RuntimeException("Can not restart a node agent.");
@@ -214,6 +216,8 @@ public class NodeAgentImpl implements NodeAgent {
         } catch (InterruptedException e) {
             logger.error("Interrupted; Could not stop filebeatrestarter thread");
         }
+
+        logger.info("Stopped");
     }
 
     private void runLocalResumeScriptIfNeeded() {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index 1885b54e9c0..68bdffd75f9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -25,6 +25,7 @@ import com.yahoo.vespa.hosted.provision.node.filter.StateFilter;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
 import com.yahoo.vespa.hosted.provision.persistence.DnsNameResolver;
 import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
+import com.yahoo.vespa.hosted.provision.restapi.v2.NotFoundException;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -468,24 +469,26 @@ public class NodeRepository extends AbstractComponent {
 
     /**
      * Removes a node. A node must be in a legal state before it can be removed.
-     *
-     * @return true if the node was removed, false if it was not found in one of the legal states
      */
-    public boolean remove(String hostname) {
-
-        Node.State[] legalStates = {Node.State.provisioned, Node.State.failed, Node.State.parked};
-        Node.State[] legalDynamicStates = {Node.State.provisioned, Node.State.failed, Node.State.parked, Node.State.dirty};
-
-        Optional<Node> nodeToRemove = getNode(hostname, dynamicAllocationEnabled() ? legalDynamicStates : legalStates);
-        if ( ! nodeToRemove.isPresent()) return false;
+    public void remove(String hostname) {
+        Node nodeToRemove = getNode(hostname).orElseThrow(() ->  new NotFoundException("No node with hostname \"" + hostname + '"'));
+        List<Node.State> legalStates = dynamicAllocationEnabled() ?
+                Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked, Node.State.dirty) :
+                Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked);
+
+        if (! legalStates.contains(nodeToRemove.state())) {
+            throw new IllegalArgumentException("Can only remove node from following states: " +
+                    legalStates.stream().map(Node.State::name).collect(Collectors.joining(", ")));
+        }
 
-        // Only docker nodes are allowed to be deleted in state dirty.
-        if ( nodeToRemove.get().state().equals(Node.State.dirty)) {
-            if (!(nodeToRemove.get().flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER))) return false;
+        if (nodeToRemove.state().equals(Node.State.dirty)) {
+            if (!(nodeToRemove.flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER))) {
+                throw new IllegalArgumentException("Only docker nodes can be deleted from state dirty");
+            }
         }
 
-        try (Mutex lock = lock(nodeToRemove.get())) {
-            return db.removeNode(nodeToRemove.get().state(), hostname);
+        try (Mutex lock = lock(nodeToRemove)) {
+            db.removeNode(nodeToRemove.state(), hostname);
         }
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index cd63599fed6..651ee44415f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -108,16 +108,14 @@ public class CuratorDatabaseClient {
      *
      * @param state the current state of the node
      * @param hostName the host name of the node to remove
-     * @return true if the node was removed, false if it was not found
      */
-    public boolean removeNode(Node.State state, String hostName) {
+    public void removeNode(Node.State state, String hostName) {
         Path path = toPath(state, hostName);
         NestedTransaction transaction = new NestedTransaction();
         CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.delete(path.getAbsolute()));
         transaction.commit();
         log.log(LogLevel.INFO, "Removed: " + state + " node " + hostName);
-        return true;
     }
 
     /**
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
index db75894673e..4e7bb1f7d16 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
@@ -135,10 +135,8 @@ public class NodesApiHandler extends LoggingRequestHandler {
              */
             String hostname = lastElement(path);
             if (nodeRepository.dynamicAllocationEnabled()) {
-                if (nodeRepository.remove(hostname))
-                    return new MessageResponse("Removed " + hostname);
-                else
-                    throw new NotFoundException("No node in the provisioned, parked, dirty or failed state with hostname " + hostname);
+                nodeRepository.remove(hostname);
+                return new MessageResponse("Removed " + hostname);
             } else {
                 nodeRepository.setReady(hostname);
                 return new MessageResponse("Moved " + hostname + " to ready");
@@ -182,10 +180,8 @@ public class NodesApiHandler extends LoggingRequestHandler {
         String path = request.getUri().getPath();
         if (path.startsWith("/nodes/v2/node/")) {
             String hostname = lastElement(path);
-            if (nodeRepository.remove(hostname))
-                return new MessageResponse("Removed " + hostname);
-            else
-                throw new NotFoundException("No node in the provisioned, parked or failed state with hostname " + hostname);
+            nodeRepository.remove(hostname);
+            return new MessageResponse("Removed " + hostname);
         }
         else if (path.startsWith("/nodes/v2/maintenance/inactive/")) {
             return setActive(lastElement(path), true);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
index 12c76638604..bcae9d293f6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -13,6 +13,7 @@ import org.junit.Test;
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
+import static junit.framework.TestCase.fail;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -36,7 +37,7 @@ public class NodeRepositoryTest {
         assertEquals(3, tester.getNodes(NodeType.tenant).size());
         
         tester.nodeRepository().park("host2", Agent.system, "Parking to unit test");
-        assertTrue(tester.nodeRepository().remove("host2"));
+        tester.nodeRepository().remove("host2");
 
         assertEquals(2, tester.getNodes(NodeType.tenant).size());
     }
@@ -71,17 +72,18 @@ public class NodeRepositoryTest {
     @Test
     public void only_allow_to_delete_dirty_nodes_when_dynamic_allocation_feature_enabled() {
         NodeRepositoryTester tester = new NodeRepositoryTester();
-        try {
-            tester.addNode("id1", "host1", "default", NodeType.host);
-            tester.addNode("id2", "host2", "docker", NodeType.tenant);
-            tester.nodeRepository().setDirty("host2");
-
-            assertFalse(tester.nodeRepository().remove("host2"));
+        tester.addNode("id1", "host1", "default", NodeType.host);
+        tester.addNode("id2", "host2", "docker", NodeType.tenant);
+        tester.nodeRepository().setDirty("host2");
 
-            tester.curator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]);
-            assertTrue(tester.nodeRepository().remove("host2"));
-        } finally {
-            tester.curator().delete(Path.fromString("/provision/v1/dynamicDockerAllocation"));
+        try {
+            tester.nodeRepository().remove("host2");
+            fail("Should not be able to delete tenant node in state dirty");
+        } catch (IllegalArgumentException ignored) {
+            // Expected
         }
+
+        tester.curator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]);
+        tester.nodeRepository().remove("host2");
     }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java
index ee0b8f55a4b..cb46d0a4624 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java
@@ -49,7 +49,7 @@ public class ZooKeeperAccessMaintainerTest {
         assertEquals(asSet("host1,host2,host3,host4,host5,server1,server2"), ZooKeeperServer.getAllowedClientHostnames());
 
         tester.nodeRepository().park("host2", Agent.system, "Parking to unit test");
-        assertTrue(tester.nodeRepository().remove("host2"));
+        tester.nodeRepository().remove("host2");
         maintainer.maintain();
 
         assertEquals(2, tester.getNodes(NodeType.tenant).size());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index deb3378679c..565dfe0457e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -401,7 +401,7 @@ public class RestApiTest {
         // Attempt to DELETE a node which is not put in a deletable state first
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/host2.yahoo.com",
                                    new byte[0], Request.Method.DELETE),
-                       404, "{\"error-code\":\"NOT_FOUND\",\"message\":\"No node in the provisioned, parked or failed state with hostname host2.yahoo.com\"}");
+                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can only remove node from following states: provisioned, failed, parked, dirty\"}");
 
         // PUT current restart generation with string instead of long
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com",
diff --git a/travis/travis-build-cpp.sh b/travis/travis-build-cpp.sh
index 4386be3b878..825da67bf54 100755
--- a/travis/travis-build-cpp.sh
+++ b/travis/travis-build-cpp.sh
@@ -7,7 +7,7 @@ BUILD_DIR=~/build
 
 mkdir "${BUILD_DIR}"
 
-export CCACHE_MAXSIZE="1G"
+export CCACHE_MAXSIZE="1250M"
 export CCACHE_COMPRESS=1
 NUM_THREADS=4
 ccache --print-config