From 2ea5717a9fd2a647a7058b2a219816027c443532 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Tue, 16 Jul 2019 12:19:55 +1200 Subject: Migrations for adding issue_id to versions table These migrations do the following: - Adds a new `issue_id` column to `versions`. This fixes an n+1 problem when loading versions for an issue in GraphQL as AR can now load from cache - Change the unique restraint on versions.sha to be scoped to `issue_id` as in order to import version data, we need to allow duplicate `sha` values for versions - Update all versions with an `issue_id` https://gitlab.com/gitlab-org/gitlab-ee/issues/11090 --- .../20190715042813_add_issue_id_to_versions.rb | 13 ++++++++ ...20190715043954_set_issue_id_for_all_versions.rb | 19 +++++++++++ ...0190715043944_remove_sha_index_from_versions.rb | 17 ++++++++++ ...01_add_unique_issue_id_sha_index_to_versions.rb | 17 ++++++++++ db/schema.rb | 5 ++- .../set_issue_id_for_all_versions_spec.rb | 38 ++++++++++++++++++++++ 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190715042813_add_issue_id_to_versions.rb create mode 100644 db/migrate/20190715043954_set_issue_id_for_all_versions.rb create mode 100644 db/post_migrate/20190715043944_remove_sha_index_from_versions.rb create mode 100644 db/post_migrate/20190715044501_add_unique_issue_id_sha_index_to_versions.rb create mode 100644 spec/migrations/set_issue_id_for_all_versions_spec.rb diff --git a/db/migrate/20190715042813_add_issue_id_to_versions.rb b/db/migrate/20190715042813_add_issue_id_to_versions.rb new file mode 100644 index 00000000000..1cefdbc9df2 --- /dev/null +++ b/db/migrate/20190715042813_add_issue_id_to_versions.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddIssueIdToVersions < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + add_reference :design_management_versions, :issue, index: true, foreign_key: { on_delete: :cascade } + end + + def down + remove_reference :design_management_versions, :issue + end +end diff --git a/db/migrate/20190715043954_set_issue_id_for_all_versions.rb b/db/migrate/20190715043954_set_issue_id_for_all_versions.rb new file mode 100644 index 00000000000..345b749f1a4 --- /dev/null +++ b/db/migrate/20190715043954_set_issue_id_for_all_versions.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class SetIssueIdForAllVersions < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + execute('UPDATE design_management_versions as versions SET issue_id = ( + SELECT design_management_designs.issue_id + FROM design_management_designs + INNER JOIN design_management_designs_versions ON design_management_designs.id = design_management_designs_versions.design_id + WHERE design_management_designs_versions.version_id = versions.id + LIMIT 1 + )') + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20190715043944_remove_sha_index_from_versions.rb b/db/post_migrate/20190715043944_remove_sha_index_from_versions.rb new file mode 100644 index 00000000000..b23abb80dda --- /dev/null +++ b/db/post_migrate/20190715043944_remove_sha_index_from_versions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveShaIndexFromVersions < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + remove_concurrent_index :design_management_versions, :sha + end + + def down + add_concurrent_index :design_management_versions, :sha, unique: true, using: :btree + end +end diff --git a/db/post_migrate/20190715044501_add_unique_issue_id_sha_index_to_versions.rb b/db/post_migrate/20190715044501_add_unique_issue_id_sha_index_to_versions.rb new file mode 100644 index 00000000000..27b0c9648f9 --- /dev/null +++ b/db/post_migrate/20190715044501_add_unique_issue_id_sha_index_to_versions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddUniqueIssueIdShaIndexToVersions < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :design_management_versions, [:sha, :issue_id], unique: true, using: :btree + end + + def down + remove_concurrent_index :design_management_versions, [:sha, :issue_id] + end +end diff --git a/db/schema.rb b/db/schema.rb index 67479937b47..90399e50710 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1106,7 +1106,9 @@ ActiveRecord::Schema.define(version: 2019_07_25_012225) do create_table "design_management_versions", force: :cascade do |t| t.binary "sha", null: false - t.index ["sha"], name: "index_design_management_versions_on_sha", unique: true + t.bigint "issue_id" + t.index ["issue_id"], name: "index_design_management_versions_on_issue_id" + t.index ["sha", "issue_id"], name: "index_design_management_versions_on_sha_and_issue_id", unique: true end create_table "draft_notes", force: :cascade do |t| @@ -3690,6 +3692,7 @@ ActiveRecord::Schema.define(version: 2019_07_25_012225) do add_foreign_key "design_management_designs", "projects", on_delete: :cascade add_foreign_key "design_management_designs_versions", "design_management_designs", column: "design_id", name: "fk_03c671965c", on_delete: :cascade add_foreign_key "design_management_designs_versions", "design_management_versions", column: "version_id", name: "fk_f4d25ba00c", on_delete: :cascade + add_foreign_key "design_management_versions", "issues", on_delete: :cascade add_foreign_key "draft_notes", "merge_requests", on_delete: :cascade add_foreign_key "draft_notes", "users", column: "author_id", on_delete: :cascade add_foreign_key "elasticsearch_indexed_namespaces", "namespaces", on_delete: :cascade diff --git a/spec/migrations/set_issue_id_for_all_versions_spec.rb b/spec/migrations/set_issue_id_for_all_versions_spec.rb new file mode 100644 index 00000000000..bfc2731181b --- /dev/null +++ b/spec/migrations/set_issue_id_for_all_versions_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20190715043954_set_issue_id_for_all_versions.rb') + +describe SetIssueIdForAllVersions, :migration do + let(:projects) { table(:projects) } + let(:issues) { table(:issues) } + let(:designs) { table(:design_management_designs) } + let(:designs_versions) { table(:design_management_designs_versions) } + let(:versions) { table(:design_management_versions) } + + before do + @project = projects.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce', namespace_id: 1) + + @issue_1 = issues.create!(description: 'first', project_id: @project.id) + @issue_2 = issues.create!(description: 'second', project_id: @project.id) + + @design_1 = designs.create!(issue_id: @issue_1.id, filename: 'homepage-1.jpg', project_id: @project.id) + @design_2 = designs.create!(issue_id: @issue_2.id, filename: 'homepage-2.jpg', project_id: @project.id) + + @version_1 = versions.create!(sha: 'foo') + @version_2 = versions.create!(sha: 'bar') + + designs_versions.create!(version_id: @version_1.id, design_id: @design_1.id) + designs_versions.create!(version_id: @version_2.id, design_id: @design_2.id) + end + + it 'correctly sets issue_id' do + expect(versions.where(issue_id: nil).count).to eq(2) + + migrate! + + expect(versions.where(issue_id: nil).count).to eq(0) + expect(versions.find(@version_1.id).issue_id).to eq(@issue_1.id) + expect(versions.find(@version_2.id).issue_id).to eq(@issue_2.id) + end +end -- cgit v1.2.1