diff options
| -rw-r--r-- | changelogs/unreleased/jc-detect-nfs-for-rugged.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/git/rugged_impl/blob.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/git/rugged_impl/commit.rb | 8 | ||||
| -rw-r--r-- | lib/gitlab/git/rugged_impl/repository.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/git/rugged_impl/tree.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/git/rugged_impl/use_rugged.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client.rb | 40 | ||||
| -rw-r--r-- | spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb | 1 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/commit_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb | 97 |
10 files changed, 171 insertions, 7 deletions
diff --git a/changelogs/unreleased/jc-detect-nfs-for-rugged.yml b/changelogs/unreleased/jc-detect-nfs-for-rugged.yml new file mode 100644 index 00000000000..ca738181a55 --- /dev/null +++ b/changelogs/unreleased/jc-detect-nfs-for-rugged.yml @@ -0,0 +1,5 @@ +--- +title: Use Rugged if we detect storage is NFS and we can access the disk +merge_request: 29725 +author: +type: performance diff --git a/lib/gitlab/git/rugged_impl/blob.rb b/lib/gitlab/git/rugged_impl/blob.rb index 11ee4ebda4b..86c9f33d82a 100644 --- a/lib/gitlab/git/rugged_impl/blob.rb +++ b/lib/gitlab/git/rugged_impl/blob.rb @@ -11,10 +11,11 @@ module Gitlab module Blob module ClassMethods extend ::Gitlab::Utils::Override + include Gitlab::Git::RuggedImpl::UseRugged override :tree_entry def tree_entry(repository, sha, path, limit) - if Feature.enabled?(:rugged_tree_entry) + if use_rugged?(repository, :rugged_tree_entry) rugged_tree_entry(repository, sha, path, limit) else super diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb index bce4fa14fb4..971a33b2e99 100644 --- a/lib/gitlab/git/rugged_impl/commit.rb +++ b/lib/gitlab/git/rugged_impl/commit.rb @@ -12,6 +12,7 @@ module Gitlab module Commit module ClassMethods extend ::Gitlab::Utils::Override + include Gitlab::Git::RuggedImpl::UseRugged def rugged_find(repo, commit_id) obj = repo.rev_parse_target(commit_id) @@ -34,7 +35,7 @@ module Gitlab override :find_commit def find_commit(repo, commit_id) - if Feature.enabled?(:rugged_find_commit) + if use_rugged?(repo, :rugged_find_commit) rugged_find(repo, commit_id) else super @@ -43,7 +44,7 @@ module Gitlab override :batch_by_oid def batch_by_oid(repo, oids) - if Feature.enabled?(:rugged_list_commits_by_oid) + if use_rugged?(repo, :rugged_list_commits_by_oid) rugged_batch_by_oid(repo, oids) else super @@ -52,6 +53,7 @@ module Gitlab end extend ::Gitlab::Utils::Override + include Gitlab::Git::RuggedImpl::UseRugged override :init_commit def init_commit(raw_commit) @@ -65,7 +67,7 @@ module Gitlab override :commit_tree_entry def commit_tree_entry(path) - if Feature.enabled?(:rugged_commit_tree_entry) + if use_rugged?(@repository, :rugged_commit_tree_entry) rugged_tree_entry(path) else super diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb index e91b0ddcd31..9268abdfed9 100644 --- a/lib/gitlab/git/rugged_impl/repository.rb +++ b/lib/gitlab/git/rugged_impl/repository.rb @@ -11,6 +11,7 @@ module Gitlab module RuggedImpl module Repository extend ::Gitlab::Utils::Override + include Gitlab::Git::RuggedImpl::UseRugged FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry rugged_list_commits_by_oid).freeze @@ -46,7 +47,7 @@ module Gitlab override :ancestor? def ancestor?(from, to) - if Feature.enabled?(:rugged_commit_is_ancestor) + if use_rugged?(self, :rugged_commit_is_ancestor) rugged_is_ancestor?(from, to) else super diff --git a/lib/gitlab/git/rugged_impl/tree.rb b/lib/gitlab/git/rugged_impl/tree.rb index 9c37bb01961..f3721a3f1b7 100644 --- a/lib/gitlab/git/rugged_impl/tree.rb +++ b/lib/gitlab/git/rugged_impl/tree.rb @@ -11,10 +11,11 @@ module Gitlab module Tree module ClassMethods extend ::Gitlab::Utils::Override + include Gitlab::Git::RuggedImpl::UseRugged override :tree_entries def tree_entries(repository, sha, path, recursive) - if Feature.enabled?(:rugged_tree_entries) + if use_rugged?(repository, :rugged_tree_entries) tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive) else super diff --git a/lib/gitlab/git/rugged_impl/use_rugged.rb b/lib/gitlab/git/rugged_impl/use_rugged.rb new file mode 100644 index 00000000000..99091b03cd1 --- /dev/null +++ b/lib/gitlab/git/rugged_impl/use_rugged.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Gitlab + module Git + module RuggedImpl + module UseRugged + def use_rugged?(repo, feature_key) + feature = Feature.get(feature_key) + return feature.enabled? if Feature.persisted?(feature) + + Gitlab::GitalyClient.can_use_disk?(repo.storage) + end + end + end + end +end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 9e3de910e3c..cf0157269a8 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -30,6 +30,7 @@ module Gitlab SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION' MAXIMUM_GITALY_CALLS = 30 CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze + GITALY_METADATA_FILENAME = '.gitaly-metadata' MUTEX = Mutex.new @@ -378,6 +379,45 @@ module Gitlab 0 end + def self.storage_metadata_file_path(storage) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join( + Gitlab.config.repositories.storages[storage].legacy_disk_path, GITALY_METADATA_FILENAME + ) + end + end + + def self.can_use_disk?(storage) + cached_value = MUTEX.synchronize do + @can_use_disk ||= {} + @can_use_disk[storage] + end + + return cached_value unless cached_value.nil? + + gitaly_filesystem_id = filesystem_id(storage) + direct_filesystem_id = filesystem_id_from_disk(storage) + + MUTEX.synchronize do + @can_use_disk[storage] = gitaly_filesystem_id.present? && + gitaly_filesystem_id == direct_filesystem_id + end + end + + def self.filesystem_id(storage) + response = Gitlab::GitalyClient::ServerService.new(storage).info + storage_status = response.storage_statuses.find { |status| status.storage_name == storage } + storage_status.filesystem_id + end + + def self.filesystem_id_from_disk(storage) + metadata_file = File.read(storage_metadata_file_path(storage)) + metadata_hash = JSON.parse(metadata_file) + metadata_hash['gitaly_filesystem_id'] + rescue Errno::ENOENT, JSON::ParserError + nil + end + def self.timeout(timeout_name) Gitlab::CurrentSettings.current_application_settings[timeout_name] end diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb index 483c5ea9cff..972dd7e0d2b 100644 --- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb +++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb @@ -26,6 +26,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do end it 'loads 10 projects without hitting Gitaly call limit', :request_store do + allow(Gitlab::GitalyClient).to receive(:can_access_disk?).and_return(false) projects = Gitlab::GitalyClient.allow_n_plus_1_calls do (1..10).map { create(:project, :repository) } end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 25052a79916..3f0e6b34291 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -408,7 +408,7 @@ describe Gitlab::Git::Commit, :seed_helper do context 'when oids is empty' do it 'makes no Gitaly request' do - expect(Gitlab::GitalyClient).not_to receive(:call) + expect(Gitlab::GitalyClient).not_to receive(:call).with(repository.storage, :commit_service, :list_commits_by_oid) described_class.batch_by_oid(repository, []) end diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb new file mode 100644 index 00000000000..f957ed00945 --- /dev/null +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'json' +require 'tempfile' + +describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:feature_flag_name) { 'feature-flag-name' } + let(:feature_flag) { Feature.get(feature_flag_name) } + let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } + + before(:all) do + create_gitaly_metadata_file + end + + subject(:wrapper) do + klazz = Class.new { include Gitlab::Git::RuggedImpl::UseRugged } + klazz.new + end + + before do + Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {}) + end + + context 'when feature flag is not persisted' do + before do + allow(Feature).to receive(:persisted?).with(feature_flag).and_return(false) + end + + it 'returns true when gitaly matches disk' do + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + end + + it 'returns false when disk access fails' do + allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return("/fake/path/doesnt/exist") + + expect(subject.use_rugged?(repository, feature_flag_name)).to be false + end + + it "returns false when gitaly doesn't match disk" do + allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + + File.delete(temp_gitaly_metadata_file) + end + + it "doesn't lead to a second rpc call because gitaly client should use the cached value" do + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + + expect(Gitlab::GitalyClient).not_to receive(:filesystem_id) + + subject.use_rugged?(repository, feature_flag_name) + end + end + + context 'when feature flag is persisted' do + before do + allow(Feature).to receive(:persisted?).with(feature_flag).and_return(true) + end + + it 'returns false when the feature flag is off' do + allow(feature_flag).to receive(:enabled?).and_return(false) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + end + + it "returns true when feature flag is on" do + allow(feature_flag).to receive(:enabled?).and_return(true) + allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(false) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + end + end + + def create_temporary_gitaly_metadata_file + tmp = Tempfile.new('.gitaly-metadata') + gitaly_metadata = { + "gitaly_filesystem_id" => "some-value" + } + tmp.write(gitaly_metadata.to_json) + tmp.flush + tmp.close + tmp.path + end + + def create_gitaly_metadata_file + File.open(File.join(SEED_STORAGE_PATH, '.gitaly-metadata'), 'w+') do |f| + gitaly_metadata = { + "gitaly_filesystem_id" => SecureRandom.uuid + } + f.write(gitaly_metadata.to_json) + end + end +end |