summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--changelogs/unreleased/jc-detect-nfs-for-rugged.yml5
-rw-r--r--lib/gitlab/git/rugged_impl/blob.rb3
-rw-r--r--lib/gitlab/git/rugged_impl/commit.rb8
-rw-r--r--lib/gitlab/git/rugged_impl/repository.rb3
-rw-r--r--lib/gitlab/git/rugged_impl/tree.rb3
-rw-r--r--lib/gitlab/git/rugged_impl/use_rugged.rb16
-rw-r--r--lib/gitlab/gitaly_client.rb40
-rw-r--r--spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb1
-rw-r--r--spec/lib/gitlab/git/commit_spec.rb2
-rw-r--r--spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb97
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