[Django] #30490: migrations unique

Django

unread,

May 19, 2019, 2:41:54 PM5/19/19

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name)
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
rkojedzinszky |
Type: Bug | Status: new
Component: | Version: 1.11
Uncategorized | Keywords: migrations parallel
Severity: Normal | run
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I've a django based api service. I run it in containers, in kubernetes.
When I update my image, basically I run the migiration process, then start
the application. With k8s for example, when I start the deployment with 2
or more replicas, actually 2 parallel running migration process will
start. With the following very simple migration process, unfortunately the
migration will be applied twice:

{{{
from django.db import migrations
from django.db.models import F
import time

def forward(apps, schema_editor):
TT = apps.get_model('center', 'TestTable')
time.sleep(10)
TT.objects.all().update(value=F('value') + 1)

def backward(apps, schema_editor):
TT = apps.get_model('center', 'TestTable')
TT.objects.all().update(value=F('value') - 1)

class Migration(migrations.Migration):

dependencies = [
('center', '0007_testtable'),
]

operations = [
migrations.RunPython(forward, backward)
]
}}}

Even if the migration process is ran in a transaction, due to the unique
index missing, it will be applied multiple times.

Even though my setup may be rare, I think it would really be necessary to
prevent migrations to be applied more than once. Db unique_together could
be utilized for that.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

May 20, 2019, 3:09:59 AM5/20/19

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: new => closed
* version: 1.11 => master
* resolution: => wontfix

Comment:

Thanks for the report, however it is rather a support issue, IMO. Adding
unique on `(app, name)` will not fix your issue, because Django adds entry
to the table with migrations history outside the main transaction.

First of all I would use `select_for_update()` to lock table before update
(to prevent race condition). Secondly you should run migrations (on the
same database) only on one instance.

Closing per TicketClosingReasons/UseSupportChannels.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:1>

Django

unread,

May 20, 2019, 4:27:27 AM5/20/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Richard Kojedzinszky):

That is why I wrote, that my approach might not be the best practice. With
an automated deployments, I would really like the case when Django itself
would prevent applying the migrations multiple times. That would even help
preventing errors made by humans. You are right, the whole migration step
should be in one database transaction for this to work. And as it is now,
I rather think it is a bug.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:2>

Django

unread,

May 20, 2019, 2:54:53 PM5/20/19

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name).
-------------------------------------+-------------------------------------
Reporter: Richard | Owner: nobody
Kojedzinszky |

Type: Bug | Status: new

Component: Uncategorized | Version: master
Severity: Normal | Resolution:

Keywords: migrations parallel | Triage Stage:
run | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Richard Kojedzinszky):

* status: closed => new
* resolution: wontfix =>

Comment:

Unfortunately, knowing that the migration tasks and recording its
existence is not run in the same transaction, in a worst case scenario,
the info that a migration has been applied can be lost at all. So, in any
circumstances, they should be in the same transaction.

Replying to [ticket:30490 Richard Kojedzinszky]:

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:3>

Django

unread,

May 20, 2019, 3:11:42 PM5/20/19

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name).
-------------------------------------+-------------------------------------
Reporter: Richard | Owner: nobody
Kojedzinszky |

Type: Bug | Status: new

Component: Uncategorized | Version: master
Severity: Normal | Resolution:
Keywords: migrations parallel | Triage Stage:
run | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Richard Kojedzinszky):

Perhaps, a similar patch would be a good starting point:

https://github.com/rkojedzinszky/django/commit/12477598fb3711f50ff482328ddcda62934caee6

Replying to [comment:3 Richard Kojedzinszky]:

>
> Unfortunately, knowing that the migration tasks and recording its
existence is not run in the same transaction, in a worst case scenario,
the info that a migration has been applied can be lost at all. So, in any
circumstances, they should be in the same transaction.
>
>
> Replying to [ticket:30490 Richard Kojedzinszky]:

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:4>

Django

unread,

May 20, 2019, 8:26:35 PM5/20/19

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name).
-------------------------------------+-------------------------------------
Reporter: Richard | Owner: nobody
Kojedzinszky |

Type: Bug | Status: closed
Component: Uncategorized | Version: master
Severity: Normal | Resolution: wontfix

