From 24ce4ebe47c0b4ac6a82690dd8c1754a638a0f33 Mon Sep 17 00:00:00 2001 From: BiFangKNT <1320414964@qq.com> Date: Fri, 23 Oct 2026 14:58:22 +0800 Subject: [PATCH] perf(persistence): remove unused resource URL indexes - Keep only primary-key and URL-hash unique indexes - Fold index cleanup into migration 0041 - Verify SQLite and PostgreSQL migration behavior --- ARCHITECTURE.md | 2 +- .../pkg/entity/persistence/resource_url.py | 3 +- .../alembic/versions/0041_resource_urls.py | 2 ++ .../persistence/test_resource_urls.py | 33 +++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a18f99565..8553cdfb0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -275,7 +275,7 @@ Persistence is centered on `pkg/persistence/mgr.py`. - SQLite is the default database; PostgreSQL is supported. - Models live under `pkg/entity/persistence/`. - `pkg/persistence/media.py::MediaReferenceText` validates bound conversation payloads on ORM/Core writes while preserving the existing TEXT schema and string API. It rejects recognized structured inline images: retained images go through `MediaCache.externalize_chain_dump()` before serialization, while metadata-only snapshots and transcripts use `redact_inline_images()`. Reads and comparisons preserve historical values for incremental cleanup. Runtime/adapter messages, arbitrary diagnostic text, and opaque strings are outside this contract. -- `pkg/persistence/resource_urls.py` interns structured resource URL fields in retained messages, history and monitoring at the typed SQL boundary, in the source transaction. `MediaReferenceJSON` preserves native JSON monitoring details. Payloads store opaque references to the global `resource_urls` catalog; typed SQL reads and comparisons expand them to current URLs, preserving API shapes and source-record authorization. Configuration URLs and URLs in free text are excluded. The URL hash has a unique index; canonical IDs and check timestamps are indexed. Successful refreshes overwrite the current URL and persist the check time even when standard `get_msg` returns the same URL. Converging URLs retain only ID redirects, with no old URL. PostgreSQL row locks and SQLite's immediate write transaction serialize concurrent refreshes; SQLite holds its write lock during the upstream call. Bounded in-memory aliases let reused Runner payloads resolve old URLs without retaining those URLs in the database. Migration `0041_resource_urls` normalizes existing payloads using their source creation timestamps before producers start; downgrade expands references before dropping the catalog. +- `pkg/persistence/resource_urls.py` interns structured resource URL fields in retained messages, history and monitoring at the typed SQL boundary, in the source transaction. `MediaReferenceJSON` preserves native JSON monitoring details. Payloads store opaque references to the global `resource_urls` catalog; typed SQL reads and comparisons expand them to current URLs, preserving API shapes and source-record authorization. Configuration URLs and URLs in free text are excluded. Catalog lookups use only the primary-key and URL-hash unique indexes. Successful refreshes overwrite the current URL and persist the check time even when standard `get_msg` returns the same URL. Converging URLs retain only ID redirects, with no old URL. PostgreSQL row locks and SQLite's immediate write transaction serialize concurrent refreshes; SQLite holds its write lock during the upstream call. Bounded in-memory aliases let reused Runner payloads resolve old URLs without retaining those URLs in the database. Migration `0041_resource_urls` normalizes existing payloads using their source creation timestamps before producers start; downgrade expands references before dropping the catalog. - Timezone-less SQL `DateTime` columns store UTC-naive values on both backends. Normalize aware values with `pkg/persistence/datetime_utils.py::as_naive_utc` at the bind boundary, including deadlines, leases, event times, and query/retention cutoffs; do not merely strip an offset. Existing naive rows represent UTC. Restore UTC awareness for application comparisons and epoch serialization. Runner ledger/event/transcript stores follow this existing schema contract without a data migration or changes to journal/tenant authorization. - Fresh schemas are created from current metadata, then Alembic migrations run to head. LangBot 4.x does not upgrade 3.x databases. - New schema changes use Alembic under `pkg/persistence/alembic/versions/`; there is no legacy migration chain in 4.x. diff --git a/src/langbot/pkg/entity/persistence/resource_url.py b/src/langbot/pkg/entity/persistence/resource_url.py index 6d9a19ced..03c0fa893 100644 --- a/src/langbot/pkg/entity/persistence/resource_url.py +++ b/src/langbot/pkg/entity/persistence/resource_url.py @@ -11,7 +11,7 @@ class ResourceURL(Base): id = sa.Column(sa.String(36), primary_key=True) url = sa.Column(sa.Text, nullable=True) url_hash = sa.Column(sa.String(64), nullable=True, unique=True) - canonical_id = sa.Column(sa.String(36), sa.ForeignKey('resource_urls.id'), nullable=True, index=True) + canonical_id = sa.Column(sa.String(36), sa.ForeignKey('resource_urls.id'), nullable=True) created_at = sa.Column(sa.DateTime, nullable=False) last_checked_at = sa.Column(sa.DateTime, nullable=True) updated_at = sa.Column(sa.DateTime, nullable=False) @@ -22,5 +22,4 @@ class ResourceURL(Base): 'OR (canonical_id IS NOT NULL AND url IS NULL AND url_hash IS NULL)', name='ck_resource_url_canonical', ), - sa.Index('ix_resource_urls_last_checked', 'last_checked_at'), ) diff --git a/src/langbot/pkg/persistence/alembic/versions/0041_resource_urls.py b/src/langbot/pkg/persistence/alembic/versions/0041_resource_urls.py index aaa90bd0a..3c5c8821f 100644 --- a/src/langbot/pkg/persistence/alembic/versions/0041_resource_urls.py +++ b/src/langbot/pkg/persistence/alembic/versions/0041_resource_urls.py @@ -20,6 +20,8 @@ def upgrade(): connection = op.get_bind() ResourceURL.__table__.create(connection, checkfirst=True) + op.drop_index('ix_resource_urls_canonical_id', table_name='resource_urls', if_exists=True) + op.drop_index('ix_resource_urls_last_checked', table_name='resource_urls', if_exists=True) install(connection.engine) if connection.dialect.name == 'postgresql': create_postgres_expansion(connection) diff --git a/tests/unit_tests/persistence/test_resource_urls.py b/tests/unit_tests/persistence/test_resource_urls.py index 320baefd6..45bab81a2 100644 --- a/tests/unit_tests/persistence/test_resource_urls.py +++ b/tests/unit_tests/persistence/test_resource_urls.py @@ -321,6 +321,39 @@ async def test_napcat_uses_key_ttl_instead_of_the_standard_interval(database): assert 'rkey=key2' in await db.scalar(sa.select(EventLog.input_json)) +@pytest.mark.asyncio +async def test_index_migration_preserves_primary_key_unique_hash_and_payload(database): + migration = importlib.import_module('langbot.pkg.persistence.alembic.versions.0041_resource_urls') + removed = {'ix_resource_urls_canonical_id', 'ix_resource_urls_last_checked'} + + def indexes(sync): + return {index['name'] for index in sa.inspect(sync).get_indexes('resource_urls')} + + def upgrade(sync): + with Operations.context(MigrationContext.configure(sync)): + migration.upgrade() + + def create_legacy_indexes(sync): + operations = Operations(MigrationContext.configure(sync)) + operations.create_index('ix_resource_urls_canonical_id', 'resource_urls', ['canonical_id']) + operations.create_index('ix_resource_urls_last_checked', 'resource_urls', ['last_checked_at']) + + async with database.begin() as db: + await append(db, 'a') + assert removed.isdisjoint(await db.run_sync(indexes)) + # Recreate the schema deployed before the indexes were removed. + await db.run_sync(create_legacy_indexes) + assert removed <= await db.run_sync(indexes) + await db.run_sync(upgrade) + await db.run_sync(upgrade) + assert removed.isdisjoint(await db.run_sync(indexes)) + primary_key = await db.run_sync(lambda sync: sa.inspect(sync).get_pk_constraint('resource_urls')) + assert primary_key['constrained_columns'] == ['id'] + constraints = await db.run_sync(lambda sync: sa.inspect(sync).get_unique_constraints('resource_urls')) + assert any(constraint['column_names'] == ['url_hash'] for constraint in constraints) + assert OLD in await db.scalar(sa.select(EventLog.input_json)) + + @pytest.mark.asyncio async def test_migration_preserves_source_creation_and_downgrade(database): migration = importlib.import_module('langbot.pkg.persistence.alembic.versions.0041_resource_urls')