diff --git a/.rubocop_todo/gitlab/bounded_contexts.yml b/.rubocop_todo/gitlab/bounded_contexts.yml index 747a3150bd269f5a7e46eddb460bd2c01a57843e..19cfb6eba3ed9ebccc815d675b01b85e7ebf0173 100644 --- a/.rubocop_todo/gitlab/bounded_contexts.yml +++ b/.rubocop_todo/gitlab/bounded_contexts.yml @@ -3427,7 +3427,6 @@ Gitlab/BoundedContexts: - 'ee/app/workers/click_house/rebuild_materialized_view_cron_worker.rb' - 'ee/app/workers/concerns/elastic/bulk_cron_worker.rb' - 'ee/app/workers/concerns/elastic/migration_create_index.rb' - - 'ee/app/workers/concerns/elastic/migration_helper.rb' - 'ee/app/workers/concerns/elastic/migration_options.rb' - 'ee/app/workers/concerns/elastic/migration_state.rb' - 'ee/app/workers/concerns/geo_backoff_delay.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index 2c85dd1948a579c93f6b8c4975352d04e4a54edc..75ecde8c27d8238b14588825ed0ff4cb4a84ff06 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -1035,7 +1035,6 @@ RSpec/NamedSubject: - 'ee/spec/workers/ci/minutes/update_project_and_namespace_usage_worker_spec.rb' - 'ee/spec/workers/ci/runners/stale_group_runners_prune_cron_worker_spec.rb' - 'ee/spec/workers/compliance_management/chain_of_custody_report_worker_spec.rb' - - 'ee/spec/workers/concerns/elastic/migration_helper_spec.rb' - 'ee/spec/workers/concerns/elastic/migration_options_spec.rb' - 'ee/spec/workers/create_github_webhook_worker_spec.rb' - 'ee/spec/workers/deployments/auto_rollback_worker_spec.rb' diff --git a/.rubocop_todo/search/namespaced_class.yml b/.rubocop_todo/search/namespaced_class.yml index acce04589269f635726a343e19cbdaa4dffadece..93e1d1121b1ec29afa7b49de2f184188013924da 100644 --- a/.rubocop_todo/search/namespaced_class.yml +++ b/.rubocop_todo/search/namespaced_class.yml @@ -43,7 +43,6 @@ Search/NamespacedClass: - 'ee/app/services/protected_environments/search_service.rb' - 'ee/app/workers/concerns/elastic/bulk_cron_worker.rb' - 'ee/app/workers/concerns/elastic/migration_create_index.rb' - - 'ee/app/workers/concerns/elastic/migration_helper.rb' - 'ee/app/workers/concerns/elastic/migration_options.rb' - 'ee/app/workers/concerns/elastic/migration_state.rb' - 'ee/app/workers/elastic/migration_worker.rb' diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index abc7b70753ebf60971ef61583c13d407177f6efe..17ea871c0fa46bead2f1106669b243501884b13f 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -332,7 +332,6 @@ Style/IfUnlessModifier: - 'ee/app/validators/password/complexity_validator.rb' - 'ee/app/workers/app_sec/dast/profile_schedule_worker.rb' - 'ee/app/workers/audit_events/audit_event_streaming_worker.rb' - - 'ee/app/workers/concerns/elastic/migration_helper.rb' - 'ee/app/workers/ee/ci/build_finished_worker.rb' - 'ee/app/workers/epics/new_epic_issue_worker.rb' - 'ee/app/workers/geo/secondary/registry_consistency_worker.rb' diff --git a/doc/development/search/advanced_search_migration_styleguide.md b/doc/development/search/advanced_search_migration_styleguide.md index 2388d5c1aac48894d58543efa043bb812d026b5f..11363c456475a22de1bdc33b6ecbe75f025e008b 100644 --- a/doc/development/search/advanced_search_migration_styleguide.md +++ b/doc/development/search/advanced_search_migration_styleguide.md @@ -418,13 +418,13 @@ class MigrationName < Elastic::Migration end ``` -#### `Elastic::MigrationHelper` +#### `Search::Elastic::MigrationHelper` Contains methods you can use when a migration doesn't fit the previous examples. ```ruby class MigrationName < Elastic::Migration - include Elastic::MigrationHelper + include ::Search::Elastic::MigrationHelper def migrate ... diff --git a/ee/app/workers/concerns/elastic/migration_create_index.rb b/ee/app/workers/concerns/elastic/migration_create_index.rb index f88b005aa60c86bc71080cacc53ee310b76a044c..7a07c5ec748d7f276155221ba810f6a1d6ee9a97 100644 --- a/ee/app/workers/concerns/elastic/migration_create_index.rb +++ b/ee/app/workers/concerns/elastic/migration_create_index.rb @@ -2,7 +2,7 @@ module Elastic module MigrationCreateIndex - include Elastic::MigrationHelper + include ::Search::Elastic::MigrationHelper def migrate reindexing_cleanup! diff --git a/ee/app/workers/concerns/elastic/migration_helper.rb b/ee/app/workers/concerns/elastic/migration_helper.rb deleted file mode 100644 index cd10824f98e7c5e5f24bcf48b64ebfada5879159..0000000000000000000000000000000000000000 --- a/ee/app/workers/concerns/elastic/migration_helper.rb +++ /dev/null @@ -1,138 +0,0 @@ -# frozen_string_literal: true - -module Elastic - module MigrationHelper - def get_number_of_shards(index_name: new_index_name) - helper.get_settings(index_name: index_name)['number_of_shards'].to_i - end - - def get_max_slices(index_name: new_index_name) - number_of_shards = get_number_of_shards(index_name: index_name) - - number_of_shards.to_i <= 1 ? 2 : number_of_shards - end - - private - - def document_type - raise NotImplementedError - end - - def document_type_fields - raise NotImplementedError - end - - def document_type_plural - document_type.to_s.pluralize - end - - def default_index_name - helper.target_name - end - - def new_index_name - "#{default_index_name}-#{document_type_plural}" - end - - def original_documents_count - query = { - size: 0, - aggs: { - documents: { - filter: { - term: { - type: { - value: document_type - } - } - } - } - } - } - - results = client.search(index: default_index_name, body: query) - results.dig('aggregations', 'documents', 'doc_count') - end - - def new_documents_count - helper.refresh_index(index_name: new_index_name) - helper.documents_count(index_name: new_index_name) - end - - def reindexing_cleanup! - helper.delete_index(index_name: new_index_name) if helper.index_exists?(index_name: new_index_name) - end - - def reindex(slice:, max_slices:, script: nil) - body = reindex_query(slice: slice, max_slices: max_slices, script: script) - - response = client.reindex(body: body, wait_for_completion: false) - - response['task'] - end - - def reindexing_completed?(task_id:) - response = helper.task_status(task_id: task_id) - completed = response['completed'] - - return false unless completed - - stats = response['response'] - if stats['failures'].present? - log_raise "Reindexing failed with #{stats['failures']}" - end - - if stats['total'] != (stats['updated'] + stats['created'] + stats['deleted']) - log_raise "Slice reindexing seems to have failed, total is not equal to updated + created + deleted" - end - - true - end - - def reindex_query(slice:, max_slices:, script: nil) - query = { - source: { - index: default_index_name, - _source: document_type_fields, - query: { - match: { - type: document_type - } - }, - slice: { - id: slice, - max: max_slices - } - }, - dest: { - index: new_index_name - } - } - - if script - query[:script] = { - lang: 'painless', - source: script - } - end - - query - end - - def create_index_for_first_batch!(target_classes) - return if migration_state[:slice].present? - - reindexing_cleanup! # support retries - - log "Change index settings for #{document_type_plural} index under #{new_index_name}" - - default_setting = Elastic::IndexSetting.default - Elastic::IndexSetting[new_index_name].update!(number_of_replicas: default_setting.number_of_replicas, - number_of_shards: default_setting.number_of_shards) - - log "Create standalone #{document_type_plural} index under #{new_index_name}" - - helper.create_standalone_indices(target_classes: target_classes) - end - end -end diff --git a/ee/app/workers/concerns/search/elastic/migration_helper.rb b/ee/app/workers/concerns/search/elastic/migration_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..6629d69a3aeb6c7cfb517e3f320d6bc540563437 --- /dev/null +++ b/ee/app/workers/concerns/search/elastic/migration_helper.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +module Search + module Elastic + module MigrationHelper + def get_number_of_shards(index_name: new_index_name) + helper.get_settings(index_name: index_name)['number_of_shards'].to_i + end + + def get_max_slices(index_name: new_index_name) + number_of_shards = get_number_of_shards(index_name: index_name) + + number_of_shards.to_i <= 1 ? 2 : number_of_shards + end + + private + + def document_type + raise NotImplementedError + end + + def document_type_fields + raise NotImplementedError + end + + def document_type_plural + document_type.to_s.pluralize + end + + def default_index_name + helper.target_name + end + + def new_index_name + "#{default_index_name}-#{document_type_plural}" + end + + def original_documents_count + query = { + size: 0, + aggs: { + documents: { + filter: { + term: { + type: { + value: document_type + } + } + } + } + } + } + + results = client.search(index: default_index_name, body: query) + results.dig('aggregations', 'documents', 'doc_count') + end + + def new_documents_count + helper.refresh_index(index_name: new_index_name) + helper.documents_count(index_name: new_index_name) + end + + def reindexing_cleanup! + helper.delete_index(index_name: new_index_name) if helper.index_exists?(index_name: new_index_name) + end + + def reindex(slice:, max_slices:, script: nil) + body = reindex_query(slice: slice, max_slices: max_slices, script: script) + + response = client.reindex(body: body, wait_for_completion: false) + + response['task'] + end + + def reindexing_completed?(task_id:) + response = helper.task_status(task_id: task_id) + completed = response['completed'] + + return false unless completed + + stats = response['response'] + log_raise "Reindexing failed with #{stats['failures']}" if stats['failures'].present? + + if stats['total'] != (stats['updated'] + stats['created'] + stats['deleted']) + log_raise "Slice reindexing seems to have failed, total is not equal to updated + created + deleted" + end + + true + end + + def reindex_query(slice:, max_slices:, script: nil) + query = { + source: { + index: default_index_name, + _source: document_type_fields, + query: { + match: { + type: document_type + } + }, + slice: { + id: slice, + max: max_slices + } + }, + dest: { + index: new_index_name + } + } + + if script + query[:script] = { + lang: 'painless', + source: script + } + end + + query + end + + def create_index_for_first_batch!(target_classes) + return if migration_state[:slice].present? + + reindexing_cleanup! # support retries + + log "Change index settings for #{document_type_plural} index under #{new_index_name}" + + default_setting = ::Elastic::IndexSetting.default + ::Elastic::IndexSetting[new_index_name].update!(number_of_replicas: default_setting.number_of_replicas, + number_of_shards: default_setting.number_of_shards) + + log "Create standalone #{document_type_plural} index under #{new_index_name}" + + helper.create_standalone_indices(target_classes: target_classes) + end + end + end +end diff --git a/ee/elastic/migrate/20231019223356_reindex_wikis_to_fix_routing_and_backfill_archived.rb b/ee/elastic/migrate/20231019223356_reindex_wikis_to_fix_routing_and_backfill_archived.rb index 9974c70b6834f8ceecba13c54ddeca4b51564fb9..9be125cce1fe86bdc431a3af2d00494a7ac988fc 100644 --- a/ee/elastic/migrate/20231019223356_reindex_wikis_to_fix_routing_and_backfill_archived.rb +++ b/ee/elastic/migrate/20231019223356_reindex_wikis_to_fix_routing_and_backfill_archived.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ReindexWikisToFixRoutingAndBackfillArchived < Elastic::Migration - include Elastic::MigrationHelper + include ::Search::Elastic::MigrationHelper batched! throttle_delay 5.minutes diff --git a/ee/elastic/migrate/20240130215043_reindex_wikis_to_fix_id.rb b/ee/elastic/migrate/20240130215043_reindex_wikis_to_fix_id.rb index 3e36448101fa15cb2e7ae6cb4782310fe69ef723..8a811ec92c0e41e3635fa564debb22419808e4ab 100644 --- a/ee/elastic/migrate/20240130215043_reindex_wikis_to_fix_id.rb +++ b/ee/elastic/migrate/20240130215043_reindex_wikis_to_fix_id.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ReindexWikisToFixId < Elastic::Migration - include Elastic::MigrationHelper + include ::Search::Elastic::MigrationHelper batched! throttle_delay 5.minutes diff --git a/ee/spec/workers/concerns/elastic/migration_helper_spec.rb b/ee/spec/workers/concerns/search/elastic/migration_helper_spec.rb similarity index 60% rename from ee/spec/workers/concerns/elastic/migration_helper_spec.rb rename to ee/spec/workers/concerns/search/elastic/migration_helper_spec.rb index a5eb95f46252b2c15f7205fb4805939d29b3d040..726f7e7e100299e8cb491728b6556ca10093a857 100644 --- a/ee/spec/workers/concerns/elastic/migration_helper_spec.rb +++ b/ee/spec/workers/concerns/search/elastic/migration_helper_spec.rb @@ -2,19 +2,19 @@ require 'spec_helper' -RSpec.describe Elastic::MigrationHelper, feature_category: :global_search do +RSpec.describe ::Search::Elastic::MigrationHelper, feature_category: :global_search do let(:index_name) { 'index_name' } let(:helper) { Gitlab::Elastic::Helper.new } let(:migration_class) do Class.new do - include Elastic::MigrationHelper + include ::Search::Elastic::MigrationHelper end end - subject { migration_class.new } + subject(:migration) { migration_class.new } before do - allow(subject).to receive(:helper).and_return(helper) + allow(migration).to receive(:helper).and_return(helper) end describe '#get_number_of_shards' do @@ -24,7 +24,7 @@ it 'uses get_settings' do expect(helper).to receive(:get_settings).with(index_name: index_name).and_return(settings) - expect(subject.get_number_of_shards(index_name: index_name)).to eq(number_of_shards) + expect(migration.get_number_of_shards(index_name: index_name)).to eq(number_of_shards) end end @@ -32,7 +32,7 @@ using RSpec::Parameterized::TableSyntax before do - allow(subject).to receive(:get_number_of_shards).with(index_name: index_name).and_return(number_of_shards) + allow(migration).to receive(:get_number_of_shards).with(index_name: index_name).and_return(number_of_shards) end where(:number_of_shards, :result) do @@ -44,7 +44,7 @@ with_them do it 'returns correct max_slice' do - expect(subject.get_max_slices(index_name: index_name)).to eq(result) + expect(migration.get_max_slices(index_name: index_name)).to eq(result) end end end