Commit 4f17c6b7 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Track enqueued and ongoing MRs

parent d7f61bae
...@@ -318,14 +318,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -318,14 +318,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
elsif @merge_request.head_pipeline.success? elsif @merge_request.head_pipeline.success?
# This can be triggered when a user clicks the auto merge button while # This can be triggered when a user clicks the auto merge button while
# the tests finish at about the same time # the tests finish at about the same time
MergeWorker.perform_async(@merge_request.id, current_user.id, params) @merge_request.async_merge(current_user.id, params)
:success :success
else else
:failed :failed
end end
else else
MergeWorker.perform_async(@merge_request.id, current_user.id, params) @merge_request.async_merge(current_user.id, params)
:success :success
end end
......
...@@ -241,6 +241,14 @@ class MergeRequest < ActiveRecord::Base ...@@ -241,6 +241,14 @@ class MergeRequest < ActiveRecord::Base
end end
end end
# Calls `MergeWorker` to proceed with the merge process and
# updates `merge_jid` with the MergeWorker#jid.
# This helps tracking enqueued and ongoing merge jobs.
def async_merge(user_id, params)
jid = MergeWorker.perform_async(id, user_id, params)
update_column(:merge_jid, jid)
end
def first_commit def first_commit
merge_request_diff ? merge_request_diff.first_commit : compare_commits.first merge_request_diff ? merge_request_diff.first_commit : compare_commits.first
end end
...@@ -384,9 +392,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -384,9 +392,7 @@ class MergeRequest < ActiveRecord::Base
end end
def merge_ongoing? def merge_ongoing?
return false unless merge_jid merge_jid && !merged?
Gitlab::SidekiqStatus.num_running([merge_jid]) > 0
end end
def closed_without_fork? def closed_without_fork?
......
...@@ -31,6 +31,9 @@ module MergeRequests ...@@ -31,6 +31,9 @@ module MergeRequests
end end
rescue MergeError => e rescue MergeError => e
log_merge_error(e.message, save_message_on_model: true) log_merge_error(e.message, save_message_on_model: true)
ensure
# Make sure to clean up merge_jid in the end of the merge process.
merge_request.update_column(:merge_jid, nil)
end end
private private
......
...@@ -30,7 +30,7 @@ module MergeRequests ...@@ -30,7 +30,7 @@ module MergeRequests
next next
end end
MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params) merge_request.async_merge(merge_request.merge_user_id, merge_request.merge_params)
end end
end end
......
...@@ -83,7 +83,7 @@ module MergeRequests ...@@ -83,7 +83,7 @@ module MergeRequests
if merge_request.head_pipeline && merge_request.head_pipeline.active? if merge_request.head_pipeline && merge_request.head_pipeline.active?
MergeRequests::MergeWhenPipelineSucceedsService.new(project, current_user).execute(merge_request) MergeRequests::MergeWhenPipelineSucceedsService.new(project, current_user).execute(merge_request)
else else
MergeWorker.perform_async(merge_request.id, current_user.id, {}) merge_request.async_merge(current_user.id, {})
end end
end end
......
...@@ -7,8 +7,6 @@ class MergeWorker ...@@ -7,8 +7,6 @@ class MergeWorker
current_user = User.find(current_user_id) current_user = User.find(current_user_id)
merge_request = MergeRequest.find(merge_request_id) merge_request = MergeRequest.find(merge_request_id)
merge_request.update_column(:merge_jid, jid)
MergeRequests::MergeService.new(merge_request.target_project, current_user, params) MergeRequests::MergeService.new(merge_request.target_project, current_user, params)
.execute(merge_request) .execute(merge_request)
end end
......
...@@ -931,6 +931,23 @@ describe MergeRequest do ...@@ -931,6 +931,23 @@ describe MergeRequest do
end end
end end
describe '#async_merge' do
it 'enqueues MergeWorker job and updates merge_jid' do
merge_request = create(:merge_request)
user_id = double(:user_id)
params = double(:params)
merge_jid = 'hash-123'
expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do
merge_jid
end
merge_request.async_merge(user_id, params)
expect(merge_request.reload.merge_jid).to eq(merge_jid)
end
end
describe '#check_if_can_be_merged' do describe '#check_if_can_be_merged' do
let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) }
...@@ -1370,29 +1387,11 @@ describe MergeRequest do ...@@ -1370,29 +1387,11 @@ describe MergeRequest do
end end
describe '#merge_ongoing?' do describe '#merge_ongoing?' do
it 'returns true when merge process is ongoing for merge_jid' do it 'returns true when merge_id is present and MR is not merged' do
merge_request = create(:merge_request, merge_jid: 'foo') merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(1)
expect(merge_request.merge_ongoing?).to be(true) expect(merge_request.merge_ongoing?).to be(true)
end end
it 'returns false when no merge process running for merge_jid' do
merge_request = build(:merge_request, merge_jid: 'foo')
allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(0)
expect(merge_request.merge_ongoing?).to be(false)
end
it 'returns false when merge_jid is nil' do
merge_request = build(:merge_request, merge_jid: nil)
expect(Gitlab::SidekiqStatus).not_to receive(:num_running)
expect(merge_request.merge_ongoing?).to be(false)
end
end end
describe "#closed_without_fork?" do describe "#closed_without_fork?" do
......
...@@ -12,6 +12,15 @@ describe MergeRequests::MergeService do ...@@ -12,6 +12,15 @@ describe MergeRequests::MergeService do
end end
describe '#execute' do describe '#execute' do
it 'cleans up merge_jid from MergeRequest' do
merge_request.update_column(:merge_jid, 'hash-123')
service = described_class.new(project, user, commit_message: 'Awesome message')
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
context 'valid params' do context 'valid params' do
let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment