From fe31f63ce82e79332df9cf853c01ce318b9f92d5 Mon Sep 17 00:00:00 2001 From: John Cai Date: Mon, 29 Apr 2019 07:21:36 +0000 Subject: Add gitaly session id & catfile-cache feature flag --- .../unreleased/jc-client-gitaly-session-id.yml | 5 +++ lib/gitlab/gitaly_client.rb | 8 ++++- spec/lib/gitlab/gitaly_client_spec.rb | 42 ++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/jc-client-gitaly-session-id.yml diff --git a/changelogs/unreleased/jc-client-gitaly-session-id.yml b/changelogs/unreleased/jc-client-gitaly-session-id.yml new file mode 100644 index 00000000000..ae5b7144b98 --- /dev/null +++ b/changelogs/unreleased/jc-client-gitaly-session-id.yml @@ -0,0 +1,5 @@ +--- +title: Add gitaly session id & catfile-cache feature flag +merge_request: 27472 +author: +type: performance diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c432317eb24..d34b50c5215 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -31,6 +31,9 @@ module Gitlab MAXIMUM_GITALY_CALLS = 30 CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze + SERVER_FEATURE_CATFILE_CACHE = 'catfile-cache'.freeze + SERVER_FEATURE_FLAGS = [SERVER_FEATURE_CATFILE_CACHE].freeze + MUTEX = Mutex.new define_histogram :gitaly_controller_action_duration_seconds do @@ -219,6 +222,7 @@ module Gitlab metadata['call_site'] = feature.to_s if feature metadata['gitaly-servers'] = address_metadata(remote_storage) if remote_storage metadata['x-gitlab-correlation-id'] = Labkit::Correlation::CorrelationId.current_id if Labkit::Correlation::CorrelationId.current_id + metadata['gitaly-session-id'] = session_id if feature_enabled?(SERVER_FEATURE_CATFILE_CACHE) metadata.merge!(server_feature_flags) @@ -235,7 +239,9 @@ module Gitlab result end - SERVER_FEATURE_FLAGS = %w[].freeze + def self.session_id + Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid + end def self.server_feature_flags SERVER_FEATURE_FLAGS.map do |f| diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index f1acb1d9bc4..da1eb0c2618 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -142,6 +142,48 @@ describe Gitlab::GitalyClient do end end + describe '.request_kwargs' do + context 'when catfile-cache feature is enabled' do + before do + stub_feature_flags('gitaly_catfile-cache': true) + end + + it 'sets the gitaly-session-id in the metadata' do + results = described_class.request_kwargs('default', nil) + expect(results[:metadata]).to include('gitaly-session-id') + end + + context 'when RequestStore is not enabled' do + it 'sets a different gitaly-session-id per request' do + gitaly_session_id = described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id'] + + expect(described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']).not_to eq(gitaly_session_id) + end + end + + context 'when RequestStore is enabled', :request_store do + it 'sets the same gitaly-session-id on every outgoing request metadata' do + gitaly_session_id = described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id'] + + 3.times do + expect(described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']).to eq(gitaly_session_id) + end + end + end + end + + context 'when catfile-cache feature is disabled' do + before do + stub_feature_flags({ 'gitaly_catfile-cache': false }) + end + + it 'does not set the gitaly-session-id in the metadata' do + results = described_class.request_kwargs('default', nil) + expect(results[:metadata]).not_to include('gitaly-session-id') + end + end + end + describe 'enforce_gitaly_request_limits?' do def call_gitaly(count = 1) (1..count).each do -- cgit v1.2.1