diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-02-12 13:32:26 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2020-02-12 14:38:45 +0100 |
commit | b2df0af1d3e4cbb888e0cbe425f9994dcafbacf9 (patch) | |
tree | 66f84af876a194fc4b25a44c5e01b068d91c936d /node-repository | |
parent | 5d657d4537d753b09f9cedce19f785c35eba2d80 (diff) |
Limit reads to load balancers owned by application
Previously this would read *all* load balancers and then filter on owner
afterwards, which is unnecessarily expensive.
Diffstat (limited to 'node-repository')
7 files changed, 64 insertions, 44 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 7d875434e1c..48a53104f7f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision; import com.google.inject.Inject; @@ -17,6 +17,7 @@ import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList; import com.yahoo.vespa.hosted.provision.maintenance.InfrastructureVersions; @@ -50,6 +51,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.function.BiFunction; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -194,7 +196,16 @@ public class NodeRepository extends AbstractComponent { /** Returns a filterable list of all load balancers in this repository */ public LoadBalancerList loadBalancers() { - return LoadBalancerList.copyOf(database().readLoadBalancers().values()); + return loadBalancers((ignored) -> true); + } + + /** Returns a filterable list of load balancers belonging to given application */ + public LoadBalancerList loadBalancers(ApplicationId application) { + return loadBalancers((id) -> id.application().equals(application)); + } + + private LoadBalancerList loadBalancers(Predicate<LoadBalancerId> predicate) { + return LoadBalancerList.copyOf(db.readLoadBalancers(predicate).values()); } public List<Node> getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); } @@ -221,7 +232,7 @@ public class NodeRepository extends AbstractComponent { candidates.parentOf(node).ifPresent(trustedNodes::add); node.allocation().ifPresent(allocation -> { trustedNodes.addAll(candidates.owner(allocation.owner()).asList()); - loadBalancers.owner(allocation.owner()).asList().stream() + loadBalancers.asList().stream() .map(LoadBalancer::instance) .map(LoadBalancerInstance::networks) .forEach(trustedNetworks::addAll); @@ -293,7 +304,12 @@ public class NodeRepository extends AbstractComponent { */ public List<NodeAcl> getNodeAcls(Node node, boolean children) { NodeList candidates = list(); - LoadBalancerList loadBalancers = loadBalancers(); + LoadBalancerList loadBalancers; + if (node.allocation().isPresent()) { + loadBalancers = loadBalancers(node.allocation().get().owner()); + } else { + loadBalancers = LoadBalancerList.EMPTY; + } if (children) { return candidates.childrenOf(node).asList().stream() .map(childNode -> getNodeAcl(childNode, candidates, loadBalancers)) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java index 014d3df8d9a..870184b0d9e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java @@ -1,8 +1,6 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.lb; -import com.yahoo.config.provision.ApplicationId; - import java.time.Instant; import java.util.Collection; import java.util.Iterator; @@ -18,17 +16,14 @@ import java.util.stream.Stream; */ public class LoadBalancerList implements Iterable<LoadBalancer> { + public static LoadBalancerList EMPTY = new LoadBalancerList(List.of()); + private final List<LoadBalancer> loadBalancers; private LoadBalancerList(Collection<LoadBalancer> loadBalancers) { this.loadBalancers = List.copyOf(Objects.requireNonNull(loadBalancers, "loadBalancers must be non-null")); } - /** Returns the subset of load balancers owned by given application */ - public LoadBalancerList owner(ApplicationId application) { - return of(loadBalancers.stream().filter(lb -> lb.id().application().equals(application))); - } - /** Returns the subset of load balancers that are in given state */ public LoadBalancerList in(LoadBalancer.State state) { return of(loadBalancers.stream().filter(lb -> lb.state() == state)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index ce0bcc2e337..1b90b8d3461 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.persistence; import com.google.common.util.concurrent.UncheckedTimeoutException; @@ -38,6 +38,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -483,18 +484,16 @@ public class CuratorDatabaseClient { // Load balancers public List<LoadBalancerId> readLoadBalancerIds() { - return curatorDatabase.getChildren(loadBalancersRoot).stream() - .map(LoadBalancerId::fromSerializedForm) - .collect(Collectors.toUnmodifiableList()); + return readLoadBalancerIds((ignored) -> true); } - public Map<LoadBalancerId, LoadBalancer> readLoadBalancers() { - return readLoadBalancerIds().stream() - .map(this::readLoadBalancer) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(collectingAndThen(toMap(LoadBalancer::id, Function.identity()), - Collections::unmodifiableMap)); + public Map<LoadBalancerId, LoadBalancer> readLoadBalancers(Predicate<LoadBalancerId> filter) { + return readLoadBalancerIds(filter).stream() + .map(this::readLoadBalancer) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(collectingAndThen(toMap(LoadBalancer::id, Function.identity()), + Collections::unmodifiableMap)); } public Optional<LoadBalancer> readLoadBalancer(LoadBalancerId id) { @@ -530,6 +529,13 @@ public class CuratorDatabaseClient { return loadBalancersRoot.append(id.serializedForm()); } + private List<LoadBalancerId> readLoadBalancerIds(Predicate<LoadBalancerId> predicate) { + return curatorDatabase.getChildren(loadBalancersRoot).stream() + .map(LoadBalancerId::fromSerializedForm) + .filter(predicate) + .collect(Collectors.toUnmodifiableList()); + } + private Transaction.Operation createOrSet(Path path, byte[] data) { if (curatorDatabase.exists(path)) { return CuratorOperations.setData(path.getAbsolute(), data); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 1500154aa07..25f245120ea 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath 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.yahoo.config.provision.ApplicationId; @@ -109,15 +109,14 @@ public class LoadBalancerProvisioner { public void deactivate(ApplicationId application, NestedTransaction transaction) { try (var applicationLock = nodeRepository.lock(application)) { try (Mutex loadBalancersLock = db.lockLoadBalancers()) { - deactivate(nodeRepository.loadBalancers().owner(application).asList(), transaction); + deactivate(nodeRepository.loadBalancers(application).asList(), transaction); } } } /** Returns load balancers of given application that are no longer referenced by given clusters */ private List<LoadBalancer> surplusLoadBalancersOf(ApplicationId application, Set<ClusterSpec.Id> activeClusters) { - var activeLoadBalancersByCluster = nodeRepository.loadBalancers() - .owner(application) + var activeLoadBalancersByCluster = nodeRepository.loadBalancers(application) .in(LoadBalancer.State.active) .asList() .stream() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java index 9f8f4a804d1..3147d4caded 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.restapi.v2; import com.yahoo.config.provision.ApplicationId; @@ -37,10 +37,14 @@ public class LoadBalancersResponse extends HttpResponse { } private List<LoadBalancer> loadBalancers() { - LoadBalancerList loadBalancers = nodeRepository.loadBalancers(); - return application().map(loadBalancers::owner) - .map(LoadBalancerList::asList) - .orElseGet(loadBalancers::asList); + LoadBalancerList loadBalancers; + var application = application(); + if (application.isPresent()) { + loadBalancers = nodeRepository.loadBalancers(application.get()); + } else { + loadBalancers = nodeRepository.loadBalancers(); + } + return loadBalancers.asList(); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java index 12b48fd7a35..f0faf36fc5d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. 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.component.Vtag; @@ -39,7 +39,7 @@ public class LoadBalancerExpirerTest { LoadBalancerExpirer expirer = new LoadBalancerExpirer(tester.nodeRepository(), Duration.ofDays(1), tester.loadBalancerService()); - Supplier<Map<LoadBalancerId, LoadBalancer>> loadBalancers = () -> tester.nodeRepository().database().readLoadBalancers(); + Supplier<Map<LoadBalancerId, LoadBalancer>> loadBalancers = () -> tester.nodeRepository().database().readLoadBalancers((ignored) -> true); // Deploy two applications with a total of three load balancers ClusterSpec.Id cluster1 = ClusterSpec.Id.from("qrs"); @@ -67,7 +67,7 @@ public class LoadBalancerExpirerTest { // Expirer prunes reals before expiration time of load balancer itself expirer.maintain(); assertEquals(Set.of(), tester.loadBalancerService().instances().get(lb1).reals()); - assertEquals(Set.of(), tester.nodeRepository().loadBalancers().owner(lb1.application()).asList().get(0).instance().reals()); + assertEquals(Set.of(), tester.nodeRepository().loadBalancers(lb1.application()).asList().get(0).instance().reals()); // Expirer defers removal of load balancer until expiration time passes expirer.maintain(); @@ -103,7 +103,7 @@ public class LoadBalancerExpirerTest { LoadBalancerExpirer expirer = new LoadBalancerExpirer(tester.nodeRepository(), Duration.ofDays(1), tester.loadBalancerService()); - Supplier<Map<LoadBalancerId, LoadBalancer>> loadBalancers = () -> tester.nodeRepository().database().readLoadBalancers(); + Supplier<Map<LoadBalancerId, LoadBalancer>> loadBalancers = () -> tester.nodeRepository().database().readLoadBalancers((ignored) -> true); // Prepare application diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index a26e802dfe8..7031c368e1d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath 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.Iterators; @@ -47,8 +47,8 @@ public class LoadBalancerProvisionerTest { @Test public void provision_load_balancer() { - Supplier<List<LoadBalancer>> lbApp1 = () -> tester.nodeRepository().loadBalancers().owner(app1).asList(); - Supplier<List<LoadBalancer>> lbApp2 = () -> tester.nodeRepository().loadBalancers().owner(app2).asList(); + Supplier<List<LoadBalancer>> lbApp1 = () -> tester.nodeRepository().loadBalancers(app1).asList(); + Supplier<List<LoadBalancer>> lbApp2 = () -> tester.nodeRepository().loadBalancers(app2).asList(); ClusterSpec.Id containerCluster1 = ClusterSpec.Id.from("qrs1"); ClusterSpec.Id contentCluster = ClusterSpec.Id.from("content"); @@ -80,7 +80,7 @@ public class LoadBalancerProvisionerTest { tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.container, containerCluster1), clusterRequest(ClusterSpec.Type.content, contentCluster))); - LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers().owner(app1).asList().get(0); + LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers(app1).asList().get(0); assertEquals(2, loadBalancer.instance().reals().size()); assertTrue("Failed node is removed", loadBalancer.instance().reals().stream() .map(Real::hostname) @@ -158,7 +158,7 @@ public class LoadBalancerProvisionerTest { var nodes = prepare(app1, Capacity.fromCount(2, new NodeResources(1, 4, 10, 0.3), false, true), true, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs"))); - Supplier<LoadBalancer> lb = () -> tester.nodeRepository().loadBalancers().owner(app1).asList().get(0); + Supplier<LoadBalancer> lb = () -> tester.nodeRepository().loadBalancers(app1).asList().get(0); assertTrue("Load balancer provisioned with empty reals", tester.loadBalancerService().instances().get(lb.get().id()).reals().isEmpty()); assignIps(tester.nodeRepository().getNodes(app1)); tester.activate(app1, nodes); @@ -189,7 +189,7 @@ public class LoadBalancerProvisionerTest { clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("tenant-host")))); assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty()); - assertEquals(List.of(), tester.nodeRepository().loadBalancers().owner(infraApp1).asList()); + assertEquals(List.of(), tester.nodeRepository().loadBalancers(infraApp1).asList()); } @Test @@ -197,12 +197,12 @@ public class LoadBalancerProvisionerTest { tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.content, ClusterSpec.Id.from("tenant-host")))); assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty()); - assertEquals(List.of(), tester.nodeRepository().loadBalancers().owner(app1).asList()); + assertEquals(List.of(), tester.nodeRepository().loadBalancers(app1).asList()); } @Test public void provision_load_balancer_combined_cluster() { - Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().owner(app1).asList(); + Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(app1).asList(); ClusterSpec.Id cluster = ClusterSpec.Id.from("foo"); var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, cluster)); |