Merge branch 'jc-wrap-rugged-calls-with-disk-access' into 'master'
Wrap rugged calls with access disk block See merge request gitlab-org/gitlab-ce!30592
This commit is contained in:
commit
ddf2dcf7fd
|
@ -16,7 +16,7 @@ module Gitlab
|
||||||
override :tree_entry
|
override :tree_entry
|
||||||
def tree_entry(repository, sha, path, limit)
|
def tree_entry(repository, sha, path, limit)
|
||||||
if use_rugged?(repository, :rugged_tree_entry)
|
if use_rugged?(repository, :rugged_tree_entry)
|
||||||
rugged_tree_entry(repository, sha, path, limit)
|
wrap_rugged_call { rugged_tree_entry(repository, sha, path, limit) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
|
@ -36,7 +36,7 @@ module Gitlab
|
||||||
override :find_commit
|
override :find_commit
|
||||||
def find_commit(repo, commit_id)
|
def find_commit(repo, commit_id)
|
||||||
if use_rugged?(repo, :rugged_find_commit)
|
if use_rugged?(repo, :rugged_find_commit)
|
||||||
rugged_find(repo, commit_id)
|
wrap_rugged_call { rugged_find(repo, commit_id) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
@ -45,7 +45,7 @@ module Gitlab
|
||||||
override :batch_by_oid
|
override :batch_by_oid
|
||||||
def batch_by_oid(repo, oids)
|
def batch_by_oid(repo, oids)
|
||||||
if use_rugged?(repo, :rugged_list_commits_by_oid)
|
if use_rugged?(repo, :rugged_list_commits_by_oid)
|
||||||
rugged_batch_by_oid(repo, oids)
|
wrap_rugged_call { rugged_batch_by_oid(repo, oids) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
@ -68,7 +68,7 @@ module Gitlab
|
||||||
override :commit_tree_entry
|
override :commit_tree_entry
|
||||||
def commit_tree_entry(path)
|
def commit_tree_entry(path)
|
||||||
if use_rugged?(@repository, :rugged_commit_tree_entry)
|
if use_rugged?(@repository, :rugged_commit_tree_entry)
|
||||||
rugged_tree_entry(path)
|
wrap_rugged_call { rugged_tree_entry(path) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
|
@ -48,7 +48,7 @@ module Gitlab
|
||||||
override :ancestor?
|
override :ancestor?
|
||||||
def ancestor?(from, to)
|
def ancestor?(from, to)
|
||||||
if use_rugged?(self, :rugged_commit_is_ancestor)
|
if use_rugged?(self, :rugged_commit_is_ancestor)
|
||||||
rugged_is_ancestor?(from, to)
|
wrap_rugged_call { rugged_is_ancestor?(from, to) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
|
@ -16,7 +16,7 @@ module Gitlab
|
||||||
override :tree_entries
|
override :tree_entries
|
||||||
def tree_entries(repository, sha, path, recursive)
|
def tree_entries(repository, sha, path, recursive)
|
||||||
if use_rugged?(repository, :rugged_tree_entries)
|
if use_rugged?(repository, :rugged_tree_entries)
|
||||||
tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive)
|
wrap_rugged_call { tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive) }
|
||||||
else
|
else
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
|
@ -10,6 +10,12 @@ module Gitlab
|
||||||
|
|
||||||
Gitlab::GitalyClient.can_use_disk?(repo.storage)
|
Gitlab::GitalyClient.can_use_disk?(repo.storage)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def wrap_rugged_call(&block)
|
||||||
|
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
|
||||||
|
yield
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -387,21 +387,20 @@ module Gitlab
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.can_use_disk?(storage)
|
def self.can_use_disk?(storage)
|
||||||
false
|
cached_value = MUTEX.synchronize do
|
||||||
# cached_value = MUTEX.synchronize do
|
@can_use_disk ||= {}
|
||||||
# @can_use_disk ||= {}
|
@can_use_disk[storage]
|
||||||
# @can_use_disk[storage]
|
end
|
||||||
# end
|
|
||||||
|
|
||||||
# return cached_value unless cached_value.nil?
|
return cached_value if cached_value.present?
|
||||||
|
|
||||||
# gitaly_filesystem_id = filesystem_id(storage)
|
gitaly_filesystem_id = filesystem_id(storage)
|
||||||
# direct_filesystem_id = filesystem_id_from_disk(storage)
|
direct_filesystem_id = filesystem_id_from_disk(storage)
|
||||||
|
|
||||||
# MUTEX.synchronize do
|
MUTEX.synchronize do
|
||||||
# @can_use_disk[storage] = gitaly_filesystem_id.present? &&
|
@can_use_disk[storage] = gitaly_filesystem_id.present? &&
|
||||||
# gitaly_filesystem_id == direct_filesystem_id
|
gitaly_filesystem_id == direct_filesystem_id
|
||||||
# end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.filesystem_id(storage)
|
def self.filesystem_id(storage)
|
||||||
|
@ -414,7 +413,7 @@ module Gitlab
|
||||||
metadata_file = File.read(storage_metadata_file_path(storage))
|
metadata_file = File.read(storage_metadata_file_path(storage))
|
||||||
metadata_hash = JSON.parse(metadata_file)
|
metadata_hash = JSON.parse(metadata_file)
|
||||||
metadata_hash['gitaly_filesystem_id']
|
metadata_hash['gitaly_filesystem_id']
|
||||||
rescue Errno::ENOENT, JSON::ParserError
|
rescue Errno::ENOENT, Errno::ACCESS, JSON::ParserError
|
||||||
nil
|
nil
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -26,7 +26,6 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'loads 10 projects without hitting Gitaly call limit', :request_store do
|
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
|
projects = Gitlab::GitalyClient.allow_n_plus_1_calls do
|
||||||
(1..10).map { create(:project, :repository) }
|
(1..10).map { create(:project, :repository) }
|
||||||
end
|
end
|
||||||
|
|
|
@ -21,6 +21,7 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
|
||||||
end
|
end
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_call_original
|
||||||
Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {})
|
Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {})
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -30,7 +31,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns true when gitaly matches disk' do
|
it 'returns true when gitaly matches disk' do
|
||||||
pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338')
|
|
||||||
expect(subject.use_rugged?(repository, feature_flag_name)).to be true
|
expect(subject.use_rugged?(repository, feature_flag_name)).to be true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -49,7 +49,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
|
||||||
end
|
end
|
||||||
|
|
||||||
it "doesn't lead to a second rpc call because gitaly client should use the cached value" do
|
it "doesn't lead to a second rpc call because gitaly client should use the cached value" do
|
||||||
pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338')
|
|
||||||
expect(subject.use_rugged?(repository, feature_flag_name)).to be true
|
expect(subject.use_rugged?(repository, feature_flag_name)).to be true
|
||||||
|
|
||||||
expect(Gitlab::GitalyClient).not_to receive(:filesystem_id)
|
expect(Gitlab::GitalyClient).not_to receive(:filesystem_id)
|
||||||
|
|
|
@ -134,6 +134,8 @@ RSpec.configure do |config|
|
||||||
allow(Feature).to receive(:enabled?).with(flag).and_return(enabled)
|
allow(Feature).to receive(:enabled?).with(flag).and_return(enabled)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enabled)
|
||||||
|
|
||||||
# The following can be removed when we remove the staged rollout strategy
|
# The following can be removed when we remove the staged rollout strategy
|
||||||
# and we can just enable it using instance wide settings
|
# and we can just enable it using instance wide settings
|
||||||
# (ie. ApplicationSetting#auto_devops_enabled)
|
# (ie. ApplicationSetting#auto_devops_enabled)
|
||||||
|
|
Loading…
Reference in New Issue