Skip to content

Presto: Fix Java memory config#115

Closed
simoneves wants to merge 2 commits intomainfrom
seves/fix_presto_java_config_once_and_for_all_alt
Closed

Presto: Fix Java memory config#115
simoneves wants to merge 2 commits intomainfrom
seves/fix_presto_java_config_once_and_for_all_alt

Conversation

@simoneves
Copy link
Copy Markdown
Contributor

The recent PR #109 broke Java mode because of a typo in the config file mapping.

Also, there are still occasional failures due to the JVM heap size being set too large
compared with the remainder of the memory options, which are hard-wired in Java
mode, so we hard-wire that too instead of it being RAM-size dependent. We do this
in the generate script as a post-modification to avoid having to fork the config file.

volumes:
- ./config/generated/java/etc_common:/opt/presto-server/etc
- ./config/generated/java/etc_coordinator/config_java.properties:/opt/presto-server/etc/config.
- ./config/generated/java/etc_coordinator/config_java.properties:/opt/presto-server/etc/config.properties
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure how (1) I broke this, and (2) didn't find it before landing #109

# for Java variant, hard-wire the JVM heap sizes to 24GB
JVM_CONFIG="${CONFIG_DIR}/etc_common/jvm.config"
sed -i 's/Xmx.*G/Xmx24G/' ${JVM_CONFIG}
sed -i 's/Xms.*G/Xms24G/' ${JVM_CONFIG}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This resets the JVM configs to 24GB overwriting the auto-config, so that there's no mismatch with the separately-hard-wired other memory configs in Java mode.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we have to use fixed values in the Java config files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The values in the other config files are fixed. The auto-config PR broke that by having the JVM heap be auto when the rest were hard-wired, and that was unstable. I was never able to get auto-config stable with Java, and when I asked at the time, the consensus was that I shouldn't spend any more time on it as long as it worked with small SFs as a smoke test.

@simoneves
Copy link
Copy Markdown
Contributor Author

Closing as replaced bt #117

@simoneves simoneves closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants