-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[JPMS Support] Initial phase-0 Refactor of org.apache.lucene
codebase to eliminate top level split packages
#17241
Conversation
❌ Gradle check result for 5c0e270: FAILURE 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? |
55bfb3d
to
4788400
Compare
❌ Gradle check result for 4788400: FAILURE 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? |
❌ Gradle check result for 4788400: FAILURE 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? |
server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java
Outdated
Show resolved
Hide resolved
Thanks @prudhvigodithi , the general direction definitely makes sense to me, but some changes may lead to regressions or functional changes, that we should try to avoid at all costs (I think), thank you. |
Thanks @reta, I see you pointed one #17241 (comment), I will double check but is there anything else we should avoid ? |
I would be surprised to see no test failures ... |
server/src/main/java/org/opensearch/common/util/CuckooFilter.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/lucene/similarity/LegacyBM25Similarity.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java
Outdated
Show resolved
Hide resolved
4788400
to
b3347c3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17241 +/- ##
============================================
+ Coverage 72.36% 72.45% +0.09%
+ Complexity 65733 65662 -71
============================================
Files 5318 5314 -4
Lines 305674 305141 -533
Branches 44349 44265 -84
============================================
- Hits 221197 221092 -105
+ Misses 66341 65945 -396
+ Partials 18136 18104 -32 ☔ View full report in Codecov by Sentry. |
b3347c3
to
4fc0267
Compare
❕ Gradle check result for 4fc0267: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
4fc0267
to
2b85763
Compare
Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
2b85763
to
17a2020
Compare
org.apache.lucene
codebase to eliminate top level split packages org.apache.lucene
codebase to eliminate top level split packages
❌ Gradle check result for 17a2020: FAILURE 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? |
❌ Gradle check result for 17a2020: FAILURE 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? |
❌ Gradle check result for 17a2020: FAILURE 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? |
Description
Refactored most of the code part of
:server
org.apache.lucene
.Removed
server/src/main/java/org/apache/lucene/util/packed
which is used forCuckooFilter
. Moved the implementation directly toCuckooFilter
class asPacked64
.Removed
server/src/main/java/org/apache/lucene/index/OneMergeHelper.java
(used byOpenSearchConcurrentMergeScheduler
), moved some its methods toOpenSearchConcurrentMergeScheduler
(reuired few logging changes).From https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/SegmentInfo.java#L98, updated the
ShuffleForcedMergePolicy
to useaddDiagnostics
.Updated the
server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java
to to the more standardQueryParser
(fromXQueryParser
) as the base class. Looks like we dont needhandleBareFuzzy
anymore.Updated
CustomFieldHighlighter.java
withgetFieldOffsetStrategy
method.To
server/src/main/java/org/opensearch/lucene/util/automaton/MinimizationOperations.java
added newAutomaton
class copied from https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/automaton/Operations.java . Also removedtransition.transitionUpto = -1;
as from lucene https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/automaton/Transition.java#L51 transitionUpto is already initialized to-1
Pending
org.apache.lucene.codecs
andorg.apache.lucene.index
are part of Star Tree implementation.The pending
org.apache.lucene.search.grouping
I will address this in upcoming PR's.Related Issues
Part of #8110
Check List
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.