From 1b11bece76394b66a0aa40c2bb2e848621e58b92 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 30 Apr 2020 13:32:20 +0200 Subject: Resize on resource reduce --- .../provision/provisioning/NodeAllocation.java | 12 +++-- .../provision/provisioning/NodePrioritizer.java | 4 +- .../provisioning/InPlaceResizeProvisionTest.java | 52 ++++++++++++++-------- 3 files changed, 43 insertions(+), 25 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 896a5f78d93..a67a5b8e3fb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -56,7 +56,7 @@ class NodeAllocation { private int accepted = 0; /** The number of already allocated nodes accepted and not retired and not needing resize */ - private int acceptedWithoutResize = 0; + private int acceptedWithoutResizingRetired = 0; /** The number of nodes rejected because of clashing parentHostname */ private int rejectedWithClashingParentHost = 0; @@ -234,8 +234,12 @@ class NodeAllocation { if (! wantToRetire) { accepted++; - if (node.allocation().isEmpty() || ! requestedNodes.needsResize(node)) - acceptedWithoutResize++; + + // We want to allocate new nodes rather than unretiring with resize, so count without those + // for the purpose of deciding when to stop accepting nodes (saturation) + if (node.allocation().isEmpty() + || ! ( requestedNodes.needsResize(node) && node.allocation().get().membership().retired())) + acceptedWithoutResizingRetired++; if (resizeable && ! ( node.allocation().isPresent() && node.allocation().get().membership().retired())) node = resize(node); @@ -272,7 +276,7 @@ class NodeAllocation { /** Returns true if no more nodes are needed in this list */ private boolean saturated() { - return requestedNodes.saturatedBy(acceptedWithoutResize); + return requestedNodes.saturatedBy(acceptedWithoutResizingRetired); } /** Returns true if the content of this list is sufficient to meet the request */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 2b0b5bba1e2..a7d83bbfad9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -213,8 +213,8 @@ public class NodePrioritizer { builder.parent(parent).freeParentCapacity(parentCapacity); if (!isNewNode) - builder.resizable(!allocateFully && requestedNodes.canResize( - node.flavor().resources(), parentCapacity, isTopologyChange, currentClusterSize)); + builder.resizable(!allocateFully + && requestedNodes.canResize(node.flavor().resources(), parentCapacity, isTopologyChange, currentClusterSize)); if (spareHosts.contains(parent)) builder.violatesSpares(true); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java index 3a9d87b9db6..aab5f8b0078 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java @@ -158,6 +158,21 @@ public class InPlaceResizeProvisionTest { assertTrue("All initial nodes should still be allocated to the application", initialHostnames.isEmpty()); } + @Test + public void in_place_resource_decrease() { + addParentHosts(30, new NodeResources(10, 100, 1000, 8, fast, local)); + + var largeResources = new NodeResources(6, 64, 800, 1); + new PrepareHelper(tester, app).prepare(content1, 12, 1, largeResources).activate(); + assertSizeAndResources(content1, 12, largeResources.with(local)); + + var smallerResources = new NodeResources(6, 48, 500, 1); + new PrepareHelper(tester, app).prepare(content1, 12, 1, smallerResources).activate(); + assertSizeAndResources(content1, 12, smallerResources.with(local)); + assertEquals(0, listCluster(content1).retired().size()); + } + + /** In this scenario there should be no resizing */ @Test public void increase_size_decrease_resources() { @@ -166,37 +181,36 @@ public class InPlaceResizeProvisionTest { NodeResources resources = new NodeResources(4, 8, 16, 1); NodeResources halvedResources = new NodeResources(2, 4, 8, 1); - new PrepareHelper(tester, app).prepare(container1, 4, 1, resources).activate(); - assertSizeAndResources(container1, 4, resources); + new PrepareHelper(tester, app).prepare(content1, 4, 1, resources).activate(); + assertSizeAndResources(content1, 4, resources); // No resizing since it would initially (before redistribution) lead to too few resources: - new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); - assertSizeAndResources(listCluster(container1).retired(), 4, resources); - assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + new PrepareHelper(tester, app).prepare(content1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(content1).retired(), 4, resources); + assertSizeAndResources(listCluster(content1).not().retired(), 8, halvedResources); // Redeploying the same capacity should also not lead to any resizing - new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); - assertSizeAndResources(listCluster(container1).retired(), 4, resources); - assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + new PrepareHelper(tester, app).prepare(content1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(content1).retired(), 4, resources); + assertSizeAndResources(listCluster(content1).not().retired(), 8, halvedResources); // Failing one of the new nodes should cause another new node to be allocated rather than // unretiring one of the existing nodes, to avoid resizing during unretiring - Node nodeToFail = listCluster(container1).not().retired().asList().get(0); + Node nodeToFail = listCluster(content1).not().retired().asList().get(0); tester.nodeRepository().fail(nodeToFail.hostname(), Agent.system, "testing"); - new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); - assertFalse(listCluster(container1).stream().anyMatch(n -> n.equals(nodeToFail))); - assertSizeAndResources(listCluster(container1).retired(), 4, resources); - assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + new PrepareHelper(tester, app).prepare(content1, 8, 1, halvedResources).activate(); + assertFalse(listCluster(content1).stream().anyMatch(n -> n.equals(nodeToFail))); + assertSizeAndResources(listCluster(content1).retired(), 4, resources); + assertSizeAndResources(listCluster(content1).not().retired(), 8, halvedResources); // ... same with setting a node to want to retire - System.out.println("marking wantToRetire"); - Node nodeToWantoToRetire = listCluster(container1).not().retired().asList().get(0); + Node nodeToWantoToRetire = listCluster(content1).not().retired().asList().get(0); tester.nodeRepository().write(nodeToWantoToRetire.with(nodeToWantoToRetire.status().withWantToRetire(true)), tester.nodeRepository().lock(nodeToWantoToRetire)); - new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); - assertTrue(listCluster(container1).retired().stream().anyMatch(n -> n.equals(nodeToWantoToRetire))); - assertEquals(5, listCluster(container1).retired().size()); - assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + new PrepareHelper(tester, app).prepare(content1, 8, 1, halvedResources).activate(); + assertTrue(listCluster(content1).retired().stream().anyMatch(n -> n.equals(nodeToWantoToRetire))); + assertEquals(5, listCluster(content1).retired().size()); + assertSizeAndResources(listCluster(content1).not().retired(), 8, halvedResources); } @Test(expected = OutOfCapacityException.class) -- cgit v1.2.3