Revert "Add feature flag control of default jvm gc options"

3 files changed, 36 insertions, 30 deletions

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 af3a8335475..b00eb900221 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
@@ -43,7 +43,6 @@ public class TestProperties implements ModelContext.Properties {
     private double threadPoolSizeFactor = 0.0;
     private double queueSizeFactor = 0.0;
     private String docprocLoadBalancerType = null;
-    private String jvmGCOptions = null;
     private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty();
     private AthenzDomain athenzDomain;
     private ApplicationRoles applicationRoles;
@@ -57,7 +56,6 @@ public class TestProperties implements ModelContext.Properties {
     @Override public boolean hostedVespa() { return hostedVespa; }
     @Override public Zone zone() { return zone; }
     @Override public Set<ContainerEndpoint> endpoints() { return endpoints; }
-    @Override public String jvmGCOptions() { return jvmGCOptions; }
 
     @Override public boolean isBootstrap() { return false; }
     @Override public boolean isFirstTimeDeployment() { return false; }
@@ -80,10 +78,6 @@ public class TestProperties implements ModelContext.Properties {
         docprocLoadBalancerType = type;
         return this;
     }
-    public TestProperties setJvmGCOptions(String gcOptions) {
-        jvmGCOptions = gcOptions;
-        return this;
-    }
     public TestProperties setDefaultTermwiseLimit(double limit) {
         defaultTermwiseLimit = limit;
         return this;
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 6ae01ad0177..559a4b8b668 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
@@ -513,14 +513,17 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return (gcAlgorithm.matcher(jvmargs).find() ||cmsArgs.matcher(jvmargs).find());
     }
 
-    private static String buildJvmGCOptions(DeployState deployState, String jvmGCOPtions) {
-        String options = (jvmGCOPtions != null)
-                ? jvmGCOPtions
-                : deployState.getProperties().jvmGCOptions();
-        return (options == null ||options.isEmpty()) ? ContainerCluster.G1GC : options;
+    private static String buildJvmGCOptions(Zone zone, String jvmGCOPtions, boolean isHostedVespa) {
+        if (jvmGCOPtions != null) {
+            return jvmGCOPtions;
+        } else if ((zone.system() == SystemName.dev) || isHostedVespa) {
+            return null;
+        } else {
+            return ContainerCluster.G1GC;
+        }
     }
     private static String getJvmOptions(ApplicationContainerCluster cluster, Element nodesElement, DeployLogger deployLogger) {
-        String jvmOptions;
+        String jvmOptions = "";
         if (nodesElement.hasAttribute(VespaDomBuilder.JVM_OPTIONS)) {
             jvmOptions = nodesElement.getAttribute(VespaDomBuilder.JVM_OPTIONS);
             if (nodesElement.hasAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME)) {
@@ -538,17 +541,15 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return jvmOptions;
     }
 
-    private static String extractAttribute(Element element, String attrName) {
-        return element.hasAttribute(attrName) ? element.getAttribute(attrName) : null;
-    }
-
     void extractJvmFromLegacyNodesTag(List<ApplicationContainer> nodes, ApplicationContainerCluster cluster,
                                       Element nodesElement, ConfigModelContext context) {
         applyNodesTagJvmArgs(nodes, getJvmOptions(cluster, nodesElement, context.getDeployLogger()));
 
         if (!cluster.getJvmGCOptions().isPresent()) {
-            String jvmGCOptions = extractAttribute(nodesElement, VespaDomBuilder.JVM_GC_OPTIONS);
-            cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState(), jvmGCOptions));
+            String jvmGCOptions = nodesElement.hasAttribute(VespaDomBuilder.JVM_GC_OPTIONS)
+                    ? nodesElement.getAttribute(VespaDomBuilder.JVM_GC_OPTIONS)
+                    : null;
+            cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState().zone(), jvmGCOptions, context.getDeployState().isHosted()));
         }
 
         applyMemoryPercentage(cluster, nodesElement.getAttribute(VespaDomBuilder.Allocated_MEMORY_ATTRIB_NAME));
@@ -558,8 +559,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
                        Element jvmElement, ConfigModelContext context) {
         applyNodesTagJvmArgs(nodes, jvmElement.getAttribute(VespaDomBuilder.OPTIONS));
         applyMemoryPercentage(cluster, jvmElement.getAttribute(VespaDomBuilder.Allocated_MEMORY_ATTRIB_NAME));
-        String jvmGCOptions = extractAttribute(jvmElement, VespaDomBuilder.GC_OPTIONS);
-        cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState(), jvmGCOptions));
+        String jvmGCOptions = jvmElement.hasAttribute(VespaDomBuilder.GC_OPTIONS)
+                ? jvmElement.getAttribute(VespaDomBuilder.GC_OPTIONS)
+                : null;
+        cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState().zone(), jvmGCOptions, context.getDeployState().isHosted()));
     }
 
     /**
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 448ffffc04b..484709a0c18 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
@@ -50,6 +50,7 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder()
                 .applicationPackage(applicationPackage)
                 .deployLogger(logger)
+                .properties(new TestProperties().setHostedVespa(true))
                 .build());
         QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder();
         model.getConfig(qrStartBuilder, "container/container.0");
@@ -84,11 +85,11 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
     }
 
     private static void verifyIgnoreJvmGCOptions(boolean isHosted) throws IOException, SAXException {
-        verifyIgnoreJvmGCOptionsIfJvmArgs("jvmargs", ContainerCluster.G1GC);
-        verifyIgnoreJvmGCOptionsIfJvmArgs( "jvm-options", "-XX:+UseG1GC");
+        verifyIgnoreJvmGCOptionsIfJvmArgs(isHosted, "jvmargs", ContainerCluster.G1GC);
+        verifyIgnoreJvmGCOptionsIfJvmArgs(isHosted, "jvm-options", "-XX:+UseG1GC");
 
     }
-    private static void verifyIgnoreJvmGCOptionsIfJvmArgs(String jvmOptionsName, String expectedGC) throws IOException, SAXException {
+    private static void verifyIgnoreJvmGCOptionsIfJvmArgs(boolean isHosted, String jvmOptionsName, String expectedGC) throws IOException, SAXException {
         String servicesXml =
                 "<container version='1.0'>" +
                         "  <nodes jvm-gc-options='-XX:+UseG1GC' " + jvmOptionsName + "='-XX:+UseParNewGC'>" +
@@ -101,6 +102,8 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder()
                 .applicationPackage(applicationPackage)
                 .deployLogger(logger)
+                .zone(new Zone(SystemName.cd, Environment.dev, RegionName.from("here")))
+                .properties(new TestProperties().setHostedVespa(isHosted))
                 .build());
         QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder();
         model.getConfig(qrStartBuilder, "container/container.0");
@@ -114,7 +117,7 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         verifyIgnoreJvmGCOptions(true);
     }
 
-    private void verifyJvmGCOptions(String featureFlagDefault, String override, String expected) throws IOException, SAXException  {
+    private void verifyJvmGCOptions(boolean isHosted, String override, Zone zone, String expected) throws IOException, SAXException  {
         String servicesXml =
                 "<container version='1.0'>" +
                         "  <nodes " + ((override == null) ? ">" : ("jvm-gc-options='" + override + "'>")) +
@@ -127,7 +130,8 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder()
                 .applicationPackage(applicationPackage)
                 .deployLogger(logger)
-                .properties(new TestProperties().setJvmGCOptions(featureFlagDefault))
+                .zone(zone)
+                .properties(new TestProperties().setHostedVespa(isHosted))
                 .build());
         QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder();
         model.getConfig(qrStartBuilder, "container/container.0");
@@ -135,13 +139,18 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         assertEquals(expected, qrStartConfig.jvm().gcopts());
     }
 
+    private void verifyJvmGCOptions(boolean isHosted, Zone zone, String expected)  throws IOException, SAXException {
+        verifyJvmGCOptions(isHosted, null, zone, expected);
+        verifyJvmGCOptions(isHosted, "-XX:+UseG1GC", zone, "-XX:+UseG1GC");
+        Zone DEV = new Zone(SystemName.dev, zone.environment(), zone.region());
+        verifyJvmGCOptions(isHosted, null, DEV, ContainerCluster.G1GC);
+        verifyJvmGCOptions(isHosted, "-XX:+UseConcMarkSweepGC", DEV, "-XX:+UseConcMarkSweepGC");
+    }
+
     @Test
     public void requireThatJvmGCOptionsIsHonoured()  throws IOException, SAXException {
-        verifyJvmGCOptions(null,null, ContainerCluster.G1GC);
-        verifyJvmGCOptions("-XX:+UseConcMarkSweepGC",null, "-XX:+UseConcMarkSweepGC");
-        verifyJvmGCOptions(null,"-XX:+UseG1GC", "-XX:+UseG1GC");
-        verifyJvmGCOptions("-XX:+UseConcMarkSweepGC","-XX:+UseG1GC", "-XX:+UseG1GC");
-        verifyJvmGCOptions(null,"-XX:+UseConcMarkSweepGC", "-XX:+UseConcMarkSweepGC");
+        verifyJvmGCOptions(false, Zone.defaultZone(),ContainerCluster.G1GC);
+        verifyJvmGCOptions(true, Zone.defaultZone(), ContainerCluster.G1GC);
     }
 
 }