fix(collections): nest the quantizer inside a dynamic index - #613
Open
dudanogueira wants to merge 2 commits into
Open
fix(collections): nest the quantizer inside a dynamic index#613dudanogueira wants to merge 2 commits into
dudanogueira wants to merge 2 commits into
Conversation
Weaviate parses quantizer settings out of the config map by exact key,
and returns them under the same names. The client used snake_case for
half of them, so rescoreLimit, trainingLimit and bitCompression were
dropped on write and came back null on read -- on every index type. A
user who set a rescore limit never set one.
rescore_limit -> rescoreLimit (BQ, SQ, RQ)
training_limit -> trainingLimit (PQ, SQ)
bit_compression -> bitCompression (PQ)
PQ's encoder needed a shape change rather than a rename: the server
nests it as encoder: {type, distribution} while the client had two flat
components. PQ.encoderType() and PQ.encoderDistribution() are kept as
derived accessors and the builder is unchanged, so only the canonical
constructor differs.
No alternate names: the snake_case spellings were never valid on the
wire, so no stored config uses them.
Closes #611
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU
A dynamic index is {distance, threshold, hnsw: {...}, flat: {...}} and
each sub-index carries its own quantizer, but the client put the
quantizer beside them and looked for it there on the way back. So a
dynamic index created through this client was silently unquantized, and
one quantized by other means read as quantization() == null.
VectorConfig has a single quantization slot, so a dynamic index is read
from hnsw when it has a quantizer and from flat otherwise, and always
written to hnsw -- the only sub-index accepting every quantizer type,
flat being limited to bq. Giving hnsw and flat different quantizers
stays inexpressible; that needs per-index slots on Hnsw and Flat.
The same nesting applies to the skipDefaultQuantization flag that the
update request has to preserve, so both call the shared QuantizerJson.
Also on Dynamic: "distance" is read and written (the server keeps one at
the dynamic level), and the response parser no longer assumes
"threshold" is present -- a dropped index has neither.
Closes #606
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A dynamic index is
{distance, threshold, hnsw: {...}, flat: {...}}and each sub-index carries its own quantizer (entities/vectorindex/dynamic/config.go:HnswUC hnsw.UserConfig,FlatUC flat.UserConfig). The client put the quantizer besidehnsw/flatand scanned for it there on the way back.Both directions were broken, not just the read side the issue reports:
VectorConfig.readscanned only the top level ofvectorIndexConfig, sohnsw.rqwas never found andquantization()came back null. This is what v6: dynamic index quantization is never parsed into VectorConfig #606 reports, verified against a live 1.38.0 collection.VectorConfig.writeappended the quantizer as a sibling ofhnsw/flat, where the server's dynamic parser never looks. A dynamic + quantized collection created through this client was silently unquantized.The write half makes the read half partly self-consistent: a config created by this client really did have no quantizer to read back.
Approach
VectorConfighas a singlequantization()slot, so:hnswwhen it has a quantizer,flatotherwise;hnsw— also the only sub-index that accepts every quantizer type,flatbeing limited tobq.Both sides go through
QuantizerJson.host(...), whichUpdateCollectionRequestalso uses: theskipDefaultQuantizationflag it has to preserve on update was being written at the same wrong level.Documented limitation: giving
hnswandflatdifferent quantizers is not expressible. That needs aquantizationcomponent onHnswandFlatwithVectorConfig.quantization()becoming a derived view — a breaking change across 33 vectorizer records, worth doing only if someone actually needs it.Also on
Dynamic, both called out in #606:distanceis now read and written; the server keeps one at the dynamic level and it was dropped in both directions.thresholdis present — a dropped index has neither, andgetAsLong()on a missing key would NPE.Key areas for review
QuantizerJson.host— thecreateflag distinguishes "make room for it" (write) from "find it or find nothing" (read). The read path returning an empty object rather than null is what keeps the scan loop unchanged.UpdateCollectionRequest— worth confirming the flag now lands where the server reads it for dynamic indexes.Testing
DynamicQuantizationTest— 6 read cases the round-trip rows cannot cover, since this client always writes tohnsw: quantizer on hnsw, on flat only, on both (hnsw wins), absent, disabled, anddistanceat the dynamic level.JSONTest— a dynamic row carryingrqanddistance, asserted in both directions.CollectionsITest.test_dynamicIndexQuantizationRoundTrip— creates a dynamic index withrqagainst a real server and reads it back. NeedsASYNC_INDEXING=true, which the shared container is not (async indexing makes freshly inserted vectors searchable only eventually, and the other suites rely on immediate searchability), so it gets its own container via a newWeaviate.Builder.enableAsyncIndexing.Verified the integration test fails without the fix:
[quantizer nested under hnsw] Expecting actual not to be null.Locally green: 390 unit tests, and
CollectionsITestagainst a 1.39.0 container (17 run, 0 failures).Conflict note
enableAsyncIndexingis also added by #598, which is still open. Whichever merges second will want that hunk dropped — same method, same body.Breaking changes
Dynamic's canonical constructor gains adistancecomponent. The builder and accessors are additive; only positional construction is affected.Behaviourally: a dynamic index created after this change actually applies its quantizer, where before it was accepted and ignored. Existing collections are unaffected — they have no quantizer stored to migrate.
Closes #606
🤖 Generated with Claude Code
https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU