Merge branch '1865-add-global-minimum-mirror-sync-time-options' into 'master'

adds more options to global minimum mirror sync time

Closes #1865

See merge request !1400

lib/gitlab/mirror.rb

@@ -4,26 +4,38 @@ module Gitlab
 
     FIFTEEN = 15
     HOURLY  = 60
-    DAILY = 1440
+    THREE   = 180
+    SIX     = 360
+    TWELVE  = 720
+    DAILY   = 1440
 
     INTERVAL_BEFORE_FIFTEEN = 14.minutes
 
     SYNC_TIME_TO_CRON = {
       FIFTEEN => "*/15 * * * *",
       HOURLY  => "0 * * * *",
-      DAILY   => "0 0 * * *"
+      THREE   => "0 */3 * * *",
+      SIX     => "0 */6 * * *",
+      TWELVE  => "0 */12 * * *",
+      DAILY   => "0 0 * * *",
     }.freeze
 
     SYNC_TIME_OPTIONS = {
-      "Update every 15 minutes" => FIFTEEN,
-      "Update hourly" => HOURLY,
-      "Update every day" => DAILY,
+      "Update every 15 minutes"   => FIFTEEN,
+      "Update hourly"             => HOURLY,
+      "Update every three hours"  => THREE,
+      "Update every six hours"    => SIX,
+      "Update every twelve hours" => TWELVE,
+      "Update every day"          => DAILY,
     }.freeze
 
     class << self
       def sync_times
         sync_times = [FIFTEEN]
         sync_times << DAILY  if at_beginning_of_day?
+        sync_times << TWELVE if at_beginning_of_hour?(12)
+        sync_times << SIX    if at_beginning_of_hour?(6)
+        sync_times << THREE  if at_beginning_of_hour?(3)
         sync_times << HOURLY if at_beginning_of_hour?
 
         sync_times
@@ -59,11 +71,14 @@ module Gitlab
         DateTime.now.between?(start_at, end_at)
       end
 
-      def at_beginning_of_hour?
+      def at_beginning_of_hour?(hour_mark = nil)
         start_at = DateTime.now.at_beginning_of_hour
         end_at = start_at + INTERVAL_BEFORE_FIFTEEN
 
-        DateTime.now.between?(start_at, end_at)
+        between_interval = DateTime.now.between?(start_at, end_at)
+        return between_interval unless hour_mark
+
+        between_interval && DateTime.now.hour % hour_mark == 0
       end
     end
   end

spec/features/projects/mirror_spec.rb

@@ -21,39 +21,17 @@ feature 'Project mirror', feature: true do
     end
 
     describe 'synchronization times' do
-      describe 'daily minimum mirror sync_time' do
-        before do
-          stub_application_setting(minimum_mirror_sync_time: Gitlab::Mirror::DAILY)
-          visit namespace_project_mirror_path(project.namespace, project)
-        end
-
-        it 'shows the correct selector options' do
-          expect(page).to have_selector('.project-mirror-sync-time > option', count: 1)
-          expect(page).to have_selector('.remote-mirror-sync-time > option', count: 1)
-        end
-      end
-
-      describe 'hourly minimum mirror sync_time' do
-        before do
-          stub_application_setting(minimum_mirror_sync_time: Gitlab::Mirror::HOURLY)
-          visit namespace_project_mirror_path(project.namespace, project)
-        end
-
-        it 'shows the correct selector options' do
-          expect(page).to have_selector('.project-mirror-sync-time > option', count: 2)
-          expect(page).to have_selector('.remote-mirror-sync-time > option', count: 2)
-        end
-      end
-
-      describe 'fifteen minimum mirror sync_time' do
-        before do
-          stub_application_setting(minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN)
-          visit namespace_project_mirror_path(project.namespace, project)
-        end
-
-        it 'shows the correct selector options' do
-          expect(page).to have_selector('.project-mirror-sync-time > option', count: 3)
-          expect(page).to have_selector('.remote-mirror-sync-time > option', count: 3)
+      Gitlab::Mirror::SYNC_TIME_TO_CRON.keys.reverse.each_with_index do |sync_time, index|
+        describe "#{sync_time} minimum mirror sync time" do
+          before do
+            stub_application_setting(minimum_mirror_sync_time: sync_time)
+            visit namespace_project_mirror_path(project.namespace, project)
+          end
+
+          it 'shows the correct selector options' do
+            expect(page).to have_selector('.project-mirror-sync-time > option', count: index + 1)
+            expect(page).to have_selector('.remote-mirror-sync-time > option', count: index + 1)
+          end
         end
       end
     end

spec/models/application_setting_spec.rb

@@ -82,52 +82,38 @@ describe ApplicationSetting, models: true do
     end
 
     context "update minimum_mirror_sync_time" do
+      sync_times = Gitlab::Mirror::SYNC_TIME_TO_CRON.keys
+
       before do
         Sidekiq::Logging.logger = nil
-        Gitlab::Mirror::SYNC_TIME_TO_CRON.keys.each do |sync_time|
+        sync_times.each do |sync_time|
           create(:project, :mirror, sync_time: sync_time)
           create(:project, :remote_mirror, sync_time: sync_time)
         end
       end
 
-      context 'with daily sync_time' do
-        let(:sync_time) { Gitlab::Mirror::DAILY }
-
-        it 'updates minimum_mirror_sync_time to daily and updates cron jobs' do
-          expect_any_instance_of(ApplicationSetting).to receive(:update_mirror_cron_jobs).and_call_original
-          expect(Gitlab::Mirror).to receive(:configure_cron_jobs!)
-
-          setting.update_attributes(minimum_mirror_sync_time: sync_time)
-        end
+      sync_times.drop(1).each_with_index do |sync_time, index|
+        context "with #{sync_time} sync_time" do
+          subject { setting.update_attributes(minimum_mirror_sync_time: sync_time) }
 
-        it 'updates every mirror to the current minimum_mirror_sync_time' do
-          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { Project.mirror.where('sync_time < ?', sync_time).count }.from(2).to(0)
-        end
-
-        it 'updates every remote mirror to the current minimum_mirror_sync_time' do
-          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { RemoteMirror.where('sync_time < ?', sync_time).count }.from(2).to(0)
-        end
-      end
+          it "updates minimum mirror sync time to #{sync_time}" do
+            expect_any_instance_of(ApplicationSetting).to receive(:update_mirror_cron_jobs).and_call_original
+            expect(Gitlab::Mirror).to receive(:configure_cron_jobs!)
 
-      context 'with hourly sync time' do
-        let(:sync_time) { Gitlab::Mirror::HOURLY }
-
-        it 'updates minimum_mirror_sync_time to daily and updates cron jobs' do
-          expect_any_instance_of(ApplicationSetting).to receive(:update_mirror_cron_jobs).and_call_original
-          expect(Gitlab::Mirror).to receive(:configure_cron_jobs!)
-
-          setting.update_attributes(minimum_mirror_sync_time: sync_time)
-        end
+            subject
+          end
 
-        it 'updates every mirror to the current minimum_mirror_sync_time' do
-          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { Project.mirror.where('sync_time < ?', sync_time).count }.from(1).to(0)
-        end
+          it 'updates every mirror to the current minimum_mirror_sync_time' do
+            expect { subject }.to change { Project.mirror.where('sync_time < ?', sync_time).count }.from(index + 1).to(0)
+          end
 
-        it 'updates every remote mirror to the current minimum_mirror_sync_time' do
-          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { RemoteMirror.where('sync_time < ?', sync_time).count }.from(1).to(0)
+          it 'updates every remote mirror to the current minimum_mirror_sync_time' do
+            expect { subject }.to change { RemoteMirror.where('sync_time < ?', sync_time).count }.from(index + 1).to(0)
+          end
         end
       end
 
+      # fifteen is a special case so we isolate it
       context 'with default fifteen sync time' do
         let(:sync_time) { Gitlab::Mirror::FIFTEEN }
 

spec/workers/update_all_mirrors_worker_spec.rb

 require 'rails_helper'
 
 describe UpdateAllMirrorsWorker do
+  subject(:worker) { described_class.new }
+
   before { allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true) }
 
   describe '#perform' do
-    let(:worker) { described_class.new }
-    let!(:mirror1) { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::FIFTEEN) }
-    let!(:mirror2) { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::HOURLY) }
-    let!(:mirror3) { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::DAILY) }
-    let!(:mirror4) { create(:empty_project, :mirror) }
+    let!(:fifteen_mirror)  { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::FIFTEEN) }
+    let!(:hourly_mirror)   { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::HOURLY) }
+    let!(:three_mirror)    { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::THREE) }
+    let!(:six_mirror)      { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::SIX) }
+    let!(:twelve_mirror)   { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::TWELVE) }
+    let!(:daily_mirror)    { create(:empty_project, :mirror, sync_time: Gitlab::Mirror::DAILY) }
+    let!(:outdated_mirror) { create(:empty_project, :mirror) }
 
     it 'fails stuck mirrors' do
       expect(worker).to receive(:fail_stuck_mirrors!)
