Add timeouts for Gitaly calls

5 files changed, 65 insertions, 23 deletions

diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 06373fe5069..cee4d309816 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -123,6 +123,9 @@ module API
       end
       optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
       optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.'
+      optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.'
+      optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.'
+      optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.'
 
       ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
         optional :"#{type}_key_restriction",
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index d7375938ab6..f27cd800bdd 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -117,11 +117,11 @@ module Gitlab
     #   kwargs.merge(deadline: Time.now + 10)
     # end
     #
-    def self.call(storage, service, rpc, request, remote_storage: nil)
+    def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil)
       start = Gitlab::Metrics::System.monotonic_time
       enforce_gitaly_request_limits(:call)
 
-      kwargs = request_kwargs(storage, remote_storage: remote_storage)
+      kwargs = request_kwargs(storage, timeout, remote_storage: remote_storage)
       kwargs = yield(kwargs) if block_given?
 
       stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend
@@ -140,7 +140,7 @@ module Gitlab
     end
     private_class_method :current_transaction_labels
 
-    def self.request_kwargs(storage, remote_storage: nil)
+    def self.request_kwargs(storage, timeout, remote_storage: nil)
       encoded_token = Base64.strict_encode64(token(storage).to_s)
       metadata = {
         'authorization' => "Bearer #{encoded_token}",
@@ -152,7 +152,22 @@ module Gitlab
       metadata['call_site'] = feature.to_s if feature
       metadata['gitaly-servers'] = address_metadata(remote_storage) if remote_storage
 
-      { metadata: metadata }
+      result = { metadata: metadata }
+
+      # nil timeout indicates that we should use the default
+      timeout = default_timeout if timeout.nil?
+
+      return result unless timeout > 0
+
+      # Do not use `Time.now` for deadline calculation, since it
+      # will be affected by Timecop in some tests, but grpc's c-core
+      # uses system time instead of timecop's time, so tests will fail
+      # `Time.at(Process.clock_gettime(Process::CLOCK_REALTIME))` will
+      # circumvent timecop
+      deadline = Time.at(Process.clock_gettime(Process::CLOCK_REALTIME)) + timeout
+      result[:deadline] = deadline
+
+      result
     end
 
     def self.token(storage)
@@ -325,6 +340,26 @@ module Gitlab
       Google::Protobuf::RepeatedField.new(:bytes, a.map { |s| self.encode(s) } )
     end
 
+    # The default timeout on all Gitaly calls
+    def self.default_timeout
+      return 0 if Sidekiq.server?
+
+      timeout(:gitaly_timeout_default)
+    end
+
+    def self.fast_timeout
+      timeout(:gitaly_timeout_fast)
+    end
+
+    def self.medium_timeout
+      timeout(:gitaly_timeout_medium)
+    end
+
+    def self.timeout(timeout_name)
+      Gitlab::CurrentSettings.current_application_settings[timeout_name]
+    end
+    private_class_method :timeout
+
     # Count a stack. Used for n+1 detection
     def self.count_stack
       return unless RequestStore.active?
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index da5505cb2fe..34807d280e5 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -16,7 +16,7 @@ module Gitlab
           revision: GitalyClient.encode(revision)
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :list_files, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :list_files, request, timeout: GitalyClient.medium_timeout)
         response.flat_map do |msg|
           msg.paths.map { |d| EncodingHelper.encode!(d.dup) }
         end
@@ -29,7 +29,7 @@ module Gitlab
           child_id: child_id
         )
 
-        GitalyClient.call(@repository.storage, :commit_service, :commit_is_ancestor, request).value
+        GitalyClient.call(@repository.storage, :commit_service, :commit_is_ancestor, request, timeout: GitalyClient.fast_timeout).value
       end
 
       def diff(from, to, options = {})
@@ -77,7 +77,7 @@ module Gitlab
           limit: limit.to_i
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :tree_entry, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :tree_entry, request, timeout: GitalyClient.medium_timeout)
 
         entry = nil
         data = ''
@@ -102,7 +102,7 @@ module Gitlab
           path: path.present? ? GitalyClient.encode(path) : '.'
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :get_tree_entries, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :get_tree_entries, request, timeout: GitalyClient.medium_timeout)
 
         response.flat_map do |message|
           message.entries.map do |gitaly_tree_entry|
@@ -129,7 +129,7 @@ module Gitlab
         request.before = Google::Protobuf::Timestamp.new(seconds: options[:before].to_i) if options[:before].present?
         request.path = options[:path] if options[:path].present?
 
-        GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count
+        GitalyClient.call(@repository.storage, :commit_service, :count_commits, request, timeout: GitalyClient.medium_timeout).count
       end
 
       def last_commit_for_path(revision, path)
