Add support for controlling gcopts in services.xml

7 files changed, 53 insertions, 41 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
index 502df074ec4..dc9e1c542d2 100755
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -181,6 +181,7 @@ public final class ContainerCluster
     private Zone zone;
     
     private String hostClusterId = null;
+    private String gcopts = null;
     private Integer memoryPercentage = null;
 
     private static class AcceptAllVerifier implements ContainerClusterVerifier {
@@ -784,6 +785,8 @@ public final class ContainerCluster
     public Optional<String> getHostClusterId() { return Optional.ofNullable(hostClusterId); }
 
     public void setMemoryPercentage(Integer memoryPercentage) { this.memoryPercentage = memoryPercentage; }
+    public void setGCOpts(String gcopts) { this.gcopts = gcopts; }
+    public Optional<String> getGCOpts() { return Optional.ofNullable(gcopts); }
 
     /** 
      * Returns the percentage of host physical memory this application has specified for nodes in this cluster,
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java
index c35da58575a..3d31106fd9f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java
@@ -28,6 +28,7 @@ import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * @author gjoranv
@@ -125,7 +126,10 @@ public class ContainerSearch extends ContainerSubsystem<SearchChains>
             internalBuilder.heapSizeAsPercentageOfPhysicalMemory(owningCluster.getHostClusterId().isPresent() ? 17 : 60);
         }
         qsB.jvm(internalBuilder.directMemorySizeCache(totalCacheSizeMb()));
-        if (owningCluster.isHostedVespa()) {
+        Optional<String> gcopts = owningCluster.getGCOpts();
+        if (gcopts.isPresent()) {
+            qsB.jvm.gcopts(gcopts.get());
+        } else if (owningCluster.isHostedVespa()) {
             if ((owningCluster.getZone().environment() != Environment.prod) || RegionName.from("us-east-3").equals(owningCluster.getZone().region())) {
                 qsB.jvm.gcopts("-XX:-UseConcMarkSweepGC -XX:+UseG1GC -XX:MaxTenuringThreshold=15");
             } else {
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 f3649361a74..aeb3123398c 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
@@ -460,8 +460,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         }
         else {
             List<Container> nodes = createNodes(cluster, nodesElement, context);
+            cluster.setGCOpts(nodesElement.getAttribute(VespaDomBuilder.GCOPTS_ATTRIB_NAME));
             applyNodesTagJvmArgs(nodes, nodesElement.getAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME));
-            applyNodesTagGCOpts(nodes, nodesElement.getAttribute(VespaDomBuilder.GCOPTS_ATTRIB_NAME));
             applyRoutingAliasProperties(nodes, cluster);
             applyDefaultPreload(nodes, nodesElement);
             applyMemoryPercentage(cluster, nodesElement.getAttribute(VespaDomBuilder.Allocated_MEMORY_ATTRIB_NAME));
@@ -660,16 +660,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
             return false;
     }
 
-    private void applyNodesTagJvmArgs(List<Container> containers, String nodesTagJvnArgs) {
+    private void applyNodesTagJvmArgs(List<Container> containers, String jvmArgs) {
         for (Container container: containers) {
             if (container.getAssignedJvmArgs().isEmpty())
-                container.prependJvmArgs(nodesTagJvnArgs);
-        }
-    }
-    private void applyNodesTagGCOpts(List<Container> containers, String nodesTagJvnArgs) {
-        for (Container container: containers) {
-            if (container.getAssignedJvmArgs().isEmpty())
-                container.prependJvmArgs(nodesTagJvnArgs);
+                container.prependJvmArgs(jvmArgs);
         }
     }
 
diff --git a/config-model/src/main/resources/schema/common.rnc b/config-model/src/main/resources/schema/common.rnc
index 85963764139..6a82556f01b 100644
--- a/config-model/src/main/resources/schema/common.rnc
+++ b/config-model/src/main/resources/schema/common.rnc
@@ -2,6 +2,7 @@
 service.attlist &= attribute hostalias { xsd:NCName }
 service.attlist &= attribute baseport { xsd:unsignedShort }?
 service.attlist &= attribute jvmargs { text }?
+service.attlist &= attribute gcopts { text }?
 # preload is for internal use only
 service.attlist &= attribute preload { text }?
 
diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc
index 4934ce113bb..4862fdf7a50 100644
--- a/config-model/src/main/resources/schema/containercluster.rnc
+++ b/config-model/src/main/resources/schema/containercluster.rnc
@@ -211,6 +211,7 @@ DocumentApi = element document-api {
 
 NodesOfContainerCluster = element nodes {
     attribute jvmargs { text }? &
+    attribute gcopts { text }? &
     attribute preload { text }? &
     attribute allocated-memory { text }? &
     attribute cpu-socket-affinity { xsd:boolean }? &
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 4a9a6d3dff3..839ddcecde3 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
@@ -87,7 +87,7 @@ public class ModelProvisioningTest {
                         "  <handler id='myHandler'>" +
                         "    <component id='injected' />" +
                         "  </handler>" +
-                        "  <nodes count='2' allocated-memory='45%' jvmargs='-verbosegc' preload='lib/blablamalloc.so'/>" +
+                        "  <nodes count='2' allocated-memory='45%' gcopts='-XX:+UseParNewGC' jvmargs='-verbosegc' preload='lib/blablamalloc.so'/>" +
                         "</jdisc>" +
                         "</services>";
         String hosts ="<hosts>"
@@ -112,35 +112,38 @@ public class ModelProvisioningTest {
                 + "</hosts>";
         VespaModelCreatorWithMockPkg creator = new VespaModelCreatorWithMockPkg(null, services);
         VespaModel model = creator.create(new DeployState.Builder().modelHostProvisioner(new InMemoryProvisioner(Hosts.readFrom(new StringReader(hosts)), true)));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().size(), is(3));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(0).getConfigId(), is("mydisc/container.0"));
-        assertTrue(model.getContainerClusters().get("mydisc").getContainers().get(0).isInitialized());
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(1).getConfigId(), is("mydisc/container.1"));
-        assertTrue(model.getContainerClusters().get("mydisc").getContainers().get(1).isInitialized());
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(2).getConfigId(), is("mydisc/container.2"));
-        assertTrue(model.getContainerClusters().get("mydisc").getContainers().get(2).isInitialized());
-
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().size(), is(2));
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(0).getConfigId(), is("mydisc2/container.0"));
-        assertTrue(model.getContainerClusters().get("mydisc2").getContainers().get(0).isInitialized());
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(1).getConfigId(), is("mydisc2/container.1"));
-        assertTrue(model.getContainerClusters().get("mydisc2").getContainers().get(1).isInitialized());
-
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(0).getJvmArgs(), is(""));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(1).getJvmArgs(), is(""));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(2).getJvmArgs(), is(""));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(0).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(1).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
-        assertThat(model.getContainerClusters().get("mydisc").getContainers().get(2).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
-        assertThat(model.getContainerClusters().get("mydisc").getMemoryPercentage(), is(Optional.empty()));
-
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(0).getJvmArgs(), is("-verbosegc"));
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(1).getJvmArgs(), is("-verbosegc"));
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(0).getPreLoad(), is("lib/blablamalloc.so"));
-        assertThat(model.getContainerClusters().get("mydisc2").getContainers().get(1).getPreLoad(), is("lib/blablamalloc.so"));
-        assertThat(model.getContainerClusters().get("mydisc2").getMemoryPercentage(), is(Optional.of(45)));
+        ContainerCluster mydisc = model.getContainerClusters().get("mydisc");
+        ContainerCluster mydisc2 = model.getContainerClusters().get("mydisc2");
+        assertThat(mydisc.getContainers().size(), is(3));
+        assertThat(mydisc.getContainers().get(0).getConfigId(), is("mydisc/container.0"));
+        assertTrue(mydisc.getContainers().get(0).isInitialized());
+        assertThat(mydisc.getContainers().get(1).getConfigId(), is("mydisc/container.1"));
+        assertTrue(mydisc.getContainers().get(1).isInitialized());
+        assertThat(mydisc.getContainers().get(2).getConfigId(), is("mydisc/container.2"));
+        assertTrue(mydisc.getContainers().get(2).isInitialized());
+
+        assertThat(mydisc2.getContainers().size(), is(2));
+        assertThat(mydisc2.getContainers().get(0).getConfigId(), is("mydisc2/container.0"));
+        assertTrue(mydisc2.getContainers().get(0).isInitialized());
+        assertThat(mydisc2.getContainers().get(1).getConfigId(), is("mydisc2/container.1"));
+        assertTrue(mydisc2.getContainers().get(1).isInitialized());
+
+        assertThat(mydisc.getContainers().get(0).getJvmArgs(), is(""));
+        assertThat(mydisc.getContainers().get(1).getJvmArgs(), is(""));
+        assertThat(mydisc.getContainers().get(2).getJvmArgs(), is(""));
+        assertThat(mydisc.getContainers().get(0).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
+        assertThat(mydisc.getContainers().get(1).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
+        assertThat(mydisc.getContainers().get(2).getPreLoad(), is(getDefaults().underVespaHome("lib64/vespa/malloc/libvespamalloc.so")));
+        assertThat(mydisc.getMemoryPercentage(), is(Optional.empty()));
+
+        assertThat(mydisc2.getContainers().get(0).getJvmArgs(), is("-verbosegc"));
+        assertThat(mydisc2.getContainers().get(1).getJvmArgs(), is("-verbosegc"));
+        assertThat(mydisc2.getContainers().get(0).getPreLoad(), is("lib/blablamalloc.so"));
+        assertThat(mydisc2.getContainers().get(1).getPreLoad(), is("lib/blablamalloc.so"));
+        assertThat(mydisc2.getMemoryPercentage(), is(Optional.of(45)));
+        assertThat(mydisc2.getGCOpts(), is(Optional.of("-XX:+UseParNewGC")));
         QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder();
-        model.getContainerClusters().get("mydisc2").getConfig(qrStartBuilder);
+        mydisc2.getConfig(qrStartBuilder);
         QrStartConfig qrsStartConfig = new QrStartConfig(qrStartBuilder);
         assertEquals(45, qrsStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory());
         
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
index 6e962146c79..ee86183dd9e 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
@@ -179,10 +179,11 @@ public class ContainerClusterTest {
         verifyJvmArgs(isHosted, hasDocProc, "", container.getJvmArgs());
     }
 
-    private void verifyGCOpts(boolean isHosted, Zone zone, String expected) {
+    private void verifyGCOpts(boolean isHosted, String override, Zone zone, String expected) {
         MockRoot root = createRoot(isHosted, zone);
         ContainerCluster cluster = createContainerCluster(root, false);
         addContainer(root.deployLogger(), cluster, "c1", "host-c1");
+        cluster.setGCOpts(override);
         assertEquals(1, cluster.getContainers().size());
         QrStartConfig.Builder qsB = new QrStartConfig.Builder();
         cluster.getSearch().getConfig(qsB);
@@ -190,9 +191,14 @@ public class ContainerClusterTest {
         assertEquals(expected, qsC.jvm().gcopts());
     }
 
+    private void verifyGCOpts(boolean isHosted, Zone zone, String expected) {
+        verifyGCOpts(isHosted, null, zone, expected);
+        verifyGCOpts(isHosted, "-XX:+UseG1GC", zone, "-XX:+UseG1GC");
+    }
+
     @Test
     public void requireThatGCOptsIsHonoured() {
-        verifyGCOpts(false, Zone.defaultZone(), "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1");
+        verifyGCOpts(false, Zone.defaultZone(),"-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1");
         verifyGCOpts(false, new Zone(Environment.prod, RegionName.from("us-east-3")), "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1");
         verifyGCOpts(true, Zone.defaultZone(), "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1");
         verifyGCOpts(true, new Zone(Environment.prod, RegionName.from("us-east-3")), "-XX:-UseConcMarkSweepGC -XX:+UseG1GC -XX:MaxTenuringThreshold=15");