diff --git a/backend/db/migrate.py b/backend/db/migrate.py index 5a9a1ad2..a7a1d0b3 100644 --- a/backend/db/migrate.py +++ b/backend/db/migrate.py @@ -37,6 +37,26 @@ MIGRATIONS_DIR = Path(__file__).parent / "migrations" +# Index builds size their working set from maintenance_work_mem, and the server +# default is not enough for every migration in this repo. 0039_rag_vector_store +# builds an ivfflat index over VECTOR(768) and needs ~35 MB; Supabase defaults to +# 32 MB, so it dies with +# ProgramLimitExceeded: memory required is 35 MB, maintenance_work_mem is 32 MB +# and — because apply_migration runs each file plus its ledger INSERT in one +# transaction, and run() has no per-file recovery — takes every migration queued +# behind it down too. Staging hit exactly this with 5 files still pending. +# +# Set it PER SESSION rather than per environment. `ALTER DATABASE ... SET` only +# reaches backends started after it, and a pooled connection is often already +# established, so the change appears to do nothing (observed against Supavisor: +# a fresh backend saw the new value while a reused one still reported 32 MB). A +# session-level SET always lands on the connection actually running the DDL. +# +# Transient — this is per-operation memory during an index build, not a +# reservation. Override for a memory-tight instance: +# MIGRATE_MAINTENANCE_WORK_MEM=64MB python -m db.migrate +MAINTENANCE_WORK_MEM = os.environ.get("MIGRATE_MAINTENANCE_WORK_MEM", "128MB") + # Two accepted filename shapes, both sortable and both fixed-width: # NNNN_ legacy sequential prefix (frozen — see below) @@ -115,6 +135,12 @@ def run( baseline: bool = False, ) -> list[str]: """Apply (or baseline-record) all pending migrations. Returns filenames handled.""" + with conn.cursor() as cur: + # Quoted as a literal, not a bound parameter: SET does not accept one. + # The value is operator-supplied config, never request input, and a bad + # value fails loudly here rather than mid-migration. + cur.execute(f"SET maintenance_work_mem = '{MAINTENANCE_WORK_MEM}'") + conn.commit() ensure_tracking_table(conn) applied = applied_filenames(conn) pending = pending_migrations(discover_migrations(migrations_dir), applied) diff --git a/backend/tests/test_migrate_work_mem.py b/backend/tests/test_migrate_work_mem.py new file mode 100644 index 00000000..dee62a06 --- /dev/null +++ b/backend/tests/test_migrate_work_mem.py @@ -0,0 +1,89 @@ +"""run() must raise maintenance_work_mem before applying anything (#510 fallout). + +0039_rag_vector_store builds an ivfflat index over VECTOR(768) and needs ~35 MB. +Supabase defaults maintenance_work_mem to 32 MB, so the migration dies with +"memory required is 35 MB, maintenance_work_mem is 32 MB" — and because +apply_migration wraps each file plus its ledger INSERT in one transaction with +no per-file recovery, everything queued behind it dies too. Staging hit this +with 5 migrations still pending. + +The SET has to happen on the connection that runs the DDL: `ALTER DATABASE` +only reaches backends started after it, and a pooled connection is often +already established. +""" +from pathlib import Path + +import pytest + +from db import migrate + + +class _FakeCursor: + def __init__(self, log: list[str]): + self.log = log + + def execute(self, sql, params=None): + self.log.append(sql if isinstance(sql, str) else str(sql)) + + def fetchall(self): + return [] + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +class _FakeConn: + """Records executed SQL; no pending migrations so run() short-circuits.""" + + def __init__(self): + self.log: list[str] = [] + self.commits = 0 + + def cursor(self): + return _FakeCursor(self.log) + + def commit(self): + self.commits += 1 + + +@pytest.fixture +def empty_dir(tmp_path) -> Path: + return tmp_path + + +def test_run_sets_maintenance_work_mem_before_touching_the_ledger(empty_dir): + """Order matters: an index build inside the very first migration must + already see the raised value.""" + conn = _FakeConn() + + migrate.run(conn, migrations_dir=empty_dir) + + set_stmts = [s for s in conn.log if s.startswith("SET maintenance_work_mem")] + assert set_stmts, f"run() never raised maintenance_work_mem; executed: {conn.log}" + + first_set = conn.log.index(set_stmts[0]) + first_ddl = next( + (i for i, s in enumerate(conn.log) if "schema_migrations" in s), len(conn.log) + ) + assert first_set < first_ddl, "the SET must precede any migration work" + + +def test_the_value_is_configurable_for_a_memory_tight_instance(empty_dir, monkeypatch): + """A small instance must be able to dial this down without editing code.""" + monkeypatch.setattr(migrate, "MAINTENANCE_WORK_MEM", "64MB") + conn = _FakeConn() + + migrate.run(conn, migrations_dir=empty_dir) + + assert any("'64MB'" in s for s in conn.log), conn.log + + +def test_the_default_clears_the_known_requirement(empty_dir): + """0039 needs ~35 MB. The default must exceed that with headroom, or this + fix does not actually fix the migration that motivated it.""" + value = migrate.MAINTENANCE_WORK_MEM + assert value.upper().endswith("MB"), f"expected an MB value, got {value!r}" + assert int(value[:-2]) >= 64, f"{value} leaves no headroom over 0039's ~35 MB"