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.

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

Open
rschlussel opened this issue Jul 22, 2020 · 19 comments
Open

Update Java requirement to Java 11 #14873

rschlussel opened this issue Jul 22, 2020 · 19 comments

Comments

@rschlussel
Copy link
Contributor

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?

@beinan
Copy link
Member

beinan commented Jul 31, 2020

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

@mbasmanova
Copy link
Contributor

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

@beinan
Copy link
Member

beinan commented Aug 12, 2020

@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

@junyi1313
Copy link
Contributor

@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
Copy link
Member

beinan commented Jun 8, 2021

@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
Copy link
Contributor

@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
Copy link
Member

beinan commented Jun 8, 2021

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
Copy link
Contributor

Thank you @beinan, that's really helpful.

@beinan
Copy link
Member

beinan commented Jun 9, 2021

@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
Copy link
Contributor

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

@mbasmanova
Copy link
Contributor

CC: @amitkdutta

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

CC: @tdcmeehan @steveburnett @elharo

@steveburnett
Copy link
Contributor

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
Copy link
Collaborator

zhenxiao commented Jul 3, 2024

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
Copy link
Contributor

elharo commented Jul 3, 2024

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

@zhenxiao
Copy link
Collaborator

zhenxiao commented Jul 3, 2024

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
Copy link
Contributor

elharo commented Jul 3, 2024

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
Copy link
Collaborator

zhenxiao commented Jul 3, 2024

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
Copy link
Collaborator

zhenxiao commented Jul 3, 2024

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
Copy link
Contributor

elharo commented Jul 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants