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

[Backport] [2.x] Update bundled JDK to JDK-21.0.1 (#10576) #11003

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

reta
Copy link
Collaborator

@reta reta commented Oct 30, 2023

Backport of #10576 to 2.x

@reta
Copy link
Collaborator Author

reta commented Oct 30, 2023

@dblock @nknize @andrross switching bundled JDK in 2.x to JDK-21 as we discussed several times (primarily, to address #9423 & Apache Lucene's Panama usage)

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@reta reta force-pushed the backport/backport-10576-to-2.x branch from 9423fe1 to 3d7aa59 Compare October 30, 2023 14:32
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2023

Compatibility status:

Checks if related components are compatible with change cef267e

Incompatible components

Incompatible components: [https://github.com/opensearch-project/performance-analyzer.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #11003 (cef267e) into 2.x (822e253) will increase coverage by 0.00%.
Report is 1 commits behind head on 2.x.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##                2.x   #11003   +/-   ##
=========================================
  Coverage     70.90%   70.90%           
- Complexity    58928    58957   +29     
=========================================
  Files          4862     4862           
  Lines        278394   278401    +7     
  Branches      40810    40812    +2     
=========================================
+ Hits         197396   197408   +12     
- Misses        64189    64280   +91     
+ Partials      16809    16713   -96     
Files Coverage Δ
...a/org/opensearch/gradle/test/DistroTestPlugin.java 0.00% <ø> (ø)
.../opensearch/common/util/concurrent/BaseFuture.java 62.71% <ø> (ø)
...rg/opensearch/gradle/OpenSearchTestBasePlugin.java 6.38% <0.00%> (-0.22%) ⬇️
...g/opensearch/tools/launchers/SystemJvmOptions.java 0.00% <0.00%> (ø)

... and 466 files with indirect coverage changes

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

@reta I'm going to approve but as there might be downstream impact, please merge when you think it the best time

@reta
Copy link
Collaborator Author

reta commented Nov 1, 2023

@reta I'm going to approve but as there might be downstream impact, please merge when you think it the best time

Thanks a lot @peternied , I am waiting for @nknize @andrross @dblock @CEHENKLE signoff before moving forwards, the impact of this change (going unnoticed) is understood :)

@dblock
Copy link
Member

dblock commented Nov 1, 2023

I don't know what the implications of this change are.

@bbarani
Copy link
Member

bbarani commented Nov 1, 2023

I don't know what the implications of this change are.

At a high level, this might cause Gradle compatibility issues across multiple repos so we need to start a campaign soon.

@reta
Copy link
Collaborator Author

reta commented Nov 1, 2023

At a high level, this might cause Gradle compatibility issues across multiple repos so we need to start a campaign soon.

Thanks @bbarani, for Gradle specifically - no issues and no changes, this is only runtime change (which is as important as Gradle): we would need to make sure that all plugins work under JDK-21 as a runtime.

I don't know what the implications of this change are.

Thanks @dblock , explained above

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I don't have any strong opinions of when to merge this, your call @reta, PR approved.

IMO downstream plugin pain is not solvable without the kind of decoupling introduced by extensions

@reta
Copy link
Collaborator Author

reta commented Nov 10, 2023

Wouldn't it be better to merge early than later if we are targeting 2.12?

Sure, but this change needs awareness and alignment that it is OK to target 2.12

reta and others added 4 commits November 10, 2023 08:46
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 0d7d1e9)
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
…rch-project#5279)

This switch is removed in JDK version 17. https://openjdk.org/jeps/403

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@reta reta force-pushed the backport/backport-10576-to-2.x branch from fa560e8 to cef267e Compare November 10, 2023 13:46
@reta reta requested a review from tlfeng as a code owner November 10, 2023 13:46
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@bbarani
Copy link
Member

bbarani commented Nov 13, 2023

@dblock @reta @andrross What are the next steps? We will start incorporating the build changes in 2.x line based on the outcome.

@reta
Copy link
Collaborator Author

reta commented Nov 13, 2023

@dblock @reta @andrross What are the next steps? We will start incorporating the build changes in 2.x line based on the outcome.

@bbarani I will plan to merge it tomorrow

@reta reta merged commit b25f5a3 into opensearch-project:2.x Nov 14, 2023
17 of 18 checks passed
@reta
Copy link
Collaborator Author

reta commented Nov 14, 2023

Seeing no objections, merging JDK-21 into 2.x branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants