Commit fa4eeb49 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'handle-invalid-kept-around-references' into 'master'

Gracefully handle case when keep-around references are corrupted or exist already

We were seeing a number of error messages when attempting to create a keep-around ref:
    
1. Failed to create locked file `refs/keep-around/XYZ`: File exists
2. Failed to write reference `refs/keep-around/XYZ`: a reference with that name already exists.
    
I'm not sure how these happen, but I suspect when multiple workers attempt to write the same file we may have an issue. The force parameter should help ensure the file gets created,
as well as the rescues to prevent 500 Errors.
    
Rugged/libgit2 unfortunately does not allow you to delete or re-create a reference that has been corrupted, even with the force parameter. A truncated reference will stay that way until manual intervention.
    
Closes #20109


See merge request !5430
parents 5237957c 39106483
...@@ -211,11 +211,20 @@ class Repository ...@@ -211,11 +211,20 @@ class Repository
return if kept_around?(sha) return if kept_around?(sha)
rugged.references.create(keep_around_ref_name(sha), sha) # This will still fail if the file is corrupted (e.g. 0 bytes)
begin
rugged.references.create(keep_around_ref_name(sha), sha, force: true)
rescue Rugged::ReferenceError => ex
Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
end
end end
def kept_around?(sha) def kept_around?(sha)
begin
ref_exists?(keep_around_ref_name(sha)) ref_exists?(keep_around_ref_name(sha))
rescue Rugged::ReferenceError
false
end
end end
def tag_names def tag_names
......
...@@ -1164,10 +1164,30 @@ describe Repository, models: true do ...@@ -1164,10 +1164,30 @@ describe Repository, models: true do
end end
describe "#keep_around" do describe "#keep_around" do
it "does not fail if we attempt to reference bad commit" do
expect(repository.kept_around?('abc1234')).to be_falsey
end
it "stores a reference to the specified commit sha so it isn't garbage collected" do it "stores a reference to the specified commit sha so it isn't garbage collected" do
repository.keep_around(sample_commit.id) repository.keep_around(sample_commit.id)
expect(repository.kept_around?(sample_commit.id)).to be_truthy expect(repository.kept_around?(sample_commit.id)).to be_truthy
end end
it "attempting to call keep_around on truncated ref does not fail" do
repository.keep_around(sample_commit.id)
ref = repository.send(:keep_around_ref_name, sample_commit.id)
path = File.join(repository.path, ref)
# Corrupt the reference
File.truncate(path, 0)
expect(repository.kept_around?(sample_commit.id)).to be_falsey
repository.keep_around(sample_commit.id)
expect(repository.kept_around?(sample_commit.id)).to be_falsey
File.delete(path)
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment