diff options
8 files changed, 122 insertions, 181 deletions
diff --git a/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java b/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java index 3a8d80e5ffe..43caf2d52fe 100644 --- a/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java +++ b/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java @@ -1,4 +1,4 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.benchmark; import com.yahoo.collections.Tuple2; @@ -33,6 +33,8 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ThreadLocalRandom; +import static com.yahoo.vespa.config.ConfigKey.createFull; + /** * A config client for generating load against a config server or config proxy. * <p> @@ -69,8 +71,7 @@ public class LoadTester { String configsList = parser.getBinarySwitches().get("-l"); String defPath = parser.getBinarySwitches().get("-dd"); debug = parser.getUnarySwitches().contains("-d"); - LoadTester loadTester = new LoadTester(); - loadTester.runLoad(host, port, iterations, threads, configsList, defPath); + new LoadTester().runLoad(host, port, iterations, threads, configsList, defPath); } private void runLoad(String host, int port, int iterations, int threads, @@ -97,14 +98,17 @@ public class LoadTester { private Map<ConfigDefinitionKey, Tuple2<String, String[]>> readDefs(String defPath) throws IOException { Map<ConfigDefinitionKey, Tuple2<String, String[]>> ret = new HashMap<>(); if (defPath == null) return ret; + File defDir = new File(defPath); if (!defDir.isDirectory()) { - System.out.println("# Given def file dir is not a directory: " + defDir.getPath() + " , will not send def contents in requests."); + System.out.println("# Given def file dir is not a directory: " + + defDir.getPath() + " , will not send def contents in requests."); return ret; } - final File[] files = defDir.listFiles(); + File[] files = defDir.listFiles(); if (files == null) { - System.out.println("# Given def file dir has no files: " + defDir.getPath() + " , will not send def contents in requests."); + System.out.println("# Given def file dir has no files: " + + defDir.getPath() + " , will not send def contents in requests."); return ret; } for (File f : files) { @@ -131,7 +135,7 @@ public class LoadTester { sb.append((metrics.failedRequests)); sb.append("\n"); sb.append('#').append(TransportMetrics.getInstance().snapshot().toString()).append('\n'); - System.out.println(sb.toString()); + System.out.println(sb); } private List<ConfigKey<?>> readConfigs(String configsList) throws IOException { @@ -189,10 +193,10 @@ public class LoadTester { private class LoadThread extends Thread { - int iterations = 0; - String host = ""; - int port = 0; - Metrics metrics = new Metrics(); + private final int iterations; + private final String host; + private final int port; + private final Metrics metrics = new Metrics(); LoadThread(int iterations, String host, int port) { this.iterations = iterations; @@ -204,44 +208,24 @@ public class LoadTester { public void run() { Spec spec = new Spec(host, port); Target target = connect(spec); - ConfigKey<?> reqKey; - JRTClientConfigRequest request; - int totConfs = configs.size(); - boolean reconnCycle = false; // to log reconn message only once, for instance at restart + for (int i = 0; i < iterations; i++) { - reqKey = configs.get(ThreadLocalRandom.current().nextInt(totConfs)); + ConfigKey<?> reqKey = configs.get(ThreadLocalRandom.current().nextInt(configs.size())); ConfigDefinitionKey dKey = new ConfigDefinitionKey(reqKey); Tuple2<String, String[]> defContent = defs.get(dKey); if (defContent == null && defs.size() > 0) { // Only complain if we actually did run with a def dir System.out.println("# No def found for " + dKey + ", not sending in request."); } - request = getRequest(ConfigKey.createFull(reqKey.getName(), reqKey.getConfigId(), reqKey.getNamespace(), defContent.first), defContent.second); + ConfigKey<?> configKey = createFull(reqKey.getName(), reqKey.getConfigId(), reqKey.getNamespace(), defContent.first); + JRTClientConfigRequest request = createRequest(configKey, defContent.second); if (debug) System.out.println("# Requesting: " + reqKey); long start = System.currentTimeMillis(); target.invokeSync(request.getRequest(), 10.0); long end = System.currentTimeMillis(); if (request.isError()) { - if ("Connection lost".equals(request.errorMessage()) || "Connection down".equals(request.errorMessage())) { - try { - Thread.sleep(100); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - if (!reconnCycle) { - System.out.println("# Connection lost, reconnecting..."); - reconnCycle = true; - } - target.close(); - target = connect(spec); - } else { - System.err.println(request.errorMessage()); - } - metrics.incFailedRequests(); + target = handleError(request, spec, target); } else { - if (reconnCycle) { - reconnCycle = false; - System.out.println("# Connection OK"); - } + System.out.println("# Connection OK"); long duration = end - start; if (debug) { @@ -255,7 +239,7 @@ public class LoadTester { } } - private JRTClientConfigRequest getRequest(ConfigKey<?> reqKey, String[] defContent) { + private JRTClientConfigRequest createRequest(ConfigKey<?> reqKey, String[] defContent) { if (defContent == null) defContent = new String[0]; final long serverTimeout = 1000; return JRTClientConfigRequestV3.createWithParams(reqKey, DefContent.fromList(Arrays.asList(defContent)), @@ -266,6 +250,24 @@ public class LoadTester { private Target connect(Spec spec) { return supervisor.connect(spec); } + + private Target handleError(JRTClientConfigRequest request, Spec spec, Target target) { + if (List.of("Connection lost", "Connection down").contains(request.errorMessage())) { + try { + Thread.sleep(100); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + System.out.println("# Connection lost, reconnecting..."); + target.close(); + target = connect(spec); + } else { + System.err.println(request.errorMessage()); + } + metrics.incFailedRequests(); + return target; + } + } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java index e1135063f97..d406cafc3b8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java @@ -127,8 +127,10 @@ public class ClusterDeploymentMetricsRetriever { case VESPA_CONTAINER: optionalDouble(values.field("query_latency.sum")).ifPresent(qlSum -> aggregator.get() - .addContainerLatency(qlSum, values.field("query_latency.count").asDouble()) - .addFeedLatency(values.field("feed.latency.sum").asDouble(), values.field("feed.latency.count").asDouble())); + .addContainerLatency(qlSum, values.field("query_latency.count").asDouble())); + optionalDouble(values.field("feed.latency.sum")).ifPresent(flSum -> + aggregator.get() + .addFeedLatency(flSum, values.field("feed.latency.count").asDouble())); break; case VESPA_QRSERVER: optionalDouble(values.field("query_latency.sum")).ifPresent(qlSum -> diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index cdb5202603a..96373bd764f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -67,7 +67,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { maintainers.add(new ScalingSuggestionsMaintainer(nodeRepository, defaults.scalingSuggestionsInterval, metric)); maintainers.add(new SwitchRebalancer(nodeRepository, defaults.switchRebalancerInterval, metric, deployer)); maintainers.add(new HostEncrypter(nodeRepository, defaults.hostEncrypterInterval, metric)); - maintainers.add(new ParkedExpirer(nodeRepository, defaults.parkedExpirerInterval, metric)); provisionServiceProvider.getLoadBalancerService(nodeRepository) .map(lbService -> new LoadBalancerExpirer(nodeRepository, defaults.loadBalancerExpirerInterval, lbService, metric)) @@ -120,7 +119,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration scalingSuggestionsInterval; private final Duration switchRebalancerInterval; private final Duration hostEncrypterInterval; - private final Duration parkedExpirerInterval; private final NodeFailer.ThrottlePolicy throttlePolicy; @@ -151,7 +149,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { throttlePolicy = NodeFailer.ThrottlePolicy.hosted; inactiveConfigServerExpiry = Duration.ofMinutes(5); inactiveControllerExpiry = Duration.ofMinutes(5); - parkedExpirerInterval = Duration.ofMinutes(30); if (zone.environment().isProduction() && ! zone.system().isCd()) { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirer.java deleted file mode 100644 index ec7826658e3..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirer.java +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.config.provision.NodeType; -import com.yahoo.jdisc.Metric; -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.Agent; -import com.yahoo.vespa.hosted.provision.node.History; - -import java.time.Duration; -import java.time.Instant; -import java.util.Comparator; -import java.util.logging.Logger; - -/** - * - * Expires parked nodes in dynamically provisioned zones. - * If number of parked hosts exceed MAX_ALLOWED_PARKED_HOSTS, recycle in a queue order - * - * @author olaa - */ -public class ParkedExpirer extends NodeRepositoryMaintainer { - - private static final int MAX_ALLOWED_PARKED_HOSTS = 20; - private static final Logger log = Logger.getLogger(ParkedExpirer.class.getName()); - - private final NodeRepository nodeRepository; - - ParkedExpirer(NodeRepository nodeRepository, Duration interval, Metric metric) { - super(nodeRepository, interval, metric); - this.nodeRepository = nodeRepository; - } - - @Override - protected double maintain() { - if (!nodeRepository.zone().getCloud().dynamicProvisioning()) - return 1.0; - - NodeList parkedHosts = nodeRepository.nodes() - .list(Node.State.parked) - .nodeType(NodeType.host) - .not().deprovisioning(); - int hostsToExpire = Math.max(0, parkedHosts.size() - MAX_ALLOWED_PARKED_HOSTS); - parkedHosts.sortedBy(Comparator.comparing(this::parkedAt)) - .first(hostsToExpire) - .forEach(host -> { - log.info("Allowed number of parked nodes exceeded. Recycling " + host.hostname()); - nodeRepository.nodes().deallocate(host, Agent.ParkedExpirer, "Expired by ParkedExpirer"); - }); - - return 1.0; - } - - private Instant parkedAt(Node node) { - return node.history().event(History.Event.Type.parked) - .map(History.Event::at) - .orElse(Instant.EPOCH); // Should not happen - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java index 856d534bbd2..76c8210338e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java @@ -4,15 +4,18 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; 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.Agent; import com.yahoo.vespa.hosted.provision.node.History; import java.time.Duration; +import java.time.Instant; import java.util.List; /** * This moves nodes of type {@link NodeType#host} from provisioned to parked if they have been in provisioned too long. + * Parked hosts are deprovisioned as well, if too many hosts are being expired. * * Only {@link NodeType#host} is moved because any number of nodes of that type can exist. Other node types such as * {@link NodeType#confighost} have a fixed number and thus cannot be replaced while the fixed number of nodes exist in @@ -22,17 +25,40 @@ import java.util.List; */ public class ProvisionedExpirer extends Expirer { + private final NodeRepository nodeRepository; + private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 20; + ProvisionedExpirer(NodeRepository nodeRepository, Duration dirtyTimeout, Metric metric) { super(Node.State.provisioned, History.Event.Type.provisioned, nodeRepository, dirtyTimeout, metric); + this.nodeRepository = nodeRepository; } @Override protected void expire(List<Node> expired) { + int previouslyExpired = numberOfPreviouslyExpired(); for (Node expiredNode : expired) { - if (expiredNode.type() == NodeType.host) { - nodeRepository().nodes().parkRecursively(expiredNode.hostname(), Agent.ProvisionedExpirer, "Node is stuck in provisioned"); + if (expiredNode.type() != NodeType.host) + continue; + nodeRepository().nodes().parkRecursively(expiredNode.hostname(), Agent.ProvisionedExpirer, "Node is stuck in provisioned"); + if (MAXIMUM_ALLOWED_EXPIRED_HOSTS < ++previouslyExpired) { + nodeRepository.nodes().deprovision(expiredNode.hostname(), Agent.ProvisionedExpirer, nodeRepository.clock().instant()); } } } + private int numberOfPreviouslyExpired() { + return nodeRepository.nodes() + .list(Node.State.parked) + .nodeType(NodeType.host) + .matching(this::parkedByProvisionedExpirer) + .not().deprovisioning() + .size(); + } + + private boolean parkedByProvisionedExpirer(Node node) { + return node.history().event(History.Event.Type.parked) + .map(History.Event::agent) + .map(Agent.ProvisionedExpirer::equals) + .orElse(false); + } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirerTest.java deleted file mode 100644 index bc60801c1d6..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ParkedExpirerTest.java +++ /dev/null @@ -1,71 +0,0 @@ -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.config.provision.Cloud; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; -import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.node.Agent; -import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; -import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; -import org.junit.Test; - -import java.time.Duration; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.IntStream; - -import static org.junit.Assert.*; - -/** - * @author olaa - */ -public class ParkedExpirerTest { - - private ProvisioningTester tester; - - @Test - public void noop_if_not_dynamic_provisioning() { - tester = getTester(false); - populateNodeRepo(); - - var expirer = new ParkedExpirer(tester.nodeRepository(), Duration.ofMinutes(4), new TestMetric()); - expirer.maintain(); - - assertEquals(0, tester.nodeRepository().nodes().list(Node.State.dirty).size()); - assertEquals(25, tester.nodeRepository().nodes().list(Node.State.parked).size()); - } - - @Test - public void recycles_correct_subset_of_parked_hosts() { - tester = getTester(true); - populateNodeRepo(); - - var expirer = new ParkedExpirer(tester.nodeRepository(), Duration.ofMinutes(4), new TestMetric()); - expirer.maintain(); - - assertEquals(4, tester.nodeRepository().nodes().list(Node.State.dirty).size()); - assertEquals(21, tester.nodeRepository().nodes().list(Node.State.parked).size()); - - } - - private ProvisioningTester getTester(boolean dynamicProvisioning) { - var zone = new Zone(Cloud.builder().dynamicProvisioning(dynamicProvisioning).build(), SystemName.main, Environment.prod, RegionName.from("us-east")); - return new ProvisioningTester.Builder().zone(zone) - .hostProvisioner(dynamicProvisioning ? new MockHostProvisioner(List.of()) : null) - .build(); - } - - private void populateNodeRepo() { - var nodes = IntStream.range(0, 25) - .mapToObj(i -> Node.create("id-" + i, "host-" + i, new Flavor(NodeResources.unspecified()), Node.State.parked, NodeType.host).build()) - .collect(Collectors.toList()); - tester.nodeRepository().database().addNodesInState(nodes, Node.State.parked, Agent.system); - tester.nodeRepository().nodes().deprovision(nodes.get(0).hostname(), Agent.system, tester.clock().instant()); // Deprovisioning host is not recycled - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java new file mode 100644 index 00000000000..786faae24b4 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java @@ -0,0 +1,50 @@ +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.Cloud; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; +import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; +import org.junit.Test; + +import java.time.Duration; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.Assert.*; + +/** + * @author olaa + */ +public class ProvisionedExpirerTest { + + private ProvisioningTester tester; + + @Test + public void deprovisions_hosts_if_excessive_expiry() { + tester = new ProvisioningTester.Builder().build(); + populateNodeRepo(); + + tester.clock().advance(Duration.ofMinutes(5)); + new ProvisionedExpirer(tester.nodeRepository(), Duration.ofMinutes(4), new TestMetric()).maintain(); + + assertEquals(5, tester.nodeRepository().nodes().list().deprovisioning().size()); + assertEquals(20, tester.nodeRepository().nodes().list().not().deprovisioning().size()); + } + + private void populateNodeRepo() { + var nodes = IntStream.range(0, 25) + .mapToObj(i -> Node.create("id-" + i, "host-" + i, new Flavor(NodeResources.unspecified()), Node.State.provisioned, NodeType.host).build()) + .collect(Collectors.toList()); + tester.nodeRepository().database().addNodesInState(nodes, Node.State.provisioned, Agent.system); + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json index 26d711945c6..72224ef3cba 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json @@ -43,9 +43,6 @@ "name": "OsUpgradeActivator" }, { - "name": "ParkedExpirer" - }, - { "name": "PeriodicApplicationMaintainer" }, { |