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

[PROPOSAL] Set OpenSearch 3.0.0 baseline JDK version to JDK-21 #10745

Closed
1 of 2 tasks
reta opened this issue Oct 19, 2023 · 14 comments
Closed
1 of 2 tasks

[PROPOSAL] Set OpenSearch 3.0.0 baseline JDK version to JDK-21 #10745

reta opened this issue Oct 19, 2023 · 14 comments
Assignees
Labels
Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request v3.0.0 Issues and PRs related to version 3.0.0

Comments

@reta
Copy link
Collaborator

reta commented Oct 19, 2023

Is your feature request related to a problem? Please describe.
Currently, the 3.0.0 baseline JDK version is set to JDK-11. The Apache Lucene 10 is going to require JDK-21 and it would make sense to align the OpenSearch JDK baseline requirements with that.

Describe the solution you'd like
Set the target / source level to JDK-17. Allow to use new features of the languages and the standard library.

Describe alternatives you've considered
Why not JDK-21? As of today, for a few reasons:

Depending on the release date of 3.0.0, we may reconsider the decision for a more aggressing JDK baseline.

Additional context
Backports to 2.x will be more complicated

@reta reta added the enhancement Enhancement or improvement to existing feature or request label Oct 19, 2023
@reta reta changed the title [PROPOSAL] Set 3.0.0 baseline JDK version to JDK-17 [PROPOSAL] Set OpenSearch 3.0.0 baseline JDK version to JDK-17 Oct 19, 2023
@reta reta self-assigned this Oct 19, 2023
@nknize
Copy link
Collaborator

nknize commented Oct 19, 2023

we are still trying to move to JDK-21 as a runtime

Note that a JDK 21 runtime is already supported and project Panama MMap and SIMD vector incubation APIs will be used by Lucene. It's just not the default, so I think this is more about deciding which version to bump for min supported and bundled JDK.

@reta
Copy link
Collaborator Author

reta commented Oct 19, 2023

Thanks @nknize , are still trying to move is about the rollout (the codebase, at least core, should be ready, there are issues with that outside of core): we are blocked on distributions availability.

@nknize
Copy link
Collaborator

nknize commented Oct 19, 2023

we are blocked on distributions availability.

By "distribution" I assume you mean the OpenSearch bundle? Min distribution is ready to go, just change the system runtime.

there are issues with that outside of core

This is a reason I'm not a fan of the OpenSearch bundle force installing all bundled OpenSearch plugins (originally decided back in the ODFE days). IMHO it's a better user experience to let the user choose which plugins they want to install instead of assuming they know which are safe to uninstall. It gives the user more control in keeping their runtime lean with only the features they want to use. If a plugin(s) is not JDK 21 compatible a user could still bump their bundle runtime if they didn't install the JDK incompatible plugin.

We could also provide a version compatibility check mechanism at plugin install and/or upgrade migration time to inform the user of JDK incompatibilities.

@reta
Copy link
Collaborator Author

reta commented Oct 19, 2023

By "distribution" I assume you mean the OpenSearch bundle? Min distribution is ready to go, just change the system runtime.

No, JDK-21 distributions, see please #10334 (TLDR; ppc64 is still being cooked, we need it)

If a plugin(s) is not JDK 21 compatible a user could still bump their bundle runtime if they didn't install the JDK incompatible plugin.

The problem is that some plugins use Gradle with Kotlin and this beautiful combination does not support JDK-21 at all, also see please #10334 (TLDR; people physically cannot build with JDK-21 even to verify compatibility)

We could also provide a version compatibility check mechanism at plugin install and/or upgrade migration time to inform the user of JDK incompatibilities.

That's correct, enforcing the baseline (JDK-21) at this point will be too much, JDK-17 seems to be reasonable.

@nknize
Copy link
Collaborator

nknize commented Oct 20, 2023

No, JDK-21 distributions.

Sorry I think there was confusion. I'm not suggesting we bump bundled JDK to 21. I'm very much +1 bumping bundled to 17 or maybe even 19. I was just making a comment that the vast majority of users can already use their own JDK 21 runtime and thus get the benefits of Panama MMap and incubating SIMD optimizations provided by the latest Lucene improvements. There are just some corner cases that can't. I want to make sure we don't send a misleading message that OpenSearch is not 21 runtime compatible. Doing this could provide marketing fodder to those less informed that wish to push reasons why a user should pick or switch to Elasticsearch instead.

TLDR; people physically cannot build with JDK-21 even to verify compatibility

I think maybe I'm not being clear here either so my apologies for the confusion. I'm not suggesting users trial by fire build with 21. I'm suggesting we provide a compatibility mechanism for them that checks their runtime and their list of installed plugins and informs the user if they have a plugin installed that's incompatible with the 21 runtime if they are interested in bumping it themselves. Though maybe it's not needed and a compatibility matrix is sufficient. I'm just brainstorming ideas to make the user experience more appealing.

some plugins use Gradle with Kotlin

