Skip to content

Partial index for metadata_attempt_at drain query (closes #659)#660

Merged
jakebromberg merged 1 commit into
mainfrom
task/659-metadata-attempt-pending-index
Apr 29, 2026
Merged

Partial index for metadata_attempt_at drain query (closes #659)#660
jakebromberg merged 1 commit into
mainfrom
task/659-metadata-attempt-pending-index

Conversation

@jakebromberg

Copy link
Copy Markdown
Member

Summary

Migration 0070 adds flowsheet_metadata_attempt_pending_idx — a partial B-tree on flowsheet(id) covering the metadata_attempt_at IS NULL tail. Required by both #638 (historical drain) and #639 Phase 2 (recurring drift-repair sweep). Without it each batch seq-scans 2.6M+ rows just to find the NULL slice, and #639 Phase 2's recurring cadence amplifies the cost.

CREATE INDEX IF NOT EXISTS "flowsheet_metadata_attempt_pending_idx"
  ON "wxyc_schema"."flowsheet" USING btree ("id")
  WHERE "entry_type" = 'track'
    AND "artist_name" IS NOT NULL
    AND "metadata_attempt_at" IS NULL;

The metadata_attempt_at column shipped in #658, so the partial predicate is now valid against prod.

Production runbook (please run before merging)

Drizzle wraps each migration in a transaction, which means the in-migration CREATE INDEX runs not as CONCURRENTLY and acquires a ShareLock on flowsheet for the build duration — that's the failure mode this index is supposed to prevent. The established workaround (per 0057, 0068) is: build out-of-band on prod first via CREATE INDEX CONCURRENTLY, then merge this PR so the migration's IF NOT EXISTS makes apply a no-op.

ssh wxyc-ec2
psql "$DATABASE_URL" -c "
  CREATE INDEX CONCURRENTLY \"flowsheet_metadata_attempt_pending_idx\"
    ON \"wxyc_schema\".\"flowsheet\" USING btree (\"id\")
    WHERE \"entry_type\" = 'track'
      AND \"artist_name\" IS NOT NULL
      AND \"metadata_attempt_at\" IS NULL;
"

Expected build window: a few minutes, no AccessExclusiveLock, no INSERT pause.

After the CONCURRENTLY build completes, verify and merge:

SELECT indexname FROM pg_indexes
 WHERE schemaname='wxyc_schema'
   AND tablename='flowsheet'
   AND indexname='flowsheet_metadata_attempt_pending_idx';
-- expect: 1 row

Shape choices (also documented in the migration comment)

  • Single-column (id) is enough. The drain orders by id and pages by id > $lastId. Heap lookup for the row body is cheap given the small post-filter row count.
  • add_time not in the predicate. The now() - interval '60 seconds' filter exists to dodge the legacy mirror's eventual-consistency window on freshly-inserted rows (see Re-emit liveFs SSE event after metadata enrichment UPDATE lands #628), not for selectivity. Adding it would inflate the index without improving plans.
  • Partial keeps the index small. ~1.86M NULL rows pre-drain → ~50 MB. Long-term the residual stays small and the index shrinks accordingly.

CLAUDE.md update

The new CREATE INDEX IF NOT EXISTS paragraph documents the established pattern (0068 set it; 0070 follows). Tightens the prior "DDL exactly as drizzle-kit emitted" guidance from #658, which was over-broad — IF NOT EXISTS on CREATE INDEX is the only sanctioned hand-edit on the DDL itself, scoped to migrations whose deploy runbook involves a CONCURRENTLY pre-build.

Tests

tests/unit/database/schema.flowsheet-metadata-attempt-pending-idx.test.ts — 6 assertions modelled on the 0057 test:

  • migration exists at the journal-pointed path
  • DDL has IF NOT EXISTS (idempotent against prod-prebuilt index)
  • DDL targets flowsheet(id)
  • WHERE predicate covers all three filter clauses (entry_type='track', artist_name IS NOT NULL, metadata_attempt_at IS NULL)
  • DDL does not use CREATE INDEX CONCURRENTLY (drizzle txn-wrap incompatible). The negative regex is anchored on non-comment lines so the runbook's CREATE INDEX CONCURRENTLY example in the comment block doesn't false-positive.
  • schema.ts declares the index so drizzle-kit drift detection sees it.

TDD: all six failed first, then the schema + migration brought them green.

Test plan

  • npm run typecheck clean
  • npm run lint 0 errors
  • npm run format:check clean
  • npm run lint:migrations passes (1 historical allowlisted warning, unchanged)
  • npm run test:unit 1362 / 1362 passing (the 6 new assertions plus everything else)
  • CI passes
  • Pre-merge: run the CREATE INDEX CONCURRENTLY ... command against prod RDS (see runbook above)
  • Post-merge: Auto Build & Deploy migrate finds the index already present and skips it via IF NOT EXISTS
  • Post-merge: EXPLAIN the drain query and confirm Index Scan using flowsheet_metadata_attempt_pending_idx (rather than Seq Scan on flowsheet)

Unblocks

Closes #659

Migration 0070 adds `flowsheet_metadata_attempt_pending_idx` —
a partial B-tree on flowsheet(id) WHERE entry_type='track' AND
artist_name IS NOT NULL AND metadata_attempt_at IS NULL. Both
#638's historical drain and #639 Phase 2's recurring drift-repair
sweep keyset-paginate by id through this slice; without the index
every batch seq-scans 2.6M+ rows just to find the NULL residual,
and the recurring sweep amplifies that cost.

Shape choices in the migration comment:
- Single-column (id) is enough — drain orders by id, heap lookup is
  cheap given the small post-filter row count.
- add_time NOT in the predicate; the `now() - interval '60 seconds'`
  filter is to dodge the legacy mirror's eventual-consistency window
  (#628), not selectivity.

Production ops:
- This is NOT CREATE INDEX CONCURRENTLY because Drizzle wraps each
  migration in a transaction (same constraint as 0057, 0061, 0068).
- The runbook is: build CONCURRENTLY out-of-band on prod first
  (the exact command is in the migration's comment block), then
  merge so the migration's IF NOT EXISTS makes apply a no-op.
  Without the pre-build, the migration's regular CREATE INDEX
  acquires ShareLock on flowsheet for the build duration and
  queues every concurrent INSERT.

Tests: new tests/unit/database/schema.flowsheet-metadata-attempt-pending-idx.test.ts
mirrors the 0057 pattern — 6 assertions pin the migration shape
(IF NOT EXISTS, table+column, the three WHERE clauses, no CONCURRENTLY
in the actual DDL while tolerating CONCURRENTLY in the comment block)
plus the schema.ts declaration so drizzle-kit drift detection can't
silently lose any clause. TDD: all six failed first, then schema +
migration brought them green.

CLAUDE.md gains a paragraph documenting that `CREATE INDEX IF NOT
EXISTS` is the established hand-edit pattern (0068 set it; 0070
follows). Tightens the prior "DDL exactly as drizzle-kit emitted"
guidance which was over-broad. The IF NOT EXISTS hand-edit is
scoped to CREATE INDEX migrations whose deploy runbook involves
a CONCURRENTLY pre-build — explicitly NOT a license to add IF
NOT EXISTS to other DDL.

Closes #659
@jakebromberg jakebromberg merged commit 380ceeb into main Apr 29, 2026
4 checks passed
@jakebromberg jakebromberg deleted the task/659-metadata-attempt-pending-index branch April 29, 2026 13:25
jakebromberg added a commit that referenced this pull request Apr 29, 2026
One-shot containerized job at jobs/flowsheet-metadata-backfill/ that
drains the ~1.86M-row historical tail where the LML metadata
enrichment never ran. Adopts the canonical filter
(metadata_attempt_at IS NULL + entry_type='track' + artist_name IS
NOT NULL + 60s race guard) from day one — #639 Phase 1 (PR #658)
ensured every pre-existing flowsheet row has a NULL stamp; #659
(PR #660) ensured the partial index covers the SELECT so each
batch is an Index Scan rather than a seq scan over the 2.6M-row
table.

Files:
  jobs/flowsheet-metadata-backfill/
    package.json        — workspace registration; "job-type": "one-shot"
    tsconfig.json       — extends base
    tsup.config.ts      — minified ESM bundle
    logger.ts           — Phase-A JSON logger + Sentry init (mirrors
                          jobs/flowsheet-etl/logger.ts; tags repo /
                          tool / step / run_id per #538)
    lml-types.ts        — minimal slice of LookupResponse, widened to
                          cover the artwork fields enrich.ts writes
    lml-fetch.ts        — minimal LML /lookup client; 30s timeout;
                          duplicates apps/backend/services/lml/lml.client.ts
                          to keep the job's build graph independent
    enrich.ts           — single-row 10-column UPDATE + metadata_attempt_at
                          stamp; mirrors enrichment.service.ts shape;
                          inline spacer.gif filter (#638 note 1, until
                          #649 lands); inline Discogs-bio cleaner;
                          synthesized search URLs on no-match
    orchestrate.ts      — id-cursor batch loop, partition mod-N filter,
                          throttle, processRow with error classification
    job.ts              — entrypoint: initLogger, runBackfill,
                          closeDatabaseConnection, closeLogger
  Dockerfile.flowsheet-metadata-backfill  — DB_STATEMENT_TIMEOUT_MS=300000,
                                            DB_SYNCHRONOUS_COMMIT=off,
                                            DB_APPLICATION_NAME tag
  tests/unit/jobs/flowsheet-metadata-backfill/
    enrich.test.ts      — 9 tests pinning: 10-column UPDATE on match,
                          search-URL-only UPDATE on no-match, stamp
                          shape (sql`now()` not Date), idempotency
                          guard, spacer.gif filter, extractArtwork
                          handles three no-match shapes
    orchestrate.test.ts — 13 tests pinning: SELECT carries the
                          canonical filter, id-cursor advances across
                          batches, lml_error counted + enrich NOT
                          called (row stays retryable),
                          resolvePartitionFilter validates inputs,
                          BATCH_SIZE / THROTTLE_MS exposed for tuning
  CLAUDE.md             — monorepo layout table grows the new package

TDD: the 22 new tests went red first against empty modules, then
green as enrich.ts and orchestrate.ts came in. Pinning the lml_error
path explicitly (enrich NOT called) guards the row-stays-retryable
contract that #639 Phase 2's recurring sweep depends on.

Implementation notes from your fresh-eyes review on the issue
(2026-04-29 comments) addressed:
  1. Inline spacer.gif filter — done in enrich.ts; ready to swap
     for #649's shared helper when it lands.
  2. Trace observability gap (job's own lml-fetch.ts, not #646's
     wrapped client) — relying on @sentry/node v10+ undici auto-
     instrumentation; comment on lml-fetch.ts flags the fallback if
     the pilot run (#640) shows propagation breaks.
  3. Concurrent runtime+job stamp race — documented inline in
     orchestrate.ts header; no CAS pattern (last-write-wins is
     correct because both writers produce identical data, and the
     applyEnrichment WHERE narrows by metadata_attempt_at IS NULL
     to make the second write a no-op).
  4. Manual Build & Deploy target — no GHA workflow change needed;
     deploy-base.yml uses Turborepo to detect affected targets
     dynamically, so registering the workspace is sufficient.

Run procedure (deferred until #640 pilot):
  Manual Build & Deploy with target=flowsheet-metadata-backfill,
  then SSH to EC2 and `docker run --rm --env-file .env <image>
  2>&1 | tee log`. LML's Discogs rate budget (50 req/min) is the
  limiting factor across all parallel partitions, not the database.

Out of scope (tracked elsewhere):
  - #639 Phase 2 (drop "job-type": "one-shot" + EC2 cron) — fires
    after #641 plateaus.
  - #640 (pilot run + parallelism scaling decision).
  - #641 (phased rollout — blocked by #649).
  - #642 (close-out tests/docs).
  - #661 (delete obsolete scripts/backfill-metadata.ts) — blocked
    by this PR landing + #641 closing.

Closes #638
jakebromberg added a commit that referenced this pull request Apr 29, 2026
One-shot containerized job at jobs/flowsheet-metadata-backfill/ that
drains the ~1.86M-row historical tail where the LML metadata
enrichment never ran. Adopts the canonical filter
(metadata_attempt_at IS NULL + entry_type='track' + artist_name IS
NOT NULL + 60s race guard) from day one — #639 Phase 1 (PR #658)
ensured every pre-existing flowsheet row has a NULL stamp; #659
(PR #660) ensured the partial index covers the SELECT so each
batch is an Index Scan rather than a seq scan over the 2.6M-row
table.

Files:
  jobs/flowsheet-metadata-backfill/
    package.json        — workspace registration; "job-type": "one-shot"
    tsconfig.json       — extends base
    tsup.config.ts      — minified ESM bundle
    logger.ts           — Phase-A JSON logger + Sentry init (mirrors
                          jobs/flowsheet-etl/logger.ts; tags repo /
                          tool / step / run_id per #538)
    lml-types.ts        — minimal slice of LookupResponse, widened to
                          cover the artwork fields enrich.ts writes
    lml-fetch.ts        — minimal LML /lookup client; 30s timeout;
                          duplicates apps/backend/services/lml/lml.client.ts
                          to keep the job's build graph independent
    enrich.ts           — single-row 10-column UPDATE + metadata_attempt_at
                          stamp; mirrors enrichment.service.ts shape;
                          inline spacer.gif filter (#638 note 1, until
                          #649 lands); inline Discogs-bio cleaner;
                          synthesized search URLs on no-match
    orchestrate.ts      — id-cursor batch loop, partition mod-N filter,
                          throttle, processRow with error classification
    job.ts              — entrypoint: initLogger, runBackfill,
                          closeDatabaseConnection, closeLogger
  Dockerfile.flowsheet-metadata-backfill  — DB_STATEMENT_TIMEOUT_MS=300000,
                                            DB_SYNCHRONOUS_COMMIT=off,
                                            DB_APPLICATION_NAME tag
  tests/unit/jobs/flowsheet-metadata-backfill/
    enrich.test.ts      — 9 tests pinning: 10-column UPDATE on match,
                          search-URL-only UPDATE on no-match, stamp
                          shape (sql`now()` not Date), idempotency
                          guard, spacer.gif filter, extractArtwork
                          handles three no-match shapes
    orchestrate.test.ts — 13 tests pinning: SELECT carries the
                          canonical filter, id-cursor advances across
                          batches, lml_error counted + enrich NOT
                          called (row stays retryable),
                          resolvePartitionFilter validates inputs,
                          BATCH_SIZE / THROTTLE_MS exposed for tuning
  CLAUDE.md             — monorepo layout table grows the new package

TDD: the 22 new tests went red first against empty modules, then
green as enrich.ts and orchestrate.ts came in. Pinning the lml_error
path explicitly (enrich NOT called) guards the row-stays-retryable
contract that #639 Phase 2's recurring sweep depends on.

Implementation notes from your fresh-eyes review on the issue
(2026-04-29 comments) addressed:
  1. Inline spacer.gif filter — done in enrich.ts; ready to swap
     for #649's shared helper when it lands.
  2. Trace observability gap (job's own lml-fetch.ts, not #646's
     wrapped client) — relying on @sentry/node v10+ undici auto-
     instrumentation; comment on lml-fetch.ts flags the fallback if
     the pilot run (#640) shows propagation breaks.
  3. Concurrent runtime+job stamp race — documented inline in
     orchestrate.ts header; no CAS pattern (last-write-wins is
     correct because both writers produce identical data, and the
     applyEnrichment WHERE narrows by metadata_attempt_at IS NULL
     to make the second write a no-op).
  4. Manual Build & Deploy target — no GHA workflow change needed;
     deploy-base.yml uses Turborepo to detect affected targets
     dynamically, so registering the workspace is sufficient.

Run procedure (deferred until #640 pilot):
  Manual Build & Deploy with target=flowsheet-metadata-backfill,
  then SSH to EC2 and `docker run --rm --env-file .env <image>
  2>&1 | tee log`. LML's Discogs rate budget (50 req/min) is the
  limiting factor across all parallel partitions, not the database.

Out of scope (tracked elsewhere):
  - #639 Phase 2 (drop "job-type": "one-shot" + EC2 cron) — fires
    after #641 plateaus.
  - #640 (pilot run + parallelism scaling decision).
  - #641 (phased rollout — blocked by #649).
  - #642 (close-out tests/docs).
  - #661 (delete obsolete scripts/backfill-metadata.ts) — blocked
    by this PR landing + #641 closing.

Closes #638
jakebromberg added a commit that referenced this pull request Apr 29, 2026
One-shot containerized job at jobs/flowsheet-metadata-backfill/ that
drains the ~1.86M-row historical tail where the LML metadata
enrichment never ran. Adopts the canonical filter
(metadata_attempt_at IS NULL + entry_type='track' + artist_name IS
NOT NULL + 60s race guard) from day one — #639 Phase 1 (PR #658)
ensured every pre-existing flowsheet row has a NULL stamp; #659
(PR #660) ensured the partial index covers the SELECT so each
batch is an Index Scan rather than a seq scan over the 2.6M-row
table.

Files:
  jobs/flowsheet-metadata-backfill/
    package.json        — workspace registration; "job-type": "one-shot"
    tsconfig.json       — extends base
    tsup.config.ts      — minified ESM bundle
    logger.ts           — Phase-A JSON logger + Sentry init (mirrors
                          jobs/flowsheet-etl/logger.ts; tags repo /
                          tool / step / run_id per #538)
    lml-types.ts        — minimal slice of LookupResponse, widened to
                          cover the artwork fields enrich.ts writes
    lml-fetch.ts        — minimal LML /lookup client; 30s timeout;
                          duplicates apps/backend/services/lml/lml.client.ts
                          to keep the job's build graph independent
    enrich.ts           — single-row 10-column UPDATE + metadata_attempt_at
                          stamp; mirrors enrichment.service.ts shape;
                          inline spacer.gif filter (#638 note 1, until
                          #649 lands); inline Discogs-bio cleaner;
                          synthesized search URLs on no-match
    orchestrate.ts      — id-cursor batch loop, partition mod-N filter,
                          throttle, processRow with error classification
    job.ts              — entrypoint: initLogger, runBackfill,
                          closeDatabaseConnection, closeLogger
  Dockerfile.flowsheet-metadata-backfill  — DB_STATEMENT_TIMEOUT_MS=300000,
                                            DB_SYNCHRONOUS_COMMIT=off,
                                            DB_APPLICATION_NAME tag
  tests/unit/jobs/flowsheet-metadata-backfill/
    enrich.test.ts      — 9 tests pinning: 10-column UPDATE on match,
                          search-URL-only UPDATE on no-match, stamp
                          shape (sql`now()` not Date), idempotency
                          guard, spacer.gif filter, extractArtwork
                          handles three no-match shapes
    orchestrate.test.ts — 13 tests pinning: SELECT carries the
                          canonical filter, id-cursor advances across
                          batches, lml_error counted + enrich NOT
                          called (row stays retryable),
                          resolvePartitionFilter validates inputs,
                          BATCH_SIZE / THROTTLE_MS exposed for tuning
  CLAUDE.md             — monorepo layout table grows the new package

TDD: the 22 new tests went red first against empty modules, then
green as enrich.ts and orchestrate.ts came in. Pinning the lml_error
path explicitly (enrich NOT called) guards the row-stays-retryable
contract that #639 Phase 2's recurring sweep depends on.

Implementation notes from your fresh-eyes review on the issue
(2026-04-29 comments) addressed:
  1. Inline spacer.gif filter — done in enrich.ts; ready to swap
     for #649's shared helper when it lands.
  2. Trace observability gap (job's own lml-fetch.ts, not #646's
     wrapped client) — relying on @sentry/node v10+ undici auto-
     instrumentation; comment on lml-fetch.ts flags the fallback if
     the pilot run (#640) shows propagation breaks.
  3. Concurrent runtime+job stamp race — documented inline in
     orchestrate.ts header; no CAS pattern (last-write-wins is
     correct because both writers produce identical data, and the
     applyEnrichment WHERE narrows by metadata_attempt_at IS NULL
     to make the second write a no-op).
  4. Manual Build & Deploy target — no GHA workflow change needed;
     deploy-base.yml uses Turborepo to detect affected targets
     dynamically, so registering the workspace is sufficient.

Run procedure (deferred until #640 pilot):
  Manual Build & Deploy with target=flowsheet-metadata-backfill,
  then SSH to EC2 and `docker run --rm --env-file .env <image>
  2>&1 | tee log`. LML's Discogs rate budget (50 req/min) is the
  limiting factor across all parallel partitions, not the database.

Out of scope (tracked elsewhere):
  - #639 Phase 2 (drop "job-type": "one-shot" + EC2 cron) — fires
    after #641 plateaus.
  - #640 (pilot run + parallelism scaling decision).
  - #641 (phased rollout — blocked by #649).
  - #642 (close-out tests/docs).
  - #661 (delete obsolete scripts/backfill-metadata.ts) — blocked
    by this PR landing + #641 closing.

Closes #638
jakebromberg added a commit that referenced this pull request May 23, 2026
…BS#1022)

The dual-count comparison shipped in PR #1020 traded one un-indexed scan for
another: count(DISTINCT album_id) WHERE metadata_attempt_at IS NOT NULL walks
the opposite partition from the drain partial index (#660), still ~2.6M rows
on a heap scan, still tripped the backend's 5s default statement_timeout on
the 2026-05-22 prod re-run.

Wrap verifyComplete in db.transaction so SET LOCAL statement_timeout actually
scopes (postgres-js auto-commits per execute otherwise). Both SET LOCAL and
the dual-count SELECT execute through the closure-bound tx handle. Timeout is
read from ALBUM_METADATA_BACKFILL_VERIFY_TIMEOUT_MS (default 120000ms),
following the operational-knob pattern from BS#995 / BS#1017 on the same job
family.

Unit tests pin tx-binding by installing a per-call fake-tx with its own
execute spy and asserting db.execute is never called inside the transaction
body — the real failure mode the previous PR missed wasn't call-ordering, it
was the two statements landing on different pooled connections.

Closes #1022.
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.

Partial index on flowsheet.metadata_attempt_at IS NULL for #638 / #639 Phase 2 sweep

1 participant