Skip to content

Fix setting jvm heap size params #1107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 28, 2025
Merged

Fix setting jvm heap size params #1107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f6753d
Fix setting jvm heap size params
patelsmit32123 Sep 26, 2025
a1e847c
Merge branch 'opensearch-project:main' into jvm-heap-size-settings
patelsmit32123 Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions opensearch-operator/pkg/builders/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ func NewSTSForNodePool(
// vendor ="elasticsearch"
}

jvm := helpers.CalculateJvmHeapSize(&node)
jvmHeapSizeSettings := helpers.CalculateJvmHeapSizeSettings(node.Resources.Requests.Memory())
jvm := helpers.AppendJvmHeapSizeSettings(node.Jvm, jvmHeapSizeSettings)

// If node role `search` defined add required experimental flag if version less than 2.7
if helpers.ContainsString(selectedRoles, "search") && helpers.CompareVersions(cr.Spec.General.Version, "2.7.0") {
Expand Down Expand Up @@ -785,12 +786,8 @@ func NewBootstrapPod(
}
resources := cr.Spec.Bootstrap.Resources

var jvm string
if cr.Spec.Bootstrap.Jvm == "" {
jvm = "-Xmx512M -Xms512M"
} else {
jvm = cr.Spec.Bootstrap.Jvm
}
jvmHeapSizeSettings := helpers.CalculateJvmHeapSizeSettings(cr.Spec.Bootstrap.Resources.Requests.Memory())
jvm := helpers.AppendJvmHeapSizeSettings(cr.Spec.Bootstrap.Jvm, jvmHeapSizeSettings)

image := helpers.ResolveImage(cr, nil)
initHelperImage := helpers.ResolveInitHelperImage(cr)
Expand Down
20 changes: 10 additions & 10 deletions opensearch-operator/pkg/builders/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ var _ = Describe("Builders", func() {

Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx512M -Xms512M -Dopensearch.experimental.feature.searchable_snapshot.enabled=true -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms512M -Xmx512M -Dopensearch.experimental.feature.searchable_snapshot.enabled=true -Dopensearch.transport.cname_in_publish_address=true",
}))
})

Expand All @@ -297,7 +297,7 @@ var _ = Describe("Builders", func() {

Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx512M -Xms512M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms512M -Xmx512M -Dopensearch.transport.cname_in_publish_address=true",
}))
})

Expand Down Expand Up @@ -409,7 +409,7 @@ var _ = Describe("Builders", func() {
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx1024M -Xms1024M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms1024M -Xmx1024M -Dopensearch.transport.cname_in_publish_address=true",
}))
})
It("should set jvm to half of memory request when memory request is fraction and jvm are not provided", func() {
Expand All @@ -424,7 +424,7 @@ var _ = Describe("Builders", func() {
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx768M -Xms768M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms768M -Xmx768M -Dopensearch.transport.cname_in_publish_address=true",
}))
})

Expand All @@ -440,7 +440,7 @@ var _ = Describe("Builders", func() {
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx953M -Xms953M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms953M -Xmx953M -Dopensearch.transport.cname_in_publish_address=true",
}))
})
It("should set jvm to default when memory request and jvm are not provided", func() {
Expand All @@ -449,24 +449,24 @@ var _ = Describe("Builders", func() {
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx512M -Xms512M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms512M -Xmx512M -Dopensearch.transport.cname_in_publish_address=true",
}))
})
It("should set NodePool.Jvm as jvm when it jvm is provided", func() {
clusterObject := ClusterDescWithVersion("2.2.1")
nodePool := opsterv1.NodePool{
Jvm: "-Xmx1024M -Xms1024M",
Jvm: "-Xms1024M -Xmx1024M",
}
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx1024M -Xms1024M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms1024M -Xmx1024M -Dopensearch.transport.cname_in_publish_address=true",
}))
})
It("should set NodePool.jvm as jvm when jvm and memory request are provided", func() {
clusterObject := ClusterDescWithVersion("2.2.1")
nodePool := opsterv1.NodePool{
Jvm: "-Xmx1024M -Xms1024M",
Jvm: "-Xms1024M -Xmx1024M",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("4Gi"),
Expand All @@ -476,7 +476,7 @@ var _ = Describe("Builders", func() {
result := NewSTSForNodePool("foobar", &clusterObject, nodePool, "foobar", nil, nil, nil)
Expect(result.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{
Name: "OPENSEARCH_JAVA_OPTS",
Value: "-Xmx1024M -Xms1024M -Dopensearch.transport.cname_in_publish_address=true",
Value: "-Xms1024M -Xmx1024M -Dopensearch.transport.cname_in_publish_address=true",
}))
})

Expand Down
34 changes: 17 additions & 17 deletions opensearch-operator/pkg/helpers/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"errors"
"fmt"
"io"
"k8s.io/apimachinery/pkg/api/resource"
"log"
"reflect"
"sort"
"strings"
"time"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand Down Expand Up @@ -444,25 +446,23 @@ func ComposePDB(cr *opsterv1.OpenSearchCluster, nodepool *opsterv1.NodePool) pol
return newpdb
}

func CalculateJvmHeapSize(nodePool *opsterv1.NodePool) string {
jvmHeapSizeTemplate := "-Xmx%s -Xms%s"

if nodePool.Jvm == "" {
memoryLimit := nodePool.Resources.Requests.Memory()

// Memory request is not present
if memoryLimit.IsZero() {
return fmt.Sprintf(jvmHeapSizeTemplate, "512M", "512M")
}

// Set Java Heap size to half of the node pool memory size
megabytes := float64((memoryLimit.Value() / 2) / 1024.0 / 1024.0)

heapSize := fmt.Sprintf("%vM", megabytes)
return fmt.Sprintf(jvmHeapSizeTemplate, heapSize, heapSize)
func AppendJvmHeapSizeSettings(jvm string, heapSizeSettings string) string {
if strings.Contains(jvm, "Xms") || strings.Contains(jvm, "Xmx") {
return jvm
}
if jvm == "" {
return heapSizeSettings
}
return fmt.Sprintf("%s %s", jvm, heapSizeSettings)
}
Comment on lines +449 to +457
Copy link
Contributor

@josedev-union josedev-union Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patelsmit32123 This change works as expected. I tested it in my local lab.
Btw, I think this change causes the default heap size configuration to be ignored when either -Xms or -Xmx is already defined in JAVA_OPTS.

In other words, if a user sets a custom JAVA_OPTS containing -Xms or -Xmx, the operator won’t apply the default heap size anymore. That might be fine if intentional, but it also means users who only partially override Java options (e.g., setting -Xmx but not -Xms) will lose the default configuration for the missing value.

Copy link
Contributor Author

@patelsmit32123 patelsmit32123 Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, its intentional considering usually people provide both params

Copy link
Contributor

@josedev-union josedev-union Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patelsmit32123 it would be good to consider all cases.


return nodePool.Jvm
func CalculateJvmHeapSizeSettings(memoryRequest *resource.Quantity) string {
var memoryRequestMb int64 = 512
if memoryRequest != nil && !memoryRequest.IsZero() {
memoryRequestMb = ((memoryRequest.Value() / 2.0) / 1024.0) / 1024.0
}
// Set Java Heap size to half of the node pool memory request for both Xms and Xmx
return fmt.Sprintf("-Xms%dM -Xmx%dM", memoryRequestMb, memoryRequestMb)
}

func IsUpgradeInProgress(status opsterv1.ClusterStatus) bool {
Expand Down
80 changes: 80 additions & 0 deletions opensearch-operator/pkg/helpers/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/utils/ptr"
)

Expand Down Expand Up @@ -162,3 +163,82 @@ var _ = Describe("Helper Functions", func() {
})

})

var _ = Describe("JVM Heap Size Functions", func() {
Describe("AppendJvmHeapSizeSettings", func() {
Context("when JVM string already contains Xmx", func() {
It("should return the original JVM string unchanged", func() {
jvm := "-XX:+UseG1GC -Xmx2g -XX:MaxDirectMemorySize=1g"
heapSizeSettings := "-Xms1g -Xmx2g"

result := AppendJvmHeapSizeSettings(jvm, heapSizeSettings)

Expect(result).To(Equal(jvm))
})
})

Context("when JVM string already contains Xms", func() {
It("should return the original JVM string unchanged", func() {
jvm := "-XX:+UseG1GC -Xms1g -XX:MaxDirectMemorySize=1g"
heapSizeSettings := "-Xms1g -Xmx2g"

result := AppendJvmHeapSizeSettings(jvm, heapSizeSettings)

Expect(result).To(Equal(jvm))
})
})

Context("when JVM string is empty", func() {
It("should return only the heap size settings", func() {
jvm := ""
heapSizeSettings := "-Xmx1g -Xms1g"

result := AppendJvmHeapSizeSettings(jvm, heapSizeSettings)

Expect(result).To(Equal(heapSizeSettings))
})
})

Context("when JVM string does not contain Xmx or Xms", func() {
It("should append the heap size settings", func() {
jvm := "-XX:+UseG1GC -XX:MaxDirectMemorySize=1g"
heapSizeSettings := "-Xmx1g -Xms1g"
expected := "-XX:+UseG1GC -XX:MaxDirectMemorySize=1g -Xmx1g -Xms1g"

result := AppendJvmHeapSizeSettings(jvm, heapSizeSettings)

Expect(result).To(Equal(expected))
})
})
})

Describe("CalculateJvmHeapSizeSettings", func() {
Context("when memory request is nil", func() {
It("should return default 512M for both Xms and Xmx", func() {
result := CalculateJvmHeapSizeSettings(nil)

Expect(result).To(Equal("-Xms512M -Xmx512M"))
})
})

Context("when memory request is zero", func() {
It("should return default 512M for both Xms and Xmx", func() {
memoryRequest := resource.MustParse("0")

result := CalculateJvmHeapSizeSettings(&memoryRequest)

Expect(result).To(Equal("-Xms512M -Xmx512M"))
})
})

Context("when memory request is provided", func() {
It("should calculate both Xms and Xmx from request", func() {
memoryRequest := resource.MustParse("2Gi")

result := CalculateJvmHeapSizeSettings(&memoryRequest)

Expect(result).To(Equal("-Xms1024M -Xmx1024M"))
})
})
})
})