Merge branch '294210-gitlab-elasticsearch-reindexing-allow-slice-tuning-in-ui' into 'master'

Customize settings for Advanced Search reindex operations

See merge request gitlab-org/gitlab!60730

doc/integration/elasticsearch.md

@@ -328,16 +328,57 @@ index alias to it which becomes the new `primary` index. At the end, we resume t
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34069) in GitLab 13.2.
 > - A scheduled index deletion and the ability to cancel it was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38914) in GitLab 13.3.
+> - Support for retries during reindexing was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55681) in GitLab 13.12.
 
-Under **Admin Area > Settings > Advanced Search > Elasticsearch zero-downtime reindexing**, click on **Trigger cluster reindexing**.
+To trigger the reindexing process:
+
+1. Sign in to your GitLab instance as an administrator.
+1. Go to **Admin Area > Settings > Advanced Search > Elasticsearch zero-downtime reindexing**.
+1. Select **Trigger cluster reindexing**.
 
 Reindexing can be a lengthy process depending on the size of your Elasticsearch cluster.
 
-WARNING:
-After the reindexing is completed, the original index will be scheduled to be deleted after 14 days. You can cancel this action by pressing the cancel button.
+After this process is completed, the original index is scheduled to be deleted after
+14 days. You can cancel this action by pressing the **Cancel** button on the same
+page you triggered the reindexing process.
 
 While the reindexing is running, you will be able to follow its progress under that same section.
 
