Merge branch '296892-fj-move-housekeeping-service-namespace' into 'master'

Move Housekeeping service to a different namespace

See merge request gitlab-org/gitlab!51572

app/controllers/projects_controller.rb

@@ -197,13 +197,13 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def housekeeping
-    ::Projects::HousekeepingService.new(@project, :gc).execute
+    ::Repositories::HousekeepingService.new(@project, :gc).execute
 
     redirect_to(
       project_path(@project),
       notice: _("Housekeeping successfully started")
     )
-  rescue ::Projects::HousekeepingService::LeaseTaken => ex
+  rescue ::Repositories::HousekeepingService::LeaseTaken => ex
     redirect_to(
       edit_project_path(@project, anchor: 'js-project-advanced-settings'),
       alert: ex.to_s

app/models/concerns/can_housekeep_repository.rb

-# frozen_string_literal: true
-
-module CanHousekeepRepository
-  extend ActiveSupport::Concern
-
-  def pushes_since_gc
-    Gitlab::Redis::SharedState.with { |redis| redis.get(pushes_since_gc_redis_shared_state_key).to_i }
-  end
-
-  def increment_pushes_since_gc
-    Gitlab::Redis::SharedState.with { |redis| redis.incr(pushes_since_gc_redis_shared_state_key) }
-  end
-
-  def reset_pushes_since_gc
-    Gitlab::Redis::SharedState.with { |redis| redis.del(pushes_since_gc_redis_shared_state_key) }
-  end
-
-  private
-
-  def pushes_since_gc_redis_shared_state_key
-    "#{self.class.name.underscore.pluralize}/#{id}/pushes_since_gc"
-  end
-end

app/models/concerns/repositories/can_housekeep_repository.rb

+# frozen_string_literal: true
+
+module Repositories
+  module CanHousekeepRepository
+    extend ActiveSupport::Concern
+
+    def pushes_since_gc
+      Gitlab::Redis::SharedState.with { |redis| redis.get(pushes_since_gc_redis_shared_state_key).to_i }
+    end
+
+    def increment_pushes_since_gc
+      Gitlab::Redis::SharedState.with { |redis| redis.incr(pushes_since_gc_redis_shared_state_key) }
+    end
+
+    def reset_pushes_since_gc
+      Gitlab::Redis::SharedState.with { |redis| redis.del(pushes_since_gc_redis_shared_state_key) }
+    end
+
+    private
+
+    def pushes_since_gc_redis_shared_state_key
+      "#{self.class.name.underscore.pluralize}/#{id}/pushes_since_gc"
+    end
+  end
+end

app/models/project.rb

@@ -34,7 +34,7 @@ class Project < ApplicationRecord
   include FromUnion
   include IgnorableColumns
   include Integration
-  include CanHousekeepRepository
+  include Repositories::CanHousekeepRepository
   include EachBatch
   extend Gitlab::Cache::RequestCache
   extend Gitlab::Utils::Override

app/models/wiki.rb

@@ -3,7 +3,7 @@
 class Wiki
   extend ::Gitlab::Utils::Override
   include HasRepository
-  include CanHousekeepRepository
+  include Repositories::CanHousekeepRepository
   include Gitlab::Utils::StrongMemoize
   include GlobalID::Identification
 

app/services/git/branch_push_service.rb

@@ -72,10 +72,10 @@ module Git
     end
 
     def perform_housekeeping
-      housekeeping = Projects::HousekeepingService.new(project)
+      housekeeping = Repositories::HousekeepingService.new(project)
       housekeeping.increment!
       housekeeping.execute if housekeeping.needed?
-    rescue Projects::HousekeepingService::LeaseTaken
+    rescue Repositories::HousekeepingService::LeaseTaken
     end
 
     def removing_branch?

app/services/projects/after_import_service.rb

@@ -9,7 +9,7 @@ module Projects
     end
 
     def execute
-      service = Projects::HousekeepingService.new(@project)
+      service = Repositories::HousekeepingService.new(@project)
 
       service.execute do
         import_failure_service.with_retry(action: 'delete_all_refs') do
@@ -21,7 +21,7 @@ module Projects
       # import actually changed, so we increment the counter to avoid
       # causing GC to run every time.
       service.increment!
-    rescue Projects::HousekeepingService::LeaseTaken => e
+    rescue Repositories::HousekeepingService::LeaseTaken => e
       Gitlab::Import::Logger.info(
         message: 'Project housekeeping failed',
         project_full_path: @project.full_path,

app/services/projects/housekeeping_service.rb

 # frozen_string_literal: true
 
-# Projects::HousekeepingService class
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
 #
-# Used for git housekeeping
+# We're deploying the rename of this class in 13.9. Nevertheless,
+# we cannot remove this class entirely because there can be jobs
+# referencing it.
 #
-# Ex.
-#   Projects::HousekeepingService.new(project).execute
+# We can get rid of this class in 13.10
+# https://gitlab.com/gitlab-org/gitlab/-/issues/297580
 #
 module Projects
-  class HousekeepingService < BaseService
-    # Timeout set to 24h
-    LEASE_TIMEOUT = 86400
-    PACK_REFS_PERIOD = 6
-
-    class LeaseTaken < StandardError
-      def to_s
-        "Somebody already triggered housekeeping for this project in the past #{LEASE_TIMEOUT / 60} minutes"
-      end
-    end
-
-    def initialize(project, task = nil)
-      @project = project
-      @task = task
-    end
-
-    def execute
-      lease_uuid = try_obtain_lease
-      raise LeaseTaken unless lease_uuid.present?
-
-      yield if block_given?
-
-      execute_gitlab_shell_gc(lease_uuid)
-    end
-
-    def needed?
-      pushes_since_gc > 0 && period_match? && housekeeping_enabled?
-    end
-
-    def increment!
-      Gitlab::Metrics.measure(:increment_pushes_since_gc) do
-        @project.increment_pushes_since_gc
-      end
-    end
-
-    private
-
-    def execute_gitlab_shell_gc(lease_uuid)
-      GitGarbageCollectWorker.perform_async(@project.id, task, lease_key, lease_uuid)
-    ensure
-      if pushes_since_gc >= gc_period
-        Gitlab::Metrics.measure(:reset_pushes_since_gc) do
-          @project.reset_pushes_since_gc
-        end
-      end
-    end
-
-    def try_obtain_lease
-      Gitlab::Metrics.measure(:obtain_housekeeping_lease) do
-        lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT)
-        lease.try_obtain
-      end
-    end
-
-    def lease_key
-      "project_housekeeping:#{@project.id}"
-    end
-
-    def pushes_since_gc
-      @project.pushes_since_gc
-    end
-
-    def task
-      return @task if @task
-
-      if pushes_since_gc % gc_period == 0
-        :gc
-      elsif pushes_since_gc % full_repack_period == 0
-        :full_repack
-      elsif pushes_since_gc % repack_period == 0
-        :incremental_repack
-      else
-        :pack_refs
-      end
-    end
-
-    def period_match?
-      [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 }
-    end
-
-    def housekeeping_enabled?
-      Gitlab::CurrentSettings.housekeeping_enabled
-    end
-
-    def gc_period
-      Gitlab::CurrentSettings.housekeeping_gc_period
-    end
-
-    def full_repack_period
-      Gitlab::CurrentSettings.housekeeping_full_repack_period
-    end
-
-    def repack_period
-      Gitlab::CurrentSettings.housekeeping_incremental_repack_period
-    end
+  class HousekeepingService < ::Repositories::HousekeepingService
   end
 end

app/services/projects/update_repository_storage_service.rb

@@ -33,8 +33,8 @@ module Projects
       return unless Gitlab::CurrentSettings.housekeeping_enabled?
       return unless Feature.enabled?(:repack_after_shard_migration, project)
 
-      Projects::HousekeepingService.new(project, :gc).execute
-    rescue Projects::HousekeepingService::LeaseTaken
+      Repositories::HousekeepingService.new(project, :gc).execute
+    rescue Repositories::HousekeepingService::LeaseTaken
       # No action required
     end
 

app/services/repositories/housekeeping_service.rb

+# frozen_string_literal: true
+
+# Used for git housekeeping
+#
+# Ex.
+#   Repositories::HousekeepingService.new(project).execute
+#   Repositories::HousekeepingService.new(project.wiki).execute
+#
+module Repositories
+  class HousekeepingService < BaseService
+    # Timeout set to 24h
+    LEASE_TIMEOUT = 86400
+    PACK_REFS_PERIOD = 6
+
+    class LeaseTaken < StandardError
+      def to_s
+        "Somebody already triggered housekeeping for this resource in the past #{LEASE_TIMEOUT / 60} minutes"
+      end
+    end
+
+    def initialize(resource, task = nil)
+      @resource = resource
+      @task = task
+    end
+
+    def execute
+      lease_uuid = try_obtain_lease
+      raise LeaseTaken unless lease_uuid.present?
+
+      yield if block_given?
+
+      execute_gitlab_shell_gc(lease_uuid)
+    end
+
+    def needed?
+      pushes_since_gc > 0 && period_match? && housekeeping_enabled?
+    end
+
+    def increment!
+      Gitlab::Metrics.measure(:increment_pushes_since_gc) do
+        @resource.increment_pushes_since_gc
+      end
+    end
+
+    private
+
+    def execute_gitlab_shell_gc(lease_uuid)
+      GitGarbageCollectWorker.perform_async(@resource.id, task, lease_key, lease_uuid)
+    ensure
+      if pushes_since_gc >= gc_period
+        Gitlab::Metrics.measure(:reset_pushes_since_gc) do
+          @resource.reset_pushes_since_gc
+        end
+      end
+    end
+
+    def try_obtain_lease
+      Gitlab::Metrics.measure(:obtain_housekeeping_lease) do
+        lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT)
+        lease.try_obtain
+      end
+    end
+
+    def lease_key
+      "#{@resource.class.name.underscore.pluralize}_housekeeping:#{@resource.id}"
+    end
+
+    def pushes_since_gc
+      @resource.pushes_since_gc
+    end
+
+    def task
+      return @task if @task
+
+      if pushes_since_gc % gc_period == 0
+        :gc
+      elsif pushes_since_gc % full_repack_period == 0
+        :full_repack
+      elsif pushes_since_gc % repack_period == 0
+        :incremental_repack
+      else
+        :pack_refs
+      end
+    end
+
+    def period_match?
+      [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 }
+    end
+
+    def housekeeping_enabled?
+      Gitlab::CurrentSettings.housekeeping_enabled
+    end
+
+    def gc_period
+      Gitlab::CurrentSettings.housekeeping_gc_period
+    end
+
+    def full_repack_period
+      Gitlab::CurrentSettings.housekeeping_full_repack_period
+    end
+
+    def repack_period
+      Gitlab::CurrentSettings.housekeeping_incremental_repack_period
+    end
+  end
+end

app/workers/object_pool/join_worker.rb

@@ -15,7 +15,7 @@ module ObjectPool
 
       project.link_pool_repository
 
-      Projects::HousekeepingService.new(project).execute
+      Repositories::HousekeepingService.new(project).execute
     end
   end
 end

doc/administration/housekeeping.md

@@ -48,7 +48,7 @@ It is ultimately performed by the Gitaly RPC `FetchIntoObjectPool`.
 
 This is the current call stack by which it is invoked:
 
-1. `Projects::HousekeepingService#execute_gitlab_shell_gc`
+1. `Repositories::HousekeepingService#execute_gitlab_shell_gc`
 1. `GitGarbageCollectWorker#perform`
 1. `Projects::GitDeduplicationService#fetch_from_source`
 1. `ObjectPool#fetch`

doc/administration/raketasks/check.md

@@ -214,7 +214,7 @@ If the issue persists, try triggering `gc` via the
 
 ```ruby
 p = Project.find_by_path("project-name")
-Projects::HousekeepingService.new(p, :gc).execute
+Repositories::HousekeepingService.new(p, :gc).execute
 ```
 
 ### Delete references to missing remote uploads

ee/app/services/projects/update_mirror_service.rb

@@ -155,11 +155,11 @@ module Projects
     end
 
     def run_housekeeping
-      service = Projects::HousekeepingService.new(project)
+      service = Repositories::HousekeepingService.new(project)
 
       service.increment!
       service.execute if service.needed?
-    rescue Projects::HousekeepingService::LeaseTaken
+    rescue Repositories::HousekeepingService::LeaseTaken
       # best-effort
     end
 

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -33,8 +33,7 @@ RSpec.describe Projects::UpdateMirrorService do
     it 'runs project housekeeping' do
       stub_fetch_mirror(project)
 
-      expect_next_instance_of(Projects::HousekeepingService) do |svc|
-        expect(svc.project).to eq(project)
+      expect_next_instance_of(Repositories::HousekeepingService) do |svc|
         expect(svc).to receive(:increment!)
         expect(svc).to receive(:needed?).and_return(true)
         expect(svc).to receive(:execute)

ee/spec/services/repositories/housekeeping_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Repositories::HousekeepingService do
+  it_behaves_like 'housekeeps repository' do
+    let_it_be(:resource) { create(:group_wiki) }
+  end
+end

lib/api/projects.rb

@@ -566,8 +566,8 @@ module API
         authorize_admin_project
 
         begin
-          ::Projects::HousekeepingService.new(user_project, :gc).execute
-        rescue ::Projects::HousekeepingService::LeaseTaken => error
+          ::Repositories::HousekeepingService.new(user_project, :gc).execute
+        rescue ::Repositories::HousekeepingService::LeaseTaken => error
           conflict!(error.message)
         end
       end

lib/gitlab/github_import/importer/repository_importer.rb

@@ -56,7 +56,7 @@ module Gitlab
 
           # The initial fetch can bring in lots of loose refs and objects.
           # Running a `git gc` will make importing pull requests faster.
-          Projects::HousekeepingService.new(project, :gc).execute
+          Repositories::HousekeepingService.new(project, :gc).execute
 
           true
         rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error => e

spec/controllers/projects_controller_spec.rb

@@ -483,14 +483,14 @@ RSpec.describe ProjectsController do
   describe '#housekeeping' do
     let_it_be(:group) { create(:group) }
     let_it_be(:project) { create(:project, group: group) }
-    let(:housekeeping) { Projects::HousekeepingService.new(project) }
+    let(:housekeeping) { Repositories::HousekeepingService.new(project) }
 
     context 'when authenticated as owner' do
       before do
         group.add_owner(user)
         sign_in(user)
 
-        allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping)
+        allow(Repositories::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping)
       end
 
       it 'forces a full garbage collection' do

spec/lib/gitlab/github_import/importer/repository_importer_spec.rb

@@ -205,7 +205,7 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do
         .with(project.import_url, refmap: Gitlab::GithubImport.refmap, forced: true, remote_name: 'github')
 
       service = double
-      expect(Projects::HousekeepingService)
+      expect(Repositories::HousekeepingService)
         .to receive(:new).with(project, :gc).and_return(service)
       expect(service).to receive(:execute)
 

spec/models/project_import_state_spec.rb

@@ -103,7 +103,7 @@ RSpec.describe ProjectImportState, type: :model do
         allow(after_import_service)
           .to receive(:execute) { housekeeping_service.execute }
 
-        allow(Projects::HousekeepingService)
+        allow(Repositories::HousekeepingService)
           .to receive(:new) { housekeeping_service }
       end
 

spec/requests/api/projects_spec.rb

@@ -3394,10 +3394,10 @@ RSpec.describe API::Projects do
   end
 
   describe 'POST /projects/:id/housekeeping' do
-    let(:housekeeping) { Projects::HousekeepingService.new(project) }
+    let(:housekeeping) { Repositories::HousekeepingService.new(project) }
 
     before do
-      allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping)
+      allow(Repositories::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping)
     end
 
     context 'when authenticated as owner' do
@@ -3411,12 +3411,12 @@ RSpec.describe API::Projects do
 
       context 'when housekeeping lease is taken' do
         it 'returns conflict' do
-          expect(housekeeping).to receive(:execute).once.and_raise(Projects::HousekeepingService::LeaseTaken)
+          expect(housekeeping).to receive(:execute).once.and_raise(Repositories::HousekeepingService::LeaseTaken)
 
           post api("/projects/#{project.id}/housekeeping", user)
 
           expect(response).to have_gitlab_http_status(:conflict)
-          expect(json_response['message']).to match(/Somebody already triggered housekeeping for this project/)
+          expect(json_response['message']).to match(/Somebody already triggered housekeeping for this resource/)
         end
       end
     end

spec/services/git/branch_push_service_spec.rb

@@ -554,7 +554,7 @@ RSpec.describe Git::BranchPushService, services: true do
   end
 
   describe "housekeeping" do
-    let(:housekeeping) { Projects::HousekeepingService.new(project) }
+    let(:housekeeping) { Repositories::HousekeepingService.new(project) }
 
     before do
       # Flush any raw key-value data stored by the housekeeping code.
@@ -562,7 +562,7 @@ RSpec.describe Git::BranchPushService, services: true do
       Gitlab::Redis::Queues.with { |conn| conn.flushall }
       Gitlab::Redis::SharedState.with { |conn| conn.flushall }
 
-      allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
+      allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
     end
 
     after do

spec/services/projects/after_import_service_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Projects::AfterImportService do
 
   describe '#execute' do
     before do
-      allow(Projects::HousekeepingService)
+      allow(Repositories::HousekeepingService)
         .to receive(:new).with(project).and_return(housekeeping_service)
 
       allow(housekeeping_service)
@@ -73,10 +73,10 @@ RSpec.describe Projects::AfterImportService do
     end
 
     context 'when housekeeping service lease is taken' do
-      let(:exception) { Projects::HousekeepingService::LeaseTaken.new }
+      let(:exception) { Repositories::HousekeepingService::LeaseTaken.new }
 
       it 'logs the error message' do
-        allow_next_instance_of(Projects::HousekeepingService) do |instance|
+        allow_next_instance_of(Repositories::HousekeepingService) do |instance|
           expect(instance).to receive(:execute).and_raise(exception)
         end
 

spec/services/projects/housekeeping_service_spec.rb

@@ -2,127 +2,19 @@
 
 require 'spec_helper'
 
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
+#
+# We're deploying the name of the referenced class in 13.9. Nevertheless,
+# we cannot remove the class entirely because there can be jobs
+# referencing it. We still need this specs to ensure that the old
+# class still has the old behavior.
+#
+# We can get rid of this class in 13.10
+# https://gitlab.com/gitlab-org/gitlab/-/issues/297580
+#
 RSpec.describe Projects::HousekeepingService do
-  subject { described_class.new(project) }
-
-  let_it_be(:project) { create(:project, :repository) }
-
-  before do
-    project.reset_pushes_since_gc
-  end
-
-  after do
-    project.reset_pushes_since_gc
-  end
-
-  describe '#execute' do
-    it 'enqueues a sidekiq job' do
-      expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
-      expect(subject).to receive(:lease_key).and_return(:the_lease_key)
-      expect(subject).to receive(:task).and_return(:incremental_repack)
-      expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid).and_call_original
-
-      Sidekiq::Testing.fake! do
-        expect { subject.execute }.to change(GitGarbageCollectWorker.jobs, :size).by(1)
-      end
-    end
-
-    it 'yields the block if given' do
-      expect do |block|
-        subject.execute(&block)
-      end.to yield_with_no_args
-    end
-
-    it 'resets counter after execution' do
-      expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
-      allow(subject).to receive(:gc_period).and_return(1)
-      project.increment_pushes_since_gc
-
-      perform_enqueued_jobs do
-        expect { subject.execute }.to change { project.pushes_since_gc }.to(0)
-      end
-    end
-
-    context 'when no lease can be obtained' do
-      before do
-        expect(subject).to receive(:try_obtain_lease).and_return(false)
-      end
-
-      it 'does not enqueue a job' do
-        expect(GitGarbageCollectWorker).not_to receive(:perform_async)
-
-        expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
-      end
-
-      it 'does not reset pushes_since_gc' do
-        expect do
-          expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
-        end.not_to change { project.pushes_since_gc }
-      end
-
-      it 'does not yield' do
-        expect do |block|
-          expect { subject.execute(&block) }
-            .to raise_error(Projects::HousekeepingService::LeaseTaken)
-        end.not_to yield_with_no_args
-      end
-    end
-
-    context 'task type' do
-      it 'goes through all three housekeeping tasks, executing only the highest task when there is overlap' do
-        allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
-        allow(subject).to receive(:lease_key).and_return(:the_lease_key)
-
-        # At push 200
-        expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :the_lease_key, :the_uuid)
-          .once
-        # At push 50, 100, 150
-        expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :full_repack, :the_lease_key, :the_uuid)
-          .exactly(3).times
-        # At push 10, 20, ... (except those above)
-        expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid)
-          .exactly(16).times
-        # At push 6, 12, 18, ... (except those above)
-        expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :pack_refs, :the_lease_key, :the_uuid)
-          .exactly(27).times
-
-        201.times do
-          subject.increment!
-          subject.execute if subject.needed?
-        end
-
-        expect(project.pushes_since_gc).to eq(1)
-      end
-    end
-
-    it 'runs the task specifically requested' do
-      housekeeping = described_class.new(project, :gc)
-
-      allow(housekeeping).to receive(:try_obtain_lease).and_return(:gc_uuid)
-      allow(housekeeping).to receive(:lease_key).and_return(:gc_lease_key)
-
-      expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :gc_lease_key, :gc_uuid).twice
-
-      2.times do
-        housekeeping.execute
-      end
-    end
-  end
-
-  describe '#needed?' do
-    it 'when the count is low enough' do
-      expect(subject.needed?).to eq(false)
-    end
-
-    it 'when the count is high enough' do
-      allow(project).to receive(:pushes_since_gc).and_return(10)
-      expect(subject.needed?).to eq(true)
-    end
-  end
-
-  describe '#increment!' do
-    it 'increments the pushes_since_gc counter' do
-      expect { subject.increment! }.to change { project.pushes_since_gc }.by(1)
-    end
+  it_behaves_like 'housekeeps repository' do
+    let_it_be(:resource) { create(:project, :repository) }
   end
 end

spec/services/repositories/housekeeping_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Repositories::HousekeepingService do
+  it_behaves_like 'housekeeps repository' do
+    let_it_be(:resource) { create(:project, :repository) }
+  end
+
+  it_behaves_like 'housekeeps repository' do
+    let_it_be(:project) { create(:project, :wiki_repo) }
+    let_it_be(:resource) { project.wiki }
+  end
+end

spec/support/shared_examples/models/concerns/repositories/can_housekeep_repository_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'can housekeep repository' do
+  context 'with a clean redis state', :clean_gitlab_redis_shared_state do
+    describe '#pushes_since_gc' do
+      context 'without any pushes' do
+        it 'returns 0' do
+          expect(resource.pushes_since_gc).to eq(0)
+        end
+      end
+
+      context 'with a number of pushes' do
+        it 'returns the number of pushes' do
+          3.times { resource.increment_pushes_since_gc }
+
+          expect(resource.pushes_since_gc).to eq(3)
+        end
+      end
+    end
+
+    describe '#increment_pushes_since_gc' do
+      it 'increments the number of pushes since the last GC' do
+        3.times { resource.increment_pushes_since_gc }
+
+        expect(resource.pushes_since_gc).to eq(3)
+      end
+    end
+
+    describe '#reset_pushes_since_gc' do
+      it 'resets the number of pushes since the last GC' do
+        3.times { resource.increment_pushes_since_gc }
+
+        resource.reset_pushes_since_gc
+
+        expect(resource.pushes_since_gc).to eq(0)
+      end
+    end
+
+    describe '#pushes_since_gc_redis_shared_state_key' do
+      it 'returns the proper redis key format' do
+        expect(resource.send(:pushes_since_gc_redis_shared_state_key)).to eq("#{resource_key}/#{resource.id}/pushes_since_gc")
+      end
+    end
+  end
+end

