From 9c2baada57a6a4c6746ce9fa37ec33d94e2dff09 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 25 Jul 2019 16:15:00 -0700
Subject: [PATCH] Ignore Gitaly errors if cache flushing fails on project
 destruction

We should just ignore these errors and move along with the deletion
since the repositories are going to be trashed anyway.

Closes https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31164
---
 app/services/projects/destroy_service.rb        | 13 +++++++++++--
 .../sh-ignore-git-errors-delete-project.yml     |  5 +++++
 spec/services/projects/destroy_service_spec.rb  | 17 ++++++++++++++++-
 3 files changed, 32 insertions(+), 3 deletions(-)
 create mode 100644 changelogs/unreleased/sh-ignore-git-errors-delete-project.yml

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index b805a7f1211..a1279bfb3a3 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -210,11 +210,20 @@ module Projects
     end
 
     def flush_caches(project)
-      project.repository.before_delete
+      ignore_git_errors(repo_path) { project.repository.before_delete }
 
-      Repository.new(wiki_path, project, disk_path: repo_path).before_delete
+      ignore_git_errors(wiki_path) { Repository.new(wiki_path, project, disk_path: repo_path).before_delete }
 
       Projects::ForksCountService.new(project).delete_cache
     end
+
+    # If we get a Gitaly error, the repository may be corrupted. We can
+    # ignore these errors since we're going to trash the repositories
+    # anyway.
+    def ignore_git_errors(disk_path, &block)
+      yield
+    rescue Gitlab::Git::CommandError => e
+      Gitlab::GitLogger.warn(class: self.class.name, project_id: project.id, disk_path: disk_path, message: e.to_s)
+    end
   end
 end
diff --git a/changelogs/unreleased/sh-ignore-git-errors-delete-project.yml b/changelogs/unreleased/sh-ignore-git-errors-delete-project.yml
new file mode 100644
index 00000000000..f5e2147f00e
--- /dev/null
+++ b/changelogs/unreleased/sh-ignore-git-errors-delete-project.yml
@@ -0,0 +1,5 @@
+---
+title: Ignore Gitaly errors if cache flushing fails on project destruction
+merge_request: 31164
+author:
+type: fixed
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 3af7ee3ad50..e436af77ed4 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -121,7 +121,22 @@ describe Projects::DestroyService do
     it { expect(Dir.exist?(remove_path)).to be_truthy }
   end
 
-  context 'when flushing caches fail' do
+  context 'when flushing caches fail due to Git errors' do
+    before do
+      allow(project.repository).to receive(:before_delete).and_raise(::Gitlab::Git::CommandError)
+      allow(Gitlab::GitLogger).to receive(:warn).with(
+        class: described_class.name,
+        project_id: project.id,
+        disk_path: project.disk_path,
+        message: 'Gitlab::Git::CommandError').and_call_original
+
+      perform_enqueued_jobs { destroy_project(project, user, {}) }
+    end
+
+    it_behaves_like 'deleting the project'
+  end
+
+  context 'when flushing caches fail due to Redis' do
     before do
       new_user = create(:user)
       project.team.add_user(new_user, Gitlab::Access::DEVELOPER)
-- 
2.30.9