Keywords: migrations parallel | Triage Stage:
run | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* status: new => closed

* resolution: => wontfix

Comment:

Hey Richard, I understand your desire to get this issue fixed but it's
more complex than wrapping `apply_migration` and `unapply_migration` in a
transaction.

For example, some backends don't support performing schema alteration
within a transaction (e.g. MySQL will silently commit the current
transaction on such operation) and
[https://docs.djangoproject.com/en/2.2/howto/writing-migrations/#non-
atomic-migrations some migrations are explicitly marked as non-atomic] to
prevent long running transactions (e.g. large back-filling data
migrations).

For these reasons holding a database level lock for the duration of a
migration application is probably not a feasible option. Altering the
`django_migrations` table is also problematic given it's not tracked by
migrations itself which is another kind of worms.

In your particular case I'd suggest you explore using another form of
distributed lock (e.g. Redis) namespaced by your deploy hash to prevent
concurrent migration but you're probably better off getting support from
our support channels of k8s ones where other users might have already come
up with a solution to your problem.

For these reasons I agree with Mariusz' resolution and ask that
[https://docs.djangoproject.com/en/2.2/internals/contributing/triaging-
tickets/#closing-tickets you follow triaging guidelines with regards to
wontfix tickets]. Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:5>

Django

unread,

May 27, 2019, 4:24:02 AM5/27/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

You don't need to use Redis for distributed locking, you can use advisory
locking in your database server. They can live outside of transactions.

In PostgreSQL they are called advisory locks -
https://hashrocket.com/blog/posts/advisory-locks-in-postgres . They are by
an integer key.

In MySQL/MariaDB they are called user locks -
https://mariadb.com/kb/en/library/get_lock/ . They are by a string key.

I don't know about SQLite or Oracle. Quick internet searches don't reveal
anything

Potentially Django could use these advisory locks.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:6>

Django

unread,

May 27, 2019, 4:24:33 AM5/27/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam (Chainz) Johnson):

* cc: Adam (Chainz) Johnson (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:7>

Django

unread,

May 27, 2019, 4:56:45 AM5/27/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tom Forbes):

The way we solved this with Kubernetes is to create and launch a Job which
will run manage.py migrate as part of the deployment pipeline. Once that
Job is complete, update the Deployment/ReplicaSet as needed.

As always you have to be completely sure the migration can run
concurrently with your app, but in genera running migrations as an initpod
or as part of the web app pod is really not advised, and adding a hack
like a distributed lock will only bring you pain.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:8>

Django

unread,

Jun 5, 2023, 4:20:49 AM6/5/23

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name).
-------------------------------------+-------------------------------------
Reporter: Richard | Owner: nobody
Kojedzinszky |

Type: Bug | Status: new
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:

Keywords: migrations parallel | Triage Stage:
run | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Richard Kojedzinszky):

* status: closed => new
* resolution: wontfix =>

Comment:

@simon As the migration script and recording it now runs in the same
transaction, the only thing left here is to add the unique key on the
records table. Am I right?

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:9>

Django

unread,

Jun 5, 2023, 5:21:14 AM6/5/23

to django-...@googlegroups.com

#30490: migrations unique_index on (app, name).
-------------------------------------+-------------------------------------
Reporter: Richard | Owner: nobody
Kojedzinszky |

Type: Bug | Status: closed
Component: Uncategorized | Version: dev
Severity: Normal | Resolution: wontfix

Keywords: migrations parallel | Triage Stage:
run | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => wontfix

Comment:

I appreciate you'd like to reopen the ticket, but please
[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets follow the triaging guidelines with regards to
wontfix tickets] and don't reopen closed tickets before reaching a
consensus on the [https://forum.djangoproject.com/t/add-unique-key-for-
django-migration-tracking-model/21283 forum].

#34610 was a duplicate.

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:10>

Django

unread,

Jun 5, 2023, 11:34:38 PM6/5/23

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by David Sanders):

Forum discussion: https://forum.djangoproject.com/t/add-unique-key-for-
django-migration-tracking-model/21283

--
Ticket URL: <https://code.djangoproject.com/ticket/30490#comment:11>

Django

unread,

Oct 9, 2023, 1:03:54 PM10/9/23

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

[Django] #30490: migrations unique_index on (app, name)

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django