Add a column linking an MR to its diff
We already had this the other way around (merge_request_diffs.merge_request_id), but this is needed to gather only the most recent diffs for a set of merge requests.
This commit is contained in:
parent
bfb5107ae7
commit
d8299e320e
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Make finding most recent merge request diffs more efficient
|
||||||
|
merge_request: 15035
|
||||||
|
author:
|
||||||
|
type: performance
|
|
@ -0,0 +1,26 @@
|
||||||
|
class AddLatestMergeRequestDiffIdToMergeRequests < ActiveRecord::Migration
|
||||||
|
include Gitlab::Database::MigrationHelpers
|
||||||
|
|
||||||
|
DOWNTIME = false
|
||||||
|
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def up
|
||||||
|
add_column :merge_requests, :latest_merge_request_diff_id, :integer
|
||||||
|
add_concurrent_index :merge_requests, :latest_merge_request_diff_id
|
||||||
|
|
||||||
|
add_concurrent_foreign_key :merge_requests, :merge_request_diffs,
|
||||||
|
column: :latest_merge_request_diff_id,
|
||||||
|
on_delete: :nullify
|
||||||
|
end
|
||||||
|
|
||||||
|
def down
|
||||||
|
remove_foreign_key :merge_requests, column: :latest_merge_request_diff_id
|
||||||
|
|
||||||
|
if index_exists?(:merge_requests, :latest_merge_request_diff_id)
|
||||||
|
remove_concurrent_index :merge_requests, :latest_merge_request_diff_id
|
||||||
|
end
|
||||||
|
|
||||||
|
remove_column :merge_requests, :latest_merge_request_diff_id
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,27 @@
|
||||||
|
class PopulateMergeRequestsLatestMergeRequestDiffId < ActiveRecord::Migration
|
||||||
|
include Gitlab::Database::MigrationHelpers
|
||||||
|
|
||||||
|
DOWNTIME = false
|
||||||
|
BATCH_SIZE = 1_000
|
||||||
|
|
||||||
|
class MergeRequest < ActiveRecord::Base
|
||||||
|
self.table_name = 'merge_requests'
|
||||||
|
|
||||||
|
include ::EachBatch
|
||||||
|
end
|
||||||
|
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def up
|
||||||
|
update = '
|
||||||
|
latest_merge_request_diff_id = (
|
||||||
|
SELECT MAX(id)
|
||||||
|
FROM merge_request_diffs
|
||||||
|
WHERE merge_requests.id = merge_request_diffs.merge_request_id
|
||||||
|
)'.squish
|
||||||
|
|
||||||
|
MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation|
|
||||||
|
relation.update_all(update)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -11,7 +11,7 @@
|
||||||
#
|
#
|
||||||
# It's strongly recommended that you check this file into your version control system.
|
# It's strongly recommended that you check this file into your version control system.
|
||||||
|
|
||||||
ActiveRecord::Schema.define(version: 20171017145932) do
|
ActiveRecord::Schema.define(version: 20171026082505) do
|
||||||
|
|
||||||
# These are extensions that must be enabled in order to support this database
|
# These are extensions that must be enabled in order to support this database
|
||||||
enable_extension "plpgsql"
|
enable_extension "plpgsql"
|
||||||
|
@ -973,6 +973,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do
|
||||||
t.boolean "ref_fetched"
|
t.boolean "ref_fetched"
|
||||||
t.string "merge_jid"
|
t.string "merge_jid"
|
||||||
t.boolean "discussion_locked"
|
t.boolean "discussion_locked"
|
||||||
|
t.integer "latest_merge_request_diff_id"
|
||||||
end
|
end
|
||||||
|
|
||||||
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
|
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
|
||||||
|
@ -981,6 +982,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do
|
||||||
add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree
|
add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree
|
||||||
add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
|
add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
|
||||||
add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
|
add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
|
||||||
|
add_index "merge_requests", ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id", using: :btree
|
||||||
add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree
|
add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree
|
||||||
add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree
|
add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree
|
||||||
add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree
|
add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree
|
||||||
|
@ -1846,6 +1848,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do
|
||||||
add_foreign_key "merge_request_metrics", "ci_pipelines", column: "pipeline_id", on_delete: :cascade
|
add_foreign_key "merge_request_metrics", "ci_pipelines", column: "pipeline_id", on_delete: :cascade
|
||||||
add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade
|
add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade
|
||||||
add_foreign_key "merge_requests", "ci_pipelines", column: "head_pipeline_id", name: "fk_fd82eae0b9", on_delete: :nullify
|
add_foreign_key "merge_requests", "ci_pipelines", column: "head_pipeline_id", name: "fk_fd82eae0b9", on_delete: :nullify
|
||||||
|
add_foreign_key "merge_requests", "merge_request_diffs", column: "latest_merge_request_diff_id", name: "fk_06067f5644", on_delete: :nullify
|
||||||
add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade
|
add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade
|
||||||
add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade
|
add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade
|
||||||
add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade
|
add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade
|
||||||
|
|
|
@ -113,6 +113,7 @@ excluded_attributes:
|
||||||
- :milestone_id
|
- :milestone_id
|
||||||
- :ref_fetched
|
- :ref_fetched
|
||||||
- :merge_jid
|
- :merge_jid
|
||||||
|
- :merge_request_diff_id
|
||||||
award_emoji:
|
award_emoji:
|
||||||
- :awardable_id
|
- :awardable_id
|
||||||
statuses:
|
statuses:
|
||||||
|
|
|
@ -0,0 +1,61 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
require Rails.root.join('db', 'post_migrate', '20171026082505_populate_merge_requests_latest_merge_request_diff_id')
|
||||||
|
|
||||||
|
describe PopulateMergeRequestsLatestMergeRequestDiffId, :migration do
|
||||||
|
let(:projects_table) { table(:projects) }
|
||||||
|
let(:merge_requests_table) { table(:merge_requests) }
|
||||||
|
let(:merge_request_diffs_table) { table(:merge_request_diffs) }
|
||||||
|
|
||||||
|
let(:project) { projects_table.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce') }
|
||||||
|
|
||||||
|
def create_mr!(name, diffs: 0)
|
||||||
|
merge_request =
|
||||||
|
merge_requests_table.create!(target_project_id: project.id,
|
||||||
|
target_branch: 'master',
|
||||||
|
source_project_id: project.id,
|
||||||
|
source_branch: name,
|
||||||
|
title: name)
|
||||||
|
|
||||||
|
diffs.times do
|
||||||
|
merge_request_diffs_table.create!(merge_request_id: merge_request.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
merge_request
|
||||||
|
end
|
||||||
|
|
||||||
|
def diffs_for(merge_request)
|
||||||
|
merge_request_diffs_table.where(merge_request_id: merge_request.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#up' do
|
||||||
|
it 'ignores MRs without diffs' do
|
||||||
|
merge_request_without_diff = create_mr!('without_diff')
|
||||||
|
|
||||||
|
expect(merge_request_without_diff.latest_merge_request_diff_id).to be_nil
|
||||||
|
|
||||||
|
expect { migrate! }
|
||||||
|
.not_to change { merge_request_without_diff.reload.latest_merge_request_diff_id }
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'ignores MRs that have a diff ID already set' do
|
||||||
|
merge_request_with_multiple_diffs = create_mr!('with_multiple_diffs', diffs: 3)
|
||||||
|
diff_id = diffs_for(merge_request_with_multiple_diffs).minimum(:id)
|
||||||
|
|
||||||
|
merge_request_with_multiple_diffs.update!(latest_merge_request_diff_id: diff_id)
|
||||||
|
|
||||||
|
expect { migrate! }
|
||||||
|
.not_to change { merge_request_with_multiple_diffs.reload.latest_merge_request_diff_id }
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'migrates multiple MR diffs to the correct values' do
|
||||||
|
merge_requests = Array.new(3).map.with_index { |_, i| create_mr!(i, diffs: 3) }
|
||||||
|
|
||||||
|
migrate!
|
||||||
|
|
||||||
|
merge_requests.each do |merge_request|
|
||||||
|
expect(merge_request.reload.latest_merge_request_diff_id)
|
||||||
|
.to eq(diffs_for(merge_request).maximum(:id))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue