Backport Repository#keep_around changes from EE to CEmk/geo-replicate-keep-around-refs-ce-backport

5 files changed, 42 insertions, 18 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index e4aed76f611..526bf7af99b 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -649,8 +649,7 @@ module Ci
     def keep_around_commits
       return unless project
 
-      project.repository.keep_around(self.sha)
-      project.repository.keep_around(self.before_sha)
+      project.repository.keep_around(self.sha, self.before_sha)
     end
 
     def valid_source
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 58d949315e0..716cf6574d3 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -191,14 +191,18 @@ class DiffNote < Note
   end
 
   def keep_around_commits
-    project.repository.keep_around(self.original_position.base_sha)
-    project.repository.keep_around(self.original_position.start_sha)
-    project.repository.keep_around(self.original_position.head_sha)
+    shas = [
+      self.original_position.base_sha,
+      self.original_position.start_sha,
+      self.original_position.head_sha
+    ]
 
     if self.position != self.original_position
-      project.repository.keep_around(self.position.base_sha)
-      project.repository.keep_around(self.position.start_sha)
-      project.repository.keep_around(self.position.head_sha)
+      shas << self.position.base_sha
+      shas << self.position.start_sha
+      shas << self.position.head_sha
     end
+
+    project.repository.keep_around(*shas)
   end
 end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index d9393b4e545..bbe4f6f7969 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -314,9 +314,7 @@ class MergeRequestDiff < ActiveRecord::Base
 
   def keep_around_commits
     [repository, merge_request.source_project.repository].uniq.each do |repo|
-      repo.keep_around(start_commit_sha)
-      repo.keep_around(head_commit_sha)
-      repo.keep_around(base_commit_sha)
+      repo.keep_around(start_commit_sha, head_commit_sha, base_commit_sha)
     end
   end
 end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 69f375dc6f3..cf255c8951f 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -247,15 +247,22 @@ class Repository
   # Git GC will delete commits from the repository that are no longer in any
   # branches or tags, but we want to keep some of these commits around, for
   # example if they have comments or CI builds.
-  def keep_around(sha)
-    return unless sha.present? && commit_by(oid: sha)
+  #
+  # For Geo's sake, pass in multiple shas rather than calling it multiple times,
+  # to avoid unnecessary syncing.
+  def keep_around(*shas)
+    shas.each do |sha|
+      begin
+        next unless sha.present? && commit_by(oid: sha)
 
-    return if kept_around?(sha)
+        next if kept_around?(sha)
 
-    # This will still fail if the file is corrupted (e.g. 0 bytes)
-    raw_repository.write_ref(keep_around_ref_name(sha), sha, shell: false)
-  rescue Gitlab::Git::CommandError => ex
-    Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}"
+        # This will still fail if the file is corrupted (e.g. 0 bytes)
+        raw_repository.write_ref(keep_around_ref_name(sha), sha, shell: false)
+      rescue Gitlab::Git::CommandError => ex
+        Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}"
+      end
+    end
   end
 
   def kept_around?(sha)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 2859d5149ec..1dc84c012ba 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2005,6 +2005,22 @@ describe Repository do
 
       File.delete(path)
     end
+
+    context 'for multiple SHAs' do
+      it 'skips non-existent SHAs' do
+        repository.keep_around('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', sample_commit.id)
+
+        expect(repository.kept_around?(sample_commit.id)).to be_truthy
+      end
+
+      it 'skips already-kept-around SHAs' do
+        repository.keep_around(sample_commit.id)
+
+        repository.keep_around(sample_commit.id, another_sample_commit.id)
+
+        expect(repository.kept_around?(another_sample_commit.id)).to be_truthy
+      end
+    end
   end
 
   describe '#update_ref' do