Commit a54d67bd authored by Douwe Maan's avatar Douwe Maan

Merge branch '2831-hard-fail-mirror-when-retry-count-number-exceeds-the-limit' into 'master'

Add maximum retry count migration and related logic

Closes #2831 and gitlab-ce#33872

See merge request gitlab-org/gitlab-ee!3117
parents 278a7561 333728a4
module MirrorHelper
def render_mirror_failed_message(raw_message:)
mirror_last_update_at = @project.mirror_last_update_at
message = "The repository failed to update #{time_ago_with_tooltip(mirror_last_update_at)}.".html_safe
return message if raw_message
message.insert(0, "#{icon('warning triangle')} ")
if can?(current_user, :admin_project, @project)
link_to message, project_mirror_path(@project)
else
message
end
end
def branch_diverged_tooltip_message
message = s_('Branches|The branch could not be updated automatically because it has diverged from its upstream counterpart.')
......
......@@ -512,12 +512,6 @@ module ProjectsHelper
end
end
def can_force_update_mirror?(project)
return true unless project.mirror_last_update_at
Time.now - project.mirror_last_update_at >= 5.minutes
end
def membership_locked?
if @project.group && @project.group.membership_lock
true
......
class ProjectMirrorData < ActiveRecord::Base
include Gitlab::CurrentSettings
BACKOFF_PERIOD = 24.seconds
JITTER = 6.seconds
......@@ -9,21 +7,19 @@ class ProjectMirrorData < ActiveRecord::Base
validates :project, presence: true
validates :next_execution_timestamp, presence: true
before_validation on: :create do
self.next_execution_timestamp = Time.now
end
before_validation :set_next_execution_to_now, on: :create
def reset_retry_count!
def reset_retry_count
self.retry_count = 0
end
def increment_retry_count!
def increment_retry_count
self.retry_count += 1
end
# We schedule the next sync time based on the duration of the
# last mirroring period and add it a fixed backoff period with a random jitter
def set_next_execution_timestamp!
def set_next_execution_timestamp
timestamp = Time.now
retry_factor = [1, self.retry_count].max
delay = [base_delay(timestamp), Gitlab::Mirror.min_delay].max
......@@ -32,8 +28,12 @@ class ProjectMirrorData < ActiveRecord::Base
self.next_execution_timestamp = timestamp + delay
end
def set_next_execution_to_now!
self.update_attributes(next_execution_timestamp: Time.now)
def set_next_execution_to_now
self.next_execution_timestamp = Time.now
end
def retry_limit_exceeded?
self.retry_count > Gitlab::Mirror::MAX_RETRY
end
private
......
- if @project.mirror_last_update_failed?
.panel.panel-danger
.panel-heading
The repository failed to update #{time_ago_with_tooltip(@project.mirror_last_update_at)}.
- if @project.mirror_ever_updated_successfully?
Last successful update #{time_ago_with_tooltip(@project.mirror_last_successful_update_at)}.
= render partial: 'shared/mirror_status', locals: { raw_message: true }
.panel-body
%pre
:preserve
......
- last_successful_update_at = @project.mirror_last_successful_update_at
- raw_message = local_assigns.fetch(:raw_message, false)
- case @project.mirror_last_update_status
- when :success
Updated #{time_ago_with_tooltip(@project.mirror_last_successful_update_at)}.
Updated #{time_ago_with_tooltip(last_successful_update_at)}.
- when :failed
- if can?(current_user, :admin_project, @project)
= link_to project_mirror_path(@project) do
= icon("exclamation-triangle")
Failed to update #{time_ago_with_tooltip(@project.mirror_last_update_at)}.
- else
= icon("exclamation-triangle")
Failed to update #{time_ago_with_tooltip(@project.mirror_last_update_at)}.
= render_mirror_failed_message(raw_message: raw_message)
- if @project.mirror_hard_failed?
%br
Repository mirroring has been paused due to too many failed attempts, and can be resumed by a project admin.
- if @project.mirror_ever_updated_successfully?
Last successful update #{time_ago_with_tooltip(@project.mirror_last_successful_update_at)}.
%br
Last successful update #{time_ago_with_tooltip(last_successful_update_at)}.
......@@ -17,6 +17,6 @@
= link_to update_now_project_mirror_path(@project), method: :post, class: 'btn' do
= icon("refresh")
Update Now
- if @project.mirror_last_update_success?
- if @project.mirror_last_update_succeeded?
%p.inline.prepend-left-10
Successfully updated #{time_ago_with_tooltip(@project.mirror_last_successful_update_at)}.
---
title: Mirrors can now hard fail, keeping them from being retried until a project
admin takes action.
merge_request: 3117
author:
type: added
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddCompositeIndexToProjectMirrorDataNextExecutionTimestampAndRetryCount < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'index_mirror_data_on_next_execution_and_retry_count'
disable_ddl_transaction!
def up
add_concurrent_index(:project_mirror_data, [:next_execution_timestamp, :retry_count], name: INDEX_NAME)
end
def down
if index_exists?(:project_mirror_data, [:next_execution_timestamp, :retry_count], name: INDEX_NAME)
remove_concurrent_index(:project_mirror_data, [:next_execution_timestamp, :retry_count], name: INDEX_NAME)
end
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20171026082505) do
ActiveRecord::Schema.define(version: 20171107090120) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -1585,6 +1585,7 @@ ActiveRecord::Schema.define(version: 20171026082505) do
t.datetime_with_timezone "updated_at"
end
add_index "project_mirror_data", ["next_execution_timestamp", "retry_count"], name: "index_mirror_data_on_next_execution_and_retry_count", using: :btree
add_index "project_mirror_data", ["project_id"], name: "index_project_mirror_data_on_project_id", unique: true, using: :btree
create_table "project_statistics", force: :cascade do |t|
......
......@@ -84,6 +84,22 @@ this branch to prevent any changes from being lost.
![Diverged branch](repository_mirroring/repository_mirroring_diverged_branch.png)
### Hard failure
>[Introduced][ee-3117] in GitLab Enterprise Edition 10.2.
Once a mirror gets retried 14 times in a row, it will get marked as hard failed,
this will become visible in either the project main dashboard or in the
pull mirror settings page.
![Hard failed mirror main notice](repository_mirroring/repository_mirroring_hard_failed_main.png)
![Hard failed mirror settings notice](repository_mirroring/repository_mirroring_hard_failed_settings.png)
When a project is hard failed, it will no longer get picked up for mirroring.
A user can resume the project mirroring again by either [forcing an update](#forcing-an-update)
or by changing the import URL in repository settings.
### SSH authentication
> [Introduced][ee-2551] in GitLab Enterprise Edition Starter 9.5
......@@ -248,6 +264,7 @@ to resolve this issue.
[ee-51]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/51
[ee-2551]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2551
[ee-3117]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3117
[perms]: ../user/permissions.md
[hooks]: https://docs.gitlab.com/ee/administration/custom_hooks.html
[deploy-key]: ../ssh/README.md#deploy-keys
module EE
module ProjectsHelper
def can_force_update_mirror?(project)
return true if project.mirror_hard_failed?
return true unless project.mirror_last_update_at
Time.now - project.mirror_last_update_at >= 5.minutes
end
def can_change_push_rule?(push_rule, rule)
return true if push_rule.global?
......
......@@ -40,7 +40,13 @@ module EE
scope :with_shared_runners_limit_enabled, -> { with_shared_runners.non_public_only }
scope :mirror, -> { where(mirror: true) }
scope :mirrors_to_sync, ->(freeze_at) { mirror.joins(:mirror_data).without_import_status(:scheduled, :started).where("next_execution_timestamp <= ?", freeze_at) }
scope :mirrors_to_sync, ->(freeze_at) do
mirror.joins(:mirror_data).without_import_status(:scheduled, :started)
.where("next_execution_timestamp <= ?", freeze_at)
.where("project_mirror_data.retry_count <= ?", ::Gitlab::Mirror::MAX_RETRY)
end
scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct }
scope :with_wiki_enabled, -> { with_feature_enabled(:wiki) }
......@@ -100,6 +106,7 @@ module EE
def scheduled_mirror?
return false unless mirror_with_content?
return false if mirror_hard_failed?
return true if import_scheduled?
self.mirror_data.next_execution_timestamp <= Time.now
......@@ -119,7 +126,7 @@ module EE
end
end
def mirror_last_update_success?
def mirror_last_update_succeeded?
mirror_last_update_status == :success
end
......@@ -131,6 +138,10 @@ module EE
mirror_updated? && self.mirror_last_successful_update_at
end
def mirror_hard_failed?
self.mirror_data.retry_limit_exceeded?
end
def has_remote_mirror?
feature_available?(:repository_mirrors) &&
remote_mirror_available? &&
......@@ -204,7 +215,13 @@ module EE
end
def force_import_job!
self.mirror_data.set_next_execution_to_now!
mirror_data = self.mirror_data
mirror_data.set_next_execution_to_now
mirror_data.reset_retry_count if mirror_data.retry_limit_exceeded?
mirror_data.save!
UpdateAllMirrorsWorker.perform_async
end
......
......@@ -15,9 +15,8 @@ module EE
before_transition scheduled: :failed do |project, _|
if project.mirror?
timestamp = Time.now
project.mirror_last_update_at = timestamp
project.mirror_data.next_execution_timestamp = timestamp
project.mirror_last_update_at = Time.now
project.mirror_data.set_next_execution_to_now
end
end
......@@ -30,8 +29,8 @@ module EE
project.mirror_last_update_at = Time.now
mirror_data = project.mirror_data
mirror_data.increment_retry_count!
mirror_data.set_next_execution_timestamp!
mirror_data.increment_retry_count
mirror_data.set_next_execution_timestamp
end
end
......@@ -42,8 +41,8 @@ module EE
project.mirror_last_successful_update_at = timestamp
mirror_data = project.mirror_data
mirror_data.reset_retry_count!
mirror_data.set_next_execution_timestamp!
mirror_data.reset_retry_count
mirror_data.set_next_execution_timestamp
end
if ::Gitlab::CurrentSettings.current_application_settings.elasticsearch_indexing?
......
......@@ -6,7 +6,14 @@ module Gitlab
SCHEDULER_CRON = '* * * * *'.freeze
PULL_CAPACITY_KEY = 'MIRROR_PULL_CAPACITY'.freeze
JITTER = 1.minute
# TODO: Make MIN_DELAY configurable and MAX_RETRY adjust according to it
# ISSUE: https://gitlab.com/gitlab-org/gitlab-ee/issues/3885
# MAX RETRY value was calculated based on the triangular number with a 15 minutes factor
# https://en.wikipedia.org/wiki/Triangular_number in order to make sure the mirror
# gets retried for a full day before it becomes hard failed
MIN_DELAY = 15.minutes
MAX_RETRY = 14
class << self
def configure_cron_job!
......
......@@ -148,11 +148,11 @@ describe Projects::MirrorsController do
describe 'forcing an update on a pull mirror' do
it 'forces update' do
expect_any_instance_of(EE::Project).to receive(:force_import_job!)
project = create(:project, :mirror)
sign_in(project.owner)
expect_any_instance_of(EE::Project).to receive(:force_import_job!)
put :update_now, { namespace_id: project.namespace.to_param, project_id: project.to_param }
end
end
......
......@@ -19,6 +19,62 @@ describe Project do
it { is_expected.to have_many(:audit_events) }
end
describe '.mirrors_to_sync' do
let(:timestamp) { Time.now }
context 'when mirror is scheduled' do
it 'returns empty' do
create(:project, :mirror, :import_scheduled)
expect(described_class.mirrors_to_sync(timestamp)).to be_empty
end
end
context 'when mirror is started' do
it 'returns empty' do
create(:project, :mirror, :import_scheduled)
expect(described_class.mirrors_to_sync(timestamp)).to be_empty
end
end
context 'when mirror is finished' do
let!(:project) { create(:project, :mirror, :import_finished) }
it 'returns project if next_execution_timestamp is not in the future' do
expect(described_class.mirrors_to_sync(timestamp)).to match_array(project)
end
it 'returns empty if next_execution_timestamp is in the future' do
project.mirror_data.update_attributes(next_execution_timestamp: timestamp + 2.minutes)
expect(described_class.mirrors_to_sync(timestamp)).to be_empty
end
end
context 'when project is failed' do
let!(:project) { create(:project, :mirror, :import_failed) }
it 'returns project if next_execution_timestamp is not in the future' do
expect(described_class.mirrors_to_sync(timestamp)).to match_array(project)
end
it 'returns empty if next_execution_timestamp is in the future' do
project.mirror_data.update_attributes(next_execution_timestamp: timestamp + 2.minutes)
expect(described_class.mirrors_to_sync(timestamp)).to be_empty
end
context 'with retry limit exceeded' do
let!(:project) { create(:project, :mirror, :import_hard_failed) }
it 'returns empty' do
expect(described_class.mirrors_to_sync(timestamp)).to be_empty
end
end
end
end
describe '#push_rule' do
let(:project) { create(:project, push_rule: create(:push_rule)) }
......@@ -200,6 +256,33 @@ describe Project do
end
end
describe '#force_import_job!' do
it 'sets next execution timestamp to now and schedules UpdateAllMirrorsWorker' do
timestamp = Time.now
project = create(:project, :mirror)
expect(UpdateAllMirrorsWorker).to receive(:perform_async)
Timecop.freeze(timestamp) do
expect { project.force_import_job! }.to change(project.mirror_data, :next_execution_timestamp).to(timestamp)
end
end
context 'when mirror is hard failed' do
it 'resets retry count and schedules a mirroring worker' do
timestamp = Time.now
project = create(:project, :mirror, :import_hard_failed)
expect(UpdateAllMirrorsWorker).to receive(:perform_async)
Timecop.freeze(timestamp) do
expect { project.force_import_job! }.to change(project.mirror_data, :retry_count).to(0)
expect(project.mirror_data.next_execution_timestamp).to eq(timestamp)
end
end
end
end
describe '#fetch_mirror' do
where(:import_url, :auth_method, :expected) do
'http://foo:bar@example.com' | 'password' | 'http://foo:bar@example.com'
......@@ -239,6 +322,95 @@ describe Project do
end
end
describe '#scheduled_mirror?' do
context 'when mirror is expected to run soon' do
it 'returns true' do
timestamp = Time.now
project = create(:project, :mirror, :import_finished, :repository)
project.mirror_last_update_at = timestamp - 3.minutes
project.mirror_data.next_execution_timestamp = timestamp - 2.minutes
expect(project.scheduled_mirror?).to be true
end
end
context 'when mirror was scheduled' do
it 'returns true' do
project = create(:project, :mirror, :import_scheduled, :repository)
expect(project.scheduled_mirror?).to be true
end
end
context 'when mirror is hard_failed' do
it 'returns false' do
project = create(:project, :mirror, :import_hard_failed)
expect(project.scheduled_mirror?).to be false
end
end
end
describe '#updating_mirror?' do
context 'when repository is empty' do
it 'returns false' do
project = create(:project, :mirror, :import_started)
expect(project.updating_mirror?).to be false
end
end
context 'when project is not a mirror' do
it 'returns false' do
project = create(:project, :import_started)
expect(project.updating_mirror?).to be false
end
end
context 'when mirror is in progress' do
it 'returns true' do
project = create(:project, :mirror, :import_started, :repository)
expect(project.updating_mirror?).to be true
end
end
end
describe '#mirror_last_update_status' do
let(:project) { create(:project, :mirror) }
context 'when mirror has not updated' do
it 'returns nil' do
expect(project.mirror_last_update_status).to be_nil
end
end
context 'when mirror has updated' do
let(:timestamp) { Time.now }
before do
project.mirror_last_update_at = timestamp
end
context 'when last update time equals the time of the last successful update' do
it 'returns success' do
project.mirror_last_successful_update_at = timestamp
expect(project.mirror_last_update_status).to eq(:success)
end
end
context 'when last update time does not equal the time of the last successful update' do
it 'returns failed' do
project.mirror_last_successful_update_at = Time.now - 1.minute
expect(project.mirror_last_update_status).to eq(:failed)
end
end
end
end
describe '#has_remote_mirror?' do
let(:project) { create(:project, :remote_mirror, :import_started) }
subject { project.has_remote_mirror? }
......
......@@ -78,11 +78,25 @@ FactoryGirl.define do
end
trait :import_finished do
timestamp = Time.now
import_status :finished
mirror_last_update_at timestamp
mirror_last_successful_update_at timestamp
end
trait :import_failed do
import_status :failed
mirror_last_update_at Time.now
end
trait :import_hard_failed do
import_status :failed
mirror_last_update_at Time.now - 1.minute
after(:create) do |project|
project.mirror_data&.update_attributes(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
end
end
trait :mirror do
......
......@@ -7,10 +7,6 @@ describe ProjectMirrorData, type: :model do
it { is_expected.to belong_to(:project) }
end
describe 'modules' do
it { is_expected.to include_module(Gitlab::CurrentSettings) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:project) }
end
......@@ -27,25 +23,25 @@ describe ProjectMirrorData, type: :model do
end
end
describe '#reset_retry_count!' do
describe '#reset_retry_count' do
let(:mirror_data) { create(:project, :mirror, :import_finished).mirror_data }
it 'resets retry_count to 0' do
mirror_data.retry_count = 3
expect { mirror_data.reset_retry_count! }.to change { mirror_data.retry_count }.from(3).to(0)
expect { mirror_data.reset_retry_count }.to change { mirror_data.retry_count }.from(3).to(0)
end
end
describe '#increment_retry_count!' do
describe '#increment_retry_count' do
let(:mirror_data) { create(:project, :mirror, :import_finished).mirror_data }
it 'increments retry_count' do
expect { mirror_data.increment_retry_count! }.to change { mirror_data.retry_count }.from(0).to(1)
expect { mirror_data.increment_retry_count }.to change { mirror_data.retry_count }.from(0).to(1)
end
end
describe '#set_next_execution_timestamp!' do
describe '#set_next_execution_timestamp' do
let(:mirror_data) { create(:project, :mirror, :import_finished).mirror_data }
let!(:timestamp) { Time.now }
let!(:jitter) { 2.seconds }
......@@ -68,7 +64,7 @@ describe ProjectMirrorData, type: :model do
context 'when incrementing retry count' do
it 'applies transition successfully' do
mirror_data.retry_count = 2
mirror_data.increment_retry_count!
mirror_data.increment_retry_count
expect_next_execution_timestamp(mirror_data, timestamp + 78.minutes)
end
......@@ -96,7 +92,7 @@ describe ProjectMirrorData, type: :model do
context 'when incrementing retry count' do
it 'applies transition successfully' do
mirror_data.retry_count = 3
mirror_data.increment_retry_count!
mirror_data.increment_retry_count
expect_next_execution_timestamp(mirror_data, timestamp + 62.minutes)
end
......@@ -119,7 +115,7 @@ describe ProjectMirrorData, type: :model do
context 'when incrementing retry count' do
it 'applies transition successfully' do
mirror_data.retry_count = 2
mirror_data.increment_retry_count!
mirror_data.increment_retry_count
expect_next_execution_timestamp(mirror_data, max_timestamp + mirror_jitter)
end
......@@ -130,7 +126,7 @@ describe ProjectMirrorData, type: :model do
def expect_next_execution_timestamp(mirror_data, new_timestamp)
Timecop.freeze(timestamp) do
expect do
mirror_data.set_next_execution_timestamp!
mirror_data.set_next_execution_timestamp
end.to change { mirror_data.next_execution_timestamp }.to eq(new_timestamp)
end
end
......
......@@ -1938,68 +1938,6 @@ describe Project do
end
end
describe '#scheduled_mirror?' do
context 'when mirror is expected to run soon' do
it 'returns true' do
timestamp = Time.now
project = create(:project, :mirror, :import_finished, :repository)
project.mirror_last_update_at = timestamp - 3.minutes
project.mirror_data.next_execution_timestamp = timestamp - 2.minutes
expect(project.scheduled_mirror?).to be true
end
end
context 'when mirror was scheduled' do
it 'returns true' do
project = create(:project, :mirror, :import_scheduled, :repository)
expect(project.scheduled_mirror?).to be true
end
end
end
describe '#updating_mirror?' do
context 'when repository is empty' do
it 'returns false' do
project = create(:project, :mirror, :import_started)
expect(project.updating_mirror?).to be false
end
end
context 'when project is not a mirror' do
it 'returns false' do
project = create(:project, :import_started)
expect(project.updating_mirror?).to be false
end
end
context 'when mirror is in progress' do
it 'returns true' do
project = create(:project, :mirror, :import_started, :repository)
expect(project.updating_mirror?).to be true
end
end
end
describe '#force_import_job!' do
it 'sets next execution timestamp to now and schedules UpdateAllMirrorsWorker' do
timestamp = Time.now
project = create(:project, :mirror)
project.mirror_data.update_attributes(next_execution_timestamp: timestamp - 3.minutes)
expect(UpdateAllMirrorsWorker).to receive(:perform_async)
Timecop.freeze(timestamp) do
expect { project.force_import_job! }.to change { project.mirror_data.reload.next_execution_timestamp }.to be_within(1.second).of(timestamp)
end
end
end
describe '#add_import_job' do
let(:import_jid) { '123' }
......
require 'spec_helper'
describe 'shared/_mirror_status.html.haml' do
include ApplicationHelper
context 'when mirror has not updated yet' do
it 'does not render anything' do
@project = create(:project, :mirror)
sign_in(@project.owner)
render 'shared/mirror_status'
expect(rendered).to be_empty
end
end
context 'when mirror successful' do
it 'renders success message' do
@project = create(:project, :mirror, :import_finished)
sign_in(@project.owner)
render 'shared/mirror_status'
expect(rendered).to have_content("Updated")
end
end
context 'when mirror failed' do
before do
@project = create(:project, :mirror, :import_failed)
sign_in(@project.owner)
end
it 'renders failure message' do
render 'shared/mirror_status', raw_message: true
expect(rendered).to have_content("The repository failed to update")
end
context 'with a previous successful update' do
it 'renders failure message' do
@project.mirror_last_successful_update_at = Time.now - 1.minute
render 'shared/mirror_status', raw_message: true
expect(rendered).to have_content("Last successful update")
end
end
context 'with a hard failed mirror' do
it 'renders hard failed message' do
@project.mirror_data.retry_count = Gitlab::Mirror::MAX_RETRY + 1
render 'shared/mirror_status', raw_message: true
expect(rendered).to have_content("Repository mirroring has been paused due to too many failed attempts, and can be resumed by a project admin.")
end
end
end
end
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