Skip to content

[gRPC] Streaming & pagination correctness (result close, row_index, replay dup, streamPaged, language drop) #5047

Description

@robfrank

Summary

Several correctness and efficiency defects in the gRPC streaming and pagination paths: closing a result set drains the entire remaining stream instead of cancelling; InsertError.row_index violates its documented contract; bidi replay after a failed chunk can duplicate rows; streamPaged has no snapshot isolation and is O(n²); is_last_batch is not emitted on exact-multiple boundaries; and the batched stream client APIs silently drop the query language.

Affected module / files

  • grpc-client/src/main/java/com/arcadedb/remote/grpc/StreamingResultSet.javaclose() (~147-160) — CON-3
  • grpc-client/src/main/java/com/arcadedb/remote/grpc/RemoteGrpcDatabase.javaqueryStreamBatched/queryStreamBatchesIterator (~868-869, 889-890) — PERF-2
  • grpcw/src/main/java/com/arcadedb/server/grpc/ArcadeDbGrpcService.java
    • streamCursor (~1572, 1589-1620) and streamPaged (~1700-1797) — STR-3, STR-4, PERF-3
    • insert-error row index (ctx.received - 1) at ~2755, 2783-2807, 2515 — STR-1
    • bidi failed-chunk watermark not advanced (~2519), rows already committed (~2810-2817) — STR-2
  • grpc/src/main/proto/arcadedb-server.protoInsertError.row_index doc (~357)

Defects

  • CON-3 (Medium) — close() drains instead of cancels. StreamingResultSet.close() loops while (stream.hasNext()) stream.read(), so closing after reading 10 rows of a 10M-row result transfers and decodes the entire remaining stream, keeping the server cursor/worker busy. It also has no checkCrossThreadUse guard, so a cleanup/timeout thread closing while the owner iterates makes two threads use the non-thread-safe BlockingClientCall concurrently. → Cancel the underlying call.
  • STR-1 (Medium) — row_index contract violation. InsertError.row_index is documented as "index within the chunk" but the server reports the cumulative stream-wide ctx.received - 1; the bidi whole-chunk-failure path reports Math.max(0, ctx.received - 1). Clients cannot identify which row failed.
  • STR-2 (Medium) — bidi replay duplicates rows. On chunk failure the watermark is deliberately not advanced (to allow client replay), but under PER_ROW/PER_BATCH some rows of the failed chunk were already committed; replay re-inserts them (no per-row idempotency) unless key_columns + CONFLICT_UPDATE/IGNORE are set.
  • STR-3 (Medium) — streamPaged no snapshot isolation + wrapper fragility. Each page re-executes SELECT FROM (<original>) ORDER BY @rid SKIP :_skip LIMIT :_limit; concurrent inserts/deletes between pages shift SKIP offsets → duplicated/skipped rows. The @rid wrapper also breaks for projection-only results (no @rid) and silently overwrites user parameters named _skip/_limit.
  • PERF-3 (Medium) — streamPaged is O(n²). Re-evaluating and re-sorting the inner query per page, with SKIP walking past all prior rows, is ~R²/(2B) row visits + R/B sorts for R rows / batch B. convertParameters + a wrapping HashMap are also rebuilt per page though parameters are invariant.
  • STR-4 (Low) — is_last_batch missing on exact multiples. When total rows are an exact multiple of batchSize, streamCursor emits the final full batch with isLastBatch=false and the trailing inBatch > 0 check never fires; streamPaged returns after the count == 0 probe page without a terminal batch. Clients (queryStreamBatchesIterator drained logic, ~939-941) do an extra round trip and rely on stream completion.
  • PERF-2 (High-ish correctness) — batched stream drops language. queryStreamBatched (~868-869) and queryStreamBatchesIterator (~889-890) never call .setLanguage(language), so a Cypher/Gremlin stream query silently executes as SQL (server default langOrDefault).

Impact

  • Wrong/duplicated/skipped rows (STR-2, STR-3), silent wrong query language (PERF-2), wasted bandwidth + a cross-thread data race on close (CON-3), quadratic paging cost (PERF-3), unusable error indices (STR-1), extra round trips (STR-4).

How to reproduce

  • PERF-2: queryStreamBatched("MATCH (n) RETURN n", language="cypher", ...); assert it runs as Cypher — it runs as SQL and fails/misbehaves. (Highest-value quick fix.)
  • CON-3: Stream a large result, read a few rows, close(); observe the server keeps producing until the stream is drained (measure time / server-side cursor lifetime).
  • STR-3: Page through a query while another client inserts/deletes rows between pages; assert no duplicates/skips — there will be.
  • STR-1: Force a row failure mid-stream; assert row_index is the index within the chunk — it is the cumulative index.

Proposed fix

  1. PERF-2: set .setLanguage(language) on both batched-stream request builders.
  2. CON-3: close() cancels the underlying ClientCall (stream.cancel(...)) instead of draining; add the same checkCrossThreadUse guard as hasNext/next.
  3. STR-1: report row_index relative to the current chunk (or add a separate stream_index field and document both).
  4. STR-2: advance the watermark for rows already committed, or make chunk processing atomic per chunk (commit only at chunk end) so replay cannot double-insert; document the idempotency contract.
  5. STR-3 / PERF-3: prefer CURSOR mode internally for streamPaged (single execution, snapshot-consistent), or open one transaction spanning the paging; hoist convertParameters out of the loop; guard the @rid wrapper against projection-only results and reserved parameter-name collisions.
  6. STR-4: emit a terminal batch (or set is_last_batch=true on the final full batch) for exact-multiple totals in both streamCursor and streamPaged.

Acceptance criteria

  • Cypher/Gremlin batched stream queries execute in the requested language.
  • Closing a partially-read stream cancels promptly (server cursor released) with no cross-thread misuse.
  • Paging is snapshot-consistent and not quadratic; is_last_batch is correct on boundaries; insert-error indices identify the failing row.

Provenance

From the 2026-07 gRPC modules audit (docs/2026-07-06-grpc-modules-audit.md, findings CON-3, STR-1, STR-2, STR-3, STR-4, PERF-2, PERF-3).

Metadata

Metadata

Assignees

Labels

auditFindings from the 2026-07 engine deep auditbuggrpcgRPC wire protocol (grpc/grpc-client/grpcw)

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions