Skip to content

fix(backend): make roms_facets/collection migrations idempotent#3930

Merged
gantoine merged 1 commit into
masterfrom
fix/idempotent-roms-facets-migrations
Jul 24, 2026
Merged

fix(backend): make roms_facets/collection migrations idempotent#3930
gantoine merged 1 commit into
masterfrom
fix/idempotent-roms-facets-migrations

Conversation

@gantoine

Copy link
Copy Markdown
Member

Description
Explain the changes or enhancements you are proposing with this pull request.

On the latest alpha, upgrading wedges the container in a permanent boot loop with:

sqlalchemy.exc.OperationalError: (mariadb.OperationalError) Table 'roms_facets' already exists
ERROR: [RomM][init] Failed to run database migrations

Root cause: MariaDB/MySQL auto-commit each DDL statement (no transactional DDL). When migration 0100 failed partway through (on the backfill or a CREATE TRIGGER), the CREATE TABLE roms_facets had already committed, but alembic_version never advanced past 0099. Every subsequent boot re-ran 0100 from the top and died on CREATE TABLE ... already exists, so the instance could never recover on its own. Migrations 0100, 0101, and 0103 were all non-idempotent and share this failure mode; 0102 already guarded itself with if_not_exists / catalog checks.

Changes (bringing the whole 0100-0103 batch in line with 0102):

  • 0100: CREATE TABLE ... if_not_exists=True; catalog-check the index (MySQL has no CREATE INDEX IF NOT EXISTS); make the backfill idempotent with a WHERE id NOT IN (...) anti-join; DROP TRIGGER IF EXISTS before each CREATE TRIGGER (MySQL and Postgres paths).
  • 0101: same treatment; backfill uses INSERT IGNORE / ON CONFLICT DO NOTHING, triggers are dropped before recreation (the view already drops first).
  • 0103: skip add_column for provider-id columns already present.

Each is now safe to re-run after a partial failure, so a wedged instance self-heals on the next boot with no manual SQL.

Testing

  • Reproduced the exact wedged state (alembic_version at 0099 with an orphaned roms_facets table) and confirmed alembic upgrade head now recovers cleanly all the way to 0103 instead of crashing.
  • Also verified a full rewind-to-0099 with every later table/trigger/view left in place recovers to 0103 with no errors.
  • Clean 0001 -> head still works; test_rom_facets.py and test_stats_handler.py pass (14/14).
  • trunk check clean on all three files.

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

Screenshots (if applicable)


AI assistance disclosure

This change was developed with AI assistance (Claude Code). The AI diagnosed the root cause, wrote the migration guards, and ran the recovery/regression tests; all changes were reviewed by the author.

🤖 Generated with Claude Code

MariaDB/MySQL auto-commit each DDL statement, so a failure partway
through 0100/0101/0103 committed the CREATE TABLE / ADD COLUMN but never
advanced alembic_version. Every reboot then re-ran the migration from the
top and died on "Table ... already exists", wedging the boot loop
permanently.

Guard the schema changes so a re-run after a partial failure recovers:
- 0100: CREATE TABLE ... if_not_exists, catalog-check the index, make the
  backfill idempotent with an anti-join, DROP TRIGGER IF EXISTS before
  each CREATE TRIGGER (both MySQL and Postgres paths).
- 0101: same treatment; backfill uses INSERT IGNORE / ON CONFLICT DO
  NOTHING, triggers dropped before recreation, view already drops first.
- 0103: skip add_column for provider ids that are already present.

0102 already used if_not_exists / catalog checks; this brings the rest of
the batch in line. Verified by reproducing the wedged state (version at
0099 with the later tables already present) and confirming upgrade head
now recovers cleanly to 0103, plus the rom_facets and stats test suites.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 24, 2026 01:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Makes migrations 0100, 0101, and 0103 recover safely after partially committed DDL.

  • Guards table, index, and provider-column creation when schema objects already exist.
  • Makes facet and virtual-collection backfills tolerate data inserted by an earlier attempt.
  • Drops and recreates synchronization triggers and the virtual-collections view safely.

Confidence Score: 5/5

The PR appears safe to merge, with the changed migrations consistently handling the supported partial-application recovery paths.

Existing schema objects are detected or created conditionally, backfills complete missing data without duplicate-key failures, and synchronization triggers are recreated after the schema and data steps succeed.

Reviews (1): Last reviewed commit: "fix(backend): make roms_facets/collectio..." | Re-trigger Greptile

@gantoine
gantoine merged commit dcc04aa into master Jul 24, 2026
13 checks passed
@gantoine
gantoine deleted the fix/idempotent-roms-facets-migrations branch July 24, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants