1320136 - Squash Django migration files

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P3)

Description

[Ed Morley [:emorley]]

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

[William Lachance (:wlach)]

I believe the large number of migrations are also responsible for our long unit test setup times.

Comment 2

[Ed Morley [:emorley]]

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

Comment 3

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:cleanup-runsql-operations > mozilla:master

Comment 4

[Ed Morley [:emorley]]

Comment on attachment 8827266 [details] [review]
[treeherder] mozilla:cleanup-runsql-operations > mozilla:master

Initial cleanup prior to squashing.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 8827266 [details] [review]
[treeherder] mozilla:cleanup-runsql-operations > mozilla:master

One question in review.

Comment 6

[Treeherder GitHub Bugbot]

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

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:squash-migrations > mozilla:master

Comment 8

[Ed Morley [:emorley]]

Attachment: Schema before squash

Comment 9

[Ed Morley [:emorley]]

Attachment: Schema after squash

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).

Comment 10

[Ed Morley [:emorley]]

Attachment: Schema diff

Comment 11

[Ed Morley [:emorley]]

Comment on attachment 8830792 [details] [review]
[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.)

Comment 12

[William Lachance (:wlach)]

Comment on attachment 8830792 [details] [review]
[treeherder] mozilla:squash-migrations > mozilla:master

This is great, thank you so much for doing this.

Comment 13

[Treeherder GitHub Bugbot]

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.