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)
I'm considering two ways to fix this:
apply_migrationwould passon_exit=lambda: self.record_migration(migration.app_label, migration.name)to ensure it happens after deferred sql.flush_deferred_sqlmethod 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_migrationwhen callflush_deferred_sql()thenrecord_migration()inside theschema_editorwithblock. 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_migrationinserts adjango_migrationsrow 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.