Skip to content

Commit

Permalink
Add test to check that heap size is not computed when cgroups memory …
Browse files Browse the repository at this point in the history
…limits are over 1TB (#363)

Add test to check that heap size is not computed when cgroups memory limits are over 1TB
  • Loading branch information
mpritham authored Nov 16, 2023
1 parent 35e9be0 commit a61087c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-363.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: Add test to check that heap size is not computed when cgroups memory
limits are over 1TB
links:
- https://github.com/palantir/go-java-launcher/pull/363
20 changes: 18 additions & 2 deletions integration_test/go_java_launcher_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,32 +209,48 @@ func TestComputeJVMHeapSize(t *testing.T) {
numHostProcessors int
memoryLimit uint64
expectedMaxHeapSize uint64
expectError bool
}{
{
name: "at least 50% of heap",
numHostProcessors: 1,
memoryLimit: 10 * launchlib.BytesInMebibyte,
// 75% of heap - 3mb*processors = 4.5mb
expectedMaxHeapSize: 5 * launchlib.BytesInMebibyte,
expectError: false,
},
{
name: "computes 75% of heap minus 3mb per processor",
numHostProcessors: 1,
memoryLimit: 16 * launchlib.BytesInMebibyte,
// 75% of heap - 3mb*processors = 9mb
expectedMaxHeapSize: 9 * launchlib.BytesInMebibyte,
expectError: false,
},
{
name: "multiple processors",
numHostProcessors: 3,
memoryLimit: 120 * launchlib.BytesInMebibyte,
// 75% of heap - 3mb*processors = 81mb
expectedMaxHeapSize: 81 * launchlib.BytesInMebibyte,
expectError: false,
},
{
name: "memory limit too large",
numHostProcessors: 1,
memoryLimit: 1_000_001 * launchlib.BytesInMebibyte,
expectedMaxHeapSize: 0,
expectError: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
heapSizeInBytes := launchlib.ComputeJVMHeapSizeInBytes(tc.numHostProcessors, tc.memoryLimit)
assert.Equal(t, heapSizeInBytes, tc.expectedMaxHeapSize)
heapSizeInBytes, err := launchlib.ComputeJVMHeapSizeInBytes(tc.numHostProcessors, tc.memoryLimit)
if tc.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
assert.Equal(t, heapSizeInBytes, tc.expectedMaxHeapSize)
}
})
}
}
Expand Down
11 changes: 7 additions & 4 deletions launchlib/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,10 @@ func filterHeapSizeArgsV2(args []string) ([]string, error) {
if err != nil {
return filtered, errors.Wrap(err, "failed to get cgroup memory limit")
}
if cgroupMemoryLimitInBytes > 1_000_000*BytesInMebibyte {
jvmHeapSizeInBytes, err := ComputeJVMHeapSizeInBytes(runtime.NumCPU(), cgroupMemoryLimitInBytes)
if err != nil {
return filtered, errors.New("cgroups memory limit is unusually high. Not setting JVM heap size options")
}
jvmHeapSizeInBytes := ComputeJVMHeapSizeInBytes(runtime.NumCPU(), cgroupMemoryLimitInBytes)
filtered = append(filtered, fmt.Sprintf("-Xms%d", jvmHeapSizeInBytes))
filtered = append(filtered, fmt.Sprintf("-Xmx%d", jvmHeapSizeInBytes))
}
Expand Down Expand Up @@ -427,9 +427,12 @@ func isInitialRAMPercentage(arg string) bool {

// 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.
func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) uint64 {
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")
}
var memoryLimit = float64(cgroupMemoryLimitInBytes)
var processorAdjustment = 3 * BytesInMebibyte * float64(hostProcessorCount)
var computedHeapSize = max(0.5*memoryLimit, 0.75*memoryLimit-processorAdjustment)
return uint64(computedHeapSize)
return uint64(computedHeapSize), nil
}

0 comments on commit a61087c

Please sign in to comment.