diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java index 16016815b7c6..6a81c17d362c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java @@ -122,6 +122,11 @@ public Optional lastScalingEvent() { return Optional.of(scalingEvents.get(scalingEvents.size() - 1)); } + /** Returns whether the last scaling event in this has yet to complete. */ + public boolean scalingInProgress() { + return lastScalingEvent().isPresent() && lastScalingEvent().get().completion().isEmpty(); + } + public Cluster withConfiguration(boolean exclusive, Capacity capacity) { return new Cluster(id, exclusive, capacity.minResources(), capacity.maxResources(), capacity.groupSize(), capacity.isRequired(), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/ScalingEvent.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/ScalingEvent.java index e88989514c41..91270f14fbbd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/ScalingEvent.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/ScalingEvent.java @@ -64,8 +64,7 @@ public ScalingEvent withCompletion(Instant completion) { @Override public boolean equals(Object o) { if (o == this) return true; - if ( ! (o instanceof ScalingEvent)) return true; - ScalingEvent other = (ScalingEvent)o; + if ( ! (o instanceof ScalingEvent other)) return true; if ( other.generation != this.generation) return false; if ( ! other.at.equals(this.at)) return false; if ( ! other.from.equals(this.from)) return false; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaling.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaling.java index 2cc43a1eb339..0c86108b36cf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaling.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaling.java @@ -155,7 +155,7 @@ public enum Status { /** The cluster should be rescaled further, but no better configuration is allowed by the current limits */ insufficient, - /** Rescaling of this cluster has been scheduled */ + /** This cluster should be rescaled */ rescaling } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java index 281d9efe51ab..2f9ad28a0722 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java @@ -243,8 +243,10 @@ private Load adjustQueryDependentIdealLoadByBcpGroupInfo(Load ideal) { } private boolean hasScaledIn(Duration period) { - return cluster.lastScalingEvent().map(event -> event.at()).orElse(Instant.MIN) - .isAfter(clock.instant().minus(period)); + if (cluster.lastScalingEvent().isEmpty()) return false; + var lastCompletion = cluster.lastScalingEvent().get().completion(); + if (lastCompletion.isEmpty()) return true; // Ongoing + return lastCompletion.get().isAfter(clock.instant().minus(period)); } private ClusterNodesTimeseries nodeTimeseries() { return nodeTimeseries; } 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 44944af15d7d..4c9fab748d17 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 @@ -74,6 +74,7 @@ private boolean autoscale(ApplicationId applicationId, ClusterSpec.Id clusterId) if (application.isEmpty()) return true; if (application.get().cluster(clusterId).isEmpty()) return true; Cluster cluster = application.get().cluster(clusterId).get(); + Cluster unchangedCluster = cluster; NodeList clusterNodes = nodeRepository().nodes().list(Node.State.active).owner(applicationId).cluster(clusterId); cluster = updateCompletion(cluster, clusterNodes); @@ -82,14 +83,15 @@ private boolean autoscale(ApplicationId applicationId, ClusterSpec.Id clusterId) // Autoscale unless an autoscaling is already in progress Autoscaling autoscaling = null; - if (cluster.target().resources().isEmpty() || current.equals(cluster.target().resources().get())) { + if (cluster.target().resources().isEmpty() && !cluster.scalingInProgress()) { autoscaling = autoscaler.autoscale(application.get(), cluster, clusterNodes); - if ( autoscaling.isPresent() || cluster.target().isEmpty()) // Ignore empty from recently started servers + if (autoscaling.isPresent() || cluster.target().isEmpty()) // Ignore empty from recently started servers cluster = cluster.withTarget(autoscaling); } - // Always store updates - applications().put(application.get().with(cluster), lock); + // Always store any updates + if (cluster != unchangedCluster) + applications().put(application.get().with(cluster), lock); // Attempt to perform the autoscaling immediately, and log it regardless if (autoscaling != null && autoscaling.resources().isPresent() && !current.equals(autoscaling.resources().get())) { @@ -127,7 +129,7 @@ private Cluster updateCompletion(Cluster cluster, NodeList clusterNodes) { if (clusterNodes.retired().stream() .anyMatch(node -> node.history().hasEventAt(History.Event.Type.retired, event.at()))) return cluster; - // - 2. all nodes have switched to the right config generation (currently only measured on containers) + // - 2. all nodes have switched to the right config generation for (var nodeTimeseries : nodeRepository().metricsDb().getNodeTimeseries(Duration.between(event.at(), clock().instant()), clusterNodes)) { Optional onNewGeneration = diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index d69d9267cfd5..f1cf33d04774 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -77,6 +77,11 @@ public void test_autoscaling_single_content_group() { fixture.tester().clock().advance(Duration.ofDays(2)); fixture.loader().applyCpuLoad(0.1f, 10); + assertTrue("Last scaling not completed", fixture.autoscale().resources().isEmpty()); + + fixture.completeLastScaling(); + fixture.tester().clock().advance(Duration.ofDays(7)); + fixture.loader().applyCpuLoad(0.1f, 10); fixture.tester().assertResources("Scaling cpu down since usage has gone down significantly", 8, 1, 1.0, 7.3, 22.1, fixture.autoscale()); @@ -243,14 +248,15 @@ public void test_autoscaling_single_container_group() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).clusterType(ClusterSpec.Type.container).build(); fixture.loader().applyCpuLoad(0.25f, 120); - ClusterResources scaledResources = fixture.tester().assertResources("Scaling cpu up", - 4, 1, 4, 16.0, 40.8, - fixture.autoscale()); + var scaledResources = fixture.tester().assertResources("Scaling cpu up", + 3, 1, 4, 16.0, 40.8, + fixture.autoscale()); fixture.deploy(Capacity.from(scaledResources)); fixture.deactivateRetired(Capacity.from(scaledResources)); + fixture.completeLastScaling(); fixture.loader().applyCpuLoad(0.1f, 120); fixture.tester().assertResources("Scaling down since cpu usage has gone down", - 3, 1, 4, 16, 30.6, + 3, 1, 2, 16, 27.2, fixture.autoscale()); } @@ -585,7 +591,7 @@ public void scaling_down_only_after_delay() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); fixture.loader().applyCpuLoad(0.02, 120); assertTrue("Too soon after initial deployment", fixture.autoscale().resources().isEmpty()); - fixture.tester().clock().advance(Duration.ofDays(2)); + fixture.tester().clock().advance(Duration.ofHours(12 * 3 + 1)); fixture.loader().applyCpuLoad(0.02, 120); fixture.tester().assertResources("Scaling down since enough time has passed", 3, 1, 1.0, 24.6, 101.4, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java index 48db3fade95a..5d1fd58489bd 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java @@ -55,6 +55,7 @@ public Fixture(Fixture.Builder builder, Optional initialResour var deployCapacity = initialResources.isPresent() ? Capacity.from(initialResources.get()) : capacity; tester.deploy(builder.application, builder.cluster, deployCapacity); this.loader = new Loader(this); + store(cluster().with(cluster().lastScalingEvent().get().withCompletion(tester.clock().instant()))); } public DynamicProvisioningTester tester() { return tester; } @@ -141,6 +142,16 @@ public void store(BcpGroupInfo bcpGroupInfo) { tester.nodeRepository().applications().put(application, tester.nodeRepository().applications().lock(applicationId)); } + public void store(Cluster cluster) { + var application = application(); + application = application.with(cluster); + tester.nodeRepository().applications().put(application, tester.nodeRepository().applications().lock(applicationId)); + } + + public void completeLastScaling() { + store(cluster().with(cluster().lastScalingEvent().get().withCompletion(tester().clock().instant()))); + } + public static class Builder { ApplicationId application = DynamicProvisioningTester.applicationId("application1"); @@ -162,7 +173,7 @@ public Fixture.Builder zone(Zone zone) { } /** Set to true to behave as if hosts are provisioned dynamically. */ - public Fixture. Builder dynamicProvisioning(boolean dynamicProvisioning) { + public Fixture.Builder dynamicProvisioning(boolean dynamicProvisioning) { this.zone = new Zone(Cloud.builder() .dynamicProvisioning(dynamicProvisioning) .allowHostSharing(zone.cloud().allowHostSharing()) @@ -174,7 +185,7 @@ public Fixture. Builder dynamicProvisioning(boolean dynamicProvisioning) { } /** Set to true to allow multiple nodes be provisioned on the same host. */ - public Fixture. Builder allowHostSharing(boolean allowHostSharing) { + public Fixture.Builder allowHostSharing(boolean allowHostSharing) { this.zone = new Zone(Cloud.builder() .dynamicProvisioning(zone.cloud().dynamicProvisioning()) .allowHostSharing(allowHostSharing) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java index 2786da4b69ed..e6c183d02ce9 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java @@ -58,10 +58,11 @@ public void testScalingSuggestionsMaintainer() { tester.deploy(app1, cluster1, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty())); + storeCompletion(app1, cluster1.id(), tester.nodeRepository()); tester.deploy(app2, cluster2, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), new ClusterResources(10, 1, new NodeResources(6.5, 5, 15, 0.1)), IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty())); - + storeCompletion(app2, cluster2.id(), tester.nodeRepository()); tester.clock().advance(Duration.ofHours(13)); Duration timeAdded = addMeasurements(0.90f, 0.90f, 0.90f, 0, 500, app1, tester.nodeRepository()); tester.clock().advance(timeAdded.negated()); @@ -109,6 +110,16 @@ public void testScalingSuggestionsMaintainer() { assertFalse("Suggestion is not made as it matches what we have", shouldSuggest(app1, cluster1, tester)); } + private void storeCompletion(ApplicationId appId, ClusterSpec.Id clusterId, NodeRepository nodeRepository) { + try (var lock = nodeRepository.applications().lock(appId)) { + var app = nodeRepository.applications().require(appId); + var cluster = app.cluster(clusterId).get(); + cluster = cluster.with(cluster.lastScalingEvent().get().withCompletion(nodeRepository.clock().instant())); + app = app.with(cluster); + nodeRepository.applications().put(app, lock); + } + } + private Autoscaling suggestionOf(ApplicationId app, ClusterSpec cluster, ProvisioningTester tester) { return tester.nodeRepository().applications().get(app).get().cluster(cluster.id()).get().suggested(); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java index a3fc6aae9ac2..92aa65eac997 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java @@ -48,9 +48,8 @@ public void optimize(RankingExpression expression, ContextIndex context, Optimiz */ private ExpressionNode findAndOptimize(ExpressionNode node) { ExpressionNode newNode = optimize(node); - if ( ! (newNode instanceof CompositeNode)) return newNode; // + if ( ! (newNode instanceof CompositeNode newComposite)) return newNode; - CompositeNode newComposite = (CompositeNode)newNode; List newChildren = new ArrayList<>(); for (ExpressionNode child : newComposite.children()) { newChildren.add(findAndOptimize(child)); @@ -84,10 +83,9 @@ private boolean optimize(ExpressionNode node, List forest) { currentTreesOptimized++; return true; } - if (!(node instanceof OperationNode)) { + if (!(node instanceof OperationNode aNode)) { return false; } - OperationNode aNode = (OperationNode)node; for (Operator op : aNode.operators()) { if (op != Operator.plus) { return false; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java index 7ba671e62ebb..eb79c8ba2c17 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java @@ -95,8 +95,7 @@ private ExpressionNode createGBDTNode(IfNode cNode,ContextIndex context) { */ private int consumeNode(ExpressionNode node, List values, ContextIndex context) { int beforeIndex = values.size(); - if ( node instanceof IfNode) { - IfNode ifNode = (IfNode)node; + if (node instanceof IfNode ifNode) { int jumpValueIndex = consumeIfCondition(ifNode.getCondition(), values, context); values.add(0d); // jumpValue goes here after the next line int jumpValue = consumeNode(ifNode.getTrueExpression(), values, context) + 1;