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 to Gradle 7.6 and JDK-19 #4973

Merged
merged 5 commits into from
Nov 8, 2022
Merged

Conversation

reta
Copy link
Collaborator

@reta reta commented Oct 28, 2022

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

Update to Gradle 7.6 and JDK-19

Issues Resolved

Closes #4637

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:


try {
try {
// The getMetadata(File) is used by Gradle pre-7.6
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:-(

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@reta
Copy link
Collaborator Author

reta commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

* **RESULT:** FAILURE x

* **URL:** https://build.ci.opensearch.org/job/gradle-check/6292/

* **CommitID:** [2cc47de](https://github.com/opensearch-project/OpenSearch/commit/2cc47dead0926577b12062432a35880e9849abe1)
  Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
  Is the failure [a flaky test](https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#flaky-tests) unrelated to your change?

:o his is new

[Pipeline] End of Pipeline
java.lang.NullPointerException

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

} catch (KillWorkerError kwe) {
// hacks everywhere
}
super(name, 0, 1, 0L, TimeUnit.MILLISECONDS, r -> new Thread(() -> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@dblock
Copy link
Member

dblock commented Nov 8, 2022

I want to make sure we're ok upgrading the bundled JDK to 19. I'm going to merge and we can always revert. But any downsides? Thoughts? cc: @nknize

@dblock dblock merged commit ab85c67 into opensearch-project:main Nov 8, 2022
@reta
Copy link
Collaborator Author

reta commented Nov 8, 2022

I want to make sure we're ok upgrading the bundled JDK to 19. I'm going to merge and we can always revert. But any downsides? Thoughts? cc: @nknize

@dblock I think @nknize is all in for JDK-19 (#4637 (comment))

@dblock
Copy link
Member

dblock commented Nov 8, 2022

I want to make sure we're ok upgrading the bundled JDK to 19. I'm going to merge and we can always revert. But any downsides? Thoughts? cc: @nknize

@dblock I think @nknize is all in for JDK-19 (#4637 (comment))

Now I am really concerned 🤣

@andrross
Copy link
Member

andrross commented Nov 8, 2022

I want to make sure we're ok upgrading the bundled JDK to 19

I have strong reservations about creating any OpenSearch release that bundles a non-LTS version of Java. I know it has been done in the past but it feels like we're setting users up to quickly be in a bad spot. However, I have no objections to integrating early with new Java features on the unstable main branch here.

@reta
Copy link
Collaborator Author

reta commented Nov 8, 2022

I want to make sure we're ok upgrading the bundled JDK to 19

I have strong reservations about creating any OpenSearch release that bundles a non-LTS version of Java. I know it has been done in the past but it feels like we're setting users up to quickly be in a bad spot. However, I have no objections to integrating early with new Java features on the unstable main branch here.

@andrross I share your concern, @dblock what we could do:

  • revert bundled JDK to LTS (17)
  • configure Gradle check against Java runtimes: 11/17/19 (we have all needed support now)

What do you think guys?

@dblock
Copy link
Member

dblock commented Nov 8, 2022

I am 50/50. On one hand main is ok to go fast and forward, but on the other hand maybe we're being too quick. I like the plan above.

@nknize
Copy link
Collaborator

nknize commented Nov 8, 2022

I have strong reservations about creating any OpenSearch release that bundles a non-LTS version of Java.

Bundled for 3.0, yes. Not for 2.x. OpenSearch 3.0 will not be released until TBD next year, most likely 6 months (at the earliest) after JDK 19 stable (which was already released). The next LTS version is not planned until, jdk21? Non-LTS doesn't mean unstable so I don't think we need to succumb to FUD for bundling main w/ JDK 19 now.

/cc @uschindler @rmuir?, curious on your thoughts since y'all stay plugged in w/ the JDK community.

@rmuir
Copy link
Contributor

rmuir commented Nov 8, 2022

we require java 17 for lucene, but we have some stuff that kicks in if you use 19, and pass --preview.

I don't understand the bundling of JDKs at all though: seems like a nightmare to include such a thing even if it is LTS version. Who patches it with critical security vulnerabilities etc? Do you release a new opensearch everytime jdk releases a new minor version? You are bundling it!

To me it seems better to just require a minimal version and leave the system package manager to the task of maintaining the JDK.

@uschindler
Copy link
Contributor

I think that's inherited from the former codebase. I always try to download the zip of Elasticsearch or Opensearch without bundled JDK. But it gets harder to do that. Otherwise I do rm -rf

@reta
Copy link
Collaborator Author

reta commented Nov 8, 2022

I don't understand the bundling of JDKs at all though: seems like a nightmare to include such a thing even if it is LTS version. Who patches it with critical security vulnerabilities etc? Do you release a new opensearch everytime jdk releases a new minor version?

Yeah, it is inherited. We regularly update but AFAIK we do not align OpenSearch releases with JDK patch releases, we also have no-jdk bundles.

@nknize
Copy link
Collaborator

nknize commented Nov 9, 2022

I think that's inherited from the former codebase.

That's right. I only vaguely recall conversations there about justifications for bundling (might've been after @rmuir time?). I seem to recall the reasons revolving around OS distribution / container (docker, kubernetes) control (e.g., having control over every dependency to support glibc / CentOS nightmares - or at least make debugging them easier).

I always try to download the zip of Elasticsearch or Opensearch without bundled JDK. But it gets harder to do that. Otherwise I do rm -rf

I think for self managed this is probably pretty common? Of course there is a large base that just uses containers (which I believe primarily uses the bundled jdk)

To me it seems better to just require a minimal version and leave the system package manager to the task of maintaining the JDK.

+1 , and probably leave the jdk version/config up to the user in the dockerfile? (I'm not a big container user so my knowledge is inch deep here)

@rmuir
Copy link
Contributor

rmuir commented Nov 9, 2022

If i built a container for opensearch, let's say using centos as a base image, i still don't want a bundled JDK :) I would expect to just issue 'yum install -y opensearch-X-Y-Z' in my Dockerfile.

And I'd expect the opensearch package there not to contain a (Possibly outdated) JDK, but instead to have a package dependency, say on java-11-openjdk-headless or java-17-openjdk-headless or something like that: always with the latest bugfixes and security patches. I can rebuild container to have the latest stuff, no opensearch involvement needed.

Personally I think bundling JDKs is a big risk... a "bundled" JDK is like a "secret" JDK that exists outside of my package manager, never to get security updates etc. It isn't even obvious that it exists to someone doing a security audit or anything like that, they may miss it.

@dblock dblock mentioned this pull request Nov 9, 2022
@dblock
Copy link
Member

dblock commented Nov 9, 2022

I am pretty sure that a bundled JDK was for "ease of use". I opened #5187 to continue discussing, and your thoughts would very much be appreciated. Particularly I'd like to understand in that issue what would break if we removed JDK bundling altogether (tar distribution would not work without a pre-installed JDK, but other than that? please add your comments to the issue).

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

Successfully merging this pull request may close these issues.

After update to Lucene 9.4 use --enable-preview on Java==19 (exact) to allow mmap use new JDK APIs
6 participants