diff options
author | Martin Polden <mpolden@mpolden.no> | 2019-12-18 16:03:13 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2019-12-19 10:31:52 +0100 |
commit | c5e77a5d27cea6f96166c9beca6149c15f947bb9 (patch) | |
tree | faf66f9b4d81e23b0aa6eb5b74a8c9a6678380d2 /node-repository | |
parent | 6838ebd5ad1bcc98669f6ecf112408bf441655ca (diff) |
Prune reals no longer allocated to application
Diffstat (limited to 'node-repository')
5 files changed, 59 insertions, 9 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 f86c05da3c2..682d1419a5c 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 @@ -193,7 +193,7 @@ public class NodeRepository extends AbstractComponent { /** Returns a filterable list of all load balancers in this repository */ public LoadBalancerList loadBalancers() { - return new LoadBalancerList(database().readLoadBalancers().values()); + return LoadBalancerList.copyOf(database().readLoadBalancers().values()); } public List<Node> getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java index c1b4fe9b0f2..1e60b689489 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java @@ -56,6 +56,11 @@ public class LoadBalancerInstance { return reals; } + /** Returns a copy of this with reals set to given reals */ + public LoadBalancerInstance withReals(Set<Real> reals) { + return new LoadBalancerInstance(hostname, dnsZone, ports, networks, reals); + } + private static Set<Integer> requirePorts(Set<Integer> ports) { Objects.requireNonNull(ports, "ports must be non-null"); if (ports.isEmpty()) { 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 7fd50bf0930..014d3df8d9a 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 @@ -20,7 +20,7 @@ public class LoadBalancerList implements Iterable<LoadBalancer> { private final List<LoadBalancer> loadBalancers; - public LoadBalancerList(Collection<LoadBalancer> loadBalancers) { + private LoadBalancerList(Collection<LoadBalancer> loadBalancers) { this.loadBalancers = List.copyOf(Objects.requireNonNull(loadBalancers, "loadBalancers must be non-null")); } @@ -47,6 +47,10 @@ public class LoadBalancerList implements Iterable<LoadBalancer> { return new LoadBalancerList(stream.collect(Collectors.toUnmodifiableList())); } + public static LoadBalancerList copyOf(Collection<LoadBalancer> loadBalancers) { + return new LoadBalancerList(loadBalancers); + } + @Override public Iterator<LoadBalancer> iterator() { return loadBalancers.iterator(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java index a8268fe7237..bcb0c901f14 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.log.LogLevel; +import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer.State; @@ -11,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; import java.time.Duration; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -44,8 +46,10 @@ public class LoadBalancerExpirer extends Maintainer { protected void maintain() { expireReserved(); removeInactive(); + pruneReals(); } + /** Move reserved load balancer that have expired to inactive */ private void expireReserved() { try (var lock = db.lockLoadBalancers()) { var now = nodeRepository().clock().instant(); @@ -57,8 +61,9 @@ public class LoadBalancerExpirer extends Maintainer { } } + /** Deprovision inactive load balancers that have expired */ private void removeInactive() { - List<LoadBalancerId> failed = new ArrayList<>(); + var failed = new ArrayList<LoadBalancerId>(); Exception lastException = null; try (var lock = db.lockLoadBalancers()) { var now = nodeRepository().clock().instant(); @@ -67,9 +72,7 @@ public class LoadBalancerExpirer extends Maintainer { .in(State.inactive) .changedBefore(expirationTime); for (var lb : expired) { - if (hasNodes(lb.id())) { // Defer removal if there are still nodes allocated - continue; - } + if (!allocatedNodes(lb.id()).isEmpty()) continue; // Defer removal if there are still nodes allocated to application try { service.remove(lb.id().application(), lb.id().cluster()); db.removeLoadBalancer(lb.id()); @@ -90,8 +93,39 @@ public class LoadBalancerExpirer extends Maintainer { } } - private boolean hasNodes(LoadBalancerId loadBalancer) { - return nodeRepository().list().owner(loadBalancer.application()).cluster(loadBalancer.cluster()).size() > 0; + /** Remove reals from inactive load balancers */ + private void pruneReals() { + var failed = new ArrayList<LoadBalancerId>(); + Exception lastException = null; + try (var lock = db.lockLoadBalancers()) { + var deactivated = nodeRepository().loadBalancers().in(State.inactive); + for (var lb : deactivated) { + var allocatedNodes = allocatedNodes(lb.id()).stream().map(Node::hostname).collect(Collectors.toSet()); + var reals = new LinkedHashSet<>(lb.instance().reals()); + // Remove any real no longer allocated to this application + reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value())); + try { + service.create(lb.id().application(), lb.id().cluster(), reals, true); + db.writeLoadBalancer(lb.with(lb.instance().withReals(reals))); + } catch (Exception e) { + failed.add(lb.id()); + lastException = e; + } + } + } + if (!failed.isEmpty()) { + log.log(LogLevel.WARNING, String.format("Failed to remove reals from %d load balancers: %s, retrying in %s", + failed.size(), + failed.stream() + .map(LoadBalancerId::serializedForm) + .collect(Collectors.joining(", ")), + interval()), + lastException); + } + } + + private List<Node> allocatedNodes(LoadBalancerId loadBalancer) { + return nodeRepository().list().owner(loadBalancer.application()).cluster(loadBalancer.cluster()).asList(); } } 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 75224021464..12b48fd7a35 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 @@ -17,6 +17,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -60,9 +61,15 @@ public class LoadBalancerExpirerTest { // Expirer defers removal while nodes are still allocated to application expirer.maintain(); assertEquals(3, tester.loadBalancerService().instances().size()); + assertEquals(2, tester.loadBalancerService().instances().get(lb1).reals().size()); dirtyNodesOf(app1, cluster1); - // Expirer defers removal until expiration time passes + // 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()); + + // Expirer defers removal of load balancer until expiration time passes expirer.maintain(); assertTrue("Inactive load balancer not removed", tester.loadBalancerService().instances().containsKey(lb1)); |