From e7da585b15958eace956be3c5cc7f77cb5fc11a8 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 1 Jul 2021 12:30:52 +0200 Subject: More testing of setting resource limits and add TODOs --- .../content/cluster/DomResourceLimitsBuilder.java | 10 ++-- .../model/content/ClusterResourceLimitsTest.java | 63 ++++++++++++++++------ 2 files changed, 53 insertions(+), 20 deletions(-) (limited to 'config-model') diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java index 37adb73bc15..32b0f5b6477 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java @@ -24,10 +24,12 @@ public class DomResourceLimitsBuilder { if (hostedVespa) { String message = "Element '" + resourceLimits + "' is not allowed to be set"; - if (throwIfSpecified) - throw new IllegalArgumentException(message); - else - deployLogger.logApplicationPackage(Level.WARNING, message); + if (throwIfSpecified) throw new IllegalArgumentException(message); + + + deployLogger.logApplicationPackage(Level.WARNING, message); + // TODO: return (default values will then be used). Cannot be done now as an app needs current behavior + //return builder; } if (resourceLimits.child("disk") != null) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java index 4324f257922..22618ea433b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.searchdefinition.derived.TestableDeployLogger; import com.yahoo.text.XML; import com.yahoo.vespa.model.builder.xml.dom.ModelElement; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -24,15 +25,23 @@ public class ClusterResourceLimitsTest { private static class Fixture { private final boolean enableFeedBlockInDistributor; + private final boolean hostedVespa; + private final boolean throwIfSpecified; private final ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder(); private final ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder(); public Fixture() { - this.enableFeedBlockInDistributor = false; + this(false); } public Fixture(boolean enableFeedBlockInDistributor) { + this(enableFeedBlockInDistributor, false, false); + } + + public Fixture(boolean enableFeedBlockInDistributor, boolean hostedVespa, boolean throwIfSpecified) { this.enableFeedBlockInDistributor = enableFeedBlockInDistributor; + this.hostedVespa = hostedVespa; + this.throwIfSpecified = throwIfSpecified; } public Fixture ctrlDisk(double limit) { @@ -53,8 +62,8 @@ public class ClusterResourceLimitsTest { } public ClusterResourceLimits build() { var builder = new ClusterResourceLimits.Builder(enableFeedBlockInDistributor, - false, - false, + hostedVespa, + throwIfSpecified, new BaseDeployLogger()); builder.setClusterControllerBuilder(ctrlBuilder); builder.setContentNodeBuilder(nodeBuilder); @@ -130,27 +139,49 @@ public class ClusterResourceLimitsTest { } @Test - public void exception_is_thrown_when_resource_limits_are_specified() { + @Ignore // TODO: Remove hosted_limits_are_used_if_app_is_allowed_to_set_limits and enable this when code is fixed to do sp + public void hosted_log_when_resource_limits_are_specified() { TestableDeployLogger logger = new TestableDeployLogger(); - buildClusterResourceLimitsAndLogIfSpecified(logger); + var limits = hostedBuildAndLogIfSpecified(logger); assertEquals(1, logger.warnings.size()); assertEquals("Element 'resource-limits' is not allowed to be set", logger.warnings.get(0)); + // Verify that default limits are used + assertLimits(0.8, 0.8, limits.getClusterControllerLimits()); + assertLimits(0.9, 0.9, limits.getContentNodeLimits()); + } + + @Test + public void hosted_exception_is_thrown_when_resource_limits_are_specified() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage(containsString("Element 'resource-limits' is not allowed to be set")); - buildClusterResourceLimitsAndThrowIfSpecified(logger); + hostedBuildAndThrowIfSpecified(); + } + + @Test + // TODO: Remove this and enable hosted_log_when_resource_limits_are_specified when code is fixed + public void hosted_limits_are_used_if_app_is_allowed_to_set_limits() { + TestableDeployLogger logger = new TestableDeployLogger(); + + var limits = hostedBuildAndLogIfSpecified(logger); + assertEquals(1, logger.warnings.size()); + assertEquals("Element 'resource-limits' is not allowed to be set", logger.warnings.get(0)); + + // Verify that limits in XML are used + assertLimits(0.8, 0.92, limits.getClusterControllerLimits()); + assertLimits(0.9, 0.96, limits.getContentNodeLimits()); } - private void buildClusterResourceLimitsAndThrowIfSpecified(DeployLogger deployLogger) { - buildClusterResourceLimits(true, deployLogger); + private void hostedBuildAndThrowIfSpecified() { + hostedBuild(true, new TestableDeployLogger()); } - private void buildClusterResourceLimitsAndLogIfSpecified(DeployLogger deployLogger) { - buildClusterResourceLimits(false, deployLogger); + private ClusterResourceLimits hostedBuildAndLogIfSpecified(DeployLogger deployLogger) { + return hostedBuild(false, deployLogger); } - private void buildClusterResourceLimits(boolean throwIfSpecified, DeployLogger deployLogger) { + private ClusterResourceLimits hostedBuild(boolean throwIfSpecified, DeployLogger deployLogger) { Document clusterXml = XML.getDocument("" + " \n" + " \n" + @@ -163,7 +194,7 @@ public class ClusterResourceLimitsTest { true, throwIfSpecified, deployLogger); - builder.build(new ModelElement(clusterXml.getDocumentElement())); + return builder.build(new ModelElement(clusterXml.getDocumentElement())); } private void assertLimits(Double expCtrlDisk, Double expCtrlMemory, Double expNodeDisk, Double expNodeMemory, Fixture f) { @@ -173,15 +204,15 @@ public class ClusterResourceLimitsTest { } private void assertLimits(Double expDisk, Double expMemory, ResourceLimits limits) { - assertLimit(expDisk, limits.getDiskLimit()); - assertLimit(expMemory, limits.getMemoryLimit()); + assertLimit(expDisk, limits.getDiskLimit(), "disk"); + assertLimit(expMemory, limits.getMemoryLimit(), "memory"); } - private void assertLimit(Double expLimit, Optional actLimit) { + private void assertLimit(Double expLimit, Optional actLimit, String limitType) { if (expLimit == null) { assertFalse(actLimit.isPresent()); } else { - assertEquals(expLimit, actLimit.get(), 0.00001); + assertEquals(limitType + " limit not as expected", expLimit, actLimit.get(), 0.00001); } } -- cgit v1.2.3