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:
authorBob Van Landuyt <bob@gitlab.com>2019-08-13 23:52:01 +0300
committerDouwe Maan <douwe@gitlab.com>2019-08-13 23:52:01 +0300
commit452bc36d603ed89d3fa5e3409338dd905230bd2f (patch)
tree3ef260430db93ef2b9fa9236ea601a0b3e53adee /spec/services
parent1c3b570c117cc41f5f4838a8366c4367ef0749cb (diff)
Rework retry strategy for remote mirrors
**Prevention of running 2 simultaneous updates** Instead of using `RemoteMirror#update_status` and raise an error if it's already running to prevent the same mirror being updated at the same time we now use `Gitlab::ExclusiveLease` for that. When we fail to obtain a lease in 3 tries, 30 seconds apart, we bail and reschedule. We'll reschedule faster for the protected branches. If the mirror already ran since it was scheduled, the job will be skipped. **Error handling: Remote side** When an update fails because of a `Gitlab::Git::CommandError`, we won't track this error in sentry, this could be on the remote side: for example when branches have diverged. In this case, we'll try 3 times scheduled 1 or 5 minutes apart. In between, the mirror is marked as "to_retry", the error would be visible to the user when they visit the settings page. After 3 tries we'll mark the mirror as failed and notify the user. We won't track this error in sentry, as it's not likely we can help it. The next event that would trigger a new refresh. **Error handling: our side** If an unexpected error occurs, we mark the mirror as failed, but we'd still retry the job based on the regular sidekiq retries with backoff. Same as we used to The error would be reported in sentry, since its likely we need to do something about it.
Diffstat (limited to 'spec/services')
-rw-r--r--spec/services/projects/update_remote_mirror_service_spec.rb66
1 files changed, 54 insertions, 12 deletions
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index be2811ab1e7..4396ccab584 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -10,49 +10,91 @@ describe Projects::UpdateRemoteMirrorService do
subject(:service) { described_class.new(project, project.creator) }
- describe "#execute" do
+ describe '#execute' do
+ subject(:execute!) { service.execute(remote_mirror, 0) }
+
before do
project.repository.add_branch(project.owner, 'existing-branch', 'master')
allow(remote_mirror).to receive(:update_repository).and_return(true)
end
- it "ensures the remote exists" do
+ it 'ensures the remote exists' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
expect(remote_mirror).to receive(:ensure_remote!)
- service.execute(remote_mirror)
+ execute!
end
- it "fetches the remote repository" do
+ it 'fetches the remote repository' do
expect(project.repository)
.to receive(:fetch_remote)
- .with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror)
+ .with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror)
- service.execute(remote_mirror)
+ execute!
end
- it "returns success when updated succeeds" do
+ it 'marks the mirror as started when beginning' do
+ expect(remote_mirror).to receive(:update_start!).and_call_original
+
+ execute!
+ end
+
+ it 'marks the mirror as successfully finished' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
- result = service.execute(remote_mirror)
+ result = execute!
expect(result[:status]).to eq(:success)
+ expect(remote_mirror).to be_finished
+ end
+
+ it 'marks the mirror as failed and raises the error when an unexpected error occurs' do
+ allow(project.repository).to receive(:fetch_remote).and_raise('Badly broken')
+
+ expect { execute! }.to raise_error /Badly broken/
+
+ expect(remote_mirror).to be_failed
+ expect(remote_mirror.last_error).to include('Badly broken')
+ end
+
+ context 'when the update fails because of a `Gitlab::Git::CommandError`' do
+ before do
+ allow(project.repository).to receive(:fetch_remote).and_raise(Gitlab::Git::CommandError.new('fetch failed'))
+ end
+
+ it 'wraps `Gitlab::Git::CommandError`s in a service error' do
+ expect(execute!).to eq(status: :error, message: 'fetch failed')
+ end
+
+ it 'marks the mirror as to be retried' do
+ execute!
+
+ expect(remote_mirror).to be_to_retry
+ expect(remote_mirror.last_error).to include('fetch failed')
+ end
+
+ it "marks the mirror as failed after #{described_class::MAX_TRIES} tries" do
+ service.execute(remote_mirror, described_class::MAX_TRIES)
+
+ expect(remote_mirror).to be_failed
+ expect(remote_mirror.last_error).to include('fetch failed')
+ end
end
context 'when syncing all branches' do
- it "push all the branches the first time" do
+ it 'push all the branches the first time' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
expect(remote_mirror).to receive(:update_repository).with({})
- service.execute(remote_mirror)
+ execute!
end
end
context 'when only syncing protected branches' do
- it "sync updated protected branches" do
+ it 'sync updated protected branches' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
protected_branch = create_protected_branch(project)
remote_mirror.only_protected_branches = true
@@ -61,7 +103,7 @@ describe Projects::UpdateRemoteMirrorService do
.to receive(:update_repository)
.with(only_branches_matching: [protected_branch.name])
- service.execute(remote_mirror)
+ execute!
end
def create_protected_branch(project)