[Django] #23745: Migrations migrate is slow
Django
------------------------------------------------+------------------------
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Slowness around the optimizer is tracked in #22608. This ticket is
specifically targeted to slowness with the `migrate` command. To repeat a
comment previously done in #22608, each migration in the Django test suite
is taking up 3.5 seconds. That should be improved.
Profiling migrations in the current test suite:
{{{
ncalls tottime percall cumtime percall filename:lineno(function)
1 0.000 0.000 27.714 27.714
django/db/migrations/executor.py:55(migrate)
8 0.000 0.000 27.714 3.464
django/db/migrations/executor.py:83(apply_migration)
8 0.042 0.005 13.157 1.645
django/db/migrations/executor.py:128(detect_soft_applied)
8 0.000 0.000 14.500 1.812
django/db/migrations/migration.py:79(apply)
8 0.042 0.005 12.945 1.618
django/db/migrations/operations/models.py:32(database_forwards)
17 0.094 0.006 27.447 1.615
django/db/migrations/state.py:42(ProjectState.render)
21511 0.791 0.000 21.881 0.001
django/db/migrations/state.py:293(ModelState.render)
21488 0.549 0.000 5.290 0.000
django/db/migrations/state.py:164(ModelState.from_model)
21515/21511 0.766 0.000 15.861 0.001
django/db/models/base.py:63(ModelBase.__new__)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23745>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Migrations | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
Comment:
See https://github.com/django/django/pull/3458. With that patch, each
migration is now only taking 0.5 sec. (down from 3.5 sec.).
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:1>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Migrations | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:2>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:3>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 1 => 0
Comment:
New try: https://github.com/django/django/pull/3484
Before:
Applying admin.0001_initial... OK (2.9538629055)
Applying commands_sql_migrations.0001_initial... OK (3.98874592781)
Applying fixtures_migration.0001_initial... OK (2.9278049469)
Applying sites.0001_initial... OK (2.65014600754)
Applying flatpages.0001_initial... OK (2.63765001297)
Applying migration_test_data_persistence.0001_initial... OK
(2.87452602386)
Applying migration_test_data_persistence.0002_add_book... OK
(2.58737111092)
Applying postgres_tests.0001_setup_extensions... OK (1.30905008316)
Applying postgres_tests.0002_create_test_models... OK (9.76085591316)
Applying redirects.0001_initial... OK (3.11744999886)
Applying sessions.0001_initial... OK (2.7917330265)
After:
Applying admin.0001_initial... OK (2.94719195366)
Applying commands_sql_migrations.0001_initial... OK (0.251703977585)
Applying fixtures_migration.0001_initial... OK (0.135380983353)
Applying sites.0001_initial... OK (0.141701936722)
Applying flatpages.0001_initial... OK (0.408670186996)
Applying migration_test_data_persistence.0001_initial... OK
(0.138519048691)
Applying migration_test_data_persistence.0002_add_book... OK
(0.0787410736084)
Applying postgres_tests.0001_setup_extensions... OK (0.284782886505)
Applying postgres_tests.0002_create_test_models... OK (0.369908094406)
Applying redirects.0001_initial... OK (0.170851945877)
Applying sessions.0001_initial... OK (0.371510028839)
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:4>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
As noted on the PR, "There is still an unsolved issue, though, with the
line `if app_label == settings.AUTH_USER_MODEL and ignore_swappable:`
which is untested and would fail as `ignore_swappable` is no more."
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:5>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* severity: Normal => Release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:6>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I did some research, and I can hit the logic in question:
{{{
if app_label == settings.AUTH_USER_MODEL and ignore_swappable:
continue
}}}
on both master and with Claude's patch if I follow the steps in this
comment: https://code.djangoproject.com/ticket/22563#comment:8
Andrew first added an error message saying that changing `AUTH_USER_MODEL`
after creating an initial migration was invalid:
https://github.com/django/django/commit/fc974313b85da932a70f1f993b33207d78d31831
and then someone reporting hitting that error with a false positive and he
changed it to the current version:
https://github.com/django/django/commit/5182efce8d73ec552a1d7f3e86a24369bb261044
His comment was "I've committed a patch that suppresses the error in that
one case (where we know it's safe to do so), and I can now switch
`AUTH_USER_MODEL` even midway through a migration set without errors."
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:7>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Test for `ignore_swappable` from Markus has been added in
fca866763acb6b3414c20ca3772b94cb5d111733.
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:8>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:9>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"057305e588ac90474fef47e2fb2890552a543731"]:
{{{
#!CommitTicketReference repository=""
revision="057305e588ac90474fef47e2fb2890552a543731"
Added ignore_swappable to StateApps
Refs #23745.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:11>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"a159b1facd222c20dde12f1e0117643af528d511"]:
{{{
#!CommitTicketReference repository=""
revision="a159b1facd222c20dde12f1e0117643af528d511"
Replaced migration state render() by apps cached property
Refs #23745.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:10>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"2a9c4b4901d385f68a2c3f9cf98b7a776c2976f0"]:
{{{
#!CommitTicketReference repository=""
revision="2a9c4b4901d385f68a2c3f9cf98b7a776c2976f0"
Passed around the state between migrations
Refs #23745.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:12>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"1aa3e09c2043c88a760e8b73fb95dc8f1ffef50e"]:
{{{
#!CommitTicketReference repository=""
revision="1aa3e09c2043c88a760e8b73fb95dc8f1ffef50e"
Fixed #23745 -- Reused states as much as possible in migrations
Thanks Tim Graham and Markus Holtermann for the reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:13>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sirex):
Recently I started to work on a project, that uses Django 1.7, and found,
that running one small test, takes about 10 seconds, white the test itself
takes 0.001s! I love TDD, but with Django 1.7 it became impossible.
With South, there was
[https://south.readthedocs.org/en/latest/settings.html#south-tests-migrate
SOUTH_TESTS_MIGRATE] option and with this option running single test was
instant.
Also, I have [https://github.com/python-
dirbtuves/akl.lt/blob/master/scripts/runtests.py#L29 a helper script],
that allows to switch database to in memory SQlite easily, this makes
tests even faster, because 98% of all tests works perfectly without
PostgreSQL. So I only switch to PostgreSQL for running tests, when I need
to.
In other words, running one single test has to be instant fast. If Django
itself takes 10 seconds to start, it is deal breaker for everyone who uses
TDD.
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:14>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sirex):
I think, that would be great if Django could have option for test command
to pretend, that none of apps have migrations and would run good old
syncdb.
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:15>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sirex):
Here is my suggestion:
{{{
#!python
if settings.TESTS_MIGRATE:
call_command(
'migrate',
verbosity=max(verbosity - 1, 0),
interactive=False,
database=self.connection.alias,
test_database=True,
test_flush=True,
)
else:
from django.apps import apps
from django.core.management.commands.migrate import Command as
MigrateCommand
command = MigrateCommand()
command.verbosity = 0
command.interactive = False
command.load_initial_data = False
app_labels = [app.label for app in apps.get_app_configs()]
created_models = command.sync_apps(self.connection, app_labels)
}}}
This is just a quick and dirty modification of
`django/db/backends/creation.py:create_test_db`, but the idea is to add
option for not running migrations in tests.
With this modification, my tests became instant fast again.
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:16>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I proposed a similar idea on the mailing list and no one rejected it right
away at least. I think it might be accepted as a new feature for 1.9 --
probably as a test runner flag instead of a setting. Could you please open
a new ticket (and if you are interested in working on this feature, a pull
request)?
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:17>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vstoykov):
I found a snippet for disabling migrations
https://gist.github.com/NotSqrt/5f3c76cd15e40ef62d09 and assembled it in
my settings.py to easy turn off them when I want.
{{{
if os.getenv('DISABLE_MIGRATIONS'):
class DisableMigrations(object):
def __contains__(self, item):
return True
def __getitem__(self, item):
return "notmigrations"
MIGRATION_MODULES = DisableMigrations()
}}}
This is dirty hack put in settings.py but do the job. For Django 1.9 even
can be simplified by {{{return None}}} instead of {{{return
"notmigrations"}}}
Probably some switch to test command {{{--no-migrations}}} or something
that will do something like that can be useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:18>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
New in Django 1.9, you can disable migrations on a per app basis using the
[https://docs.djangoproject.com/en/1.9/ref/settings/#std:setting-
MIGRATION_MODULES MIGRATION_MODULES] setting by setting the app label to
`None` (#24919).
--
Ticket URL: <https://code.djangoproject.com/ticket/23745#comment:19>