Skip to content

Bump JDK to 24, gradle to 8.14 #2792

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 16, 2025
Merged

Bump JDK to 24, gradle to 8.14 #2792

merged 8 commits into from
Jul 16, 2025

Conversation

@finnroblin
Copy link
Contributor

@finnroblin finnroblin commented Jul 14, 2025

Description

Upgrade Java version to 24 and gradle to 8.14 as per ask from infra team.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@finnroblin
Copy link
Contributor Author

finnroblin commented Jul 14, 2025

Build failures are coming from the :remote-index-build-client:compileJava task.

...
Caused by: java.lang.NoSuchFieldException: com.sun.tools.javac.code.TypeTag :: UNKNOWN
        at lombok.permit.Permit.getField(Permit.java:144)
        at lombok.javac.JavacTreeMaker$SchroedingerType.getFieldCached(JavacTreeMaker.java:171)
        at lombok.javac.JavacTreeMaker$TypeTag.typeTag(JavacTreeMaker.java:259)
        at lombok.javac.Javac.<clinit>(Javac.java:186)

I am investigating if bumping the lombok version will fix this.

@finnroblin
Copy link
Contributor Author

finnroblin commented Jul 14, 2025

Build is working locally, rerunning CI with lombok upgrade.

@heemin32 heemin32 mentioned this pull request Jul 14, 2025
Merged
5 tasks
@finnroblin finnroblin marked this pull request as ready for review July 15, 2025 00:09
@finnroblin
Copy link
Contributor Author

finnroblin commented Jul 15, 2025

Ah. seems that we need accessUnixDomainSocket permission for windows, thanks to @expani for the help! (ref here)

@finnroblin
Copy link
Contributor Author

finnroblin commented Jul 15, 2025

Hi @reta, the k-NN plugin windows CI is failing after JDK 24 bump likely due to security manager deprecation. The cause appears to be an AccessController.doPrivileged action that makes a HTTP request to the remote vector build service.

I added permission java.net.NetPermission "accessUnixDomainSocket" based on your PR in transport plugins. However, that does not fix the CI run. Do you have any suggestions of other security/java agent changes required to onboard to JDK 24? Thanks

@reta
Copy link
Contributor

reta commented Jul 15, 2025

Do you have any suggestions of other security/java agent changes required to onboard to JDK 24? Thanks

Hi @finnroblin , sure, I will take a look shortly, couldn't point to the exact problem yet since we have seen a few. Thank you

@cwperks
Copy link
Member

cwperks commented Jul 15, 2025

@reta I'm about to raise a PR in core. A few min.

@cwperks cwperks mentioned this pull request Jul 15, 2025
Closed
3 tasks
@cwperks
Copy link
Member

cwperks commented Jul 15, 2025

@reta Not sure how to test, but I believe the changes in opensearch-project/OpenSearch#18752 would resolve the issue.

@finnroblin
changelog change to rerun CI
af11361 
Signed-off-by: Finn Roblin <[email protected]>
@finnroblin
Copy link
Contributor Author

finnroblin commented Jul 16, 2025

Windows CI passing after this change merged into core. Thanks @cwperks @reta!

jmazanec15
jmazanec15 approved these changes Jul 16, 2025
View reviewed changes
Copy link
Member

@jmazanec15 jmazanec15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@martin-gaievski martin-gaievski mentioned this pull request Jul 16, 2025
Merged
naveentatikonda
naveentatikonda reviewed Jul 16, 2025
View reviewed changes
src/main/plugin-metadata/plugin-security.policy
permission java.lang.RuntimePermission "loadLibrary.opensearchknn_faiss_avx512";
permission java.lang.RuntimePermission "loadLibrary.opensearchknn_faiss_avx512_spr";
permission java.net.SocketPermission "*", "connect,resolve";
permission java.net.NetPermission "accessUnixDomainSocket";
Copy link
Member

@naveentatikonda naveentatikonda Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding this?

Copy link
Contributor Author

@finnroblin finnroblin Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this permission for remote index build client on windows. Otherwise it will fail due to the OS security manager. OS core needed to add it to their transport plugins which use similar mechanisms to remote index build client.

Copy link
Member

@naveentatikonda naveentatikonda Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. Thanks!

@naveentatikonda naveentatikonda merged commit 6c9c621 into opensearch-project:main Jul 16, 2025
37 checks passed
leoyu808 pushed a commit to leoyu808/k-NN that referenced this pull request Jul 30, 2025
@finnroblin
Bump JDK to 24, gradle to 8.14 (opensearch-project#2792)
bdb9698 
* Bump JDK to 24, gradle to 8.14

Signed-off-by: Finn Roblin <[email protected]>

* bump lombok to 8.14, add changelog entry

Signed-off-by: Finn Roblin <[email protected]>

* bump jacoco version

Signed-off-by: Finn Roblin <[email protected]>

* remove java 24 from bwc to avoid getTotalPhysicalMemorySize/getTotalMemorySize deprecation issue

Signed-off-by: Finn Roblin <[email protected]>

* switch from deprecate java SM to opensearch SM in RemoteIndexHTTPClient

Signed-off-by: Finn Roblin <[email protected]>

* add accessUnixDomainSocket to fix windows CI

Signed-off-by: Finn Roblin <[email protected]>

* changelog change to rerun CI

Signed-off-by: Finn Roblin <[email protected]>

---------

Signed-off-by: Finn Roblin <[email protected]>
Signed-off-by: Leo Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@naveentatikonda naveentatikonda naveentatikonda approved these changes

@jmazanec15 jmazanec15 jmazanec15 approved these changes

@heemin32 heemin32 Awaiting requested review from heemin32 heemin32 is a code owner

@navneet1v navneet1v Awaiting requested review from navneet1v navneet1v is a code owner

@VijayanB VijayanB Awaiting requested review from VijayanB VijayanB is a code owner

@vamshin vamshin Awaiting requested review from vamshin vamshin is a code owner

@junqiu-lei junqiu-lei Awaiting requested review from junqiu-lei junqiu-lei is a code owner

@martin-gaievski martin-gaievski Awaiting requested review from martin-gaievski martin-gaievski is a code owner

@ryanbogan ryanbogan Awaiting requested review from ryanbogan ryanbogan is a code owner

@luyuncheng luyuncheng Awaiting requested review from luyuncheng luyuncheng is a code owner

@shatejas shatejas Awaiting requested review from shatejas shatejas is a code owner

@0ctopus13prime 0ctopus13prime Awaiting requested review from 0ctopus13prime 0ctopus13prime is a code owner

@Vikasht34 Vikasht34 Awaiting requested review from Vikasht34 Vikasht34 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@finnroblin @reta @cwperks @jmazanec15 @naveentatikonda