spec/support/shared_examples/services/repositories/housekeeping_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'housekeeps repository' do
+  subject { described_class.new(resource) }
+
+  context 'with a clean redis state', :clean_gitlab_redis_shared_state do
+    describe '#execute' do
+      it 'enqueues a sidekiq job' do
+        expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+        expect(subject).to receive(:lease_key).and_return(:the_lease_key)
+        expect(subject).to receive(:task).and_return(:incremental_repack)
+        expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :incremental_repack, :the_lease_key, :the_uuid).and_call_original
+
+        Sidekiq::Testing.fake! do
+          expect { subject.execute }.to change(GitGarbageCollectWorker.jobs, :size).by(1)
+        end
+      end
+
+      it 'yields the block if given' do
+        expect do |block|
+          subject.execute(&block)
+        end.to yield_with_no_args
+      end
+
+      it 'resets counter after execution' do
+        expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+        allow(subject).to receive(:gc_period).and_return(1)
+        resource.increment_pushes_since_gc
+
+        perform_enqueued_jobs do
+          expect { subject.execute }.to change { resource.pushes_since_gc }.to(0)
+        end
+      end
+
+      context 'when no lease can be obtained' do
+        before do
+          expect(subject).to receive(:try_obtain_lease).and_return(false)
+        end
+
+        it 'does not enqueue a job' do
+          expect(GitGarbageCollectWorker).not_to receive(:perform_async)
+
+          expect { subject.execute }.to raise_error(Repositories::HousekeepingService::LeaseTaken)
+        end
+
+        it 'does not reset pushes_since_gc' do
+          expect do
+            expect { subject.execute }.to raise_error(Repositories::HousekeepingService::LeaseTaken)
+          end.not_to change { resource.pushes_since_gc }
+        end
+
+        it 'does not yield' do
+          expect do |block|
+            expect { subject.execute(&block) }
+              .to raise_error(Repositories::HousekeepingService::LeaseTaken)
+          end.not_to yield_with_no_args
+        end
+      end
+
+      context 'task type' do
+        it 'goes through all three housekeeping tasks, executing only the highest task when there is overlap' do
+          allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+          allow(subject).to receive(:lease_key).and_return(:the_lease_key)
+
+          # At push 200
+          expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :gc, :the_lease_key, :the_uuid)
+            .once
+          # At push 50, 100, 150
+          expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :full_repack, :the_lease_key, :the_uuid)
+            .exactly(3).times
+          # At push 10, 20, ... (except those above)
+          expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :incremental_repack, :the_lease_key, :the_uuid)
+            .exactly(16).times
+          # At push 6, 12, 18, ... (except those above)
+          expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :pack_refs, :the_lease_key, :the_uuid)
+            .exactly(27).times
+
+          201.times do
+            subject.increment!
+            subject.execute if subject.needed?
+          end
+
+          expect(resource.pushes_since_gc).to eq(1)
+        end
+      end
+
+      it 'runs the task specifically requested' do
+        housekeeping = described_class.new(resource, :gc)
+
+        allow(housekeeping).to receive(:try_obtain_lease).and_return(:gc_uuid)
+        allow(housekeeping).to receive(:lease_key).and_return(:gc_lease_key)
+
+        expect(GitGarbageCollectWorker).to receive(:perform_async).with(resource.id, :gc, :gc_lease_key, :gc_uuid).twice
+
+        2.times do
+          housekeeping.execute
+        end
+      end
+    end
+
+    describe '#needed?' do
+      it 'when the count is low enough' do
+        expect(subject.needed?).to eq(false)
+      end
+
+      it 'when the count is high enough' do
+        allow(resource).to receive(:pushes_since_gc).and_return(10)
+        expect(subject.needed?).to eq(true)
+      end
+    end
+
+    describe '#increment!' do
+      it 'increments the pushes_since_gc counter' do
+        expect { subject.increment! }.to change { resource.pushes_since_gc }.by(1)
+      end
+    end
+  end
+end