Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-08-15 21:54:08 +0300
committerMayra Cabrera <mcabrera@gitlab.com>2019-08-15 21:54:08 +0300
commitd31b733fee8fc739157ca0dc6f0d6fba7a8efc21 (patch)
tree80ebfff6d4b94445954aa63e3c3eab476422454c
parentcf5d64e25061411e0b28040afcaa08215efb949d (diff)
Only read rebase status from the model
Prior to 12.1, rebase status was looked up directly from Gitaly. In https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/14417 , a DB column was added to track the status instead. However, we couldn't stop looking at the gitaly status immediately, since some rebases may been running across the upgrade. Now that we're in 12.3, it is safe to remove the direct-to-gitaly lookup. This also happens to fix a 500 error that is seen when viewing an MR for a fork where the source project has been removed. We still look at the Gitaly status in the service, just in case Gitaly and Sidekiq get out of sync - I assume this is possible, and it's a relatively cheap check. Since we atomically check and set `merge_requests.rebase_jid`, we should never enqueue two `RebaseWorker` jobs in parallel.
-rw-r--r--app/models/merge_request.rb13
-rw-r--r--app/services/merge_requests/rebase_service.rb3
-rw-r--r--spec/models/merge_request_spec.rb39
-rw-r--r--spec/services/merge_requests/rebase_service_spec.rb2
4 files changed, 4 insertions, 53 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 4306dd9266f..bfd636fa62a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -220,18 +220,7 @@ class MergeRequest < ApplicationRecord
end
def rebase_in_progress?
- (rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid)) ||
- gitaly_rebase_in_progress?
- end
-
- # TODO: remove the Gitaly lookup after v12.1, when rebase_jid will be reliable
- def gitaly_rebase_in_progress?
- strong_memoize(:gitaly_rebase_in_progress) do
- # The source project can be deleted
- next false unless source_project
-
- source_project.repository.rebase_in_progress?(id)
- end
+ rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid)
end
# Use this method whenever you need to make sure the head_pipeline is synced with the
diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb
index 8d3b9b05819..27c16ba1777 100644
--- a/app/services/merge_requests/rebase_service.rb
+++ b/app/services/merge_requests/rebase_service.rb
@@ -15,7 +15,8 @@ module MergeRequests
end
def rebase
- if merge_request.gitaly_rebase_in_progress?
+ # Ensure Gitaly isn't already running a rebase
+ if source_project.repository.rebase_in_progress?(merge_request.id)
log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true)
return false
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 53424204db7..d344a6d0f0d 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -3015,9 +3015,6 @@ describe MergeRequest do
subject { merge_request.rebase_in_progress? }
it do
- # Stub out the legacy gitaly implementation
- allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false }
-
allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid }
merge_request.rebase_jid = rebase_jid
@@ -3027,42 +3024,6 @@ describe MergeRequest do
end
end
- describe '#gitaly_rebase_in_progress?' do
- let(:repo_path) do
- Gitlab::GitalyClient::StorageSettings.allow_disk_access do
- subject.source_project.repository.path
- end
- end
- let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") }
-
- before do
- system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master))
- end
-
- it 'returns true when there is a current rebase directory' do
- expect(subject.rebase_in_progress?).to be_truthy
- end
-
- it 'returns false when there is no rebase directory' do
- FileUtils.rm_rf(rebase_path)
-
- expect(subject.rebase_in_progress?).to be_falsey
- end
-
- it 'returns false when the rebase directory has expired' do
- time = 20.minutes.ago.to_time
- File.utime(time, time, rebase_path)
-
- expect(subject.rebase_in_progress?).to be_falsey
- end
-
- it 'returns false when the source project has been removed' do
- allow(subject).to receive(:source_project).and_return(nil)
-
- expect(subject.rebase_in_progress?).to be_falsey
- end
- end
-
describe '#allow_collaboration' do
let(:merge_request) do
build(:merge_request, source_branch: 'fixes', allow_collaboration: true)
diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb
index ee9caaf2f47..7b8c94c86fe 100644
--- a/spec/services/merge_requests/rebase_service_spec.rb
+++ b/spec/services/merge_requests/rebase_service_spec.rb
@@ -25,7 +25,7 @@ describe MergeRequests::RebaseService do
describe '#execute' do
context 'when another rebase is already in progress' do
before do
- allow(merge_request).to receive(:gitaly_rebase_in_progress?).and_return(true)
+ allow(repository).to receive(:rebase_in_progress?).with(merge_request.id).and_return(true)
end
it 'saves the error message' do