From f8290c2862c04f9f4cd4973824ea732ef7f6871b Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 10 Jun 2016 10:00:32 +0200 Subject: Fix timing issues on convertion migration award emoji This commit does two things: 1. It adds logic which prevents timing issues when running the migration. During the migration, notes can be created which _should_ be award emoji and thus migrated. To prevent these timing issues, a lock is obtained on the table (MySQL) or on Transaction level (PG). 2. There was no down migration before as you'd probably lose some data. Data effected is all awards on notes. These could be migrated back, as the noteable type would just be Note, though this would litter the DB with data which should not be there. This down migration does not yet delete the table. --- ...0416182152_convert_award_note_to_emoji_award.rb | 37 +++++++++++++++++++--- db/migrate/20160416190505_remove_note_is_award.rb | 6 ---- 2 files changed, 33 insertions(+), 10 deletions(-) delete mode 100644 db/migrate/20160416190505_remove_note_is_award.rb diff --git a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb index c226bc11f6c..b523f17417a 100644 --- a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb +++ b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb @@ -1,10 +1,39 @@ # rubocop:disable all class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration - def change - def up - execute "INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" + disable_ddl_transaction! - execute "DELETE FROM notes WHERE is_award = true" + def up + if Gitlab::Database.postgresql? + migrate_postgresql + else + migrate_mysql end end + + def down + add_column :notes, :is_award, :boolean + + # This migration does NOT move the awards on notes, if the table is dropped in another migration, these notes will be lost. + execute "INSERT INTO notes (noteable_type, noteable_id, author_id, note, created_at, updated_at, is_award) (SELECT awardable_type, awardable_id, user_id, name, created_at, updated_at, TRUE FROM award_emoji)" + end + + def migrate_postgresql + connection.transaction do + execute 'LOCK notes IN EXCLUSIVE' + migrate_notes + end + end + + def migrate_mysql + execute 'LOCK TABLES notes WRITE' + migrate_notes + ensure + execute 'UNLOCK TABLES' + end + + def migrate_notes + execute "INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" + execute "DELETE FROM notes WHERE is_award = true" + remove_column :notes, :is_award, :boolean + end end diff --git a/db/migrate/20160416190505_remove_note_is_award.rb b/db/migrate/20160416190505_remove_note_is_award.rb deleted file mode 100644 index dd24917feb9..00000000000 --- a/db/migrate/20160416190505_remove_note_is_award.rb +++ /dev/null @@ -1,6 +0,0 @@ -# rubocop:disable all -class RemoveNoteIsAward < ActiveRecord::Migration - def change - remove_column :notes, :is_award, :boolean - end -end -- cgit v1.2.1 From fc5b3a8fa51a4cbc116cc7e702602dd03cb726e1 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 10 Jun 2016 17:33:23 +0200 Subject: Fix MySQL migration, obtain lock the right way As suggested by @yorrickpeterse in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4581#note_12373882 the locking of the MySQL database wasn't correct. --- .../20160416182152_convert_award_note_to_emoji_award.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb index b523f17417a..3906ab79398 100644 --- a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb +++ b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb @@ -20,20 +20,21 @@ class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration def migrate_postgresql connection.transaction do execute 'LOCK notes IN EXCLUSIVE' - migrate_notes + execute "INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" + execute "DELETE FROM notes WHERE is_award = true" + remove_column :notes, :is_award, :boolean end end def migrate_mysql - execute 'LOCK TABLES notes WRITE' - migrate_notes - ensure - execute 'UNLOCK TABLES' - end + execute <<-EOF + lock tables notes WRITE, award_emoji WRITE; + INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true); + EOF - def migrate_notes - execute "INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" execute "DELETE FROM notes WHERE is_award = true" remove_column :notes, :is_award, :boolean + ensure + execute 'UNLOCK TABLES' end end -- cgit v1.2.1 From d032c6b0ff77a9a7568e1be8c728e252ddc1b11a Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 13 Jun 2016 14:15:11 +0200 Subject: Move LOCK TABLES to a separate execute MySQL apparently doesn't support executing multiple queries in the same `execute` call so we have to use a separate one for the "LOCK TABLES" statement. --- db/migrate/20160416182152_convert_award_note_to_emoji_award.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb index 3906ab79398..6d57b796151 100644 --- a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb +++ b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb @@ -27,11 +27,8 @@ class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration end def migrate_mysql - execute <<-EOF - lock tables notes WRITE, award_emoji WRITE; - INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true); - EOF - + execute 'LOCK TABLES notes WRITE, award_emoji WRITE;' + execute 'INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true);' execute "DELETE FROM notes WHERE is_award = true" remove_column :notes, :is_award, :boolean ensure -- cgit v1.2.1 From c6744b49497fca340c8ee5b9913f805ec8ea9be8 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 14 Jun 2016 12:17:41 +0200 Subject: Fixed locking syntax for PostgreSQL --- db/migrate/20160416182152_convert_award_note_to_emoji_award.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb index 6d57b796151..95ee03611d9 100644 --- a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb +++ b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb @@ -19,7 +19,7 @@ class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration def migrate_postgresql connection.transaction do - execute 'LOCK notes IN EXCLUSIVE' + execute 'LOCK notes IN EXCLUSIVE MODE' execute "INSERT INTO award_emoji (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" execute "DELETE FROM notes WHERE is_award = true" remove_column :notes, :is_award, :boolean -- cgit v1.2.1