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 /config-model | |
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.
Diffstat (limited to 'config-model')
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); } |