Skip to content

perf(TC-1274): look up a batch read's kv/get and kv/metadata keys in one statement per space - #375

Merged
samgbafa merged 2 commits into
mainfrom
feat/tc-1274-batched-kv-read
Oct 19, 2026
Merged

samgbafa merged 2 commits into
mainfrom
feat/tc-1274-batched-kv-read

Conversation

@samgbafa

@samgbafa samgbafa commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

TC-1274. Ships in the release after 1.21.2. Fix E from the 2026-10-19 CPU analysis (§3, "Node" option 2).

invoke_read_only looked up each kv/get and kv/metadata capability with its own current_kv statement, so a 100-key batchGet/batchHead (the SDK's batchRead) was ~100 round trips. It now looks the keys up with one statement per space and answers the capabilities one at a time, in their own order, from that result.

Before / after (PostgreSQL, wire-counted)

postgres_round_trip_budget, new flows kv batch get / kv batch head: one /invoke of 100 capabilities over the fixture (98 distinct keys: an invocation's capabilities are a map, so the repeated key collapses). Measured on this branch's base (a742ab8) and on this branch:

round trips statements current_kv statements audit statements
kv/get ×100, before 216 108 98 1
kv/get ×100, after 22 11 1 1
kv/metadata ×100, before 216 108 98 1
kv/metadata ×100, after 22 11 1 1
single kv/get (unchanged) 22 11 1 1

A 100-key batch now costs what one key costs. (Each statement is two round trips: the statement and sqlx's release ping.) Budgets are 22 for both flows, and the test fails if a batch reads current_kv in more than one statement. Before/after logs: the kv batch rows of the postgres_round_trip_budget -- --nocapture report.

Identical responses. The test hashes the two response bodies. On the base commit and on this branch both are byte-identical across all four rounds: get 31e517b31fd4deb4…5f8f1, head 10a8a698f0f145c0…aab8e.

What does not change

  • Results and order. Capabilities are still processed in a loop in capability order; the lookup only replaces the per-capability query. Responses, their order, repeated, missing and deleted keys, kv/list and capabilities/read interleaved in the same invocation, are as before.
  • x-tinycloud-max-response-bytes. Still checked per kv/get capability, after its content is read, in capability order. The first oversized capability is the one reported (not the largest), with the same 413 and message, and a value of exactly the limit passes. There is no partial response.
  • Errors. A failed lookup is held and returned by the first capability of that space, as the error of its own point lookup would have been, after earlier capabilities (and their size errors) were answered. Same variants: TxStoreError::Tx(Db) for the index lookup, StoreRead for the block read.
  • Read audit. The record is still committed before any data is returned (db.rs after the loop), and its invoked_abilities rows per capability are unchanged. read_audit.rs is untouched.
  • Authorization is before the loop and untouched.
  • Single capability per space: keeps the existing point select, unchanged. So does MySQL: its usual collations compare keys case- and accent-insensitively, so a returned row cannot be matched back to the key that asked for it by string equality (the same reason has_unmigrated_artifacts has a MySQL exact path). Its behaviour is identical to before.
  • The transact path's kv/get (writes) is not touched.

How

  • PostgreSQL: SELECT … FROM current_kv WHERE space = $1 AND deleted = false AND key = ANY($2), one text-array parameter, so the statement text is the same for every batch size. Plan checked on a synthetic 1M-row table with the same primary key: Index Scan using current_kv_pkey, Index Cond on space and key = ANY(...).
  • SQLite: key IN (…), chunks of 500 (limit 32766 variables).
  • tinycloud-core enables sea-orm's postgres-array feature for the array parameter (Cargo.lock unchanged).
  • Telemetry: the kv_index_lookup stage is observed once per batched statement instead of once per key.

Tests

  • kv_batch_read (new, SQLite, through the real node with SDK-signed invocations) and postgres_round_trip_budget (PostgreSQL) check a 100-key batch get and head against the fixture: present, deleted and never-written keys, a repeated key, capability order, the cross-check of head against get ETags, the oversize cases (first oversized capability wins, exact-limit passes). Both pass unchanged on the base commit (old per-capability path) and on this branch, which is the old/new comparison.
  • Core db.rs (SQLite, and PostgreSQL when TINYCLOUD_TEST_POSTGRES_URL is set): *_kv_lookup_matches_point_lookups compares, for 200 pseudo-random mixes of up to 120 capabilities (two spaces holding the same key names, live / deleted / absent keys, repeated keys, kv/get and kv/metadata mixed, other capabilities mixed in), the batched lookup against get_kv_entity for every capability. *_kv_lookup_statement_counts pins the statement count per request shape, including 520 keys (2 statements on SQLite, 1 on PostgreSQL) and a lone capability (point lookup). A failed lookup is an error, not a panic.
  • Ran: fmt, clippy -D warnings, cargo test -p tinycloud-core --no-fail-fast (lib 497 passed) on SQLite and PostgreSQL 16.15 (ICU); cargo test -p tinycloud-node all binaries pass (CI and TINYCLOUD_TEST_BLACKHOLE unset: the 4 black-hole tests skip).
  • Not run: MySQL (its migration chain cannot run, TC-1314; its path is the unchanged per-capability one). Mixed get + metadata in one request is a route-level 501 (unchanged), so mixing is covered at the core level only. Multi-space invocations are covered at the core level, not end to end.

Conflicts with open PRs

…one statement per space

invoke_read_only fetched the current_kv row of each kv/get and kv/metadata capability with its own statement. It now fetches them with one statement per space (key = ANY($2) on PostgreSQL, IN lists of at most 500 on SQLite) and answers the capabilities one at a time in their own order from that result, so responses, order, errors and the read-audit record are unchanged. MySQL and single-capability spaces keep the point lookup.
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1274 (reviewed exact head aba7e12)

@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1274 (main-merge only; code +/- lines byte-identical vs aba7e12; one CHANGELOG header overlap, same class Naomi waived on #374; head 4c3cce7)

Detail: merged origin/main 50cb05c (1.21.3 so far: #163 pin, #371, #372, #370, #374) into the approved head aba7e12 with a merge commit. The only conflict was CHANGELOG.md. The PR diff against main has the same 5 files. Four files have byte-identical added/removed lines vs aba7e12 (compared with the corrected +/- filter): tinycloud-core/Cargo.toml, tinycloud-core/src/db.rs, tinycloud-node-server/tests/kv_batch_read.rs, tinycloud-node-server/tests/postgres_round_trip_budget.rs (db.rs, Cargo.toml and the budget test auto-merged with main's #370/#374 changes). Single difference, a SUBSET of the reviewed lines: CHANGELOG.md: the ### Changes header (+ blank line) that this PR added already exists in main's Unreleased section, so this PR now adds only its one bullet (the same overlap class as #374: benign header; Naomi's #374 waiver named it). Resolution: main's Unreleased entries first, then this PR's bullet; main's [1.21.2] section untouched. No test-support or other code line changed. CI must be fully green at this exact head (incl. clippy -D warnings and the PostgreSQL-backed test job, which runs the kv batch round-trip budget) before merge.

@samgbafa
samgbafa merged commit 108717e into main Oct 19, 2026
22 checks passed
samgbafa pushed a commit that referenced this pull request Oct 19, 2026
…shot

Budget conflict only: main now measures five flows. With the
one-statement auth snapshot the constants are set to the values measured
on PostgreSQL (see the PR body).
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.

1 participant