fix(db): re-running 0095/0096 rebuilds an invalid leftover index
A concurrent build that failed part-way leaves an INVALID index under its name; IF NOT EXISTS kept it on the re-run, and 0095 then dropped the old channel_id index, leaving none usable. The leftover is now dropped and built again, and the old index goes only once the new one is valid.
This commit is contained in:
@@ -17,10 +17,13 @@ cascade, the sync upsert, channel detail) rides this one. The old index is dropp
|
||||
tests/test_channel_list_index_only.py holds the queries and this column list together.
|
||||
|
||||
CONCURRENTLY (autocommit_block) builds it without blocking a live prod. IF (NOT) EXISTS keeps the drift
|
||||
test's downgrade → re-upgrade round trip idempotent.
|
||||
test's downgrade → re-upgrade round trip idempotent. A re-run after a build that failed part-way rebuilds
|
||||
the INVALID leftover, and the old index is dropped only once the new one is valid: a re-run that kept the
|
||||
leftover and dropped the old one left `videos` with no usable channel_id index (E31 S5 review).
|
||||
"""
|
||||
|
||||
from alembic import op
|
||||
from sqlalchemy import text
|
||||
|
||||
revision: str = "0095_videos_channel_cover_index"
|
||||
down_revision: str | None = "0094_plex_genres_hu_everywhere"
|
||||
@@ -28,20 +31,49 @@ branch_labels: str | None = None
|
||||
depends_on: str | None = None
|
||||
|
||||
|
||||
def _valid(name: str) -> bool | None:
|
||||
"""The index's `indisvalid`, or None when there is no index of that name."""
|
||||
row = op.get_bind().execute(
|
||||
text(
|
||||
"SELECT i.indisvalid FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid "
|
||||
"WHERE c.relname = :name"
|
||||
),
|
||||
{"name": name},
|
||||
).first()
|
||||
return None if row is None else bool(row[0])
|
||||
|
||||
|
||||
def _build(name: str, ddl: str) -> None:
|
||||
"""CREATE INDEX CONCURRENTLY `name` (`ddl` is what follows the name), re-runnable. A build that
|
||||
failed part-way leaves an INVALID index under the name, and `IF NOT EXISTS` would keep it: the
|
||||
planner never uses it, and nothing says so (E31 S5 review). Such a leftover is dropped and built
|
||||
again."""
|
||||
if _valid(name) is False:
|
||||
op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}")
|
||||
op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} {ddl}")
|
||||
|
||||
|
||||
def _swap(build: str, ddl: str, drop: str) -> None:
|
||||
_build(build, ddl)
|
||||
if not _valid(build):
|
||||
raise RuntimeError(f"{build} did not build valid; {drop} is kept")
|
||||
op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {drop}")
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute(
|
||||
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_pub_cover "
|
||||
_swap(
|
||||
"ix_videos_channel_pub_cover",
|
||||
"ON videos (channel_id, published_at DESC NULLS LAST, id) "
|
||||
"INCLUDE (duration_seconds, unavailable_since, via_explore, is_short, live_status)"
|
||||
"INCLUDE (duration_seconds, unavailable_since, via_explore, is_short, live_status)",
|
||||
"ix_videos_channel_pub_desc",
|
||||
)
|
||||
op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_pub_desc")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute(
|
||||
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_pub_desc "
|
||||
"ON videos (channel_id, published_at DESC NULLS LAST, id ASC)"
|
||||
_swap(
|
||||
"ix_videos_channel_pub_desc",
|
||||
"ON videos (channel_id, published_at DESC NULLS LAST, id ASC)",
|
||||
"ix_videos_channel_pub_cover",
|
||||
)
|
||||
op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_pub_cover")
|
||||
|
||||
@@ -28,6 +28,7 @@ test's downgrade → re-upgrade round trip idempotent.
|
||||
"""
|
||||
|
||||
from alembic import op
|
||||
from sqlalchemy import text
|
||||
|
||||
revision: str = "0096_videos_sort_indexes"
|
||||
down_revision: str | None = "0095_videos_channel_cover_index"
|
||||
@@ -47,10 +48,32 @@ INDEXES = {
|
||||
}
|
||||
|
||||
|
||||
def _valid(name: str) -> bool | None:
|
||||
"""The index's `indisvalid`, or None when there is no index of that name."""
|
||||
row = op.get_bind().execute(
|
||||
text(
|
||||
"SELECT i.indisvalid FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid "
|
||||
"WHERE c.relname = :name"
|
||||
),
|
||||
{"name": name},
|
||||
).first()
|
||||
return None if row is None else bool(row[0])
|
||||
|
||||
|
||||
def _build(name: str, ddl: str) -> None:
|
||||
"""CREATE INDEX CONCURRENTLY `name` (`ddl` is what follows the name), re-runnable. A build that
|
||||
failed part-way leaves an INVALID index under the name, and `IF NOT EXISTS` would keep it: the
|
||||
planner never uses it, and nothing says so (E31 S5 review). Such a leftover is dropped and built
|
||||
again."""
|
||||
if _valid(name) is False:
|
||||
op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}")
|
||||
op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} {ddl}")
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
with op.get_context().autocommit_block():
|
||||
for name, columns in INDEXES.items():
|
||||
op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} ON videos ({columns})")
|
||||
_build(name, f"ON videos ({columns})")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
"""A concurrent index build that failed part-way, then re-run (E31 S5 review), DB-lane.
|
||||
|
||||
CREATE INDEX CONCURRENTLY that fails leaves an INVALID index under its name, and alembic_version is not
|
||||
stamped, so the next start runs the migration again. `IF NOT EXISTS` matched the name and kept the
|
||||
leftover, which the planner never uses; 0095 then dropped the old channel_id index, leaving `videos`
|
||||
with no usable one. A re-run now rebuilds the leftover and drops the old index only once the new one is
|
||||
valid.
|
||||
"""
|
||||
|
||||
from alembic import command
|
||||
from alembic.config import Config
|
||||
from sqlalchemy import text
|
||||
|
||||
from app.db import engine
|
||||
|
||||
|
||||
def _index(name: str) -> tuple[bool, str] | None:
|
||||
with engine.connect() as conn:
|
||||
row = conn.execute(
|
||||
text(
|
||||
"SELECT i.indisvalid, pg_get_indexdef(i.indexrelid) FROM pg_index i "
|
||||
"JOIN pg_class c ON c.oid = i.indexrelid WHERE c.relname = :name"
|
||||
),
|
||||
{"name": name},
|
||||
).first()
|
||||
return None if row is None else (bool(row[0]), row[1])
|
||||
|
||||
|
||||
def _leave_invalid(name: str) -> None:
|
||||
"""What a build that failed part-way leaves behind: the name taken, the index INVALID."""
|
||||
with engine.connect().execution_options(isolation_level="AUTOCOMMIT") as conn:
|
||||
conn.execute(text(f"CREATE INDEX {name} ON videos (id)"))
|
||||
conn.execute(
|
||||
text(f"UPDATE pg_index SET indisvalid = false WHERE indexrelid = '{name}'::regclass")
|
||||
)
|
||||
|
||||
|
||||
def test_a_rerun_rebuilds_an_invalid_leftover_before_dropping_the_old_index(db):
|
||||
db.rollback() # CONCURRENTLY waits for every open transaction, this session's too
|
||||
cfg = Config("alembic.ini")
|
||||
command.downgrade(cfg, "0094_plex_genres_hu_everywhere")
|
||||
try:
|
||||
_leave_invalid("ix_videos_channel_pub_cover")
|
||||
_leave_invalid("ix_videos_views_desc")
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
|
||||
cover = _index("ix_videos_channel_pub_cover")
|
||||
assert cover is not None and cover[0], cover
|
||||
assert "INCLUDE (duration_seconds" in cover[1]
|
||||
assert _index("ix_videos_channel_pub_desc") is None
|
||||
views = _index("ix_videos_views_desc")
|
||||
assert views is not None and views[0] and "view_count DESC NULLS LAST" in views[1], views
|
||||
Reference in New Issue
Block a user