@@ -139,7 +139,7 @@ module Gitlab
           path: GitalyClient.encode(path.to_s)
         )
 
-        gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit
+        gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request, timeout: GitalyClient.fast_timeout).commit
         return unless gitaly_commit
 
         Gitlab::Git::Commit.new(@repository, gitaly_commit)
@@ -152,7 +152,7 @@ module Gitlab
           to: to
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :commits_between, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :commits_between, request, timeout: GitalyClient.medium_timeout)
         consume_commits_response(response)
       end
 
@@ -165,7 +165,7 @@ module Gitlab
         )
         request.order = opts[:order].upcase if opts[:order].present?
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :find_all_commits, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :find_all_commits, request, timeout: GitalyClient.medium_timeout)
         consume_commits_response(response)
       end
 
@@ -179,7 +179,7 @@ module Gitlab
           offset: offset.to_i
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :commits_by_message, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :commits_by_message, request, timeout: GitalyClient.medium_timeout)
         consume_commits_response(response)
       end
 
@@ -197,7 +197,7 @@ module Gitlab
           path: GitalyClient.encode(path)
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request, timeout: GitalyClient.medium_timeout)
         response.reduce("") { |memo, msg| memo << msg.data }
       end
 
@@ -207,7 +207,7 @@ module Gitlab
           revision: GitalyClient.encode(revision)
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request, timeout: GitalyClient.medium_timeout)
 
         response.commit
       end
@@ -217,7 +217,7 @@ module Gitlab
           repository: @gitaly_repo,
           revision: GitalyClient.encode(revision)
         )
-        response = GitalyClient.call(@repository.storage, :diff_service, :commit_patch, request)
+        response = GitalyClient.call(@repository.storage, :diff_service, :commit_patch, request, timeout: GitalyClient.medium_timeout)
 
         response.sum(&:data)
       end
@@ -227,7 +227,7 @@ module Gitlab
           repository: @gitaly_repo,
           revision: GitalyClient.encode(revision)
         )
-        GitalyClient.call(@repository.storage, :commit_service, :commit_stats, request)
+        GitalyClient.call(@repository.storage, :commit_service, :commit_stats, request, timeout: GitalyClient.medium_timeout)
       end
 
       def find_commits(options)
@@ -245,7 +245,7 @@ module Gitlab
 
         request.paths = GitalyClient.encode_repeated(Array(options[:path])) if options[:path].present?
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :find_commits, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :find_commits, request, timeout: GitalyClient.medium_timeout)
 
         consume_commits_response(response)
       end
@@ -259,7 +259,7 @@ module Gitlab
         request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h)
 
         request = Gitaly::CommitDiffRequest.new(request_params)
-        response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request)
+        response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request, timeout: GitalyClient.medium_timeout)
         GitalyClient::DiffStitcher.new(response)
       end
 
diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb
index 31b04bc2650..066e4e183c0 100644
--- a/lib/gitlab/gitaly_client/ref_service.rb
+++ b/lib/gitlab/gitaly_client/ref_service.rb
@@ -46,7 +46,8 @@ module Gitlab
           commit_id: commit_id,
           prefix: ref_prefix
         )
-        encode!(GitalyClient.call(@storage, :ref_service, :find_ref_name, request).name.dup)
+        response = GitalyClient.call(@storage, :ref_service, :find_ref_name, request, timeout: GitalyClient.medium_timeout)
+        encode!(response.name.dup)
       end
 
       def count_tag_names
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 70cb16bd810..b9e606592d7 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -10,7 +10,9 @@ module Gitlab
       def exists?
         request = Gitaly::RepositoryExistsRequest.new(repository: @gitaly_repo)
 
-        GitalyClient.call(@storage, :repository_service, :repository_exists, request).exists
+        response = GitalyClient.call(@storage, :repository_service, :repository_exists, request, timeout: GitalyClient.fast_timeout)
+
+        response.exists
       end
 
       def garbage_collect(create_bitmap)
@@ -30,7 +32,8 @@ module Gitlab
 
       def repository_size
         request = Gitaly::RepositorySizeRequest.new(repository: @gitaly_repo)
-        GitalyClient.call(@storage, :repository_service, :repository_size, request).size
+        response = GitalyClient.call(@storage, :repository_service, :repository_size, request)
+        response.size
       end
 
       def apply_gitattributes(revision)
@@ -61,7 +64,7 @@ module Gitlab
 
       def has_local_branches?
         request = Gitaly::HasLocalBranchesRequest.new(repository: @gitaly_repo)
-        response = GitalyClient.call(@storage, :repository_service, :has_local_branches, request)
+        response = GitalyClient.call(@storage, :repository_service, :has_local_branches, request, timeout: GitalyClient.fast_timeout)
 
         response.value
       end