diff options
author | Morten Tokle <morten.tokle@gmail.com> | 2016-08-08 14:30:17 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-08 14:30:17 +0200 |
commit | 54776db1d91e71e8b26fe47c88976bef38d08cfd (patch) | |
tree | ec42418da9cf846dfadca17c7513f69b9530857a /node-repository | |
parent | e287de7bc17024d764f38f0827e1c2808418df92 (diff) |
Revert "VESPA-3446 Sort offered nodes by cost to make cheaper allocations"
Diffstat (limited to 'node-repository')
10 files changed, 39 insertions, 90 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 e37fff2161b..e1af3340f80 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 @@ -1,7 +1,6 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; -import com.google.common.collect.ComparisonChain; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; @@ -15,7 +14,6 @@ import com.yahoo.vespa.hosted.provision.node.Flavor; import java.time.Clock; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.HashSet; import java.util.LinkedHashSet; @@ -66,7 +64,7 @@ class GroupPreparer { if (nodeList.satisfied()) return nodeList.finalNodes(surplusActiveNodes); // Use active nodes from other groups that will otherwise be retired - List<Node> accepted = nodeList.offer(sortNodeListByCost(surplusActiveNodes), canChangeGroup); + List<Node> accepted = nodeList.offer(surplusActiveNodes, canChangeGroup); surplusActiveNodes.removeAll(accepted); if (nodeList.satisfied()) return nodeList.finalNodes(surplusActiveNodes); @@ -75,14 +73,14 @@ class GroupPreparer { if (nodeList.satisfied()) return nodeList.finalNodes(surplusActiveNodes); // Use inactive nodes - accepted = nodeList.offer(sortNodeListByCost(nodeRepository.getNodes(application, Node.State.inactive)), !canChangeGroup); + accepted = nodeList.offer(nodeRepository.getNodes(application, Node.State.inactive), !canChangeGroup); nodeList.update(nodeRepository.reserve(accepted)); if (nodeList.satisfied()) return nodeList.finalNodes(surplusActiveNodes); // 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(Node.Type.tenant, Node.State.ready); - accepted = nodeList.offer(stripeOverHosts(sortNodeListByCost(readyNodes)), !canChangeGroup); + accepted = nodeList.offer(stripeOverHosts(readyNodes), !canChangeGroup); nodeList.update(nodeRepository.reserve(accepted)); } if (nodeList.satisfied()) return nodeList.finalNodes(surplusActiveNodes); @@ -102,16 +100,6 @@ class GroupPreparer { } } - /** Sort nodes according to their cost, and if the cost is equal, sort by hostname (to get stable tests) */ - private List<Node> sortNodeListByCost(List<Node> nodeList) { - Collections.sort(nodeList, (n1, n2) -> ComparisonChain.start() - .compare(n1.configuration().flavor().cost(), n2.configuration().flavor().cost()) - .compare(n1.hostname(), n2.hostname()) - .result() - ); - 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()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionTest.java index e425505960a..3348177f750 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionTest.java @@ -22,7 +22,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; import java.util.HashSet; -import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -461,28 +460,6 @@ public class ProvisionTest { assertTrue( state2.hostByMembership("test", 1, 2).membership().get().retired()); } - @Test - public void application_deployment_allocates_cheapest_available() { - ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east"))); - tester.makeReadyNodes(6, "large"); //cost = 10 - tester.makeReadyNodes(6, "large-variant"); //cost = 9 - tester.makeReadyNodes(6, "large-variant-variant"); //cost = 11 - - ApplicationId applicationId = tester.makeApplicationId(); - ClusterSpec contentClusterSpec = ClusterSpec.from(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")); - ClusterSpec containerClusterSpec = ClusterSpec.from(ClusterSpec.Type.container, ClusterSpec.Id.from("myContainer")); - - - List<HostSpec> containerNodes = tester.prepare(applicationId, containerClusterSpec, 5, 1, "large"); //should be replaced by 5 large-variant - List<HostSpec> contentNodes = tester.prepare(applicationId, contentClusterSpec, 10, 1, "large"); // should give 1 large-variant, 6 large and 3 large-variant-variant - - tester.assertNumberOfNodesWithFlavor(containerNodes, "large-variant", 5); - tester.assertNumberOfNodesWithFlavor(contentNodes, "large-variant", 1); - tester.assertNumberOfNodesWithFlavor(contentNodes, "large", 6); - tester.assertNumberOfNodesWithFlavor(contentNodes, "large-variant-variant", 3); - } - - private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int group0Size, int group1Size, String flavor, ProvisioningTester tester) { // "deploy prepare" with a two container clusters and a storage cluster having of two groups ClusterSpec containerCluster0 = ClusterSpec.from(ClusterSpec.Type.container, ClusterSpec.Id.from("container0"), Optional.empty()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index a1733de848c..737c19cb3a7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -21,7 +21,6 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Configuration; -import com.yahoo.vespa.hosted.provision.node.Flavor; import com.yahoo.vespa.hosted.provision.node.NodeFlavors; import com.yahoo.vespa.hosted.provision.node.filter.NodeHostFilter; import com.yahoo.vespa.curator.transaction.CuratorTransaction; @@ -74,18 +73,18 @@ public class ProvisioningTester implements AutoCloseable { private NodeRepositoryConfig createConfig() { FlavorConfigBuilder b = new FlavorConfigBuilder(); - b.addFlavor("default", 2., 4., 100, "BARE_METAL").cost(3); - b.addFlavor("small", 1., 2., 50, "BARE_METAL").cost(2); - b.addFlavor("docker1", 1., 1., 10, "DOCKER_CONTAINER").cost(1); - b.addFlavor("v-4-8-100", 4., 8., 100, "VIRTUAL_MACHINE").cost(4); - b.addFlavor("old-large1", 2., 4., 100, "BARE_METAL").cost(6); - b.addFlavor("old-large2", 2., 5., 100, "BARE_METAL").cost(8); - NodeRepositoryConfig.Flavor.Builder large = b.addFlavor("large", 4., 8., 100, "BARE_METAL").cost(10); + b.addFlavor("default", 2., 4., 100, "BARE_METAL"); + b.addFlavor("small", 1., 2., 50, "BARE_METAL"); + b.addFlavor("docker1", 1., 1., 10, "DOCKER_CONTAINER"); + b.addFlavor("v-4-8-100", 4., 8., 100, "VIRTUAL_MACHINE"); + b.addFlavor("old-large1", 2., 4., 100, "BARE_METAL"); + b.addFlavor("old-large2", 2., 5., 100, "BARE_METAL"); + NodeRepositoryConfig.Flavor.Builder large = b.addFlavor("large", 4., 8., 100, "BARE_METAL"); b.addReplaces("old-large1", large); b.addReplaces("old-large2", large); - NodeRepositoryConfig.Flavor.Builder largeVariant = b.addFlavor("large-variant", 3., 9., 101, "BARE_METAL").cost(9); + NodeRepositoryConfig.Flavor.Builder largeVariant = b.addFlavor("large-variant", 3., 9., 101, "BARE_METAL"); b.addReplaces("large", largeVariant); - NodeRepositoryConfig.Flavor.Builder largeVariantVariant = b.addFlavor("large-variant-variant", 4., 9., 101, "BARE_METAL").cost(11); + NodeRepositoryConfig.Flavor.Builder largeVariantVariant = b.addFlavor("large-variant-variant", 4., 9., 101, "BARE_METAL"); b.addReplaces("large-variant", largeVariantVariant); return b.build(); } @@ -239,20 +238,6 @@ public class ProvisioningTester implements AutoCloseable { return hosts.stream().filter(host -> ! host.membership().get().retired()).collect(Collectors.toList()); } - public void assertNumberOfNodesWithFlavor(List<HostSpec> hostSpecs, String flavor, int expectedCount) { - long actualNodesWithFlavor = hostSpecs.stream() - .map(HostSpec::hostname) - .map(this::getNodeFlavor) - .map(Flavor::name) - .filter(name -> name.equals(flavor)) - .count(); - assertEquals(expectedCount, actualNodesWithFlavor); - } - - private Flavor getNodeFlavor(String hostname) { - return nodeRepository.getNode(hostname).map(Node::configuration).map(Configuration::flavor).orElseThrow(() -> new RuntimeException("No flavor for host " + hostname)); - } - private static class NullProvisionLogger implements ProvisionLogger { @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v1/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v1/RestApiTest.java index b425a46b422..fe027bddae0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v1/RestApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v1/RestApiTest.java @@ -48,8 +48,7 @@ public class RestApiTest { public void testTopLevelRequest() throws Exception { try (JDisc container = JDisc.fromServicesXml(servicesXml, Networking.disable)) { Response response = container.handleRequest(new Request("http://localhost:8080/nodes/v1/")); - - assertEquals("{\"provisioned\":[{\"id\":\"node6\",\"hostname\":\"host6.yahoo.com\",\"flavor\":\"default\"}],\"reserved\":[{\"id\":\"node2\",\"hostname\":\"host2.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":1,\"retired\":false},\"restartGeneration\":0},{\"id\":\"node1\",\"hostname\":\"host1.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":0,\"retired\":false},\"restartGeneration\":0}],\"active\":[{\"id\":\"node3\",\"hostname\":\"host3.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant2\",\"application\":\"application2\",\"instance\":\"instance2\"},\"membership\":{\"clustertype\":\"content\",\"clusterid\":\"id2\",\"index\":0,\"retired\":false},\"restartGeneration\":0},{\"id\":\"node4\",\"hostname\":\"host4.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant2\",\"application\":\"application2\",\"instance\":\"instance2\"},\"membership\":{\"clustertype\":\"content\",\"clusterid\":\"id2\",\"index\":1,\"retired\":false},\"restartGeneration\":0}],\"failed\":[{\"id\":\"node5\",\"hostname\":\"host5.yahoo.com\",\"flavor\":\"default\"}]}", + assertEquals("{\"provisioned\":[{\"id\":\"node6\",\"hostname\":\"host6.yahoo.com\",\"flavor\":\"default\"}],\"reserved\":[{\"id\":\"node3\",\"hostname\":\"host3.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":0,\"retired\":false},\"restartGeneration\":0},{\"id\":\"node2\",\"hostname\":\"host2.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":1,\"retired\":false},\"restartGeneration\":0}],\"active\":[{\"id\":\"node1\",\"hostname\":\"host1.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant2\",\"application\":\"application2\",\"instance\":\"instance2\"},\"membership\":{\"clustertype\":\"content\",\"clusterid\":\"id2\",\"index\":0,\"retired\":false},\"restartGeneration\":0},{\"id\":\"node4\",\"hostname\":\"host4.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant2\",\"application\":\"application2\",\"instance\":\"instance2\"},\"membership\":{\"clustertype\":\"content\",\"clusterid\":\"id2\",\"index\":1,\"retired\":false},\"restartGeneration\":0}],\"failed\":[{\"id\":\"node5\",\"hostname\":\"host5.yahoo.com\",\"flavor\":\"default\"}]}", response.getBodyAsString()); } } @@ -57,8 +56,8 @@ public class RestApiTest { @Test public void testSingleNodeRequest() throws Exception { try (JDisc container = JDisc.fromServicesXml(servicesXml, Networking.disable)) { - Response response1 = container.handleRequest(new Request("http://localhost:8080/nodes/v1/?hostname=host1.yahoo.com")); - assertEquals("{\"reserved\":[{\"id\":\"node1\",\"hostname\":\"host1.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":0,\"retired\":false},\"restartGeneration\":0}]}", + Response response1 = container.handleRequest(new Request("http://localhost:8080/nodes/v1/?hostname=host3.yahoo.com")); + assertEquals("{\"reserved\":[{\"id\":\"node3\",\"hostname\":\"host3.yahoo.com\",\"flavor\":\"default\",\"owner\":{\"tenant\":\"tenant1\",\"application\":\"application1\",\"instance\":\"instance1\"},\"membership\":{\"clustertype\":\"container\",\"clusterid\":\"id1\",\"index\":0,\"retired\":false},\"restartGeneration\":0}]}", response1.getBodyAsString()); Response response2 = container.handleRequest(new Request("http://localhost:8080/nodes/v1/?hostname=host6.yahoo.com")); 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 d70fbc299c4..2ead88c3dd4 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 @@ -39,20 +39,20 @@ public class RestApiTest { assertFile(new Request("http://localhost:8080/nodes/v2/node/host2.yahoo.com"), "node2.json"); // GET with filters - assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&hostname=host2.yahoo.com%20host1.yahoo.com"), "application2-nodes.json"); + assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&hostname=host3.yahoo.com%20host6.yahoo.com"), "application2-nodes.json"); assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&clusterType=content"), "active-nodes.json"); assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&clusterId=id2"), "application2-nodes.json"); assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&application=tenant2.application2.instance2"), "application2-nodes.json"); assertFile(new Request("http://localhost:8080/nodes/v2/node/?recursive=true&parentHost=parent.yahoo.com,parent.host.yahoo.com"), "parent-nodes.json"); // POST restart command - assertRestart(1, new Request("http://localhost:8080/nodes/v2/command/restart?hostname=host2.yahoo.com", + assertRestart(1, new Request("http://localhost:8080/nodes/v2/command/restart?hostname=host3.yahoo.com", new byte[0], Request.Method.POST)); assertRestart(2, new Request("http://localhost:8080/nodes/v2/command/restart?application=tenant2.application2.instance2", new byte[0], Request.Method.POST)); assertRestart(4, new Request("http://localhost:8080/nodes/v2/command/restart", new byte[0], Request.Method.POST)); - assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/host2.yahoo.com"), + assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/host3.yahoo.com"), "\"restartGeneration\":3"); // POST reboot command @@ -62,7 +62,7 @@ public class RestApiTest { new byte[0], Request.Method.POST)); assertReboot(9, new Request("http://localhost:8080/nodes/v2/command/reboot", new byte[0], Request.Method.POST)); - assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/host2.yahoo.com"), + assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/host3.yahoo.com"), "\"rebootGeneration\":3"); // POST new nodes diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/application2-nodes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/application2-nodes.json index 1d4d97315cd..f1285766c1b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/application2-nodes.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/application2-nodes.json @@ -1,6 +1,6 @@ { "nodes": [ - @include(node2.json), - @include(node1.json) + @include(node6.json), + @include(node3.json) ] }
\ No newline at end of file diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node1.json index f73668624b4..eeb86038e52 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node1.json @@ -13,15 +13,15 @@ "canonicalFlavor": "default", "environment":"env", "owner": { - "tenant": "tenant2", - "application": "application2", - "instance": "instance2" + "tenant": "tenant3", + "application": "application3", + "instance": "instance3" }, "membership": { "clustertype": "content", - "clusterid": "id2", + "clusterid": "id3", "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/node2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json index 387d78a231a..dddba8ccc44 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json @@ -13,15 +13,15 @@ "canonicalFlavor": "default", "environment":"env", "owner": { - "tenant": "tenant2", - "application": "application2", - "instance": "instance2" + "tenant": "tenant3", + "application": "application3", + "instance": "instance3" }, "membership": { "clustertype": "content", - "clusterid": "id2", + "clusterid": "id3", "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/node3.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json index b7c30a932ea..5f50d4b9bb1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json @@ -10,13 +10,13 @@ "canonicalFlavor":"default", "cost":200, "owner": { - "tenant": "tenant3", - "application": "application3", - "instance": "instance3" + "tenant": "tenant2", + "application": "application2", + "instance": "instance2" }, "membership": { "clustertype": "content", - "clusterid": "id3", + "clusterid": "id2", "group": "0", "index": 1, "retired": false diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json index 328e24d430b..378347d9ac2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json @@ -13,13 +13,13 @@ "canonicalFlavor": "default", "environment":"env", "owner": { - "tenant": "tenant3", - "application": "application3", - "instance": "instance3" + "tenant": "tenant2", + "application": "application2", + "instance": "instance2" }, "membership": { "clustertype": "content", - "clusterid": "id3", + "clusterid": "id2", "group": "0", "index": 0, "retired": false |