fix(obj-2): trim migration 116 to GIST index only — INSERT CTE timed out in deploy #326

Merged
lekss361 merged 1 commit from fix/116-deploy-timeout-hotfix into main 2026-05-17 20:21:59 +00:00
Owner

Hotfix

Deploy завис на migration 116_obj2_multi_feature_matcher.sql:

Applying migration: 116_obj2_multi_feature_matcher.sql
NOTICE: relation "objective_lots_project_name_trgm_idx" already exists, skipping
CREATE INDEX
BEGIN
... timeout

Root cause

Original INSERT (CROSS JOIN LATERAL similarity() на 1056 unmapped DOM.РФ × 320 distinct Objective projects × top-3) — 89M cost (раньше worker через EXPLAIN видел перед kill'ом).

GIST trgm index уже добавился в первом шаге, но INSERT с heavy CTE завис.

Fix

Trim миграция → только CREATE INDEX IF NOT EXISTS. Multi-feature INSERT уже частично применён inline 2026-05-17 (+13 mappings: 129 → 142).

Полный backfill через отдельную миграцию 117 с pre-materialized temp tables (вместо full LATERAL scan) — после EXPLAIN ANALYZE re-tuning.

Why это safe

  • CREATE INDEX IF NOT EXISTS идемпотентен — если index уже есть (как сейчас в проде после первого partial apply), skipped без ошибки
  • INSERT step deferred — no data loss, mapping at 142 already enough для OBJ-3 PR #324 (which is merged)
  • REFRESH MV не нужно — index doesn't change MV

Verify after deploy

SELECT indexname FROM pg_indexes WHERE indexname = 'objective_lots_project_name_trgm_idx';
-- expect 1 row

Part of issue #307 OBJ-2.

## Hotfix Deploy завис на migration `116_obj2_multi_feature_matcher.sql`: ``` Applying migration: 116_obj2_multi_feature_matcher.sql NOTICE: relation "objective_lots_project_name_trgm_idx" already exists, skipping CREATE INDEX BEGIN ... timeout ``` ## Root cause Original INSERT (CROSS JOIN LATERAL `similarity()` на 1056 unmapped DOM.РФ × 320 distinct Objective projects × top-3) — **89M cost** (раньше worker через EXPLAIN видел перед kill'ом). GIST trgm index уже добавился в первом шаге, но INSERT с heavy CTE завис. ## Fix Trim миграция → только `CREATE INDEX IF NOT EXISTS`. Multi-feature INSERT уже частично применён inline 2026-05-17 (+13 mappings: 129 → 142). Полный backfill через **отдельную миграцию 117** с pre-materialized temp tables (вместо full LATERAL scan) — после EXPLAIN ANALYZE re-tuning. ## Why это safe - `CREATE INDEX IF NOT EXISTS` идемпотентен — если index уже есть (как сейчас в проде после первого partial apply), skipped без ошибки - INSERT step deferred — no data loss, mapping at 142 already enough для OBJ-3 PR #324 (which is merged) - REFRESH MV не нужно — index doesn't change MV ## Verify after deploy ```sql SELECT indexname FROM pg_indexes WHERE indexname = 'objective_lots_project_name_trgm_idx'; -- expect 1 row ``` Part of issue #307 OBJ-2.
lekss361 added 1 commit 2026-05-17 20:18:31 +00:00
Original CTE (CROSS JOIN LATERAL similarity на 1056 × 320 unmapped × top-3)
завис на deploy таймауте (89M cost узнан через EXPLAIN database-expert worker).

GIST trgm index — load-bearing для downstream queries, оставлен.

Multi-feature INSERT (composite >= 0.75) уже частично применён inline
2026-05-17 (+13 мапingов: 129 → 142). Полный backfill будет в отдельной
миграции 117 с pre-materialized temp tables для avoidance of full LATERAL
scan.
Author
Owner

Deep Code Review — verdict: APPROVE

Status: APPROVE (hotfix fast-track)
Files: 1 (P0: data/sql/116_obj2_multi_feature_matcher.sql)
Lines: +10 / -91

Diff verified

  • File trimmed to header comment + single statement: CREATE INDEX IF NOT EXISTS objective_lots_project_name_trgm_idx ON objective_lots USING gist (project_name gist_trgm_ops);
  • No BEGIN/COMMIT, no INSERT, no REFRESH MV, no DROP/TRUNCATE/DELETE — only safe DDL remains.
  • Header documents the deferral to migration 117 and references the 13 inline mappings (129→142) — context preserved for future readers.

Re-application safety (critical question answered)

Read .forgejo/workflows/deploy.yml lines 225–248. Apply step:

  1. for sql_file in $(ls data/sql/*.sql | sort) ...
  2. SELECT from _schema_migrations WHERE filename=$fname → if 0, run psql < $sql_file -v ON_ERROR_STOP=on
  3. Only on psql exit 0 does INSERT INTO _schema_migrations mark it applied.

Therefore original 116 (which timed out / never exited cleanly) is NOT marked applied → trimmed 116 will re-run on next deploy.

Lock / impact analysis

  • CREATE INDEX (non-CONCURRENTLY) normally takes AccessExclusiveLock on objective_lots. However, prod deploy log already shows NOTICE: relation "objective_lots_project_name_trgm_idx" already exists, skipping. With IF NOT EXISTS, when index exists this becomes a metadata-only no-op (no rebuild, no exclusive lock on table heap). Net effect on next deploy: instant skip → mark applied → migration loop continues to 117+.
  • Worst-case (if index were somehow dropped between deploys): objective_lots is a small reference table (~few hundred K rows at most for EKB Objective lots scope) — index build is sub-second range. Acceptable.

REFRESH MV removal — safe

Other consumers (backend/app/api/v1/parcels.py, services/site_finder/best_layouts.py, layout_velocity_refresh.py) READ mv_layout_velocity. Since the trimmed migration inserts no new mapping rows, the MV's underlying data is unchanged → refresh is unnecessary. Refresh is also performed routinely by layout_velocity_refresh.py, so even drift would self-heal.

Risk score

  • Risk: Low
  • Reversibility: Trivial (drop index if ever needed; no data mutation to revert)
  • Merge window: anytime — this is the unblock

Why not BLOCK on non-CONCURRENTLY CREATE INDEX

Normally I'd flag CREATE INDEX without CONCURRENTLY on prod, but here:

  1. Deploy log proves index already exists → IF NOT EXISTS path = no lock
  2. Table is small (Objective EKB lots, not partitioned multi-million-row table)
  3. This is hot-path deploy unblock; full backfill (which had the real perf issue) is correctly deferred to a follow-up PR (117) with pre-materialized temp tables.

Merging now.

## Deep Code Review — verdict: APPROVE **Status**: APPROVE (hotfix fast-track) **Files**: 1 (P0: `data/sql/116_obj2_multi_feature_matcher.sql`) **Lines**: +10 / -91 ### Diff verified - File trimmed to header comment + single statement: `CREATE INDEX IF NOT EXISTS objective_lots_project_name_trgm_idx ON objective_lots USING gist (project_name gist_trgm_ops);` - No BEGIN/COMMIT, no INSERT, no REFRESH MV, no DROP/TRUNCATE/DELETE — only safe DDL remains. - Header documents the deferral to migration 117 and references the 13 inline mappings (129→142) — context preserved for future readers. ### Re-application safety (critical question answered) Read `.forgejo/workflows/deploy.yml` lines 225–248. Apply step: 1. `for sql_file in $(ls data/sql/*.sql | sort) ...` 2. SELECT from `_schema_migrations WHERE filename=$fname` → if 0, run `psql < $sql_file -v ON_ERROR_STOP=on` 3. Only on **psql exit 0** does `INSERT INTO _schema_migrations` mark it applied. Therefore original 116 (which timed out / never exited cleanly) is **NOT** marked applied → trimmed 116 will re-run on next deploy. ### Lock / impact analysis - `CREATE INDEX` (non-CONCURRENTLY) normally takes AccessExclusiveLock on `objective_lots`. **However**, prod deploy log already shows `NOTICE: relation "objective_lots_project_name_trgm_idx" already exists, skipping`. With `IF NOT EXISTS`, when index exists this becomes a metadata-only no-op (no rebuild, no exclusive lock on table heap). Net effect on next deploy: instant skip → mark applied → migration loop continues to 117+. - Worst-case (if index were somehow dropped between deploys): `objective_lots` is a small reference table (~few hundred K rows at most for EKB Objective lots scope) — index build is sub-second range. Acceptable. ### REFRESH MV removal — safe Other consumers (`backend/app/api/v1/parcels.py`, `services/site_finder/best_layouts.py`, `layout_velocity_refresh.py`) READ `mv_layout_velocity`. Since the trimmed migration inserts no new mapping rows, the MV's underlying data is unchanged → refresh is unnecessary. Refresh is also performed routinely by `layout_velocity_refresh.py`, so even drift would self-heal. ### Risk score - **Risk**: Low - **Reversibility**: Trivial (drop index if ever needed; no data mutation to revert) - **Merge window**: anytime — this *is* the unblock ### Why not BLOCK on non-CONCURRENTLY CREATE INDEX Normally I'd flag CREATE INDEX without CONCURRENTLY on prod, but here: 1. Deploy log proves index already exists → IF NOT EXISTS path = no lock 2. Table is small (Objective EKB lots, not partitioned multi-million-row table) 3. This is hot-path deploy unblock; full backfill (which had the real perf issue) is correctly deferred to a follow-up PR (117) with pre-materialized temp tables. Merging now.
lekss361 merged commit 2bd80c5719 into main 2026-05-17 20:21:59 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#326
No description provided.