Commit 02311ca
authored
perf(db): stop scanning every workspace file on workflow archive, cover the billing period aggregate (#6015)
* perf(files): stop loading every workspace file to archive a workflow's backing files
`cleanupWorkflowAliasBacking` runs on every workflow delete. It loaded every
`context='workspace'` row for the workspace — active and soft-deleted, all
columns — then discarded >99% of them in JS to find the handful of
`.changelogs/<workflowId>.md` and `.plans/<workflowId>/**` rows it owns.
In production this was the single worst query at 37.83% of total database
runtime: p50 6s, p95 9s, max 13.1s, 641 calls/day, 34.1M rows read. It is
index-served, so the cost is not a missing index — it is 59,061 rows and
~630MB of cold buffers read to archive a few files.
A file's `folderPath` is derived solely from its `folderId`, so matching on
folder membership is equivalent to the path comparison it replaces. The load
and JS filter become one targeted UPDATE keyed on a handful of folder ids.
Folders that are soft-deleted are still included when resolving which files a
workflow owns: path resolution ignores `deletedAt`, so a live file parented to
an archived folder previously matched and must continue to.
Also:
- Add `getWorkspaceShares`, replacing an id-list `IN` clause that grew with the
file count (59,061 elements on the worst workspace) with one indexed lookup
on `workspace_id`. Callers read the map by id, so a superset is equivalent.
- Drop `all` from the client-reachable file scope enum. It drops the
`deleted_at` predicate and so cannot use the partial index serving the other
two. No client requests it; server callers reach that scope directly.
- Set `fetch_types: false` on the app and realtime pools. postgres.js otherwise
runs a blocking `pg_catalog.pg_type` roundtrip before each new connection's
first query — 95,722 of them per day. It builds array parsers only; Drizzle
already parses this schema's two `text[]` columns itself.
* perf(nav): add route loading boundaries and cover the billing period aggregate
Dynamic routes prefetch only down to the nearest loading boundary, and the
server stops prefetching at the first one. With no loading.tsx anywhere in the
workspace tree and a dynamic layout, Link prefetch was yielding almost nothing
and every navigation waited on a full server round trip with no feedback.
Add loading.tsx only where the fallback is provably what renders today, so
perceived speed improves without changing what users see:
- home, chat/[chatId]: reuse HomeFallback, already each page's own Suspense
fallback.
- integrations, skills: reuse the tab-header chrome, byte-identical to each
page's own Suspense fallback.
Deliberately not added to workspace root, settings, or w, whose pages are
redirect-only or already self-fallbacking — a boundary there would paint a
skeleton that does not match the destination.
Billing:
- Add usage_log_billing_period_cost_idx, trailing the remaining predicate
columns and `cost` so the period aggregates resolve index-only. Confirmed
against production: the aggregate currently runs as an Index Scan touching
478,559 buffers because `cost` is absent from the existing index. That index
is superseded but left in place; dropping it is a separate migration so a
planner regression costs nothing to revert.
- Collapse the two aggregates behind /api/billing into one scan using
SUM(...) FILTER. Besides halving the work on a nav-path query, it removes a
latent inconsistency: as separate statements the two sums could observe
different snapshots, making the copilot subset exceed the total.
Caching was considered and rejected for the usage aggregate. Its callers
include usage enforcement, threshold billing, and overage calculation, where a
stale-low read permits overspend and a stale-high read double-charges.
* test(files): cover cleanupWorkflowAliasBacking and correct a stale share mock
cleanupWorkflowAliasBacking had no test coverage, and the rewrite that replaced
its load-everything-then-filter body rests on a subtle equivalence: a file's
folderPath is derived solely from its folderId, and path resolution ignores
deletedAt. The second half is the easy part to get wrong — a live file parented
to an archived folder still resolves to a backing path and must still be
archived, so folders are filtered by deletedAt only when choosing which folders
to archive, never when deciding which files the workflow owns.
These tests pin that distinction: they fail if the archived-folder case is
dropped from file ownership, and separately assert that archived folders stay
out of the folder update and that unrelated workflows are never touched.
Also point the workspace files route test at getWorkspaceShares. Its mock still
named getSharesForResources, which the route no longer imports. The suite passed
regardless because all three cases exercise the upload path, so the GET listing
has no coverage — but the stale name would have handed the first GET test an
undefined function.
* fix(perf): drop the loading boundaries and correct the usage_log index order
Adversarial review found real regressions in both.
Route loading boundaries — all four removed:
The premise in their TSDoc was wrong. Each page's existing `<Suspense>` exists so
nuqs can prerender; `useSearchParams` only suspends during SSR, so on a client
navigation those fallbacks never painted. Hoisting them to loading.tsx did not
"show the same frame" — it made a previously invisible blank frame visible.
For chat, Next keys the Suspense boundary by cache key, so a chatId change mounts
a fresh suspended boundary and always commits its fallback. Switching chats would
have gone from "previous chat stays on screen" to a blank surface for the whole
RSC round trip, on the highest-frequency navigation in the product. Partial
prefetch only warms the fallback, so no latency was saved to offset it.
The integrations and skills fallbacks were correct for their own pages but also
became the fallback for four detail routes that render different chrome, inserting
a wrong intermediate frame. Fixing that needs per-child boundaries and new skeleton
UI that cannot be verified without a browser, for pages whose only server work is
`await params`. Not worth it.
Every remaining loading.tsx in this app paints real chrome. A blank one was against
the grain, and the measurable win was zero.
usage_log index:
Column order was wrong. The daily-refresh rollup filters entity type, id and
period_start but NOT period_end, so putting period_end fourth ended the usable
prefix at column three and left user_id and created_at as in-index filters rather
than scan boundaries — turning a ~2.2k-entry bitmap scan into a ~266k-entry scan.
Verified in production that period_start functionally determines period_end
(13,612 groups, zero with more than one end), so the slot bought no selectivity.
user_id and created_at now follow the shared prefix; period_end rides as payload.
Also corrected the claim that this supersedes usage_log_billing_entity_period_idx.
That index deduplicates to 17 MB across 1.42M entries because it has no
high-cardinality key column, which is what keeps prefix-only bitmap scans cheap.
It is retained deliberately, not pending a drop.
Harden cleanupWorkflowAliasBacking: gate the UPDATE on the ownership filter list
itself. `and()` and `or()` both drop undefined, so a clause that resolved to
nothing would have left a WHERE of workspace + context + not-deleted and archived
every file in the workspace. Tests now assert no UPDATE is issued when the
workflow owns nothing, plus the filename and context predicates.
Note fetch_types also disables array serializers, not just parsers; documented.
* docs(db): correct the fetch_types constraint note after differential testing
Ran both settings against a real Postgres with the schema's actual text[] columns.
Drizzle-typed selects and .returning() are byte-identical either way; only a raw
db.execute projecting an array column differs, yielding the wire form.
The previous note also claimed a raw JS-array bind fails because the serializers
come from the same catalog fetch. It does fail — but under both settings, because
Drizzle expands an array into a row constructor before postgres.js ever sees it.
That is unrelated to this flag, so the claim is removed rather than left implying
a constraint this change introduces.
* chore(db): generate the drizzle snapshot for the usage_log index migration
0271 was hand-written, so drizzle-kit's state never learned about the new index —
the next `generate` would have re-emitted it as a fresh migration against an
already-migrated database.
Ran `drizzle-kit generate` to produce the snapshot, then restored the
CONCURRENTLY form: drizzle emits a plain CREATE INDEX, which takes an ACCESS
EXCLUSIVE lock and would block writes on a 4M-row table for the duration of the
build. The generated column order matched the hand-written SQL exactly, which
also confirms schema.ts and the migration agree.
`generate` is now a no-op, and check:migrations still passes.1 parent 79b1ed1 commit 02311ca
17 files changed
Lines changed: 18092 additions & 60 deletions
File tree
- apps
- realtime/src/database
- sim
- app/api/workspaces/[id]/files
- hooks/queries
- lib
- api/contracts
- billing/core
- copilot/vfs
- packages/db
- migrations
- meta
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
214 | 214 | | |
215 | 215 | | |
216 | 216 | | |
| 217 | + | |
| 218 | + | |
217 | 219 | | |
218 | 220 | | |
219 | 221 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | | - | |
| 12 | + | |
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
83 | | - | |
| 83 | + | |
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
| 77 | + | |
81 | 78 | | |
82 | 79 | | |
83 | 80 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
7 | 13 | | |
8 | 14 | | |
9 | 15 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| 19 | + | |
18 | 20 | | |
19 | 21 | | |
20 | 22 | | |
| |||
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| 40 | + | |
38 | 41 | | |
39 | 42 | | |
40 | 43 | | |
| |||
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
53 | | - | |
| 56 | + | |
54 | 57 | | |
55 | 58 | | |
56 | 59 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
11 | 15 | | |
12 | 16 | | |
13 | 17 | | |
| |||
429 | 433 | | |
430 | 434 | | |
431 | 435 | | |
432 | | - | |
433 | | - | |
434 | | - | |
| 436 | + | |
| 437 | + | |
435 | 438 | | |
436 | 439 | | |
437 | 440 | | |
438 | 441 | | |
439 | | - | |
440 | | - | |
| 442 | + | |
441 | 443 | | |
442 | 444 | | |
443 | 445 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
207 | 207 | | |
208 | 208 | | |
209 | 209 | | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
210 | 244 | | |
211 | 245 | | |
212 | 246 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
1 | 2 | | |
2 | 3 | | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
7 | | - | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
22 | | - | |
| 21 | + | |
23 | 22 | | |
24 | 23 | | |
25 | 24 | | |
| |||
79 | 78 | | |
80 | 79 | | |
81 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
82 | 175 | | |
0 commit comments