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

Upgrade Lucene to 9.8.0 #10276

Merged
merged 1 commit into from
Oct 1, 2023
Merged

Upgrade Lucene to 9.8.0 #10276

merged 1 commit into from
Oct 1, 2023

Conversation

msfroh
Copy link
Collaborator

@msfroh msfroh commented Sep 29, 2023

Description

9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Related Issues

N/A

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

github-actions bot commented Sep 29, 2023

Compatibility status:

Checks if related components are compatible with change 65d73e0

Incompatible components

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT:
  • URL:
  • CommitID: e5b556f
    Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
    Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@zhichao-aws
Copy link
Member

Hi, want to have a double check. This won't be backport to 2.x before 2.11 release, right?

9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Signed-off-by: Michael Froh <froh@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT:
  • URL:
  • CommitID: 65d73e0
    Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
    Is the failure a flaky test unrelated to your change?

2 similar comments
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT:
  • URL:
  • CommitID: 65d73e0
    Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
    Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT:
  • URL:
  • CommitID: 65d73e0
    Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
    Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.CloneSnapshotIT.testCloneShallowSnapshotIndex

@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

Merging #10276 (65d73e0) into main (d656e3d) will decrease coverage by 0.08%.
Report is 4 commits behind head on main.
The diff coverage is 67.64%.

@@             Coverage Diff              @@
##               main   #10276      +/-   ##
============================================
- Coverage     71.24%   71.17%   -0.08%     
+ Complexity    58260    58233      -27     
============================================
  Files          4830     4830              
  Lines        274575   274586      +11     
  Branches      40015    40020       +5     
============================================
- Hits         195629   195436     -193     
- Misses        62570    62759     +189     
- Partials      16376    16391      +15     
Files Coverage Δ
...org/opensearch/index/search/stats/SearchStats.java 82.02% <0.00%> (ø)
...java/org/opensearch/index/shard/IndexingStats.java 86.61% <0.00%> (ø)
...rg/opensearch/repositories/s3/S3BlobContainer.java 76.61% <76.66%> (-3.62%) ⬇️

... and 476 files with indirect coverage changes

@msfroh
Copy link
Collaborator Author

msfroh commented Sep 30, 2023

This won't be backport to 2.x before 2.11 release, right?

I don't know. For now, I'm just focused on applying the change to main.

We can theoretically backport to 2.x before the start of the release window.

@reta reta merged commit 797def6 into opensearch-project:main Oct 1, 2023
60 of 61 checks passed
@reta
Copy link
Collaborator

reta commented Oct 1, 2023

@CEHENKLE please let us know if you are OK making 2.11 release with Apache Lucene 9.8.0, AFAIK there are no regressions we are aware of, the changes we know about were integrated (#8668, #10031)

@heemin32
Copy link
Contributor

heemin32 commented Oct 2, 2023

+1 with backporting to 2.11.
With Lucene 9.8.0, #10246, and opensearch-project/k-NN#1182 together, a user can have parent join feature in nested query for vector search in OpenSearch 2.11.

@msfroh msfroh deleted the lucene_9_8_0 branch October 3, 2023 00:17
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Oct 3, 2023
9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Signed-off-by: Michael Froh <froh@amazon.com>
@dblock
Copy link
Member

dblock commented Oct 3, 2023

It’s cutting it a little short for 2.11 but it looks like the performance improvements are compelling.

@dblock
Copy link
Member

dblock commented Oct 3, 2023

@CEHENKLE please let us know if you are OK making 2.11 release with Apache Lucene 9.8.0, AFAIK there are no regressions we are aware of, the changes we know about were integrated (#8668, #10031)

Is there a backport PR ready? What are the reasons other than “it’s a little short before a code freeze” not to take it?

@reta
Copy link
Collaborator

reta commented Oct 3, 2023

Is there a backport PR ready? What are the reasons other than “it’s a little short before a code freeze” not to take it?

I could make one tomorrow morning (essentially bundling 2 PRs).

@msfroh
Copy link
Collaborator Author

msfroh commented Oct 3, 2023

I've had some folks approach me around the office about this with similar concerns to those voiced by @model-collapse in #8668:

  1. Before OpenSearch 2.11, the main branch of ml-commons is behind 2.x on some commits. Although this has fixed at the moment, we still suspect that some other repos may have the same kind of problems. The main branch should catch up with 2.x with the latest features & implementations before we back-port Lucene 9.8 to 2.x, otherwise, there will be endless conflicts to deal with.
  2. We believe that all repos have done their functional tests as well as some benchmarkings to their main branch with Lucene 9.8 snapshot and so far there was no negative feedback. But as a rigorous procedure, we suggest all the repo to reproduce their tests with the formal release artifacts of Luence 9.8 (not snapshot version) so as to validate whether there's some negative impact from the Lucene upgrade to OpenSearch. The neural search plugin is absorbing the performance improvement, we also want to see others' feedback.
  3. We'd like to see whether the change of APIs or runtime behavior will have impact to AWS hosted search service which will subscribe this upgrade in the feature. That is another topic. We shall be aware but not discuss it on github.

Based on our "release window" model where we have two weeks to produce a working distribution build from tomorrow's code freeze and the fact that we nearly missed it for 2.10, I'm starting to appreciate the risk that we could end up "just not releasing" 2.11. (Frankly, it is a time of year when people at AWS are particularly concerned about missing releases.)

@macohen
Copy link
Contributor

macohen commented Oct 4, 2023

Would it make sense to do a fast-follow 2.12 release with only Lucene 9.8?

deshsidd pushed a commit to deshsidd/OpenSearch that referenced this pull request Oct 9, 2023
9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Signed-off-by: Michael Froh <froh@amazon.com>
vikasvb90 pushed a commit to vikasvb90/OpenSearch that referenced this pull request Oct 10, 2023
9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Signed-off-by: Michael Froh <froh@amazon.com>
@LiuCao0614
Copy link

Hi @msfroh , do we have any plan on releasing a new version with this fix any time soon?

@reta
Copy link
Collaborator

reta commented Oct 25, 2023

Hi @msfroh , do we have any plan on releasing a new version with this fix any time soon?

@LiuCao0614 2.12 as per https://github.com/orgs/opensearch-project/projects/1

@LiuCao0614
Copy link

Cool thanks @reta .

shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
9.8.0 was officially released this morning:
https://lucene.apache.org/core/9_8_0/changes/Changes.html

Signed-off-by: Michael Froh <froh@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
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.

7 participants