Skip to content

improvement(tables): cut the DB round trips a table read and write spend on protocol - #8104

Merged
waleedlatif1 merged 2 commits into
stagingfrom
perf/tables-api
Sep 21, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
perf/tables-api

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

The tables read path spent more time on DB round trips than on work. Three of them were avoidable, and none of them changes what a caller gets back:

  • The run-state sidecar was read for tables that cannot have one. tableRowExecutions is keyed by (rowId, groupId), every writer takes its groupId from a group on the table's own schema, and group/column deletes strip the sidecar in the same transaction that removes the group. A table declaring no group was still paying a chunked query per 250 rows — four on a 1000-row page — each returning nothing. Both signals are checked (workflowGroups and any column still carrying a workflowGroupId), so an unexpected schema shape keeps the query rather than silently dropping run state.
  • The row drain opened a transaction per batch. seqscanOff and repeatableRead are fixed for the call, so every batch past the first paid BEGIN + set_config + COMMIT for nothing. A 1000-row page drains in two batches, so that was three round trips of pure protocol on the grid's first read.
  • setTableTxTimeouts issued three SET LOCAL statements. set_config(name, value, true) is exactly SET LOCAL and all three fit in one SELECT — which is what the read guards already do. Every table write transaction begins with this call, so this lands on insert, delete, import and column changes alike.

Statements per request, measured from the Postgres statement log:

request before after
GET rows?limit=1000 (the grid's first page) 22 15
GET rows?limit=50 15 14
POST rows (insert) 15 13
DELETE rows 13 11

Row visibility is unchanged. Under READ COMMITTED each statement still takes its own snapshot, so a batch sees exactly what a separate transaction would have. Under REPEATABLE READ the drain's batches now share one snapshot instead of taking one each, which is strictly more consistent — a row and the secret-provenance sidecar captured for it can no longer come from different points in time across a batch boundary.

Response equivalence

Rather than reason about it, the two trees were diffed on the wire. Every table endpoint the UI touches was captured against both, across four table sizes, including a full keyset page-walk: 76 endpoints / 96 HTTP responses / 88,538,429 bytes of response body, byte-identical (only the per-request requestId, random by design, normalized).

Benchmarks

Production build, real HTTP, session-authenticated, against seeded tables of 100 / 1k / 10k / 100k rows (12 columns, mixed types). Interleaved A/B between two prebuilt trees — same machine, same DB, alternating — p50 over 2–3 rounds × 10–15 iterations. A loopback TCP shim adds fixed one-way delay so a DB round trip costs what a real one does; 0ms is the direct connection, which isolates the CPU side.

Grid first page (limit=1000), before → after:

DB round trip 1k rows 10k rows 100k rows
0ms (direct) 19.0 → 13.7ms (−28%) 18.8 → 14.2ms (−25%) 18.9 → 14.4ms (−24%)
2ms 106.8 → 60.8ms (−43%) 107.4 → 60.8ms (−43%) 106.0 → 63.2ms (−40%)
10ms 422.5 → 247.9ms (−41%) 420.0 → 249.5ms (−41%) 421.5 → 249.7ms (−41%)

Opening a table — the three requests the page actually fires in parallel — at 2ms: 104.0 → 64.0ms (−39%), and −40% at 10ms.

Other paths at 2ms: limit=100 −23 to −28%, second page −40%, sorted −8 to −14%, limit=50 −3 to −5%. Writes (insert + update + delete) −1 to −4%, which is the two round trips the timeout fix removes.

The gain is there at 0ms and grows with latency because it is structural — fewer round trips, not less work — so it carries to a managed Postgres hop rather than depending on this machine. Endpoints the change does not touch (GET /api/table/[id], views, tables list) are flat within noise at every latency, which is the regression check.

Type of Change

  • Performance improvement

Testing

  • vitest run full app suite: 54,564 passed, 1 pre-existing load-flake (cli-tools-boundary, a module-graph walk that times out under full-suite load and passes in isolation — unrelated to tables)
  • Table suites: 1,968 passed across 144 files
  • New tests cover the run-state elision and its fallback, and were verified to fail when the optimization is reverted
  • bun run check:audits (47 audits), bun run lint, bun run type-check, docs-manifest:check, block-registry check — all pass
  • Response bodies diffed byte-for-byte against the base tree (above), and end-to-end HTTP benchmarked at three latencies including the write path

Follow-ups (not in this PR)

  • Answering "is a delete job running?" from the table a request already loaded saves another round trip per read, but widens a pre-existing race — the probe has always been a separate statement from the row read. Doing it safely means folding the probe into the guard statement that already opens the drain transaction, so the job is read in the row read's own snapshot at no extra cost. That is a larger change than belongs here; it was tried, reviewed, and removed.
  • The run-state sidecar still drains in serial 250-id chunks for tables that do have groups. Parallelising would change the documented "a refusal costs one over-budget chunk" memory bound.
  • A single-row insert still makes two aggregate queries (max(order_key), max(position)) inside the row-order lock that could be one statement.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…end on protocol

The grid's first page spent more time on round trips than on work. Four of them
were avoidable:

- `pendingDeleteMask` probed `table_jobs` on every read, though the table a
  request just loaded already carries its latest non-export job, and the
  `one_active_per_table` unique index makes that row the running delete when one
  exists. Callers that hold a table across a long walk (the export stream, the
  snapshot builder) keep probing per page, so a delete starting mid-walk still
  begins masking.
- The run-state sidecar was read for every table, including the ones that
  declare no workflow group and therefore cannot have a row — four chunked
  queries on a 1000-row page, all returning nothing.
- The drain opened a transaction per batch. The guards are fixed for the call,
  so each extra batch paid `BEGIN` + `set_config` + `COMMIT` for nothing.
- `setTableTxTimeouts` issued three `SET LOCAL` statements; `set_config(…, true)`
  is the same thing and fits in one round trip, as the read guards already do.

A 1000-row page goes from 22 statements to 14, a 50-row page from 15 to 13, and
every write transaction drops two.
@vercel

vercel Bot commented Sep 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 21, 2026 8:59am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported stale delete-job optimization was removed and the provenance documentation now matches the implementation.

Summary

This PR reduces database round trips in table read and write paths while preserving existing response behavior:

  • Skips run-state sidecar reads when the table schema cannot contain workflow-group execution state.
  • Reuses one guarded transaction across all bounded row-read batches.
  • Applies all transaction-local timeout settings in one parameterized statement.
  • Updates tests and provenance documentation for the new behavior.
  • The stale delete-job optimization identified previously has been fully removed.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[queryRows] --> B[Build pending-delete mask]
  B --> C[Open one guarded transaction]
  C --> D[Fetch bounded row batch]
  D --> E[Capture provenance in same transaction]
  E --> F{More rows needed?}
  F -->|Yes| D
  F -->|No| G{Schema may contain run state?}
  G -->|Yes| H[Load execution sidecar]
  G -->|No| I[Use empty execution maps]
  H --> J[Return page]
  I --> J
Loading

Reviews (2) · Last reviewed commit: "review(tables): drop the delete-mask eli..."

Comment thread apps/sim/lib/table/rows/pending-delete-mask.ts Outdated
Comment thread apps/sim/lib/table/rows/service.ts
…ce snapshot doc

Reading the delete job from the table a request already loaded widened a race
the mask probe has always had — a job committing between the check and the row
read is missed either way, but trusting the loaded fields moves the check two
queries earlier. Closing it properly means evaluating the job inside the row
read's own snapshot, which is a larger change than this one, so the elision is
removed and `pending-delete-mask.ts` is back to what it was.

The three remaining reductions are untouched: they were the bulk of the win,
and each is a read this code cannot need rather than a read it takes on faith.

Also updates `TableRowProvenanceReader`'s doc, which still described one
repeatable-read transaction per batch.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 2cecb21 into staging Sep 21, 2026
35 checks passed
@waleedlatif1
waleedlatif1 deleted the perf/tables-api branch September 21, 2026 15:44

This branch was previously deployed

1 inactive deployment
Preview bd69492e Deployed Sep 21, 2026 by vercel[bot]
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