diff options
author | Harald Musum <musum@yahooinc.com> | 2024-05-16 14:04:37 +0200 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2024-05-16 14:04:37 +0200 |
commit | 4c5dd194ebb2e5d82f3e2ba5871bc54065059947 (patch) | |
tree | f7a652965a7dee8bbcbebd3f701f2b5ed36a164c | |
parent | ebc496f2186afadde852ea05ee0c55af720b8546 (diff) |
Fix Jvm options in combination with feature flag
Need to set jvm options from services.xml first,
code prevents overriding jvm config that is already set
on container (not cluster) level.
8 files changed, 30 insertions, 29 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java index ad728d60c5a..e18ffea731e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java @@ -31,7 +31,7 @@ public class LogserverContainer extends Container { } @Override - protected String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { + public String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { return featureFlags.jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type.admin); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index ca602b447fe..e702a29b640 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -98,7 +98,7 @@ public class ClusterControllerContainer extends Container implements } @Override - protected String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { + public String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { return featureFlags.jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type.admin); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index c4702ba3543..2ff58438a07 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -78,7 +78,7 @@ public class MetricsProxyContainer extends Container implements } @Override - protected String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { + public String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { return featureFlags.jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type.admin); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java index e3fc680ad6a..1f8d7fd7520 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java @@ -82,11 +82,6 @@ public final class ApplicationContainer extends Container implements builder.myid(index()); } - @Override - protected String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { - return featureFlags.jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type.container); - } - @Override public Optional<String> getPreShutdownCommand() { return Optional.of(prepareStopCommand(Duration.ofMinutes(6))); } private Optional<NodeResources> realResources() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java index f1945ca5341..864cbc8691b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java @@ -105,12 +105,10 @@ public abstract class Container extends AbstractService implements addBuiltinHandlers(); addChild(new SimpleComponent("com.yahoo.container.jdisc.ConfiguredApplication$ApplicationContext")); - - appendJvmOptions(jvmOmitStackTraceInFastThrowOption(deployState.featureFlags())); addEnvironmentVariable("VESPA_MALLOC_MMAP_THRESHOLD","0x1000000"); // 16M } - protected String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { + public String jvmOmitStackTraceInFastThrowOption(ModelContext.FeatureFlags featureFlags) { return featureFlags.jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type.container); } 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 56e2a21e38b..dbe28d48f9e 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 @@ -986,6 +986,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { AbstractService.distributeCpuSocketAffinity(nodes); cluster.addContainers(nodes); } + // Must be done after setting Jvm options from services.xml (#extractJvmOptions()), otherwise those options will not be set + cluster.getContainers().forEach(container -> container.appendJvmOptions(container.jvmOmitStackTraceInFastThrowOption(context.featureFlags()))); } private ZoneEndpoint zoneEndpoint(ConfigModelContext context, ClusterSpec.Id cluster) { 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 2ea4249883d..9cf34b55ebf 100644 --- 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 @@ -221,24 +221,6 @@ public class ContainerClusterTest { } @Test - void requireThatJvmOmitStackTraceInFastThrowOptionWorks() { - // Empty option if option not set in property - MockRoot root = createRoot(new DeployState.Builder().build()); - ApplicationContainerCluster cluster = newClusterWithSearch(root); - addContainer(root, cluster, "c1", "host-c1"); - ApplicationContainer container = cluster.getContainers().get(0); - assertEquals("", container.getJvmOptions()); - - String jvmOption = "-XX:-foo"; - DeployState deployState = new DeployState.Builder().properties(new TestProperties().setJvmOmitStackTraceInFastThrowOption(jvmOption)).build(); - root = createRoot(deployState); - cluster = newClusterWithSearch(root); - addContainer(root, cluster, "c1", "host-c1"); - container = cluster.getContainers().get(0); - assertEquals(jvmOption, container.getJvmOptions()); - } - - @Test void requireThatWeCanHandleNullJvmOptions() { MockRoot root = createRoot(false); ApplicationContainerCluster cluster = newClusterWithSearch(root); 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 46097da434e..64afd444989 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.container.xml; +import com.yahoo.cloud.config.SentinelConfig; import com.yahoo.collections.Pair; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.NullConfigModelRegistry; @@ -205,6 +206,29 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase { assertEquals("-XX:+UseParNewGC", qrStartConfig.jvm().gcopts()); } + @Test + void verify_jvm_option_and_value_from_feature_flag_are_both_included() throws IOException, SAXException { + String servicesXml = """ + <container version='1.0'> + <search/> + <nodes> + <jvm options="-Xms1024m -Xmx2048m" /> + <node hostalias="node1" /> + </nodes> + </container> + """; + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).build(); + // Need to create VespaModel to make deploy properties have effect + VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .applicationPackage(applicationPackage) + .properties(new TestProperties().setJvmOmitStackTraceInFastThrowOption("-XX:-OmitStackTraceInFastThrow")) + .build()); + SentinelConfig.Builder builder = new SentinelConfig.Builder(); + model.getConfig(builder, "hosts/localhost"); + SentinelConfig config = builder.build(); + assertEquals("PRELOAD=/opt/vespa/lib64/vespa/malloc/libvespamalloc.so exec ${VESPA_HOME}/libexec/vespa/vespa-wrapper vespa-start-container-daemon -Xms1024m -Xmx2048m -XX:-OmitStackTraceInFastThrow ", config.service().get(0).command()); + } + private void verifyLoggingOfJvmGcOptions(boolean isHosted, String override, String... invalidOptions) throws IOException, SAXException { verifyLogMessage(isHosted, "gc-options", override, invalidOptions); } |