Merge branch '224526-es-client-timeout' into 'master'

Add admin setting of Elasticsearch client request timeout

Closes #224526

See merge request gitlab-org/gitlab!41470

changelogs/unreleased/224526-es-client-timeout.yml

+---
+title: Add admin setting of Elasticsearch client request timeout
+merge_request: 41470
+author:
+type: added

db/migrate/20200903054946_add_elasticsearch_client_timeout.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddElasticsearchClientTimeout < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :elasticsearch_client_request_timeout, :integer, null: false, default: 0
+  end
+end

db/schema_migrations/20200903054946

+f9aa112661a55c9eeed1a6aa05dd4c28d6f88971dc14bb606e677d4b4a4e5947
\ No newline at end of file

db/structure.sql

@@ -9271,6 +9271,7 @@ CREATE TABLE public.application_settings (
     elasticsearch_indexed_file_size_limit_kb integer DEFAULT 1024 NOT NULL,
     enforce_namespace_storage_limit boolean DEFAULT false NOT NULL,
     container_registry_delete_tags_service_timeout integer DEFAULT 250 NOT NULL,
+    elasticsearch_client_request_timeout integer DEFAULT 0 NOT NULL,
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)),
     CONSTRAINT check_d03919528d CHECK ((char_length(container_registry_vendor) <= 255)),

doc/integration/elasticsearch.md

@@ -227,6 +227,7 @@ The following Elasticsearch settings are available:
 | `Maximum field length`                                | See [the explanation in instance limits.](../administration/instance_limits.md#maximum-field-length). |
 | `Maximum bulk request size (MiB)` | The Maximum Bulk Request size is used by GitLab's Golang-based indexer processes and indicates how much data it ought to collect (and store in memory) in a given indexing process before submitting the payload to Elasticsearch’s Bulk API. This setting should be used with the Bulk request concurrency setting (see below) and needs to accommodate the resource constraints of both the Elasticsearch host(s) and the host(s) running GitLab's Golang-based indexer either from the `gitlab-rake` command or the Sidekiq tasks. |
 | `Bulk request concurrency`                            | The Bulk request concurrency indicates how many of GitLab's Golang-based indexer processes (or threads) can run in parallel to collect data to subsequently submit to Elasticsearch’s Bulk API. This increases indexing performance, but fills the Elasticsearch bulk requests queue faster. This setting should be used together with the Maximum bulk request size setting (see above) and needs to accommodate the resource constraints of both the Elasticsearch host(s) and the host(s) running GitLab's Golang-based indexer either from the `gitlab-rake` command or the Sidekiq tasks. |
+| `Client request timeout` | Elasticsearch HTTP client request timeout value in seconds. `0` means using the system default timeout value, which depends on the libraries that GitLab application is built upon. |
 
 ### Limiting namespaces and projects
 

ee/app/helpers/ee/application_settings_helper.rb

@@ -38,6 +38,7 @@ module EE
         :elasticsearch_limit_indexing,
         :elasticsearch_namespace_ids,
         :elasticsearch_project_ids,
+        :elasticsearch_client_request_timeout,
         :enforce_namespace_storage_limit,
         :geo_status_timeout,
         :geo_node_allowed_ips,

ee/app/models/ee/application_setting.rb

@@ -74,6 +74,10 @@ module EE
                 presence: true,
                 numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
+      validates :elasticsearch_client_request_timeout,
+                presence: true,
+                numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+
       validates :email_additional_text,
                 allow_blank: true,
                 length: { maximum: EMAIL_ADDITIONAL_TEXT_CHARACTER_LIMIT }
@@ -113,6 +117,7 @@ module EE
           elasticsearch_replicas: 1,
           elasticsearch_shards: 5,
           elasticsearch_url: ENV['ELASTIC_URL'] || 'http://localhost:9200',
+          elasticsearch_client_request_timeout: 0,
           email_additional_text: nil,
           enforce_namespace_storage_limit: false,
           enforce_pat_expiration: true,
@@ -260,14 +265,15 @@ module EE
 
     def elasticsearch_config
       {
-        url:                   elasticsearch_url,
-        aws:                   elasticsearch_aws,
-        aws_access_key:        elasticsearch_aws_access_key,
-        aws_secret_access_key: elasticsearch_aws_secret_access_key,
-        aws_region:            elasticsearch_aws_region,
-        max_bulk_size_bytes:   elasticsearch_max_bulk_size_mb.megabytes,
-        max_bulk_concurrency:  elasticsearch_max_bulk_concurrency
-      }
+        url:                    elasticsearch_url,
+        aws:                    elasticsearch_aws,
+        aws_access_key:         elasticsearch_aws_access_key,
+        aws_secret_access_key:  elasticsearch_aws_secret_access_key,
+        aws_region:             elasticsearch_aws_region,
+        max_bulk_size_bytes:    elasticsearch_max_bulk_size_mb.megabytes,
+        max_bulk_concurrency:   elasticsearch_max_bulk_concurrency,
+        client_request_timeout: (elasticsearch_client_request_timeout if elasticsearch_client_request_timeout > 0)
+      }.compact
     end
 
     def email_additional_text

ee/app/views/admin/application_settings/_elasticsearch_form.html.haml

@@ -93,6 +93,13 @@
               = _('Maximum concurrency of Elasticsearch bulk requests per indexing operation.')
               = _('This only applies to repository indexing operations.')
 
