Commit 8765d537 authored by John Cai's avatar John Cai

Wrap rugged calls with access disk block

Whenever we use the rugged implementation, we are going straight to disk
so we want to bypass the disk access check.
parent 3d9dc7db
...@@ -16,7 +16,7 @@ module Gitlab ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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
......
...@@ -388,21 +388,20 @@ module Gitlab ...@@ -388,21 +388,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)
...@@ -415,7 +414,7 @@ module Gitlab ...@@ -415,7 +414,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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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| ...@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment