diff options
author | Jon Bratseth <jonbratseth@yahoo.com> | 2017-05-10 15:56:25 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-05-10 15:56:25 +0200 |
commit | 70ab982ead81e15260e16a1448ce027a50aad875 (patch) | |
tree | c9d5c536b0956e7d8be9b4012540decdf7ef4e79 /node-repository | |
parent | 238389de531bf219aacfddee0e54f21ae1fd3033 (diff) | |
parent | 08844b1e7481437e1f63c63ec34d0d4236f51486 (diff) |
Merge pull request #2410 from yahoo/revert-2395-revert-2382-bratseth/prefer-lowest-cost
Revert "Revert "Prefer lower cost nodes over even distribution""
Diffstat (limited to 'node-repository')
6 files changed, 9 insertions, 71 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index c611438fcbe..52e689441d1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -82,7 +82,7 @@ class GroupPreparer { // Use new, ready nodes. Lock ready pool to ensure that nodes are not grabbed by others. try (Mutex readyLock = nodeRepository.lockUnallocated()) { List<Node> readyNodes = nodeRepository.getNodes(requestedNodes.type(), Node.State.ready); - accepted = nodeList.offer(stripeOverHosts(prioritizeNodes(readyNodes, requestedNodes)), !canChangeGroup); + accepted = nodeList.offer(prioritizeNodes(readyNodes, requestedNodes), !canChangeGroup); nodeList.update(nodeRepository.reserve(accepted)); } @@ -109,17 +109,18 @@ class GroupPreparer { * to use comes first */ private List<Node> prioritizeNodes(List<Node> nodeList, NodeSpec nodeSpec) { - if ( nodeSpec.specifiesNonStockFlavor()) { // sort by exact before inexact flavor match, increasing cost, hostname + if ( nodeSpec.specifiesNonStockFlavor()) { // prefer exact flavor, docker hosts, lower cost, tie break by hostname Collections.sort(nodeList, (n1, n2) -> ComparisonChain.start() .compareTrueFirst(nodeSpec.matchesExactly(n1.flavor()), nodeSpec.matchesExactly(n2.flavor())) + .compareTrueFirst(n1.parentHostname().isPresent(), n2.parentHostname().isPresent()) .compare(n1.flavor().cost(), n2.flavor().cost()) .compare(n1.hostname(), n2.hostname()) .result() ); } - else { // sort by increasing cost, hostname + else { // prefer docker hosts, lower cost, tie break by hostname Collections.sort(nodeList, (n1, n2) -> ComparisonChain.start() - .compareTrueFirst(nodeSpec.matchesExactly(n1.flavor()), nodeSpec.matchesExactly(n1.flavor())) + .compareTrueFirst(n1.parentHostname().isPresent(), n2.parentHostname().isPresent()) .compare(n1.flavor().cost(), n2.flavor().cost()) .compare(n1.hostname(), n2.hostname()) .result() @@ -128,44 +129,6 @@ class GroupPreparer { return nodeList; } - /** Return the input nodes in an order striped over their parent hosts */ - static List<Node> stripeOverHosts(List<Node> input) { - List<Node> output = new ArrayList<>(input.size()); - - // first deal with nodes having a parent host - long nodesHavingParent = input.stream() - .filter(n -> n.parentHostname().isPresent()) - .collect(Collectors.counting()); - if (nodesHavingParent > 0) { - // Make a map where each parent host maps to its list of child nodes - Map<String, List<Node>> byParentHosts = input.stream() - .filter(n -> n.parentHostname().isPresent()) - .collect(Collectors.groupingBy(n -> n.parentHostname().get())); - - // sort keys, those parent hosts with the most (remaining) ready nodes first - List<String> sortedParentHosts = byParentHosts - .keySet().stream() - .sorted((k1, k2) -> byParentHosts.get(k2).size() - byParentHosts.get(k1).size()) - .collect(Collectors.toList()); - while (nodesHavingParent > 0) { - // take one node from each parent host, round-robin. - for (String k : sortedParentHosts) { - List<Node> leftFromHost = byParentHosts.get(k); - if (! leftFromHost.isEmpty()) { - output.add(leftFromHost.remove(0)); - --nodesHavingParent; - } - } - } - } - - // now add non-VMs (nodes without a parent): - input.stream() - .filter(n -> ! n.parentHostname().isPresent()) - .forEach(n -> output.add(n)); - return output; - } - /** Used to manage a list of nodes during the node reservation process */ private class NodeList { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 6ba2c40186d..59601832e59 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester.createConfig; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 1cb27df3972..d62deac2855 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -45,32 +45,6 @@ public class VirtualNodeProvisioningTest { } @Test - public void optimize_sorts_by_more_vms() { - List<Node> readyList = new ArrayList<>(); - readyList.addAll(tester.makeReadyNodes(2, flavor)); - readyList.addAll(tester.makeReadyVirtualNodes(2, flavor, "parentHost1")); - readyList.addAll(tester.makeReadyNodes(2, flavor)); - readyList.addAll(tester.makeReadyVirtualNodes(3, flavor, "parentHost2")); - readyList.addAll(tester.makeReadyVirtualNodes(2, flavor, "parentHost3")); - readyList.addAll(tester.makeReadyVirtualNodes(1, flavor, "parentHost4")); - readyList.addAll(tester.makeReadyNodes(3, flavor)); - assertEquals(15, readyList.size()); - List<Node> optimized = GroupPreparer.stripeOverHosts(readyList); - assertEquals(15, optimized.size()); - assertEquals("parentHost2", optimized.get(0).parentHostname().get()); - assertEquals("parentHost4", optimized.get(3).parentHostname().get()); - assertEquals("parentHost2", optimized.get(4).parentHostname().get()); - assertEquals("parentHost2", optimized.get(7).parentHostname().get()); - assertEquals(false, optimized.get(8).parentHostname().isPresent()); - assertEquals(false, optimized.get(9).parentHostname().isPresent()); - assertEquals(false, optimized.get(10).parentHostname().isPresent()); - assertEquals(false, optimized.get(11).parentHostname().isPresent()); - assertEquals(false, optimized.get(12).parentHostname().isPresent()); - assertEquals(false, optimized.get(13).parentHostname().isPresent()); - assertEquals(false, optimized.get(14).parentHostname().isPresent()); - } - - @Test public void distinct_parent_host_for_each_node_in_a_cluster() { tester.makeReadyVirtualNodes(2, flavor, "parentHost1"); tester.makeReadyVirtualNodes(2, flavor, "parentHost2"); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json index 1c1adb10d2b..a510739a793 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json @@ -22,7 +22,7 @@ "clustertype": "container", "clusterid": "id1", "group": "0", - "index": 1, + "index": 0, "retired": false }, "restartGeneration": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json index 72d4ac75e72..ff6ae96ab50 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json @@ -22,7 +22,7 @@ "clustertype": "container", "clusterid": "id1", "group": "0", - "index": 0, + "index": 1, "retired": false }, "restartGeneration": 0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json index 105473e8247..65f9ae2866b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json @@ -22,7 +22,7 @@ "clustertype": "container", "clusterid": "id1", "group": "0", - "index": 0, + "index": 1, "retired": false }, "restartGeneration": 0, |