summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorZeger-Jan van de Weg <git@zjvandeweg.nl>2018-11-16 10:53:44 +0100
committerZeger-Jan van de Weg <git@zjvandeweg.nl>2018-11-16 12:16:25 +0100
commit35239a6aec84c47895ad6664a9b1075be12bd105 (patch)
tree52395a00d1d09ad73d527e7e1bbb0832079f63e2 /lib
parente7a40cbe1637c7331e3397ed29a5e66ee7a1ade6 (diff)
downloadgitlab-ce-35239a6aec84c47895ad6664a9b1075be12bd105.tar.gz
Show what RPC is called in the performance bar
Now only the data was shown of the service, which is not valueable at times given some of those expose a lot of RPCs.
Diffstat (limited to 'lib')
-rw-r--r--lib/gitlab/git/blob.rb2
-rw-r--r--lib/gitlab/git/repository.rb6
-rw-r--r--lib/gitlab/gitaly_client.rb125
3 files changed, 10 insertions, 123 deletions
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index 9dd1c484d59..2d25389594e 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-# Gitaly note: JV: seems to be completely migrated (behind feature flags).
-
module Gitlab
module Git
class Blob
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 993955d1a6b..289796ae93b 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -885,12 +885,6 @@ module Gitlab
Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid)
end
- def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)
- wrapped_gitaly_errors do
- Gitlab::GitalyClient.migrate(method, status: status, &block)
- end
- end
-
def clean_stale_repository_files
wrapped_gitaly_errors do
gitaly_repository_client.cleanup if exists?
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index d99a9f15371..613f0d31075 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -9,11 +9,6 @@ require 'grpc/health/v1/health_services_pb'
module Gitlab
module GitalyClient
include Gitlab::Metrics::Methods
- module MigrationStatus
- DISABLED = 1
- OPT_IN = 2
- OPT_OUT = 3
- end
class TooManyInvocationsError < StandardError
attr_reader :call_site, :invocation_count, :max_call_stack
@@ -31,7 +26,7 @@ module Gitlab
end
end
- SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze
+ SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'
MAXIMUM_GITALY_CALLS = 35
CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
@@ -43,11 +38,6 @@ module Gitlab
self.query_time = 0
- define_histogram :gitaly_migrate_call_duration_seconds do
- docstring "Gitaly migration call execution timings"
- base_labels gitaly_enabled: nil, feature: nil
- end
-
define_histogram :gitaly_controller_action_duration_seconds do
docstring "Gitaly endpoint histogram by controller and action combination"
base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil)
@@ -126,7 +116,6 @@ module Gitlab
def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil)
start = Gitlab::Metrics::System.monotonic_time
request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {}
- @current_call_id ||= SecureRandom.uuid
enforce_gitaly_request_limits(:call)
@@ -145,9 +134,7 @@ module Gitlab
current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s),
duration)
- add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash)
-
- @current_call_id = nil
+ add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc)
end
def self.handle_grpc_unavailable!(ex)
@@ -222,7 +209,7 @@ module Gitlab
result
end
- SERVER_FEATURE_FLAGS = %w[gogit_findcommit].freeze
+ SERVER_FEATURE_FLAGS = %w[].freeze
def self.server_feature_flags
SERVER_FEATURE_FLAGS.map do |f|
@@ -237,82 +224,8 @@ module Gitlab
params['gitaly_token'].presence || Gitlab.config.gitaly['token']
end
- # Evaluates whether a feature toggle is on or off
- def self.feature_enabled?(feature_name, status: MigrationStatus::OPT_IN)
- # Disabled features are always off!
- return false if status == MigrationStatus::DISABLED
-
- feature = Feature.get("gitaly_#{feature_name}")
-
- # If the feature has been set, always evaluate
- if Feature.persisted?(feature)
- if feature.percentage_of_time_value > 0
- # Probabilistically enable this feature
- return Random.rand() * 100 < feature.percentage_of_time_value
- end
-
- return feature.enabled?
- end
-
- # If the feature has not been set, the default depends
- # on it's status
- case status
- when MigrationStatus::OPT_OUT
- true
- when MigrationStatus::OPT_IN
- opt_into_all_features? && !explicit_opt_in_required.include?(feature_name)
- else
- false
- end
- rescue => ex
- # During application startup feature lookups in SQL can fail
- Rails.logger.warn "exception while checking Gitaly feature status for #{feature_name}: #{ex}"
- false
- end
-
- # We have a mechanism to let GitLab automatically opt in to all Gitaly
- # features. We want to be able to exclude some features from automatic
- # opt-in. This function has an override in EE.
- def self.explicit_opt_in_required
- []
- end
-
- # opt_into_all_features? returns true when the current environment
- # is one in which we opt into features automatically
- def self.opt_into_all_features?
- Rails.env.development? || ENV["GITALY_FEATURE_DEFAULT_ON"] == "1"
- end
- private_class_method :opt_into_all_features?
-
- def self.migrate(feature, status: MigrationStatus::OPT_IN)
- # Enforce limits at both the `migrate` and `call` sites to ensure that
- # problems are not hidden by a feature being disabled
- enforce_gitaly_request_limits(:migrate)
-
- is_enabled = feature_enabled?(feature, status: status)
- metric_name = feature.to_s
- metric_name += "_gitaly" if is_enabled
-
- Gitlab::Metrics.measure(metric_name) do
- # Some migrate calls wrap other migrate calls
- allow_n_plus_1_calls do
- feature_stack = Thread.current[:gitaly_feature_stack] ||= []
- feature_stack.unshift(feature)
- begin
- start = Gitlab::Metrics::System.monotonic_time
- @current_call_id = SecureRandom.uuid
- call_details = { id: @current_call_id }
- yield is_enabled
- ensure
- total_time = Gitlab::Metrics::System.monotonic_time - start
- gitaly_migrate_call_duration_seconds.observe({ gitaly_enabled: is_enabled, feature: feature }, total_time)
- feature_stack.shift
- Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty?
-
- add_call_details(call_details.merge(feature: feature, duration: total_time))
- end
- end
- end
+ def self.feature_enabled?(feature_name)
+ Feature.enabled?("gitaly_#{feature_name}")
end
# Ensures that Gitaly is not being abuse through n+1 misuse etc
@@ -368,38 +281,20 @@ module Gitlab
end
private_class_method :decrement_call_count
- # Returns an estimate of the number of Gitaly calls made for this
- # request
+ # Returns the of the number of Gitaly calls made for this request
def self.get_request_count
- return 0 unless Gitlab::SafeRequestStore.active?
-
- gitaly_migrate_count = get_call_count("gitaly_migrate_actual")
- gitaly_call_count = get_call_count("gitaly_call_actual")
-
- # Using the maximum of migrate and call_count will provide an
- # indicator of how many Gitaly calls will be made, even
- # before a feature is enabled. This provides us with a single
- # metric, but not an exact number, but this tradeoff is acceptable
- if gitaly_migrate_count > gitaly_call_count
- gitaly_migrate_count
- else
- gitaly_call_count
- end
+ get_call_count("gitaly_call_actual")
end
def self.reset_counts
return unless Gitlab::SafeRequestStore.active?
- %w[migrate call].each do |call_site|
- Gitlab::SafeRequestStore["gitaly_#{call_site}_actual"] = 0
- Gitlab::SafeRequestStore["gitaly_#{call_site}_permitted"] = 0
- end
+ Gitlab::SafeRequestStore["gitaly_call_actual"] = 0
+ Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0
end
def self.add_call_details(details)
- id = details.delete(:id)
-
- return unless id && Gitlab::SafeRequestStore[:peek_enabled]
+ return unless Gitlab::SafeRequestStore[:peek_enabled]
Gitlab::SafeRequestStore['gitaly_call_details'] ||= {}
Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {}