Debounce pull mirror invocation

For the system to handle the load of pull mirrors further, we need
to limit the number of pull mirror invocations per project.

It's documented that within the past 5 minutes, user can only
manually trigger a pull mirror update once.

Before this change, we are letting the user update a pull mirror
immediately. Now, we are checking the `last_update_at` of the
`ImportState` and see if the mirror was updated in past 5 minutes.
If so, schedule the next execution 5 minutes since last update.

ee/app/controllers/ee/projects/mirrors_controller.rb

@@ -41,7 +41,7 @@ module EE
           project.update_remote_mirrors
           flash[:notice] = _('The remote repository is being updated...')
         else
-          project.import_state.force_import_job!
+          StartPullMirroringService.new(project, current_user, pause_on_hard_failure: false).execute
           flash[:notice] = _('The repository is being updated...')
         end
 

ee/app/services/start_pull_mirroring_service.rb

 # frozen_string_literal: true
 
 class StartPullMirroringService < BaseService
+  INTERVAL = 5.minutes
+
   def execute
-    return error('Mirroring for the project is on pause', 403) if project.import_state.hard_failed?
+    import_state = project.import_state
+
+    if import_state.hard_failed?
+      return error('Mirroring for the project is on pause', 403) if params[:pause_on_hard_failure]
+
+      import_state.reset_retry_count
+    end
+
+    if update_now?(import_state)
+      import_state.force_import_job!
+    else
+      import_state.update(next_execution_timestamp: INTERVAL.since(import_state.last_update_at))
+    end
 
-    project.import_state.force_import_job!
     success
   end
+
+  private
+
+  def update_now?(import_state)
+    import_state.last_update_at.nil? || import_state.last_update_at < INTERVAL.ago
+  end
 end

ee/changelogs/unreleased/118753-debounce-pull-mirror.yml

+---
+title: Debounce pull mirror invocation
+merge_request: 30157
+author:
+type: performance

ee/lib/api/project_mirror.rb

@@ -59,7 +59,7 @@ module API
       end
 
       def start_pull_mirroring
-        result = StartPullMirroringService.new(project, mirror_user).execute
+        result = StartPullMirroringService.new(project, mirror_user, pause_on_hard_failure: true).execute
 
         render_api_error!(result[:message], result[:http_status]) if result[:status] == :error
       end

ee/spec/controllers/projects/mirrors_controller_spec.rb

@@ -122,9 +122,11 @@ describe Projects::MirrorsController do
       project = create(:project, :mirror)
       sign_in(project.owner)
 
-      expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!)
-
-      put :update_now, params: { namespace_id: project.namespace.to_param, project_id: project.to_param }
+      Sidekiq::Testing.fake! do
+        expect { put :update_now, params: { namespace_id: project.namespace.to_param, project_id: project.to_param } }
+          .to change { UpdateAllMirrorsWorker.jobs.size }
+          .by(1)
+      end
     end
   end
 

ee/spec/features/projects/mirror_spec.rb

@@ -19,18 +19,20 @@ describe 'Project mirror', :js do
       let(:timestamp) { Time.now }
 
       before do
-        import_state.update(next_execution_timestamp: timestamp + 10.minutes)
+        import_state.update(last_update_at: 6.minutes.ago, next_execution_timestamp: timestamp + 10.minutes)
       end
 
       context 'when able to force update' do
         it 'forces import' do
-          expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!)
-
           Timecop.freeze(timestamp) do
             visit project_mirror_path(project)
           end
 
-          Sidekiq::Testing.fake! { find('.js-force-update-mirror').click }
+          Sidekiq::Testing.fake! do
+            expect { find('.js-force-update-mirror').click }
+              .to change { UpdateAllMirrorsWorker.jobs.size }
+              .by(1)
+          end
         end
       end
 

ee/spec/requests/api/project_mirror_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe API::ProjectMirror do
   describe 'POST /projects/:id/mirror/pull' do
     let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
-    let(:project_mirrored) { create(:project, :repository, :mirror, :import_finished, visibility: visibility) }
+    let(:project_mirrored) { create(:project, :repository, :mirror, visibility: visibility) }
 
     def do_post(user: nil, params: {}, headers: { 'X-Hub-Signature' => 'signature' })
       api_path = api("/projects/#{project_mirrored.id}/mirror/pull", user)
@@ -267,30 +267,28 @@ describe API::ProjectMirror do
               it 'triggers the pull mirroring operation' do
                 project_member(:maintainer, user)
 
-                expect(StartPullMirroringService)
-                  .to receive(:new)
-                  .with(project_mirrored, user)
-                  .and_call_original
-
-                do_post(user: user, headers: {})
+                Sidekiq::Testing.fake! do
+                  expect { do_post(user: user, headers: {}) }
+                    .to change { UpdateAllMirrorsWorker.jobs.size }
+                    .by(1)
 
                   expect(response).to have_gitlab_http_status(:ok)
                 end
               end
+            end
 
             context 'is authenticated as owner' do
               it 'triggers the pull mirroring operation' do
-                expect(StartPullMirroringService)
-                  .to receive(:new)
-                  .with(project_mirrored, project_mirrored.creator)
-                  .and_call_original
-
-                do_post(user: project_mirrored.creator, headers: {})
+                Sidekiq::Testing.fake! do
+                  expect { do_post(user: project_mirrored.creator, headers: {}) }
+                    .to change { UpdateAllMirrorsWorker.jobs.size }
+                    .by(1)
 
                   expect(response).to have_gitlab_http_status(:ok)
                 end
               end
             end
+          end
 
           context 'when repository_mirrors feature is not available' do
             before do

ee/spec/services/start_pull_mirroring_service_spec.rb

@@ -7,16 +7,88 @@ describe StartPullMirroringService do
   let(:import_state) { create(:import_state, project: project) }
   let(:user) { create(:user) }
 
-  subject { described_class.new(project, user) }
+  subject { described_class.new(project, user, pause_on_hard_failure: pause_on_hard_failure) }
+
+  shared_examples_for 'retry count did not reset' do
+    it 'does not reset import state retry_count' do
+      expect { execute }.not_to change { import_state.retry_count }
+    end
+  end
+
+  shared_examples_for 'pull mirroring has started' do
+    shared_examples_for 'force mirror update' do
+      it 'enqueues UpdateAllMirrorsWorker' do
+        Sidekiq::Testing.fake! do
+          expect { execute }
+            .to change { UpdateAllMirrorsWorker.jobs.size }
+            .by(1)
+          expect(execute[:status]).to eq(:success)
+        end
+      end
+    end
+
+    it_behaves_like 'force mirror update'
+
+    context 'when project mirror has been updated in the last 5 minutes' do
+      it 'schedules next execution' do
+        Timecop.freeze(Time.current) do
+          import_state.update(last_update_at: 3.minutes.ago)
+
+          expect { execute }
+            .to change { import_state.next_execution_timestamp }
+            .to(2.minutes.from_now)
+            .and not_change { UpdateAllMirrorsWorker.jobs.size }
+        end
+      end
+    end
+
+    context 'when project mirror has been updated more than 5 minutes ago' do
+      before do
+        import_state.update(last_update_at: 6.minutes.ago)
+      end
+
+      it_behaves_like 'force mirror update'
+    end
+  end
+
+  context 'when pause_on_hard_failure is false' do
+    let(:pause_on_hard_failure) { false }
 
     context "when retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
       before do
         import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
       end
 
+      it_behaves_like 'pull mirroring has started'
+
+      it 'resets the import state retry_count' do
+        expect { execute }.to change { import_state.retry_count }.to(0)
+      end
+    end
+
+    context 'when does not reach the max retry limit yet' do
+      before do
+        import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY - 1)
+      end
+
+      it_behaves_like 'pull mirroring has started'
+      it_behaves_like 'retry count did not reset'
+    end
+  end
+
+  context 'when pause_on_hard_failure is true' do
+    let(:pause_on_hard_failure) { true }
+
+    context "when retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
+      before do
+        import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
+      end
+
+      it_behaves_like 'retry count did not reset'
+
       it 'does not start pull mirroring' do
-      expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
-      expect(subject.execute[:status]).to eq(:error)
+        expect { execute }.to not_change { UpdateAllMirrorsWorker.jobs.size }
+        expect(execute[:status]).to eq(:error)
       end
     end
 
@@ -25,10 +97,12 @@ describe StartPullMirroringService do
         import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY - 1)
       end
 
-    it 'starts pull mirroring' do
-      expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
-      expect(import_state.reload.retry_count).not_to eq(0)
-      expect(subject.execute[:status]).to eq(:success)
+      it_behaves_like 'pull mirroring has started'
+      it_behaves_like 'retry count did not reset'
     end
   end
+
+  def execute
+    subject.execute
+  end
 end