diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-06-10 08:48:11 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2020-06-10 08:48:11 +0000 |
commit | 845281768802fc72fe10cc11ea8c1532e9c48524 (patch) | |
tree | 6857ff3132365869360a030501b18dae7343c4be | |
parent | fbc3461127b6edf235e11e231fdbde0b250dd4f4 (diff) |
Add feature flag control of default jvm gc options
6 files changed, 42 insertions, 36 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index ad91ad6e1bc..86563552ebd 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -89,6 +89,8 @@ public interface ModelContext { String docprocLoadBalancerType(); + String jvmGCOptions(); + boolean useDistributorBtreeDb(); boolean useThreePhaseUpdates(); 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 b00eb900221..af3a8335475 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,6 +43,7 @@ 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; @@ -56,6 +57,7 @@ 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; } @@ -78,6 +80,10 @@ 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 559a4b8b668..6ae01ad0177 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,17 +513,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { return (gcAlgorithm.matcher(jvmargs).find() ||cmsArgs.matcher(jvmargs).find()); } - 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 buildJvmGCOptions(DeployState deployState, String jvmGCOPtions) { + String options = (jvmGCOPtions != null) + ? jvmGCOPtions + : deployState.getProperties().jvmGCOptions(); + return (options == null ||options.isEmpty()) ? ContainerCluster.G1GC : options; } 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)) { @@ -541,15 +538,17 @@ 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 = nodesElement.hasAttribute(VespaDomBuilder.JVM_GC_OPTIONS) - ? nodesElement.getAttribute(VespaDomBuilder.JVM_GC_OPTIONS) - : null; - cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState().zone(), jvmGCOptions, context.getDeployState().isHosted())); + String jvmGCOptions = extractAttribute(nodesElement, VespaDomBuilder.JVM_GC_OPTIONS); + cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState(), jvmGCOptions)); } applyMemoryPercentage(cluster, nodesElement.getAttribute(VespaDomBuilder.Allocated_MEMORY_ATTRIB_NAME)); @@ -559,10 +558,8 @@ 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 = jvmElement.hasAttribute(VespaDomBuilder.GC_OPTIONS) - ? jvmElement.getAttribute(VespaDomBuilder.GC_OPTIONS) - : null; - cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState().zone(), jvmGCOptions, context.getDeployState().isHosted())); + String jvmGCOptions = extractAttribute(jvmElement, VespaDomBuilder.GC_OPTIONS); + cluster.setJvmGCOptions(buildJvmGCOptions(context.getDeployState(), jvmGCOptions)); } /** 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 484709a0c18..448ffffc04b 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,7 +50,6 @@ 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"); @@ -85,11 +84,11 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { } private static void verifyIgnoreJvmGCOptions(boolean isHosted) throws IOException, SAXException { - verifyIgnoreJvmGCOptionsIfJvmArgs(isHosted, "jvmargs", ContainerCluster.G1GC); - verifyIgnoreJvmGCOptionsIfJvmArgs(isHosted, "jvm-options", "-XX:+UseG1GC"); + verifyIgnoreJvmGCOptionsIfJvmArgs("jvmargs", ContainerCluster.G1GC); + verifyIgnoreJvmGCOptionsIfJvmArgs( "jvm-options", "-XX:+UseG1GC"); } - private static void verifyIgnoreJvmGCOptionsIfJvmArgs(boolean isHosted, String jvmOptionsName, String expectedGC) throws IOException, SAXException { + private static void verifyIgnoreJvmGCOptionsIfJvmArgs(String jvmOptionsName, String expectedGC) throws IOException, SAXException { String servicesXml = "<container version='1.0'>" + " <nodes jvm-gc-options='-XX:+UseG1GC' " + jvmOptionsName + "='-XX:+UseParNewGC'>" + @@ -102,8 +101,6 @@ 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"); @@ -117,7 +114,7 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { verifyIgnoreJvmGCOptions(true); } - private void verifyJvmGCOptions(boolean isHosted, String override, Zone zone, String expected) throws IOException, SAXException { + private void verifyJvmGCOptions(String featureFlagDefault, String override, String expected) throws IOException, SAXException { String servicesXml = "<container version='1.0'>" + " <nodes " + ((override == null) ? ">" : ("jvm-gc-options='" + override + "'>")) + @@ -130,8 +127,7 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() .applicationPackage(applicationPackage) .deployLogger(logger) - .zone(zone) - .properties(new TestProperties().setHostedVespa(isHosted)) + .properties(new TestProperties().setJvmGCOptions(featureFlagDefault)) .build()); QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder(); model.getConfig(qrStartBuilder, "container/container.0"); @@ -139,18 +135,13 @@ 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(false, Zone.defaultZone(),ContainerCluster.G1GC); - verifyJvmGCOptions(true, Zone.defaultZone(), ContainerCluster.G1GC); + 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"); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index c925157b980..3a2bc9f1a81 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -154,6 +154,7 @@ public class ModelContextImpl implements ModelContext { private final double threadPoolSizeFactor; private final double queueSizefactor; private final String docprocLoadBalancerType; + private final String jvmGCOPtions; private final Optional<AthenzDomain> athenzDomain; private final Optional<ApplicationRoles> applicationRoles; private final int jdiscHealthCheckProxyClientTimeout; @@ -197,6 +198,8 @@ public class ModelContextImpl implements ModelContext { .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); docprocLoadBalancerType = Flags.DOCPROC_LOADBALANCER_TYPE.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + jvmGCOPtions = Flags.JVM_GC_OPTIONS.bindTo(flagSource) + .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.athenzDomain = athenzDomain; this.applicationRoles = applicationRoles; jdiscHealthCheckProxyClientTimeout = Flags.JDISC_HEALTH_CHECK_PROXY_CLIENT_TIMEOUT.bindTo(flagSource) @@ -280,6 +283,8 @@ public class ModelContextImpl implements ModelContext { } @Override public Duration jdiscHealthCheckProxyClientTimeout() { return Duration.ofMillis(jdiscHealthCheckProxyClientTimeout); } + @Override public String jvmGCOptions() { return jvmGCOPtions; } + } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index a9ded0f5fd2..30383393e97 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -152,6 +152,11 @@ public class Flags { "Selects what kind of load balancer to use for document processing {'adaptive', 'legacy' ''}", "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); + public static final UnboundStringFlag JVM_GC_OPTIONS = defineStringFlag( + "jvm-gc-options", "", + "Sets deafult jvm gc options", + "Takes effect at redeployment", + ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag USE_DISTRIBUTOR_BTREE_DB = defineFeatureFlag( "use-distributor-btree-db", false, |