Opened 110 minutes ago

Last modified 75 minutes ago

#37416 new Uncategorized

Migrations are not recorded atomically

Reported by: Gavin Wahl Owned by:
Component: Database layer (models, ORM) Version: 6.1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Migrations apply their operations transactionally, and then record that the migration was applied outside of the transaction. If any sort of failure or crash happens between this steps, this results in the migration being run again, despite it have already actually been applied, just not recorded.

There is a narrow window of failure in MigrationExecutor.apply_migration between the call to migration.apply() and self.record_migration(). It doesn't happen often, but there are mulitple ways it could -- the python process crashing, the computer running python crashing, postgres crashing, or a network failure. I believe the most realistic and reliable reproducible reproducer is to simulate a hard crash during migration recording by adding import os; os.kill(os.getpid(), 9) or import sys; sys.exit() to the first line of BaseDatabaseSchemaEditor.record_migration. A unrecoverable hard crash can happen at any time, that's why we have transactions.

This was fixed #29721 but then regressed in #32374. record_migration is now atomic for migrations that don't use deferred sql, but non-atomic for those that do (which is a lot, like CreateModel, AlterField...). The correct order is

  • Migrate
  • Apply deferred sql
  • Record
  • Commit

While 0c42cdf0d moved the Commit before the Record.

It's easy to see what's happening from these [abbreviated] postgres logs from running migrations. The django_migrations insert happens after the COMMIT of the main migration, so the migration isn't really atomic.

Before:

BEGIN
CREATE TABLE "foo_foo" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "a_id" bigint NOT NULL)
ALTER TABLE "foo_foo" ADD CONSTRAINT "foo_foo_a_id_f8ce5994_fk_foo_foo_id" FOREIGN KEY ("a_id") REFERENCES "foo_foo" ("id") DEFERRABLE INITIALLY DEFERRED
CREATE INDEX "foo_foo_a_id_f8ce5994" ON "foo_foo" ("a_id")
COMMIT
INSERT INTO "django_migrations" ("app", "name", "applied") VALUES ('foo', '0001_initial', '2026-10-07 07:10:08.085619+00:00'::timestamptz) RETURNING "django_migrations"."id"

Correct behavior:

BEGIN
CREATE TABLE "foo_foo" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "a_id" bigint NOT NULL)
ALTER TABLE "foo_foo" ADD CONSTRAINT "foo_foo_a_id_f8ce5994_fk_foo_foo_id" FOREIGN KEY ("a_id") REFERENCES "foo_foo" ("id") DEFERRABLE INITIALLY DEFERRED
CREATE INDEX "foo_foo_a_id_f8ce5994" ON "foo_foo" ("a_id")
INSERT INTO "django_migrations" ("app", "name", "applied") VALUES ('foo', '0001_initial', '2026-10-07 07:13:01.602656+00:00'::timestamptz) RETURNING "django_migrations"."id"
COMMIT

Here's a minimal model to get a deferred sql migration for these logs:
Minimal model to get deferred_sql:

from django.db import models

class Foo(models.Model):
    a = models.ForeignKey('self', on_delete=models.CASCADE)

Change History (1)

comment:1 by Gavin Wahl, 105 minutes ago

I'm considering two ways to fix this:

  • Adding a callback argument to schema_editor that it would call right before exiting its atomic block. apply_migration would pass on_exit=lambda: self.record_migration(migration.app_label, migration.name) to ensure it happens after deferred sql.
  • Adding a flush_deferred_sql method to the schema editor that executes and clears the pending deferred sql. It would be called inside the schema editor wherever that happens today to preserve backwards compatibility. apply_migration when call flush_deferred_sql() then record_migration() inside the schema_editor with block. The schema editor would still call it again, but the pending deferred sql would have been cleared and it would do nothing.

Does anyone have an opinion which would be preferable?

There's also currently a performance impact of recording migrations outside a transaction. I have a squashed migration that replaces=[...] several hundred others. record_migration inserts a django_migrations row for each of those replacements. It's taking about 0.6s to record all of them applied, because the each get their own transaction. In a single transaction, it takes about 0.01s.

Version 1, edited 101 minutes ago by Gavin Wahl (previous) (next) (diff)
Note: See TracTickets for help on using tickets.
Back to Top