summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-07-22 06:38:02 -0700
committerStan Hu <stanhu@gmail.com>2016-07-22 06:38:02 -0700
commit391064830d82c4b009ab7f698cba38141f449f76 (patch)
treec7db53a78bde06854d00bd25f0e83f2886b00bdf
parent12dd0f62012c6df8bd67abc2d9c5c54bd82366f7 (diff)
downloadgitlab-ce-handle-invalid-kept-around-references.tar.gz
Gracefully handle case when keep-around references are corrupted or exist alreadyhandle-invalid-kept-around-references
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 do not allow you to delete or re-create a reference that has been corrupted, even with the force parameter. Closes #20109
-rw-r--r--app/models/repository.rb13
-rw-r--r--spec/models/repository_spec.rb20
2 files changed, 31 insertions, 2 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 46a04eb80cd..1d3df6f9eaf 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -211,11 +211,20 @@ class Repository
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
def kept_around?(sha)
- ref_exists?(keep_around_ref_name(sha))
+ begin
+ ref_exists?(keep_around_ref_name(sha))
+ rescue Rugged::ReferenceError
+ false
+ end
end
def tag_names
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 3e133143bbc..9b21d030416 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1164,10 +1164,30 @@ describe Repository, models: true do
end
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
repository.keep_around(sample_commit.id)
expect(repository.kept_around?(sample_commit.id)).to be_truthy
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