Merge branch '296882-ui-for-es-shard-replica-settings' into 'master'

Allow setting the shard/replica separately for ES indexes (step 2)

See merge request gitlab-org/gitlab!57567

ee/app/controllers/ee/admin/application_settings_controller.rb

@@ -8,6 +8,7 @@ module EE
 
       prepended do
         before_action :elasticsearch_reindexing_task, only: [:advanced_search]
+        before_action :elasticsearch_index_settings, only: [:advanced_search]
 
         feature_category :license, [:seat_link_payload]
         feature_category :source_code_management, [:templates]
@@ -16,6 +17,10 @@ module EE
         def elasticsearch_reindexing_task
           @elasticsearch_reindexing_task = Elastic::ReindexingTask.last
         end
+
+        def elasticsearch_index_settings
+          @elasticsearch_index_settings = Elastic::IndexSetting.order_by_name
+        end
       end
 
       EE_VALID_SETTING_PANELS = %w(advanced_search templates).freeze
@@ -57,6 +62,13 @@ module EE
           attrs += EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes
         end
 
+        if License.feature_available?(:elastic_search)
+          attrs += [
+            elasticsearch_shards: {},
+            elasticsearch_replicas: {}
+          ]
+        end
+
         if ::Gitlab::Geo.license_allows?
           attrs << :maintenance_mode
           attrs << :maintenance_mode_message

ee/app/models/ee/application_setting.rb

@@ -10,6 +10,10 @@ module EE
     extend ::Gitlab::Utils::Override
 
     prepended do
+      include IgnorableColumns
+
+      ignore_columns %i[elasticsearch_shards elasticsearch_replicas], remove_with: '14.1', remove_after: '2021-06-22'
+
       EMAIL_ADDITIONAL_TEXT_CHARACTER_LIMIT = 10_000
       DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL = 7
 

ee/app/models/elastic/index_setting.rb

@@ -8,6 +8,8 @@ module Elastic
     validates :number_of_replicas, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
     validates :number_of_shards, presence: true, numericality: { only_integer: true, greater_than: 0 }
 
+    scope :order_by_name, -> { order(:alias_name) }
+
     class << self
       def [](alias_name)
         safe_find_or_create_by(alias_name: alias_name)

ee/app/services/ee/application_settings/update_service.rb

@@ -51,11 +51,17 @@ module EE
       def update_elasticsearch_index_settings(number_of_replicas:, number_of_shards:)
         return if number_of_replicas.nil? && number_of_shards.nil?
 
-        Elastic::IndexSetting.default.update!(
-          number_of_replicas: number_of_replicas.to_i,
-          number_of_shards: number_of_shards.to_i
+        number_of_shards.to_h.each do |index_name, shards|
+          replicas = number_of_replicas[index_name]
+
+          next if shards.blank? || replicas.blank?
+
+          Elastic::IndexSetting[index_name].update!(
+            number_of_replicas: replicas.to_i,
+            number_of_shards: shards.to_i
           )
         end
+      end
 
       private
 

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

 - return unless License.feature_available?(:elastic_search)
 
-- recreate_index_url = help_page_url('integration/elasticsearch.md')
-- recreate_index_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recreate_index_url }
-- recreate_index_text = _("Changes won't take place until the index is %{link_start}recreated%{link_end}.").html_safe % { link_start: recreate_index_link_start, link_end: '</a>'.html_safe }
-- elasticsearch_available = Gitlab::Elastic::Helper.default.ping?
+- elastic_helper = Gitlab::Elastic::Helper.default
+- elasticsearch_available = elastic_helper.ping?
 
 %section.settings.expanded.as-elasticsearch.no-animate#js-elasticsearch-settings{ data: { qa_selector: 'elasticsearch_tab' } }
   .settings-header
@@ -73,18 +71,31 @@
                 = _('The URL to use for connecting to Elasticsearch. Use a comma-separated list to support clustering (e.g., "http://localhost:9200, http://localhost:9201").')
 
             .form-group
-              = f.label :elasticsearch_shards, _('Number of Elasticsearch shards'), class: 'label-bold'
-              = f.number_field :elasticsearch_shards, value: Elastic::IndexSetting.number_of_shards, class: 'form-control gl-form-input'
-              .form-text.gl-text-gray-600.gl-mt-0
-                = _('How many shards to split the Elasticsearch index over.')
-                = recreate_index_text
-
+              = f.label :elasticsearch_shards, _('Number of Elasticsearch shards and replicas (per index)'), class: 'gl-font-weight-bold'
+
+            %table.table
+              %thead
+                %tr
+                  %th= _('Index')
+                  %th= _('Number of shards')
+                  %th= _('Number of replicas')
+              %tbody
+                - aliases = [elastic_helper.target_name] + elastic_helper.standalone_indices_proxies.map(&:index_name)
+                - aliases.each do |alias_name|
+                  %tr
+                    - setting = Elastic::IndexSetting[alias_name]
+                    %td= alias_name
+                    %td
+                      = f.number_field :elasticsearch_shards, name: "application_setting[elasticsearch_shards][#{setting.alias_name}]", value: setting.number_of_shards, class: 'form-control gl-form-input', id: "application_setting_elasticsearch_shards[#{setting.alias_name}]"
+                    %td
+                      = f.number_field :elasticsearch_replicas, name: "application_setting[elasticsearch_replicas][#{setting.alias_name}]", value: setting.number_of_replicas, class: 'form-control gl-form-input', id: "application_setting_elasticsearch_replicas[#{setting.alias_name}]"
             .form-group
-              = f.label :elasticsearch_replicas, _('Number of Elasticsearch replicas'), class: 'label-bold'
-              = f.number_field :elasticsearch_replicas, value: Elastic::IndexSetting.number_of_replicas, class: 'form-control gl-form-input'
               .form-text.gl-text-gray-600.gl-mt-0
-                = _('How many replicas each Elasticsearch shard has.')
-                = recreate_index_text
+                - configuration_link_url = help_page_url('integration/elasticsearch.md', anchor: 'advanced-search-configuration')
+                - configuration_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: configuration_link_url }
+                - recreated_link_url = help_page_url('integration/elasticsearch.md', anchor: 'zero-downtime-reindexing')
+                - recreated_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recreated_link_url }
+                = _("Learn more about shards and replicas in the %{configuration_link_start}Advanced search configuration%{configuration_link_end} documentation. Changes won't take place until the index is %{recreated_link_start}recreated%{recreated_link_end}.").html_safe % { configuration_link_start: configuration_link_start, configuration_link_end: '</a>'.html_safe, recreated_link_start: recreated_link_start, recreated_link_end: '</a>'.html_safe }
 
             .form-group
               = f.label :elasticsearch_indexed_file_size_limit_kb, _('Maximum file size indexed (KiB)'), class: 'label-bold'

ee/changelogs/unreleased/296882-ui-for-es-shard-replica-settings.yml

+---
+title: 'Advanced Search: allow standalone shard/replica settings'
+merge_request: 57567
+author:
+type: changed

ee/lib/elastic/latest/issue_config.rb

@@ -10,7 +10,12 @@ module Elastic
       self.document_type = 'doc'
       self.index_name = [Rails.application.class.module_parent_name.downcase, Rails.env, 'issues'].join('-')
 
-      settings Elastic::Latest::Config.settings.to_hash
+      settings Elastic::Latest::Config.settings.to_hash.deep_merge(
+        index: {
+          number_of_shards: Elastic::AsJSON.new { Elastic::IndexSetting[self.index_name].number_of_shards },
+          number_of_replicas: Elastic::AsJSON.new { Elastic::IndexSetting[self.index_name].number_of_replicas }
+        }
+      )
 
       mappings dynamic: 'strict' do
         indexes :type, type: :keyword

ee/lib/elastic/latest/note_config.rb

@@ -10,7 +10,12 @@ module Elastic
       self.document_type = 'doc'
       self.index_name = [Rails.application.class.module_parent_name.downcase, Rails.env, 'notes'].join('-')
 
-      settings Elastic::Latest::Config.settings.to_hash
+      settings Elastic::Latest::Config.settings.to_hash.deep_merge(
+        index: {
+          number_of_shards: Elastic::AsJSON.new { Elastic::IndexSetting[self.index_name].number_of_shards },
+          number_of_replicas: Elastic::AsJSON.new { Elastic::IndexSetting[self.index_name].number_of_replicas }
+        }
+      )
 
       mappings dynamic: 'strict' do
         indexes :type, type: :keyword

ee/spec/features/admin/admin_settings_spec.rb

@@ -64,8 +64,14 @@ RSpec.describe 'Admin updates EE-only settings' do
       page.within('.as-elasticsearch') do
         check 'Elasticsearch indexing'
         check 'Search with Elasticsearch enabled'
-        fill_in 'Number of Elasticsearch shards', with: '120'
-        fill_in 'Number of Elasticsearch replicas', with: '2'
+
+        fill_in 'application_setting_elasticsearch_shards[gitlab-test]', with: '120'
+        fill_in 'application_setting_elasticsearch_replicas[gitlab-test]', with: '2'
+        fill_in 'application_setting_elasticsearch_shards[gitlab-test-issues]', with: '10'
+        fill_in 'application_setting_elasticsearch_replicas[gitlab-test-issues]', with: '3'
+        fill_in 'application_setting_elasticsearch_shards[gitlab-test-notes]', with: '20'
+        fill_in 'application_setting_elasticsearch_replicas[gitlab-test-notes]', with: '4'
+
         fill_in 'Maximum file size indexed (KiB)', with: '5000'
         fill_in 'Maximum field length', with: '100000'
         fill_in 'Maximum bulk request size (MiB)', with: '17'
@@ -78,8 +84,16 @@ RSpec.describe 'Admin updates EE-only settings' do
       aggregate_failures do
         expect(current_settings.elasticsearch_indexing).to be_truthy
         expect(current_settings.elasticsearch_search).to be_truthy
+
         expect(current_settings.elasticsearch_shards).to eq(120)
         expect(current_settings.elasticsearch_replicas).to eq(2)
+        expect(Elastic::IndexSetting['gitlab-test'].number_of_shards).to eq(120)
+        expect(Elastic::IndexSetting['gitlab-test'].number_of_replicas).to eq(2)
+        expect(Elastic::IndexSetting['gitlab-test-issues'].number_of_shards).to eq(10)
+        expect(Elastic::IndexSetting['gitlab-test-issues'].number_of_replicas).to eq(3)
+        expect(Elastic::IndexSetting['gitlab-test-notes'].number_of_shards).to eq(20)
+        expect(Elastic::IndexSetting['gitlab-test-notes'].number_of_replicas).to eq(4)
+
         expect(current_settings.elasticsearch_indexed_file_size_limit_kb).to eq(5000)
         expect(current_settings.elasticsearch_indexed_field_length_limit).to eq(100000)
         expect(current_settings.elasticsearch_max_bulk_size_mb).to eq(17)

ee/spec/lib/elastic/latest/config_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'config settings return correct values' do
+  it 'returns config' do
+    expect(described_class.settings).to be_a(Elasticsearch::Model::Indexing::Settings)
+  end
+
+  it 'sets correct shard/replica settings' do
+    allow(Elastic::IndexSetting).to receive(:[]).with(described_class.index_name).and_return(double(number_of_shards: 32, number_of_replicas: 2))
+
+    settings = described_class.settings.to_hash
+    expect(settings[:index][:number_of_shards].as_json).to eq(32)
+    expect(settings[:index][:number_of_replicas].as_json).to eq(2)
+  end
+end

ee/spec/lib/elastic/latest/config_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
+require_relative './config_shared_examples'
 
 RSpec.describe Elastic::Latest::Config do
   describe '.document_type' do
@@ -10,9 +11,7 @@ RSpec.describe Elastic::Latest::Config do
   end
 
   describe '.settings' do
-    it 'returns config' do
-      expect(described_class.settings).to be_a(Elasticsearch::Model::Indexing::Settings)
-    end
+    it_behaves_like 'config settings return correct values'
   end
 
   describe '.mappings' do

ee/spec/lib/elastic/latest/issue_config_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
+require_relative './config_shared_examples'
 
 RSpec.describe Elastic::Latest::IssueConfig do
   describe '.document_type' do
@@ -10,9 +11,7 @@ RSpec.describe Elastic::Latest::IssueConfig do
   end
 
   describe '.settings' do
-    it 'returns config' do
-      expect(described_class.settings).to be_a(Elasticsearch::Model::Indexing::Settings)
-    end
+    it_behaves_like 'config settings return correct values'
   end
 
   describe '.mappings' do

ee/spec/lib/elastic/latest/note_config_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
+require_relative './config_shared_examples'
 
 RSpec.describe Elastic::Latest::NoteConfig do
   describe '.document_type' do
@@ -10,9 +11,7 @@ RSpec.describe Elastic::Latest::NoteConfig do
   end
 
   describe '.settings' do
-    it 'returns config' do
-      expect(described_class.settings).to be_a(Elasticsearch::Model::Indexing::Settings)
-    end
+    it_behaves_like 'config settings return correct values'
   end
 
   describe '.mappings' do

locale/gitlab.pot

@@ -6032,9 +6032,6 @@ msgstr ""
 msgid "Changes to the title have not been saved"
 msgstr ""
 
-msgid "Changes won't take place until the index is %{link_start}recreated%{link_end}."
-msgstr ""
-
 msgid "Changing group URL can have unintended side effects."
 msgstr ""
 
@@ -16252,15 +16249,9 @@ msgstr ""
 msgid "How many days need to pass between marking entity for deletion and actual removing it."
 msgstr ""
 
-msgid "How many replicas each Elasticsearch shard has."
-msgstr ""
-
 msgid "How many seconds an IP will be counted towards the limit"
 msgstr ""
 
-msgid "How many shards to split the Elasticsearch index over."
-msgstr ""
-
 msgid "How many users will be evaluating the trial?"
 msgstr ""
 
@@ -17257,6 +17248,9 @@ msgstr ""
 msgid "Indent"
 msgstr ""
 
+msgid "Index"
+msgstr ""
+
 msgid "Index all projects"
 msgstr ""
 
@@ -19001,6 +18995,9 @@ msgstr ""
 msgid "Learn more about group-level project templates"
 msgstr ""
 
+msgid "Learn more about shards and replicas in the %{configuration_link_start}Advanced search configuration%{configuration_link_end} documentation. Changes won't take place until the index is %{recreated_link_start}recreated%{recreated_link_end}."
+msgstr ""
+
 msgid "Learn more about signing commits"
 msgstr ""
 
@@ -22316,10 +22313,7 @@ msgstr ""
 msgid "Number of %{itemTitle}"
 msgstr ""
 
-msgid "Number of Elasticsearch replicas"
-msgstr ""
-
-msgid "Number of Elasticsearch shards"
+msgid "Number of Elasticsearch shards and replicas (per index)"
 msgstr ""
 
 msgid "Number of Git pushes after which 'git gc' is run."
@@ -22358,6 +22352,12 @@ msgstr ""
 msgid "Number of files touched"
 msgstr ""
 
+msgid "Number of replicas"
+msgstr ""
+
+msgid "Number of shards"
+msgstr ""
+
 msgid "OK"
 msgstr ""