Merge branch 'id-optimize-housekeeping' into 'master'

Use OptimizeRepository and PruneUnreachableObjects for housekeeping

See merge request gitlab-org/gitlab!81465

app/workers/concerns/git_garbage_collect_methods.rb

@@ -83,17 +83,27 @@ module GitGarbageCollectMethods
   def gitaly_call(task, resource)
     repository = resource.repository.raw_repository
 
-    client = get_gitaly_client(task, repository)
-
-    case task
-    when :prune, :gc
-      client.garbage_collect(bitmaps_enabled?, prune: task == :prune)
-    when :full_repack
-      client.repack_full(bitmaps_enabled?)
-    when :incremental_repack
-      client.repack_incremental
-    when :pack_refs
-      client.pack_refs
+    if Feature.enabled?(:optimized_housekeeping, container(resource), default_enabled: :yaml)
+      client = repository.gitaly_repository_client
+
+      if task == :prune
+        client.prune_unreachable_objects
+      else
+        client.optimize_repository
+      end
+    else
+      client = get_gitaly_client(task, repository)
+
+      case task
+      when :prune, :gc
+        client.garbage_collect(bitmaps_enabled?, prune: task == :prune)
+      when :full_repack
+        client.repack_full(bitmaps_enabled?)
+      when :incremental_repack
+        client.repack_incremental
+      when :pack_refs
+        client.pack_refs
+      end
     end
   rescue GRPC::NotFound => e
     Gitlab::GitLogger.error("#{__method__} failed:\nRepository not found")

app/workers/projects/git_garbage_collect_worker.rb

@@ -7,6 +7,12 @@ module Projects
 
     private
 
+    # Used for getting a project/group out of the resource in order to scope a feature flag
+    # Can be removed within https://gitlab.com/gitlab-org/gitlab/-/issues/353607
+    def container(resource)
+      resource
+    end
+
     override :find_resource
     def find_resource(id)
       Project.find(id)

app/workers/wikis/git_garbage_collect_worker.rb

@@ -7,6 +7,12 @@ module Wikis
 
     private
 
+    # Used for getting a project/group out of the resource in order to scope a feature flag
+    # Can be removed within https://gitlab.com/gitlab-org/gitlab/-/issues/353607
+    def container(resource)
+      resource.container
+    end
+
     override :find_resource
     def find_resource(id)
       Project.find(id).wiki

config/feature_flags/development/optimized_housekeeping.yml

+---
+name: optimized_housekeeping
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81465
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353607
+milestone: '14.9'
+type: development
+group: group::source code
+default_enabled: false

ee/app/workers/group_wikis/git_garbage_collect_worker.rb

@@ -7,6 +7,12 @@ module GroupWikis
 
     private
 
+    # Used for getting a project/group out of the resource in order to scope a feature flag
+    # Can be removed within https://gitlab.com/gitlab-org/gitlab/-/issues/353607
+    def container(resource)
+      resource.container
+    end
+
     override :find_resource
     def find_resource(id)
       Group.find(id).wiki

lib/gitlab/gitaly_client/repository_service.rb

@@ -21,6 +21,16 @@ module Gitlab
         response.exists
       end
 
+      def optimize_repository
+        request = Gitaly::OptimizeRepositoryRequest.new(repository: @gitaly_repo)
+        GitalyClient.call(@storage, :repository_service, :optimize_repository, request, timeout: GitalyClient.long_timeout)
+      end
+
+      def prune_unreachable_objects
+        request = Gitaly::PruneUnreachableObjectsRequest.new(repository: @gitaly_repo)
+        GitalyClient.call(@storage, :repository_service, :prune_unreachable_objects, request, timeout: GitalyClient.long_timeout)
+      end
+
       def garbage_collect(create_bitmap, prune:)
         request = Gitaly::GarbageCollectRequest.new(repository: @gitaly_repo, create_bitmap: create_bitmap, prune: prune)
         GitalyClient.call(@storage, :repository_service, :garbage_collect, request, timeout: GitalyClient.long_timeout)

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -54,6 +54,28 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
     end
   end
 
