Commit f1ce70ab authored by Nick Thomas's avatar Nick Thomas

Merge branch 'disable-rugged-auto-detect-if-puma-threads-larger-than-1' into 'master'

Disable rugged auto-detect when Puma threads>1

See merge request gitlab-org/gitlab!22328
parents 99c61a33 622b8e62
...@@ -8,9 +8,17 @@ module Gitlab ...@@ -8,9 +8,17 @@ module Gitlab
feature = Feature.get(feature_key) feature = Feature.get(feature_key)
return feature.enabled? if Feature.persisted?(feature) return feature.enabled? if Feature.persisted?(feature)
# Disable Rugged auto-detect(can_use_disk?) when Puma threads>1
# https://gitlab.com/gitlab-org/gitlab/issues/119326
return false if running_puma_with_multiple_threads?
Gitlab::GitalyClient.can_use_disk?(repo.storage) Gitlab::GitalyClient.can_use_disk?(repo.storage)
end end
def running_puma_with_multiple_threads?
Gitlab::Runtime.puma? && ::Puma.cli_config.options[:max_threads] > 1
end
def execute_rugged_call(method_name, *args) def execute_rugged_call(method_name, *args)
Gitlab::GitalyClient::StorageSettings.allow_disk_access do Gitlab::GitalyClient::StorageSettings.allow_disk_access do
start = Gitlab::Metrics::System.monotonic_time start = Gitlab::Metrics::System.monotonic_time
......
...@@ -53,30 +53,46 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do ...@@ -53,30 +53,46 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
allow(Feature).to receive(:persisted?).with(feature_flag).and_return(false) allow(Feature).to receive(:persisted?).with(feature_flag).and_return(false)
end end
it 'returns true when gitaly matches disk' do context 'when running puma with multiple threads' do
expect(subject.use_rugged?(repository, feature_flag_name)).to be true before do
allow(subject).to receive(:running_puma_with_multiple_threads?).and_return(true)
end
it 'returns false' do
expect(subject.use_rugged?(repository, feature_flag_name)).to be false
end
end end
it 'returns false when disk access fails' do context 'when not running puma with multiple threads' do
allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return("/fake/path/doesnt/exist") before do
allow(subject).to receive(:running_puma_with_multiple_threads?).and_return(false)
end
expect(subject.use_rugged?(repository, feature_flag_name)).to be false it 'returns true when gitaly matches disk' do
end expect(subject.use_rugged?(repository, feature_flag_name)).to be true
end
it "returns false when gitaly doesn't match disk" do it 'returns false when disk access fails' do
allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file) 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_falsey expect(subject.use_rugged?(repository, feature_flag_name)).to be false
end
File.delete(temp_gitaly_metadata_file) it "returns false when gitaly doesn't match disk" do
end allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file)
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_falsey
expect(subject.use_rugged?(repository, feature_flag_name)).to be true
expect(Gitlab::GitalyClient).not_to receive(:filesystem_id) File.delete(temp_gitaly_metadata_file)
end
subject.use_rugged?(repository, feature_flag_name) 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 end
end end
...@@ -99,6 +115,37 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do ...@@ -99,6 +115,37 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
end end
end end
describe '#running_puma_with_multiple_threads?' do
context 'when using Puma' do
before do
stub_const('::Puma', class_double('Puma'))
allow(Gitlab::Runtime).to receive(:puma?).and_return(true)
end
it 'returns false for single thread Puma' do
allow(::Puma).to receive_message_chain(:cli_config, :options).and_return(max_threads: 1)
expect(subject.running_puma_with_multiple_threads?).to be false
end
it 'returns true for multi-threaded Puma' do
allow(::Puma).to receive_message_chain(:cli_config, :options).and_return(max_threads: 2)
expect(subject.running_puma_with_multiple_threads?).to be true
end
end
context 'when not using Puma' do
before do
allow(Gitlab::Runtime).to receive(:puma?).and_return(false)
end
it 'returns false' do
expect(subject.running_puma_with_multiple_threads?).to be false
end
end
end
def create_temporary_gitaly_metadata_file def create_temporary_gitaly_metadata_file
tmp = Tempfile.new('.gitaly-metadata') tmp = Tempfile.new('.gitaly-metadata')
gitaly_metadata = { gitaly_metadata = {
......
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