diff --git a/.rubocop_todo/gitlab/bounded_contexts.yml b/.rubocop_todo/gitlab/bounded_contexts.yml index 587cfea1683b9d643f62551ee0c6b699662b5475..2f9c5639e254c373cf05be25d3c502dc4aac83e8 100644 --- a/.rubocop_todo/gitlab/bounded_contexts.yml +++ b/.rubocop_todo/gitlab/bounded_contexts.yml @@ -2907,7 +2907,6 @@ Gitlab/BoundedContexts: - 'ee/app/models/elastic/index_setting.rb' - 'ee/app/models/elastic/migration_record.rb' - 'ee/app/models/elastic/reindexing_slice.rb' - - 'ee/app/models/elastic/reindexing_task.rb' - 'ee/app/models/elasticsearch_indexed_namespace.rb' - 'ee/app/models/elasticsearch_indexed_project.rb' - 'ee/app/models/embedding/application_record.rb' diff --git a/.rubocop_todo/search/namespaced_class.yml b/.rubocop_todo/search/namespaced_class.yml index 118f78429906a941f6b38baf128057bf47e9fd7a..6816e90a09dde0c6abbecb85fab5b6f790f06b76 100644 --- a/.rubocop_todo/search/namespaced_class.yml +++ b/.rubocop_todo/search/namespaced_class.yml @@ -31,7 +31,6 @@ Search/NamespacedClass: - 'ee/app/models/elastic/index_setting.rb' - 'ee/app/models/elastic/migration_record.rb' - 'ee/app/models/elastic/reindexing_slice.rb' - - 'ee/app/models/elastic/reindexing_task.rb' - 'ee/app/models/elasticsearch_indexed_namespace.rb' - 'ee/app/models/elasticsearch_indexed_project.rb' - 'ee/app/presenters/ee/search_service_presenter.rb' diff --git a/.rubocop_todo/style/class_and_module_children.yml b/.rubocop_todo/style/class_and_module_children.yml index 542cbbc3bb22965c0c7aaf133d78babd0dd63972..33a5cd6ac14c1d092abfbe577d1c7f64115ee786 100644 --- a/.rubocop_todo/style/class_and_module_children.yml +++ b/.rubocop_todo/style/class_and_module_children.yml @@ -421,7 +421,6 @@ Style/ClassAndModuleChildren: - 'ee/app/models/dast/profile_schedule.rb' - 'ee/app/models/ee/ci/job_artifact.rb' - 'ee/app/models/elastic/reindexing_slice.rb' - - 'ee/app/models/elastic/reindexing_task.rb' - 'ee/app/models/epic/metrics.rb' - 'ee/app/models/epic/related_epic_link.rb' - 'ee/app/models/geo/base_registry.rb' diff --git a/db/docs/elastic_reindexing_tasks.yml b/db/docs/elastic_reindexing_tasks.yml index b9568481b7c5782cf17121433a3d455fb86e6b67..d29ad441ed40c2b4f663b52c3ec673e21d28a0e0 100644 --- a/db/docs/elastic_reindexing_tasks.yml +++ b/db/docs/elastic_reindexing_tasks.yml @@ -1,7 +1,7 @@ --- table_name: elastic_reindexing_tasks classes: -- Elastic::ReindexingTask +- Search::Elastic::ReindexingTask feature_categories: - global_search description: TODO diff --git a/ee/app/controllers/admin/elasticsearch_controller.rb b/ee/app/controllers/admin/elasticsearch_controller.rb index 987465c88d2c45fda392ce0aaa5e99e153d661e4..96815f5e8868e0004335440fdf3c275ec69526ea 100644 --- a/ee/app/controllers/admin/elasticsearch_controller.rb +++ b/ee/app/controllers/admin/elasticsearch_controller.rb @@ -19,10 +19,10 @@ def enqueue_index # POST # Trigger reindexing task def trigger_reindexing - if Elastic::ReindexingTask.running? + if Search::Elastic::ReindexingTask.running? flash[:warning] = _('Elasticsearch reindexing is already in progress') else - @elasticsearch_reindexing_task = Elastic::ReindexingTask.new(trigger_reindexing_params) + @elasticsearch_reindexing_task = Search::Elastic::ReindexingTask.new(trigger_reindexing_params) if @elasticsearch_reindexing_task.save flash[:notice] = _('Elasticsearch reindexing triggered') else @@ -37,7 +37,7 @@ def trigger_reindexing # POST # Cancel index deletion after a successful reindexing operation def cancel_index_deletion - task = Elastic::ReindexingTask.find(params[:task_id]) + task = Search::Elastic::ReindexingTask.find(params[:task_id]) task.update!(delete_original_index_at: nil) flash[:notice] = _('Index deletion is canceled') @@ -65,7 +65,7 @@ def redirect_path(anchor: 'js-elasticsearch-settings') end def trigger_reindexing_params - permitted_params = params.require(:elastic_reindexing_task).permit(:elasticsearch_max_slices_running, + permitted_params = params.require(:search_elastic_reindexing_task).permit(:elasticsearch_max_slices_running, :elasticsearch_slice_multiplier) trigger_reindexing_params = {} if permitted_params.has_key?(:elasticsearch_max_slices_running) diff --git a/ee/app/controllers/ee/admin/application_settings_controller.rb b/ee/app/controllers/ee/admin/application_settings_controller.rb index d6961d16ba56a30ad8738636a8d13421e905f274..66c3e26b7d0255e508cd776422670141d1bcdae4 100644 --- a/ee/app/controllers/ee/admin/application_settings_controller.rb +++ b/ee/app/controllers/ee/admin/application_settings_controller.rb @@ -37,8 +37,8 @@ module ApplicationSettingsController urgency :low, [:advanced_search, :seat_link_payload] def elasticsearch_reindexing_task - @last_elasticsearch_reindexing_task = Elastic::ReindexingTask.last - @elasticsearch_reindexing_task = Elastic::ReindexingTask.new + @last_elasticsearch_reindexing_task = ::Search::Elastic::ReindexingTask.last + @elasticsearch_reindexing_task = ::Search::Elastic::ReindexingTask.new end def elasticsearch_index_settings diff --git a/ee/app/models/elastic/reindexing_task.rb b/ee/app/models/elastic/reindexing_task.rb deleted file mode 100644 index 7acb414e39ad1983e7156438a6de4777b963e482..0000000000000000000000000000000000000000 --- a/ee/app/models/elastic/reindexing_task.rb +++ /dev/null @@ -1,61 +0,0 @@ -# frozen_string_literal: true - -class Elastic::ReindexingTask < ApplicationRecord - include EachBatch - - self.table_name = 'elastic_reindexing_tasks' - - validates :max_slices_running, presence: true - validates :slice_multiplier, presence: true - validates :options, json_schema: { filename: 'elastic_reindexing_task_options' } - - attribute :options, :ind_jsonb # for indifferent access - - has_many :subtasks, class_name: 'Search::Elastic::ReindexingSubtask', - foreign_key: :elastic_reindexing_task_id, inverse_of: :elastic_reindexing_task - - enum state: { - initial: 0, - indexing_paused: 1, - reindexing: 2, - success: 10, # states less than 10 are considered in_progress - failure: 11, - original_index_deleted: 12 - } - - scope :old_indices_scheduled_for_deletion, -> { where(state: :success).where.not(delete_original_index_at: nil) } - 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 - where(in_progress: true).last - end - - def self.running? - current.present? - end - - def self.drop_old_indices! - old_indices_to_be_deleted.find_each do |task| - task.subtasks.each do |subtask| - Gitlab::Elastic::Helper.default.delete_index(index_name: subtask.index_name_from) - end - task.update!(state: :original_index_deleted) - end - end - - def target_classes - return ::Gitlab::Elastic::Helper::INDEXED_CLASSES if targets.blank? - - targets.map(&:constantize) - end - - private - - def set_in_progress_flag - in_progress_states = self.class.states.select { |_, v| v < 10 }.keys - - self.in_progress = in_progress_states.include?(state) - end -end diff --git a/ee/app/models/search/elastic/reindexing_subtask.rb b/ee/app/models/search/elastic/reindexing_subtask.rb index 80f6b9fe3245589062a79afb90210d2f2c155fde..2749002f6588e7ebda1ee6438bd4d6339a95866f 100644 --- a/ee/app/models/search/elastic/reindexing_subtask.rb +++ b/ee/app/models/search/elastic/reindexing_subtask.rb @@ -5,7 +5,7 @@ module Elastic class ReindexingSubtask < ApplicationRecord self.table_name = 'elastic_reindexing_subtasks' - belongs_to :elastic_reindexing_task, class_name: 'Elastic::ReindexingTask' + belongs_to :elastic_reindexing_task, class_name: 'Search::Elastic::ReindexingTask' has_many :slices, class_name: 'Elastic::ReindexingSlice', foreign_key: :elastic_reindexing_subtask_id, inverse_of: :elastic_reindexing_subtask diff --git a/ee/app/models/search/elastic/reindexing_task.rb b/ee/app/models/search/elastic/reindexing_task.rb new file mode 100644 index 0000000000000000000000000000000000000000..9ed97bec5d6b2694542a1ed5755464d500ba4326 --- /dev/null +++ b/ee/app/models/search/elastic/reindexing_task.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Search + module Elastic + class ReindexingTask < ApplicationRecord + include EachBatch + + self.table_name = 'elastic_reindexing_tasks' + + validates :max_slices_running, presence: true + validates :slice_multiplier, presence: true + validates :options, json_schema: { filename: 'elastic_reindexing_task_options' } + + attribute :options, :ind_jsonb # for indifferent access + + has_many :subtasks, class_name: 'Search::Elastic::ReindexingSubtask', + foreign_key: :elastic_reindexing_task_id, inverse_of: :elastic_reindexing_task + + enum state: { + initial: 0, + indexing_paused: 1, + reindexing: 2, + success: 10, # states less than 10 are considered in_progress + failure: 11, + original_index_deleted: 12 + } + + scope :old_indices_scheduled_for_deletion, -> { where(state: :success).where.not(delete_original_index_at: nil) } + scope :old_indices_to_be_deleted, -> do + old_indices_scheduled_for_deletion.where('delete_original_index_at < NOW()') + end + + before_save :set_in_progress_flag + + def self.current + where(in_progress: true).last + end + + def self.running? + current.present? + end + + def self.drop_old_indices! + old_indices_to_be_deleted.find_each do |task| + task.subtasks.each do |subtask| + Gitlab::Elastic::Helper.default.delete_index(index_name: subtask.index_name_from) + end + task.update!(state: :original_index_deleted) + end + end + + def target_classes + return ::Gitlab::Elastic::Helper::INDEXED_CLASSES if targets.blank? + + targets.map(&:constantize) + end + + private + + def set_in_progress_flag + in_progress_states = self.class.states.select { |_, v| v < 10 }.keys + + self.in_progress = in_progress_states.include?(state) + end + end + end +end diff --git a/ee/app/services/search/elastic/cluster_reindexing_service.rb b/ee/app/services/search/elastic/cluster_reindexing_service.rb index ceaceb854286fe6559ed916abd3bceb5c6084537..a0f44294188abe411fe62933b4604f0588a49c3c 100644 --- a/ee/app/services/search/elastic/cluster_reindexing_service.rb +++ b/ee/app/services/search/elastic/cluster_reindexing_service.rb @@ -28,7 +28,7 @@ def execute end def current_task - ::Elastic::ReindexingTask.current + ::Search::Elastic::ReindexingTask.current end strong_memoize_attr :current_task diff --git a/ee/app/services/search/rake_task_executor_service.rb b/ee/app/services/search/rake_task_executor_service.rb index 00a715c6f250a174a7e47536a9f14dbd8899749d..185cdc5a959a0951944e2dad604eab0f70031385 100644 --- a/ee/app/services/search/rake_task_executor_service.rb +++ b/ee/app/services/search/rake_task_executor_service.rb @@ -60,7 +60,7 @@ def clear_index_status end def clear_reindex_status - ::Elastic::ReindexingTask.each_batch do |batch| + ::Search::Elastic::ReindexingTask.each_batch do |batch| batch.delete_all end @@ -120,7 +120,7 @@ def recreate_index end def reindex_cluster - ::Elastic::ReindexingTask.create! + ::Search::Elastic::ReindexingTask.create! ::ElasticClusterReindexingCronWorker.perform_async @@ -222,8 +222,8 @@ def estimate_cluster_size end def mark_reindex_failed - if ::Elastic::ReindexingTask.running? - ::Elastic::ReindexingTask.current.failure! + if ::Search::Elastic::ReindexingTask.running? + ::Search::Elastic::ReindexingTask.current.failure! logger.info(Rainbow('Marked the current reindexing job as failed.').green) else logger.info('Did not find the current running reindexing job.') diff --git a/ee/app/views/admin/application_settings/_elasticsearch_form.html.haml b/ee/app/views/admin/application_settings/_elasticsearch_form.html.haml index a686eeaeaa5d7ab5afb33a896cd007a3c6581033..291393827cb3d7584a82795592bed581cbe1f565 100644 --- a/ee/app/views/admin/application_settings/_elasticsearch_form.html.haml +++ b/ee/app/views/admin/application_settings/_elasticsearch_form.html.haml @@ -241,7 +241,7 @@ = f.submit _('Trigger cluster reindexing'), pajamas_button: true, 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| + - Search::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 diff --git a/ee/app/workers/elastic/migration_worker.rb b/ee/app/workers/elastic/migration_worker.rb index 89c3a04253e33ef06d43c8988bd0e82b299bf48f..52ca2197afa5a03b10a50fb9080b37f5bea95961 100644 --- a/ee/app/workers/elastic/migration_worker.rb +++ b/ee/app/workers/elastic/migration_worker.rb @@ -103,7 +103,7 @@ def preflight_check_successful? return false if Feature.disabled?(:elastic_migration_worker, type: :ops) # rubocop:disable Gitlab/ FeatureFlagWithoutActor -- this is an ops flag that should be on or off return false unless Gitlab::CurrentSettings.elasticsearch_indexing? return false unless helper.alias_exists? - return false if Elastic::ReindexingTask.current + return false if Search::Elastic::ReindexingTask.current if helper.unsupported_version? msg = <<~MSG.strip_heredoc.tr("\n", ' ') diff --git a/ee/app/workers/elastic_cluster_reindexing_cron_worker.rb b/ee/app/workers/elastic_cluster_reindexing_cron_worker.rb index 1ca2b1c853bc2fc460c5209007618045076e6c77..09f4b65b96738fa284f5c303831d1034ae1579ea 100644 --- a/ee/app/workers/elastic_cluster_reindexing_cron_worker.rb +++ b/ee/app/workers/elastic_cluster_reindexing_cron_worker.rb @@ -16,9 +16,9 @@ class ElasticClusterReindexingCronWorker def perform in_lock(self.class.name.underscore, ttl: 1.hour, retries: 10, sleep_sec: 1) do - Elastic::ReindexingTask.drop_old_indices! + Search::Elastic::ReindexingTask.drop_old_indices! - task = Elastic::ReindexingTask.current + task = Search::Elastic::ReindexingTask.current break false unless task service.execute diff --git a/ee/elastic/migrate/20231130202203_reindex_issues_to_update_analyzer.rb b/ee/elastic/migrate/20231130202203_reindex_issues_to_update_analyzer.rb index 7410e5f7c6da497c923cb60a309d98613b34c201..8d140cf47c3388c28ac186b738544a28a73b9304 100644 --- a/ee/elastic/migrate/20231130202203_reindex_issues_to_update_analyzer.rb +++ b/ee/elastic/migrate/20231130202203_reindex_issues_to_update_analyzer.rb @@ -2,7 +2,7 @@ class ReindexIssuesToUpdateAnalyzer < Elastic::Migration def migrate - Elastic::ReindexingTask.create!(targets: %w[Issue], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[Issue], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20240119130539_reindex_notes_to_update_analyzer.rb b/ee/elastic/migrate/20240119130539_reindex_notes_to_update_analyzer.rb index cf189518221beed5fe0528a41349f6e9b0daafd1..ae6b3717736de579594e1174974209858284cdb5 100644 --- a/ee/elastic/migrate/20240119130539_reindex_notes_to_update_analyzer.rb +++ b/ee/elastic/migrate/20240119130539_reindex_notes_to_update_analyzer.rb @@ -2,7 +2,7 @@ class ReindexNotesToUpdateAnalyzer < Elastic::Migration def migrate - Elastic::ReindexingTask.create!(targets: %w[Note], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[Note], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20240123181031_reindex_issue_to_update_analyzer_for_title.rb b/ee/elastic/migrate/20240123181031_reindex_issue_to_update_analyzer_for_title.rb index 4dbeef88848372252becc223a62401da531aeed6..734d9a65f24e4622ddffee9fc1d8bdc269b6ad33 100644 --- a/ee/elastic/migrate/20240123181031_reindex_issue_to_update_analyzer_for_title.rb +++ b/ee/elastic/migrate/20240123181031_reindex_issue_to_update_analyzer_for_title.rb @@ -2,7 +2,7 @@ class ReindexIssueToUpdateAnalyzerForTitle < Elastic::Migration def migrate - Elastic::ReindexingTask.create!(targets: %w[Issue], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[Issue], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20240130144625_reindex_epics_to_update_analyzer.rb b/ee/elastic/migrate/20240130144625_reindex_epics_to_update_analyzer.rb index 691b127a9c3136faf7a2c90f0fdfd2c3cb5d805a..464d809a34642eab6323647b1491b921ca61d0c8 100644 --- a/ee/elastic/migrate/20240130144625_reindex_epics_to_update_analyzer.rb +++ b/ee/elastic/migrate/20240130144625_reindex_epics_to_update_analyzer.rb @@ -2,7 +2,7 @@ class ReindexEpicsToUpdateAnalyzer < Elastic::Migration def migrate - Elastic::ReindexingTask.create!(targets: %w[Epic], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[Epic], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20240201164432_reindex_merge_request_to_update_analyzer.rb b/ee/elastic/migrate/20240201164432_reindex_merge_request_to_update_analyzer.rb index 39222b092d95c141c7543af664132920ab12863b..b64e2ce7704fd8f67aeaf851fb38c50749f3aa74 100644 --- a/ee/elastic/migrate/20240201164432_reindex_merge_request_to_update_analyzer.rb +++ b/ee/elastic/migrate/20240201164432_reindex_merge_request_to_update_analyzer.rb @@ -2,7 +2,7 @@ class ReindexMergeRequestToUpdateAnalyzer < Elastic::Migration def migrate - Elastic::ReindexingTask.create!(targets: %w[MergeRequest], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[MergeRequest], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion.rb b/ee/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion.rb index 5006716d32b3021f91e98768e3c28a194db347dc..a5f952614c65e221af8f053ca1d6c458a779e855 100644 --- a/ee/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion.rb +++ b/ee/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion.rb @@ -4,7 +4,7 @@ class ReindexMergeRequestsForTitleCompletion < Elastic::Migration skip_if -> { !Gitlab::Saas.feature_available?(:advanced_search) } def migrate - Elastic::ReindexingTask.create!(targets: %w[MergeRequest], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[MergeRequest], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch.rb b/ee/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch.rb index f90588ae37fbb04724b8a404096d2a7e794eccb4..662a300fe3951547aa11911223d98986ef0e41ec 100644 --- a/ee/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch.rb +++ b/ee/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch.rb @@ -4,7 +4,7 @@ class AddEmbeddingToWorkItemsOpensearch < Elastic::Migration skip_if -> { !opensearch? } def migrate - Elastic::ReindexingTask.create!(targets: %w[WorkItem], options: { skip_pending_migrations_check: true }) + Search::Elastic::ReindexingTask.create!(targets: %w[WorkItem], options: { skip_pending_migrations_check: true }) end def completed? diff --git a/ee/spec/controllers/admin/elasticsearch_controller_spec.rb b/ee/spec/controllers/admin/elasticsearch_controller_spec.rb index c068b497e0b10e1dda388d2f7ebb0c66a5e6219e..c03e10961363ba0214166c06b481ff51e71e389d 100644 --- a/ee/spec/controllers/admin/elasticsearch_controller_spec.rb +++ b/ee/spec/controllers/admin/elasticsearch_controller_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Admin::ElasticsearchController, feature_category: :global_search do - let(:admin) { create(:admin) } - let(:helper) { Gitlab::Elastic::Helper.new } + let_it_be(:admin) { create(:admin) } + let_it_be(:helper) { Gitlab::Elastic::Helper.new } describe 'POST #enqueue_index' do before do @@ -16,7 +16,8 @@ post :enqueue_index - expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings') + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings') + expect(response).to redirect_to expected_redirect end end @@ -26,30 +27,45 @@ end it 'creates a reindexing task' do - expect_next_instance_of(Elastic::ReindexingTask) do |task| + expect_next_instance_of(Search::Elastic::ReindexingTask) do |task| expect(task).to receive(:save).and_return(true) end - post :trigger_reindexing, params: { elastic_reindexing_task: { elasticsearch_max_slices_running: 60, elasticsearch_slice_multiplier: 2 } } + params = { + search_elastic_reindexing_task: { elasticsearch_max_slices_running: 60, elasticsearch_slice_multiplier: 2 } + } + post :trigger_reindexing, params: params expect(controller).to set_flash[:notice].to include('reindexing triggered') - expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing') + + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing') + expect(response).to redirect_to expected_redirect 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)) + allow(Search::Elastic::ReindexingTask).to receive(:current).and_return(build(:elastic_reindexing_task)) - post :trigger_reindexing, params: { elastic_reindexing_task: { elasticsearch_max_slices_running: 60, elasticsearch_slice_multiplier: 2 } } + params = { + search_elastic_reindexing_task: { elasticsearch_max_slices_running: 60, elasticsearch_slice_multiplier: 2 } + } + post :trigger_reindexing, params: params 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-reindexing') + + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing') + expect(response).to redirect_to expected_redirect end it 'does not create a reindexing task if a required param is nil' do - post :trigger_reindexing, params: { elastic_reindexing_task: { elasticsearch_max_slices_running: nil, elasticsearch_slice_multiplier: 2 } } + params = { + search_elastic_reindexing_task: { elasticsearch_max_slices_running: nil, elasticsearch_slice_multiplier: 2 } + } + post :trigger_reindexing, params: params 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') + + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing') + expect(response).to redirect_to expected_redirect end end @@ -65,7 +81,9 @@ 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-reindexing') + + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-reindexing') + expect(response).to redirect_to expected_redirect end end @@ -79,7 +97,8 @@ it 'deletes the migration record and drops the halted cache' do allow(Elastic::MigrationRecord).to receive(:new).and_call_original - allow(Elastic::MigrationRecord).to receive(:new).with(version: migration.version, name: migration.name, filename: migration.filename).and_return(migration) + allow(Elastic::MigrationRecord).to receive(:new) + .with(version: migration.version, name: migration.name, filename: migration.filename).and_return(migration) allow(Elastic::DataMigrationService).to receive(:migration_halted?).and_return(false) allow(Elastic::DataMigrationService).to receive(:migration_halted?).with(migration).and_return(true, false) expect(Elastic::DataMigrationService.halted_migrations?).to be_truthy @@ -88,7 +107,9 @@ expect(Elastic::DataMigrationService.halted_migrations?).to be_falsey expect(controller).to set_flash[:notice].to include('Migration has been scheduled to be retried') - expect(response).to redirect_to advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings') + + expected_redirect = advanced_search_admin_application_settings_path(anchor: 'js-elasticsearch-settings') + expect(response).to redirect_to expected_redirect end end end diff --git a/ee/spec/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion_spec.rb b/ee/spec/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion_spec.rb index 59f9d9c81f39df1241dcc4248c9fa1763133355a..af3ed0bd2b9eb6f1694f1082710a62f4d356f576 100644 --- a/ee/spec/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion_spec.rb +++ b/ee/spec/elastic/migrate/20241002103536_reindex_merge_requests_for_title_completion_spec.rb @@ -14,8 +14,8 @@ describe '#migrate' do it 'creates reindexing task with correct target and options' do - expect { migration.migrate }.to change { Elastic::ReindexingTask.count }.by(1) - task = Elastic::ReindexingTask.last + expect { migration.migrate }.to change { Search::Elastic::ReindexingTask.count }.by(1) + task = Search::Elastic::ReindexingTask.last expect(task.targets).to eq(%w[MergeRequest]) expect(task.options).to eq('skip_pending_migrations_check' => true) end diff --git a/ee/spec/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch_spec.rb b/ee/spec/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch_spec.rb index 5291109e1d33cccc0d23db5a2ec9acfe8aaa720e..2cdf22667aa8f14110e6f54a43d5b848137a781a 100644 --- a/ee/spec/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch_spec.rb +++ b/ee/spec/elastic/migrate/20241017094601_add_embedding_to_work_items_opensearch_spec.rb @@ -20,8 +20,8 @@ describe '#migrate' do it 'creates reindexing task with correct target and options' do - expect { migration.migrate }.to change { Elastic::ReindexingTask.count }.by(1) - task = Elastic::ReindexingTask.last + expect { migration.migrate }.to change { Search::Elastic::ReindexingTask.count }.by(1) + task = Search::Elastic::ReindexingTask.last expect(task.targets).to eq(%w[WorkItem]) expect(task.options).to eq({ 'skip_pending_migrations_check' => true }) end diff --git a/ee/spec/factories/elastic/reindexing_tasks.rb b/ee/spec/factories/elastic/reindexing_tasks.rb index 62d1fc23a54da2c1816544644dd84d25bd10a2c5..b9f73d891bb3c98bc61da0cb0427b0c98d6b01e1 100644 --- a/ee/spec/factories/elastic/reindexing_tasks.rb +++ b/ee/spec/factories/elastic/reindexing_tasks.rb @@ -1,14 +1,12 @@ # frozen_string_literal: true FactoryBot.define do - factory :elastic_reindexing_task, class: 'Elastic::ReindexingTask' do + factory :elastic_reindexing_task, class: 'Search::Elastic::ReindexingTask' do state { :initial } in_progress { true } trait :with_subtask do - after(:create) do |task| - create :elastic_reindexing_subtask, elastic_reindexing_task: task - end + subtasks { [association(:elastic_reindexing_subtask)] } end end end diff --git a/ee/spec/models/elastic/reindexing_task_spec.rb b/ee/spec/models/search/elastic/reindexing_task_spec.rb similarity index 71% rename from ee/spec/models/elastic/reindexing_task_spec.rb rename to ee/spec/models/search/elastic/reindexing_task_spec.rb index d3924bdf054437c887478c97230ea5e41448bea4..893257251ce0db9ec72964363129afb65466909c 100644 --- a/ee/spec/models/elastic/reindexing_task_spec.rb +++ b/ee/spec/models/search/elastic/reindexing_task_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Elastic::ReindexingTask, type: :model do +RSpec.describe Search::Elastic::ReindexingTask, type: :model, feature_category: :global_search do let(:helper) { Gitlab::Elastic::Helper.new } before do @@ -26,18 +26,30 @@ it 'sets in_progress flag' do task = create(:elastic_reindexing_task, state: :success) - expect(task.in_progress).to eq(false) + expect(task.in_progress).to be(false) task.update!(state: :reindexing) - expect(task.in_progress).to eq(true) + expect(task.in_progress).to be(true) end describe '.drop_old_indices!' do - let(:task_1) { create(:elastic_reindexing_task, :with_subtask, state: :reindexing, delete_original_index_at: 1.day.ago) } + let(:task_1) do + create(:elastic_reindexing_task, :with_subtask, state: :reindexing, delete_original_index_at: 1.day.ago) + end + let(:task_2) { create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: nil) } - let(:task_3) { create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 1.day.ago) } - let(:task_4) { create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 5.days.ago) } - let(:task_5) { create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 14.days.from_now) } + let(:task_3) do + create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 1.day.ago) + end + + let(:task_4) do + create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 5.days.ago) + end + + let(:task_5) do + create(:elastic_reindexing_task, :with_subtask, state: :success, delete_original_index_at: 14.days.from_now) + end + let(:tasks_for_deletion) { [task_3, task_4] } let(:other_tasks) { [task_1, task_2, task_5] } @@ -68,7 +80,7 @@ end it 'returns all classes when targets are empty' do - expect(task.target_classes).to eq(::Gitlab::Elastic::Helper::INDEXED_CLASSES) + expect(task.target_classes).to be(::Gitlab::Elastic::Helper::INDEXED_CLASSES) end end end diff --git a/ee/spec/services/search/rake_task_executor_service_spec.rb b/ee/spec/services/search/rake_task_executor_service_spec.rb index bd5d11c984f0b454985245dffcb2d96c14d693b2..2ae26c276184c505148c449f72667791f17e79e8 100644 --- a/ee/spec/services/search/rake_task_executor_service_spec.rb +++ b/ee/spec/services/search/rake_task_executor_service_spec.rb @@ -330,14 +330,14 @@ describe '#mark_reindex_failed' do context 'when there is a running reindex job' do before do - Elastic::ReindexingTask.create! + Search::Elastic::ReindexingTask.create! end it 'marks the current reindex job as failed' do expect(logger).to receive(:info).with(/Marked the current reindexing job as failed/) expect { service.execute(:mark_reindex_failed) } - .to change { Elastic::ReindexingTask.running? }.from(true).to(false) + .to change { Search::Elastic::ReindexingTask.running? }.from(true).to(false) end it 'prints a message after marking it as failed' do @@ -1071,7 +1071,7 @@ subject(:reindex_cluster) { service.execute(:reindex_cluster) } it 'creates a reindexing task and queues the cron worker' do - expect(::Elastic::ReindexingTask).to receive(:create!) + expect(::Search::Elastic::ReindexingTask).to receive(:create!) expect(::ElasticClusterReindexingCronWorker).to receive(:perform_async) expect(logger).to receive(:info).with(/Reindexing job was successfully scheduled/) @@ -1081,7 +1081,7 @@ context 'when a reindexing task is in progress' do it 'logs an error' do - ::Elastic::ReindexingTask.create! + ::Search::Elastic::ReindexingTask.create! expect(::ElasticClusterReindexingCronWorker).not_to receive(:perform_async) @@ -1109,7 +1109,7 @@ it 'calls deletes all reindex records' do create(:elastic_reindexing_task) - expect { clear_reindex_status }.to change { ::Elastic::ReindexingTask.count }.from(1).to(0) + expect { clear_reindex_status }.to change { ::Search::Elastic::ReindexingTask.count }.from(1).to(0) end end diff --git a/ee/spec/workers/elastic_cluster_reindexing_cron_worker_spec.rb b/ee/spec/workers/elastic_cluster_reindexing_cron_worker_spec.rb index ffb630c2be9c2f3f231ba376aa1e8d244d3b728a..711b7b02058860855ed5b71df24f77f6e3d786d7 100644 --- a/ee/spec/workers/elastic_cluster_reindexing_cron_worker_spec.rb +++ b/ee/spec/workers/elastic_cluster_reindexing_cron_worker_spec.rb @@ -7,7 +7,7 @@ describe '#perform' do it 'calls execute method' do - expect(Elastic::ReindexingTask).to receive(:current).and_return(build(:elastic_reindexing_task)) + expect(Search::Elastic::ReindexingTask).to receive(:current).and_return(build(:elastic_reindexing_task)) expect_next_instance_of(::Search::Elastic::ClusterReindexingService) do |service| expect(service).to receive(:execute).and_return(false) @@ -17,8 +17,8 @@ end 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(Search::Elastic::ReindexingTask).to receive(:current).and_return(nil) + expect(Search::Elastic::ReindexingTask).to receive(:drop_old_indices!) expect(worker.perform).to eq(false) end