Minor fixes after review round

3 files changed, 12 insertions, 5 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeMutex.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeMutex.java
index 8e606a4af3d..814b40a73b7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeMutex.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeMutex.java
@@ -21,7 +21,11 @@ public class NodeMutex implements Mutex {
     @Override public void close() { mutex.close(); }
 
     /** Returns a node mutex with the same mutex as this, but the given node.  Be sure to close only one. */
-    public NodeMutex with(Node newNode) {
-        return new NodeMutex(newNode, mutex);
+    public NodeMutex with(Node updatedNode) {
+        if (!node.equals(updatedNode)) {
+            throw new IllegalArgumentException("Updated node not equal to current");
+        }
+
+        return new NodeMutex(updatedNode, mutex);
     }
 }
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 76cae7c0e50..4eb38fa650e 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
@@ -928,9 +928,11 @@ public class NodeRepository extends AbstractComponent {
     public Optional<NodeMutex> lockAndGet(Node node) {
         Node staleNode = node;
 
-        for (int i = 0; i < 4; ++i) {
+        final int maxRetries = 4;
+        for (int i = 0; i < maxRetries; ++i) {
             Mutex lockToClose = lock(staleNode);
             try {
+                // As an optimization we first try finding the node in the same state
                 Optional<Node> freshNode = getNode(staleNode.hostname(), staleNode.state());
                 if (freshNode.isEmpty()) {
                     freshNode = getNode(staleNode.hostname());
@@ -953,7 +955,8 @@ public class NodeRepository extends AbstractComponent {
             }
         }
 
-        throw new IllegalStateException("Giving up trying to fetch an up to date node under lock: " + node.hostname());
+        throw new IllegalStateException("Giving up (after " + maxRetries + " attempts) " +
+                "fetching an up to date node under lock: " + node.hostname());
     }
 
     /** Returns the unallocated/application lock, and the node acquired under that lock. */
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java
index 15bf9dee805..47b0021874d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java
@@ -332,7 +332,7 @@ public class NodePatcher implements AutoCloseable {
         }
 
         public List<Node> nodes() {
-            return List.copyOf(nodes.values().stream().map(NodeMutex::node).collect(Collectors.toList()));
+            return nodes.values().stream().map(NodeMutex::node).collect(Collectors.toList());
         }
 
         @Override