Skip to content
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

Increase GCLocker retry allocation count to avoid OOM too early #19026

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Increase GCLocker retry allocation count to avoid OOM too early #19026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 0 deletions core/docker/default/etc/jvm.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
-XX:+UseAESCTRIntrinsics
# Disable Preventive GC for performance reasons (JDK-8293861)
-XX:-G1UsePreventiveGC
# Reduce starvation of threads by GClocker, recommend to set about the number of cpu cores (JDK-8192647)
-XX:GCLockerRetryAllocationCount=32
Copy link
Member

Choose a reason for hiding this comment

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

update product test's jvm.config as well

Copy link
Member Author

@hackeryang hackeryang Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

update product test's jvm.config as well

I actually considered whether I should add in testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/jvm.config as well at the beginning of our contribution~

But I found that the heap space is 2G, which is shown below:
image
So I thought that most Trino nodes have less CPU cores than the number of memory(mostly 1:4 or 1:6), and this config may be used to a node having 1 CPU core, it may not need to add the parameter and keep the default value 2~

And I also saw this pr, it seems similar to what you need: #15632

Copy link
Member

Choose a reason for hiding this comment

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

i am not saying GCLockerRetryAllocationCount is needed for product tests
but, product tests jvm.config should be as close to the recommended settings as possible
so it will use lower Xmx by necessity, but GCLockerRetryAllocationCount should be same as recommended (unless there is a reason not to)

2 changes: 2 additions & 0 deletions core/trino-server-rpm/src/main/resources/dist/config/jvm.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@
-XX:+UseAESCTRIntrinsics
# Disable Preventive GC for performance reasons (JDK-8293861)
-XX:-G1UsePreventiveGC
# Reduce starvation of threads by GClocker, recommend to set about the number of cpu cores (JDK-8192647)
-XX:GCLockerRetryAllocationCount=32
9 changes: 6 additions & 3 deletions docs/src/main/sphinx/installation/deployment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ The following provides a good starting point for creating `etc/jvm.config`:
-XX:+UseAESCTRIntrinsics
-Dfile.encoding=UTF-8
# Disable Preventive GC for performance reasons (JDK-8293861)
-XX:-G1UsePreventiveGC
-XX:-G1UsePreventiveGC
# Reduce starvation of threads by GClocker, recommend to set about the number of cpu cores (JDK-8192647)
-XX:GCLockerRetryAllocationCount=32
```

You must adjust the value for the memory used by Trino, specified with `-Xmx`
Expand Down Expand Up @@ -173,8 +175,9 @@ prevents Trino from starting. You can workaround this by overriding the
temporary directory by adding `-Djava.io.tmpdir=/path/to/other/tmpdir` to the
list of JVM options.

We enable `-XX:+UnlockDiagnosticVMOptions` and `-XX:+UseAESCTRIntrinsics` to improve AES performance for S3, etc. on ARM64 ([JDK-8271567](https://bugs.openjdk.java.net/browse/JDK-8271567))
We disable Preventive GC (`-XX:-G1UsePreventiveGC`) for performance reasons (see [JDK-8293861](https://bugs.openjdk.org/browse/JDK-8293861))
We enable `-XX:+UnlockDiagnosticVMOptions` and `-XX:+UseAESCTRIntrinsics` to improve AES performance for S3, etc. on ARM64 ([JDK-8271567](https://bugs.openjdk.java.net/browse/JDK-8271567))
We disable Preventive GC (`-XX:-G1UsePreventiveGC`) for performance reasons (see [JDK-8293861](https://bugs.openjdk.org/browse/JDK-8293861))
We set GCLocker retry allocation count (`-XX:GCLockerRetryAllocationCount=32`) to avoid OOM too early (see [JDK-8192647](https://bugs.openjdk.org/browse/JDK-8192647))

(config-properties)=

Expand Down