db/migrate/20190826100605_add_group_column_to_events.rb uses `add_reference` which causes downtime
The following discussion from !32257 (merged) should be addressed:
-
@yorickpeterse started a discussion: @jprovaznik @tkuah This migration should have never been merged.
add_reference
usese a regularCREATE INDEX
, which acquires a full table lock on the (massive) events table, causing downtime in the progress. In this case it also caused a statement timeout on canary, hence we were able to catch it.Please read https://docs.gitlab.com/ee/development/what_requires_downtime.html#adding-indexes carefully about how to add indexes the right way. As a sidenote, I am a bit worried this made its way past both RuboCop and a database review. We have to blacklist
add_reference
as soon as possible.@jprovaznik Please create a merge request as soon as possible that uses the right set of methods for creating indexes and foreign keys. You can use the linked document and past migrations as a reference.
Pardon me if I come across as a bit harsh, but we just dodged a cannonball in production by this migration timing out instead of causing downtime.
This migration should be modified so that it does not use add_reference
. This should be done for both CE and EE, and these MRs should be picked into the auto deploy branch (add Pick into auto-deploy to the merge requests for that).
This is a P1/S1 as it prevents us from deploying to canary and production, and cause downtime if it wasn't for our 15 second SQL statement timeout.