Merge branch '225436-reindex-delete-original-index' into 'master'

Automate the deletion of the old Index after a reindex

Closes #225436

See merge request gitlab-org/gitlab!38914

db/migrate/20200807132726_add_delete_original_index_at_to_reindexing_tasks.rb

+# frozen_string_literal: true
+
+class AddDeleteOriginalIndexAtToReindexingTasks < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :elastic_reindexing_tasks, :delete_original_index_at, :datetime_with_timezone
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :elastic_reindexing_tasks, :delete_original_index_at
+    end
+  end
+end

db/schema_migrations/20200807132726

+867cea94f966c1ad3d9e02f7fa5b641ceac5a71667426330c2c96d6181164f66
\ No newline at end of file

db/structure.sql

@@ -11353,6 +11353,7 @@ CREATE TABLE public.elastic_reindexing_tasks (
     elastic_task text,
     error_message text,
     documents_count_target integer,
+    delete_original_index_at timestamp with time zone,
     CONSTRAINT check_04151aca42 CHECK ((char_length(index_name_from) <= 255)),
     CONSTRAINT check_7f64acda8e CHECK ((char_length(error_message) <= 255)),
     CONSTRAINT check_85ebff7124 CHECK ((char_length(index_name_to) <= 255)),

doc/integration/elasticsearch.md

@@ -548,13 +548,17 @@ To trigger the re-index from `primary` index:
 
 ### Trigger the reindex via the Elasticsearch administration
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34069) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.2.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34069) in [GitLab Starter](https://about.gitlab.com/pricing/) 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 Starter 13.3.
 
 Under **Admin Area > Integration > Elasticsearch zero-downtime reindexing**, click on **Trigger cluster reindexing**.
 
 NOTE: **Note:**
 Reindexing can be a lengthy process depending on the size of your Elasticsearch cluster.
 
+CAUTION: **Caution:**
+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.
+
 While the reindexing is running, you will be able to follow its progress under that same section.
 
 ## GitLab Elasticsearch Rake tasks

ee/app/controllers/admin/elasticsearch_controller.rb

@@ -30,6 +30,17 @@ class Admin::ElasticsearchController < Admin::ApplicationController
     redirect_to redirect_path
   end
 
+  # POST
+  # Cancel index deletion after a successful reindexing operation
+  def cancel_index_deletion
+    task = Elastic::ReindexingTask.find(params[:task_id])
+    task.update!(delete_original_index_at: nil)
+
+    flash[:notice] = _('Index deletion is canceled')
+
+    redirect_to redirect_path
+  end
+
   private
 
   def redirect_path

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

@@ -9,7 +9,7 @@ module EE
       include ::Admin::MergeRequestApprovalSettingsHelper
 
       prepended do
-        before_action :elasticsearch_reindexing_task, only: [:integrations]
+        before_action :elasticsearch_reindexing_task, only: [:general]
 
         def elasticsearch_reindexing_task
           @elasticsearch_reindexing_task = Elastic::ReindexingTask.last

ee/app/models/elastic/reindexing_task.rb

@@ -8,9 +8,13 @@ class Elastic::ReindexingTask < ApplicationRecord
     indexing_paused:        1,
     reindexing:             2,
     success:                10, # states less than 10 are considered in_progress
-    failure:         11
+    failure:                11,
+    original_index_deleted: 12
   }
 
+  scope :old_indices_scheduled_for_deletion, -> { where(state: :success).where('delete_original_index_at IS NOT NULL') }
+  scope :old_indices_to_be_deleted, -> { old_indices_scheduled_for_deletion.where('delete_original_index_at < NOW()') }
+
   before_save :set_in_progress_flag
 
   def self.current
@@ -21,6 +25,14 @@ class Elastic::ReindexingTask < ApplicationRecord
     current.present?
   end
 
+  def self.drop_old_indices!
+    old_indices_to_be_deleted.find_each do |task|
+      next unless Gitlab::Elastic::Helper.default.delete_index(index_name: task.index_name_from)
+
+      task.update!(state: :original_index_deleted)
+    end
+  end
+
   private
 
   def set_in_progress_flag

ee/app/services/elastic/cluster_reindexing_service.rb

@@ -10,6 +10,8 @@ module Elastic
         translog: { durability: 'async' }
     }.freeze
 
+    DELETE_ORIGINAL_INDEX_AFTER = 14.days
+
     def execute
       case current_task.state.to_sym
       when :initial
@@ -124,7 +126,7 @@ module Elastic
     def finalize_reindexing
       Gitlab::CurrentSettings.update!(elasticsearch_pause_indexing: false)
 
-      current_task.update!(state: :success)
+      current_task.update!(state: :success, delete_original_index_at: DELETE_ORIGINAL_INDEX_AFTER.from_now)
     end
 
     def reindexing!

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

