prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
15.75k stars 5.29k forks source link

Update Java requirement to Java 11 #14873

Open rschlussel opened 3 years ago

rschlussel commented 3 years ago

This blocks https://github.com/prestodb/presto/issues/14549#issuecomment-656571077 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?

beinan commented 3 years ago

An open PR I post earlier. https://github.com/prestodb/presto/pull/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

mbasmanova commented 3 years ago

@beinan Beinan, FYI, @sujay-jain Sujay and @bhhari Bhavani are rolling out Java 11 and Graal at FB at this very moment.

beinan commented 3 years ago

@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 https://github.com/prestodb/presto/pull/15018

junyi1313 commented 3 years ago

@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.

beinan commented 3 years ago

@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.

junyi1313 commented 3 years ago

@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.

beinan commented 3 years ago

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) -server -Xms200G -Xmx200G -XX:+PreserveFramePointer -XX:-UseBiasedLocking -XX:+UnlockExperimentalVMOptions -XX:G1HeapRegionSize=32M -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent -XX:+UseGCOverheadLimit -XX:MaxMetaspaceSize=1G -XX:ReservedCodeCacheSize=250M -XX:+AggressiveOpts -XX:+HeapDumpOnOutOfMemoryError -XX:+ExitOnOutOfMemoryError -XX:+PrintGCDetails -XX:+PrintSafepointStatistics -XX:PrintSafepointStatisticsCount=1 -XX:-UseAdaptiveSizePolicy -XX:+UnlockDiagnosticVMOptions -XX:InitiatingHeapOccupancyPercent=40 -XX:+UseStringDeduplication -XX:StringDeduplicationAgeThreshold=6 -verbose:gc -Xlog:gc:var/log/gc.log -Djdk.nio.maxCachedBufferSize=0 -Djdk.attach.allowAttachSelf=true

junyi1313 commented 3 years ago

Thank you @beinan, that's really helpful.

beinan commented 3 years ago

@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.

junyi1313 commented 3 years ago

@beinan sure, we will upgrade to java 11 soon.

mbasmanova commented 5 days ago

CC: @amitkdutta

@rschlussel Is there anything left to do here? Do we document required Java version ?

CC: @tdcmeehan @steveburnett @elharo

steveburnett commented 5 days ago

CC: @amitkdutta

@rschlussel Is there anything left to do here? Do we document required Java version ?

CC: @tdcmeehan @steveburnett @elharo

Yes, the required Java version is documented in the README for Presto.

https://github.com/prestodb/presto/blob/master/README.md#requirements

zhenxiao commented 4 days ago

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+. @beinan is making progress on this

elharo commented 4 days ago

Don't confuse building and supporting for Java 8 with running on Java 8. Presto runs fine on Java 11.

zhenxiao commented 4 days ago

yep, I think this thread is for: we need Presto both compiling and running on Java11. ? Or we only need run Presto on Java11?

elharo commented 4 days ago

You can compile it with Java 11. You can run it on 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.

zhenxiao commented 3 days ago

oh, are we good on compiling and running on java11, java17, and java21? Last time I hit compiling errors when using Java11 or Java17

zhenxiao commented 3 days ago

here is the compilation error: [ERROR] Failed to execute goal com.github.spotbugs:spotbugs-maven-plugin:3.1.10:spotbugs (spotbugs) on project presto-root: Execution spotbugs of goal com.github.spotbugs:spotbugs-maven-plugin:3.1.10:spotbugs failed: Unable to load the mojo 'spotbugs' in the plugin 'com.github.spotbugs:spotbugs-maven-plugin:3.1.10'. A required class is missing: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

elharo commented 3 days ago

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.