Migration 091 broken for SQLite

Bug #1070559 reported by Rick Harris
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Rick Harris

Bug Description

Migration 091 swallows exceptions generated by SQLAlchemy-Migrate leaving the database in an inconsistent state, in particular, leaving a 'migration_tmp` table around that will cause future migrations to fail.

The failure seems to be:

1. The script attempts to drop foreign-key constraints. The SQLAlchemy-Migrate SQLite driver raises a NotSupportedError since ALTER TABLE DROP CONSTRAINT is not supported directly by SQLite and SQLAlchemy-MIgrate doesn't have work-around code in place for this.

  File "/usr/local/lib/python2.7/dist-packages/migrate/changeset/databases/sqlite.py", line 22, in _not_supported
    "%s; see http://www.sqlite.org/lang_altertable.html" % op)
NotSupportedError: SQLite does not support ALTER TABLE DROP CONSTRAINT; see http://www.sqlite.org/lang_altertable.html

The migration script simply ignores this error by swallowing the exception.

2. After performing the data fix-up, the script attempts to re-create the new foreign-key constraints. The causes a different exception in SQLAlchemy-Migrate:

  File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/sql/compiler.py", line 1693, in quote
    if self._requires_quotes(ident):
  File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/sql/compiler.py", line 1674, in _requires_quotes
    lc_value = value.lower()
AttributeError: 'int' object has no attribute 'lower'

This appears to be caused by us not having dropped the original constraint which happens to have the name 0 which is misinterpreted as an integer causing the `lower()` to fail.

This exception prevents the SQLAlchemy-Migrate code from emitting the ALTER TABLE RENAME or DROP TABLE for `migration_tmp` leaving it hanging around for the next migration to hit and fail (in this case 098 seems to be the next failure for me).

Possible solutions:

1. Switch to Alembic (and fix it's SQLite support in the process)
2. Write a 091_sqlite_upgrade/downgrade script to paper over this issue (EASIEST)
3. Fix SQLAlchemy-MIgrate to support DROP CONSTRAINT by using a tmp table as well as possibly fixing SQLAlchemy's cast code (BEST FOR UPSTREAM)

Changed in nova:
importance: Undecided → Medium
importance: Medium → High
assignee: nobody → Rick Harris (rconradharris)
Revision history for this message
Rick Harris (rconradharris) wrote :

The problem turned out to be me using SQLAlchemy 0.7.2 instead of 0.7.8. Upgrading fixed the issue.

That said, the handling of exceptions in the script seem overly broad, so I'm going to propose a patch which clamps down on which exceptions are truly acceptable, since catching arbitrary exceptions just causes failures in future migrations due to DB inconsistency.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/14714

Changed in nova:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/14714
Committed: http://github.com/openstack/nova/commit/5f3c2f9b8ea24b2a0e1909dbeac4dc47f2410d7a
Submitter: Jenkins
Branch: master

commit 5f3c2f9b8ea24b2a0e1909dbeac4dc47f2410d7a
Author: Rick Harris <email address hidden>
Date: Tue Oct 23 21:55:59 2012 +0000

    More specific exception handling in migration 091.

    The original code would catch and discard any exception generated when
    trying to create or drop fkey constraints using the SQLite engine. This
    has the side-effect of allowing the DB to become inconsistent (by
    leaving a `migration_tmp` table around), causing future migrations to
    fail and making bugs even hard to track down.

    Long term, we should fix the underlying cause of these exceptions,
    either by switching to Alembic or by fixing SQLALchemy-Migrate to fully
    support dropping of constraints via a temp table.

    For now though, tightening up the exception handling will at least make
    any future bugs a bit easier to diagnose.

    Fixes bug 1070559

    Change-Id: If549d65afa758b6f8a0db6fc8478a5b0c9277e23

Changed in nova:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in nova:
milestone: none → grizzly-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in nova:
milestone: grizzly-1 → 2013.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.