diff options
18 files changed, 172 insertions, 148 deletions
diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java index d804e50c67d..c52bf66626a 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java @@ -14,6 +14,7 @@ import com.yahoo.vespa.config.search.core.OnnxModelsConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import java.util.HashMap; @@ -261,11 +262,13 @@ public class ModelsEvaluationHandlerTest { "tensor(a[2],b[2],c{},d[2]):{a:[[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]]], b:[[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]]]}"); } + @Ignore @Test public void testMnistSavedEvaluateSpecificFunction() { assumeTrue(OnnxEvaluator.isRuntimeAvailable()); Map<String, String> properties = new HashMap<>(); properties.put("input", inputTensor()); + properties.put("format.tensors", "long"); String url = "http://localhost/model-evaluation/v1/mnist_saved/serving_default.y/eval"; String expected = "{\"cells\":[{\"address\":{\"d0\":\"0\",\"d1\":\"0\"},\"value\":-0.6319251673007533},{\"address\":{\"d0\":\"0\",\"d1\":\"1\"},\"value\":-7.577770600619843E-4},{\"address\":{\"d0\":\"0\",\"d1\":\"2\"},\"value\":-0.010707969042025622},{\"address\":{\"d0\":\"0\",\"d1\":\"3\"},\"value\":-0.6344759233540788},{\"address\":{\"d0\":\"0\",\"d1\":\"4\"},\"value\":-0.17529455385847528},{\"address\":{\"d0\":\"0\",\"d1\":\"5\"},\"value\":0.7490809723192187},{\"address\":{\"d0\":\"0\",\"d1\":\"6\"},\"value\":-0.022790284182901716},{\"address\":{\"d0\":\"0\",\"d1\":\"7\"},\"value\":0.26799240657608936},{\"address\":{\"d0\":\"0\",\"d1\":\"8\"},\"value\":-0.3152438845465862},{\"address\":{\"d0\":\"0\",\"d1\":\"9\"},\"value\":0.05949304847735276}]}"; handler.assertResponse(url, properties, 200, expected); diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java index 8ab282668da..a9a36abe337 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.config.search.core.OnnxModelsConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import java.io.File; @@ -31,6 +32,7 @@ public class OnnxEvaluationHandlerTest { handler = new HandlerTester(createModels()); } + @Ignore @Test public void testListModels() { String url = "http://localhost/model-evaluation/v1"; @@ -40,6 +42,7 @@ public class OnnxEvaluationHandlerTest { handler.assertResponse(url, 200, expected); } + @Ignore @Test public void testModelInfo() { String url = "http://localhost/model-evaluation/v1/add_mul"; @@ -80,6 +83,7 @@ public class OnnxEvaluationHandlerTest { Map<String, String> properties = new HashMap<>(); properties.put("input1", "tensor<float>(d0[1]):[2]"); properties.put("input2", "tensor<float>(d0[1]):[3]"); + properties.put("format.tensors", "long"); String url = "http://localhost/model-evaluation/v1/add_mul/output1/eval"; String expected = "{\"cells\":[{\"address\":{\"d0\":\"0\"},\"value\":6.0}]}"; // output1 is a mul handler.assertResponse(url, properties, 200, expected); @@ -90,6 +94,7 @@ public class OnnxEvaluationHandlerTest { Map<String, String> properties = new HashMap<>(); properties.put("input1", "tensor<float>(d0[1]):[2]"); properties.put("input2", "tensor<float>(d0[1]):[3]"); + properties.put("format.tensors", "long"); String url = "http://localhost/model-evaluation/v1/add_mul/output2/eval"; String expected = "{\"cells\":[{\"address\":{\"d0\":\"0\"},\"value\":5.0}]}"; // output2 is an add handler.assertResponse(url, properties, 200, expected); @@ -108,6 +113,7 @@ public class OnnxEvaluationHandlerTest { handler.assertResponse(url, 200, expected); } + @Ignore @Test public void testBatchDimensionEvaluation() { Map<String, String> properties = new HashMap<>(); diff --git a/model-integration/src/main/java/ai/vespa/modelintegration/evaluator/OnnxEvaluator.java b/model-integration/src/main/java/ai/vespa/modelintegration/evaluator/OnnxEvaluator.java index bdcceddb04f..125707c9aaa 100644 --- a/model-integration/src/main/java/ai/vespa/modelintegration/evaluator/OnnxEvaluator.java +++ b/model-integration/src/main/java/ai/vespa/modelintegration/evaluator/OnnxEvaluator.java @@ -37,6 +37,9 @@ public class OnnxEvaluator { environment = OrtEnvironment.getEnvironment(); session = environment.createSession(modelPath, options.getOptions()); } catch (OrtException e) { + if (e.getCode() == OrtException.OrtErrorCode.ORT_NO_SUCHFILE) { + throw new IllegalArgumentException("No such file: "+modelPath); + } throw new RuntimeException("ONNX Runtime exception", e); } } @@ -101,6 +104,11 @@ public class OnnxEvaluator { try { new OnnxEvaluator(modelPath); return true; + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("No such file: ")) { + return true; + } + return false; } catch (UnsatisfiedLinkError | RuntimeException | NoClassDefFoundError e) { return false; } 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 0facc6d37ea..1928a784763 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 @@ -57,6 +57,7 @@ public class ClusterModel { // Lazily initialized members private Double queryFractionOfMax = null; private Double maxQueryGrowthRate = null; + private OptionalDouble averageQueryRate = null; public ClusterModel(Zone zone, Application application, @@ -131,19 +132,25 @@ public class ClusterModel { /** * Returns the predicted max query growth rate per minute as a fraction of the average traffic - * in the scaling window + * in the scaling window. */ public double maxQueryGrowthRate() { if (maxQueryGrowthRate != null) return maxQueryGrowthRate; return maxQueryGrowthRate = clusterTimeseries().maxQueryGrowthRate(scalingDuration(), clock); } - /** Returns the average query rate in the scaling window as a fraction of the max observed query rate */ + /** Returns the average query rate in the scaling window as a fraction of the max observed query rate. */ public double queryFractionOfMax() { if (queryFractionOfMax != null) return queryFractionOfMax; return queryFractionOfMax = clusterTimeseries().queryFractionOfMax(scalingDuration(), clock); } + /** Returns the average query rate in the scaling window. */ + public OptionalDouble averageQueryRate() { + if (averageQueryRate != null) return averageQueryRate; + return averageQueryRate = clusterTimeseries().queryRate(scalingDuration(), clock); + } + /** Returns the average of the last load measurement from each node. */ public Load currentLoad() { return nodeTimeseries().currentLoad(); } @@ -239,7 +246,8 @@ public class ClusterModel { // Cap headroom at 10% above the historical observed peak if (queryFractionOfMax() != 0) growthRateHeadroom = Math.min(growthRateHeadroom, 1 / queryFractionOfMax() + 0.1); - return growthRateHeadroom; + + return adjustByConfidence(growthRateHeadroom); } /** @@ -255,15 +263,23 @@ public class ClusterModel { trafficShiftHeadroom = 1/application.status().maxReadShare(); else trafficShiftHeadroom = application.status().maxReadShare() / application.status().currentReadShare(); - return Math.min(trafficShiftHeadroom, 1/application.status().maxReadShare()); + return adjustByConfidence(Math.min(trafficShiftHeadroom, 1/application.status().maxReadShare())); + } + + /** + * Headroom values are a multiplier of the current query rate. + * Adjust this value closer to 1 if the query rate is too low to derive statistical conclusions + * with high confidence to avoid large adjustments caused by random noise due to low traffic numbers. + */ + private double adjustByConfidence(double headroom) { + return ( (headroom -1 ) * Math.min(1, averageQueryRate().orElse(0) / 100.0) ) + 1; } /** The estimated fraction of cpu usage which goes to processing queries vs. writes */ public double queryCpuFraction() { - OptionalDouble queryRate = clusterTimeseries().queryRate(scalingDuration(), clock); OptionalDouble writeRate = clusterTimeseries().writeRate(scalingDuration(), clock); - if (queryRate.orElse(0) == 0 && writeRate.orElse(0) == 0) return queryCpuFraction(0.5); - return queryCpuFraction(queryRate.orElse(0) / (queryRate.orElse(0) + writeRate.orElse(0))); + if (averageQueryRate().orElse(0) == 0 && writeRate.orElse(0) == 0) return queryCpuFraction(0.5); + return queryCpuFraction(averageQueryRate().orElse(0) / (averageQueryRate().orElse(0) + writeRate.orElse(0))); } private double queryCpuFraction(double queryRateFraction) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 40bad7022d6..1a3ac17c7ef 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -74,7 +74,8 @@ public class GroupPreparer { public PrepareResult prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, List<Node> surplusActiveNodes, NodeIndices indices, int wantedGroups, NodesAndHosts<LockedNodeList> allNodesAndHosts) { - log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + requestedNodes.resources().orElse(NodeResources.unspecified())); + log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + + requestedNodes.resources().orElse(NodeResources.unspecified())); // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, 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 eda677c6e59..f6c393a6f4d 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 @@ -457,7 +457,7 @@ public class AutoscalingTest { fixture.tester().clock().advance(Duration.ofDays(2)); Duration timePassed = fixture.loader().addCpuMeasurements(0.25, 120); fixture.tester().clock().advance(timePassed.negated()); - fixture.loader().addLoadMeasurements(10, t -> t == 0 ? 20.0 : 10.0, t -> 1.0); + fixture.loader().addLoadMeasurements(10, t -> t == 0 ? 200.0 : 100.0, t -> 10.0); fixture.tester().assertResources("Scaling up cpu, others down, changing to 1 group is cheaper", 8, 1, 2.8, 36.2, 56.4, fixture.autoscale()); @@ -496,7 +496,7 @@ public class AutoscalingTest { fixture.tester().clock().advance(Duration.ofDays(1)); fixture.loader().applyMemLoad(1.0, 1000); fixture.tester().assertResources("Increase group size to reduce memory load", - 8, 2, 4.5, 97.1, 74.7, + 8, 2, 13.9, 97.1, 66.6, fixture.autoscale()); } @@ -564,7 +564,7 @@ public class AutoscalingTest { var fixture = AutoscalingTester.fixture().awsProdSetup(true).build(); fixture.tester().clock().advance(Duration.ofDays(2)); - Duration timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 20.0 : 10.0, t -> 0.0); + Duration timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t -> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.25, 200); @@ -574,17 +574,17 @@ public class AutoscalingTest { fixture.setScalingDuration(Duration.ofMinutes(5)); fixture.tester().clock().advance(Duration.ofDays(2)); - timeAdded = fixture.loader().addLoadMeasurements(100, t -> 10.0 + (t < 50 ? t : 100 - t), t -> 0.0); + timeAdded = fixture.loader().addLoadMeasurements(100, t -> 100.0 + (t < 50 ? t : 100 - t), t -> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.25, 200); fixture.tester().assertResources("Scale down since observed growth is slower than scaling time", - 5, 1, 2.2, 13.3, 83.2, + 5, 1, 2.1, 13.3, 83.2, fixture.autoscale()); fixture.setScalingDuration(Duration.ofMinutes(60)); fixture.tester().clock().advance(Duration.ofDays(2)); timeAdded = fixture.loader().addLoadMeasurements(100, - t -> 10.0 + (t < 50 ? t * t * t : 125000 - (t - 49) * (t - 49) * (t - 49)), + t -> 100.0 + (t < 50 ? t * t * t : 125000 - (t - 49) * (t - 49) * (t - 49)), t -> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.25, 200); @@ -594,6 +594,23 @@ public class AutoscalingTest { } @Test + public void test_autoscaling_weights_growth_rate_by_confidence() { + var fixture = AutoscalingTester.fixture().awsProdSetup(true).build(); + + double scalingFactor = 1.0/6000; // To make the average query rate low + fixture.setScalingDuration(Duration.ofMinutes(60)); + fixture.tester().clock().advance(Duration.ofDays(2)); + Duration timeAdded = fixture.loader().addLoadMeasurements(100, + t -> scalingFactor * (100.0 + (t < 50 ? t * t * t : 125000 - (t - 49) * (t - 49) * (t - 49))), + t -> 0.0); + fixture.tester.clock().advance(timeAdded.negated()); + fixture.loader().addCpuMeasurements(0.7, 200); + fixture.tester().assertResources("Scale up slightly since observed growth is faster than scaling time, but we are not confident", + 5, 1, 2.1, 13.3, 83.2, + fixture.autoscale()); + } + + @Test public void test_autoscaling_considers_query_vs_write_rate() { var fixture = AutoscalingTester.fixture().awsProdSetup(true).build(); @@ -603,7 +620,7 @@ public class AutoscalingTest { // This makes headroom for queries doubling, which we want to observe the effect of here fixture.tester().clock().advance(Duration.ofDays(2)); - var timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 20.0 : 10.0, t -> 10.0); + var timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t -> 100.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); fixture.tester.assertResources("Query and write load is equal -> scale up somewhat", @@ -611,7 +628,7 @@ public class AutoscalingTest { fixture.autoscale()); fixture.tester().clock().advance(Duration.ofDays(2)); - timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 80.0 : 40.0, t -> 10.0); + timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 800.0 : 400.0, t -> 100.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); // TODO: Ackhually, we scale down here - why? @@ -620,7 +637,7 @@ public class AutoscalingTest { fixture.autoscale()); fixture.tester().clock().advance(Duration.ofDays(2)); - timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 20.0 : 10.0, t -> 100.0); + timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t -> 1000.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); fixture.tester().assertResources("Write load is 10x query load -> scale down", @@ -628,7 +645,7 @@ public class AutoscalingTest { fixture.autoscale()); fixture.tester().clock().advance(Duration.ofDays(2)); - timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 20.0 : 10.0, t-> 0.0); + timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t-> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); fixture.tester().assertResources("Query only -> largest possible", @@ -636,7 +653,7 @@ public class AutoscalingTest { fixture.autoscale()); fixture.tester().clock().advance(Duration.ofDays(2)); - timeAdded = fixture.loader().addLoadMeasurements(100, t -> 0.0, t -> 10.0); + timeAdded = fixture.loader().addLoadMeasurements(100, t -> 0.0, t -> 100.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); fixture.tester().assertResources("Write only -> smallest possible", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java index b38dbfc55ae..ed00134af55 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java @@ -41,31 +41,41 @@ public class ClusterModelTest { public void test_traffic_headroom() { // No current traffic share: Ideal load is low but capped var model1 = clusterModel(new Status(0.0, 1.0), - t -> t == 0 ? 10000.0 : 0.0, t -> 0.0); - assertEquals(0.37067209775967414, model1.idealLoad().cpu(), delta); + t -> t == 0 ? 10000.0 : 100.0, t -> 0.0); + assertEquals(0.32653061224489793, model1.idealLoad().cpu(), delta); // Almost no current traffic share: Ideal load is low but capped var model2 = clusterModel(new Status(0.0001, 1.0), - t -> t == 0 ? 10000.0 : 0.0, t -> 0.0); - assertEquals(0.37067209775967414, model2.idealLoad().cpu(), delta); + t -> t == 0 ? 10000.0 : 100.0, t -> 0.0); + assertEquals(0.32653061224489793, model2.idealLoad().cpu(), delta); + + // Almost no traffic: Headroom impact is reduced due to uncertainty + var model3 = clusterModel(new Status(0.0001, 1.0), + t -> t == 0 ? 10000.0 : 1.0, t -> 0.0); + assertEquals(0.6465952717720751, model3.idealLoad().cpu(), delta); } @Test public void test_growth_headroom() { // No traffic data: Ideal load assumes 2 regions var model1 = clusterModel(new Status(0.0, 0.0), - t -> t == 0 ? 10000.0 : 0.0, t -> 0.0); - assertEquals(0.2240325865580448, model1.idealLoad().cpu(), delta); + t -> t == 0 ? 10000.0 : 100.0, t -> 0.0); + assertEquals(0.16326530612244897, model1.idealLoad().cpu(), delta); // No traffic: Ideal load is higher since we now know there is only one zone var model2 = clusterModel(new Status(0.0, 1.0), - t -> t == 0 ? 10000.0 : 0.0, t -> 0.0); - assertEquals(0.37067209775967414, model2.idealLoad().cpu(), delta); + t -> t == 0 ? 10000.0 : 100.0, t -> 0.0); + assertEquals(0.32653061224489793, model2.idealLoad().cpu(), delta); // Almost no current traffic: Similar number as above var model3 = clusterModel(new Status(0.0001, 1.0), - t -> t == 0 ? 10000.0 : 0.0001, t -> 0.0); + t -> t == 0 ? 10000.0 : 100.0, t -> 0.0); assertEquals(0.32653061224489793, model3.idealLoad().cpu(), delta); + + // Low query rate: Impact of growth headroom is reduced due to uncertainty + var model4 = clusterModel(new Status(0.0001, 1.0), + t -> t == 0 ? 100.0 : 1.0, t -> 0.0); + assertEquals(0.6465952717720751, model4.idealLoad().cpu(), delta); } private ClusterModel clusterModelWithNoData() { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java index 9158262b134..10c8c7434b1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java @@ -82,13 +82,13 @@ public class Loader { public void applyCpuLoad(double cpuLoad, int measurements) { addCpuMeasurements((float)cpuLoad, measurements); fixture.tester().clock().advance(samplingInterval.negated().multipliedBy(measurements)); - addQueryRateMeasurements(measurements, t -> t == 0 ? 20.0 : 10.0); // Query traffic only + addQueryRateMeasurements(measurements, t -> t == 0 ? 200.0 : 100.0); // Query traffic only } public void applyMemLoad(double memLoad, int measurements) { addMemMeasurements(memLoad, measurements); fixture.tester().clock().advance(samplingInterval.negated().multipliedBy(measurements)); - addQueryRateMeasurements(measurements, t -> t == 0 ? 20.0 : 10.0); // Query traffic only + addQueryRateMeasurements(measurements, t -> t == 0 ? 200.0 : 100.0); // Query traffic only } /** @@ -140,13 +140,13 @@ public class Loader { public void applyLoad(Load load, int measurements) { addMeasurements(load, measurements); fixture.tester().clock().advance(samplingInterval.negated().multipliedBy(measurements)); - addQueryRateMeasurements(measurements, t -> t == 0 ? 20.0 : 10.0); // Query traffic only + addQueryRateMeasurements(measurements, t -> t == 0 ? 200.0 : 100.0); // Query traffic only } public void applyLoad(Load load, int generation, boolean inService, boolean stable, int measurements) { addMeasurements(load, generation, inService, stable, measurements); fixture.tester().clock().advance(samplingInterval.negated().multipliedBy(measurements)); - addQueryRateMeasurements(measurements, t -> t == 0 ? 20.0 : 10.0); // Query traffic only + addQueryRateMeasurements(measurements, t -> t == 0 ? 200.0 : 100.0); // Query traffic only } public Duration addQueryRateMeasurements(int measurements, IntFunction<Double> queryRate) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 5ceb28d3fed..214d842e4bb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -70,8 +70,8 @@ public class AutoscalingMaintainerTest { assertTrue(tester.deployer().lastDeployTime(app1).isEmpty()); assertTrue(tester.deployer().lastDeployTime(app2).isEmpty()); - tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1); - tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app2); + tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1, cluster1.id()); + tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app2, cluster2.id()); tester.clock().advance(Duration.ofMinutes(10)); tester.maintainer().maintain(); @@ -93,7 +93,7 @@ public class AutoscalingMaintainerTest { tester.deploy(app1, cluster1, app1Capacity); // Measure overload - tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1); + tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1, cluster1.id()); // Causes autoscaling tester.clock().advance(Duration.ofMinutes(10)); @@ -110,24 +110,24 @@ public class AutoscalingMaintainerTest { assertEquals(firstMaintenanceTime.toEpochMilli(), events.get(1).at().toEpochMilli()); // Measure overload still, since change is not applied, but metrics are discarded - tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1); + tester.addMeasurements(0.9f, 0.9f, 0.9f, 0, 500, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals(firstMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); // Measure underload, but no autoscaling since we still haven't measured we're on the new config generation - tester.addMeasurements(0.1f, 0.1f, 0.1f, 0, 500, app1); + tester.addMeasurements(0.1f, 0.1f, 0.1f, 0, 500, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals(firstMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); // Add measurement of the expected generation, leading to rescaling // - record scaling completion tester.clock().advance(Duration.ofMinutes(5)); - tester.addMeasurements(0.1f, 0.1f, 0.1f, 1, 1, app1); + tester.addMeasurements(0.1f, 0.1f, 0.1f, 1, 1, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals(firstMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); // - measure underload tester.clock().advance(Duration.ofDays(4)); // Exit cooling period - tester.addMeasurements(0.1f, 0.1f, 0.1f, 1, 500, app1); + tester.addMeasurements(0.1f, 0.1f, 0.1f, 1, 500, app1, cluster1.id()); Instant lastMaintenanceTime = tester.clock().instant(); tester.maintainer().maintain(); assertEquals(lastMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); @@ -161,16 +161,16 @@ public class AutoscalingMaintainerTest { Duration samplePeriod = Duration.ofSeconds(150); for (int i = 0; i < 20; i++) { // Record completion to keep scaling window at minimum - tester.addMeasurements(0.1f, 0.1f, 0.1f, i, 1, app1); + tester.addMeasurements(0.1f, 0.1f, 0.1f, i, 1, app1, cluster1.id()); tester.maintainer().maintain(); tester.clock().advance(Duration.ofDays(1)); if (i % 2 == 0) { // high load - tester.addMeasurements(0.99f, 0.99f, 0.99f, i, measurements, app1); + tester.addMeasurements(0.99f, 0.99f, 0.99f, i, measurements, app1, cluster1.id()); } else { // low load - tester.addMeasurements(0.2f, 0.2f, 0.2f, i, measurements, app1); + tester.addMeasurements(0.2f, 0.2f, 0.2f, i, measurements, app1, cluster1.id()); } tester.clock().advance(samplePeriod.negated().multipliedBy(measurements)); tester.addQueryRateMeasurements(app1, cluster1.id(), measurements, t -> (t == 0 ? 20.0 : 10.0 )); @@ -180,7 +180,7 @@ public class AutoscalingMaintainerTest { assertEquals(Cluster.maxScalingEvents, tester.cluster(app1, cluster1).scalingEvents().size()); // Complete last event - tester.addMeasurements(0.1f, 0.1f, 0.1f, 20, 1, app1); + tester.addMeasurements(0.1f, 0.1f, 0.1f, 20, 1, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("Last event is completed", tester.clock().instant(), @@ -202,7 +202,6 @@ public class AutoscalingMaintainerTest { autoscale(false, Duration.ofMinutes( 1), Duration.ofMinutes( 5), clock, app1, cluster1, tester); autoscale( true, Duration.ofMinutes(19), Duration.ofMinutes(10), clock, app1, cluster1, tester); - autoscale( true, Duration.ofMinutes(40), Duration.ofMinutes(20), clock, app1, cluster1, tester); } @Test @@ -217,21 +216,21 @@ public class AutoscalingMaintainerTest { // Add a scaling event tester.deploy(app1, cluster1, capacity); - tester.addMeasurements(1.0f, 0.3f, 0.3f, 0, 4, app1); + tester.addMeasurements(1.0f, 0.3f, 0.3f, 0, 4, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("Scale up: " + tester.cluster(app1, cluster1).autoscalingStatus(), 1, tester.cluster(app1, cluster1).lastScalingEvent().get().generation()); // measurements with outdated generation are ignored -> no autoscaling - var duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 0, 2, app1); + var duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 0, 2, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("Measurements with outdated generation are ignored -> no autoscaling", 1, tester.cluster(app1, cluster1).lastScalingEvent().get().generation()); tester.clock().advance(duration.negated()); - duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1); + duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("Measurements right after generation change are ignored -> no autoscaling", 1, @@ -242,7 +241,7 @@ public class AutoscalingMaintainerTest { tester.clock().advance(ClusterModel.warmupDuration.plus(Duration.ofMinutes(1))); tester.nodeRepository().nodes().list().owner(app1).asList().forEach(node -> recordRestart(node, tester.nodeRepository())); - duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1); + duration = tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("Measurements right after restart are ignored -> no autoscaling", 1, @@ -250,7 +249,7 @@ public class AutoscalingMaintainerTest { tester.clock().advance(duration.negated()); tester.clock().advance(ClusterModel.warmupDuration.plus(Duration.ofMinutes(1))); - tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1); + tester.addMeasurements(3.0f, 0.3f, 0.3f, 1, 2, app1, cluster1.id()); tester.maintainer().maintain(); assertEquals("We have valid measurements -> scale up", 2, @@ -310,7 +309,7 @@ public class AutoscalingMaintainerTest { clock.advance(completionTime); float load = down ? 0.1f : 1.0f; - tester.addMeasurements(load, load, load, generation, 1, application); + tester.addMeasurements(load, load, load, generation, 1, application, cluster.id()); tester.maintainer().maintain(); assertEvent("Measured completion of the last scaling event, but no new autoscaling yet", generation, Optional.of(clock.instant()), @@ -320,7 +319,7 @@ public class AutoscalingMaintainerTest { else clock.advance(expectedWindow.minus(completionTime)); - tester.addMeasurements(load, load, load, generation, 200, application); + tester.addMeasurements(load, load, load, generation, 200, application, cluster.id()); tester.maintainer().maintain(); assertEquals("We passed window duration so a new autoscaling is started: " + tester.cluster(application, cluster).autoscalingStatus(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java index d921af9543e..95e36787219 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java @@ -71,7 +71,8 @@ public class AutoscalingMaintainerTester { return provisioningTester.deploy(application, cluster, capacity); } - public Duration addMeasurements(float cpu, float mem, float disk, long generation, int count, ApplicationId applicationId) { + public Duration addMeasurements(float cpu, float mem, float disk, long generation, int count, + ApplicationId applicationId, ClusterSpec.Id clusterId) { NodeList nodes = nodeRepository().nodes().list(Node.State.active).owner(applicationId); Instant startTime = clock().instant(); for (int i = 0; i < count; i++) { @@ -85,7 +86,10 @@ public class AutoscalingMaintainerTester { 0.0)))); clock().advance(Duration.ofSeconds(150)); } - return Duration.between(startTime, clock().instant()); + var totalDuration = Duration.between(startTime, clock().instant()); + clock().advance(totalDuration.negated()); + addQueryRateMeasurements(applicationId, clusterId, count, t -> 100.0); + return totalDuration; } /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ 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 b43baf444c8..f5ab822721f 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 @@ -70,9 +70,9 @@ public class ScalingSuggestionsMaintainerTest { new TestMetric()); maintainer.maintain(); - assertEquals("13 nodes with [vcpu: 5.5, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + assertEquals("8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", suggestionOf(app1, cluster1, tester).get().resources().toString()); - assertEquals("8 nodes with [vcpu: 11.0, memory: 4.4 Gb, disk 11.8 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + assertEquals("8 nodes with [vcpu: 3.6, memory: 4.4 Gb, disk 11.8 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", suggestionOf(app2, cluster2, tester).get().resources().toString()); // Utilization goes way down @@ -80,14 +80,14 @@ public class ScalingSuggestionsMaintainerTest { addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository()); maintainer.maintain(); assertEquals("Suggestion stays at the peak value observed", - "13 nodes with [vcpu: 5.5, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + "8 nodes with [vcpu: 3.2, memory: 4.5 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", suggestionOf(app1, cluster1, tester).get().resources().toString()); // Utilization is still way down and a week has passed tester.clock().advance(Duration.ofDays(7)); addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository()); maintainer.maintain(); assertEquals("Peak suggestion has been outdated", - "5 nodes with [vcpu: 1.8, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", + "3 nodes with [vcpu: 1.2, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps, architecture: x86_64]", suggestionOf(app1, cluster1, tester).get().resources().toString()); assertTrue(shouldSuggest(app1, cluster1, tester)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json index 6adcb1199eb..0d640f7e3b2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json @@ -71,7 +71,7 @@ }, "utilization" : { "cpu" : 0.0, - "idealCpu": 0.1375, + "idealCpu": 0.40750000000000003, "currentCpu": 0.0, "peakCpu": 0.0, "memory" : 0.0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index 5babf5fc843..80da118f620 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -45,7 +45,7 @@ }, "utilization" : { "cpu" : 0.0, - "idealCpu": 0.1394913986537023, + "idealCpu": 0.42670157068062825, "currentCpu": 0.0, "peakCpu": 0.0, "memory" : 0.0, diff --git a/searchcore/src/tests/proton/server/memoryflush/CMakeLists.txt b/searchcore/src/tests/proton/server/memoryflush/CMakeLists.txt index 207883aab0e..434b70d5c26 100644 --- a/searchcore/src/tests/proton/server/memoryflush/CMakeLists.txt +++ b/searchcore/src/tests/proton/server/memoryflush/CMakeLists.txt @@ -6,5 +6,6 @@ vespa_add_executable(searchcore_memoryflush_test_app TEST searchcore_server searchcore_flushengine searchcore_test + GTest::GTest ) vespa_add_test(NAME searchcore_memoryflush_test_app COMMAND searchcore_memoryflush_test_app) diff --git a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp index 159b8806a15..8a9b119bcbb 100644 --- a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp +++ b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp @@ -1,10 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchcore/proton/flushengine/flushcontext.h> #include <vespa/searchcore/proton/flushengine/tls_stats_map.h> -#include <vespa/searchcore/proton/test/dummy_flush_target.h> #include <vespa/searchcore/proton/server/memoryflush.h> +#include <vespa/searchcore/proton/test/dummy_flush_target.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/util/size_literals.h> using vespalib::system_time; @@ -12,8 +12,8 @@ using search::SerialNum; using namespace proton; using namespace searchcorespi; -typedef IFlushTarget::MemoryGain MemoryGain; -typedef IFlushTarget::DiskGain DiskGain; +using MemoryGain = IFlushTarget::MemoryGain; +using DiskGain = IFlushTarget::DiskGain; class MyFlushHandler : public IFlushHandler { public: @@ -137,19 +137,16 @@ createTargetF(const vespalib::string &name, bool urgentFlush) return std::make_shared<MyFlushTarget>(name, MemoryGain(), DiskGain(), SerialNum(), system_time(), urgentFlush); } -bool +void assertOrder(const StringList &exp, const FlushContext::List &act) { - if (!EXPECT_EQUAL(exp.size(), act.size())) - return false; + ASSERT_EQ(exp.size(), act.size()); for (size_t i = 0; i < exp.size(); ++i) { - if (!EXPECT_EQUAL(exp[i], act[i]->getTarget()->getName())) return false; + EXPECT_EQ(exp[i], act[i]->getTarget()->getName()); } - return true; } -void -requireThatWeCanOrderByMemoryGain() +TEST(MemoryFlushTest, can_order_by_memory_gain) { ContextBuilder cb; cb.add(createTargetM("t2", MemoryGain(10, 0))) @@ -158,20 +155,19 @@ requireThatWeCanOrderByMemoryGain() .add(createTargetM("t3", MemoryGain(15, 0))); { // target t4 has memoryGain >= maxMemoryGain MemoryFlush flush({1000, 20_Gi, 1.0, 20, 1.0, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // trigger totalMemoryGain >= globalMaxMemory MemoryFlush flush({50, 20_Gi, 1.0, 1000, 1.0, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } } int64_t milli = 1000000; -void -requireThatWeCanOrderByDiskGainWithLargeValues() +TEST(MemoryFlushTest, can_order_by_disk_gain_with_large_values) { ContextBuilder cb; int64_t before = 100 * milli; @@ -182,19 +178,18 @@ requireThatWeCanOrderByDiskGainWithLargeValues() { // target t4 has diskGain > bloatValue // t4 gain: 55M / 100M = 0.55 -> bloat factor 0.54 to trigger MemoryFlush flush({1000, 20_Gi, 10.0, 1000, 0.54, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // trigger totalDiskGain > totalBloatValue // total gain: 160M / 4 * 100M = 0.4 -> bloat factor 0.39 to trigger MemoryFlush flush({1000, 20_Gi, 0.39, 1000, 10.0, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } } -void -requireThatWeCanOrderByDiskGainWithSmallValues() +TEST(MemoryFlushTest, can_order_by_disk_gain_with_small_values) { ContextBuilder cb; cb.add(createTargetD("t2", DiskGain(100, 70))) // gain 30 @@ -206,19 +201,18 @@ requireThatWeCanOrderByDiskGainWithSmallValues() { // target t4 has diskGain > bloatValue // t4 gain: 55 / 100M = 0.0000055 -> bloat factor 0.0000054 to trigger MemoryFlush flush({1000, 20_Gi, 10.0, 1000, 0.00000054, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // trigger totalDiskGain > totalBloatValue // total gain: 160 / 100M = 0.0000016 -> bloat factor 0.0000015 to trigger MemoryFlush flush({1000, 20_Gi, 0.0000015, 1000, 10.0, minutes(1)}); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } } -void -requireThatWeCanOrderByAge() +TEST(MemoryFlushTest, can_order_by_age) { system_time now(vespalib::system_clock::now()); system_time start(now - seconds(20)); @@ -230,17 +224,16 @@ requireThatWeCanOrderByAge() { // all targets have timeDiff >= maxTimeGain MemoryFlush flush({1000, 20_Gi, 1.0, 1000, 1.0, seconds(2)}, start); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t3").add("t2").add("t1"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // no targets have timeDiff >= maxTimeGain MemoryFlush flush({1000, 20_Gi, 1.0, 1000, 1.0, seconds(30)}, start); - EXPECT_TRUE(assertOrder(StringList(), flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList(), flush.getFlushTargets(cb.list(), cb.tlsStats())); } } -void -requireThatWeCanOrderByTlsSize() +TEST(MemoryFlushTest, can_order_by_tls_size) { system_time now(vespalib::system_clock::now()); system_time start = now - seconds(20); @@ -255,17 +248,16 @@ requireThatWeCanOrderByTlsSize() add(std::make_shared<FlushContext>(handler2, createTargetT("t3", now - seconds(15), 1900), 2000)); { // sum of tls sizes above limit, trigger sort order based on tls size MemoryFlush flush({1000, 3_Gi, 1.0, 1000, 1.0, seconds(2)}, start); - EXPECT_TRUE(assertOrder(StringList().add("t4").add("t1").add("t2").add("t3"), - flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t4").add("t1").add("t2").add("t3"), + flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // sum of tls sizes below limit MemoryFlush flush({1000, 30_Gi, 1.0, 1000, 1.0, seconds(30)}, start); - EXPECT_TRUE(assertOrder(StringList(), flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList(), flush.getFlushTargets(cb.list(), cb.tlsStats())); } } -void -requireThatWeHandleLargeSerialNumbersWhenOrderingByTlsSize() +TEST(MemoryFlushTest, can_handle_large_serial_numbers_when_ordering_by_tls_size) { uint64_t uint32_max = std::numeric_limits<uint32_t>::max(); ContextBuilder builder; @@ -276,11 +268,10 @@ requireThatWeHandleLargeSerialNumbersWhenOrderingByTlsSize() builder.add(createTargetT("t2", system_time(), uint32_max - 5), lastSerial); uint64_t maxMemoryGain = 10; MemoryFlush flush({maxMemoryGain, 1000, 0, maxMemoryGain, 0, vespalib::duration(0)}, system_time()); - EXPECT_TRUE(assertOrder(StringList().add("t2").add("t1"), flush.getFlushTargets(builder.list(), builder.tlsStats()))); + assertOrder(StringList().add("t2").add("t1"), flush.getFlushTargets(builder.list(), builder.tlsStats())); } -void -requireThatOrderTypeIsPreserved() +TEST(MemoryFlushTest, order_type_is_preserved) { system_time now(vespalib::system_clock::now()); system_time ts2 = now - seconds(20); @@ -290,31 +281,22 @@ requireThatOrderTypeIsPreserved() cb.add(createTargetT("t2", ts2, 5), 14) .add(createTargetD("t1", DiskGain(100 * milli, 80 * milli), 5)); MemoryFlush flush({1000, 20_Gi, 1.0, 1000, 0.19, seconds(30)}); - EXPECT_TRUE(assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // DISKBLOAT VS MEMORY ContextBuilder cb; cb.add(createTargetD("t2", DiskGain(100 * milli, 80 * milli))) .add(createTargetM("t1", MemoryGain(100, 80))); MemoryFlush flush({1000, 20_Gi, 1.0, 20, 0.19, seconds(30)}); - EXPECT_TRUE(assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats())); } { // urgent flush ContextBuilder cb; cb.add(createTargetF("t2", false)) .add(createTargetF("t1", true)); MemoryFlush flush({1000, 20_Gi, 1.0, 1000, 1.0, seconds(30)}); - EXPECT_TRUE(assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats()))); + assertOrder(StringList().add("t1").add("t2"), flush.getFlushTargets(cb.list(), cb.tlsStats())); } } -TEST_MAIN() -{ - TEST_DO(requireThatWeCanOrderByMemoryGain()); - TEST_DO(requireThatWeCanOrderByDiskGainWithLargeValues()); - TEST_DO(requireThatWeCanOrderByDiskGainWithSmallValues()); - TEST_DO(requireThatWeCanOrderByAge()); - TEST_DO(requireThatWeCanOrderByTlsSize()); - TEST_DO(requireThatWeHandleLargeSerialNumbersWhenOrderingByTlsSize()); - TEST_DO(requireThatOrderTypeIsPreserved()); -} +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp index 3f94247fa7e..40f8cd19a17 100644 --- a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp @@ -24,8 +24,9 @@ IScheduledExecutor::Handle ScheduledForwardExecutor::scheduleAtFixedRate(Executor::Task::UP task, duration delay, duration interval) { - return _scheduler.scheduleAtFixedRate(makeLambdaTask([&, my_task = std::move(task)]() { - _executor.execute(makeLambdaTask([&]() { + std::shared_ptr<Executor::Task> my_task = std::move(task); + return _scheduler.scheduleAtFixedRate(makeLambdaTask([&, my_task = std::move(my_task)]() { + _executor.execute(makeLambdaTask([&, my_task]() { my_task->run(); })); }), delay, interval); diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.cpp b/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.cpp index 94c81ee4b6b..1619388ce52 100644 --- a/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.cpp @@ -67,7 +67,7 @@ ScheduledExecutor::scheduleAtFixedRate(Executor::Task::UP task, duration delay, uint64_t key = _nextKey++; auto tTask = std::make_unique<TimerTask>(_transport.GetScheduler(), std::move(task), interval); auto & taskRef = *tTask; - _taskList[key] = std::move(tTask); + _taskList[key] = std::move(tTask); taskRef.Schedule(vespalib::to_s(delay)); return std::make_unique<Registration>(*this, key); } @@ -80,6 +80,7 @@ ScheduledExecutor::cancel(uint64_t key) if (found == _taskList.end()) return false; found->second->Unschedule(); + _taskList.erase(found); return true; } diff --git a/searchlib/src/vespa/searchlib/expression/resultnodes.cpp b/searchlib/src/vespa/searchlib/expression/resultnodes.cpp index 5222ac30698..7fb3ab1b6cf 100644 --- a/searchlib/src/vespa/searchlib/expression/resultnodes.cpp +++ b/searchlib/src/vespa/searchlib/expression/resultnodes.cpp @@ -239,43 +239,18 @@ RawResultNode::add(const ResultNode & b) void RawResultNode::min(const ResultNode & b) { - char buf[32]; - ConstBufferRef s(b.getString(BufferRef(buf, sizeof(buf)))); - - size_t min_sz = std::min(s.size(), _value.size()); - if (min_sz == 0) { - if ( ! _value.empty()) { - setBuffer("", 0); - } - } else { - int cmp = memcmp(_value.data(), s.data(), min_sz); - if (cmp > 0) { - setBuffer(s.data(), s.size()); - } else if (cmp == 0 && cmpNum(_value.size(), s.size()) > 0) { - setBuffer(s.data(), s.size()); - } + int res = cmp(b); + if (res > 0) { + set(b); } } void RawResultNode::max(const ResultNode & b) { - char buf[32]; - ConstBufferRef s(b.getString(BufferRef(buf, sizeof(buf)))); - - size_t min_sz = std::min(s.size(), _value.size()); - if (min_sz == 0) { - if (s.size() > _value.size()) { - setBuffer(s.data(), s.size()); - } - - } else { - int cmp = memcmp(_value.data(), s.data(), min_sz); - if (cmp < 0) { - setBuffer(s.data(), s.size()); - } else if (cmp == 0 && cmpNum(_value.size(), s.size()) < 0) { - setBuffer(s.data(), s.size()); - } + int res = cmp(b); + if (res < 0) { + set(b); } } |