-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
Always use G1GC for product tests #21311
Conversation
@findepi do you recall why we've used other GC settings for some of the product test environments? |
yes, to reduce memory footprint. production deployments start with 64GB main memory, and product tests run in a very constraint environment. |
@findepi since then github workers doubled the memory. It seems that this change builds fine so I'd propose to merge it and observe the stability of the CI and revert if it degrades. WDYT? |
i don't see a point in such a change, nor i want to use more when running locally. |
@findepi consistency. We only used non-G1 for some product test environments but not for others. G1 is recommended and used in production environments. If there will be some problem with next JDK version and G1 I want to learn from the CI, not from production environments. Hence keeping it consistent with our own recommendations is desirable from my perspective. |
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
I agree with the idea that we should use consistent JVM setting across docs, tests, and everything else. Or alternatively if different JVM settings in terms of GC are appropriate we should also expose this fact to users in the docs .. and then test both. In the interest of keeping it simple.. I vote for one setting... |
What is the minimal heap size recommended for running Trino? |
I dont think we have such a recommendation as a generic advice. |
https://trino.io/docs/current/installation/deployment.html Large memory allocation beyond 32GB is recommended for production clusters. |
Sure.. but that is for production clusters in typical deployments. Its not the minimal heap size that it potentially workable for a minimal deployment for simple use cases. |
Agreed! |
Well... even in that case we ideally use the same GC though. Anyway .. this PR is dead so no worries. |
Why would we insist on using same GC algorithm if we run in widely different -- tiny -- heap for test purposes? |
In an ideal world we know what GC works best for specific workloads and machine sizes and assemble all that and expose the info to our users. I am not aware we have such knowledge anywhere or even if we tested these things. They also change across Java versions (since GC improvements are always a focus). For now .. we only recommend one type of setting to our users in the docs and the idea of this PR is to use and hence validate that setting for all scenarios. |
My intent was to have a gc that is close to production use cases when we land on 22 (thus G1 + region pinning). But I agree that heap sizes are too small probably for G1 to function properly, hence I'll leave this PR closed. |
G1 can function with small heaps too. It’s the default GC in the JVM nowadays. |
@martint do you see value in that PR? |
Yes, I think it's a good thing to be consistent across the board with respect to what GC algorithm we use. |
Does switching over to G1 increase OS memory consumption for product tests? |
@findepi if you are concerned about local memory usage we can put some constraint on memory used by Trino docker containers in the product tests launcher. What's the ideal cap? |
The settings that this PR removed were for that purpose. |
No description provided.