+#### Elasticsearch zero-downtime reindexing
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55681) in GitLab 13.12.
+
+The following reindex settings are available in **Admin Area > Settings > Advanced Search > Elasticsearch zero-downtime reindexing**:
+
+- [Slice multiplier](#slice-multiplier)
+- [Maximum running slices](#maximum-running-slices)
+
+##### Slice multiplier
+
+The slice multiplier calculates the [number of slices during reindexing](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-reindex.html#docs-reindex-slice).
+
+GitLab uses [manual slicing](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-reindex.html#docs-reindex-manual-slice)
+to control the reindex efficiently and safely, which enables users to retry only
+failed slices.
+
+The multiplier defaults to `2` and applies to the number of shards per index.
+For example, if this value is `2` and your index has 20 shards, then the
+reindex task is split into 40 slices.
+
+##### Maximum running slices
+
+The maximum running slices parameter defaults to `60` and corresponds to the
+maximum number of slices allowed to run concurrently during Elasticsearch
+reindexing.
+
+Setting this value too high can have adverse performance impacts as your cluster
+may become heavily saturated with searches and writes. Setting this value too
+low may lead the reindexing process to take a very long time to complete.
+
+The best value for this will depend on your cluster size, whether you're willing
+to accept some degraded search performance during reindexing, and how important
+it is for the reindex to finish quickly and unpause indexing.
+
 ### Mark the most recent reindex job as failed and resume the indexing
 
 Sometimes, you might want to abandon the unfinished reindex job and resume the indexing. You can achieve this via the following steps:

ee/app/controllers/admin/elasticsearch_controller.rb

@@ -25,11 +25,16 @@ class Admin::ElasticsearchController < Admin::ApplicationController
     if Elastic::ReindexingTask.running?
       flash[:warning] = _('Elasticsearch reindexing is already in progress')
     else
-      Elastic::ReindexingTask.create!
-      flash[:notice] = _('Elasticsearch reindexing triggered')
+      @elasticsearch_reindexing_task = Elastic::ReindexingTask.new(trigger_reindexing_params)
+      if @elasticsearch_reindexing_task.save
+        flash[:notice] = _('Elasticsearch reindexing triggered')
+      else
+        errors = @elasticsearch_reindexing_task.errors.full_messages.join(', ')
+        flash[:alert] = _("Elasticsearch reindexing was not started: %{errors}") % { errors: errors }
+      end
     end
 
-    redirect_to redirect_path
+    redirect_to redirect_path(anchor: 'js-elasticsearch-reindexing')
   end
 
   # POST
@@ -40,7 +45,7 @@ class Admin::ElasticsearchController < Admin::ApplicationController
 
     flash[:notice] = _('Index deletion is canceled')
 
-    redirect_to redirect_path
+    redirect_to redirect_path(anchor: 'js-elasticsearch-reindexing')
   end
 
   # POST
@@ -58,7 +63,11 @@ class Admin::ElasticsearchController < Admin::ApplicationController
 
   private
 
-  def redirect_path
-    advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+  def redirect_path(anchor: 'js-elasticsearch-settings')
+    advanced_search_admin_application_settings_path(anchor: anchor)
+  end
+
+  def trigger_reindexing_params
+    params.require(:elastic_reindexing_task).permit(%i(max_slices_running slice_multiplier))
   end
 end

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

@@ -15,7 +15,8 @@ module EE
         feature_category :global_search, [:advanced_search]
 
         def elasticsearch_reindexing_task
-          @elasticsearch_reindexing_task = Elastic::ReindexingTask.last
+          @last_elasticsearch_reindexing_task = Elastic::ReindexingTask.last
+          @elasticsearch_reindexing_task = Elastic::ReindexingTask.new
         end
 
         def elasticsearch_index_settings

ee/app/models/elastic/reindexing_subtask.rb

@@ -8,4 +8,6 @@ class Elastic::ReindexingSubtask < ApplicationRecord
   has_many :slices, class_name: 'Elastic::ReindexingSlice', foreign_key: :elastic_reindexing_subtask_id
 
   validates :index_name_from, :index_name_to, presence: true
+
+  scope :order_by_alias_name_asc, -> { order(alias_name: :asc) }
 end

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

@@ -3,12 +3,14 @@
 - 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' } }
+%section.settings.as-elasticsearch.no-animate#js-elasticsearch-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'elasticsearch_tab' } }
   .settings-header
     %h4
       = _('Advanced Search')
+    %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded_by_default? ? _('Collapse') : _('Expand')
     %p
-      = _('Advanced Search with Elasticsearch')
+      = _('Configure settings for Advanced Search with Elasticsearch.')
 
   .settings-content
     = form_for @application_setting, url: advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings'), html: { class: 'fieldset-form' } do |f|
@@ -48,7 +50,7 @@
               .form-group
                 .form-check
                 - pending_migrations = elasticsearch_available && Elastic::DataMigrationService.pending_migrations? && Gitlab::CurrentSettings.elasticsearch_pause_indexing?
-                - disable_checkbox = !Gitlab::CurrentSettings.elasticsearch_indexing? || pending_migrations || @elasticsearch_reindexing_task&.in_progress?
+                - disable_checkbox = !Gitlab::CurrentSettings.elasticsearch_indexing? || pending_migrations || @last_elasticsearch_reindexing_task&.in_progress?
                 = f.check_box :elasticsearch_pause_indexing, class: 'form-check-input', data: { qa_selector: 'pause_checkbox' }, disabled: disable_checkbox
                 = f.label :elasticsearch_pause_indexing, class: 'form-check-label' do
                   = _('Pause Elasticsearch indexing')
@@ -135,35 +137,6 @@
                 = _('Elasticsearch HTTP client timeout value in seconds.')
                 = _('Setting this to 0 means using the system default timeout value.')
 
-        .gl-card.gl-bg-gray-10.gl-mb-6
-          .gl-card-body
-            %h4= _('Elasticsearch zero-downtime reindexing')
-            = link_to _('Trigger cluster reindexing'), admin_elasticsearch_trigger_reindexing_path, class: "gl-button btn btn-info", disabled: @elasticsearch_reindexing_task&.in_progress?, data: { confirm: _('Are you sure you want to reindex?') }, method: :post
-            .form-text.gl-text-gray-600
-              = _('This feature should be used with an index that was created after 13.0')
-            - Elastic::ReindexingTask.old_indices_scheduled_for_deletion.each do |task|
-              .form-text.gl-text-red-500.gl-mt-0
-                = _("Unused, previous indices: %{index_names} will be deleted after %{time} automatically.") % { index_names: task.subtasks.map(&:index_name_from).join(', '), time: task.delete_original_index_at }
-                = link_to _('Cancel index deletion'), admin_elasticsearch_cancel_index_deletion_path(task_id: task.id), class: 'gl-mb-2', method: :post
-            - if @elasticsearch_reindexing_task
-              %h5= _('Reindexing Status: %{status}') % { status: @elasticsearch_reindexing_task.state }
-              - if @elasticsearch_reindexing_task.error_message
-                %p= _('Error: %{error_message}') % { error_message: @elasticsearch_reindexing_task.error_message }
-              - @elasticsearch_reindexing_task.subtasks.each do |subtask|
-                .gl-card-body.form-group
-                  %h5= subtask.alias_name
-                  - expected_documents = subtask.documents_count
-                  - if subtask.elastic_task
-                    %p= _('Task ID: %{elastic_task}') % { elastic_task: subtask.elastic_task }
-                  - if expected_documents
-                    - processed_documents = subtask.documents_count_target
-                    %p= _('Expected documents: %{expected_documents}') % { expected_documents: expected_documents }
-                    - if processed_documents && expected_documents
-                      - percentage = ((processed_documents / expected_documents.to_f) * 100).round(2)
-                      %p= _('Documents reindexed: %{processed_documents} (%{percentage}%%)') % { processed_documents: processed_documents, percentage: percentage }
-                      .progress
-                        .progress-bar{ "aria-valuemax" => "100", "aria-valuemin" => "0", "aria-valuenow" => percentage, :role => "progressbar", :style => "width: #{percentage}%" }
-
         .gl-card.gl-bg-gray-10.gl-mb-6
           .gl-card-body
             %h4= _('Elasticsearch indexing restrictions')
@@ -251,3 +224,56 @@
                 = _('AWS Secret Access Key. Only required if not using role instance credentials')
 
       = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'submit_button' }
+
+%section.settings.as-elasticsearch-reindexing.no-animate#js-elasticsearch-reindexing{ class: ('expanded' if expanded_by_default?) }
+  .settings-header
+    %h4
+      = _('Elasticsearch zero-downtime reindexing')
+    %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded_by_default? ? _('Collapse') : _('Expand')
+    %p
+      = _('Trigger cluster reindexing. This feature should be used with an index that was created after 13.0.')
+
+  .settings-content
+    = form_for @elasticsearch_reindexing_task, url: admin_elasticsearch_trigger_reindexing_path, method: :post, html: { class: 'fieldset-form' } do |f|
+      %fieldset
+        .form-group
+          = f.label :slice_multiplier, _('Slice multiplier'), class: 'label-bold'
+          = f.number_field :slice_multiplier, class: 'form-control gl-form-input', value: @elasticsearch_reindexing_task.slice_multiplier
+          .form-text.gl-text-gray-600.gl-mt-0
+            - slice_multiplier_link_url = help_page_path('integration/elasticsearch.md', anchor: 'slice-multiplier')
+            - slice_multiplier_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: slice_multiplier_link_url }
+            = _('Used to calculate the number of slices during reindexing. The multiplier will be applied to the number of shards per index. Learn more about %{slice_multiplier_link_start}slice multiplier configuration%{slice_multiplier_link_end}.').html_safe % { slice_multiplier_link_start: slice_multiplier_link_start, slice_multiplier_link_end: '</a>'.html_safe }
+
+        .form-group
+          = f.label :max_slices_running, _('Maximum running slices'), class: 'label-bold'
+          = f.number_field :max_slices_running, class: 'form-control gl-form-input', value: @elasticsearch_reindexing_task.max_slices_running
+          .form-text.gl-text-gray-600.gl-mt-0
+            - max_slices_running_link_url = help_page_path('integration/elasticsearch.md', anchor: 'maximum-running-slices')
+            - max_slices_running_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: max_slices_running_link_url }
+            = _('The maximum number of slices allowed to run concurrently during Elasticsearch reindexing. Learn more about %{max_slices_running_link_start}maximum running slices configuration%{max_slices_link_end}.').html_safe % { max_slices_running_link_start: max_slices_running_link_start, max_slices_link_end: '</a>'.html_safe }
+
+        = f.submit _('Trigger cluster reindexing'), class: "gl-button btn btn-info", disabled: @last_elasticsearch_reindexing_task&.in_progress?, data: { confirm: _('Are you sure you want to reindex?') }
+        .form-text.gl-text-gray-600
+        - Elastic::ReindexingTask.old_indices_scheduled_for_deletion.each do |task|
+          .form-text.gl-text-red-500.gl-mt-0
+            = _("Unused, previous indices: %{index_names} will be deleted after %{time} automatically.") % { index_names: task.subtasks.map(&:index_name_from).join(', '), time: task.delete_original_index_at }
+            = link_to _('Cancel index deletion'), admin_elasticsearch_cancel_index_deletion_path(task_id: task.id), class: 'gl-mb-2', method: :post
+        - if @last_elasticsearch_reindexing_task
+          %h5= _('Reindexing Status: %{status} (Slice multiplier: %{multiplier}, Maximum running slices: %{max_slices})') % { status: @last_elasticsearch_reindexing_task.state, max_slices: @last_elasticsearch_reindexing_task.max_slices_running, multiplier: @last_elasticsearch_reindexing_task.slice_multiplier }
+          - if @last_elasticsearch_reindexing_task.error_message
+            %p= _('Error: %{error_message}') % { error_message: @last_elasticsearch_reindexing_task.error_message }
+          - @last_elasticsearch_reindexing_task.subtasks.order_by_alias_name_asc.each do |subtask|
+            .gl-card-body.form-group
+              %h5= subtask.alias_name
+              - expected_documents = subtask.documents_count
+              - if subtask.elastic_task
+                %p= _('Task ID: %{elastic_task}') % { elastic_task: subtask.elastic_task }
+              - if expected_documents
+                - processed_documents = subtask.documents_count_target
+                %p= _('Expected documents: %{expected_documents}') % { expected_documents: expected_documents }
+                - if processed_documents && expected_documents
+                  - percentage = ((processed_documents / expected_documents.to_f) * 100).round(2)
+                  %p= _('Documents reindexed: %{processed_documents} (%{percentage}%%)') % { processed_documents: processed_documents, percentage: percentage }
+                  .progress
+                    .progress-bar{ "aria-valuemax" => "100", "aria-valuemin" => "0", "aria-valuenow" => percentage, :role => "progressbar", :style => "width: #{percentage}%" }

ee/changelogs/unreleased/294210-gitlab-elasticsearch-reindexing-allow-slice-tuning-in-ui.yml

+---
+title: Customize settings for Advanced Search reindex operations
+merge_request: 60730
+author:
+type: changed

ee/spec/controllers/admin/application_settings_controller_spec.rb

@@ -326,10 +326,10 @@ RSpec.describe Admin::ApplicationSettingsController do
 
       let!(:task) { create(:elastic_reindexing_task) }
 
-      it 'assigns elasticsearch reindexing task' do
+      it 'assigns last elasticsearch reindexing task' do
         get :advanced_search
 
-        expect(assigns(:elasticsearch_reindexing_task)).to eq(task)
+        expect(assigns(:last_elasticsearch_reindexing_task)).to eq(task)
         expect(response.body).to include("Reindexing Status: #{task.state}")
       end
     end

ee/spec/controllers/admin/elasticsearch_controller_spec.rb

@@ -46,22 +46,30 @@ RSpec.describe Admin::ElasticsearchController do
     end
 
     it 'creates a reindexing task' do
-      expect(Elastic::ReindexingTask).to receive(:create!)
+      expect_next_instance_of(Elastic::ReindexingTask) do |task|
+        expect(task).to receive(:save).and_return(true)
+      end
 
-      post :trigger_reindexing
+      post :trigger_reindexing, params: { elastic_reindexing_task: { max_slices_running: 60, slice_multiplier: 2 } }
 
       expect(controller).to set_flash[:notice].to include('reindexing triggered')
-      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing')
     end
 
     it 'does not create a reindexing task if there is another one' do
       allow(Elastic::ReindexingTask).to receive(:current).and_return(build(:elastic_reindexing_task))
-      expect(Elastic::ReindexingTask).not_to receive(:create!)
 
-      post :trigger_reindexing
+      post :trigger_reindexing, params: { elastic_reindexing_task: { max_slices_running: 60, slice_multiplier: 2 } }
 
       expect(controller).to set_flash[:warning].to include('already in progress')
-      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing')
+    end
+
+    it 'does not create a reindexing task if a required param is nil' do
+      post :trigger_reindexing, params: { elastic_reindexing_task: { max_slices_running: nil, slice_multiplier: 2 } }
+
+      expect(controller).to set_flash[:alert].to include('Elasticsearch reindexing was not started')
+      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing')
     end
   end
 
@@ -77,7 +85,7 @@ RSpec.describe Admin::ElasticsearchController do
 
       expect(task.reload.delete_original_index_at).to be_nil
       expect(controller).to set_flash[:notice].to include('deletion is canceled')
-      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing')
     end
   end
 

ee/spec/features/admin/admin_settings_spec.rb

@@ -182,9 +182,9 @@ RSpec.describe 'Admin updates EE-only settings' do
     end
 
     it 'zero-downtime reindexing shows popup', :js do
-      page.within('.as-elasticsearch') do
+      page.within('.as-elasticsearch-reindexing') do
         expect(page).to have_content 'Trigger cluster reindexing'
-        click_link 'Trigger cluster reindexing'
+        click_button 'Trigger cluster reindexing'
       end
 
       text = page.driver.browser.switch_to.alert.text

ee/spec/views/admin/application_settings/_elasticsearch_form.html.haml_spec.rb

@@ -8,9 +8,11 @@ RSpec.describe 'admin/application_settings/_elasticsearch_form' do
   let(:page) { Capybara::Node::Simple.new(rendered) }
   let(:pause_indexing) { false }
   let(:pending_migrations) { false }
+  let(:elastic_reindexing_task) { build(:elastic_reindexing_task) }
 
   before do
     assign(:application_setting, application_setting)
+    assign(:elasticsearch_reindexing_task, elastic_reindexing_task)
     allow(view).to receive(:current_user) { admin }
     allow(view).to receive(:expanded) { true }
   end
@@ -94,7 +96,7 @@ RSpec.describe 'admin/application_settings/_elasticsearch_form' do
     let(:application_setting) { build(:application_setting) }
 
     before do
-      assign(:elasticsearch_reindexing_task, task)
+      assign(:last_elasticsearch_reindexing_task, task)
     end
 
     context 'when task is in progress' do
@@ -109,7 +111,7 @@ RSpec.describe 'admin/application_settings/_elasticsearch_form' do
       it 'renders a disabled trigger cluster reindexing link' do
         render
 
-        expect(rendered).to have_css('a.gl-button[disabled="disabled"]', text: 'Trigger cluster reindexing')
+        expect(rendered).to have_button('Trigger cluster reindexing', disabled: true)
       end
     end
 

locale/gitlab.pot

@@ -2777,9 +2777,6 @@ msgstr ""
 msgid "Advanced Search"
 msgstr ""
 
-msgid "Advanced Search with Elasticsearch"
-msgstr ""
-
 msgid "Advanced Settings"
 msgstr ""
 
@@ -8425,6 +8422,9 @@ msgstr ""
 msgid "Configure repository mirroring."
 msgstr ""
 
+msgid "Configure settings for Advanced Search with Elasticsearch."
+msgstr ""
+
 msgid "Configure specific limits for Packages API requests that supersede the general user and IP rate limits."
 msgstr ""
 
@@ -11945,6 +11945,9 @@ msgstr ""
 msgid "Elasticsearch reindexing triggered"
 msgstr ""
 
+msgid "Elasticsearch reindexing was not started: %{errors}"
+msgstr ""
+
 msgid "Elasticsearch returned status code: %{status_code}"
 msgstr ""
 
@@ -20266,6 +20269,9 @@ msgstr ""
 msgid "Maximum push size (MB)"
 msgstr ""
 
+msgid "Maximum running slices"
+msgstr ""
+
 msgid "Maximum size limit for a single commit."
 msgstr ""
 
@@ -26988,7 +26994,7 @@ msgstr ""
 msgid "Regulate approvals by authors/committers. Affects all projects."
 msgstr ""
 
-msgid "Reindexing Status: %{status}"
+msgid "Reindexing Status: %{status} (Slice multiplier: %{multiplier}, Maximum running slices: %{max_slices})"
 msgstr ""
 
 msgid "Rejected (closed)"
@@ -30113,6 +30119,9 @@ msgstr ""
 msgid "SlackService|This service allows users to perform common operations on this project by entering slash commands in Slack."
 msgstr ""
 
+msgid "Slice multiplier"
+msgstr ""
+
 msgid "Smartcard"
 msgstr ""
 
@@ -32285,6 +32294,9 @@ msgstr ""
 msgid "The maximum file size is %{size}."
 msgstr ""
 
+msgid "The maximum number of slices allowed to run concurrently during Elasticsearch reindexing. Learn more about %{max_slices_running_link_start}maximum running slices configuration%{max_slices_link_end}."
+msgstr ""
+
 msgid "The maximum number of tags that a single worker accepts for cleanup. If the number of tags goes above this limit, the list of tags to delete is truncated to this number. To remove this limit, set it to 0."
 msgstr ""
 
@@ -33044,9 +33056,6 @@ msgstr ""
 msgid "This feature requires local storage to be enabled"
 msgstr ""
 
-msgid "This feature should be used with an index that was created after 13.0"
-msgstr ""
-
 msgid "This field is required."
 msgstr ""
 
@@ -34244,6 +34253,9 @@ msgstr ""
 msgid "Trigger cluster reindexing"
 msgstr ""
 
+msgid "Trigger cluster reindexing. This feature should be used with an index that was created after 13.0."
+msgstr ""
+
 msgid "Trigger manual job"
 msgstr ""
 
@@ -35204,6 +35216,9 @@ msgstr ""
 msgid "Used programming language"
 msgstr ""
 
+msgid "Used to calculate the number of slices during reindexing. The multiplier will be applied to the number of shards per index. Learn more about %{slice_multiplier_link_start}slice multiplier configuration%{slice_multiplier_link_end}."
+msgstr ""
+
 msgid "Used to help configure your identity provider"
 msgstr ""