Add DocValues Support for Lucene Byte Sized Vector #953

naveentatikonda · 2023-07-07T00:22:08Z

Description

The changes in this PR adds doc values support for fields with byte sized vectors, which helps users to perform script scoring and painless scripting search functionalities on these indices.

Issues Resolved

#812

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed as per the DCO using --signoff

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.

codecov · 2023-07-07T00:40:47Z

Codecov Report

Merging #953 (0eb0adf) into feature/lucene_byte_vector (1386519) will decrease coverage by 0.01%.
The diff coverage is 92.30%.

@@                       Coverage Diff                        @@
##             feature/lucene_byte_vector     #953      +/-   ##
================================================================
- Coverage                         85.07%   85.07%   -0.01%     
- Complexity                         1129     1141      +12     
================================================================
  Files                               154      154              
  Lines                              4616     4682      +66     
  Branches                            412      422      +10     
================================================================
+ Hits                               3927     3983      +56     
- Misses                              499      507       +8     
- Partials                            190      192       +2

Impacted Files	Coverage Δ
...opensearch/knn/index/mapper/LuceneFieldMapper.java	`71.15% <50.00%> (ø)`
...nsearch/knn/index/mapper/KNNVectorFieldMapper.java	`82.93% <79.16%> (-1.17%)`	⬇️
...rg/opensearch/knn/index/query/KNNQueryFactory.java	`88.13% <85.00%> (-2.35%)`	⬇️
...opensearch/knn/index/KNNVectorDVLeafFieldData.java	`75.00% <100.00%> (+2.27%)`	⬆️
.../opensearch/knn/index/KNNVectorIndexFieldData.java	`100.00% <100.00%> (ø)`
...opensearch/knn/index/KNNVectorScriptDocValues.java	`84.61% <100.00%> (-5.87%)`	⬇️
.../java/org/opensearch/knn/index/VectorDataType.java	`100.00% <100.00%> (ø)`
...rg/opensearch/knn/index/codec/KNNCodecService.java	`100.00% <100.00%> (ø)`
...rch/knn/index/mapper/KNNVectorFieldMapperUtil.java	`90.47% <100.00%> (+3.63%)`	⬆️
...rg/opensearch/knn/index/query/KNNQueryBuilder.java	`84.10% <100.00%> (+1.00%)`	⬆️
... and 3 more

... and 1 file with indirect coverage changes

martin-gaievski

Can we also add uni tests in scripting for BYTE type. Seems that in this PR we only add FLOAT to existing test cases.

src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java

src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java

naveentatikonda · 2023-07-07T21:23:45Z

Can we also add uni tests in scripting for BYTE type. Seems that in this PR we only add FLOAT to existing test cases.

Will raise a separate PR to improve test coverage. Will add these unit tests in that PR

naveentatikonda · 2023-07-07T21:46:24Z

Build is failing due to this change in core opensearch-project/OpenSearch#8312

navneet1v · 2023-07-10T18:03:39Z

@naveentatikonda can you make build and test github action for this change.

navneet1v · 2023-07-10T18:06:11Z

@naveentatikonda Seems like there are some changes in core.

/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecService.java:21: error: constructor CodecService in class CodecService cannot be applied to given types;

        super(codecServiceConfig.getMapperService(), codecServiceConfig.getLogger());
        ^
  required: MapperService,IndexSettings,Logger
  found:    MapperService,Logger
  reason: actual and formal argument lists differ in length
/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecService.java:21: error: incompatible types: Logger cannot be converted to IndexSettings
        super(codecServiceConfig.getMapperService(), codecServiceConfig.getLogger());
                                                                                 ^
> Task :compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
2 errors

You might need to fix it.

naveentatikonda · 2023-07-10T18:12:59Z

@naveentatikonda can you make build and test github action for this change.

Yes, the build was succeeding for this PR before opensearch core made this change.

naveentatikonda · 2023-07-10T18:16:28Z

@naveentatikonda Seems like there are some changes in core.

/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecService.java:21: error: constructor CodecService in class CodecService cannot be applied to given types;

        super(codecServiceConfig.getMapperService(), codecServiceConfig.getLogger());
        ^
  required: MapperService,IndexSettings,Logger
  found:    MapperService,Logger
  reason: actual and formal argument lists differ in length
/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecService.java:21: error: incompatible types: Logger cannot be converted to IndexSettings
        super(codecServiceConfig.getMapperService(), codecServiceConfig.getLogger());
                                                                                 ^
> Task :compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
2 errors

You might need to fix it.

Sure, let me check and raise a separate PR to fix this. I thought Junqiu was working on this to fix the build on Friday.

navneet1v · 2023-07-10T18:37:59Z

@naveentatikonda he was working on fixing the 2.x branch.

src/main/java/org/opensearch/knn/index/VectorDataType.java

jmazanec15 · 2023-07-11T17:43:55Z

src/main/java/org/opensearch/knn/index/VectorDataType.java

+
+        @Override
+        public float[] getValue(BytesRef binaryValue) {
+            float[] vector = new float[binaryValue.length];


Why use a float[] for byte type? Shouldnt it be an int[]?

We are using this method for scripting functions to retrieve the vector from docValues to calculate the score. SO, it doesn't make any difference if we return it as int[] or float[]. If we return it as int[] then again we need to do some method overloading and add methods for the spacetype functions in ScoringUtils to accept int[].

jmazanec15 · 2023-07-11T17:45:11Z

src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java

 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Supplier;

 import static org.opensearch.knn.common.KNNConstants.DEFAULT_VECTOR_DATA_TYPE_FIELD;
 import static org.opensearch.knn.common.KNNConstants.KNN_METHOD;
 import static org.opensearch.knn.common.KNNConstants.VECTOR_DATA_TYPE_FIELD;
+import static org.opensearch.knn.index.KNNSettings.KNN_INDEX;


Do changes in this file related to adding docvalues support for lucene byte sized vectors?

Yes, here we are trying to add some extra validation checks wrt knnIndex setting and knn engine. Also, to ingest doc values for byte vectors using script scoring

jmazanec15 · 2023-07-11T17:48:01Z

src/test/java/org/opensearch/knn/index/VectorDataTypeIT.java

+    }
+
+    @SneakyThrows
+    private void ingestL2ByteTestData() {


Why does this need to be L2?

I actually thought of creating different methods for test data for other spacetypes later. So, I created it with L2 as I'm using it for spacetype L2.

…earch-project#962) Signed-off-by: Martin Gaievski <[email protected]>

Signed-off-by: Naveen Tatikonda <[email protected]>

…t#953)

…t#953) Signed-off-by: Naveen Tatikonda <[email protected]>

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]>

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> (cherry picked from commit bf04854)

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> (cherry picked from commit bf04854) Signed-off-by: Naveen Tatikonda <[email protected]>

naveentatikonda added Features Introduces a new unit of functionality that satisfies a requirement v2.9.0 labels Jul 7, 2023

naveentatikonda requested a review from heemin32 as a code owner July 7, 2023 00:22

naveentatikonda self-assigned this Jul 7, 2023

naveentatikonda requested review from navneet1v, VijayanB, vamshin, jmazanec15, junqiu-lei and martin-gaievski as code owners July 7, 2023 00:22

naveentatikonda force-pushed the add_doc_values_support_byte_vector branch from f852135 to 1fcba06 Compare July 7, 2023 00:22

martin-gaievski reviewed Jul 7, 2023

View reviewed changes

naveentatikonda force-pushed the add_doc_values_support_byte_vector branch 2 times, most recently from 081d157 to 4c24661 Compare July 7, 2023 21:42

naveentatikonda requested a review from martin-gaievski July 7, 2023 21:46

martin-gaievski approved these changes Jul 8, 2023

View reviewed changes

junqiu-lei approved these changes Jul 11, 2023

View reviewed changes

