From b7dedde0a83d681966641e2cb4439ad60bf07702 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 20 Dec 2021 09:59:33 +0100 Subject: LOg or fail deployments with invalid JVM GC options --- .../com/yahoo/config/model/deploy/TestProperties.java | 8 ++++++++ .../vespa/model/container/xml/ContainerModelBuilder.java | 15 ++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) (limited to 'config-model/src/main/java/com/yahoo') diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index cf649162c08..b0c1dfadfd7 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -76,6 +76,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean unorderedMergeChaining = false; private List zoneDnsSuffixes = List.of(); private int maxCompactBuffers = 1; + private boolean failDeploymentWithInvalidJvmOptions = false; @Override public ModelContext.FeatureFlags featureFlags() { return this; } @Override public boolean multitenant() { return multitenant; } @@ -132,6 +133,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean unorderedMergeChaining() { return unorderedMergeChaining; } @Override public List zoneDnsSuffixes() { return zoneDnsSuffixes; } @Override public int maxCompactBuffers() { return maxCompactBuffers; } + @Override public boolean failDeploymentWithInvalidJvmOptions() { return failDeploymentWithInvalidJvmOptions; } + public TestProperties maxUnCommittedMemory(int maxUnCommittedMemory) { this.maxUnCommittedMemory = maxUnCommittedMemory; @@ -347,6 +350,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties failDeploymentWithInvalidJvmOptions(boolean fail) { + failDeploymentWithInvalidJvmOptions = fail; + return this; + } + public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d7691f4c486..a8d876f904c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -1140,12 +1140,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder { private final String jvmGcOptions; private final DeployLogger logger; private final boolean isHosted; + private final boolean failDeploymentWithInvalidJvmOptions; public JvmGcOptions(DeployState deployState, String jvmGcOptions) { this.deployState = deployState; this.jvmGcOptions = jvmGcOptions; this.logger = deployState.getDeployLogger(); this.isHosted = deployState.isHosted(); + this.failDeploymentWithInvalidJvmOptions = deployState.featureFlags().failDeploymentWithInvalidJvmOptions(); } private String build() { @@ -1162,13 +1164,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder { // CMS GC options cannot be used in hosted, CMS is unsupported in JDK 17 invalidOptions.addAll(Arrays.stream(optionList) .filter(option -> !option.isEmpty()) - .filter(option -> Pattern - .matches(invalidCMSPattern.pattern(), option) || + .filter(option -> Pattern.matches(invalidCMSPattern.pattern(), option) || option.equals("-XX:+UseConcMarkSweepGC")) .collect(Collectors.toList())); } - logInvalidOptions(invalidOptions); + logOrFailInvalidOptions(invalidOptions); } if (options == null || options.isEmpty()) @@ -1177,11 +1178,15 @@ public class ContainerModelBuilder extends ConfigModelBuilder { return options; } - private void logInvalidOptions(List options) { + private void logOrFailInvalidOptions(List options) { if (options.isEmpty()) return; Collections.sort(options); - logger.logApplicationPackage(WARNING, "Invalid JVM GC options from services.xml: " + String.join(",", options)); + String message = "Invalid JVM GC options from services.xml: " + String.join(",", options); + if (failDeploymentWithInvalidJvmOptions) + throw new IllegalArgumentException(message); + else + logger.logApplicationPackage(WARNING, message); } } -- cgit v1.2.3 From 4aa046f63e62c50ba7b11deed3d15a00890b1c2f Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 20 Dec 2021 10:09:23 +0100 Subject: Handle JVM GC options without x or - --- .../com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java | 2 +- .../test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'config-model/src/main/java/com/yahoo') diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index a8d876f904c..c8b2197fc29 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -1133,7 +1133,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { */ private static class JvmGcOptions { - private static final Pattern validPattern = Pattern.compile("-XX:[+-][a-zA-z0-9=]+"); + private static final Pattern validPattern = Pattern.compile("-XX:[+-]*[a-zA-z0-9=]+"); private static final Pattern invalidCMSPattern = Pattern.compile("-XX:[+-]\\w*CMS[a-zA-z0-9=]+"); private final DeployState deployState; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java index a1b27299d56..5fe2baaca37 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java @@ -171,8 +171,8 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { "$(touch", "/tmp/hello-from-gc-options)"); // Valid options, should not log anything - verifyLoggingOfJvmGcOptions(true, - "-XX:+ParallelGCThreads=8"); + verifyLoggingOfJvmGcOptions(true, "-XX:+ParallelGCThreads=8"); + verifyLoggingOfJvmGcOptions(true, "-XX:MaxTenuringThreshold"); // No + or - after colon verifyLoggingOfJvmGcOptions(false, "-XX:+UseConcMarkSweepGC"); } -- cgit v1.2.3