Skip to content

Presto: Fix Java memory config #115

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

Closed
simoneves wants to merge 2 commits into from
Closed

Presto: Fix Java memory config #115

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
5b0692b
Force JVM Heap Sizes to 24GB in Java mode
simoneves Oct 30, 2025
784327e
Fix broken config file mapping
simoneves Oct 30, 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
2 changes: 1 addition & 1 deletion presto/docker/docker-compose.java.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ services:
service: presto-base-coordinator
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

@simoneves simoneves Oct 30, 2025

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

- ./config/generated/java/etc_coordinator/node.properties:/opt/presto-server/etc/node.properties

presto-java-worker:
Expand Down
5 changes: 5 additions & 0 deletions presto/scripts/generate_presto_config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ EOF
# optimizer.default-filter-factor-enabled=true
COORD_CONFIG="${CONFIG_DIR}/etc_coordinator/config_native.properties"
sed -i 's/\#optimizer/optimizer/g' ${COORD_CONFIG}
elif [[ "${VARIANT_TYPE}" == "java" ]]; then
# 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

@simoneves simoneves Oct 30, 2025

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

@paul-aiyedun paul-aiyedun Oct 30, 2025

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

@simoneves simoneves Oct 30, 2025

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.

fi

# success message
Expand Down