+          .form-group
+            = f.label :elasticsearch_client_request_timeout, _('Client request timeout'), class: 'label-bold'
+            = f.number_field :elasticsearch_client_request_timeout, value: @application_setting.elasticsearch_client_request_timeout, class: 'form-control'
+            .form-text.text-muted
+              = _('Elasticsearch HTTP client timeout value in seconds.')
+              = _('Setting this to 0 means using the system default timeout value.')
+
         .sub-section
           %h4= _('Elasticsearch zero-downtime reindexing')
           = link_to _('Trigger cluster reindexing'), admin_elasticsearch_trigger_reindexing_path, class: 'btn btn-success', method: :post, disabled: @elasticsearch_reindexing_task&.in_progress?

ee/lib/gitlab/elastic/client.rb

@@ -12,9 +12,10 @@ module Gitlab
       def self.build(config)
         base_config = {
           urls: config[:url],
+          request_timeout: config[:client_request_timeout],
           randomize_hosts: true,
           retry_on_failure: true
-        }
+        }.compact
 
         if config[:aws]
           creds = resolve_aws_credentials(config)

ee/spec/features/admin/admin_settings_spec.rb

@@ -68,6 +68,7 @@ RSpec.describe 'Admin updates EE-only settings' do
         fill_in 'Maximum field length', with: '100000'
         fill_in 'Maximum bulk request size (MiB)', with: '17'
         fill_in 'Bulk request concurrency', with: '23'
+        fill_in 'Client request timeout', with: '30'
 
         click_button 'Save changes'
       end
@@ -81,6 +82,7 @@ RSpec.describe 'Admin updates EE-only settings' do
         expect(current_settings.elasticsearch_indexed_field_length_limit).to eq(100000)
         expect(current_settings.elasticsearch_max_bulk_size_mb).to eq(17)
         expect(current_settings.elasticsearch_max_bulk_concurrency).to eq(23)
+        expect(current_settings.elasticsearch_client_request_timeout).to eq(30)
         expect(page).to have_content 'Application settings saved successfully'
       end
     end

ee/spec/lib/gitlab/elastic/client_spec.rb

@@ -16,6 +16,18 @@ RSpec.describe Gitlab::Elastic::Client do
 
         expect(client.get(index: 'foo', id: 1)).to eq([:fake_response])
       end
+
+      it 'does not set request timeout in transport' do
+        expect(client.transport.options).not_to include(:request_timeout)
+      end
+
+      context 'with client_request_timeout in config' do
+        let(:params) { { url: 'http://dummy-elastic:9200', client_request_timeout: 30 } }
+
+        it 'does not set request timeout in transport' do
+          expect(client.transport.options).to include(request_timeout: 30)
+        end
+      end
     end
 
     context 'with AWS IAM static credentials' do

ee/spec/models/application_setting_spec.rb

@@ -65,6 +65,12 @@ RSpec.describe ApplicationSetting do
     it { is_expected.not_to allow_value(1.1).for(:elasticsearch_max_bulk_concurrency) }
     it { is_expected.not_to allow_value(-1).for(:elasticsearch_max_bulk_concurrency) }
 
+    it { is_expected.to allow_value(30).for(:elasticsearch_client_request_timeout) }
+    it { is_expected.to allow_value(0).for(:elasticsearch_client_request_timeout) }
+    it { is_expected.not_to allow_value(nil).for(:elasticsearch_client_request_timeout) }
+    it { is_expected.not_to allow_value(1.1).for(:elasticsearch_client_request_timeout) }
+    it { is_expected.not_to allow_value(-1).for(:elasticsearch_client_request_timeout) }
+
     it { is_expected.to allow_value(nil).for(:required_instance_ci_template) }
     it { is_expected.not_to allow_value("").for(:required_instance_ci_template) }
     it { is_expected.not_to allow_value("  ").for(:required_instance_ci_template) }
@@ -288,7 +294,8 @@ RSpec.describe ApplicationSetting do
         elasticsearch_aws_access_key: 'test-access-key',
         elasticsearch_aws_secret_access_key: 'test-secret-access-key',
         elasticsearch_max_bulk_size_mb: 67,
-        elasticsearch_max_bulk_concurrency: 8
+        elasticsearch_max_bulk_concurrency: 8,
+        elasticsearch_client_request_timeout: 30
       )
 
       expect(setting.elasticsearch_config).to eq(
@@ -298,8 +305,15 @@ RSpec.describe ApplicationSetting do
         aws_access_key: 'test-access-key',
         aws_secret_access_key: 'test-secret-access-key',
         max_bulk_size_bytes: 67.megabytes,
-        max_bulk_concurrency: 8
+        max_bulk_concurrency: 8,
+        client_request_timeout: 30
       )
+
+      setting.update!(
+        elasticsearch_client_request_timeout: 0
+      )
+
+      expect(setting.elasticsearch_config).not_to include(:client_request_timeout)
     end
 
     context 'limiting namespaces and projects' do

locale/gitlab.pot

@@ -5167,6 +5167,9 @@ msgstr ""
 msgid "Client authentication key password"
 msgstr ""
 
+msgid "Client request timeout"
+msgstr ""
+
 msgid "Clients"
 msgstr ""
 
@@ -9249,6 +9252,9 @@ msgstr ""
 msgid "Elasticsearch AWS IAM credentials"
 msgstr ""
 
+msgid "Elasticsearch HTTP client timeout value in seconds."
+msgstr ""
+
 msgid "Elasticsearch indexing restrictions"
 msgstr ""
 
@@ -23045,6 +23051,9 @@ msgstr ""
 msgid "Sets weight to %{weight}."
 msgstr ""
 
+msgid "Setting this to 0 means using the system default timeout value."
+msgstr ""
+
 msgid "Settings"
 msgstr ""