Use Rugged if we detect storage is NFS and we can access the disk
Add a module we use as a singleton to determine whether or not rails is able to access the disk
This commit is contained in:
parent
50441577f0
commit
8152e1aa4a
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -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
|
||||
|
||||
|
@ -387,6 +388,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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
Loading…
Reference in New Issue