Closed
Bug 1320136
Opened 8 years ago
Closed 8 years ago
Squash Django migration files
Categories
(Tree Management :: Treeherder, defect, P3)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(5 files)
Between the `model`, `perf` and `credentials` apps, we have 68 migrations in the repo.
Squashing these will:
* reduce the time taken to run `./manage.py migrate` (which for a new Vagrant instance is currently ~50 seconds)
* get rid of redundant cruft from the repo
Comment 1•8 years ago
|
||
I believe the large number of migrations are also responsible for our long unit test setup times.
Assignee | ||
Comment 2•8 years ago
|
||
Adding the Django 1.10 update bug as a dep, since it has improved migration squashing logic, plus the new `elidable=True` feature for `RunSQL` commands:
https://docs.djangoproject.com/en/1.10/ref/migration-operations/#runsql
Depends on: 1311967
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8827266 [details]
[treeherder] mozilla:cleanup-runsql-operations > mozilla:master
Initial cleanup prior to squashing.
Attachment #8827266 -
Flags: review?(wlachance)
Comment 5•8 years ago
|
||
Comment on attachment 8827266 [details]
[treeherder] mozilla:cleanup-runsql-operations > mozilla:master
One question in review.
Attachment #8827266 -
Flags: review?(wlachance)
Updated•8 years ago
|
Attachment #8827266 -
Flags: review+
Comment 6•8 years ago
|
||
Commits pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/67741306284c8bd43738c80c630beb5ac0ec7429
Bug 1320136 - Mark redundant RunSQL migration operations as elidable
Django's `squashmigrations` command cannot optimise past `RunSQL` or
`RunPython` migration operations, since it cannot guarantee that the
order of migration operations isn't important. However as of Django 1.10
these commands can be marked as `elidable`, if they are not required in
the final migration (eg if they only manipulate legacy data and have
already run in all necessary environments), which solves this problem.
https://github.com/mozilla/treeherder/commit/6e507e7affdc40f69c00179878a99bb0e7d2a530
Bug 1320136 - Clean up the signature prefix index RunSQL operation
* Uses the list notation form for the sql arguments (avoids the need for
the `sqlparse` dependency.
* Correctly passes `reverse_sql` as a keyword argument.
See:
https://docs.djangoproject.com/en/1.10/ref/migration-operations/#runsql
https://github.com/mozilla/treeherder/commit/dd164fd5ac8f441a8128fe91b70b5b6fb9fcf08a
Bug 1320136 - Combine migrations for failureline composite indexes
Two composite indexes (that contain prefix indexes so have to be created
manually) were created in earlier migrations and then later dropped and
recreated in 0028_test_match_created_index.py. That migration landed in
June 2016, so has had plenty of time to run in all environments.
https://github.com/mozilla/treeherder/commit/5bfc824d115e5314278d421e3334b837006696e8
Bug 1320136 - Convert jobdetail RunSQL operation to AlterUniqueTogether
The `RunSQL` operation was only required to handle legacy data, which
has already happened in all relevant environments since this migration
landed in August 2016.
Comment 7•8 years ago
|
||
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
These schema dumps were generated by:
* Purging the existing DB:
mysql -uroot -e 'DROP DATABASE if exists treeherder; create database treeherder;'
* Dumping the schema (sed usage to reduce noise in diff):
mysqldump -u root --no-data --compact treeherder | sed -r 's/(KEY|CONSTRAINT) `[^`]+` /\1 `<NAME>` /' | sed -r 's/AUTO_INCREMENT=[0-9]+ /AUTO_INCREMENT=NNN /' > schema-after.sql
...and then making a couple of no-op tweaks to the field/index order of the "after" schema to ensure the diff was an clean as possible (there's no way to get MySQL to dump eg an alphabetical field/index order, it's done in the order on disk, which if different for a CREATE TABLE+ALTER TABLE vs a squashed CREATE TABLE etc).
Assignee | ||
Comment 10•8 years ago
|
||
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8830792 [details]
[treeherder] mozilla:squash-migrations > mozilla:master
The easiest way of reviewing this is probably to just look at the attached schema diff. These changes will be a no-op on stage/prod (since it will have already run all of the migrations that this encompasses).
In brand new Vagrant instances, only the squashed migration will run, which will mean the schema has the various fixes that can be seen in the diff. Bug 1303763 will be where any deviations between this clean-slate schema and prod are fixed.
For existing Vagrant instances, when migrate is run, it will "catch up" using the old migrations files (since the squashed migration cannot be applied to in-between states), so the squashed migration will be a no-op there too.
Once enough time has passed for all environments (including Vagrant instances of contributors) to be up to date with the squashed migration, the old migrations files can be deleted. (Doing so too soon will mean people will get errors when running migrate and would have to drop-recreate the DB.)
Attachment #8830792 -
Flags: review?(wlachance)
Assignee | ||
Updated•8 years ago
|
Attachment #8830798 -
Attachment mime type: text/x-sql → text/plain
Comment 12•8 years ago
|
||
Comment on attachment 8830792 [details]
[treeherder] mozilla:squash-migrations > mozilla:master
This is great, thank you so much for doing this.
Attachment #8830792 -
Flags: review?(wlachance) → review+
Comment 13•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/ad3a85d0875eb4b083335ff9ac7a09811d5c5ea1
Bug 1320136 - Squash Django migrations
This reduces the time for Vagrant provision & also stops us hitting
some Django migration bugs around index changes (which was causing
redundant indexes on fields that were unique keys as well as a missing
index on `signature_hash`).
Django has a built-in migrations squashing feature:
https://docs.djangoproject.com/en/1.10/topics/migrations/#squashing-migrations
...however it cannot optimise across RunSQL commands and several other
cases, so results in poorly optimised migrations. As such these squashed
migrations are a combination of the output of squashmigrations:
./manage.py squashmigrations credentials 0003
./manage.py squashmigrations model 0053
./manage.py squashmigrations perf 0032
./manage.py squashmigrations seta 0002
...supplemented with the output from deleting all migrations and
running `./manage.py makemigrations` from scratch.
For apps that have already run all migrations that this squashed
migration replaces, this migrations file is a no-op.
Once enough time has passed for all environments (including local
Vagrant instances) to be up to date with the squashed migration, the
old migrations files can be deleted.
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•