Merge branch 'pks-gitaly-drop-cleanup-calls' into 'master'

git: Stop calling Gitaly's Cleanup RPC

See merge request gitlab-org/gitlab!80485

ee/app/services/geo/design_repository_sync_service.rb

@@ -44,8 +44,6 @@ module Geo
       log_error(message, error)
 
       registry.fail_sync!(message, error, attrs)
-
-      repository.clean_stale_repository_files
     end
 
     def start_registry_sync!

ee/app/services/geo/framework_repository_sync_service.rb

@@ -168,8 +168,6 @@ module Geo
       registry = replicator.registry
       registry.force_to_redownload = force_to_redownload
       registry.failed!(message: message, error: error)
-
-      repository.clean_stale_repository_files
     end
 
     def download_time_in_seconds

ee/app/services/geo/repository_base_sync_service.rb

@@ -181,8 +181,6 @@ module Geo
       log_error(message, error)
 
       registry.fail_sync!(type, message, error, attrs)
-
-      repository.clean_stale_repository_files
     end
 
     def type

ee/spec/services/geo/framework_repository_sync_service_spec.rb

@@ -226,12 +226,6 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
                                               last_sync_failure: 'Error syncing repository: shell error'
                                              )
         end
-
-        it 'calls repository cleanup' do
-          expect(repository).to receive(:clean_stale_repository_files)
-
-          subject.execute
-        end
       end
     end
 

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -285,12 +285,6 @@ RSpec.describe Geo::RepositorySyncService, :geo do
                                               last_repository_sync_failure: 'Error syncing repository: shell error'
                                              )
         end
-
-        it 'calls repository cleanup' do
-          expect(repository).to receive(:clean_stale_repository_files)
-
-          subject.execute
-        end
       end
     end
 

ee/spec/services/geo/wiki_sync_service_spec.rb

@@ -224,12 +224,6 @@ RSpec.describe Geo::WikiSyncService, :geo do
                                               last_wiki_sync_failure: 'Error syncing wiki repository: shell error'
                                              )
         end
-
-        it 'calls repository cleanup' do
-          expect(repository).to receive(:clean_stale_repository_files)
-
-          subject.execute
-        end
       end
 
       context 'no Wiki repository' do

lib/gitlab/git/repository.rb

@@ -972,18 +972,6 @@ module Gitlab
         @praefect_info_client ||= Gitlab::GitalyClient::PraefectInfoService.new(self)
       end
 
-      def clean_stale_repository_files
-        wrapped_gitaly_errors do
-          gitaly_repository_client.cleanup if exists?
-        end
-      rescue Gitlab::Git::CommandError => e # Don't fail if we can't cleanup
-        Gitlab::AppLogger.error("Unable to clean repository on storage #{storage} with relative path #{relative_path}: #{e.message}")
-        Gitlab::Metrics.counter(
-          :failed_repository_cleanup_total,
-          'Number of failed repository cleanup events'
-        ).increment
-      end
-
       def branch_names_contains_sha(sha)
         gitaly_ref_client.branch_names_contains_sha(sha)
       end

lib/gitlab/git_access.rb

@@ -328,11 +328,6 @@ module Gitlab
           raise ForbiddenError, error_message(:push_code)
         end
       else
-        # If there are worktrees with a HEAD pointing to a non-existent object,
-        # calls to `git rev-list --all` will fail in git 2.15+. This should also
-        # clear stale lock files.
-        project.repository.clean_stale_repository_files if project.present?
-
         check_access!
       end
     end
@@ -467,11 +462,6 @@ module Gitlab
     def check_push_size!
       return unless check_size_limit?
 
-      # If there are worktrees with a HEAD pointing to a non-existent object,
-      # calls to `git rev-list --all` will fail in git 2.15+. This should also
-      # clear stale lock files.
-      repository.clean_stale_repository_files
-
       # Use #check_repository_disk_size to get correct push size whenever a lot of changes
       # gets pushed at the same time containing the same blobs. This is only
       # doable if GIT_OBJECT_DIRECTORY_RELATIVE env var is set and happens

lib/gitlab/gitaly_client/repository_service.rb

@@ -21,11 +21,6 @@ module Gitlab
         response.exists
       end
 
-      def cleanup
-        request = Gitaly::CleanupRequest.new(repository: @gitaly_repo)
-        GitalyClient.call(@storage, :repository_service, :cleanup, request, timeout: GitalyClient.fast_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/git/repository_spec.rb

@@ -2252,44 +2252,6 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
-  describe '#clean_stale_repository_files' do
-    let(:worktree_id) { 'rebase-1' }
-    let(:gitlab_worktree_path) { File.join(repository_path, 'gitlab-worktree', worktree_id) }
-    let(:admin_dir) { File.join(repository_path, 'worktrees') }
-
-    it 'cleans up the files' do
-      create_worktree = %W[git -C #{repository_path} worktree add --detach #{gitlab_worktree_path} master]
-      raise 'preparation failed' unless system(*create_worktree, err: '/dev/null')
-
-      FileUtils.touch(gitlab_worktree_path, mtime: Time.now - 8.hours)
-      # git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object,
-      # but the HEAD must be 40 characters long or git will ignore it.
-      File.write(File.join(admin_dir, worktree_id, 'HEAD'), Gitlab::Git::BLANK_SHA)
-
-      expect(rev_list_all).to be(false)
-      repository.clean_stale_repository_files
-
-      expect(rev_list_all).to be(true)
-      expect(File.exist?(gitlab_worktree_path)).to be_falsey
-    end
-
-    def rev_list_all
-      system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null')
-    end
-
-    it 'increments a counter upon an error' do
-      expect(repository.gitaly_repository_client).to receive(:cleanup).and_raise(Gitlab::Git::CommandError)
-
-      counter = double(:counter)
-
-      expect(counter).to receive(:increment)
-      expect(Gitlab::Metrics).to receive(:counter).with(:failed_repository_cleanup_total,
-                                                        'Number of failed repository cleanup events').and_return(counter)
-
-      repository.clean_stale_repository_files
-    end
-  end
-
   describe '#squash' do
     let(:branch_name) { 'fix' }
     let(:start_sha) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' }

spec/lib/gitlab/git_access_spec.rb

@@ -1008,11 +1008,6 @@ RSpec.describe Gitlab::GitAccess do
         end
       end
 
-      it 'cleans up the files' do
-        expect(project.repository).to receive(:clean_stale_repository_files).and_call_original
-        expect { push_access_check }.not_to raise_error
-      end
-
       it 'avoids N+1 queries', :request_store do
         # Run this once to establish a baseline. Cached queries should get
         # cached, so that when we introduce another change we shouldn't see

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -21,16 +21,6 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
     end
   end
 
-  describe '#cleanup' do
-    it 'sends a cleanup message' do
-      expect_any_instance_of(Gitaly::RepositoryService::Stub)
-        .to receive(:cleanup)
-        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
-
-      client.cleanup
-    end
-  end
-
   describe '#garbage_collect' do
     it 'sends a garbage_collect message' do
       expect_any_instance_of(Gitaly::RepositoryService::Stub)