Add Lucene 9.5 codec and make it new default #700
Conversation
martin-gaievski
added
the
Maintenance
Signed-off-by: Martin Gaievski <[email protected]>
martin-gaievski
force-pushed
the
add_lucene95_codec
branch
from
8d07865 to
10c6a2c
Compare
There was a problem hiding this comment.
Can we mention in https://github.com/opensearch-project/k-NN/blob/main/DEVELOPER_GUIDE.md#codec-versioning that it should be in sync with OpenSearch's codec?
Yes, let me update the doc, in addition to version sync we moved lot of logic from factory to codec version class. |
Signed-off-by: Martin Gaievski <[email protected]>
| testMultiFieldsKnnIndex(KNN950Codec.builder().delegate(V_9_5_0.getDefaultCodecDelegate()).build()); | ||
| } | ||
|
|
||
| public void testBuildFromModelTemplate() throws InterruptedException, ExecutionException, IOException { |
There was a problem hiding this comment.
see if you want to use @SneakyThrows to remove these exceptions from method signature.
There was a problem hiding this comment.
oh, nice advise, let me replace list of exceptions with a sneakythrows
| @@ -74,9 +77,24 @@ public enum KNNCodecVersion { | |||
| .knnVectorsFormat(new KNN940PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService))) | |||
| .build(), | |||
| KNN940Codec::new | |||
| ), | |||
|
|
|||
| V_9_5_0( | |||
There was a problem hiding this comment.
[Question]: The other enums which are present in the like V_9_4_0, are they getting used somewhere? if not lets remove them.
There was a problem hiding this comment.
they are referenced from codec classes, like here. So if we keep older codecs we also need that other enum members.
There was a problem hiding this comment.
For clarification - in enum we do have only codec versions starting from 9.1 (9.1.0, 9.2.0, 9.4.0). There are no references for older codecs, so it's not a problem if we decide to drop them in future to be consistent with what core OS supports.
Signed-off-by: Martin Gaievski <[email protected]>
| new KNN80DocValuesFormat(delegate.docValuesFormat()), | ||
| new KNN80CompoundFormat(delegate.compoundFormat()) | ||
| ), | ||
| (userCodec, mapperService) -> KNN940Codec.builder() |
There was a problem hiding this comment.
Is this supposed to be 95? KNN940Codec
There was a problem hiding this comment.
oh yes, my bad, correcting it now
Signed-off-by: Martin Gaievski <[email protected]>
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-700-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4f9a8b296241c316bbc2d7dbfe67f20a59893302
# Push it to GitHub
git push --set-upstream origin backport/backport-700-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.xThen, create a pull request where the |
* Add Lucene 9.5 codec and make it new default Signed-off-by: Martin Gaievski <[email protected]>
* Update lucene94 package Signed-off-by: Naveen Tatikonda <[email protected]> * Add Lucene 9.5 codec and make it new default (#700) * Add Lucene 9.5 codec and make it new default Signed-off-by: Martin Gaievski <[email protected]> * Update tests for backwards codecs (#710) Updates tests for backwards codecs to prevent backwards codecs from writing with a read only codec. Signed-off-by: John Mazanec <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> Signed-off-by: Martin Gaievski <[email protected]> Signed-off-by: John Mazanec <[email protected]> Co-authored-by: Martin Gaievski <[email protected]> Co-authored-by: John Mazanec <[email protected]>
Signed-off-by: Martin Gaievski [email protected]
Description
Adding new kNN codec that wraps Lucene new codec that is part of Lucene 9.5 and latest core. Existing 9.4 codec doesn't work as we use it for both reads and writes, but writes only allowed for latest codec version. This already blocking PRs for main in our CI.
Backport is not required as core 2.x is still on Lucene 9.4
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.