fix(flowsheet): switch nextPlayOrder index to composite (show_id, play_order DESC)#1413
Merged
Merged
Conversation
Schema constraint shape reportdata-shape report errored (exit 0): node:internal/modules/runmain:107 triggerUncaughtException( ^ Error ERRMODULENOTFOUND: Cannot find package 'postgres' imported from /home/runner/work/Backend-Service/Backend-Service/scripts/schema-shape-report.mjs Did you mean to import "postgres/cjs/src/index.js"? at Object.getPackageJ; manual check required |
…y_order DESC) (#1133) `flowsheet_play_order_idx` was introduced by 0073 as a single-column DESC index to back the then-global `SELECT max(play_order) FROM flowsheet`. #693 then scoped `nextPlayOrder()` to `WHERE show_id = ?`, leaving the index misaligned with the actual query shape: on the 2.6M-row flowsheet the planner has to either backward-scan the global DESC index and filter by show_id (slow for any non-current show), bitmap-scan `flowsheet_show_id_idx` + aggregate (multiple heap reads), or seq scan for small mis-estimated shows — none are the O(1) leaf lookup the original migration claimed. This swaps in a composite `(show_id, play_order DESC)` that makes the per-show MAX a true O(1) leaf-page lookup, with each show's rows forming a contiguous run inside the index whose leading edge is the per-show max. The same shape also covers the legacy mirror's `WHERE show_id = ? ORDER BY play_order DESC LIMIT 1` announcement lookups and `getEntriesByShow`'s `WHERE show_id IN (...) ORDER BY play_order DESC` listing — both of which the misaligned single-column index never served well either. The predecessor `flowsheet_play_order_idx` is dropped in the same migration. The only remaining global-MAX(play_order) caller is `jobs/flowsheet-etl/job.ts:resetSequences()`, a one-shot post-bulk-load sequence reset that runs occasionally and is not performance-sensitive. Test pins the migration shape (DROP + CREATE composite), the schema declaration (column order + DESC), and the post-#693 query shape in `nextPlayOrder()` so future drift either way trips before reaching prod. Closes #1133
4f1eeed to
f5f5386
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
flowsheet_play_order_idx(migration 0073) was introduced as a single-column DESC index onplay_orderto back the then-globalSELECT max(play_order) FROM flowsheetthatnextPlayOrder()ran on every flowsheet insert — correctly O(1) at the top of the index. #693 then rewrotenextPlayOrder()to scope byshow_id, leaving the index misaligned with the actual query shape: on the 2.6M-rowflowsheettable the planner now has three choices, none of them O(1):show_id— fast for the current show, increasingly slow as the show recedes from the leading edge.flowsheet_show_id_idx(0068) + Bitmap Heap Scan + Aggregate — multiple heap reads per show, well beyond 0073's measured 0.119 ms.This PR swaps in a composite
(show_id, play_order DESC)index that makes the per-show MAX a true O(1) leaf-page lookup: each show's rows form a contiguous run inside the index, with DESC ordering placing the per-show max at the run's leading edge. The same shape also covers two related per-show queries that the misaligned single-column index never served well either:apps/backend/middleware/legacy/flowsheet.mirror.ts's show_start/show_end announcement lookups (WHERE show_id = ? ORDER BY play_order DESC LIMIT 1).apps/backend/services/flowsheet.service.ts:getEntriesByShow(WHERE show_id IN (...) ORDER BY play_order DESC).The predecessor
flowsheet_play_order_idxis dropped in the same migration. The only remaining global-MAX(play_order) caller isjobs/flowsheet-etl/job.ts:resetSequences()— a one-shot post-bulk-load sequence reset that is not on a hot request path, so the planner falling back to a seq scan there is acceptable and the cleanup reclaims the 17 MB the single-column index used.Changes
0094_flowsheet-show-id-play-order-idx.sql:DROP INDEX IF EXISTS flowsheet_play_order_idx+CREATE INDEX IF NOT EXISTS flowsheet_show_id_play_order_idx ON flowsheet USING btree (show_id, play_order DESC). SameIF [NOT] EXISTS+ CONCURRENTLY-out-of-band runbook pattern as 0068 / 0070 / 0074 / 0078 / 0080.shared/database/src/schema.ts: replace theflowsheet_play_order_idxdeclaration withflowsheet_show_id_play_order_idx, with a comment block explaining the misalignment history and the two additional query shapes the composite covers.tests/unit/database/schema.flowsheet-show-id-play-order-idx.test.ts: new regression test pinning the migration shape (DROP + CREATE composite, no CONCURRENTLY in DDL), the schema declaration (column order + DESC ordering), and thenextPlayOrder()query shape (WHERE show_id = ?). Drift on either side trips the test before reaching prod.Production ops
Run out-of-band BEFORE merging this PR so the in-migration DDL is a no-op:
CREATE INDEX CONCURRENTLYtakes only aShareUpdateExclusiveLock, so DJs continue inserting flowsheet rows during the build. Expected size ~70 MB; build typically completes in under a minute on the prod RDS instance.Test plan
npm run lint(0 errors)npm run format:checknpm run typechecknpm run test:unit(3158/3158 passing, including the new schema test + the pre-existingnextPlayOrderregression test from nextPlayOrder() should scope to current show_id, not global max #693)shared/database/src/migrations/**changesCloses #1133