Opened 12 hours ago

Last modified 2 hours ago

#36170 new Cleanup/optimization

Running no-op migrations (verbose_name, ...) slow on sqlite

Reported by: Klaas van Schelven Owned by:
Component: Migrations Version: 5.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Running no-op migrations (verbose_name, help_text, choices) triggers DB-wide check_constraints on sqlite

This is because this is done as part of the DatabaseSchemaEditor's exit which is unconditionally called as part of apply_migration.

Because no-op migrations cannot be ignored by design (also here) this means that changing your help-text will lead to an unnecessary check of all constraints in the database (if no workarounds are employed, e.g. squashing with something else).

If we accept the principle of "no ignoring of fields on-detect" as per the issues mentioned above, we might still try to detect the fact that for any given field and DB nothing happened in practice (and hence no check_constraints is needed)

In particular: the call to check_constraints was introduced as part of this commit, in which it is directly tied to a 5-step process.

Without claiming to have fully thought through all consequences: can't we pull the check closer to those 5 steps, or make it conditional to _remake_table having been actually called between enter & exit?

Refs / background:

I'm doing this in the context of Bugsink, a Self-hosted Error Tracker; SQLite is a first level citizen (for production) in that tool, which means large databases can be expected. On a not all too big ~100K-row database I timed the above unnecessary check to take ~20 seconds. Scaling that up by 1 or 2 orders of magnitude would be my goal. Charitably assuming linear growth in time-complexity, you can see how this can quickly become really annoying.

this SO post provides tricks for avoiding the creation of such migrations in the first place (may be relevant context as long as the above isn't fixed yet).

Change History (3)

comment:1 by Klaas van Schelven, 12 hours ago

Component: UncategorizedMigrations
Type: UncategorizedCleanup/optimization

comment:2 by Simon Charette, 9 hours ago

Triage Stage: UnreviewedAccepted

Interesting use case. I think it's worth a shot but we should definitely error on the side of caution here as to avoid causing subtle integrity violation regressions. That is the PRAGMA foreign_key_check might be necessary for other operations that SQLite added support for over the years such as ADD FIELD assuming it's a foreign key that can be populated with invalid data.

I believe the following should cover all no-op migrations while avoiding the daunting task of figuring out for which operations are safe to skip foreign key checks for

  • django/db/backends/sqlite3/schema.py

    diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py
    index 3617d17fac..a74c010384 100644
    a b def __enter__(self):  
    3333                "SQLite does not support disabling them in the middle of "
    3434                "a multi-statement transaction."
    3535            )
     36        # Avoid checking foreign key constraint for no-op migrations that
     37        # don't execute SQL as it can be expensive on large databases.
     38        self.must_check_constraints = False
    3639        return super().__enter__()
    3740
    3841    def __exit__(self, exc_type, exc_value, traceback):
    39         self.connection.check_constraints()
     42        if self.must_check_constraints:
     43            self.connection.check_constraints()
    4044        super().__exit__(exc_type, exc_value, traceback)
    4145        self.connection.enable_constraint_checking()
    4246
     47    def execute(self, *args, **kwargs):
     48        # Execution of any SQL results in a subsequent foreign key constraint
     49        # check to account for them being disabled during schema alteration.
     50        self.must_check_constraints = True
     51        return super().execute(*args, **kwargs)
     52
    4353    def quote_value(self, value):
    4454        # The backend "mostly works" without this function and there are use
    4555        # cases for compiling Python without the sqlite3 libraries (e.g.

Does that sound like a reasonable compromise?

comment:3 by Klaas van Schelven, 2 hours ago

I'm not sure I'd call it a "compromise", or simply a "solution" :-)

A quick test on both a real migration and a non-DB-altering one shows that this diff has the desired effect.

Note: See TracTickets for help on using tickets.
Back to Top