Bug 1423549 - PerformanceSignature specifies unique_together twice, so the first is ignored #3979
Conversation
|
|
||
| operations = [ | ||
| migrations.AlterUniqueTogether( | ||
| name='performancesignature', |
There was a problem hiding this comment.
How big is this table? (Time taken to run the migration)
Could we try the migration on prototype? Also worth noting that there's presumably a chance the constraint could fail when it runs on production, due to it not having been enforced before.
There was a problem hiding this comment.
How big is this table? (Time taken to run the migration)
I couldn't say something about the duration of running this. ATM the performance_signature table contains around 350K rows.
Could we try the migration on prototype?
Yes, we should do that.
Also worth noting that there's presumably a chance the constraint could fail when it runs on production, due to it not having been enforced before.
@edmorley How easy is it for us to recover from a migration exception during deployment?
There was a problem hiding this comment.
How easy is it for us to recover from a migration exception during deployment?
I think in this case the new indexes/constraints would just get rolled back, but you'd need to check the MySQL docs or with a DBA to be sure.
There was a problem hiding this comment.
ATM the performance_signature table contains around 350K rows.
Ah small enough that time taken won't be an issue (guessing seconds to minutes).
There was a problem hiding this comment.
I found this link which says there are DDL statements (including table altering) that can not be rolled back. Unfortunately, it doesn't mention about ALTER TABLE ADD CONSTRAINT UNIQUE specifically.
There was a problem hiding this comment.
@edmorley Found this in the MySQL docs:
When you create a UNIQUE or PRIMARY KEY index, MySQL must do some extra work. For UNIQUE indexes, MySQL checks that the table contains no duplicate values for the key.
This sounds like we would only get a console error, but the database shouldn't have problems (schema & data wise). This person here confirms my assumption, saying that an error message will be prompted if duplicated data already exists.
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('model', '0008_remove_failure_match'), |
There was a problem hiding this comment.
Could you adjust this for the new migrations on master? (s/0008_remove_failure_match/0010_remove_runnable_job/)
There was a problem hiding this comment.
Could you adjust this for the new migrations on
master? (s/0008_remove_failure_match/0010_remove_runnable_job/)
Done.
There was a problem hiding this comment.
Travis is failing; the dependency now references a deleted migration
ionutgoldan
force-pushed
the
fix-unique-together
branch
from
8050ae2
to
36b248a
Compare
ionutgoldan
force-pushed
the
fix-unique-together
branch
from
36b248a
to
d3d1dfd
Compare
|
@edmorley I deployed this on treeherder-prototype last Friday, tested it a little bit and all went fine. It's good to land. |
|
Sounds good - I'll merge now, thank you :-) |
No description provided.