Wire Alembic to live Postgres and fix startup DDL race
- alembic.ini: remove hardcoded sqlite URL - alembic/env.py: inject DATABASE_URL from container env - main.py: serialize Base.metadata.create_all() + setup_pgvector() behind a Postgres advisory lock (_run_startup_ddl). Previously all 4 uvicorn workers ran the DDL in parallel and occasionally deadlocked each other on ALTER TABLE ordering, killing one worker at startup. - CLAUDE.md: add Database migrations (Alembic) section DB was stamped at 9bac7bf02e38; no schema changes in this commit.
This commit is contained in:
parent
b39e09f393
commit
0f3a267fb6
4 changed files with 78 additions and 4 deletions
36
CLAUDE.md
36
CLAUDE.md
|
|
@ -110,6 +110,42 @@ frontend/src/
|
||||||
- **useEffect dependencies**: Use `.join(',')` on arrays to create a stable string key (e.g., `tagIdsKey`, `catIdsKey`)
|
- **useEffect dependencies**: Use `.join(',')` on arrays to create a stable string key (e.g., `tagIdsKey`, `catIdsKey`)
|
||||||
- **Admin page data refresh**: `loadData(false)` — the `false` param skips the loading spinner on re-fetch after actions
|
- **Admin page data refresh**: `loadData(false)` — the `false` param skips the loading spinner on re-fetch after actions
|
||||||
|
|
||||||
|
## Database migrations (Alembic)
|
||||||
|
|
||||||
|
**What's a migration?** A migration is a small, ordered change to the database schema — adding a column, renaming a table, changing a type. Each change lives in a Python file under `backend/alembic/versions/`. Alembic tracks which ones have been applied in an `alembic_version` table inside Postgres, so it knows what's new next time you run it.
|
||||||
|
|
||||||
|
**Why it exists here:** until now, schema was created via `Base.metadata.create_all()` in `main.py:478`, which only creates *missing tables* — it never alters existing ones. Every column change required manual `ALTER TABLE`. Alembic makes schema changes versioned, reversible, and reproducible across environments.
|
||||||
|
|
||||||
|
**Current setup**
|
||||||
|
- `alembic.ini` contains no hardcoded URL; `alembic/env.py` injects `DATABASE_URL` from the container's env.
|
||||||
|
- Live DB is stamped at revision `9bac7bf02e38` (the latest in `alembic/versions/`).
|
||||||
|
- `Base.metadata.create_all()` remains in place as a fallback for fresh deploys — **don't remove it** without first generating a complete baseline migration from the live schema.
|
||||||
|
|
||||||
|
**Developer workflow**
|
||||||
|
```bash
|
||||||
|
# where am I?
|
||||||
|
docker compose exec backend alembic current
|
||||||
|
docker compose exec backend alembic heads
|
||||||
|
|
||||||
|
# create a new migration (auto-diff model vs live DB)
|
||||||
|
docker compose exec backend alembic revision --autogenerate -m "add some column"
|
||||||
|
# ↑ review the generated file under backend/alembic/versions/ before applying
|
||||||
|
|
||||||
|
# apply pending migrations
|
||||||
|
docker compose exec backend alembic upgrade head
|
||||||
|
|
||||||
|
# roll back the last one
|
||||||
|
docker compose exec backend alembic downgrade -1
|
||||||
|
```
|
||||||
|
|
||||||
|
**When to write one** — any schema change: new column, dropped column, renamed field, new table, altered index, new FK. Model edit → migration → apply → commit both together.
|
||||||
|
|
||||||
|
**Gotchas**
|
||||||
|
- Migrations run as a normal transaction. A failed migration rolls back cleanly.
|
||||||
|
- `--autogenerate` doesn't catch: server_default changes, CHECK constraints, enum value additions, data migrations. Hand-edit the file when needed.
|
||||||
|
- After applying a new migration in dev, rebuild the backend image (`docker compose build backend celery`) so it ships with the migration file baked in.
|
||||||
|
- The `alembic_version` table should only ever have one row. If you see multiple, you have branched heads — run `alembic merge` to reconcile.
|
||||||
|
|
||||||
## What NOT to do
|
## What NOT to do
|
||||||
- Don't add `from sqlalchemy import text as X` inside functions — import at module top only
|
- Don't add `from sqlalchemy import text as X` inside functions — import at module top only
|
||||||
- Don't use `Question.quiz_id` — it's `Question.source_quiz_id`
|
- Don't use `Question.quiz_id` — it's `Question.source_quiz_id`
|
||||||
|
|
|
||||||
|
|
@ -60,7 +60,8 @@ version_path_separator = os # Use os.pathsep. Default configuration used for ne
|
||||||
# are written from script.py.mako
|
# are written from script.py.mako
|
||||||
# output_encoding = utf-8
|
# output_encoding = utf-8
|
||||||
|
|
||||||
sqlalchemy.url = sqlite:///./quiz.db
|
# sqlalchemy.url is intentionally not set here.
|
||||||
|
# The URL is injected from the DATABASE_URL environment variable in alembic/env.py.
|
||||||
|
|
||||||
|
|
||||||
[post_write_hooks]
|
[post_write_hooks]
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,4 @@
|
||||||
|
import os
|
||||||
from logging.config import fileConfig
|
from logging.config import fileConfig
|
||||||
|
|
||||||
from sqlalchemy import engine_from_config
|
from sqlalchemy import engine_from_config
|
||||||
|
|
@ -9,6 +10,12 @@ from alembic import context
|
||||||
# access to the values within the .ini file in use.
|
# access to the values within the .ini file in use.
|
||||||
config = context.config
|
config = context.config
|
||||||
|
|
||||||
|
# Inject the live database URL from the app's environment so alembic talks to
|
||||||
|
# the same Postgres as the running backend (alembic.ini no longer hardcodes it).
|
||||||
|
_db_url = os.environ.get("DATABASE_URL")
|
||||||
|
if _db_url:
|
||||||
|
config.set_main_option("sqlalchemy.url", _db_url)
|
||||||
|
|
||||||
# Interpret the config file for Python logging.
|
# Interpret the config file for Python logging.
|
||||||
# This line sets up loggers basically.
|
# This line sets up loggers basically.
|
||||||
if config.config_file_name is not None:
|
if config.config_file_name is not None:
|
||||||
|
|
|
||||||
|
|
@ -472,11 +472,41 @@ def _acquire_singleton_lock() -> bool:
|
||||||
return True # Redis unavailable — assume single worker, run everything
|
return True # Redis unavailable — assume single worker, run everything
|
||||||
|
|
||||||
|
|
||||||
|
# Stable int64 for pg_advisory_lock — arbitrary but must not collide with other uses.
|
||||||
|
_STARTUP_DDL_LOCK_KEY = 8472931
|
||||||
|
|
||||||
|
|
||||||
|
def _run_startup_ddl():
|
||||||
|
"""Serialize startup DDL across uvicorn workers using a Postgres advisory lock.
|
||||||
|
|
||||||
|
Without this, N workers run ALTER TABLE / CREATE TABLE in parallel at startup
|
||||||
|
and occasionally acquire AccessExclusiveLocks in different orders, tripping
|
||||||
|
Postgres's deadlock detector and killing one worker. The DDL itself is
|
||||||
|
idempotent (IF NOT EXISTS / create_all), so the losing worker runs it again
|
||||||
|
as a no-op after the winner releases the lock.
|
||||||
|
"""
|
||||||
|
from sqlalchemy import text
|
||||||
|
import logging
|
||||||
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# The advisory lock is session-scoped, held for the lifetime of this connection.
|
||||||
|
with engine.connect() as lock_conn:
|
||||||
|
log.info("Acquiring startup DDL advisory lock...")
|
||||||
|
lock_conn.execute(text("SELECT pg_advisory_lock(:k)"), {"k": _STARTUP_DDL_LOCK_KEY})
|
||||||
|
lock_conn.commit()
|
||||||
|
try:
|
||||||
|
Base.metadata.create_all(bind=engine)
|
||||||
|
setup_pgvector()
|
||||||
|
finally:
|
||||||
|
lock_conn.execute(text("SELECT pg_advisory_unlock(:k)"), {"k": _STARTUP_DDL_LOCK_KEY})
|
||||||
|
lock_conn.commit()
|
||||||
|
log.info("Startup DDL complete.")
|
||||||
|
|
||||||
|
|
||||||
@asynccontextmanager
|
@asynccontextmanager
|
||||||
async def lifespan(app: FastAPI):
|
async def lifespan(app: FastAPI):
|
||||||
# Startup — all workers run DDL/seeds (idempotent), only one runs scheduler/backfill
|
# Startup — all workers gate DDL behind a Postgres advisory lock to avoid deadlocks.
|
||||||
Base.metadata.create_all(bind=engine)
|
_run_startup_ddl()
|
||||||
setup_pgvector()
|
|
||||||
os.makedirs(settings.UPLOAD_DIR, exist_ok=True)
|
os.makedirs(settings.UPLOAD_DIR, exist_ok=True)
|
||||||
os.makedirs(os.path.join(settings.UPLOAD_DIR, "images"), exist_ok=True)
|
os.makedirs(os.path.join(settings.UPLOAD_DIR, "images"), exist_ok=True)
|
||||||
os.makedirs(settings.CHROMA_PERSIST_DIR, exist_ok=True)
|
os.makedirs(settings.CHROMA_PERSIST_DIR, exist_ok=True)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue