Skip to content

GG-49932 Decode vector query responses in one pass (2/2) - #86

Draft
PakhomovAlexander wants to merge 5 commits into
gg-49932from
gg-49932-fastpath
Draft

GG-49932 Decode vector query responses in one pass (2/2)#86
PakhomovAlexander wants to merge 5 commits into
gg-49932from
gg-49932-fastpath

Conversation

@PakhomovAlexander

@PakhomovAlexander PakhomovAlexander commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Ticket: GG-49932 · Part 2 of 2, stacked on gg-49932 (part 1: the ctypes-class cache and the paging default)

What this changes

Vector query responses are decoded in one pass, straight off the response buffer, by a new
VectorResponse (the same specialisation pattern as the existing SQLResponse). Today a vector
row costs three walks: the page parse builds a ctypes class tree per element and copies every
wrapped binary value out as an opaque blob (WrappedDataObject), and the cursor then re-parses
each blob (unwrap_binary -> BinaryObject.parse -> to_python). This is the ticket's
"unwrap_binary fast path": keys, values and scores leave parse() as final Python values,
already shaped the way the cursor yields them, and the vector cursors stop calling
unwrap_binary entirely.

Scope and safety rails:

  • Vector operations only (OP_QUERY_VECTOR and its cursor pages). Scan/SQL/get keep the
    generic path (which part 1 already made cheaper).
  • Direct readers cover only what vector rows actually carry: long/int/double/string elements,
    raw float scores, and binary objects made of those plus float arrays, resolved once per
    (type_id, schema_id) per page. Anything else falls back to the generic machinery at the same
    stream position, one element at a time
    - unknown type codes, raw-data or non-user-type
    objects, unknown schemas, big-endian hosts. The realistic failure mode for an exotic payload is
    the old speed, not wrong data.
  • Both response layouts are covered - the flagged (protocol v2) row struct and the legacy v1
    key-value map - on the sync and asyncio clients.
  • The ctypes layout contract Query.perform relies on is kept byte-exact: the row section is
    described as one opaque byte-blob field, and the decoder tests assert the returned class spans
    the response exactly.

Behaviour notes for review

Two deliberate edge differences, both invisible at the cursor level:

  • asyncio legacy (key, value) rows were a list (an asyncio.gather artifact); they are
    now a tuple, matching the sync cursor.
  • At the low-level pygridgain.api.sql.vector() result, the legacy data value changes from a
    dict to a list of (key, value) tuples (wire order kept).

Measured (incremental over part 1, same bracketed invocation)

leg part 1 + this PR vs part 1 vs base #83
(key, value) k=10 · mean ms (QPS) 4.40 (227) 4.05 (247) -8% -27%
(key, value) k=100 16.93 (59) 13.92 (72) -18% -41%
keys-only k=10 · client CPU ms 0.39 0.30 -23% -38%
(key, value) k=100 · client CPU ms 10.0 7.03 -30% -58%

After this PR, ~38% of the remaining (key, value) CPU is the float-list boxing
(memoryview.cast('f').tolist()) - that ceiling belongs to a possible future opt-in numpy return,
not to this PR.

How it was verified

  • Unit (tests/test_vector_response_decode.py, server-free): frames assembled with the
    client's own writers - every row shape, paging pages, the legacy map, Nones, the generic
    fallback (a UUID element), empty results, asyncio parity, and the exact-span framing contract.
  • Offline suite: outcome-identical to base (174 passed = base + this branch's tests; same 6
    environment-only failures). flake8 3.8.4: no new finding.
  • e2e, live UE node: rows byte-identical to the base client across 640 row-lists over 16
    legs (all shapes, paging, asyncio; scores bit-exact as float32); private EE suite 53/53.
  • e2e, real 2-node cluster from the assembled enterprise release: same byte-identity gate;
    result posted as a PR comment.

Review guide

Everything lives in queries/response.py (VectorResponse + _NeedsAsync), with two thin
integration edits: api/sql.py points the two vector ops at it, and cursors.py drops the
per-row unwrap/shaping. The decoder makes the same two assumptions the existing generic
BinaryObject path already makes - field order comes from the complex-types registry, and the
schema footer is skipped via the header's length - so a future wire change touches the same
assumptions in two places instead of one; both are named in comments.

@PakhomovAlexander

Copy link
Copy Markdown
Contributor Author

e2e against a real 2-node GridGain cluster: both nodes started from the enterprise release assembled from CE/EE master + gg-50942 + gg-50943 (bin/ignite.sh, topology ver=2, servers=2), 20k x 1536-d dbpedia loaded through the base client, index consolidated to one segment before measurement. Rows from this branch are byte-identical to the base client across 520 row-lists over 16 legs (four row shapes x {k=10, k=10 paged at 3, k=100}, sync + asyncio with paging; scores compared bit-exact as float32) - equal files, 814,794 bytes each. The private EE suite then passed 53/53 with this branch installed as the client against its own node from the same release.

@PakhomovAlexander

Copy link
Copy Markdown
Contributor Author

Added differential tests (52925b5, tests/test_vector_response_objects.py) for the case a reviewer would probe first: a value with many fields, not only vec. A Rich type with ten fields — string, float array, int, double, long, bool, string array, timestamp, a None, and a nested object — is written by the client's own object writer, wrapped as the server ships it, and decoded twice: once by VectorResponse, once by the generic path the cursor used before (page parse, then unwrap_binary). Both results must equal each other and the objects written. Covered: both footer encodings (compact and full), the legacy map layout and the flagged rows-with-scores layout, and two value types in one page (each resolving its own class). The bool/string-array/timestamp/nested fields exercise the per-field generic fallback; the None exercises the TC_NULL branch. Tree total: 183 offline tests pass, flake8 3.8.4 unchanged. A live-cluster run with the same rich value type follows.

@PakhomovAlexander

Copy link
Copy Markdown
Contributor Author

Bug found by the rich-value e2e, fixed in 5acab4d. On the asyncio client, a value with a field that has no direct reader (a nested object, here) crashed: the per-field generic fallback called the sync BinaryObject.parse on an AioBinaryStream, whose registry lookup is a coroutine ('coroutine' object has no attribute 'schema'). The sync client and the single-field Article were unaffected, which is why the earlier gates passed. Fix: any fallback reached on an async stream (_decode_field, _decode_element_generic) now raises _NeedsAsync, and _decode_any_async decodes that whole element through the async generic path (AnyDataObject.parse_async + unwrap_binary). New test test_rich_objects_on_the_asyncio_stream_match_the_generic_async_path reproduces the exact failure on the unfixed decoder and passes on the fix (verified by reverting). Tree: 14 decoder/object tests, full offline suite unchanged vs base modulo my tests, flake8 unchanged. Live-cluster rich-value run re-running now.

@PakhomovAlexander

Copy link
Copy Markdown
Contributor Author

Rich-value e2e on the real 2-node cluster (release assembled from CE/EE master + gg-50942 + gg-50943): a value type with ten fields — string, float vector, int, double, long, bool, string array, timestamp, a None, and a nested object — 5,000 rows loaded through the base client, index settled before any dump, base client dumped first and again last as the order control (bracket stable). Rows from this branch are byte-identical to the base client: 400 row-lists over 16 legs (four row shapes x {k=10, k=10 paged at 3, k=100} on the sync client, plus asyncio with paging). Raw: runs-gcp/pyprof-20260831/pyprof-cluster-rich/. (This is the run that first exposed the asyncio nested-object bug fixed in 5acab4d; with the fix it is clean.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants