GG-50943 Support vector storage, segment parameters and INT8 on the thin client - #83
Conversation
…hin client The client knew feature bits 33 and 38 and stopped there, so three server-side vector index parameters were unreachable from Python: quantization (bit 39), the segment target and query-thread count (bit 40), and INT8 storage (bit 41). Bit 41 is not a field. INT8 is a value on the enum bit 39 already carried, so a peer with bit 39 reads the ordinal and resolves a value it does not know to the engine default -- the caller asks for byte codes and silently gets a full-precision index. The server gave INT8 its own bit for exactly that, and this client refuses an INT8 request the peer cannot honour rather than sending it. BINARY still reaches a peer without bit 41, which is the case a naive guard breaks; there is a test for each direction. The refusals follow the shape already here for the HNSW parameters: a parameter the wire cannot carry is an error, never dropped, because a dropped parameter is invisible until somebody measures recall. Also fixes a memoisation hazard this change would otherwise have introduced. query_entities_struct is lru_cached on its argument tuple, so once the parameters had defaults, (True, True) and (True, True, False, False, False) became two cache entries for one feature set -- and the cache config struct and the query entities property would each get their own layout instead of sharing one. Both entry points now normalise to a single five-argument key. The pre-existing identity test caught this, which is what it was written for. 26 new offline wire tests: byte-level layout deltas per bit, INT8 adding no bytes, the new fields never reaching a non-VECTOR index, round trips, and every refusal with its negative control. Full offline suite 152 passed, flake8 clean.
Audited the six the server declares in ClientBitmaskFeature -- 33 similarity, 35 extended vector query, 38 HNSW parameters, 39 quantization, 40 segment parameters, 41 INT8 -- against this client. All six are now declared, each has a protocol-context accessor, and each is honoured: 33 and 38 through the index layout, 35 through ef_search/with_scores/no_content on the query API, and 39, 40 and 41 through this change. Three tests make that a property rather than a coincidence. The server's list is a literal here because this client cannot read the server's enum, so the list IS the contract and a seventh bit has to be reflected deliberately. One test pins the declared set against it, one requires an accessor for every declared bit, and one requires each to be advertised by all_supported(). The accessor test is the one that matters: a declared bit is an advertised bit, so the server may send a payload for it. Declaring a bit with no way to ask whether it was negotiated is a promise the client cannot keep, and it fails on the wire instead of in the suite. Offline suite 160 passed, flake8 clean.
Tested end to end against a real nodeThe PR body said end-to-end needed a live cluster and that I had not done it. Now done, against a 1. The bits are actually negotiated, not just declared. 2. The server accepts all three new parameters from this client. A cache created from Python with 3. They survive a real round trip. Reading the configuration back off the server:
4. The INT8 index is searchable, and the bit-35 options work on it. 20 documents inserted, then:
That last group matters beyond the flags: the search runs over an index the engine actually built Two things worth recordingThe first attempt failed with I have not added this as a committed test. It needs a licensed node with the EE vector-query |
The server took bit 40 for CONTINUOUS_QUERY_COMPACT_VALUE in master commit fe1325fa8bf, after this client claimed 40 and 41 for the vector parameters. The server keeps 40, so this client follows: QUERY_INDEX_VECTOR_SEGMENT_PARAMS 40 -> 41 QUERY_INDEX_VECTOR_INT8_STORAGE 41 -> 42 The SERVER_VECTOR_BITS guard is what makes this safe to change: it holds the server's bit numbers as a literal, so the assertion moves with the enum and a client that drifts from the server fails the test. The docstring now says bit 40 belongs to a non-vector feature, so the gap between 39 and 41 is deliberate. Nobody should close it. 168 offline tests pass, 1 skipped. flake8 clean on both files.
Re-run end to end after the bit renumberThe earlier end-to-end run negotiated bits 40 and 41. Those numbers are gone: master commit Node built from the current heads — CE 1. The server negotiates the new numbers. This is the check that matters after a renumber: it 2. All three parameters still reach the server and come back. A 3. The INT8 index is searchable, and the bit-35 options work on it.
Every check passed. The offline suite is 168 passed, 1 skipped, and flake8 is clean on both This stays a manual procedure rather than a committed test, for the same reason as before: it needs |
ivanzlenko
left a comment
ivanzlenko
left a comment
There was a problem hiding this comment.
Verified the protocol work against the Java side (gridgain#4009 fetched locally), not just this description. Bits 39/41/42, the 0..3 quantization ordinals, the 1024/64 ceilings, and the wire order (quantization int, then maxSegments, queryThreads) all match ClientBitmaskFeature, VectorQuantization, QueryIndex and PlatformConfigurationUtils.writeQueryIndex on that branch. The gap at bit 40 is correct, and correctly not declared here. The Java PR title's "deleted-vector budget" is a system property, not a per-index wire field, so nothing is missing from the layout.
Ran it: 151 passed, 18 skipped, flake8 clean on all four changed files. Also hand-checked a mixed [SORTED, VECTOR, SORTED] array round-trips without stream desync.
Five comments below. Only the merge-ordering one should gate the merge; the other two code points are one-line fixes.
Also confirmed as not problems, since both look like they could be:
- The
query_entities_structnormalisation is load-bearing, not defensive padding.cache_config.py:436still calls it with two arguments, and without the wrapperf(True, False) is f(True, False, False, False, False)is False. - Excluding
has_int8from the layout gate in_prop_query_entities_cachedleaves no hole. Checked all three: the two int8 variants are distinct classes with identical bytes, and int8-without-quantization still refuses INT8 via thequantization_supportedbranch.
| QUERY_VECTOR_EXTENDED = 1 << 35 | ||
| QUERY_INDEX_VECTOR_HNSW_PARAMS = 1 << 38 | ||
| QUERY_INDEX_VECTOR_QUANTIZATION = 1 << 39 | ||
| QUERY_INDEX_VECTOR_SEGMENT_PARAMS = 1 << 41 |
There was a problem hiding this comment.
Merge ordering. origin/master ClientBitmaskFeature still ends at CONTINUOUS_QUERY_COMPACT_VALUE(40); — 41 and 42 exist only on the unmerged gridgain#4009 (state: OPEN).
Until that lands this client advertises two bits no released server has, and #4009 has already renumbered once when master took 40. SERVER_VECTOR_BITS is a literal, so a second renumber would not be caught here. Land this after #4009.
There was a problem hiding this comment.
Still open — nothing to fix in this repo. Re-checked just now: gridgain#4009 is state=OPEN, mergedAt=null, and origin/master ClientBitmaskFeature still ends at CONTINUOUS_QUERY_COMPACT_VALUE(40);. Bits 41 and 42 exist only on that branch.
Leaving this thread open as the merge gate. Nothing else blocks.
From ivanzlenko's review of this PR. - str.capitalize() lowercases the tail, so the refusal read "Per-index hnsw build parameters" where master read "HNSW". The callers now pass sentence-cased text and the format string leaves it alone. The cluster-side message lower-cases the first word again, because there it reads mid-sentence. Neither pin caught this: both match only "VECTOR indexes only". - The wire-order comment cited ClientUtils, which is the Java thin client. The writer this layout has to match is the server's PlatformConfigurationUtils.writeQueryIndex. - An ordinal outside VectorQuantization now fails here instead of reaching the server. VectorQuantization is an Int subclass, not an Enum, so it cannot be iterated; VECTOR_QUANTIZATIONS names the set. - test_async_path_matches_the_sync_path, following the precedent the HNSW ticket set. from_python_async and parse_async are hand-written twins of the sync path, so parity is worth pinning. 169 offline tests pass, 1 skipped. flake8 clean on the changed files.
ivanzlenko
left a comment
ivanzlenko
left a comment
There was a problem hiding this comment.
Re-reviewed 0f57b5c. Four of the five are fixed and verified — resolved those threads with the evidence on each.
- Casing:
.capitalize()gone, acronym survives both branches. The NotSupported message is byte-identical to master's wording. - Citation: now names
PlatformConfigurationUtils.writeQueryIndex/ClientCacheConfigurationSerializer. - Unknown ordinal: refused client-side;
0/1/2/3all still serialize, INT8-without-bit-42 unchanged. - Async: parity test passes over INT8 plus both segment fields, byte-equal to the sync writer.
Suite 152 passed, 18 skipped (up one), flake8 clean on all four changed files.
Bit 41/42 thread stays open — gridgain#4009 is still state=OPEN, mergedAt=null and master's ClientBitmaskFeature still ends at CONTINUOUS_QUERY_COMPACT_VALUE(40);. Nothing to fix here; it is the merge gate.
Two small things the fix commit introduced, below. Neither blocks.
…rder ivanzlenko's re-review found the unknown-ordinal guard was the only refusal in the PR without a test, and that its position was load-bearing and untested: it fired before the index-type check, so a SORTED index with quantization=7 reported "Unknown vector storage mode" when the value is irrelevant on a non-VECTOR index. The order is now: engine default returns, a non-VECTOR index is refused by its type, an unknown ordinal is refused before the feature bits so garbage input is never reported as a cluster capability gap. Three tests pin it. The refusal itself, mutation-checked (disabling the membership check fails it). A loop over all four known ordinals as the negative control, because a membership check is easy to write so it rejects something valid. And the precedence case. VECTOR_QUANTIZATIONS is _VECTOR_QUANTIZATIONS now, not an __all__ entry. It exists only because VectorQuantization is an Int subclass that cannot be iterated, and a public name would outlive that workaround. 175 offline tests pass, 1 skipped. flake8 clean.
ivanzlenko
left a comment
ivanzlenko
left a comment
There was a problem hiding this comment.
Re-reviewed 684a2c7. Both open code findings fixed; verified and resolved with the evidence on each thread.
Refusal precedence now behaves exactly as the new comments claim — checked all four orderings, not just the one the tests cover:
SORTED + unknown 7 -> ValueError: Vector storage is supported by VECTOR indexes only
VECTOR + unknown 7 -> ValueError: Unknown vector storage mode 7
VECTOR + 7, no bit 39 -> ValueError: Unknown vector storage mode 7 (not a capability gap)
VECTOR + INT8, no 42 -> NotSupportedByClusterError, unchanged
all four known modes -> serialize, 82 bytes each
_VECTOR_QUANTIZATIONS renamed, #: correctly downgraded to a plain comment with it, no stale reference to the old public name, correctly absent from __all__.
Suite 158 passed, 18 skipped (was 152). flake8 clean on all four changed files.
No new findings. One cosmetic thing not worth a thread: the unknown-ordinal message is the only refusal that omits where, so with several indexes it does not say which one carries the bad ordinal — where is already built two lines above it.
The bit 41/42 thread stays open, and it is the only thing left. Re-checked today: gridgain#4009 is still state=OPEN, mergedAt=null, and master's ClientBitmaskFeature still ends at CONTINUOUS_QUERY_COMPACT_VALUE(40);. The client code is done — this only needs #4009 to land first, and to be re-checked for a renumber if it does not land clean.
ivanzlenko's re-review closed both code findings and left one cosmetic: this was the only refusal that omitted the index name, so with several indexes it did not say which one carried the bad ordinal. The where clause is built two lines above; it is in the message now. 41 tests in the file pass, flake8 clean.
ivanzlenko
left a comment
ivanzlenko
left a comment
There was a problem hiding this comment.
Approving 8351a30.
The client work is complete and correct. Verified against the Java side rather than the description: bits 39/41/42, the 0..3 quantization ordinals, the 1024/64 ceilings and the wire order (quantization int, then maxSegments, queryThreads) all match ClientBitmaskFeature, VectorQuantization, QueryIndex and PlatformConfigurationUtils.writeQueryIndex on gridgain#4009. The gap at bit 40 is real and correctly left undeclared. The Java PR title's deleted-vector budget is a system property, not a per-index wire field, so nothing is missing from the layout.
All six code findings from the three earlier passes are fixed and re-verified individually. Latest commit lands the where addition, so every refusal now names the index it came from:
Unknown vector storage mode 7 (quantization=7 on index 'by_embedding'). Use a VectorQuantization value; ...
158 passed, 18 skipped, flake8 clean. Also hand-checked a mixed [SORTED, VECTOR, SORTED] array round-trips without stream desync, and that async parity holds byte-for-byte with the sync writer.
One merge condition, not a code objection — the open thread on bitmask_feature.py:29. gridgain#4009 is still state=OPEN, mergedAt=null and master's ClientBitmaskFeature still ends at CONTINUOUS_QUERY_COMPACT_VALUE(40);. Until it lands, this client advertises two bits no released server has, and #4009 has already renumbered once when master took 40. Land #4009 first, and re-check 41/42 here if it does not land clean — SERVER_VECTOR_BITS is a literal, so a second renumber would not be caught by this suite.
3 participants
GG-50943
Part of the GG-50943 fan-out. The Java side of this ticket added the per-index segment
parameters (bit 41) and, during self-review, gave INT8 storage a bit of its own (bit 42). The
Python client knew bits 33 and 38 and stopped there, so none of the vector index storage
parameters were reachable from Python at all.
What was missing
QUERY_INDEX_VECTOR_QUANTIZATIONCONTINUOUS_QUERY_COMPACT_VALUE— not oursQUERY_INDEX_VECTOR_SEGMENT_PARAMSQUERY_INDEX_VECTOR_INT8_STORAGEBit 39 belongs to GG-49429 rather than this ticket. It is here because bits 40 and 41 are
unusable without it: INT8 is a value on the field bit 39 adds, so a client that cannot send
quantization cannot send INT8 either. Splitting them would land a feature that nothing can reach.
Why bit 42 is not a field
INT8was appended to an enum that had already shipped behind bit 39. A peer with bit 39 readsthe ordinal and resolves a value it does not know to
ENGINE_DEFAULT— so an INT8 request againstsuch a peer returns success and builds a full-precision index, and the only symptom is a recall
number nobody is measuring. The Java thin client refuses it before it goes out
(
ClientUtils.cacheConfigurationthrowsClientFeatureNotSupportedByServerExceptionwhen the peerlacks bit 42, in
org.apache.ignite.internal.client.thin); this client refuses it too, rather thansending an ordinal it knows will be misread. Neither refusal is the server's: the server reader gates
on bit 39 alone, which is the accepted read-back limitation below.
BINARYstill reaches a peer without bit 42. That is the case a naive guard breaks — gate everyquantization on bit 42 and the refusal test passes while every existing BINARY caller stops
working — so there is a test in each direction.
Refusals, not silent drops
The existing HNSW code already refuses a parameter the wire cannot carry instead of dropping it,
on the grounds that a dropped parameter is invisible until somebody measures recall. The new
parameters follow it, with three distinct failures: a parameter on a non-
VECTORindex(
ValueError), a parameter the cluster never negotiated (NotSupportedByClusterError), and INT8against a cluster that has the field but not the value.
A memoisation hazard this change would have introduced
query_entities_structislru_cached on its argument tuple. Once the new parameters haddefaults,
(True, True)and(True, True, False, False, False)became two cache entries for onefeature set — so the cache config struct and the query entities property would each get their own
layout object instead of sharing one. Both entry points now normalise to a single five-argument
key before the cache.
The pre-existing
test_layout_is_shared_between_config_and_propertycaught this, which is exactlywhat an identity assertion is for. Worth noting for anyone extending the layout again: adding a
defaulted parameter to a memoised layout function is enough to desynchronise the two paths.
Verification
tests/test_query_index_vector_storage_params.py: per-bit byte-levellayout deltas (+4 for quantization, +8 for the segment pair, +12 for both), bit 42 adding no
bytes, the new fields never reaching a non-
VECTORindex for any of the three non-vector types,value round trips, unset values round-tripping as the engine default, and every refusal with its
negative control.
INT8 == 3,MAX_VECTOR_INDEX_SEGMENTS == 1024,MAX_VECTOR_QUERY_THREADS == 64), because the wire carriesthe ordinal and a drift there is a silently different storage mode.
flake8clean on every changed file.Not covered here: end-to-end behaviour against a live cluster, which needs the EE vector-query
module and a vector-search licence. This client is not covered by MTCGA either — its CI is the
external Jenkins/tox run.
Renumbered 2026-08-27
master commit
fe1325fa8bf(GG-37685) took protocol bit 40 forCONTINUOUS_QUERY_COMPACT_VALUEwhile this work held it. master shipped first and keeps 40, sothe vector bits moved up:
QUERY_INDEX_VECTOR_SEGMENT_PARAMSis 41 andQUERY_INDEX_VECTOR_INT8_STORAGEis 42. Bit 40 is not a vector bit, so the gap between 39and 41 is deliberate. master is merged in, and the branch is 0 commits behind.