From 37c78fc7b147988ed6dab12f013167f38d964624 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 23 Mar 2016 20:07:37 -0400 Subject: [PATCH 1/4] Add mirror_update_frequency field to Project Defaults to one day, in seconds. --- app/models/project.rb | 3 +++ .../20160323203243_add_mirror_update_frequency_to_project.rb | 5 +++++ db/schema.rb | 3 ++- spec/models/project_spec.rb | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20160323203243_add_mirror_update_frequency_to_project.rb diff --git a/app/models/project.rb b/app/models/project.rb index ed5fc0656e9b4e..6a1fb661ad9831 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -210,6 +210,9 @@ def update_forks_visibility_level if: ->(project) { project.avatar.present? && project.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :approvals_before_merge, numericality: true, allow_blank: true + validates :mirror_update_frequency, + numericality: true, + inclusion: [15.minutes, 1.hour, 1.day] add_authentication_token_field :runners_token before_save :ensure_runners_token diff --git a/db/migrate/20160323203243_add_mirror_update_frequency_to_project.rb b/db/migrate/20160323203243_add_mirror_update_frequency_to_project.rb new file mode 100644 index 00000000000000..f37fe0cf1b1361 --- /dev/null +++ b/db/migrate/20160323203243_add_mirror_update_frequency_to_project.rb @@ -0,0 +1,5 @@ +class AddMirrorUpdateFrequencyToProject < ActiveRecord::Migration + def change + add_column :projects, :mirror_update_frequency, :integer, default: 1.day + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c7ab0b84b0ff4..36102f50a57654 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160317191509) do +ActiveRecord::Schema.define(version: 20160323203243) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -835,6 +835,7 @@ t.boolean "public_builds", default: true, null: false t.string "main_language" t.integer "pushes_since_gc", default: 0 + t.integer "mirror_update_frequency", default: 86400 end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4b81f3a7116249..1b4702e5a6b604 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -96,6 +96,8 @@ it { is_expected.to validate_presence_of(:creator) } it { is_expected.to validate_length_of(:issues_tracker_id).is_within(0..255) } it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_numericality_of(:mirror_update_frequency) } + it { is_expected.to validate_inclusion_of(:mirror_update_frequency).in_array([15.minutes, 1.hour, 1.day]) } it 'should not allow new projects beyond user limits' do project2 = build(:project) -- GitLab From e6e493fecf4575ef612df956841ce3149629356b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 23 Mar 2016 20:13:51 -0400 Subject: [PATCH 2/4] Add `Project#mirror_recently_updated?` method --- app/models/project.rb | 6 ++++++ spec/models/project_spec.rb | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 6a1fb661ad9831..4d68989bb9717b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -495,6 +495,12 @@ def mirror_updated? mirror? && self.mirror_last_update_at end + def mirror_recently_updated? + return false unless mirror_updated? + + mirror_last_update_at >= mirror_update_frequency.seconds.ago + end + def updating_mirror? mirror? && import_in_progress? && !empty_repo? end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1b4702e5a6b604..9d5f66f519f733 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -189,6 +189,41 @@ end end + describe '#mirror_recently_updated?' do + it 'returns false when mirror has never been updated' do + project = build(:empty_project) + + expect(project).not_to be_mirror_recently_updated + end + + it 'handles mirrors updated every 15 minutes' do + project = build(:empty_project, :mirror, mirror_update_frequency: 15.minutes) + + project.mirror_last_update_at = 30.minutes.ago + + expect { project.mirror_last_update_at = 10.minutes.ago }. + to change { project.mirror_recently_updated? }.from(false).to(true) + end + + it 'handles mirrors updated every hour' do + project = build(:empty_project, :mirror, mirror_update_frequency: 1.hour) + + project.mirror_last_update_at = 3.hours.ago + + expect { project.mirror_last_update_at = 45.minutes.ago }. + to change { project.mirror_recently_updated? }.from(false).to(true) + end + + it 'handles mirrors updated every day' do + project = build(:empty_project, :mirror, mirror_update_frequency: 1.day) + + project.mirror_last_update_at = 3.days.ago + + expect { project.mirror_last_update_at = 3.hours.ago }. + to change { project.mirror_recently_updated? }.from(false).to(true) + end + end + describe '#get_issue' do let(:project) { create(:empty_project) } let!(:issue) { create(:issue, project: project) } -- GitLab From 86b4192934feddbe989c020c227a26e9a1c1d4de Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 23 Mar 2016 20:20:34 -0400 Subject: [PATCH 3/4] Update `Project#update_mirror` to respect update frequencies --- app/models/project.rb | 4 ++-- spec/factories/projects.rb | 5 +++++ spec/models/project_spec.rb | 43 +++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 4d68989bb9717b..e07efdddf787e6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -527,9 +527,9 @@ def mirror_ever_updated_successfully? mirror_updated? && self.mirror_last_successful_update_at end - def update_mirror + def update_mirror(force: true) return unless mirror? - + return if mirror_recently_updated? && !force return if import_in_progress? if import_failed? diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 66b38d85652d9a..d9155638c13c6f 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -67,6 +67,11 @@ import_status :started end + trait :import_failed do + import_started + import_status :failed + end + trait :import_finished do import_started import_status :finished diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9d5f66f519f733..5b498281952d68 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -224,6 +224,49 @@ end end + describe '#update_mirror' do + it 'returns early for non-mirrors' do + project = build(:empty_project) + + expect(project).not_to receive(:import_in_progress?) + + expect(project.update_mirror).to eq nil + end + + it 'returns early for recently-updated mirrors not forcing an update' do + project = build(:empty_project, :mirror, :import_started) + + expect(project).to receive(:mirror_recently_updated?).and_return(true) + expect(project).not_to receive(:import_in_progress?) + + expect(project.update_mirror(force: false)).to eq nil + end + + it 'returns early for imports in progress' do + project = build(:empty_project, :mirror, :import_started) + + expect(project).not_to receive(:import_failed?) + + expect(project.update_mirror).to eq nil + end + + it 'retries failed imports' do + project = build(:empty_project, :mirror, :import_failed) + + expect(project).to receive(:import_retry) + + project.update_mirror + end + + it 'starts fresh imports' do + project = build(:empty_project, :mirror, :import_finished) + + expect(project).to receive(:import_start) + + project.update_mirror + end + end + describe '#get_issue' do let(:project) { create(:empty_project) } let!(:issue) { create(:issue, project: project) } -- GitLab From ca02b79620ec25b05b747fb26bee5b2ce9fe8c6b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 23 Mar 2016 20:21:11 -0400 Subject: [PATCH 4/4] Update UpdateAllMirrorsWorker to respect update frequencies It now runs every 15 minutes by default. --- app/workers/update_all_mirrors_worker.rb | 2 +- config/gitlab.yml.example | 5 +++-- spec/workers/update_all_mirrors_worker_spec.rb | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/workers/update_all_mirrors_worker.rb b/app/workers/update_all_mirrors_worker.rb index e71163eee3e48e..487c6845c260a1 100644 --- a/app/workers/update_all_mirrors_worker.rb +++ b/app/workers/update_all_mirrors_worker.rb @@ -4,7 +4,7 @@ class UpdateAllMirrorsWorker def perform fail_stuck_mirrors! - Project.mirror.each(&:update_mirror) + Project.mirror.each { |project| project.update_mirror(force: false) } end def fail_stuck_mirrors! diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index d8e66ce9b8b669..59bf5683a5bcb7 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -186,9 +186,10 @@ production: &base historical_data_worker: cron: "0 12 * * *" - # Update mirrored repositories + # Update repository mirrors, if necessary, every 15 minutes. Mirrors with an + # update frequency higher than 15 minutes will only be updated as-needed. update_all_mirrors_worker: - cron: "0 * * * *" + cron: "*/15 * * * *" # In addition to refreshing users when they log in, # periodically refresh LDAP users membership. diff --git a/spec/workers/update_all_mirrors_worker_spec.rb b/spec/workers/update_all_mirrors_worker_spec.rb index 88c11fe8062933..92e1ec1a03bd31 100644 --- a/spec/workers/update_all_mirrors_worker_spec.rb +++ b/spec/workers/update_all_mirrors_worker_spec.rb @@ -14,7 +14,8 @@ create(:empty_project, :mirror) create(:empty_project) - expect_any_instance_of(Project).to receive(:update_mirror).once + expect_any_instance_of(Project). + to receive(:update_mirror).with(force: false).once described_class.new.perform end -- GitLab