@@ -16,76 +20,88 @@ describe UpdateAllMirrorsWorker do
       worker.perform
     end
 
+    it 'does not execute if cannot get the lease' do
+      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false)
+
+      create(:empty_project, :mirror)
+
+      expect(worker).not_to receive(:fail_stuck_mirrors!)
+
+      worker.perform
+    end
+
     describe 'sync_time' do
-      def expect_worker_to_update_mirrors(mirrors)
+      def expect_worker_to_enqueue_mirrors(mirrors)
         mirrors.each do |mirror|
           expect(worker).to receive(:rand).with((mirror.sync_time / 2).minutes).and_return(mirror.sync_time / 2)
           expect(RepositoryUpdateMirrorDispatchWorker).to receive(:perform_in).with(mirror.sync_time / 2, mirror.id)
         end
+
+        worker.perform
       end
 
-      def setup(time)
+      before do
+        time = DateTime.now.change(time_params)
+
         Timecop.freeze(time)
-        mirror4.update_attributes(mirror_last_successful_update_at: time - (Gitlab::Mirror::DAILY + 5).minutes)
+        outdated_mirror.update_attributes(mirror_last_successful_update_at: time - (Gitlab::Mirror::DAILY + 5).minutes)
       end
 
       describe 'fifteen' do
-        let(:mirrors) { [mirror1, mirror4] }
-
-        before { setup(DateTime.now.beginning_of_hour + 15.minutes) }
+        let!(:time_params) { { hour: 1, min: 15 } }
+        let(:mirrors) { [fifteen_mirror, outdated_mirror] }
 
-        it 'enqueues a job on mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
-
-          worker.perform
-        end
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
       end
 
       describe 'hourly' do
-        let(:mirrors) { [mirror1, mirror2, mirror4] }
-
-        before { setup(DateTime.now.beginning_of_hour) }
-
-        it 'enqueues a job on mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
+        let!(:time_params) { { hour: 1 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, outdated_mirror] }
 
-          worker.perform
-        end
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
       end
 
-      describe 'daily' do
-        let(:mirrors) { [mirror1, mirror2, mirror3, mirror4] }
+      describe 'three' do
+        let!(:time_params) { { hour: 3 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, outdated_mirror] }
 
-        before { setup(DateTime.now.beginning_of_day) }
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
 
-        it 'enqueues a job on mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
+      describe 'six' do
+        let!(:time_params) { { hour: 6 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, outdated_mirror] }
 
-          worker.perform
-        end
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
       end
 
-      after { Timecop.return }
-    end
+      describe 'twelve' do
+        let!(:time_params) { { hour: 12 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, twelve_mirror, outdated_mirror] }
 
-    it 'does not execute if cannot get the lease' do
-      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false)
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
 
-      create(:empty_project, :mirror)
+      describe 'daily' do
+        let!(:time_params) { { hour: 0 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, twelve_mirror, daily_mirror, outdated_mirror] }
 
-      expect(worker).not_to receive(:fail_stuck_mirrors!)
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
 
-      worker.perform
+      after { Timecop.return }
     end
   end
 
   describe '#fail_stuck_mirrors!' do
+    delegate :fail_stuck_mirrors!, to: :worker
+
     it 'ignores records that are not mirrors' do
       create(:empty_project, :import_started, mirror_last_update_at: 12.hours.ago)
 
       expect_any_instance_of(Project).not_to receive(:import_fail)
 
-      perform
+      fail_stuck_mirrors!
     end
 
     it 'ignores records without in-progress import' do
@@ -93,7 +109,7 @@ describe UpdateAllMirrorsWorker do
 
       expect_any_instance_of(Project).not_to receive(:import_fail)
 
-      perform
+      fail_stuck_mirrors!
     end
 
     it 'ignores records with recently updated mirrors' do
@@ -101,13 +117,13 @@ describe UpdateAllMirrorsWorker do
 
       expect_any_instance_of(Project).not_to receive(:import_fail)
 
-      perform
+      fail_stuck_mirrors!
     end
 
     it 'transitions stuck mirrors to a failed state' do
       project = create(:empty_project, :mirror, mirror_last_update_at: 12.hours.ago)
 
-      perform
+      fail_stuck_mirrors!
       project.reload
 
       expect(project).to be_import_failed
