Files
deer-flow/backend/tests/test_persistence_bootstrap_concurrency.py
T
AnoobFengandGitHub debb0fd161 feat(persistence): wire alembic migrations, bootstrap schema on startup (#3706)
* feat(persistence): wire alembic migrations + bootstrap schema on startup

Closes #3682. Pre-#3658 DBs lack the `runs.token_usage_by_model` column
because alembic was never wired up — startup only ran `create_all`,
which never ALTERs existing tables.

Adds a hybrid bootstrap in FastAPI lifespan (replaces bare `create_all`):
- empty DB     → create_all + stamp head
- legacy DB    → stamp 0001_baseline + upgrade head
- versioned DB → upgrade head

Concurrency: Postgres `pg_advisory_lock` (cross-process); SQLite
per-engine `asyncio.Lock` + 30s `PRAGMA busy_timeout` on both prod and
alembic engines. Column revisions use `safe_add_column` /
`safe_drop_column` idempotent helpers as fallback.

Other bits:
- 0001 baseline (chain root) + 0002 add `runs.token_usage_by_model`
- `include_object` filter so alembic ignores LangGraph checkpointer tables
- `make migrate-rev MSG="..."` for authoring new revisions
  (no migrate/stamp targets — startup is the only execution path)
- Tests: three-branch decision, concurrency, #3682 regression, env
  filter, blocking-IO gate anchor
- CLAUDE.md: new "Schema migrations" section

* fix(style): fix lint error

* perf(persistence): address review feedback on alembic bootstrap

Behavioural fixes
- _SQLITE_LOCKS now keyed via WeakKeyDictionary so id-reuse after GC
  cannot return a stale, loop-bound lock and the cache cannot leak one
  entry per disposed engine.
- safe_add_column compares nullable / server_default against the desired
  column when the name already exists and emits a warning on drift,
  surfacing manual-ALTER workarounds instead of silently no-op'ing.
- _postgres_lock issues SET LOCAL idle_in_transaction_session_timeout=0
  before pg_advisory_lock, so managed Postgres cannot kill the idle
  lock-holding session mid-upgrade and silently release the advisory
  lock.
- legacy branch now backfills missing baseline tables via a restricted
  create_all (Base.metadata.create_all scoped to _BASELINE_TABLE_NAMES).
  Restores pre-#1930 upgraders whose channel_* tables were never
  provisioned, without pre-empting future create_table revisions for
  newly-added models.

Schema parity
- runs.token_usage_by_model gains server_default=text("'{}'") in both
  the ORM model and the 0001_baseline create_table, matching what 0002
  adds via ALTER. create_all and alembic-upgrade paths now produce
  identical column definitions.
- New parity test compares Base.metadata.create_all output against a
  pure alembic upgrade base->head, asserting column-set, nullable, and
  server_default agree across all tables (normalized through the same
  helper safe_add_column's drift check uses).

Guards
- test_baseline_table_names_constant_matches_0001 pins
  _BASELINE_TABLE_NAMES to 0001_baseline.upgrade()'s actual output --
  the constant cannot drift silently when someone edits 0001.
- test_legacy_backfill_skips_non_baseline_tables verifies the restricted
  backfill does not create a phantom table on Base.metadata, modelling
  a future revision that would otherwise collide on op.create_table.

Doc residuals
- Three-branch decision table is now consistent across bootstrap.py
  top docstring, engine.py comment, test module docstring, and
  CLAUDE.md.
- Stale test anchor in blocking_io/test_persistence_engine_sqlite.py
  docstring now points at the real file.

* fix(style): fix lint error

* fix(persistence): close drift detection holes

- _check_column_drift compares column type via a family equivalence
  allowlist ({JSON, JSONB}). Catches the wrong-type workaround
  `TEXT NOT NULL DEFAULT '{}'` that previously slipped through silently,
  while keeping Postgres JSON/JSONB dialect reflection quiet. Reflected
  and desired type are also echoed in every drift warning's payload for
  operator triage.

- Extract _escape_url_for_alembic so bootstrap._alembic_safe_url and
  scripts/_autogen_revision share the ConfigParser % escape rule
  instead of duplicating it.

- backend/README.md: add `make migrate-rev MSG=...` to Commands and a
  Schema Migrations section per the repo's README/CLAUDE.md sync policy.

- test_base_to_dict.py: scope the test ORM class to an isolated MetaData
  so the create_all-vs-alembic parity test (added in the previous
  commit) is not polluted by the phantom table on the full pytest
  session.
2026-06-24 13:57:12 +08:00

151 lines
5.2 KiB
Python

"""Concurrency safety tests for ``bootstrap_schema``.
The contract: N concurrent callers against the same DB always converge to
``alembic_version == head`` without exceptions and without duplicate schema
mutations.
We model concurrency at the *async-task* level here (multiple coroutines
inside one process). SQLite is single-node by deployment, so within-process
serialisation -- which is what the per-engine ``_SQLITE_LOCKS`` entry
provides -- is the realistic boundary. Cross-process serialisation falls
through to SQLite's own write lock + ``PRAGMA busy_timeout`` plus the
idempotent revision helpers.
"""
from __future__ import annotations
import asyncio
from pathlib import Path
import pytest
import sqlalchemy as sa
from sqlalchemy.ext.asyncio import create_async_engine
import deerflow.persistence.models # noqa: F401
from deerflow.persistence import bootstrap as bootstrap_mod
from deerflow.persistence.bootstrap import bootstrap_schema
pytestmark = pytest.mark.asyncio
HEAD = "0002_runs_token_usage"
def _url(tmp_path: Path) -> str:
return f"sqlite+aiosqlite:///{(tmp_path / 'concurrent.db').as_posix()}"
async def _alembic_version(engine) -> str | None:
async with engine.connect() as conn:
row = await conn.execute(sa.text("SELECT version_num FROM alembic_version"))
return row.scalar()
async def _runs_columns(engine) -> set[str]:
async with engine.connect() as conn:
return await conn.run_sync(lambda c: {col["name"] for col in sa.inspect(c).get_columns("runs")})
async def test_two_concurrent_bootstrap_callers_converge(tmp_path: Path) -> None:
engine = create_async_engine(_url(tmp_path))
try:
await asyncio.gather(
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
)
assert await _alembic_version(engine) == HEAD
assert "token_usage_by_model" in await _runs_columns(engine)
finally:
await engine.dispose()
async def test_five_concurrent_bootstrap_callers_converge(tmp_path: Path) -> None:
engine = create_async_engine(_url(tmp_path))
try:
await asyncio.gather(*(bootstrap_schema(engine, backend="sqlite") for _ in range(5)))
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_cancelled_caller_does_not_block_others(tmp_path: Path) -> None:
"""Cancelling one task mid-bootstrap must not strand the lock or the DB.
After the cancel, a subsequent ``bootstrap_schema`` call must still reach
head.
"""
engine = create_async_engine(_url(tmp_path))
try:
task = asyncio.create_task(bootstrap_schema(engine, backend="sqlite"))
# Give the event loop a turn so the task can start; then cancel.
await asyncio.sleep(0)
task.cancel()
# Cancelled task may have raced past the lock; swallow either outcome.
try:
await task
except (asyncio.CancelledError, Exception): # noqa: BLE001
pass
# Lock must be free for the next caller.
await bootstrap_schema(engine, backend="sqlite")
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_late_caller_after_head_is_noop(monkeypatch, tmp_path: Path) -> None:
"""When the first caller leaves the DB at head, the second observes
'versioned' and skips create_all / stamp -- it only runs upgrade head,
which is alembic-no-op.
We use a monkeypatched ``_upgrade`` counter to assert the second caller's
upgrade ran but did no real work (no new revision applied).
"""
engine = create_async_engine(_url(tmp_path))
try:
# First caller: empty branch.
await bootstrap_schema(engine, backend="sqlite")
first_version = await _alembic_version(engine)
assert first_version == HEAD
upgrade_calls: list[str] = []
original_upgrade = bootstrap_mod._upgrade
def counting_upgrade(cfg, rev: str) -> None:
upgrade_calls.append(rev)
original_upgrade(cfg, rev)
monkeypatch.setattr(bootstrap_mod, "_upgrade", counting_upgrade)
# Second caller: versioned branch -> calls _upgrade('head').
await bootstrap_schema(engine, backend="sqlite")
assert upgrade_calls == ["head"]
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_slow_upgrade_does_not_corrupt_concurrent_state(monkeypatch, tmp_path: Path) -> None:
"""Inject a delay into the upgrade path; concurrent callers must still
converge to head with no exceptions."""
engine = create_async_engine(_url(tmp_path))
try:
original_upgrade = bootstrap_mod._upgrade
def slow_upgrade(cfg, rev: str) -> None:
import time # noqa: PLC0415
time.sleep(0.2)
original_upgrade(cfg, rev)
monkeypatch.setattr(bootstrap_mod, "_upgrade", slow_upgrade)
await asyncio.gather(
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
)
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()