Merge branch 'mk/geo-replicate-keep-around-refs-ce-backport' into 'master'

Backport `Repository#keep_around` changes from EE to CE

See merge request gitlab-org/gitlab-ce!21290

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

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

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

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)

spec/models/repository_spec.rb

@@ -2005,6 +2005,24 @@ 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)
+
+        expect(repository.raw_repository).to receive(:write_ref).exactly(1).and_call_original
+
+        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