diff options
author | Harald Musum <musum@yahooinc.com> | 2022-05-31 22:09:46 +0200 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2022-06-08 11:45:23 +0200 |
commit | de710ef8cc2055c338d6bf29243f74e40c36af91 (patch) | |
tree | 7cbb4650a3e4a0b070256a4275064324128b6b72 | |
parent | 4c6f2f1802a8a645bbcbff0dd9352252a21e8c0d (diff) |
Remove support for jvmargs attribute in nodes element in services.xml
7 files changed, 5 insertions, 92 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index b619210155f..ea66db7b79d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -41,7 +41,6 @@ import java.util.logging.Logger; */ public class VespaDomBuilder extends VespaModelBuilder { - public static final String JVMARGS_ATTRIB_NAME = "jvmargs"; public static final String JVM_OPTIONS = "jvm-options"; public static final String OPTIONS = "options"; public static final String JVM_GC_OPTIONS = "jvm-gc-options"; @@ -145,10 +144,6 @@ public class VespaDomBuilder extends VespaModelBuilder { if (producerSpec != null) { if (producerSpec.hasAttribute(JVM_OPTIONS)) { t.appendJvmOptions(producerSpec.getAttribute(JVM_OPTIONS)); - } else { - if (producerSpec.hasAttribute(JVMARGS_ATTRIB_NAME)) { - t.appendJvmOptions(producerSpec.getAttribute(JVMARGS_ATTRIB_NAME)); - } } if (producerSpec.hasAttribute(PRELOAD_ATTRIB_NAME)) { t.setPreLoad(producerSpec.getAttribute(PRELOAD_ATTRIB_NAME)); 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 2b3ab4f15dd..d1a176e37d5 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 @@ -671,12 +671,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { cluster.addContainers(Collections.singleton(container)); } - static boolean incompatibleGCOptions(String jvmargs) { - Pattern gcAlgorithm = Pattern.compile("-XX:[-+]Use.+GC"); - Pattern cmsArgs = Pattern.compile("-XX:[-+]*CMS"); - return (gcAlgorithm.matcher(jvmargs).find() || cmsArgs.matcher(jvmargs).find()); - } - private static String buildJvmGCOptions(ConfigModelContext context, String jvmGCOptions) { return new JvmGcOptions(context.getDeployState(), jvmGCOptions).build(); } @@ -1107,33 +1101,13 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } String buildLegacyOptions() { - String jvmOptions; + String jvmOptions = null; if (nodesElement.hasAttribute(VespaDomBuilder.JVM_OPTIONS)) { jvmOptions = nodesElement.getAttribute(VespaDomBuilder.JVM_OPTIONS); - if (nodesElement.hasAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME)) { - String jvmArgs = nodesElement.getAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME); - throw new IllegalArgumentException("You have specified both deprecated jvm-options='" + jvmOptions + "'" + - " and deprecated jvmargs='" + jvmArgs + - "'. 'jvm-options' and 'jvmargs' are deprecated and will be removed in Vespa 8." + - " Please merge 'jvmargs' into 'options' or 'gc-options' in 'jvm' element." + - " See https://docs.vespa.ai/en/reference/services-container.html#jvm"); - } if (! jvmOptions.isEmpty()) logger.logApplicationPackage(WARNING, "'jvm-options' is deprecated and will be removed in Vespa 8." + " Please merge 'jvm-options' into 'options' or 'gc-options' in 'jvm' element." + " See https://docs.vespa.ai/en/reference/services-container.html#jvm"); - } else { - jvmOptions = nodesElement.getAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME); - if (incompatibleGCOptions(jvmOptions)) { - logger.logApplicationPackage(WARNING, "You need to move your GC-related options from deprecated 'jvmargs'" + - " to 'gc-options' in 'jvm' element. 'jvmargs' is deprecated and will be removed in Vespa 8." + - " See https://docs.vespa.ai/en/reference/services-container.html#jvm"); - cluster.setJvmGCOptions(ContainerCluster.G1GC); - } - if (! jvmOptions.isEmpty()) - logger.logApplicationPackage(WARNING, "'jvmargs' is deprecated and will be removed in Vespa 8." + - " Please merge 'jvmargs' into 'options' or 'gc-options' in 'jvm' element." + - " See https://docs.vespa.ai/en/reference/services-container.html#jvm"); } validateJvmOptions(jvmOptions); diff --git a/config-model/src/main/resources/schema/common.rnc b/config-model/src/main/resources/schema/common.rnc index 275c0f7f702..10ba568ca30 100644 --- a/config-model/src/main/resources/schema/common.rnc +++ b/config-model/src/main/resources/schema/common.rnc @@ -1,7 +1,6 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. service.attlist &= attribute hostalias { xsd:NCName } service.attlist &= attribute baseport { xsd:unsignedShort }? -service.attlist &= attribute jvmargs { text }? service.attlist &= attribute jvm-options { text }? service.attlist &= attribute jvm-gc-options { text }? # preload is for internal use only diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc index decf955e33b..aee003d7339 100644 --- a/config-model/src/main/resources/schema/containercluster.rnc +++ b/config-model/src/main/resources/schema/containercluster.rnc @@ -226,7 +226,6 @@ HttpClientApi = element http-client-api { # NODES: NodesOfContainerCluster = element nodes { - attribute jvmargs { text }? & attribute jvm-options { text }? & attribute jvm-gc-options { text }? & attribute preload { text }? & diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index f164a1045b6..f183020cb22 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -1549,22 +1549,6 @@ public class ModelProvisioningTest { } @Test - public void testJvmArgs() { - String services = - "<?xml version='1.0' encoding='utf-8' ?>\n" + - "<container version='1.0'>" + - " <search/>" + - " <nodes jvmargs='-DfooOption=xyz' count='3'/>" + - "</container>"; - int numberOfHosts = 3; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(numberOfHosts); - VespaModel model = tester.createModel(services, true); - assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size()); - assertEquals("-DfooOption=xyz", model.getContainerClusters().get("container").getContainers().get(0).getAssignedJvmOptions()); - } - - @Test public void testJvmOptions() { String services = "<?xml version='1.0' encoding='utf-8' ?>\n" + @@ -1583,30 +1567,6 @@ public class ModelProvisioningTest { } @Test - public void testFailWhenBothJvmOptionsAndJvmArgs() { - String services = - "<?xml version='1.0' encoding='utf-8' ?>\n" + - "<container version='1.0'>" + - " <search/>" + - " <nodes jvm-options='xyz' jvmargs='abc' count='3'/>" + - "</container>"; - int numberOfHosts = 3; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(numberOfHosts); - try { - tester.createModel(services, true); - fail("Expected exception"); - } - catch (IllegalArgumentException e) { - assertEquals("You have specified both deprecated jvm-options='xyz' and deprecated jvmargs='abc'. " + - "'jvm-options' and 'jvmargs' are deprecated and will be removed in Vespa 8. " + - "Please merge 'jvmargs' into 'options' or 'gc-options' in 'jvm' element. " + - "See https://docs.vespa.ai/en/reference/services-container.html#jvm", - e.getMessage()); - } - } - - @Test public void testUsingHostaliasWithProvisioner() { String services = "<?xml version='1.0' encoding='utf-8' ?>\n" + diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 0d1a9351923..527b29f398d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -1040,6 +1040,7 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { createModel(root, deployState, null, DomBuilderTest.parse(containerService)); assertFalse(logger.msgs.isEmpty()); + System.out.println(logger.msgs); assertEquals(Level.WARNING, logger.msgs.get(0).getFirst()); assertEquals(Level.WARNING, logger.msgs.get(1).getFirst()); assertEquals("Element 'prod' contains attribute 'global-service-id' deprecated since major version 7. See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax", 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 b2c29b88e38..146e383078e 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 @@ -15,7 +15,6 @@ import com.yahoo.vespa.model.container.ContainerCluster; import org.junit.Test; import org.w3c.dom.Element; import org.xml.sax.SAXException; - import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -23,7 +22,6 @@ import java.util.List; import java.util.logging.Level; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -57,14 +55,6 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { assertEquals(45, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); assertEquals("-XX:SoftRefLRUPolicyMSPerMB=2500", model.getContainerClusters().values().iterator().next().getContainers().get(0).getJvmOptions()); } - @Test - public void detect_conflicting_jvmgcoptions_in_jvmargs() { - assertFalse(ContainerModelBuilder.incompatibleGCOptions("")); - assertFalse(ContainerModelBuilder.incompatibleGCOptions("UseG1GC")); - assertTrue(ContainerModelBuilder.incompatibleGCOptions("-XX:+UseG1GC")); - assertTrue(ContainerModelBuilder.incompatibleGCOptions("abc -XX:+UseParNewGC xyz")); - assertTrue(ContainerModelBuilder.incompatibleGCOptions("-XX:CMSInitiatingOccupancyFraction=19")); - } @Test public void honours_jvm_gc_options() { @@ -83,14 +73,9 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { } private static void verifyIgnoreJvmGCOptions(boolean isHosted) throws IOException, SAXException { - verifyIgnoreJvmGCOptionsIfJvmArgs("jvmargs", ContainerCluster.G1GC, isHosted); - verifyIgnoreJvmGCOptionsIfJvmArgs( "jvm-options", "-XX:+UseG1GC", isHosted); - - } - private static void verifyIgnoreJvmGCOptionsIfJvmArgs(String jvmOptionsName, String expectedGC, boolean isHosted) throws IOException, SAXException { String servicesXml = "<container version='1.0'>" + - " <nodes jvm-gc-options='-XX:+UseG1GC' " + jvmOptionsName + "='-XX:+UseParNewGC'>" + + " <nodes jvm-gc-options='-XX:+UseG1GC' jvm-options='-XX:+UseParNewGC'>" + " <node hostalias='mockhost'/>" + " </nodes>" + "</container>"; @@ -105,11 +90,11 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder(); model.getConfig(qrStartBuilder, "container/container.0"); QrStartConfig qrStartConfig = new QrStartConfig(qrStartBuilder); - assertEquals(expectedGC, qrStartConfig.jvm().gcopts()); + assertEquals("-XX:+UseG1GC", qrStartConfig.jvm().gcopts()); } @Test - public void ignores_jvmgcoptions_on_conflicting_jvmargs() throws IOException, SAXException { + public void ignores_jvmgcoptions_on_conflicting_jvmoptions() throws IOException, SAXException { verifyIgnoreJvmGCOptions(false); verifyIgnoreJvmGCOptions(true); } |