-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Update Java requirement to Java 11 #14873
Comments
An open PR I post earlier. #14009 There are still quite a few unit test failures need fixing. We have been planning to canary 1~2 presto cluster on java 11 in twitter to collect some runtime data. But unfortunately we don't have bandwidth in h1 2020. Our motivation is that java 11 would have a better g1gc performance. We also wanna try enabling Graal. For this, we expect a performance improvement of around 5% , as we have observed on other services |
@beinan Beinan, FYI, @sujay-jain Sujay and @bhhari Bhavani are rolling out Java 11 and Graal at FB at this very moment. |
@mbasmanova just some updates: we're rolling out Java 11 to a canary cluster (~400 workers) at Twitter. So far so good. another PR about the java version parser for java 11 #15018 |
@beinan Hi, Beinan, any updates about Java 11? We also want to run PrestoDB on Java 11. Can we compile the code and run on Java 11 directly? Thanks. |
@junyi1313 Yes, my change had been merged last year, and we've running presto on java 11 for almost a year in twitter. Everything looks good, and the performance and stability looks better. |
@beinan That sounds great. We will try it. Besides, do we have a JVM config file template (e.g. jvm-config), and an IDE guide for Java 11 (e.g. Running Trino in your IDE)? That will be very useful for the developers, especially for JVM performance tuning. Thanks. |
Hey @junyi1313 , sure, there is a jvm config template for prestodb. https://prestodb.io/docs/current/installation/deployment.html#jvm-config Running presto in IDE (IDEA intelliJ): https://github.com/prestodb/presto/blob/master/README.md#running-presto-in-your-ide I also post the jvm config I'm using for large heap (>=100GB) |
Thank you @beinan, that's really helpful. |
@junyi1313 np, let us know if you're seeing any failure and performance downgrading on java 11. Thanks! I'm also having a working branch for java 14 and zgc, but there are still lots of fixing needed. |
@beinan sure, we will upgrade to java 11 soon. |
CC: @amitkdutta @rschlussel Is there anything left to do here? Do we document required Java version ? |
Yes, the required Java version is documented in the README for Presto. https://github.com/prestodb/presto/blob/master/README.md#requirements |
thanks for bring this up. The java version is becoming critical. Our java required version(java8) is really old. Companies are having requirements like Java service needs to be at java11+. |
Don't confuse building and supporting for Java 8 with running on Java 8. Presto runs fine on Java 11. |
yep, I think this thread is for: |
You can compile it with Java 11. You can also compile and run it on Java 8, 17, 21, and pretty much any version from 8 on. Perhaps one day Oracle will finally remove sun.misc.Unsafe, and we might want to think about what to do about that now. However current JDKs from 8 on are not a problem. I still don't see any reason presented why we should not support Java 8. |
oh, are we good on compiling and running on java11, java17, and java21? Last time I hit compiling errors when using Java11 or Java17 |
here is the compilation error: |
That's a spotbugs issue, not a compiler or code issue. Might just need a newer version of spotbugs. I think 4.8.6.1 is current. If the newer version still doesn't work with Java 11, we can remove it. It's a nice to have, not a requirement. |
This blocks #14549 (comment) which has a dependency that requires java 11 support. Presumably more cases like this will come up in the future. Also, we may want to use java 11 features.
I know at facebook, we needed to do a lot of testing and change some java configurations in order to be able to successfully run with Java 11, so I don't think we want to change it without advanced warning or some guideline.
@sujay-jain any comments to share
@zhenxiao @vkorukanti what do you think?
The text was updated successfully, but these errors were encountered: