Schema reconciliation + index hygiene, and the weekly base-image refresh #243
@@ -0,0 +1,120 @@
|
|||||||
|
"""Index the seven unindexed FKs; drop the seven redundant indexes (#3300, #3301).
|
||||||
|
|
||||||
|
Found by a structural sweep of the deployed schema done AFTER 0088 brought the
|
||||||
|
models and the migration chain into exact agreement. That agreement is what
|
||||||
|
0088 achieved, and it is worth being precise about what it does NOT prove: a
|
||||||
|
models-vs-chain diff shows the two describe the same schema. It says nothing
|
||||||
|
about whether that schema is right. Everything here was wrong in BOTH, which is
|
||||||
|
exactly the class of problem the reconciliation could not see.
|
||||||
|
|
||||||
|
## Added: seven FK indexes
|
||||||
|
|
||||||
|
`image_tag.tag_id` is the one that matters. The table's only index is
|
||||||
|
PRIMARY KEY (image_record_id, tag_id), which leads with the wrong column for
|
||||||
|
the two hottest things done with it:
|
||||||
|
|
||||||
|
* the gallery's tag filter — services/tag_query.py builds
|
||||||
|
`image_tag.c.tag_id == tid` (and `.in_(tids)`) on every tag-scoped browse;
|
||||||
|
* ON DELETE CASCADE from `tag` — deleting or merging a tag makes Postgres
|
||||||
|
find that tag's rows before it can remove them.
|
||||||
|
|
||||||
|
Both had to scan the largest table in the schema. The other six are the same
|
||||||
|
shape on much smaller tables; `presentation_review.tag_id` is the notable one,
|
||||||
|
since it also CASCADEs.
|
||||||
|
|
||||||
|
## Dropped: seven redundant indexes
|
||||||
|
|
||||||
|
`ix_image_record_sha256` was an exact duplicate. A UNIQUE constraint builds its
|
||||||
|
own index, so `uq_image_record_sha256` already covered the column and
|
||||||
|
`image_record` carried two btrees on `sha256` — on the highest-insert-rate
|
||||||
|
table in the system.
|
||||||
|
|
||||||
|
The other six are single-column indexes that a later composite superseded
|
||||||
|
without the narrow one being retired. A btree on (a, b) already serves lookups
|
||||||
|
on `a`, so each was pure write amplification. `task_run` and `backup_run` are
|
||||||
|
append-heavy operational logs, which is where that cost lands hardest.
|
||||||
|
|
||||||
|
Note for anyone reading 0088 next to this: 0088 deliberately taught the models
|
||||||
|
to declare BOTH sha256 indexes, so they would describe reality. That was right.
|
||||||
|
This migration changes the reality instead, and the models change with it.
|
||||||
|
|
||||||
|
## CONCURRENTLY, and why this migration has no transaction
|
||||||
|
|
||||||
|
`CREATE INDEX` takes an ACCESS EXCLUSIVE lock for the whole build, which on
|
||||||
|
`image_tag` means stalling every write for as long as it takes. CONCURRENTLY
|
||||||
|
builds without blocking writers, at the cost of two table passes and an
|
||||||
|
inability to run inside a transaction — hence `autocommit_block()`.
|
||||||
|
|
||||||
|
The consequence to know about: this migration is NOT atomic. If it fails
|
||||||
|
partway, the work already done stays done. Every statement is therefore written
|
||||||
|
IF NOT EXISTS / IF EXISTS so that re-running it after a failure is safe rather
|
||||||
|
than an error.
|
||||||
|
|
||||||
|
A failed CONCURRENTLY build also leaves an INVALID index behind — it is not
|
||||||
|
used by the planner and not repaired automatically. Find them with:
|
||||||
|
|
||||||
|
SELECT c.relname FROM pg_index i
|
||||||
|
JOIN pg_class c ON c.oid = i.indexrelid
|
||||||
|
WHERE NOT i.indisvalid;
|
||||||
|
|
||||||
|
Drop what that returns and re-run; nothing else is needed.
|
||||||
|
|
||||||
|
Revision ID: 0089
|
||||||
|
Revises: 0088
|
||||||
|
Create Date: 2026-08-31
|
||||||
|
|
||||||
|
"""
|
||||||
|
from typing import Sequence, Union
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
revision: str = "0089"
|
||||||
|
down_revision: Union[str, None] = "0088"
|
||||||
|
branch_labels: Union[str, Sequence[str], None] = None
|
||||||
|
depends_on: Union[str, Sequence[str], None] = None
|
||||||
|
|
||||||
|
# (index name, table, column) — names match what the models render under
|
||||||
|
# base.py's naming convention, so autogenerate stays quiet after this.
|
||||||
|
MISSING_FK_INDEXES = (
|
||||||
|
("ix_image_tag_tag_id", "image_tag", "tag_id"),
|
||||||
|
("ix_presentation_review_tag_id", "presentation_review", "tag_id"),
|
||||||
|
("ix_presentation_review_conflict_tag_id", "presentation_review", "conflict_tag_id"),
|
||||||
|
("ix_import_task_result_image_id", "import_task", "result_image_id"),
|
||||||
|
("ix_external_link_attachment_id", "external_link", "attachment_id"),
|
||||||
|
("ix_character_prototype_region_id", "character_prototype", "region_id"),
|
||||||
|
("ix_backup_run_restored_from_id", "backup_run", "restored_from_id"),
|
||||||
|
)
|
||||||
|
|
||||||
|
# (index name, table, column) — redundant; the second element of each pair in
|
||||||
|
# the docstring is what still covers the column after the drop.
|
||||||
|
REDUNDANT_INDEXES = (
|
||||||
|
("ix_image_record_sha256", "image_record", "sha256"),
|
||||||
|
("ix_backup_run_kind", "backup_run", "kind"),
|
||||||
|
("ix_backup_run_status", "backup_run", "status"),
|
||||||
|
("ix_task_run_queue", "task_run", "queue"),
|
||||||
|
("ix_task_run_status", "task_run", "status"),
|
||||||
|
("ix_task_run_task_name", "task_run", "task_name"),
|
||||||
|
("ix_external_link_post_id", "external_link", "post_id"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
with op.get_context().autocommit_block():
|
||||||
|
for name, table, column in MISSING_FK_INDEXES:
|
||||||
|
op.execute(
|
||||||
|
f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} "
|
||||||
|
f"ON {table} ({column})"
|
||||||
|
)
|
||||||
|
for name, _table, _column in REDUNDANT_INDEXES:
|
||||||
|
op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}")
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
with op.get_context().autocommit_block():
|
||||||
|
for name, table, column in REDUNDANT_INDEXES:
|
||||||
|
op.execute(
|
||||||
|
f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} "
|
||||||
|
f"ON {table} ({column})"
|
||||||
|
)
|
||||||
|
for name, _table, _column in MISSING_FK_INDEXES:
|
||||||
|
op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}")
|
||||||
@@ -37,9 +37,12 @@ class BackupRun(Base):
|
|||||||
Index("ix_backup_run_tag_partial", "tag", postgresql_where=text("tag IS NOT NULL")),
|
Index("ix_backup_run_tag_partial", "tag", postgresql_where=text("tag IS NOT NULL")),
|
||||||
)
|
)
|
||||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||||
kind: Mapped[str] = mapped_column(String(16), nullable=False, index=True)
|
# No index=True: ix_backup_run_kind_started (above) already leads with
|
||||||
|
# `kind`, so a single-column index on it was pure write cost (#3301).
|
||||||
|
kind: Mapped[str] = mapped_column(String(16), nullable=False)
|
||||||
status: Mapped[str] = mapped_column(
|
status: Mapped[str] = mapped_column(
|
||||||
String(16), nullable=False, default="pending", index=True,
|
# No index=True — ix_backup_run_status_finished leads with `status`.
|
||||||
|
String(16), nullable=False, default="pending",
|
||||||
server_default="pending",
|
server_default="pending",
|
||||||
)
|
)
|
||||||
tag: Mapped[str | None] = mapped_column(String(64), nullable=True, index=True)
|
tag: Mapped[str | None] = mapped_column(String(64), nullable=True, index=True)
|
||||||
@@ -57,7 +60,9 @@ class BackupRun(Base):
|
|||||||
manifest: Mapped[dict] = mapped_column(
|
manifest: Mapped[dict] = mapped_column(
|
||||||
JSON, nullable=False, default=dict, server_default="{}",
|
JSON, nullable=False, default=dict, server_default="{}",
|
||||||
)
|
)
|
||||||
|
# Self-referential FK, unindexed until 0089 (#3300): SET NULL has to find
|
||||||
|
# the rows pointing at a deleted run before it can null them.
|
||||||
restored_from_id: Mapped[int | None] = mapped_column(
|
restored_from_id: Mapped[int | None] = mapped_column(
|
||||||
ForeignKey("backup_run.id", ondelete="SET NULL"),
|
ForeignKey("backup_run.id", ondelete="SET NULL"),
|
||||||
nullable=True,
|
nullable=True, index=True,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -40,8 +40,10 @@ class CharacterPrototype(Base):
|
|||||||
)
|
)
|
||||||
# Provenance: the region this vector was copied from. SET NULL so pruning a
|
# Provenance: the region this vector was copied from. SET NULL so pruning a
|
||||||
# region doesn't delete the prototype mid-cycle (the next refresh reconciles).
|
# region doesn't delete the prototype mid-cycle (the next refresh reconciles).
|
||||||
|
# index=True added in 0089 — the FK was unindexed (#3300).
|
||||||
region_id: Mapped[int | None] = mapped_column(
|
region_id: Mapped[int | None] = mapped_column(
|
||||||
ForeignKey("image_region.id", ondelete="SET NULL"), nullable=True
|
ForeignKey("image_region.id", ondelete="SET NULL"), nullable=True,
|
||||||
|
index=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -57,11 +57,15 @@ class ExternalLink(Base):
|
|||||||
# — the same file linked twice in a post collapses to one row.
|
# — the same file linked twice in a post collapses to one row.
|
||||||
Index("uq_external_link_post_url", "post_id", "url", unique=True),
|
Index("uq_external_link_post_url", "post_id", "url", unique=True),
|
||||||
Index("ix_external_link_status", "status"),
|
Index("ix_external_link_status", "status"),
|
||||||
|
# Unindexed FK (#3300).
|
||||||
|
Index("ix_external_link_attachment_id", "attachment_id"),
|
||||||
)
|
)
|
||||||
|
|
||||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||||
|
# No index=True: uq_external_link_post_url (post_id, url) already leads
|
||||||
|
# with post_id (#3301).
|
||||||
post_id: Mapped[int] = mapped_column(
|
post_id: Mapped[int] = mapped_column(
|
||||||
ForeignKey("post.id", ondelete="CASCADE"), nullable=False, index=True
|
ForeignKey("post.id", ondelete="CASCADE"), nullable=False
|
||||||
)
|
)
|
||||||
artist_id: Mapped[int | None] = mapped_column(
|
artist_id: Mapped[int | None] = mapped_column(
|
||||||
ForeignKey("artist.id", ondelete="SET NULL"), nullable=True, index=True
|
ForeignKey("artist.id", ondelete="SET NULL"), nullable=True, index=True
|
||||||
|
|||||||
@@ -59,9 +59,11 @@ class ImageRecord(Base):
|
|||||||
|
|
||||||
# On-disk identity
|
# On-disk identity
|
||||||
path: Mapped[str] = mapped_column(Text, nullable=False, unique=True)
|
path: Mapped[str] = mapped_column(Text, nullable=False, unique=True)
|
||||||
# index=True only: the UNIQUE half is the named constraint in
|
# Neither unique= nor index=: uq_image_record_sha256 in __table_args__
|
||||||
# __table_args__ above, matching what 0001 actually created.
|
# above creates its own index, and the separate ix_image_record_sha256
|
||||||
sha256: Mapped[str] = mapped_column(String(64), nullable=False, index=True)
|
# that 0001 also built was an exact duplicate of it — dropped in 0089
|
||||||
|
# (#3301). Lookups by sha256 use the constraint's index.
|
||||||
|
sha256: Mapped[str] = mapped_column(String(64), nullable=False)
|
||||||
phash: Mapped[str | None] = mapped_column(String(32), nullable=True, index=True)
|
phash: Mapped[str | None] = mapped_column(String(32), nullable=True, index=True)
|
||||||
size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False)
|
size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False)
|
||||||
mime: Mapped[str] = mapped_column(String(64), nullable=False)
|
mime: Mapped[str] = mapped_column(String(64), nullable=False)
|
||||||
|
|||||||
@@ -31,6 +31,8 @@ class ImportTask(Base):
|
|||||||
|
|
||||||
__table_args__ = (
|
__table_args__ = (
|
||||||
Index("ix_import_task_created_at_desc", text("created_at DESC")),
|
Index("ix_import_task_created_at_desc", text("created_at DESC")),
|
||||||
|
# Unindexed FK (#3300).
|
||||||
|
Index("ix_import_task_result_image_id", "result_image_id"),
|
||||||
)
|
)
|
||||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||||
batch_id: Mapped[int] = mapped_column(
|
batch_id: Mapped[int] = mapped_column(
|
||||||
|
|||||||
@@ -23,6 +23,10 @@ class PresentationReview(Base):
|
|||||||
|
|
||||||
__table_args__ = (
|
__table_args__ = (
|
||||||
Index("ix_presentation_review_resolved_at", "resolved_at"),
|
Index("ix_presentation_review_resolved_at", "resolved_at"),
|
||||||
|
# Both FKs to tag were unindexed (#3300); tag_id CASCADEs, so a tag
|
||||||
|
# delete had to scan this table to find its rows.
|
||||||
|
Index("ix_presentation_review_tag_id", "tag_id"),
|
||||||
|
Index("ix_presentation_review_conflict_tag_id", "conflict_tag_id"),
|
||||||
)
|
)
|
||||||
image_record_id: Mapped[int] = mapped_column(
|
image_record_id: Mapped[int] = mapped_column(
|
||||||
ForeignKey("image_record.id", ondelete="CASCADE"), primary_key=True
|
ForeignKey("image_record.id", ondelete="CASCADE"), primary_key=True
|
||||||
|
|||||||
@@ -71,6 +71,12 @@ image_tag = Table(
|
|||||||
Column("tag_id", ForeignKey("tag.id", ondelete="CASCADE"), primary_key=True),
|
Column("tag_id", ForeignKey("tag.id", ondelete="CASCADE"), primary_key=True),
|
||||||
Column("source", String(32), nullable=False, default="manual", server_default="manual"),
|
Column("source", String(32), nullable=False, default="manual", server_default="manual"),
|
||||||
Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),
|
Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),
|
||||||
|
# The PK is (image_record_id, tag_id), which leads with the WRONG column
|
||||||
|
# for the two things that matter most here (#3300): the gallery's tag
|
||||||
|
# filter (tag_query.py builds `image_tag.c.tag_id == tid`) and the
|
||||||
|
# ON DELETE CASCADE from tag, which has to find a tag's rows to remove
|
||||||
|
# them. Without this index both scan the largest table in the schema.
|
||||||
|
Index("ix_image_tag_tag_id", "tag_id"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -35,8 +35,10 @@ class TaskRun(Base):
|
|||||||
celery_task_id: Mapped[str] = mapped_column(
|
celery_task_id: Mapped[str] = mapped_column(
|
||||||
String(64), nullable=False, index=True,
|
String(64), nullable=False, index=True,
|
||||||
)
|
)
|
||||||
queue: Mapped[str] = mapped_column(String(32), nullable=False, index=True)
|
# Neither carries index=True: ix_task_run_queue_started and
|
||||||
task_name: Mapped[str] = mapped_column(String(128), nullable=False, index=True)
|
# ix_task_run_name_started already lead with these columns (#3301).
|
||||||
|
queue: Mapped[str] = mapped_column(String(32), nullable=False)
|
||||||
|
task_name: Mapped[str] = mapped_column(String(128), nullable=False)
|
||||||
target_id: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
target_id: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
||||||
started_at: Mapped[datetime] = mapped_column(
|
started_at: Mapped[datetime] = mapped_column(
|
||||||
DateTime(timezone=True), nullable=False, index=True,
|
DateTime(timezone=True), nullable=False, index=True,
|
||||||
@@ -46,7 +48,8 @@ class TaskRun(Base):
|
|||||||
)
|
)
|
||||||
duration_ms: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
duration_ms: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
||||||
status: Mapped[str] = mapped_column(
|
status: Mapped[str] = mapped_column(
|
||||||
String(16), nullable=False, default="running", index=True,
|
# No index=True — ix_task_run_status_started leads with `status`.
|
||||||
|
String(16), nullable=False, default="running",
|
||||||
server_default="running",
|
server_default="running",
|
||||||
)
|
)
|
||||||
error_type: Mapped[str | None] = mapped_column(String(128), nullable=True)
|
error_type: Mapped[str | None] = mapped_column(String(128), nullable=True)
|
||||||
|
|||||||
Reference in New Issue
Block a user