Stiching Faiss104ScalarQuantizedKnnVectorsWriter to MemOptimizedScalarQuantizedIndexBuildStrategy by 0ctopus13prime · Pull Request #3185 · opensearch-project/k-NN

0ctopus13prime · 2026-03-18T22:25:06Z

Description

This PR introduces the Faiss BBQ (Better Binary Quantization) indexing pipeline and refactors the native engine writer hierarchy to share common flush/merge logic.

New BBQ codec classes:

Faiss104ScalarQuantizedKnnVectorsFormat — dedicated format for BBQ fields, using Lucene's Lucene104ScalarQuantizedVectorsFormat with 1-bit quantization for flat vector storage and native Faiss for HNSW graph construction.
Faiss104ScalarQuantizedKnnVectorsWriter — single-field writer that flushes flat vectors via Lucene's BBQ writer, then delegates HNSW graph building to MemOptimizedScalarQuantizedIndexBuildStrategy through the native index writer pipeline.
Faiss104ScalarQuantizedKnnVectorsReader — reader that always forces memory-optimized search, bypassing the index-level setting gate.
MemOptimizedScalarQuantizedIndexBuildStrategy — build strategy that reads quantized vectors and correction factors from Lucene's on-disk format, transfers them to off-heap memory via JNI in batches, builds the HNSW graph, and writes only the graph structure to disk (skipping flat storage duplication).

Refactoring:

Extracted AbstractNativeEnginesKnnVectorsWriter from NativeEngines990KnnVectorsWriter, pulling common doFlush/doMergeOneField logic into a shared base class. Both NativeEngines990KnnVectorsWriter and Faiss104ScalarQuantizedKnnVectorsWriter now extend this base class, with each subclass providing its own quantization strategy and build parameters.
NativeIndexWriter.getWriter now accepts buildStrategyParams to pass format-specific parameters (e.g., the FlatVectorsReader supplier for BBQ) through to the build strategy.

Bug fixes found during development:

Fixed NativeIndexWriter.getWriter (5-arg) silently dropping buildStrategyParams by always passing Collections.emptyMap() to createWriter.
Fixed Faiss104ScalarQuantizedKnnVectorsWriter.mergeOneField NPE — this.fieldInfo was null during merge because addField is only called during flush.
Fixed MemOptimizedScalarQuantizedIndexBuildStrategy passing data_type=float to writeIndex, causing Faiss to fail with "don't know how to serialize this type of index" since the BBQ index is a binary index type.
Fixed AbstractNativeEnginesKnnVectorsWriter.doFlush creating unnecessary KNNVectorValues suppliers for empty fields before the totalLiveDocs == 0 early return.
Fixed Faiss104ScalarQuantizedKnnVectorsWriter not closing flatVectorsWriter before the native build, causing file handle conflicts when the build strategy tries to open a reader on the just-written flat files.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

[O] New functionality includes testing.
[O] New functionality has been documented.
[O] API changes companion pull request created.
[O] Commits are signed 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.

0ctopus13prime · 2026-03-18T22:33:11Z

src/main/java/org/opensearch/knn/index/codec/nativeindex/model/BuildIndexParams.java

    int totalLiveDocs;
    SegmentWriteState segmentWriteState;
    boolean isFlush;
+    Map<String, Object> buildStrategyParams;


This is parameters to be passed down to build strategy from VectorsWriter.

0ctopus13prime · 2026-03-18T22:34:19Z

src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategy.java

@@ -196,10 +196,7 @@ private void writeToRepository(RepositoryContext repositoryContext, BuildIndexPa
            );


No major changes in this file.

0ctopus13prime · 2026-03-18T22:35:21Z

src/main/java/org/opensearch/knn/index/codec/nativeindex/DefaultIndexBuildStrategy.java

@@ -73,7 +73,7 @@ public void buildAndWriteIndex(final BuildIndexParams indexInfo) throws IOExcept
            }


No major changes

0ctopus13prime · 2026-03-18T22:35:30Z

...in/java/org/opensearch/knn/index/codec/nativeindex/MemOptimizedNativeIndexBuildStrategy.java

@@ -47,15 +47,15 @@ public static MemOptimizedNativeIndexBuildStrategy getInstance() {
     * enabled, the vectors are quantized before being transferred off-heap. Once all vectors are transferred, they are


No major changes

0ctopus13prime · 2026-03-18T22:36:11Z

src/main/java/org/opensearch/knn/index/codec/AbstractNativeEnginesKnnVectorsWriter.java

+import static org.opensearch.knn.index.vectorvalues.KNNVectorValuesFactory.getVectorValuesSupplier;
+
+@Log4j2
+public abstract class AbstractNativeEnginesKnnVectorsWriter extends KnnVectorsWriter {


Will add detailed java doc.

…rQuantizedIndexBuildStrategy Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

Vikasht34

LGTM ... I am not getting place to put comments ... All are perfect at first look , let me take a deeper look in night

kotwanikunal · 2026-03-18T23:53:10Z

...rg/opensearch/knn/index/codec/nativeindex/MemOptimizedScalarQuantizedIndexBuildStrategy.java

-            throw e;
        }

+        final Object flatVectorsReaderObj = ((IOSupplier<?>) flatVectorsReaderSupplierObj).get();


Any reason to not move all of this down to the format classes? We are breaking separation of concerns now with this flow. Passing down general objects and doing typecasting in a hot code path - I feel this is a concern of readers and writers.

Sorry not following, I believe since this is the build strategy, so it has faiss bbq index build responsibility.
In here, FlacVectorsReader (i.e. .veb reader) is required to build Faiss BBQ index.
So the logic is to load quantized vectors into C++ off-heap, so that C++ HNSW can perform distance calculation on them.
I can't think of another way to put this in format layer or writer layer as this is a part of building process.

@0ctopus13prime earlier there we had SeGMENT STATE all part of This change , but now we wrapped into Supplier and passing the Supplier with All Segment State make no difference with earlier code to this.

But what it tries to load is for the new segment that's being baked right now.
So SegmentWriteState does not have any references to it, because .veb is new one being built.

I understand the strategy needs quantized vectors to build the Faiss BBQ index that's not in question. The concern is about where the Lucene reader opening and reflection logic lives, not whether it happens.

The issue is consistency with the existing strategy contract. Every other strategy in this codebase receives its input data through typed fields on BuildIndexParams:

DefaultIndexBuildStrategy → indexInfo.getKnnVectorValuesSupplier().get()
MemOptimizedNativeIndexBuildStrategy → indexInfo.getKnnVectorValuesSupplier().get()

None of them open Lucene readers, do reflection, or manage reader lifecycles. They receive prepared data and build an index.

Can we pass the result as a typed Supplier on BuildIndexParams — the same pattern as knnVectorValuesSupplier. The strategy then just calls indexInfo.getQuantizedVectorValuesSupplier().get() and does the native build. This keeps all Lucene format I/O in the writer layer and all native index construction in the strategy layer.

I do have a sample implementation here: kotwanikunal@aef426b#diff-f8c4eb218bf94ceee3be08f3489f3736f701652b0a4273d8b3cb103548eb2c13

I understand your concern, but that's slow since it first copies vectors to std::vector first, then it copies what's in std::vector to Faiss index builder.
Even removed this double copy, from the benchmark this bbq indexing is showing -5% indexing throughput decrease compared to BQ. And if we move to the existing way, then the gap will be widen.
Furthermore, it's designed to pass quantized vectors, not designed for passing quantized vectors + correction factors I believe.

I think it's worth to sacrificing the vectors loading abstraction for better performance.
Please share your thoughts on this.

The performance concern about the native transfer path is valid and completely separate from the ask is what I think. The suggestion is not that you use the existing VectorTransfer/KNNVectorValues for the data transfer — the custom passBBQVectorsWithCorrectionFactors JNI path with the packed layout can be in the strategy.

The ask is smaller: move the lines that open the FlatVectorsReader from the strategy into the writer. The strategy still receives QuantizedByteVectorValues and does the exact same native transfer it does today. No change to the JNI path, no double copy, no performance impact.

Out of the strategy:

// These lines move to the writer Object supplier = indexInfo.getBuildStrategyParams().get(KEY); IOSupplier<?> ioSupplier = (IOSupplier<?>) supplier; FlatVectorsReader reader = (FlatVectorsReader) ioSupplier.get(); FloatVectorValues fvv = reader.getFloatVectorValues(field); Field f = fvv.getClass().getDeclaredField("quantizedVectorValues"); f.setAccessible(true); QuantizedByteVectorValues qv = (QuantizedByteVectorValues) f.get(fvv);

Into the strategy (unchanged):

// Strategy still does all the native work exactly as before QuantizedByteVectorValues qv = indexInfo.getQuantizedVectorValuesSupplier().get(); transferQuantizedVectors(indexMemoryAddress, qv, quantizedVecBytes, knnEngine); buildGraph(indexMemoryAddress, knnVectorValues, knnEngine);

The transferQuantizedVectors method, the batch packing, the passBBQVectorsWithCorrectionFactors call, the addDocsToBBQIndex call — all of that stays exactly where it is.

Ah! got it, somehow i misread your comments.
Good point, will update in the next rev.

...ava/org/opensearch/knn/index/codec/KNN1040Codec/Faiss104ScalarQuantizedKnnVectorsWriter.java

Vikasht34 · 2026-03-19T00:16:06Z

src/main/java/org/opensearch/knn/index/codec/KNN990Codec/NativeEngines990KnnVectorsWriter.java

 */
 @Log4j2
-public class NativeEngines990KnnVectorsWriter extends KnnVectorsWriter {
+public class NativeEngines990KnnVectorsWriter extends AbstractNativeEnginesKnnVectorsWriter {


Can we not touch this class as part of this , It will may increase blast Radius? we can have next attemept to refactor this.

Will need votes for this, I got feedbacks that avoid copy-and-paste code as possible
@kotwanikunal @navneet1v
Can you share your thoughts on this?

Intent is right ... If we are okay to test all the code path we are touching as part of this , should be fine

Vikasht34 · 2026-03-19T00:16:35Z

src/main/java/org/opensearch/knn/index/codec/nativeindex/model/BuildIndexParams.java

    int totalLiveDocs;
    SegmentWriteState segmentWriteState;
    boolean isFlush;
+    Map<String, Object> buildStrategyParams;


What Object Represent here ?

It's a general reference (i.e. similar to void* in C++), it's build strategy's responsibility to cast the Object with key.

Yeah my question is What are differnt types we are expoecting here , Why it can not be concrete

so far the only type that's being used is IOSupplier.

codecov · 2026-03-19T03:01:09Z

Codecov Report

❌ Patch coverage is 85.61644% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.44%. Comparing base (8ec315b) to head (abf26a4).
⚠️ Report is 8 commits behind head on feature/faiss-bbq.

Files with missing lines	Patch %	Lines
...MemOptimizedScalarQuantizedIndexBuildStrategy.java	64.28%	3 Missing and 2 partials ⚠️
...c/nativeindex/remote/RemoteIndexBuildStrategy.java	0.00%	5 Missing ⚠️
...Codec/Faiss104ScalarQuantizedKnnVectorsWriter.java	88.00%	0 Missing and 3 partials ⚠️
.../KNN990Codec/NativeEngines990KnnVectorsWriter.java	78.57%	3 Missing ⚠️
...nn/index/vectorvalues/KNNVectorValuesIterator.java	75.00%	0 Missing and 2 partials ⚠️
...x/codec/nativeindex/DefaultIndexBuildStrategy.java	50.00%	1 Missing ⚠️
...iveindex/MemOptimizedNativeIndexBuildStrategy.java	50.00%	1 Missing ⚠️
...knn/index/codec/nativeindex/NativeIndexWriter.java	85.71%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                   @@
##             feature/faiss-bbq    #3185      +/-   ##
=======================================================
+ Coverage                82.41%   82.44%   +0.03%     
- Complexity                3997     4036      +39     
=======================================================
  Files                      431      437       +6     
  Lines                    14939    15042     +103     
  Branches                  1891     1904      +13     
=======================================================
+ Hits                     12312    12402      +90     
- Misses                    1861     1866       +5     
- Partials                   766      774       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

0ctopus13prime self-assigned this Mar 18, 2026

0ctopus13prime added skip-changelog v3.6.0 labels Mar 18, 2026

0ctopus13prime requested review from VijayanB, Vikasht34, heemin32, jmazanec15, junqiu-lei, luyuncheng, martin-gaievski, naveentatikonda, navneet1v, ryanbogan, shatejas and vamshin as code owners March 18, 2026 22:25

0ctopus13prime commented Mar 18, 2026

View reviewed changes

0ctopus13prime force-pushed the faiss-bbq-indexing-pipeline branch from 75b695a to 5e3d623 Compare March 18, 2026 22:47

Stiching Faiss104ScalarQuantizedKnnVectorsWriter to MemOptimizedScala…

abf26a4

…rQuantizedIndexBuildStrategy Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

0ctopus13prime force-pushed the faiss-bbq-indexing-pipeline branch from 5e3d623 to abf26a4 Compare March 18, 2026 22:57

Vikasht34 approved these changes Mar 18, 2026

View reviewed changes

kotwanikunal reviewed Mar 18, 2026

View reviewed changes

Vikasht34 reviewed Mar 19, 2026

View reviewed changes

...ava/org/opensearch/knn/index/codec/KNN1040Codec/Faiss104ScalarQuantizedKnnVectorsWriter.java Show resolved Hide resolved

Vikasht34 reviewed Mar 19, 2026

View reviewed changes

		@@ -196,10 +196,7 @@ private void writeToRepository(RepositoryContext repositoryContext, BuildIndexPa
		);

		@@ -73,7 +73,7 @@ public void buildAndWriteIndex(final BuildIndexParams indexInfo) throws IOExcept
		}

		@@ -47,15 +47,15 @@ public static MemOptimizedNativeIndexBuildStrategy getInstance() {
		* enabled, the vectors are quantized before being transferred off-heap. Once all vectors are transferred, they are

Conversation

0ctopus13prime commented Mar 18, 2026

Description

New BBQ codec classes:

Refactoring:

Bug fixes found during development:

Related Issues

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ctopus13prime Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vikasht34 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ctopus13prime Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kotwanikunal Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ctopus13prime Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 19, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

0ctopus13prime Mar 18, 2026 •

edited

Loading

Vikasht34 left a comment •

edited

Loading

0ctopus13prime Mar 19, 2026 •

edited

Loading

kotwanikunal Mar 19, 2026 •

edited

Loading

0ctopus13prime Mar 19, 2026 •

edited

Loading