@@ -116,14 +132,10 @@ describe UpdateAllMirrorsWorker do
     it 'updates the import_error message' do
       project = create(:empty_project, :mirror, mirror_last_update_at: 12.hours.ago)
 
-      perform
+      fail_stuck_mirrors!
       project.reload
 
       expect(project.import_error).to eq 'The mirror update took too long to complete.'
     end
-
-    def perform
-      described_class.new.fail_stuck_mirrors!
-    end
   end
 end

spec/workers/update_all_remote_mirrors_worker.rb

-require 'rails_helper'
-
-describe UpdateAllRemoteMirrorsWorker do
-  let(:worker) { described_class.new }
-
-  describe "#perform" do
-    let!(:mirror1) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::FIFTEEN) }
-    let!(:mirror2) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::HOURLY) }
-    let!(:mirror3) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::DAILY) }
-    let!(:mirror4) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::DAILY) }
-
-    it 'fails stuck mirrors' do
-      expect(worker).to receive(:fail_stuck_mirrors!)
-
-      worker.perform
-    end
-
-    describe 'sync time' do
-      def expect_worker_to_update_mirrors(mirrors)
-        mirrors.each do |mirror|
-          expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(mirror.id)
-        end
-      end
-
-      def setup(time)
-        Timecop.freeze(time)
-        mirror4.remote_mirrors.first.update_attributes(last_successful_update_at: time - (Gitlab::Mirror::DAILY + 5).minutes)
-      end
-
-      describe 'fifteen' do
-        let(:mirrors) { [mirror1, mirror4] }
-
-        before { setup(DateTime.now.beginning_of_hour + 15.minutes) }
-
-        it 'enqueues a job on remote mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
-
-          worker.perform
-        end
-      end
-
-      describe "hourly" do
-        let(:mirrors) { [mirror1, mirror2, mirror4] }
-
-        before { setup(DateTime.now.beginning_of_hour) }
-
-        it 'enqueues a job on remote mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
-
-          worker.perform
-        end
-      end
-
-      describe "daily" do
-        let(:mirrors) { [mirror1, mirror2, mirror3, mirror4] }
-
-        before { setup(DateTime.now.beginning_of_day) }
-
-        it 'enqueues a job on remote mirrored projects' do
-          expect_worker_to_update_mirrors(mirrors)
-
-          worker.perform
-        end
-      end
-
-      after { Timecop.return }
-    end
-  end
-end

spec/workers/update_all_remote_mirrors_worker_spec.rb

+require 'rails_helper'
+
+describe UpdateAllRemoteMirrorsWorker do
+  subject(:worker) { described_class.new }
+
+  describe "#perform" do
+    let!(:fifteen_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::FIFTEEN) }
+    let!(:hourly_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::HOURLY) }
+    let!(:three_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::THREE) }
+    let!(:six_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::SIX) }
+    let!(:twelve_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::TWELVE) }
+    let!(:daily_mirror) { create(:project, :remote_mirror, sync_time: Gitlab::Mirror::DAILY) }
+    let!(:outdated_mirror) { create(:project, :remote_mirror) }
+
+    it 'fails stuck mirrors' do
+      expect(worker).to receive(:fail_stuck_mirrors!)
+
+      worker.perform
+    end
+
+    describe 'sync time' do
+      def expect_worker_to_enqueue_mirrors(mirrors)
+        mirrors.each do |mirror|
+          expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(mirror.id)
+        end
+
+        worker.perform
+      end
+
+      before do
+        time = DateTime.now.change(time_params)
+
+        Timecop.freeze(time)
+        outdated_mirror.remote_mirrors.first.update_attributes(last_successful_update_at: time - (Gitlab::Mirror::DAILY + 5).minutes)
+      end
+
+      describe 'fifteen' do
+        let!(:time_params) { { hour: 1, min: 15 } }
+        let(:mirrors) { [fifteen_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      describe "hourly" do
+        let!(:time_params) { { hour: 1 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      describe "three" do
+        let!(:time_params) { { hour: 3 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      describe "six" do
+        let!(:time_params) { { hour: 6 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      describe "twelve" do
+        let!(:time_params) { { hour: 12 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, twelve_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      describe "daily" do
+        let!(:time_params) { { hour: 0 } }
+        let(:mirrors) { [fifteen_mirror, hourly_mirror, three_mirror, six_mirror, twelve_mirror, daily_mirror, outdated_mirror] }
+
+        it { expect_worker_to_enqueue_mirrors(mirrors) }
+      end
+
+      after { Timecop.return }
+    end
+  end
+end