diff options
author | Harald Musum <musum@verizonmedia.com> | 2021-06-11 14:59:24 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-11 14:59:24 +0200 |
commit | 85700280cb9229a43e5034c3133f119f038d9afc (patch) | |
tree | 665a2ee1e848819d3c9e4b932201cfc13b27c134 /node-repository | |
parent | 6b7b83d67cec6bd6193059f393e5fcf5fcc9ed57 (diff) |
Revert "Log load balancer state transitions"
Diffstat (limited to 'node-repository')
3 files changed, 51 insertions, 57 deletions
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 c3d6f5c42b8..ac9d8d6671a 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 @@ -65,7 +65,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { Instant now = nodeRepository().clock().instant(); Instant expiry = now.minus(reservedExpiry); patchLoadBalancers(lb -> lb.state() == State.reserved && lb.changedAt().isBefore(expiry), - lb -> db.writeLoadBalancer(lb.with(State.inactive, now), lb.state())); + lb -> db.writeLoadBalancer(lb.with(State.inactive, now))); } /** Deprovision inactive load balancers that have expired */ @@ -114,7 +114,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { attempts.add(1); LOG.log(Level.INFO, () -> "Removing reals from inactive load balancer " + lb.id() + ": " + Sets.difference(lb.instance().get().reals(), reals)); service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals), true); - db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals))), lb.state()); + db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals)))); } catch (Exception e) { failed.add(lb.id()); lastException.set(e); 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 0205cc6c818..364af6308a1 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 @@ -482,29 +482,18 @@ public class CuratorDatabaseClient { return read(loadBalancerPath(id), LoadBalancerSerializer::fromJson); } - public void writeLoadBalancer(LoadBalancer loadBalancer, LoadBalancer.State fromState) { + public void writeLoadBalancer(LoadBalancer loadBalancer) { NestedTransaction transaction = new NestedTransaction(); - writeLoadBalancers(List.of(loadBalancer), fromState, transaction); + writeLoadBalancers(List.of(loadBalancer), transaction); transaction.commit(); } - public void writeLoadBalancers(Collection<LoadBalancer> loadBalancers, LoadBalancer.State fromState, NestedTransaction transaction) { + public void writeLoadBalancers(Collection<LoadBalancer> loadBalancers, NestedTransaction transaction) { CuratorTransaction curatorTransaction = db.newCuratorTransactionIn(transaction); loadBalancers.forEach(loadBalancer -> { curatorTransaction.add(createOrSet(loadBalancerPath(loadBalancer.id()), LoadBalancerSerializer.toJson(loadBalancer))); }); - transaction.onCommitted(() -> { - for (var lb : loadBalancers) { - if (lb.state() == fromState) continue; - if (fromState == null) { - log.log(Level.INFO, () -> "Creating " + lb.id() + " in " + lb.state()); - } else { - log.log(Level.INFO, () -> "Moving " + lb.id() + " from " + fromState + - " to " + lb.state()); - } - } - }); } public void removeLoadBalancer(LoadBalancerId loadBalancer) { 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 71884829b62..c114aa58a05 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 @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.transaction.NestedTransaction; @@ -60,7 +61,7 @@ public class LoadBalancerProvisioner { for (var id : db.readLoadBalancerIds()) { try (var lock = db.lock(id.application())) { var loadBalancer = db.readLoadBalancer(id); - loadBalancer.ifPresent(lb -> db.writeLoadBalancer(lb, lb.state())); + loadBalancer.ifPresent(db::writeLoadBalancer); } } } @@ -80,9 +81,11 @@ public class LoadBalancerProvisioner { if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); - LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); NodeList nodes = nodesOf(clusterId, application); - prepare(loadBalancerId, nodes); + LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); + ApplicationTransaction transaction = new ApplicationTransaction(new ProvisionLock(application, lock), new NestedTransaction()); + provision(transaction, loadBalancerId, nodes, false); + transaction.nested().commit(); } } @@ -97,15 +100,14 @@ public class LoadBalancerProvisioner { * Calling this when no load balancer has been prepared for given cluster is a no-op. */ public void activate(Set<ClusterSpec> clusters, ApplicationTransaction transaction) { - Set<ClusterSpec.Id> activatingClusters = clusters.stream() - .map(LoadBalancerProvisioner::effectiveId) - .collect(Collectors.toSet()); for (var cluster : loadBalancedClustersOf(transaction.application()).entrySet()) { - if (!activatingClusters.contains(cluster.getKey())) continue; - activate(transaction, cluster.getKey(), cluster.getValue()); + // Provision again to ensure that load balancer instance is re-configured with correct nodes + provision(transaction, cluster.getKey(), cluster.getValue()); } // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed - var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), activatingClusters); + var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), clusters.stream() + .map(LoadBalancerProvisioner::effectiveId) + .collect(Collectors.toSet())); deactivate(surplusLoadBalancers, transaction.nested()); } @@ -138,7 +140,7 @@ public class LoadBalancerProvisioner { var deactivatedLoadBalancers = loadBalancers.stream() .map(lb -> lb.with(LoadBalancer.State.inactive, now)) .collect(Collectors.toList()); - db.writeLoadBalancers(deactivatedLoadBalancers, LoadBalancer.State.active, transaction); + db.writeLoadBalancers(deactivatedLoadBalancers, transaction); } /** Find all load balancer IDs owned by given tenant and application */ @@ -163,41 +165,52 @@ public class LoadBalancerProvisioner { return loadBalancerId; } - private void prepare(LoadBalancerId id, NodeList nodes) { + /** Idempotently provision a load balancer for given application and cluster */ + private void provision(ApplicationTransaction transaction, LoadBalancerId id, NodeList nodes, boolean activate) { Instant now = nodeRepository.clock().instant(); Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); - Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer); + if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared + + Set<Real> reals = realsOf(nodes); + Optional<LoadBalancerInstance> instance = provisionInstance(id, reals, loadBalancer); LoadBalancer newLoadBalancer; - LoadBalancer.State fromState = null; if (loadBalancer.isEmpty()) { newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now); } else { - newLoadBalancer = loadBalancer.get().with(instance); - fromState = newLoadBalancer.state(); + LoadBalancer.State state = activate && instance.isPresent() + ? LoadBalancer.State.active + : loadBalancer.get().state(); + newLoadBalancer = loadBalancer.get().with(instance).with(state, now); + if (loadBalancer.get().state() != newLoadBalancer.state()) { + log.log(Level.INFO, () -> "Moving " + newLoadBalancer.id() + " from " + loadBalancer.get().state() + + " to " + newLoadBalancer.state()); + } } - // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers - db.writeLoadBalancer(newLoadBalancer, fromState); - requireInstance(id, instance); - } - private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, NodeList nodes) { - Instant now = nodeRepository.clock().instant(); - LoadBalancerId id = new LoadBalancerId(transaction.application(), cluster); - Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); - if (loadBalancer.isEmpty()) throw new IllegalArgumentException("Could not active load balancer that was never prepared: " + id); + if (activate) { + db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested()); + } else { + // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers + db.writeLoadBalancer(newLoadBalancer); + } - Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer); - LoadBalancer.State state = instance.isPresent() ? LoadBalancer.State.active : loadBalancer.get().state(); - LoadBalancer newLoadBalancer = loadBalancer.get().with(instance).with(state, now); - db.writeLoadBalancers(List.of(newLoadBalancer), loadBalancer.get().state(), transaction.nested()); - requireInstance(id, instance); + // Signal that load balancer is not ready yet + if (instance.isEmpty()) { + throw new LoadBalancerServiceException("Could not (re)configure " + id + ", targeting: " + + reals + ". The operation will be retried on next deployment", + null); + } + } + + private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) { + provision(transaction, new LoadBalancerId(transaction.application(), clusterId), nodes, true); } /** Provision or reconfigure a load balancer instance, if necessary */ - private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, NodeList nodes, Optional<LoadBalancer> currentLoadBalancer) { - Set<Real> reals = realsOf(nodes); + private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Set<Real> reals, + Optional<LoadBalancer> currentLoadBalancer) { if (hasReals(currentLoadBalancer, reals)) return currentLoadBalancer.get().instance(); - log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals); + log.log(Level.INFO, () -> "Creating " + id + ", targeting: " + reals); try { return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals), allowEmptyReals(currentLoadBalancer))); @@ -228,7 +241,7 @@ public class LoadBalancerProvisioner { /** Returns real servers for given nodes */ private Set<Real> realsOf(NodeList nodes) { - Set<Real> reals = new LinkedHashSet<Real>(); + var reals = new LinkedHashSet<Real>(); for (var node : nodes) { for (var ip : reachableIpAddresses(node)) { reals.add(new Real(HostName.from(node.hostname()), ip)); @@ -276,14 +289,6 @@ public class LoadBalancerProvisioner { return reachable; } - private static void requireInstance(LoadBalancerId id, Optional<LoadBalancerInstance> instance) { - if (instance.isEmpty()) { - // Signal that load balancer is not ready yet - throw new LoadBalancerServiceException("Could not (re)configure " + id + ". The operation will be retried on next deployment", - null); - } - } - private static ClusterSpec.Id effectiveId(ClusterSpec cluster) { return cluster.combinedId().orElse(cluster.id()); } |