diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-12-29 16:45:48 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2022-12-29 16:45:48 +0100 |
commit | 2279ce96234d01cf039a319ce390a96e431437cc (patch) | |
tree | d4cee03eb393fe56b33511fe505999dfa181d72a /node-repository/src/main/java/com | |
parent | 6391f836b21b2d22510b70d554863d432536a59b (diff) |
Get rid of Advice.present
Diffstat (limited to 'node-repository/src/main/java/com')
3 files changed, 22 insertions, 27 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java index c816abc060c..0d643dc070b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java @@ -124,13 +124,11 @@ public class Autoscaler { public static class Advice { - private final boolean present; private final Optional<ClusterResources> target; private final AutoscalingStatus reason; - private Advice(Optional<ClusterResources> target, boolean present, AutoscalingStatus reason) { + private Advice(Optional<ClusterResources> target, AutoscalingStatus reason) { this.target = target; - this.present = present; this.reason = Objects.requireNonNull(reason); } @@ -140,33 +138,26 @@ public class Autoscaler { */ public Optional<ClusterResources> target() { return target; } - /** True if this does not provide any advice */ - public boolean isEmpty() { return ! present; } - - /** True if this provides advice (which may be to keep the current allocation) */ - public boolean isPresent() { return present; } - /** The reason for this advice */ public AutoscalingStatus reason() { return reason; } private static Advice none(Status status, String description) { - return new Advice(Optional.empty(), false, new AutoscalingStatus(status, description)); + return new Advice(Optional.empty(), new AutoscalingStatus(status, description)); } private static Advice dontScale(Status status, String description) { - return new Advice(Optional.empty(), true, new AutoscalingStatus(status, description)); + return new Advice(Optional.empty(), new AutoscalingStatus(status, description)); } private static Advice scaleTo(ClusterResources target) { - return new Advice(Optional.of(target), true, + return new Advice(Optional.of(target), new AutoscalingStatus(AutoscalingStatus.Status.rescaling, "Rescaling initiated due to load changes")); } @Override public String toString() { - return "autoscaling advice: " + - (present ? (target.isPresent() ? "Scale to " + target.get() : "Don't scale") : "None"); + return "autoscaling advice: " + (target.isPresent() ? "Scale to " + target.get() : "Don't scale"); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index 65f6eecf790..7eedad98697 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -87,7 +87,7 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { applications().put(application.get().with(updatedCluster), lock); var current = new AllocatableClusterResources(clusterNodes, nodeRepository()).advertisedResources(); - if (advice.isPresent() && advice.target().isPresent() && !current.equals(advice.target().get())) { + if (advice.target().isPresent() && !current.equals(advice.target().get())) { // 2. Also autoscale try (MaintenanceDeployment deployment = new MaintenanceDeployment(applicationId, deployer, metric, nodeRepository())) { if (deployment.isValid()) { @@ -101,7 +101,7 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { private boolean anyChanges(Autoscaler.Advice advice, Cluster cluster, Cluster updatedCluster, NodeList clusterNodes) { if (updatedCluster != cluster) return true; - if (advice.isPresent() && !cluster.target().resources().equals(advice.target())) return true; + if (!cluster.target().resources().equals(advice.target())) return true; if ( ! advice.reason().equals(cluster.target().status())) return true; if (advice.target().isPresent() && !advice.target().get().equals(new AllocatableClusterResources(clusterNodes, nodeRepository()).advertisedResources())) return true; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java index 0201ffae103..248329966cc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java @@ -64,11 +64,14 @@ public class ScalingSuggestionsMaintainer extends NodeRepositoryMaintainer { Optional<Cluster> cluster = application.cluster(clusterId); if (cluster.isEmpty()) return true; var suggestion = autoscaler.suggest(application, cluster.get(), clusterNodes); - if (suggestion.isEmpty()) return true; + if (suggestion.reason().status() == AutoscalingStatus.Status.waiting) return true; + + // empty suggested resources == keep the current allocation + var suggestedResources = suggestion.target().orElse(clusterNodes.not().retired().toResources()); + if ( ! shouldUpdateSuggestion(cluster.get().suggested(), suggestedResources)) return true; + // Wait only a short time for the lock to avoid interfering with change deployments try (Mutex lock = nodeRepository().applications().lock(applicationId, Duration.ofSeconds(1))) { - // empty suggested resources == keep the current allocation, so we record that - var suggestedResources = suggestion.target().orElse(clusterNodes.not().retired().toResources()); applications().get(applicationId).ifPresent(a -> updateSuggestion(suggestedResources, clusterId, a, lock)); return true; } @@ -77,20 +80,21 @@ public class ScalingSuggestionsMaintainer extends NodeRepositoryMaintainer { } } + private boolean shouldUpdateSuggestion(Autoscaling currentSuggestion, ClusterResources suggestedResources) { + return currentSuggestion.resources().isEmpty() + || currentSuggestion.at().isBefore(nodeRepository().clock().instant().minus(Duration.ofDays(7))) + || isHigher(suggestedResources, currentSuggestion.resources().get()); + } + private void updateSuggestion(ClusterResources suggestion, ClusterSpec.Id clusterId, Application application, Mutex lock) { Optional<Cluster> cluster = application.cluster(clusterId); if (cluster.isEmpty()) return; - var at = nodeRepository().clock().instant(); - var currentSuggestion = cluster.get().suggested(); - if (currentSuggestion.resources().isEmpty() - || currentSuggestion.at().isBefore(at.minus(Duration.ofDays(7))) - || isHigher(suggestion, currentSuggestion.resources().get())) - applications().put(application.with(cluster.get().withSuggested(new Autoscaling(suggestion, - AutoscalingStatus.empty(), - at))), lock); + applications().put(application.with(cluster.get().withSuggested(new Autoscaling(suggestion, + AutoscalingStatus.empty(), + nodeRepository().clock().instant()))), lock); } private boolean isHigher(ClusterResources r1, ClusterResources r2) { |