+  describe '#optimize_repository' do
+    it 'sends a optimize_repository message' do
+      expect_any_instance_of(Gitaly::RepositoryService::Stub)
+        .to receive(:optimize_repository)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return(double(:optimize_repository))
+
+      client.optimize_repository
+    end
+  end
+
+  describe '#prune_unreachable_objects' do
+    it 'sends a prune_unreachable_objects message' do
+      expect_any_instance_of(Gitaly::RepositoryService::Stub)
+        .to receive(:prune_unreachable_objects)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return(double(:prune_unreachable_objects))
+
+      client.prune_unreachable_objects
+    end
+  end
+
   describe '#repository_size' do
     it 'sends a repository_size message' do
       expect_any_instance_of(Gitaly::RepositoryService::Stub)

spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb

@@ -19,14 +19,28 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true|
   end
 
   shared_examples 'it calls Gitaly' do
-    specify do
-      repository_service = instance_double(Gitlab::GitalyClient::RepositoryService)
+    let(:repository_service) { instance_double(Gitlab::GitalyClient::RepositoryService) }
 
-      expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service)
-      expect(repository_service).to receive(gitaly_task)
+    specify do
+      expect_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance|
+        expect(instance).to receive(:optimize_repository).and_call_original
+      end
 
       subject.perform(*params)
     end
+
+    context 'when optimized_housekeeping feature is disabled' do
+      before do
+        stub_feature_flags(optimized_housekeeping: false)
+      end
+
+      specify do
+        expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service)
+        expect(repository_service).to receive(gitaly_task)
+
+        subject.perform(*params)
+      end
+    end
   end
 
   shared_examples 'it updates the resource statistics' do
@@ -70,12 +84,31 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true|
       end
 
       it 'handles gRPC errors' do
-        allow_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance|
-          allow(instance).to receive(:garbage_collect).and_raise(GRPC::NotFound)
+        repository_service = instance_double(Gitlab::GitalyClient::RepositoryService)
+
+        allow_next_instance_of(Projects::GitDeduplicationService) do |instance|
+          allow(instance).to receive(:execute)
         end
 
+        allow(repository.raw_repository).to receive(:gitaly_repository_client).and_return(repository_service)
+        allow(repository_service).to receive(:optimize_repository).and_raise(GRPC::NotFound)
+
         expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository)
       end
+
+      context 'when optimized_housekeeping feature flag is disabled' do
+        before do
+          stub_feature_flags(optimized_housekeeping: false)
+        end
+
+        it 'handles gRPC errors' do
+          allow_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance|
+            allow(instance).to receive(:garbage_collect).and_raise(GRPC::NotFound)
+          end
+
+          expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository)
+        end
+      end
     end
 
     context 'with different lease than the active one' do
@@ -152,13 +185,8 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true|
         expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
       end
 
-      it 'calls Gitaly' do
-        repository_service = instance_double(Gitlab::GitalyClient::RefService)
-
-        expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service)
-        expect(repository_service).to receive(gitaly_task)
-
-        subject.perform(*params)
+      it_behaves_like 'it calls Gitaly' do
+        let(:repository_service) { instance_double(Gitlab::GitalyClient::RefService) }
       end
 
       it 'does not update the resource statistics' do
@@ -180,10 +208,26 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true|
       it_behaves_like 'it updates the resource statistics' if update_statistics
     end
 
+    context 'prune' do
+      before do
+        expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
+      end
+
+      specify do
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance|
+          expect(instance).to receive(:prune_unreachable_objects).and_call_original
+        end
+
+        subject.perform(resource.id, 'prune', lease_key, lease_uuid)
+      end
+    end
+
     shared_examples 'gc tasks' do
       before do
         allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
         allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled)
+
+        stub_feature_flags(optimized_housekeeping: false)
       end
 
       it 'incremental repack adds a new packfile' do