From 793b9a5ced5f97feabca329e335f8ce7b1fb6708 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 16 May 2017 21:18:15 +0300 Subject: [PATCH] Improve migration for Multipple issue assignee feature --- .../members/authorized_destroy_service.rb | 4 +- ...0320171632_create_issue_assignees_table.rb | 40 --------- .../20170320173259_migrate_assignees.rb | 8 -- ...3305_migrate_assignee_to_separate_table.rb | 83 +++++++++++++++++++ ...16183131_add_indices_to_issue_assignees.rb | 41 +++++++++ ...170516165238_cleanup_trigger_for_issues.rb | 39 +++++++++ ...dd_constraints_to_issue_assignees_table.rb | 37 +++++++++ db/schema.rb | 8 +- 8 files changed, 206 insertions(+), 54 deletions(-) delete mode 100644 db/migrate/20170320171632_create_issue_assignees_table.rb create mode 100644 db/migrate/20170516153305_migrate_assignee_to_separate_table.rb create mode 100644 db/migrate/20170516183131_add_indices_to_issue_assignees.rb create mode 100644 db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb create mode 100644 db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index 7912cac65d3..eac7ef0c9b7 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -29,7 +29,7 @@ module Members issue_ids = IssuesFinder.new(user, group_id: member.source_id, assignee_id: member.user_id). execute.pluck(:id) - IssueAssignee.destroy_all(issue_id: issue_ids, user_id: member.user_id) + IssueAssignee.delete_all(issue_id: issue_ids, user_id: member.user_id) MergeRequestsFinder.new(user, group_id: member.source_id, assignee_id: member.user_id). execute. @@ -37,7 +37,7 @@ module Members else project = member.source - IssueAssignee.destroy_all( + IssueAssignee.delete_all( user_id: member.user_id, issue_id: project.issues.opened.assigned_to(member.user).select(:id) ) diff --git a/db/migrate/20170320171632_create_issue_assignees_table.rb b/db/migrate/20170320171632_create_issue_assignees_table.rb deleted file mode 100644 index 23b8da37b6d..00000000000 --- a/db/migrate/20170320171632_create_issue_assignees_table.rb +++ /dev/null @@ -1,40 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CreateIssueAssigneesTable < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - INDEX_NAME = 'index_issue_assignees_on_issue_id_and_user_id' - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - # When a migration requires downtime you **must** uncomment the following - # constant and define a short and easy to understand explanation as to why the - # migration requires downtime. - # DOWNTIME_REASON = '' - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - - def up - create_table :issue_assignees do |t| - t.references :user, foreign_key: { on_delete: :cascade }, index: true, null: false - t.references :issue, foreign_key: { on_delete: :cascade }, null: false - end - - add_index :issue_assignees, [:issue_id, :user_id], unique: true, name: INDEX_NAME - end - - def down - drop_table :issue_assignees - end -end diff --git a/db/migrate/20170320173259_migrate_assignees.rb b/db/migrate/20170320173259_migrate_assignees.rb index ba8edbd7d32..23e7500a32d 100644 --- a/db/migrate/20170320173259_migrate_assignees.rb +++ b/db/migrate/20170320173259_migrate_assignees.rb @@ -37,16 +37,8 @@ class MigrateAssignees < ActiveRecord::Migration users.project("true").where(users[:id].eq(table[:assignee_id])).exists.not )) end - - execute <<-EOF - INSERT INTO issue_assignees(issue_id, user_id) - SELECT id, assignee_id FROM issues WHERE assignee_id IS NOT NULL - EOF end def down - execute <<-EOF - DELETE FROM issue_assignees - EOF end end diff --git a/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb b/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb new file mode 100644 index 00000000000..f269ca7fc34 --- /dev/null +++ b/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb @@ -0,0 +1,83 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateAssigneeToSeparateTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def up + drop_table(:issue_assignees) if table_exists?(:issue_assignees) + + if Gitlab::Database.mysql? + execute <<-EOF + CREATE TABLE issue_assignees AS + SELECT assignee_id AS user_id, id AS issue_id FROM issues WHERE assignee_id IS NOT NULL + EOF + else + ActiveRecord::Base.transaction do + execute('LOCK TABLE issues IN EXCLUSIVE MODE') + + execute <<-EOF + CREATE TABLE issue_assignees AS + SELECT assignee_id AS user_id, id AS issue_id FROM issues WHERE assignee_id IS NOT NULL + EOF + + execute <<-EOF + CREATE OR REPLACE FUNCTION replicate_assignee_id() + RETURNS trigger AS + $BODY$ + BEGIN + if OLD.assignee_id IS NOT NULL THEN + DELETE FROM issue_assignees WHERE issue_id = OLD.id; + END IF; + + if NEW.assignee_id IS NOT NULL THEN + INSERT INTO issue_assignees (user_id, issue_id) VALUES (NEW.assignee_id, NEW.id); + END IF; + + RETURN NEW; + END; + $BODY$ + LANGUAGE 'plpgsql' + VOLATILE; + + CREATE TRIGGER replicate_assignee_id + BEFORE INSERT OR UPDATE OF assignee_id + ON issues + FOR EACH ROW EXECUTE PROCEDURE replicate_assignee_id(); + EOF + end + end + end + + def down + drop_table(:issue_assignees) if table_exists?(:issue_assignees) + + if Gitlab::Database.postgresql? + execute <<-EOF + DROP TRIGGER IF EXISTS replicate_assignee_id ON issues; + DROP FUNCTION IF EXISTS replicate_assignee_id(); + EOF + end + end +end diff --git a/db/migrate/20170516183131_add_indices_to_issue_assignees.rb b/db/migrate/20170516183131_add_indices_to_issue_assignees.rb new file mode 100644 index 00000000000..a1f064c6848 --- /dev/null +++ b/db/migrate/20170516183131_add_indices_to_issue_assignees.rb @@ -0,0 +1,41 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndicesToIssueAssignees < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + + def up + add_concurrent_index :issue_assignees, [:issue_id, :user_id], unique: true, name: 'index_issue_assignees_on_issue_id_and_user_id' + add_concurrent_index :issue_assignees, :user_id, name: 'index_issue_assignees_on_user_id' + add_concurrent_foreign_key :issue_assignees, :users, column: :user_id, on_delete: :cascade + add_concurrent_foreign_key :issue_assignees, :issues, column: :issue_id, on_delete: :cascade + end + + def down + remove_foreign_key :issue_assignees, column: :user_id + remove_foreign_key :issue_assignees, column: :issue_id + remove_concurrent_index :issue_assignees, [:issue_id, :user_id] if index_exists?(:issue_assignees, [:issue_id, :user_id]) + remove_concurrent_index :issue_assignees, :user_id if index_exists?(:issue_assignees, :user_id) + end +end diff --git a/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb b/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb new file mode 100644 index 00000000000..378fe5603c3 --- /dev/null +++ b/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb @@ -0,0 +1,39 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CleanupTriggerForIssues < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + + def up + if Gitlab::Database.postgresql? + execute <<-EOF + DROP TRIGGER IF EXISTS replicate_assignee_id ON issues; + DROP FUNCTION IF EXISTS replicate_assignee_id(); + EOF + end + end + + def down + end +end diff --git a/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb b/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb new file mode 100644 index 00000000000..2aab1f4d14f --- /dev/null +++ b/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb @@ -0,0 +1,37 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddConstraintsToIssueAssigneesTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def up + change_column :issue_assignees, :issue_id, :integer, null: false + change_column :issue_assignees, :user_id, :integer, null: false + end + + def down + change_column :issue_assignees, :issue_id, :integer + change_column :issue_assignees, :user_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 42afef7391a..294e0b531eb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170511101000) do +ActiveRecord::Schema.define(version: 20170516183131) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -459,7 +459,7 @@ ActiveRecord::Schema.define(version: 20170511101000) do add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree - create_table "issue_assignees", force: :cascade do |t| + create_table "issue_assignees", id: false, force: :cascade do |t| t.integer "user_id", null: false t.integer "issue_id", null: false end @@ -1423,8 +1423,8 @@ ActiveRecord::Schema.define(version: 20170511101000) do add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade add_foreign_key "ci_variables", "projects", name: "fk_ada5eb64b3", on_delete: :cascade add_foreign_key "container_repositories", "projects" - add_foreign_key "issue_assignees", "issues", on_delete: :cascade - add_foreign_key "issue_assignees", "users", on_delete: :cascade + add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade + add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade