Skip to content

perf(db): codify the emergency transaction indexes and gate the conditional joins - #242

Merged
argszero merged 1 commit into
mainfrom
fix/db-transaction-indexes-migration
Sep 14, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/db-transaction-indexes-migration

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Item 3 of rant 2026-09-14T21:15:02 (host report: refreshing #/sharing on dev lands on the login page). Item 1 shipped in #240 (95d69ba); this PR makes the mitigation permanent and keeps it from regressing.

1. v15 migration: the emergency indexes now exist for every deployment

The two indexes that mitigated the incident were created by hand on the dev/prod main databases only, so a fresh deployment would still hit the same plan. SCHEMA_VERSION 14 → 15 adds them to migrate():

index why
idx_transactions_user_id_time_type_pts_tokens (user_id, time, type, pts, tokens, cached_tokens, output_tokens) summary's SUM(pts / tokens / cached_tokens / output_tokens) only touches t.*; with no column filter it must stay a covering index (dev: 0.21s vs 22.6s non-covering)
idx_transactions_key_id_type_pts sharing.rs::sharing_row runs SELECT SUM(pts) WHERE key_id = ?1 AND type = 'earn' once per row of /api/sharings, previously with no index at all (SCAN transactions, dev: 4.78s → 0.09s)

Both are IF NOT EXISTS — dev/prod already carry these names from the manual mitigation, so the migration cannot fail there.

2. Acceptance is asserted at the plan level, not by index name

EXPLAIN QUERY PLAN must report COVERING INDEX idx_transactions_user_id_time_type_pts_tokens for the summary aggregate and a SEARCH (never a SCAN) on idx_transactions_key_id_type_pts for the sharing sum. The statements in the test are copied from wallet.rs / sharing.rs, so this fails if either the index or the query shape drifts.

3. perf_gate.rs: the conditional joins must stay conditional

The other half of item 3. tx_joins_if(needs_joins(&filters)) is the only sanctioned way to attach the three LEFT JOINs; the gate asserts:

  • (a) tx_joins() has no direct call site outside the wrapper (positional: nearest preceding fn);
  • (b) the wrapper keeps its no-join branch — a wrapper edited into an always-join one leaves every call site untouched and is the same 100× regression;
  • (c) positive controls: exactly 3 tx_joins_if( sites (summary / COUNT / trend) and 2 needs_joins( sites.

This invariant is invisible locally (same aggregate values, only slower — SSD + small db), so it needs a gate rather than review.

Related Issue

Tracked as rant 2026-09-14T21:15:02 (no GitHub issue). Items 2 (sharing N+1) and 4 (boot fallback on non-401) are still open and will follow as separate PRs.

Tests

  • cargo test — 252 passed, 0 failed (249 before)
  • cargo fmt --check clean
  • A/B (fixed tree == pushed tree):
    • reverting the two indexes → exactly the two index/plan tests red, other 14 db::tests green;
    • making the COUNT statement join unconditionally → gate red, pointing at src/routes/wallet.rs:383: tx_joins() (test re-run after a restore);
    • making tx_joins_if always join → gate red on the missing no-join branch.

…tional joins

Host report (rant 2026-09-14T21:15:02, item 3): the two indexes that mitigated the
`#/sharing` incident on dev/prod were created by hand on those two databases only, so
a fresh deployment still hits the same plan.

v15 migration:

  - `idx_transactions_user_id_time_type_pts_tokens` (user_id, time, type, pts, tokens,
    cached_tokens, output_tokens) -- the only shape that lets `summary`'s
    `SUM(pts / tokens / cached_tokens / output_tokens)` stay a covering index;
  - `idx_transactions_key_id_type_pts` -- `sharing.rs::sharing_row` ran
    `SELECT SUM(pts) WHERE key_id = ?1 AND type = 'earn'` once per row of `/api/sharings`
    with no index at all (`SCAN transactions`, 4.78s -> 0.09s).

Both use `IF NOT EXISTS`: dev/prod already carry these names from the manual mitigation,
so the migration must not fail there.

Acceptance is asserted at the plan level, not by index name: `EXPLAIN QUERY PLAN` must
report `COVERING INDEX idx_transactions_user_id_time_type_pts_tokens` for the summary
aggregate and a SEARCH (never a SCAN) on `idx_transactions_key_id_type_pts` for the
sharing sum.

`perf_gate.rs` gains the other half of item 3 -- the conditional joins must stay
conditional. It asserts (a) `tx_joins()` has no direct call site outside the
`tx_joins_if` wrapper, (b) the wrapper keeps its no-join branch, and (c) two positive
controls (3 `tx_joins_if(` sites, 2 `needs_joins(` sites). One unconditional LEFT JOIN
costs 100x on dev (0.21s -> 22.6s) and is invisible locally, so it needs a gate.

`cargo test` 249 -> 252 passed; `cargo fmt --check` clean. A/B: reverting the two indexes
turns exactly the two index tests red; making one aggregate join unconditionally, and
making the wrapper always join, each turn the new gate red (recorded in the PR).
@argszero
argszero merged commit 5d4e6f4 into main Sep 14, 2026
1 check passed
@argszero
argszero deleted the fix/db-transaction-indexes-migration branch September 14, 2026 14:26
argszero added a commit that referenced this pull request Sep 15, 2026
Ships the 18 PRs merged since v0.7.24 (#242-#259). Schema 14 -> 15 (two
covering indexes, applied at startup). No config change, so no deployment-side
config.toml edit is needed.

Two themes:

Perf on the NFS dev database
- #259: stop mapping the db (PRAGMA mmap_size 64MB -> 0) and stop a real write
  per request (dao::touch_api_key gains a 60s guard). Measured on the live dev
  db: mmap=64MB 1.7-3.1s per COUNT / 250 MiB read vs mmap=0 ~10.5ms / 80 KiB;
  mmap=0 alone still leaves ~1.2s behind any write, so the pair is required.
- #242: codify the two emergency indexes in a v15 migration and gate the
  conditional joins at the plan level.
- #243: read the sharing page's earn total from one batched aggregate.

Frontend: display must equal what it filters on, and one fact, one source
- #250 one writer for the transaction cache; #251 clear every session slot at
  the identity boundary and give the wallet view a loader; #253 one shared
  writer for the wallet/dashboard month-changes; #254 boot loads only the
  destination view; #255 a model row's identity is the model, not its index;
  #256 the marketplace source follows the session, not whether data arrived;
  #257 the sidebar advertises only digits that work; #258 the admin
  total-balance card sums the gift amount its caption names.

i18n
- #249 every backend error reaches the wordlist, and the comment stripper stops
  mangling UTF-8; #252 the backend stops inventing Chinese display labels in
  response data fields.

Forms and robustness
- #244 a non-auth boot failure no longer looks like being logged out; #245 a
  credential 401 is no longer read as a session expiry; #246 wire timestamps
  reach the renderer unsliced; #247 inline cards submit from every field; #248
  a market row's availability label comes from that row.

- Cargo.toml / Cargo.lock: 0.7.24 -> 0.7.25.
- CHANGELOG.md: v0.7.25 entry.
- ui/index.html: cache-bust left as-is; the UI PRs in this release already
  advanced it past the value deployed with v0.7.24 (app.js 20260915-13,
  i18n.js 20260915-3).

cargo test 288 passed; cargo fmt --check clean; clippy unchanged.
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