Increase GCLocker retry allocation count to avoid OOM too early

[hackeryang]

Description

JDK 9 to 21 has a GCLocker bug which causes JVM to throw OOME too early, because the default value of -XX:GCLockerRetryAllocationCount is 2, most of Trino clusters in production environments has 8 to 16+ CPU physical cores(which mostly means 32 vcores), so threads may be starved from receiving memory by the GClocker. The relevant issue: https://bugs.openjdk.org/browse/JDK-8192647

The principle flow chart of obtaining GCLocker in JVM is as follows:

And this OOM phenomenon was also seen in our production environment:

After investigating the JVM tuning articles shared by some companies, I found that Tencent(i.e. the parent company of League of Legends) also set this parameter to 100:

I know our community will migrate to JDK 21 partially for auto vectorization in Project Hummingbird, so maybe we will need this parameter in a little long time.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Increase GCLocker retry allocation count to avoid OOM too early. ({issue}`19026`)

[hackeryang]

It seems that all unit tests have been passed, hi @losipiuk @wendigo , can you please help merge when you have time, thank you~

[findepi]

update product test's jvm.config as well

[hackeryang]

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:

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

[findepi]

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)

[hackeryang]

Hello @wendigo , could you please help add back the jvm config -XX:GCLockerRetryAllocationCount=32, i saw that our community removed it at jdk22:

However, the jdk community hasn't fixed the bug yet until jdk 24, or may not fix in a little long time:

[wendigo]

In our experiments it's no longer needed. Is there any particular reason why it should be there by default?

[hackeryang]

In our experiments it's no longer needed. Is there any particular reason why it should be there by default?

@wendigo It was because we have some small clusters which have little memory space and often met this error:

The Retried waiting for GCLocker too often error was because the default value of GCLockerRetryAllocationCount is 2, it means that if a thread tries to request for some memory and cannot be satisfied this time for waiting a GC(the heap memory is almost full), all the threads(based on the number of CPU cores) only share the chance of 2 times to retry(other threads can also be blocked for GC at the same time).

So workers can be easily crashed thrown by a OOM error in small clusters, which influence users' experience about robustness.

I have been told that some users at other companies also have small clusters and met this error, maybe your cluster is big and has many memory space, which is not easy to throw Retried waiting for GCLocker too often error~

[findepi]

@hackeryang the message above (Retried waiting for GCLocker too often ...), was this running with Java 21 or 22?

[hackeryang]

@hackeryang the message above (Retried waiting for GCLocker too often ...), was this running with Java 21 or 22?

@findepi It was often met in JDK 17 at our small clusters in the past, but according to the jdk issue: https://bugs.openjdk.org/browse/JDK-8192647

It also influence jdk 21 to 23, and still not decided when to fix, we don't have jdk21 environment for now, but i believe that in small clusters the error will still be met

[findepi]

Might be or might be not. AFAICT, the GCLockerRetryAllocationCount was removed after Java 22 became required for runtime. If you're on an older Trino version and do not use Java 22, you definitely are recommended to keep using the GCLockerRetryAllocationCount flag.