From 223209447bdb47ee4ea2e50fa847970498d62826 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 10 Sep 2018 13:14:32 +0200 Subject: Specify default GC arguments as default in config. --- container-search/src/main/resources/configdefinitions/qr-start.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container-search/src/main/resources/configdefinitions/qr-start.def b/container-search/src/main/resources/configdefinitions/qr-start.def index 948f360ea63..d29ebc8a826 100644 --- a/container-search/src/main/resources/configdefinitions/qr-start.def +++ b/container-search/src/main/resources/configdefinitions/qr-start.def @@ -9,7 +9,7 @@ jvm.server bool default=true restart jvm.verbosegc bool default=true restart ## Garbage Collection tuning parameters -jvm.gcopts string default="" restart +jvm.gcopts string default="-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1" restart ## Heap size (in megabytes) for the Java VM jvm.heapsize int default=1536 restart -- cgit v1.2.3 From 88978b97b607c80686eead038ba40bfbc3d6bb27 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 11 Sep 2018 09:35:41 +0200 Subject: Expect gcopts in config --- .../java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java index f4d3fbc782c..5acfe9312f6 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java @@ -217,7 +217,7 @@ public class DocprocBuilderTest extends DomBuilderTest { QrStartConfig.Jvm jvm = qrStartConfig.jvm(); assertThat(jvm.server(), is(true)); assertThat(jvm.verbosegc(), is(true)); - assertThat(jvm.gcopts(), is("")); + assertThat(jvm.gcopts(), is("-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1")); assertThat(jvm.heapsize(), is(1536)); assertThat(jvm.stacksize(), is(512)); assertThat(qrStartConfig.ulimitv(), is("")); -- cgit v1.2.3 From 176ba4c24e6c3479686b60276de0301f7d7a1b2a Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 11:44:05 +0200 Subject: Control gcopts from model. Default is now g1gc for hosted && (!prod || region=us-east-3) --- .../model/builder/xml/dom/VespaDomBuilder.java | 1 + .../model/container/search/ContainerSearch.java | 12 +++++++++++ .../model/container/xml/ContainerModelBuilder.java | 7 +++++++ .../model/container/ContainerClusterTest.java | 24 ++++++++++++++++++++++ 4 files changed, 44 insertions(+) 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 9fafe89ea54..1f6f7ad6c69 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 @@ -44,6 +44,7 @@ import java.util.logging.Logger; public class VespaDomBuilder extends VespaModelBuilder { public static final String JVMARGS_ATTRIB_NAME = "jvmargs"; + public static final String GCOPTS_ATTRIB_NAME = "gcopts"; public static final String PRELOAD_ATTRIB_NAME = "preload"; // Intended for vespa engineers public static final String MMAP_NOCORE_LIMIT = "mmap-core-limit"; // Intended for vespa engineers public static final String CORE_ON_OOM = "core-on-oom"; // Intended for vespa engineers 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 2cfd4b067cd..c35da58575a 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 @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container.search; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.prelude.fastsearch.FS4ResourcePool; @@ -54,6 +56,7 @@ public class ContainerSearch extends ContainerSubsystem this.options = options; this.owningCluster = cluster; cluster.addComponent(getFS4ResourcePool()); + } private Component getFS4ResourcePool() { @@ -122,6 +125,15 @@ public class ContainerSearch extends ContainerSubsystem internalBuilder.heapSizeAsPercentageOfPhysicalMemory(owningCluster.getHostClusterId().isPresent() ? 17 : 60); } qsB.jvm(internalBuilder.directMemorySizeCache(totalCacheSizeMb())); + 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 { + qsB.jvm.gcopts("-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"); + } + } else { + qsB.jvm.gcopts("-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"); + } } private int totalCacheSizeMb() { 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 89f2995d179..f3649361a74 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 @@ -461,6 +461,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { else { List nodes = createNodes(cluster, nodesElement, context); 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)); @@ -665,6 +666,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder { container.prependJvmArgs(nodesTagJvnArgs); } } + private void applyNodesTagGCOpts(List containers, String nodesTagJvnArgs) { + for (Container container: containers) { + if (container.getAssignedJvmArgs().isEmpty()) + container.prependJvmArgs(nodesTagJvnArgs); + } + } private void applyDefaultPreload(List containers, Element nodesElement) { if (! nodesElement.hasAttribute(VespaDomBuilder.PRELOAD_ATTRIB_NAME)) return; 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 8e096b14d85..6e962146c79 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 @@ -107,6 +107,10 @@ public class ContainerClusterTest { DeployState state = new DeployState.Builder().properties(new DeployProperties.Builder().hostedVespa(isHosted).build()).build(); return new MockRoot("foo", state); } + private MockRoot createRoot(boolean isHosted, Zone zone) { + DeployState state = new DeployState.Builder().zone(zone).properties(new DeployProperties.Builder().hostedVespa(isHosted).build()).build(); + return new MockRoot("foo", state); + } private ContainerCluster createContainerCluster(MockRoot root, boolean isCombinedCluster, Integer memoryPercentage, Optional extraComponents) { @@ -154,6 +158,7 @@ public class ContainerClusterTest { assertEquals(expectedArgs, jvmArgs); } } + private void verifyJvmArgs(boolean isHosted, boolean hasDocProc) { MockRoot root = createRoot(isHosted); ContainerCluster cluster = createContainerCluster(root, false); @@ -174,6 +179,25 @@ public class ContainerClusterTest { verifyJvmArgs(isHosted, hasDocProc, "", container.getJvmArgs()); } + private void verifyGCOpts(boolean isHosted, Zone zone, String expected) { + MockRoot root = createRoot(isHosted, zone); + ContainerCluster cluster = createContainerCluster(root, false); + addContainer(root.deployLogger(), cluster, "c1", "host-c1"); + assertEquals(1, cluster.getContainers().size()); + QrStartConfig.Builder qsB = new QrStartConfig.Builder(); + cluster.getSearch().getConfig(qsB); + QrStartConfig qsC= new QrStartConfig(qsB); + assertEquals(expected, qsC.jvm().gcopts()); + } + + @Test + public void requireThatGCOptsIsHonoured() { + 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"); + } + @Test public void testContainerClusterMaxThreads() { MockRoot root = createRoot(false); -- cgit v1.2.3 From 9c9e523ae0bbc1256886d9809fce9a1e8bb2438f Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 12:23:26 +0200 Subject: Add support for controlling gcopts in services.xml --- .../vespa/model/container/ContainerCluster.java | 3 ++ .../model/container/search/ContainerSearch.java | 6 ++- .../model/container/xml/ContainerModelBuilder.java | 12 ++--- config-model/src/main/resources/schema/common.rnc | 1 + .../src/main/resources/schema/containercluster.rnc | 1 + .../model/provision/ModelProvisioningTest.java | 61 ++++++++++++---------- .../model/container/ContainerClusterTest.java | 10 +++- 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 getHostClusterId() { return Optional.ofNullable(hostClusterId); } public void setMemoryPercentage(Integer memoryPercentage) { this.memoryPercentage = memoryPercentage; } + public void setGCOpts(String gcopts) { this.gcopts = gcopts; } + public Optional 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 internalBuilder.heapSizeAsPercentageOfPhysicalMemory(owningCluster.getHostClusterId().isPresent() ? 17 : 60); } qsB.jvm(internalBuilder.directMemorySizeCache(totalCacheSizeMb())); - if (owningCluster.isHostedVespa()) { + Optional 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 { } else { List 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 { return false; } - private void applyNodesTagJvmArgs(List containers, String nodesTagJvnArgs) { + private void applyNodesTagJvmArgs(List containers, String jvmArgs) { for (Container container: containers) { if (container.getAssignedJvmArgs().isEmpty()) - container.prependJvmArgs(nodesTagJvnArgs); - } - } - private void applyNodesTagGCOpts(List 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 { " " + " " + " " + - " " + + " " + "" + ""; String hosts ="" @@ -112,35 +112,38 @@ public class ModelProvisioningTest { + ""; 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"); -- cgit v1.2.3 From 3f77ed130bcd58f4aaa9c1e792851f168328128c Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 13:35:24 +0200 Subject: If System name is dev enable G1GC --- .../vespa/model/container/ContainerCluster.java | 2 ++ .../model/container/search/ContainerSearch.java | 31 +++++++++++++--------- .../model/container/ContainerClusterTest.java | 13 ++++++--- 3 files changed, 29 insertions(+), 17 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 dc9e1c542d2..4c283fa42ad 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 @@ -140,6 +140,8 @@ public final class ContainerCluster public static final String STATE_HANDLER_CLASS = "com.yahoo.container.jdisc.state.StateHandler"; public static final String STATISTICS_HANDLER_CLASS = "com.yahoo.container.config.StatisticsRequestHandler"; public static final String SIMPLE_LINGUISTICS_PROVIDER = "com.yahoo.language.provider.SimpleLinguisticsProvider"; + public static final String CMS = "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"; + public static final String G1GC = "-XX:-UseConcMarkSweepGC -XX:+UseG1GC -XX:MaxTenuringThreshold=15"; public static final String ROOT_HANDLER_BINDING = "*://*/"; 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 3d31106fd9f..46fcf38b7e9 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 @@ -3,6 +3,8 @@ package com.yahoo.vespa.model.container.search; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.Zone; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.prelude.fastsearch.FS4ResourcePool; @@ -117,6 +119,20 @@ public class ContainerSearch extends ContainerSubsystem if (pageTemplates!=null) pageTemplates.getConfig(builder); } + private String buildGCOpts(Zone zone) { + Optional gcopts = owningCluster.getGCOpts(); + if (gcopts.isPresent()) { + return gcopts.get(); + } else if (zone.system() == SystemName.dev) { + return ContainerCluster.G1GC; + } else if (owningCluster.isHostedVespa()) { + return ((zone.environment() != Environment.prod) || RegionName.from("us-east-3").equals(zone.region())) + ? ContainerCluster.G1GC : ContainerCluster.CMS; + } else { + return ContainerCluster.CMS; + } + } + @Override public void getConfig(QrStartConfig.Builder qsB) { QrStartConfig.Jvm.Builder internalBuilder = new QrStartConfig.Jvm.Builder(); @@ -126,18 +142,7 @@ public class ContainerSearch extends ContainerSubsystem internalBuilder.heapSizeAsPercentageOfPhysicalMemory(owningCluster.getHostClusterId().isPresent() ? 17 : 60); } qsB.jvm(internalBuilder.directMemorySizeCache(totalCacheSizeMb())); - Optional 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 { - qsB.jvm.gcopts("-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"); - } - } else { - qsB.jvm.gcopts("-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"); - } + qsB.jvm.gcopts(buildGCOpts(owningCluster.getZone())); } private int totalCacheSizeMb() { @@ -208,6 +213,6 @@ public class ContainerSearch extends ContainerSubsystem * Struct that encapsulates qrserver options. */ public static class Options { - public Map cacheSettings = new LinkedHashMap<>(); + Map cacheSettings = new LinkedHashMap<>(); } } 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 ee86183dd9e..7051ffda84a 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 @@ -194,14 +194,19 @@ public class ContainerClusterTest { private void verifyGCOpts(boolean isHosted, Zone zone, String expected) { verifyGCOpts(isHosted, null, zone, expected); verifyGCOpts(isHosted, "-XX:+UseG1GC", zone, "-XX:+UseG1GC"); + Zone DEV = new Zone(SystemName.dev, zone.environment(), zone.region()); + verifyGCOpts(isHosted, null, DEV, ContainerCluster.G1GC); + verifyGCOpts(isHosted, "-XX:+UseConcMarkSweepGC", DEV, "-XX:+UseConcMarkSweepGC"); + } @Test public void requireThatGCOptsIsHonoured() { - 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"); + final Zone US_EAST_3 = new Zone(Environment.prod, RegionName.from("us-east-3")); + verifyGCOpts(false, Zone.defaultZone(),ContainerCluster.CMS); + verifyGCOpts(false, US_EAST_3, ContainerCluster.CMS); + verifyGCOpts(true, Zone.defaultZone(), ContainerCluster.CMS); + verifyGCOpts(true, US_EAST_3, ContainerCluster.G1GC); } @Test -- cgit v1.2.3 From b2aaa73603b6b21df37639d73ed3410d3d680c42 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 15:28:46 +0200 Subject: Detect conflicting jvmargs. --- .../model/container/xml/ContainerModelBuilder.java | 19 ++++++++-- .../container/xml/ContainerModelBuilderTest.java | 41 ++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) 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 aeb3123398c..29763839dac 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 @@ -76,6 +76,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; +import java.util.logging.Level; +import java.util.regex.Pattern; import java.util.stream.Collectors; /** @@ -449,6 +451,13 @@ public class ContainerModelBuilder extends ConfigModelBuilder { cluster.addContainers(Collections.singleton(container)); } + static boolean incompatibleGCOptions(String jvmargs) { + Pattern gcAlgorithm = Pattern.compile("-XX:[-+]Use.+GC"); + if (gcAlgorithm.matcher(jvmargs).find()) { + return true; + } + return false; + } private void addNodesFromXml(ContainerCluster cluster, Element containerElement, ConfigModelContext context) { Element nodesElement = XML.getChild(containerElement, "nodes"); if (nodesElement == null) { // default single node on localhost @@ -460,8 +469,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder { } else { List nodes = createNodes(cluster, nodesElement, context); - cluster.setGCOpts(nodesElement.getAttribute(VespaDomBuilder.GCOPTS_ATTRIB_NAME)); - applyNodesTagJvmArgs(nodes, nodesElement.getAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME)); + String jvmArgs = nodesElement.getAttribute(VespaDomBuilder.JVMARGS_ATTRIB_NAME); + String gcopts = nodesElement.getAttribute(VespaDomBuilder.GCOPTS_ATTRIB_NAME); + if (incompatibleGCOptions(jvmArgs)) { + context.getDeployLogger().log(Level.WARNING, "You need to move out your GC related options from 'jvmargs' to 'gcopts'"); + } else { + cluster.setGCOpts(gcopts); + } + applyNodesTagJvmArgs(nodes, jvmArgs); applyRoutingAliasProperties(nodes, cluster); applyDefaultPreload(nodes, nodesElement); applyMemoryPercentage(cluster, nodesElement.getAttribute(VespaDomBuilder.Allocated_MEMORY_ATTRIB_NAME)); 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 f94ebab42a9..5d708dcddf4 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 @@ -28,6 +28,7 @@ import com.yahoo.container.usability.BindingsOverviewHandler; import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.net.HostName; import com.yahoo.prelude.cluster.QrMonitorConfig; +import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.Container; @@ -67,6 +68,46 @@ import static org.junit.Assert.fail; */ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { + @Test + public void detect_conflicting_gcoptions_in_jvmargs() { + assertFalse(ContainerModelBuilder.incompatibleGCOptions("")); + assertFalse(ContainerModelBuilder.incompatibleGCOptions("UseG1GC")); + assertTrue(ContainerModelBuilder.incompatibleGCOptions("-XX:+UseG1GC")); + assertTrue(ContainerModelBuilder.incompatibleGCOptions("abc -XX:+UseParNewGC xyz")); + } + + @Test + public void honours_gcopts() { + Element clusterElem = DomBuilderTest.parse( + "", + " ", + " ", + " ", + " ", + "" ); + createModel(root, clusterElem); + QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder(); + root.getConfig(qrStartBuilder, "jdisc/container.0"); + QrStartConfig qrStartConfig = new QrStartConfig(qrStartBuilder); + assertEquals("-XX:+UseG1GC", qrStartConfig.jvm().gcopts()); + } + + @Test + public void ignores_gcopts_on_conflicting_jvargs() { + Element clusterElem = DomBuilderTest.parse( + "", + " ", + " ", + " ", + " ", + "" ); + createModel(root, clusterElem); + QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder(); + root.getConfig(qrStartBuilder, "jdisc/container.0"); + QrStartConfig qrStartConfig = new QrStartConfig(qrStartBuilder); + assertEquals(ContainerCluster.CMS, qrStartConfig.jvm().gcopts()); + } + @Test public void default_port_is_4080() throws Exception { Element clusterElem = DomBuilderTest.parse( -- cgit v1.2.3 From 119dfac8566ee3807f723d9dae0370689bd7180a Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 15:36:19 +0200 Subject: Let the fallback be the old default. --- container-disc/src/main/sh/vespa-start-container-daemon.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container-disc/src/main/sh/vespa-start-container-daemon.sh b/container-disc/src/main/sh/vespa-start-container-daemon.sh index 69c8c03ad4e..21c9dc28022 100755 --- a/container-disc/src/main/sh/vespa-start-container-daemon.sh +++ b/container-disc/src/main/sh/vespa-start-container-daemon.sh @@ -115,7 +115,7 @@ configure_numactl() { } configure_gcopts() { - consider_fallback jvm_gcopts "-XX:+UseG1GC -XX:MaxTenuringThreshold=15" + consider_fallback jvm_gcopts "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1" if [ "$jvm_verbosegc" = "true" ]; then jvm_gcopts="${jvm_gcopts} -verbose:gc" fi -- cgit v1.2.3 From accef069897921c76559e09490fdc1447eb17a78 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Oct 2018 15:40:19 +0200 Subject: Since this is a replacement no need to turn off UseConcMarkSweepGC --- .../src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4c283fa42ad..fff339e6692 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 @@ -141,7 +141,7 @@ public final class ContainerCluster public static final String STATISTICS_HANDLER_CLASS = "com.yahoo.container.config.StatisticsRequestHandler"; public static final String SIMPLE_LINGUISTICS_PROVIDER = "com.yahoo.language.provider.SimpleLinguisticsProvider"; public static final String CMS = "-XX:+UseConcMarkSweepGC -XX:MaxTenuringThreshold=15 -XX:NewRatio=1"; - public static final String G1GC = "-XX:-UseConcMarkSweepGC -XX:+UseG1GC -XX:MaxTenuringThreshold=15"; + public static final String G1GC = "-XX:+UseG1GC -XX:MaxTenuringThreshold=15"; public static final String ROOT_HANDLER_BINDING = "*://*/"; -- cgit v1.2.3