Skip to content

Drop coveragerecords and equivalentscoveragerecords tables (PP-4653) #3521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dbernstein wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Drop coveragerecords and equivalentscoveragerecords tables (PP-4653) #3521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c454da4
Drop coveragerecords and equivalentscoveragerecords tables
dbernstein Jun 24, 2026
325e5db
Remove the now-orphaned BaseCoverageRecord mixin
dbernstein Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions alembic/versions/20250528_01b1e464a9d1_remove_work_coverage_table.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
from sqlalchemy import DateTime, ForeignKey, Integer, String, Unicode
from sqlalchemy.dialects import postgresql

from palace.manager.sqlalchemy.model.coverage import BaseCoverageRecord

# revision identifiers, used by Alembic.
revision = "01b1e464a9d1"
down_revision = "d671b95566fb"
Expand Down Expand Up @@ -80,8 +78,11 @@ def downgrade() -> None:
sa.Column(
"status",
postgresql.ENUM(
*BaseCoverageRecord.status_enum.enums, # type: ignore[attr-defined]
name=BaseCoverageRecord.status_enum.name,
"success",
"transient failure",
"persistent failure",
"registered",
name="coverage_status",
create_type=False,
),
index=True,
Expand Down
199 changes: 199 additions & 0 deletions alembic/versions/20260916_5ca948100902_drop_coveragerecords_and_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
"""Drop coveragerecords and equivalentscoveragerecords tables

The CoverageProvider machinery that read and wrote ``coveragerecords`` was retired
two releases ago, and the previous release removed the ``coverage_records``
relationships that were still making SQLAlchemy read the table on every parent
delete (see 58ebd34c5092). The equivalent-identifiers refresh that wrote
``equivalentscoveragerecords`` moved to a Celery task before either. N-1 therefore
maps neither table, so both are dropped here along with their shared
``coverage_status`` enum type.

The ``timestamps`` table and its separate ``service_type`` enum are unaffected.

Revision ID: 5ca948100902
Revises: 58ebd34c5092
Create Date: 2026-09-16 00:00:00.000000+00:00

"""

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = "5ca948100902"
down_revision = "58ebd34c5092"
branch_labels = None
depends_on = None

# The status enum shared by the two dropped tables. We manage the Postgres type
# explicitly (create_type=False) so it is created/dropped exactly once rather
# than once per table.
coverage_status = postgresql.ENUM(
"success",
"transient failure",
"persistent failure",
"registered",
name="coverage_status",
create_type=False,
)


def upgrade() -> None:
op.drop_index(
op.f("ix_equivalentscoveragerecords_equivalency_id"),
table_name="equivalentscoveragerecords",
)
op.drop_index(
op.f("ix_equivalentscoveragerecords_operation"),
table_name="equivalentscoveragerecords",
)
op.drop_index(
op.f("ix_equivalentscoveragerecords_status"),
table_name="equivalentscoveragerecords",
)
op.drop_index(
op.f("ix_equivalentscoveragerecords_timestamp"),
table_name="equivalentscoveragerecords",
)
op.drop_table("equivalentscoveragerecords")

op.drop_index(
op.f("ix_coveragerecords_data_source_id_operation_identifier_id"),
table_name="coveragerecords",
)
op.drop_index(op.f("ix_coveragerecords_exception"), table_name="coveragerecords")
op.drop_index(
op.f("ix_coveragerecords_identifier_id"), table_name="coveragerecords"
)
op.drop_index(op.f("ix_coveragerecords_status"), table_name="coveragerecords")
op.drop_index(op.f("ix_coveragerecords_timestamp"), table_name="coveragerecords")
op.drop_index(
"ix_identifier_id_data_source_id_operation", table_name="coveragerecords"
)
op.drop_index(
"ix_identifier_id_data_source_id_operation_collection_id",
table_name="coveragerecords",
)
op.drop_table("coveragerecords")

# The coverage_status enum was used only by the two tables just dropped.
coverage_status.drop(op.get_bind(), checkfirst=False)


def downgrade() -> None:
coverage_status.create(op.get_bind(), checkfirst=False)

op.create_table(
"coveragerecords",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("identifier_id", sa.Integer(), nullable=True),
sa.Column("data_source_id", sa.Integer(), nullable=True),
sa.Column("operation", sa.String(length=255), nullable=True),
sa.Column("timestamp", sa.DateTime(timezone=True), nullable=True),
sa.Column("status", coverage_status, nullable=True),
sa.Column("exception", sa.Unicode(), nullable=True),
sa.Column("collection_id", sa.Integer(), nullable=True),
sa.ForeignKeyConstraint(
["collection_id"],
["collections.id"],
name=op.f("coveragerecords_collection_id_fkey"),
),
sa.ForeignKeyConstraint(
["data_source_id"],
["datasources.id"],
name=op.f("coveragerecords_data_source_id_fkey"),
),
sa.ForeignKeyConstraint(
["identifier_id"],
["identifiers.id"],
name=op.f("coveragerecords_identifier_id_fkey"),
),
sa.PrimaryKeyConstraint("id", name=op.f("coveragerecords_pkey")),
)
op.create_index(
"ix_identifier_id_data_source_id_operation_collection_id",
"coveragerecords",
["identifier_id", "data_source_id", "operation", "collection_id"],
unique=True,
)
op.create_index(
"ix_identifier_id_data_source_id_operation",
"coveragerecords",
["identifier_id", "data_source_id", "operation"],
unique=True,
postgresql_where=sa.text("collection_id IS NULL"),
)
op.create_index(
op.f("ix_coveragerecords_timestamp"),
"coveragerecords",
["timestamp"],
unique=False,
)
op.create_index(
op.f("ix_coveragerecords_status"), "coveragerecords", ["status"], unique=False
)
op.create_index(
op.f("ix_coveragerecords_identifier_id"),
"coveragerecords",
["identifier_id"],
unique=False,
)
op.create_index(
op.f("ix_coveragerecords_exception"),
"coveragerecords",
["exception"],
unique=False,
)
op.create_index(
op.f("ix_coveragerecords_data_source_id_operation_identifier_id"),
"coveragerecords",
["data_source_id", "operation", "identifier_id"],
unique=False,
)

op.create_table(
"equivalentscoveragerecords",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("equivalency_id", sa.Integer(), nullable=False),
sa.Column("operation", sa.String(length=255), nullable=True),
sa.Column("timestamp", sa.DateTime(timezone=True), nullable=True),
sa.Column("status", coverage_status, nullable=True),
sa.Column("exception", sa.Unicode(), nullable=True),
sa.ForeignKeyConstraint(
["equivalency_id"],
["equivalents.id"],
name=op.f("equivalentscoveragerecords_equivalency_id_fkey"),
ondelete="CASCADE",
),
sa.PrimaryKeyConstraint("id", name=op.f("equivalentscoveragerecords_pkey")),
sa.UniqueConstraint(
"equivalency_id",
"operation",
name=op.f("equivalentscoveragerecords_equivalency_id_operation_key"),
),
)
op.create_index(
op.f("ix_equivalentscoveragerecords_timestamp"),
"equivalentscoveragerecords",
["timestamp"],
unique=False,
)
op.create_index(
op.f("ix_equivalentscoveragerecords_status"),
"equivalentscoveragerecords",
["status"],
unique=False,
)
op.create_index(
op.f("ix_equivalentscoveragerecords_operation"),
"equivalentscoveragerecords",
["operation"],
unique=False,
)
op.create_index(
op.f("ix_equivalentscoveragerecords_equivalency_id"),
"equivalentscoveragerecords",
["equivalency_id"],
unique=False,
)
112 changes: 1 addition & 111 deletions src/palace/manager/sqlalchemy/model/coverage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# BaseCoverageRecord, Timestamp, CoverageRecord
# Timestamp
from __future__ import annotations

import datetime
Expand All @@ -11,7 +11,6 @@
DateTime,
Enum,
ForeignKey,
Index,
Integer,
String,
Unicode,
Expand All @@ -30,23 +29,6 @@
from palace.manager.sqlalchemy.model.collection import Collection


class BaseCoverageRecord:
"""Holds the ``coverage_status`` enum shared by the two dormant coverage models."""

SUCCESS = "success"
TRANSIENT_FAILURE = "transient failure"
PERSISTENT_FAILURE = "persistent failure"
REGISTERED = "registered"

status_enum = Enum(
SUCCESS,
TRANSIENT_FAILURE,
PERSISTENT_FAILURE,
REGISTERED,
name="coverage_status",
)


class Timestamp(Base):
"""Tracks the activities of Monitors, CoverageProviders,
and general scripts.
Expand Down Expand Up @@ -306,95 +288,3 @@ def recording(self) -> Generator[Self]:
self.finish = utc_now()

__table_args__ = (UniqueConstraint("service", "collection_id"),)


class CoverageRecord(Base, BaseCoverageRecord):
"""A record of a Identifier being used as input into some process.

Dormant: nothing reads or writes this table. The class is retained only so
``create_all`` keeps emitting the table for freshly initialized databases,
which N-1 app servers still expect; it and the table are removed together in
a follow-up PR. It is deliberately just a table definition -- without the
parent relationships, a row written here could not be cleaned up when its
Identifier, DataSource or Collection is deleted, and would instead make that
delete fail on a foreign key.
"""

__tablename__ = "coveragerecords"

id: Mapped[int] = Column(Integer, primary_key=True)
identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True)

# If applicable, this is the ID of the data source that took the
# Identifier as input.
data_source_id = Column(Integer, ForeignKey("datasources.id"))
operation = Column(String(255), default=None)

timestamp = Column(DateTime(timezone=True), index=True)

status = Column(BaseCoverageRecord.status_enum, index=True)
exception = Column(Unicode, index=True)

# If applicable, this is the ID of the collection for which
# coverage has taken place. This is currently only applicable
# for Metadata Wrangler coverage.
collection_id = Column(Integer, ForeignKey("collections.id"), nullable=True)

__table_args__ = (
Index(
"ix_identifier_id_data_source_id_operation",
identifier_id,
data_source_id,
operation,
unique=True,
postgresql_where=collection_id.is_(None),
),
Index(
"ix_identifier_id_data_source_id_operation_collection_id",
identifier_id,
data_source_id,
operation,
collection_id,
unique=True,
),
)


Index(
"ix_coveragerecords_data_source_id_operation_identifier_id",
CoverageRecord.data_source_id,
CoverageRecord.operation,
CoverageRecord.identifier_id,
)


class EquivalencyCoverageRecord(Base, BaseCoverageRecord):
"""Dormant model retained only so the ``equivalentscoveragerecords`` table
stays in the schema for one more release.

The equivalent-identifiers refresh no longer reads or writes this table — it
was replaced by the Redis dirty-set queue and the ``equivalent_identifiers_refresh``
Celery task. But per our online-migration convention the table cannot be dropped
in the same release that stops using it: during a rolling deploy, N-1 app servers
still run the old listener that writes here, so dropping the table now would make
them error. The table and this model will be removed in a follow-up PR that ships
after this release. See https://github.com/ThePalaceProject/circulation/pull/3459.
"""

__tablename__ = "equivalentscoveragerecords"

id: Mapped[int] = Column(Integer, primary_key=True)

equivalency_id: Mapped[int] = Column(
Integer,
ForeignKey("equivalents.id", ondelete="CASCADE"),
index=True,
nullable=False,
)

operation = Column(String(255), index=True, default=None)
timestamp = Column(DateTime(timezone=True), index=True)
status = Column(BaseCoverageRecord.status_enum, index=True)
exception = Column(Unicode)

__table_args__ = (UniqueConstraint(equivalency_id, operation),)
14 changes: 12 additions & 2 deletions tests/migration/test_20260916_58ebd34c5092_empty_coveragerecords.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,30 @@ def test_empties_coveragerecords(
assert _coveragerecord_count(alembic_engine) == 1


def test_upgrade_does_not_leak_lock_timeout(alembic_engine: Engine) -> None:
def test_upgrade_does_not_leak_lock_timeout(
alembic_runner: MigrationContext,
alembic_engine: Engine,
) -> None:
"""upgrade() leaves lock_timeout exactly as it found it.

The TRUNCATE runs under a short ``lock_timeout``, set with ``SET LOCAL``,
which lasts to the end of the *transaction* rather than the end of this
revision. ``alembic/env.py`` runs every pending revision inside a single
``context.begin_transaction()``, so without an explicit reset the timeout
would silently apply to every later revision in the same upgrade.
would silently apply to every later revision in the same upgrade. That is
not hypothetical after this release: a database older than this revision
upgrades through it and the following drop in one transaction.

Driving the migration through ``alembic_runner`` could not catch that: it
commits between revisions, which discards the setting regardless of what
the migration did. So run ``upgrade()`` against a transaction we hold open
ourselves, which is the situation env.py actually creates.
"""
# The next revision drops coveragerecords, so it is absent from the schema
# built from the current models. Step back to this revision, whose
# downgrade recreates it, to give the TRUNCATE something to act on.
alembic_runner.migrate_down_to(REVISION)

revision = _load_revision()

with alembic_engine.begin() as connection:
Expand Down
Loading
Loading