Just a personal (controversial?) opinion here but kotlin is another reason I dislike the bundle installing all plugins by default. Kotlin is of course compatible with Java (jetbrains originally developed for improving java and many use it for Android) but they're not the same and there are subtle performance differences when comparing with Java that make me cautious trusting performance when calling OpenSearch java from kotlin code. I think we could do better here to have kotlin plugins provide more reliable benchmarking. Combine that with the fact that Kotlin lags JDK versions and delays upgrades like we see with JDK 21 and it only substantiates my opinion that we should rethink implementing bundled plugins in kotlin altogether. We certainly shouldn't force install them on users.

@reta
Copy link
Collaborator Author

reta commented Oct 20, 2023

Thanks @nknize , I think we touched upon very important topics but a bit derailed from the this proposal, I would blame myself for the confusion. I will try to clarify what exactly the goal is.

The OpenSearch 3.0.0 and 2.x codebases are requiring JDK-11 baseline from the source and binary level, so for anyone using the OpenSearch bits the JDK-11 is a minimum required version.

  allprojects {
    java {
        targetCompatibility = JavaVersion.VERSION_11
        sourceCompatibility = JavaVersion.VERSION_11
    }
  }

I suggest for OpenSearch 3.0.0 we elevate the baseline to JDK-17 (for the reasons I shared in the description):

  allprojects {
    java {
        targetCompatibility = JavaVersion.VERSION_17
        sourceCompatibility = JavaVersion.VERSION_17
    }
  }

How it relates to bundled distributions? The 2.x is bundled with JDK-17 LTS right now (https://github.com/opensearch-project/OpenSearch/blob/2.x/buildSrc/version.properties#L5) but anyone could run it on JDK-11+ (since this is a baseline). For 3.x (unless the release won't happen till JDK-25), we would bundle JDK-21 LTS, but anyone could run it on JDK-17+ (with this proposal).

Does look a bit more clear? Thanks again!

@nknize
Copy link
Collaborator

nknize commented Oct 20, 2023

but a bit derailed from the this proposal, I would blame myself for the confusion.

No worries at all! I should've started with "I agree bumping the min required version..." instead of saying "I think this is more about deciding which version to bump for min supported and bundled JDK." and then clarified the 21 runtime just so folks don't think we have zero support.

I suggest for OpenSearch 3.0.0 we elevate the baseline to JDK-17 (for the reasons I shared in the description):

I've been mulling this one over a bit so I just went ahead and opened an issue to transfer what's in my head to github for discussion. For 3.0.0 (which is a ways off) we could consider bumping to JDK19 to guarantee availability of preview features like virtual threads. This would enable us to explore using them as an optimization for some of the distributed I/O heavy ops (disk, network) to bump overall OpenSearch performance by default. Lucene doesn't much care because of some reasons discussed in relatively recent discussions.

@reta
Copy link
Collaborator Author

reta commented Oct 20, 2023

Thanks @nknize

For 3.0.0 (which is a ways off) we could consider bumping to JDK19 to guarantee availability of preview features like virtual threads.

What is the reason for JDK-19 (non LTS and dead) vs going JDK-21? We could also go 2 step, JDK-17 -> JDK-21, requiring JDK-19 at this point I think is not needed, no one runs on it (I am 100% sure).

@nknize
Copy link
Collaborator

nknize commented Oct 20, 2023

(non LTS and dead)

That's right I forgot it was superseded.

requiring JDK-19 at this point I think is not needed

I agree at this point we don't need it since Lucene already handles the mrjar logic for us. I think for vThreads work it would be useful but 21 will likely be fully supported by the time that work is done.

We could also go 2 step, JDK-17 -> JDK-21

+1 to this as the approach. We can prototype 21 vThreads and then make the move if we merge it as a Feature Flagged enhancement.

@reta
Copy link
Collaborator Author

reta commented Jan 18, 2024

Finally found the time to prototype the path forward. In order to have the baseline change as smooth as possible, we would need to start from the plugins first:

  • update all non-core plugins to have JDK-17 baseline
  • update all shared libraries to have JDK-17 baseline
  • update core to have JDK-17 baseline

Why: If we update core first, we will break all and every plugin out there since most of them are being built with JDK 11/17/21 (and trying to compile JDK-17 bytecode with JDK-11 will fail). Contrary, OpenSearch uses JDK-21 as the runtime and does not care if the plugins have JDK-17 baseline (or in general, any baseline <= JDK-21), it could keep JDK-11 baseline till all plugins are levelled up.

@bbarani
Copy link
Member

bbarani commented Feb 6, 2024

@reta @nknize can you please confirm if this change can be included in 2.x without breaking existing API? Basically can this change be added in a backward compatible manner in 2.x line?

We are evaluating if this change requires 3.0 release or can be included in 2.x line so need your inputs.

@reta
Copy link
Collaborator Author

reta commented Feb 6, 2024

We are evaluating if this change requires 3.0 release or can be included in 2.x line so need your inputs.

@bbarani This is a JDK baseline change - it will break every single application / plugin / extension sadly.

@reta reta changed the title [PROPOSAL] Set OpenSearch 3.0.0 baseline JDK version to JDK-17 [PROPOSAL] Set OpenSearch 3.0.0 baseline JDK version to JDK-21 Mar 11, 2024
This was referenced Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

No branches or pull requests

3 participants