From b255dceef360c969254c13035d7413947e4ea966 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Wed, 10 Jan 2024 14:48:51 -0500 Subject: [PATCH] Enable `containersV2` by default (#369) Enable `containersV2` by default. --- README.md | 19 +++++++++---------- changelog/@unreleased/pr-369.v2.yml | 5 +++++ .../go_java_launcher_integration_test.go | 2 +- ...om-dangerous-disable-container-support.yml | 2 ++ ...ntainer-v2-with-initial-ram-percentage.yml | 2 -- ...l-container-v2-with-max-ram-percentage.yml | 2 -- ...rimental-container-v2-with-xms-and-xmx.yml | 2 -- ...ncher-custom-experimental-container-v2.yml | 2 -- ...nitial-and-max-ram-percentage-override.yml | 2 ++ ...custom-initial-ram-percentage-override.yml | 2 ++ .../launcher-custom-max-ram-override.yml | 2 ++ ...her-custom-max-ram-percentage-override.yml | 2 ++ ...r-custom-multiprocess-long-sub-process.yml | 2 ++ .../testdata/launcher-custom-multiprocess.yml | 2 ++ integration_test/testdata/launcher-custom.yml | 2 ++ .../launcher-static-bad-java-home.yml | 2 ++ .../testdata/launcher-static-multiprocess.yml | 2 ++ .../testdata/launcher-static-with-dirs.yml | 2 ++ integration_test/testdata/launcher-static.yml | 2 ++ launchlib/config.go | 6 +++++- launchlib/config_test.go | 14 +++++++++++--- launchlib/launcher.go | 16 +++++++++------- 22 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 changelog/@unreleased/pr-369.v2.yml diff --git a/README.md b/README.md index e9782b17..0d1b9cea 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,16 @@ All output from `go-java-launcher` itself, and from the launch of all processes By _default_, when starting a java process inside a container (as indicated by the presence of ``CONTAINER`` env variable): +1. If the `-XX:ActiveProcessorCount` is unset, it will remain unset. +1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither + ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts + ``-Xmx|-Xms`` will both be set to be 75% of the cgroups memory limit minus 3mb per processor, with a minimum value of + 50% of the heap. +This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified +percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``. + +If the experimental flag `containerV2` is set to `false`, the behavior will be as follows: 1. If `-XX:ActiveProcessorCount` is unset, it will be set based on the discovered cgroup configurations and host information to a value between 2 and the number of processors reported by the runtime. You can read more about the reasoning behind this [here](https://github.com/palantir/go-java-launcher/issues/313). @@ -159,16 +168,6 @@ variable): custom jvm opts, both will be set to ``75.0`` (i.e. ``-XX:InitialRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0 `` will be appended after all the other jvm opts). -This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified -percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``. - -If the experimental flag `containerV2` is set: -1. The `-XX:ActiveProcessorCount` is unset, it will remain unset. -1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither - ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts - ``-Xmx|-Xms`` will both be set to be 75% of the cgroups memory limit minus 3mb per processor, with a minimum value of - 50% of the heap. - ### Overriding default values Developers can override the heap percentage in containers by specifying both ``-XX:MaxRAMPercentage=`` diff --git a/changelog/@unreleased/pr-369.v2.yml b/changelog/@unreleased/pr-369.v2.yml new file mode 100644 index 00000000..c680c2fe --- /dev/null +++ b/changelog/@unreleased/pr-369.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Enable `containersV2` by default. + links: + - https://github.com/palantir/go-java-launcher/pull/369 diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index 1839dadd..fd7cf83d 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -144,7 +144,7 @@ func TestMainMethodWithoutCustomConfig(t *testing.T) { } func TestMainMethodContainerWithoutCustomConfig(t *testing.T) { - output := testContainerSupportEnabled(t, "foo", "-XX\\:InitialRAMPercentage=75.0 -XX\\:MaxRAMPercentage=75.0 -XX\\:ActiveProcessorCount=2", []string{}) + output := testContainerSupportEnabled(t, "foo", "", []string{"-Xmx", "-Xms"}) assert.Regexp(t, `Failed to read custom config file, assuming no custom config: foo`, output) } diff --git a/integration_test/testdata/launcher-custom-dangerous-disable-container-support.yml b/integration_test/testdata/launcher-custom-dangerous-disable-container-support.yml index 7c7e9f71..cd01d9c0 100644 --- a/integration_test/testdata/launcher-custom-dangerous-disable-container-support.yml +++ b/integration_test/testdata/launcher-custom-dangerous-disable-container-support.yml @@ -3,3 +3,5 @@ configVersion: 1 jvmOpts: - '-Xmx1g' dangerousDisableContainerSupport: true +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml index f2ff4319..445aadf8 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml @@ -2,5 +2,3 @@ configType: java configVersion: 1 jvmOpts: - '-XX:InitialRAMPercentage=70.0' -experimental: - containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml index e5d905f1..04344178 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml @@ -2,5 +2,3 @@ configType: java configVersion: 1 jvmOpts: - '-XX:MaxRAMPercentage=70.0' -experimental: - containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml index 299a15bf..3275e56a 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml @@ -3,5 +3,3 @@ configVersion: 1 jvmOpts: - '-Xms1g' - '-Xmx1g' -experimental: - containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2.yml b/integration_test/testdata/launcher-custom-experimental-container-v2.yml index c5d70615..9218ad75 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2.yml @@ -1,5 +1,3 @@ configType: java configVersion: 1 jvmOpts: [] -experimental: - containerV2: true diff --git a/integration_test/testdata/launcher-custom-initial-and-max-ram-percentage-override.yml b/integration_test/testdata/launcher-custom-initial-and-max-ram-percentage-override.yml index 54d0c47e..f7428b66 100644 --- a/integration_test/testdata/launcher-custom-initial-and-max-ram-percentage-override.yml +++ b/integration_test/testdata/launcher-custom-initial-and-max-ram-percentage-override.yml @@ -4,3 +4,5 @@ jvmOpts: - '-Xmx1g' - '-XX:InitialRAMPercentage=79.9' - '-XX:MaxRAMPercentage=80.9' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-initial-ram-percentage-override.yml b/integration_test/testdata/launcher-custom-initial-ram-percentage-override.yml index 7b6f405c..7a48c70c 100644 --- a/integration_test/testdata/launcher-custom-initial-ram-percentage-override.yml +++ b/integration_test/testdata/launcher-custom-initial-ram-percentage-override.yml @@ -3,3 +3,5 @@ configVersion: 1 jvmOpts: - '-Xmx1g' - '-XX:InitialRAMPercentage=79.9' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-max-ram-override.yml b/integration_test/testdata/launcher-custom-max-ram-override.yml index eeb8f22a..a44c822d 100644 --- a/integration_test/testdata/launcher-custom-max-ram-override.yml +++ b/integration_test/testdata/launcher-custom-max-ram-override.yml @@ -2,3 +2,5 @@ configType: java configVersion: 1 jvmOpts: - '-XX:MaxRAM=1001' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-max-ram-percentage-override.yml b/integration_test/testdata/launcher-custom-max-ram-percentage-override.yml index 55e748c5..c884af10 100644 --- a/integration_test/testdata/launcher-custom-max-ram-percentage-override.yml +++ b/integration_test/testdata/launcher-custom-max-ram-percentage-override.yml @@ -3,3 +3,5 @@ configVersion: 1 jvmOpts: - '-Xmx1g' - '-XX:MaxRAMPercentage=79.9' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-multiprocess-long-sub-process.yml b/integration_test/testdata/launcher-custom-multiprocess-long-sub-process.yml index 6e9ad3f0..43da4664 100644 --- a/integration_test/testdata/launcher-custom-multiprocess-long-sub-process.yml +++ b/integration_test/testdata/launcher-custom-multiprocess-long-sub-process.yml @@ -9,3 +9,5 @@ subProcesses: SLEEP_TIME: "200" jvmOpts: - '-Xmx1g' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom-multiprocess.yml b/integration_test/testdata/launcher-custom-multiprocess.yml index e0994ce8..b2eb948d 100644 --- a/integration_test/testdata/launcher-custom-multiprocess.yml +++ b/integration_test/testdata/launcher-custom-multiprocess.yml @@ -7,3 +7,5 @@ subProcesses: configType: java jvmOpts: - '-Xmx1g' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-custom.yml b/integration_test/testdata/launcher-custom.yml index e6ade620..55382e26 100644 --- a/integration_test/testdata/launcher-custom.yml +++ b/integration_test/testdata/launcher-custom.yml @@ -2,3 +2,5 @@ configType: java configVersion: 1 jvmOpts: - '-Xmx1g' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-static-bad-java-home.yml b/integration_test/testdata/launcher-static-bad-java-home.yml index adb52326..97ae0c7f 100644 --- a/integration_test/testdata/launcher-static-bad-java-home.yml +++ b/integration_test/testdata/launcher-static-bad-java-home.yml @@ -9,3 +9,5 @@ jvmOpts: args: - arg1 javaHome: /foo/bar +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-static-multiprocess.yml b/integration_test/testdata/launcher-static-multiprocess.yml index 0c0828d8..60eb3e41 100644 --- a/integration_test/testdata/launcher-static-multiprocess.yml +++ b/integration_test/testdata/launcher-static-multiprocess.yml @@ -16,3 +16,5 @@ subProcesses: - ./testdata/ jvmOpts: - '-Xmx4M' +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-static-with-dirs.yml b/integration_test/testdata/launcher-static-with-dirs.yml index b55dd77f..66970293 100644 --- a/integration_test/testdata/launcher-static-with-dirs.yml +++ b/integration_test/testdata/launcher-static-with-dirs.yml @@ -11,3 +11,5 @@ args: dirs: - foo - bar/baz +experimental: + containerV2: false diff --git a/integration_test/testdata/launcher-static.yml b/integration_test/testdata/launcher-static.yml index e02728b3..0f3fdb45 100644 --- a/integration_test/testdata/launcher-static.yml +++ b/integration_test/testdata/launcher-static.yml @@ -8,3 +8,5 @@ jvmOpts: - '-Xmx4M' args: - arg1 +experimental: + containerV2: false diff --git a/launchlib/config.go b/launchlib/config.go index 6b1538eb..f66c4529 100644 --- a/launchlib/config.go +++ b/launchlib/config.go @@ -72,7 +72,7 @@ type CustomLauncherConfig struct { } type ExperimentalLauncherConfig struct { - ContainerV2 bool `yaml:"containerV2"` + ContainerV2 *bool `yaml:"containerV2"` } type PrimaryCustomLauncherConfig struct { @@ -267,6 +267,10 @@ func parseCustomConfig(yamlString []byte) (PrimaryCustomLauncherConfig, error) { "subProcess config %s", name) } } + if config.Experimental.ContainerV2 == nil { + var trueVal = true + config.Experimental.ContainerV2 = &trueVal + } return config, nil } diff --git a/launchlib/config_test.go b/launchlib/config_test.go index e70049f7..c934c636 100644 --- a/launchlib/config_test.go +++ b/launchlib/config_test.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/assert" ) +var trueValue = true + func TestParseStaticConfig(t *testing.T) { for i, currCase := range []struct { name string @@ -188,6 +190,7 @@ jvmOpts: }, JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, DisableContainerSupport: false, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, }, }, @@ -208,7 +211,8 @@ jvmOpts: TypedConfig: TypedConfig{ Type: "java", }, - JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, }, }, @@ -234,7 +238,8 @@ jvmOpts: Env: map[string]string{ "SOME_ENV_VAR": "{{CWD}}/etc/profile", }, - JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, }, }, @@ -272,7 +277,8 @@ subProcesses: Env: map[string]string{ "SOME_ENV_VAR": "{{CWD}}/etc/profile", }, - JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, SubProcesses: map[string]CustomLauncherConfig{ "envoy": { @@ -306,6 +312,7 @@ dangerousDisableContainerSupport: true }, JvmOpts: []string{"jvmOpt1", "jvmOpt2"}, DisableContainerSupport: true, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, }, }, @@ -330,6 +337,7 @@ env: "SOME_ENV_VAR": "/etc/profile", "OTHER_ENV_VAR": "/etc/redhat-release", }, + Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue}, }, }, }, diff --git a/launchlib/launcher.go b/launchlib/launcher.go index ebfc39f0..05dd2266 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -281,7 +281,12 @@ func delim(str string) string { func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) []string { if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) { _, _ = fmt.Fprintln(logger, "Container support enabled") - if customConfig.Experimental.ContainerV2 { + // If the containerV2 field is nil, there was a failure reading the custom config file, and we use the default + // behavior of enabling containerV2 behavior. + if customConfig.Experimental.ContainerV2 != nil && !*customConfig.Experimental.ContainerV2 { + combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) + combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger) + } else { jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts) if err != nil { // When we fail to get the memory limit from the cgroups files, fallback to using percentage-based heap @@ -294,9 +299,6 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, } else { combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs } - } else { - combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) - combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger) } return combinedJvmOpts } @@ -334,7 +336,7 @@ func filterHeapSizeArgs(args []string) []string { return filtered } -// Used when the containerV2 flag is set +// Used when the containerV2 flag is set to `true`. This is the default behavior. func filterHeapSizeArgsV2(args []string) ([]string, error) { var filtered []string var hasMaxRAMPercentage, hasInitialRAMPercentage bool @@ -425,8 +427,8 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } -// ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set, compute the heap size to be 75% of -// the heap minus 3mb per processor, with a minimum value of 50% of the heap. +// ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set to `true` (which it is by default), compute the +// heap size to be 75% of the heap minus 3mb per processor, with a minimum value of 50% of the heap. func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) (uint64, error) { if cgroupMemoryLimitInBytes > 1_000_000*BytesInMebibyte { return 0, errors.New("cgroups memory limit is unusually high. Not computing JVM heap size")