diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2021-03-10 11:46:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-10 11:46:49 +0100 |
commit | d8aba5c8b45ec3871eb808b8b69cbe76b4f409b8 (patch) | |
tree | 4b0c3d7173b65e0d481ba760713e659f19d6c0d5 | |
parent | c6f821dd6cf71ac3ae9daf2009c3d61a2d82afa7 (diff) | |
parent | ecac77e9c3196a797c6f6286e71e998efc44157d (diff) |
Merge pull request #16873 from vespa-engine/bratseth/validate-noderesources
Validate NodeRepository numbers
3 files changed, 42 insertions, 14 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index d1274472328..7f563b876a7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -104,10 +104,10 @@ public class NodeResources { } public NodeResources(double vcpu, double memoryGb, double diskGb, double bandwidthGbps, DiskSpeed diskSpeed, StorageType storageType) { - this.vcpu = vcpu; - this.memoryGb = memoryGb; - this.diskGb = diskGb; - this.bandwidthGbps = bandwidthGbps; + this.vcpu = validate(vcpu, "vcpu"); + this.memoryGb = validate(memoryGb, "memory"); + this.diskGb = validate(diskGb, "disk"); + this.bandwidthGbps = validate(bandwidthGbps, "bandwith"); this.diskSpeed = diskSpeed; this.storageType = storageType; } @@ -310,4 +310,10 @@ public class NodeResources { return new NodeResources(cpu, mem, dsk, 0.3, DiskSpeed.getDefault(), StorageType.getDefault()); } + private double validate(double value, String valueName) { + if (Double.isNaN(value)) throw new IllegalArgumentException(valueName + " cannot be NaN"); + if (Double.isInfinite(value)) throw new IllegalArgumentException(valueName + " cannot be infinite"); + return value; + } + } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java index 044afa72a5d..18eed0deecc 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java @@ -3,7 +3,10 @@ package com.yahoo.config.provision; import org.junit.Test; +import java.util.function.Supplier; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; /** * @author bratseth @@ -17,18 +20,17 @@ public class NodeResourcesTest { assertEquals("[vcpu: 0.3, memory: 3.3 Gb, disk 33.3 Gb, bandwidth: 0.3 Gbps]", new NodeResources(1/3., 10/3., 100/3., 0.3).toString()); assertEquals("[vcpu: 0.7, memory: 9.0 Gb, disk 66.7 Gb, bandwidth: 0.7 Gbps]", - new NodeResources(2/3., 8.97, 200/3., 0.67).toString()); + new NodeResources(2/3., 8.97, 200/3., 0.67).toString()); } - private long runTest(NodeResources [] resouces, int num) { - long sum = 0; - for (int i = 0; i < num; i++) { - for (NodeResources ns :resouces) { - sum += ns.toString().length(); - } - } - return sum; + @Test + public void testInvalid() { + assertInvalid("vcpu", () -> new NodeResources(Double.NaN, 1.0, 1.0, 1.0)); + assertInvalid("memory", () -> new NodeResources(1.0, Double.NaN, 1.0, 1.0)); + assertInvalid("disk", () -> new NodeResources(1.0, 1.0, Double.NaN, 1.0)); + assertInvalid("bandwith", () -> new NodeResources(1.0, 1.0, 1.0, Double.NaN)); } + @Test public void benchmark() { NodeResources [] resouces = new NodeResources[100]; @@ -44,4 +46,24 @@ public class NodeResourcesTest { assertEquals(warmup, benchmark); } + private void assertInvalid(String valueName, Supplier<NodeResources> nodeResources) { + try { + nodeResources.get(); + fail("Expected exception"); + } + catch (IllegalArgumentException e) { + assertEquals(valueName + " cannot be NaN", e.getMessage()); + } + } + + private long runTest(NodeResources [] resouces, int num) { + long sum = 0; + for (int i = 0; i < num; i++) { + for (NodeResources ns :resouces) { + sum += ns.toString().length(); + } + } + return sum; + } + } 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 d0f779a1c8b..baf7d2dbe15 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 @@ -540,7 +540,7 @@ public class AutoscalingTest { tester.addCpuMeasurements(0.5f, 1f, 10, application1); tester.addQueryRateMeasurements(application1, cluster1.id(), - 500, t -> 0.5); + 500, t -> 0.0); tester.assertResources("Advice to scale up since observed growth is much faster than scaling time", 3, 1, 1, 4, 50, |