@@ -98,6 +98,10 @@
           = link_to _('Trigger cluster reindexing'), admin_elasticsearch_trigger_reindexing_path, class: 'btn btn-success', method: :post, disabled: @elasticsearch_reindexing_task&.in_progress?
           .form-text.text-muted
             = _('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.text-danger
+              = _("Unused, previous index '%{index_name}' will be deleted after %{time} automatically.") % { index_name: task.index_name_from, time: task.delete_original_index_at }
+              = link_to _('Cancel index deletion'), admin_elasticsearch_cancel_index_deletion_path(task_id: task.id), method: :post
           - if @elasticsearch_reindexing_task
             - expected_documents = @elasticsearch_reindexing_task.documents_count
             - processed_documents = @elasticsearch_reindexing_task.documents_count_target

ee/app/workers/elastic_cluster_reindexing_cron_worker.rb

@@ -12,10 +12,12 @@ class ElasticClusterReindexingCronWorker
   idempotent!
 
   def perform
+    in_lock(self.class.name.underscore, ttl: 1.hour, retries: 10, sleep_sec: 1) do
+      Elastic::ReindexingTask.drop_old_indices!
+
       task = Elastic::ReindexingTask.current
-    return false unless task
+      break false unless task
 
-    in_lock(self.class.name.underscore, ttl: 1.hour, retries: 10, sleep_sec: 1) do
       service.execute
     end
   end

ee/changelogs/unreleased/225436-reindex-delete-original-index.yml

+---
+title: Automate the deletion of the old Index after a reindex
+merge_request: 38914
+author:
+type: changed

ee/config/routes/admin.rb

@@ -72,5 +72,6 @@ namespace :admin do
   namespace :elasticsearch do
     post :enqueue_index
     post :trigger_reindexing
+    post :cancel_index_deletion
   end
 end

ee/spec/controllers/admin/elasticsearch_controller_spec.rb

@@ -62,4 +62,20 @@ RSpec.describe Admin::ElasticsearchController do
       expect(response).to redirect_to general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
     end
   end
+
+  describe 'POST #cancel_index_deletion' do
+    before do
+      sign_in(admin)
+    end
+
+    let(:task) { create(:elastic_reindexing_task, state: :success, delete_original_index_at: Time.current) }
+
+    it 'sets delete_original_index_at to nil' do
+      post :cancel_index_deletion, params: { task_id: task.id }
+
+      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 general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+    end
+  end
 end

ee/spec/models/elastic/reindexing_task_spec.rb

@@ -16,4 +16,30 @@ RSpec.describe Elastic::ReindexingTask, type: :model do
     task.update!(state: :reindexing)
     expect(task.in_progress).to eq(true)
   end
+
+  describe '.drop_old_indices!' do
+    let(:task_1) { create(:elastic_reindexing_task, index_name_from: 'original_index_1', state: :reindexing, delete_original_index_at: 1.day.ago) }
+    let(:task_2) { create(:elastic_reindexing_task, index_name_from: 'original_index_2', state: :success, delete_original_index_at: nil) }
+    let(:task_3) { create(:elastic_reindexing_task, index_name_from: 'original_index_3', state: :success, delete_original_index_at: 1.day.ago) }
+    let(:task_4) { create(:elastic_reindexing_task, index_name_from: 'original_index_4', state: :success, delete_original_index_at: 5.days.ago) }
+    let(:task_5) { create(:elastic_reindexing_task, index_name_from: 'original_index_5', state: :success, delete_original_index_at: 14.days.from_now) }
+    let(:tasks_for_deletion) { [task_3, task_4] }
+    let(:other_tasks) { [task_1, task_2, task_5] }
+
+    it 'deletes the correct indices' do
+      other_tasks.each do |task|
+        expect(Gitlab::Elastic::Helper.default).not_to receive(:delete_index).with(index_name: task.index_name_from)
+      end
+
+      tasks_for_deletion.each do |task|
+        expect(Gitlab::Elastic::Helper.default).to receive(:delete_index).with(index_name: task.index_name_from).and_return(true)
+      end
+
+      described_class.drop_old_indices!
+
+      tasks_for_deletion.each do |task|
+        expect(task.reload.state).to eq('original_index_deleted')
+      end
+    end
+  end
 end

ee/spec/services/elastic/cluster_reindexing_service_spec.rb

@@ -84,6 +84,7 @@ RSpec.describe Elastic::ClusterReindexingService, :elastic do
         expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false)
 
         expect { subject.execute }.to change { task.reload.state }.from('reindexing').to('success')
+        expect(task.reload.delete_original_index_at).to be_within(1.minute).of(described_class::DELETE_ORIGINAL_INDEX_AFTER.from_now)
       end
     end
   end

ee/spec/workers/elastic_cluster_reindexing_cron_worker_spec.rb

@@ -16,8 +16,9 @@ RSpec.describe ElasticClusterReindexingCronWorker do
       subject.perform
     end
 
-    it 'does nothing if no task is found' do
+    it 'removes old indices if no task is found' do
       expect(Elastic::ReindexingTask).to receive(:current).and_return(nil)
+      expect(Elastic::ReindexingTask).to receive(:drop_old_indices!)
 
       expect(subject.perform).to eq(false)
     end

locale/gitlab.pot

@@ -4319,6 +4319,9 @@ msgstr ""
 msgid "Cancel"
 msgstr ""
 
+msgid "Cancel index deletion"
+msgstr ""
+
 msgid "Cancel running"
 msgstr ""
 
@@ -13010,6 +13013,9 @@ msgstr ""
 msgid "Index all projects"
 msgstr ""
 
+msgid "Index deletion is canceled"
+msgstr ""
+
 msgid "Indicates whether this runner can pick jobs without tags"
 msgstr ""
 
@@ -26265,6 +26271,9 @@ msgstr ""
 msgid "Until"
 msgstr ""
 
+msgid "Unused, previous index '%{index_name}' will be deleted after %{time} automatically."
+msgstr ""
+
 msgid "Unverified"
 msgstr ""