diff options
author | Stan Hu <stanhu@gmail.com> | 2018-10-05 21:24:00 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-10-05 23:08:55 +0300 |
commit | 8bcad890e0b7505a2c22ffe8219b77e989c1ac4c (patch) | |
tree | fb22fafb810f9a8969a6f74fdc88381a515457ad | |
parent | 0e7df36291946d7e6c53d11f53066895e4132098 (diff) |
Add support for closing Rugged/libgit2 file descriptors
Part of #1359
-rw-r--r-- | changelogs/unreleased/sh-add-rugged-cleanup.yml | 5 | ||||
-rw-r--r-- | ruby/lib/gitlab/git/repository.rb | 16 | ||||
-rw-r--r-- | ruby/spec/lib/gitlab/git/repository_spec.rb | 50 |
3 files changed, 69 insertions, 2 deletions
diff --git a/changelogs/unreleased/sh-add-rugged-cleanup.yml b/changelogs/unreleased/sh-add-rugged-cleanup.yml new file mode 100644 index 000000000..a90c7de87 --- /dev/null +++ b/changelogs/unreleased/sh-add-rugged-cleanup.yml @@ -0,0 +1,5 @@ +--- +title: Add support for closing Rugged/libgit2 file descriptors +merge_request: 903 +author: +type: added diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb index d6ea03041..fe68b45dd 100644 --- a/ruby/lib/gitlab/git/repository.rb +++ b/ruby/lib/gitlab/git/repository.rb @@ -48,6 +48,14 @@ module Gitlab ) end + def from_gitaly_with_block(gitaly_repository, call) + repository = from_gitaly(gitaly_repository, call) + + yield repository + + repository.cleanup + end + def create(repo_path) FileUtils.mkdir_p(repo_path, mode: 0770) @@ -165,7 +173,7 @@ module Gitlab end def rugged - Rugged::Repository.new(path, alternates: alternate_object_directories) + @rugged ||= Rugged::Repository.new(path, alternates: alternate_object_directories) rescue Rugged::RepositoryError, Rugged::OSError raise NoRepository.new('no repository for such path') end @@ -808,6 +816,12 @@ module Gitlab run_git!(args, lazy_block: block) end + def cleanup + # Opening a repository may be expensive, and we only need to close it + # if it's been open. + rugged&.close if defined?(@rugged) + end + private def run_git(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block) diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb index a8592e08b..f43ed2d8b 100644 --- a/ruby/spec/lib/gitlab/git/repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/repository_spec.rb @@ -1,10 +1,58 @@ require 'spec_helper' -describe Gitlab::Git::RevList do +describe Gitlab::Git::Repository do include TestRepo let(:repository) { gitlab_git_from_gitaly(test_repo_read_only) } + describe '#from_gitaly_with_block' do + let(:call_metadata) do + { + 'user-agent' => 'grpc-go/1.9.1', + 'gitaly-storage-path' => DEFAULT_STORAGE_DIR, + 'gitaly-repo-path' => TEST_REPO_PATH, + 'gitaly-gl-repository' => 'project-52', + 'gitaly-repo-alt-dirs' => '' + } + end + let(:call) { double(metadata: call_metadata) } + let(:gitlab_shell_path) { '/foo/bar/gitlab-shell' } + + before do + ENV['GITALY_RUBY_GITLAB_SHELL_PATH'] = gitlab_shell_path + end + + after do + ENV.delete('GITALY_RUBY_GITLAB_SHELL_PATH') + end + + it 'cleans up the repository' do + described_class.from_gitaly_with_block(test_repo_read_only, call) do |repository| + expect(repository.rugged).to receive(:close) + end + end + end + + describe '#cleanup' do + context 'when Rugged has been called' do + it 'calls close on Rugged::Repository' do + rugged = repository.rugged + + expect(rugged).to receive(:close).and_call_original + + repository.cleanup + end + end + + context 'when Rugged has not been called' do + it 'does not call close on Rugged::Repository' do + expect(repository).not_to receive(:rugged) + + repository.cleanup + end + end + end + describe '#parse_raw_diff_line' do let(:diff_data) { repository.parse_raw_diff_line(diff_line) } |