From c6dd9ee63a9979faac91db16d7a9bf27fa65a0d4 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 27 Feb 2020 12:03:20 +0100 Subject: Only get list of nodes when actually needed Use a supplier and get list of nodes only when needed (when wanting to retire a node). This should improve performance for all other patch requests. --- .../hosted/provision/restapi/v2/NodePatcher.java | 24 +++++++++------------- .../provision/restapi/v2/NodesApiHandler.java | 7 ++++--- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java index 11be95604c8..ce32e5a8a33 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.TreeSet; +import java.util.function.Supplier; import java.util.stream.Collectors; import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; @@ -50,17 +51,14 @@ public class NodePatcher { private final NodeFlavors nodeFlavors; private final Inspector inspector; - private final LockedNodeList nodes; + private final Supplier nodes; private final Clock clock; private Node node; - private List children; - private boolean childrenModified = false; - public NodePatcher(NodeFlavors nodeFlavors, InputStream json, Node node, LockedNodeList nodes, Clock clock) { + public NodePatcher(NodeFlavors nodeFlavors, InputStream json, Node node, Supplier nodes, Clock clock) { this.nodeFlavors = nodeFlavors; this.node = node; - this.children = node.type().isDockerHost() ? nodes.childrenOf(node).asList() : List.of(); this.nodes = nodes; this.clock = clock; try { @@ -76,6 +74,7 @@ public class NodePatcher { * children that must be updated in a consistent manner. */ public List apply() { + List patchedNodes = new ArrayList<>(List.of(node)); inspector.traverse((String name, Inspector value) -> { try { node = applyField(node, name, value); @@ -84,22 +83,19 @@ public class NodePatcher { } try { - children = applyFieldRecursive(children, name, value); - childrenModified = true; + patchedNodes.addAll(applyFieldRecursive(name, value)); } catch (IllegalArgumentException e) { // Non recursive field, ignore } } ); - List nodes = childrenModified ? new ArrayList<>(children) : new ArrayList<>(); - nodes.add(node); - - return nodes; + return patchedNodes; } - private List applyFieldRecursive(List childNodes, String name, Inspector value) { + private List applyFieldRecursive(String name, Inspector value) { switch (name) { case WANT_TO_RETIRE: + List childNodes = node.type().isDockerHost() ? nodes.get().childrenOf(node).asList() : List.of(); return childNodes.stream() .map(child -> applyField(child, name, value)) .collect(Collectors.toList()); @@ -133,9 +129,9 @@ public class NodePatcher { case "parentHostname" : return node.withParentHostname(asString(value)); case "ipAddresses" : - return IP.Config.verify(node.with(node.ipConfig().with(asStringSet(value))), nodes); + return IP.Config.verify(node.with(node.ipConfig().with(asStringSet(value))), nodes.get()); case "additionalIpAddresses" : - return IP.Config.verify(node.with(node.ipConfig().with(IP.Pool.of(asStringSet(value)))), nodes); + return IP.Config.verify(node.with(node.ipConfig().with(IP.Pool.of(asStringSet(value)))), nodes.get()); case WANT_TO_RETIRE : return node.withWantToRetire(asBoolean(value), Agent.operator, clock.instant()); case "wantToDeprovision" : 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 f0d996ef595..e75f2bbe5b8 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 @@ -145,11 +145,12 @@ public class NodesApiHandler extends LoggingRequestHandler { private HttpResponse handlePATCH(HttpRequest request) { String path = request.getUri().getPath(); if (path.startsWith("/nodes/v2/node/")) { + // TODO: Node is fetched outside of lock, might change after getting lock Node node = nodeFromRequest(request); try (var lock = nodeRepository.lock(node)) { - var patchedNode = new NodePatcher(nodeFlavors, request.getData(), node, nodeRepository.list(lock), - nodeRepository.clock()).apply(); - nodeRepository.write(patchedNode, lock); + var patchedNodes = new NodePatcher(nodeFlavors, request.getData(), node, () -> nodeRepository.list(lock), + nodeRepository.clock()).apply(); + nodeRepository.write(patchedNodes, lock); } return new MessageResponse("Updated " + node.hostname()); } -- cgit v1.2.3 From ae06cc33e932ce132867744eab058eb8fddea25d Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 27 Feb 2020 18:23:28 +0100 Subject: Add node from request last --- .../java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java index ce32e5a8a33..a060f9c8b55 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java @@ -74,7 +74,7 @@ public class NodePatcher { * children that must be updated in a consistent manner. */ public List apply() { - List patchedNodes = new ArrayList<>(List.of(node)); + List patchedNodes = new ArrayList<>(); inspector.traverse((String name, Inspector value) -> { try { node = applyField(node, name, value); @@ -88,6 +88,7 @@ public class NodePatcher { // Non recursive field, ignore } } ); + patchedNodes.add(node); return patchedNodes; } -- cgit v1.2.3