jmazanec15 reviewed Jul 11, 2023

View reviewed changes

naveentatikonda requested a review from jmazanec15 July 11, 2023 18:15

naveentatikonda force-pushed the add_doc_values_support_byte_vector branch 2 times, most recently from 185a7a0 to 598ab43 Compare July 11, 2023 21:40

Add index settings to super constructor call for codec service (opens…

3390c98

…earch-project#962) Signed-off-by: Martin Gaievski <[email protected]>

naveentatikonda added 5 commits July 11, 2023 16:51

Add DocValues Support for Lucene Byte Sized Vector

70c2e03

Signed-off-by: Naveen Tatikonda <[email protected]>

Fix Existing Tests

45ce916

Signed-off-by: Naveen Tatikonda <[email protected]>

Add Tests

51699fe

Signed-off-by: Naveen Tatikonda <[email protected]>

Add CHANGELOG

1b77b14

Signed-off-by: Naveen Tatikonda <[email protected]>

Address Review Comments

0eb0adf

Signed-off-by: Naveen Tatikonda <[email protected]>

naveentatikonda force-pushed the add_doc_values_support_byte_vector branch from 598ab43 to 0eb0adf Compare July 11, 2023 21:54

jmazanec15 approved these changes Jul 11, 2023

View reviewed changes

naveentatikonda merged commit 5b7c44e into opensearch-project:feature/lucene_byte_vector Jul 11, 2023

naveentatikonda added a commit to naveentatikonda/k-NN that referenced this pull request Jul 12, 2023

Add DocValues Support for Lucene Byte Sized Vector (opensearch-projec…

a17fa42

…t#953)

naveentatikonda added a commit to naveentatikonda/k-NN that referenced this pull request Jul 12, 2023

Add DocValues Support for Lucene Byte Sized Vector (opensearch-projec…

c7565ac

…t#953) Signed-off-by: Naveen Tatikonda <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DocValues Support for Lucene Byte Sized Vector #953

Add DocValues Support for Lucene Byte Sized Vector #953

naveentatikonda commented Jul 7, 2023

codecov bot commented Jul 7, 2023 •

edited

Loading

martin-gaievski left a comment

naveentatikonda commented Jul 7, 2023

naveentatikonda commented Jul 7, 2023

navneet1v commented Jul 10, 2023

navneet1v commented Jul 10, 2023

naveentatikonda commented Jul 10, 2023

naveentatikonda commented Jul 10, 2023

navneet1v commented Jul 10, 2023

jmazanec15 Jul 11, 2023

naveentatikonda Jul 11, 2023

jmazanec15 Jul 11, 2023

naveentatikonda Jul 11, 2023

jmazanec15 Jul 11, 2023

naveentatikonda Jul 11, 2023

Add DocValues Support for Lucene Byte Sized Vector #953

Add DocValues Support for Lucene Byte Sized Vector #953

Conversation

naveentatikonda commented Jul 7, 2023

Description

Issues Resolved

Check List

codecov bot commented Jul 7, 2023 • edited Loading

Codecov Report

martin-gaievski left a comment

Choose a reason for hiding this comment

naveentatikonda commented Jul 7, 2023

naveentatikonda commented Jul 7, 2023

navneet1v commented Jul 10, 2023

navneet1v commented Jul 10, 2023

naveentatikonda commented Jul 10, 2023

naveentatikonda commented Jul 10, 2023

navneet1v commented Jul 10, 2023

jmazanec15 Jul 11, 2023

Choose a reason for hiding this comment

naveentatikonda Jul 11, 2023

Choose a reason for hiding this comment

jmazanec15 Jul 11, 2023

Choose a reason for hiding this comment

naveentatikonda Jul 11, 2023

Choose a reason for hiding this comment

jmazanec15 Jul 11, 2023

Choose a reason for hiding this comment

naveentatikonda Jul 11, 2023

Choose a reason for hiding this comment

codecov bot commented Jul 7, 2023 •

edited

Loading