diff --git a/CLAUDE.md b/CLAUDE.md index 4cd9394..c47a767 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,6 +110,42 @@ frontend/src/ - **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 +## 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 - 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` diff --git a/backend/alembic.ini b/backend/alembic.ini index 25777f0..8f640c1 100644 --- a/backend/alembic.ini +++ b/backend/alembic.ini @@ -60,7 +60,8 @@ version_path_separator = os # Use os.pathsep. Default configuration used for ne # are written from script.py.mako # 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] diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 0075ed4..324ac60 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -1,3 +1,4 @@ +import os from logging.config import fileConfig from sqlalchemy import engine_from_config @@ -9,6 +10,12 @@ from alembic import context # access to the values within the .ini file in use. 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. # This line sets up loggers basically. if config.config_file_name is not None: diff --git a/backend/app/main.py b/backend/app/main.py index 94af015..8b49822 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -472,11 +472,41 @@ def _acquire_singleton_lock() -> bool: 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 async def lifespan(app: FastAPI): - # Startup — all workers run DDL/seeds (idempotent), only one runs scheduler/backfill - Base.metadata.create_all(bind=engine) - setup_pgvector() + # Startup — all workers gate DDL behind a Postgres advisory lock to avoid deadlocks. + _run_startup_ddl() os.makedirs(settings.UPLOAD_DIR, exist_ok=True) os.makedirs(os.path.join(settings.UPLOAD_DIR, "images"), exist_ok=True) os.makedirs(settings.CHROMA_PERSIST_DIR, exist_ok=True)