Fix setting jvm heap size params

[patelsmit32123]

Issues Resolved

Fixes #1106

Check List

• Commits are signed per the DCO using --signoff
• Unittest added for the new/changed functionality and all unit tests are successful
• Customer-visible features documented
• No linter warnings (make lint)

If CRDs are changed:

• CRD YAMLs updated (make manifests) and also copied into the helm chart
• Changes to CRDs documented

Please refer to the PR guidelines before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[patelsmit32123]

@prudhvigodithi can you please review?

[josedev-union]

@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.

[patelsmit32123]

yes, its intentional considering usually people provide both params

[josedev-union]

@patelsmit32123 it would be good to consider all cases.

[prudhvigodithi]

Seeing the documentation https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/userguide/main.md

• If jvm is not provided then the java heap size will be set to half of resources.requests.memory which is the recommend value for data nodes
• If jvm is not provided and resources.requests.memory does not exist then value will be -Xmx512M -Xms512M.

We can leverage jvm only to control the -Xmx and -Xms values. For other settings we can jvmSettings or jvmOptions as new CRD? Then finally we can append all the values to a string.

@patelsmit32123 @josedev-union WDYT?

[josedev-union]

Seeing the documentation https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/userguide/main.md

• If jvm is not provided then the java heap size will be set to half of resources.requests.memory which is the recommend value for data nodes
• If jvm is not provided and resources.requests.memory does not exist then value will be -Xmx512M -Xms512M.

We can leverage jvm only to control the -Xmx and -Xms values. For other settings we can jvmSettings or jvmOptions as new CRD? Then finally we can append all the values to a string.

@patelsmit32123 @josedev-union WDYT?

@prudhvigodithi Thank you for the suggestion. With this new approach, we’ll need to clearly define which configuration should take precedence. In particular, if users already specify heap size options under jvmSettings/jvmOptions, how should that be reconciled with the proposed behavior?
From my perspective, introducing new fields in the CRD solely to control heap configuration might not be ideal, as it could increase complexity and redundancy.

[patelsmit32123]

Seeing the documentation https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/userguide/main.md

• If jvm is not provided then the java heap size will be set to half of resources.requests.memory which is the recommend value for data nodes
• If jvm is not provided and resources.requests.memory does not exist then value will be -Xmx512M -Xms512M.

We can leverage jvm only to control the -Xmx and -Xms values. For other settings we can jvmSettings or jvmOptions as new CRD? Then finally we can append all the values to a string.
@patelsmit32123 @josedev-union WDYT?

@prudhvigodithi Thank you for the suggestion. With this new approach, we’ll need to clearly define which configuration should take precedence. In particular, if users already specify heap size options under jvmSettings/jvmOptions, how should that be reconciled with the proposed behavior? From my perspective, introducing new fields in the CRD solely to control heap configuration might not be ideal, as it could increase complexity and redundancy.

My opinion is also to not have multiple fields to handle jvm options. Also, there won't be any backward compatibility issues with approach of having just 1 field.

[patelsmit32123]

@prudhvigodithi @josedev-union can we please finalize on the approach?

[devops-taehoon-kim]

After adding the jvm flag in our production Opensearch environment, the Heap setting was fixed to 1GB.

I suffered from this issue.

I'm glad it will be fix soon.