Skip to content

OCPBUG 6952: Modified 'understanding how to override JVM max heap size' section #68617

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OCPBUG 6952: Modified 'understanding how to override JVM max heap size' section #68617

wants to merge 1 commit into from

Conversation

subhtk
Copy link
Contributor

@subhtk subhtk commented Nov 30, 2023 •
edited
Loading

Version(s): 4.12+

Issue: OCPBUG 6952

Link to docs preview: Understanding how to override the JVM maximum heap size

QE review:

  • QE has approved this change.

Additional information:

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 30, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Nov 30, 2023 •
edited
Loading

🤖 Thu Jun 06 13:31:25 - Prow CI generated the docs preview:

https://68617--ocpdocs-pr.netlify.app/openshift-dedicated/latest/nodes/clusters/nodes-cluster-resource-configure.html
https://68617--ocpdocs-pr.netlify.app/openshift-enterprise/latest/nodes/clusters/nodes-cluster-resource-configure.html
https://68617--ocpdocs-pr.netlify.app/openshift-rosa/latest/nodes/clusters/nodes-cluster-resource-configure.html

@subhtk subhtk force-pushed the OCPBUG6952 branch 2 times, most recently from f7326ef to 0c308bb Compare December 7, 2023 10:41
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 7, 2023
lyman9966
lyman9966 reviewed Dec 13, 2023
View reviewed changes
@subhtk subhtk force-pushed the OCPBUG6952 branch 2 times, most recently from b0f0353 to 3b63bf1 Compare December 14, 2023 08:44
lyman9966
lyman9966 reviewed Dec 14, 2023
View reviewed changes
@subhtk
Copy link
Contributor Author

subhtk commented Dec 14, 2023

Thanks for the review @lyman9966.

@subhtk
Copy link
Contributor Author

subhtk commented Dec 20, 2023

@lyman9966 If everything looks fine, can I move forward with the further process? And can you please add LGTM, once the review is finished?

@lyman9966
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 21, 2023
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Dec 21, 2023

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 21, 2023
@subhtk
Copy link
Contributor Author

subhtk commented Dec 21, 2023

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Dec 21, 2023
@bscott-rh bscott-rh added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Dec 21, 2023
snarayan-redhat
snarayan-redhat reviewed Dec 21, 2023
View reviewed changes
Copy link
Contributor

@snarayan-redhat snarayan-redhat left a comment

Choose a reason for hiding this comment

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

Few things, apart from the comments, that need attention:

  1. The preview link needs to be updated.
  2. The indentation of the whole section needs to be fixed.
  3. IIUC, memory is generally addressed with Gi in docs. Kindly verify with SME on the units and update according to doc standards.

@bscott-rh ptal

modules/nodes-cluster-resource-configure-jdk.adoc Outdated
* If the container memory limit is set and the experimental options are
supported by the JVM, set `-XX:+UnlockExperimentalVMOptions
-XX:+UseCGroupMemoryLimitForHeap`.
Currently, on bare metal or VM setups, the OpenJDK's default heap value is 25%, 1/4th of the compute node’s memory. However, within containers using cgroups detection (v1 and v2 for JDK 11/17), Xmx was set as 50% of the max memory resource limits by default. In ubi9 - RHEL 9 images, the default allocation is 80%(set via -XX:MaxRAMPercentage), leaving 20% for native use. It is therefore *essential* to
Copy link
Contributor

Choose a reason for hiding this comment

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

Few questions that the content should reflect:

  1. How are default values xmx related to the heap value?
  2. if xmx was set to 50%, what is it now? Question 1 might address this.
  3. Why do we have the earlier Xmx value mentioned? We should document only the current behaviour of the product.

If everything is related to the heap, then this would need rewording to make it more readable.

bscott-rh
bscott-rh reviewed Dec 21, 2023
View reviewed changes
Copy link
Contributor

@bscott-rh bscott-rh left a comment

Choose a reason for hiding this comment

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

Added 1 small comment in addition to Shubha's feedback

@subhtk subhtk force-pushed the OCPBUG6952 branch 2 times, most recently from 29b9563 to 0359a7b Compare December 26, 2023 12:48
@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 21, 2024
@jmtd
Copy link

jmtd commented Oct 21, 2024

/remove-lifecycle stale

Coming up to a year since this was opened, how are we doing?

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 21, 2024
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2025
@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 20, 2025
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jerboaa
Copy link

jerboaa commented Jan 20, 2025

Why hasn't this moved in a while? Seems a shame getting everything reviewed and then let it go stale.

@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 20, 2025
@jmtd
Copy link

jmtd commented Feb 20, 2025

/remove-lifecycle rotten
/lifecycle frozen

@openshift-ci openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 20, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 20, 2025

@jmtd: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/remove-lifecycle rotten
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jerboaa
Copy link

jerboaa commented Feb 20, 2025

It would still be good if this could go in. Existing doc is very outdated.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 22, 2025
@jeana-redhat jeana-redhat removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 27, 2025
@jeana-redhat jeana-redhat added this to the Continuous Release milestone May 27, 2025
@subhtk subhtk closed this Jun 4, 2025
@subhtk subhtk reopened this Jun 4, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 4, 2025

@subhtk: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/validate-asciidoc 4729cb7 link true /test validate-asciidoc
ci/prow/validate-portal 4729cb7 link true /test validate-portal

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmtd jmtd jmtd left review comments

@lyman9966 lyman9966 lyman9966 left review comments

@jeana-redhat jeana-redhat jeana-redhat left review comments

@snarayan-redhat snarayan-redhat snarayan-redhat left review comments

@bscott-rh bscott-rh bscott-rh left review comments

@jerboaa jerboaa jerboaa approved these changes

Assignees

@lyman9966 lyman9966

Labels
branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 branch/enterprise-4.16 branch/enterprise-4.17 branch/enterprise-4.18 branch/enterprise-4.19 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
Continuous Release
Development

Successfully merging this pull request may close these issues.