From b362191b9e7296eff89b5ff3ca6d72556f7d2157 Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Wed, 5 Nov 2025 18:21:23 +0100 Subject: [PATCH 1/6] Adds BackgroundOperation::OrchestratorWorker MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/211385 Changelog: added # Conflicts: # lib/gitlab/database/background_operation/common_worker.rb --- app/workers/all_queues.yml | 10 ++ .../base_scheduler_worker.rb | 20 ++-- .../orchestrator_worker.rb | 99 +++++++++++++++++++ config/sidekiq_queues.yml | 2 + .../background_operation/common_worker.rb | 11 ++- 5 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 app/workers/database/background_operation/orchestrator_worker.rb diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index ccba1a4fc4b0f7..52cd9fcf6cf3d9 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -113,6 +113,16 @@ :idempotent: true :tags: [] :queue_namespace: :auto_merge +- :name: background_operations:database_background_operation_orchestrator + :worker_name: Database::BackgroundOperation::OrchestratorWorker + :feature_category: :database + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: :background_operations - :name: batched_background_migrations:database_batched_background_migration_ci_execution :worker_name: Database::BatchedBackgroundMigration::CiExecutionWorker :feature_category: :database diff --git a/app/workers/database/background_operation/base_scheduler_worker.rb b/app/workers/database/background_operation/base_scheduler_worker.rb index efbf0a811ac2d8..597ca11d86a18d 100644 --- a/app/workers/database/background_operation/base_scheduler_worker.rb +++ b/app/workers/database/background_operation/base_scheduler_worker.rb @@ -11,10 +11,8 @@ class BaseSchedulerWorker data_consistency :sticky feature_category :database - MAX_RUNNING_OPERATIONS = 2 - def self.schedule_feature_flag_name - # TODO; Add a FF in https://gitlab.com/gitlab-org/gitlab/-/issues/577666 + :schedule_background_operations end def perform @@ -30,21 +28,17 @@ def perform private def queueable_workers - # Orchestrator worker will be created in https://gitlab.com/gitlab-org/gitlab/-/issues/578058 - # Which will get `MAX_RUNNING_OPERATIONS` from ApplicationSettings, similar to BBM. - - self.class.worker_class.schedulable_workers(MAX_RUNNING_OPERATIONS).to_a + self.class.worker_class.schedulable_workers(OrchestratorWorker.max_running_jobs).to_a end def queue_workers_for_execution(workers) - # Orchestrator worker will be created in https://gitlab.com/gitlab-org/gitlab/-/issues/578058 - # Which will uncomment below lines + return unless workers.present? - # return unless workers.present? - - # jobs_arguments = workers.map { |worker| [tracking_database.to_s, worker.id] } + jobs_arguments = workers.map do |worker| + [worker.class.name, worker.id, worker.partition, tracking_database.to_s] + end - # orchestrator_worker_class.perform_with_capacity(jobs_arguments) + OrchestratorWorker.perform_with_capacity(jobs_arguments) end end end diff --git a/app/workers/database/background_operation/orchestrator_worker.rb b/app/workers/database/background_operation/orchestrator_worker.rb new file mode 100644 index 00000000000000..9420f855f09231 --- /dev/null +++ b/app/workers/database/background_operation/orchestrator_worker.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module Database # rubocop:disable Gitlab/BoundedContexts -- Database Framework + module BackgroundOperation + class OrchestratorWorker + include ExclusiveLeaseGuard + include Gitlab::Utils::StrongMemoize + include ApplicationWorker + include LimitedCapacity::Worker + + INTERVAL_VARIANCE = 5.seconds.freeze + LEASE_TIMEOUT_MULTIPLIER = 3 + MAX_RUNNING_JOBS = 4 + + idempotent! + data_consistency :sticky + feature_category :database + prefer_calling_context_feature_category true + queue_namespace :background_operations + + class << self + # We have to override this one, as we want + # arguments passed as is, and not duplicated + def perform_with_capacity(args) + worker = new + worker.remove_failed_jobs + + bulk_perform_async(args) + end + + def max_running_jobs + Gitlab::CurrentSettings.database_frameworks_settings[:background_operations_max_jobs] + end + end + + def perform_work(worker_class, worker_partition, worker_id, database_name) + self.database_name = database_name + self.worker_class = worker_class + + Gitlab::Database::SharedModel.using_connection(base_model.connection) do + self.worker = find_worker(worker_partition, worker_id) + + break unless worker.present? + + try_obtain_lease do + run_operation_job if runnable_worker? + end + end + end + + def remaining_work_count(*_args) + 0 # the cron worker is the only source of new jobs + end + + def max_running_jobs + MAX_RUNNING_JOBS + end + + private + + attr_accessor :database_name, :worker_class, :worker + + def base_model + Gitlab::Database.database_base_models[database_name] + end + strong_memoize_attr(:base_model) + + # rubocop:disable CodeReuse/ActiveRecord -- Doesn't have to be a class method on the model + def find_worker(partition, id) + worker_class.executable.where(partition: partition, id: id).first + end + # rubocop:enable CodeReuse/ActiveRecord + + def runnable_worker? + worker.interval_elapsed?(variance: INTERVAL_VARIANCE) + end + + def lease_key + [ + database_name, + worker.class.name.underscore, + worker.table_name, + worker.id + ].join(':') + end + strong_memoize_attr(:lease_key) + + def lease_timeout + worker.interval * LEASE_TIMEOUT_MULTIPLIER + end + + def run_operation_job + Gitlab::Database::BackgroundOperation::Runner + .new(connection: base_model.connection) + .run_operation_job(worker) + end + end + end +end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 92c603967a254c..1a651de4c7f19e 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -129,6 +129,8 @@ - 2 - - auto_merge - 3 +- - background_operations + - 1 - - batched_background_migrations - 1 - - batched_git_ref_updates_project_cleanup diff --git a/lib/gitlab/database/background_operation/common_worker.rb b/lib/gitlab/database/background_operation/common_worker.rb index 90906c1c084877..fdf7d78b2f60f3 100644 --- a/lib/gitlab/database/background_operation/common_worker.rb +++ b/lib/gitlab/database/background_operation/common_worker.rb @@ -93,11 +93,11 @@ module CommonWorker state :failed, value: 4 event :finish do - transition [:paused, :finished, :active, :finalizing] => :finished + transition [:paused, :finished, :active] => :finished end event :failure do - transition [:failed, :finalizing, :active] => :failed + transition [:failed, :active] => :failed end event :hold do @@ -150,6 +150,13 @@ def next_min_cursor last_job&.max_cursor || min_cursor end + def interval_elapsed?(variance: 0) + return true unless last_job + + interval_with_variance = interval - variance + last_job.created_at <= Time.current - interval_with_variance + end + def health_context Gitlab::Database::HealthStatus::Context.new( self, -- GitLab From 46eb0f19ba47752908ef723e6eb998178bfeca47 Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Fri, 7 Nov 2025 23:12:25 +0100 Subject: [PATCH 2/6] Adds database_frameworks_settings to Application Setting With only background_operations_max_jobs in it now. We can then add/move general settings related to DBF in here. # Conflicts: # spec/models/application_setting_spec.rb --- app/helpers/application_settings_helper.rb | 3 ++- app/models/application_setting.rb | 5 +++++ .../application_setting_implementation.rb | 3 ++- ..._setting_database_frameworks_settings.json | 14 ++++++++++++ .../orchestrator_worker.rb | 5 ++--- .../database_frameworks_settings.yml | 12 ++++++++++ ...eworks_settings_to_application_settings.rb | 16 ++++++++++++++ ...hash_constraint_to_application_settings.rb | 21 ++++++++++++++++++ db/schema_migrations/20251106223701 | 1 + db/schema_migrations/20251106223714 | 1 + db/structure.sql | 5 +++++ spec/models/application_setting_spec.rb | 22 +++++++++++++++++++ 12 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 app/validators/json_schemas/application_setting_database_frameworks_settings.json create mode 100644 config/application_setting_columns/database_frameworks_settings.yml create mode 100644 db/migrate/20251106223701_add_database_frameworks_settings_to_application_settings.rb create mode 100644 db/migrate/20251106223714_add_database_frameworks_settings_hash_constraint_to_application_settings.rb create mode 100644 db/schema_migrations/20251106223701 create mode 100644 db/schema_migrations/20251106223714 diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 03fa486055bf40..ba892280df2033 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -647,7 +647,8 @@ def visible_attributes :resource_usage_limits, :runner_jobs_request_api_limit, :runner_jobs_patch_trace_api_limit, - :runner_jobs_endpoints_api_limit + :runner_jobs_endpoints_api_limit, + :background_operations_max_jobs ].tap do |settings| unless Gitlab.com? settings << :deactivate_dormant_users diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index ca5639656b833a..80d555c4dcaae9 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -932,6 +932,11 @@ def self.kroki_formats_attributes validates :database_reindexing, json_schema: { filename: "application_setting_database_reindexing" } + jsonb_accessor :database_frameworks_settings, + background_operations_max_jobs: [:integer, { default: 10 }] + + validates :database_frameworks_settings, json_schema: { filename: "application_setting_database_frameworks_settings" } + attr_encrypted :external_auth_client_key, encryption_options_base_32_aes_256_gcm attr_encrypted :external_auth_client_key_pass, encryption_options_base_32_aes_256_gcm attr_encrypted :lets_encrypt_private_key, encryption_options_base_32_aes_256_gcm diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 312705735429a2..6320db1dff9750 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -359,7 +359,8 @@ def defaults # rubocop:disable Metrics/AbcSize disable_invite_members: false, enforce_pipl_compliance: false, model_prompt_cache_enabled: true, - lock_model_prompt_cache_enabled: false + lock_model_prompt_cache_enabled: false, + background_operations_max_jobs: 10 }.tap do |hsh| hsh.merge!(non_production_defaults) unless Rails.env.production? end diff --git a/app/validators/json_schemas/application_setting_database_frameworks_settings.json b/app/validators/json_schemas/application_setting_database_frameworks_settings.json new file mode 100644 index 00000000000000..4145fbe08e1ee7 --- /dev/null +++ b/app/validators/json_schemas/application_setting_database_frameworks_settings.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Holds database frameworks settings", + "type": "object", + "additionalProperties": false, + "properties": { + "background_operations_max_jobs": { + "type": "integer", + "minimum": 1, + "default": 10, + "description": "Maximum number of background operations jobs that can run concurrently" + } + } +} diff --git a/app/workers/database/background_operation/orchestrator_worker.rb b/app/workers/database/background_operation/orchestrator_worker.rb index 9420f855f09231..c9482abfa67a0b 100644 --- a/app/workers/database/background_operation/orchestrator_worker.rb +++ b/app/workers/database/background_operation/orchestrator_worker.rb @@ -10,7 +10,6 @@ class OrchestratorWorker INTERVAL_VARIANCE = 5.seconds.freeze LEASE_TIMEOUT_MULTIPLIER = 3 - MAX_RUNNING_JOBS = 4 idempotent! data_consistency :sticky @@ -29,7 +28,7 @@ def perform_with_capacity(args) end def max_running_jobs - Gitlab::CurrentSettings.database_frameworks_settings[:background_operations_max_jobs] + Gitlab::CurrentSettings.background_operations_max_jobs end end @@ -53,7 +52,7 @@ def remaining_work_count(*_args) end def max_running_jobs - MAX_RUNNING_JOBS + self.class.max_running_jobs end private diff --git a/config/application_setting_columns/database_frameworks_settings.yml b/config/application_setting_columns/database_frameworks_settings.yml new file mode 100644 index 00000000000000..6b0ddaf57a6395 --- /dev/null +++ b/config/application_setting_columns/database_frameworks_settings.yml @@ -0,0 +1,12 @@ +--- +api_type: +attr: database_frameworks_settings +clusterwide: true +column: database_frameworks_settings +db_type: jsonb +default: "'{}'::jsonb" +description: Database frameworks settings +encrypted: false +gitlab_com_different_than_default: false +jihu: false +not_null: true diff --git a/db/migrate/20251106223701_add_database_frameworks_settings_to_application_settings.rb b/db/migrate/20251106223701_add_database_frameworks_settings_to_application_settings.rb new file mode 100644 index 00000000000000..49cf9a06724460 --- /dev/null +++ b/db/migrate/20251106223701_add_database_frameworks_settings_to_application_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddDatabaseFrameworksSettingsToApplicationSettings < Gitlab::Database::Migration[2.3] + milestone '18.6' + + def change + add_column( + :application_settings, + :database_frameworks_settings, + :jsonb, + default: {}, + null: false, + if_not_exists: true + ) + end +end diff --git a/db/migrate/20251106223714_add_database_frameworks_settings_hash_constraint_to_application_settings.rb b/db/migrate/20251106223714_add_database_frameworks_settings_hash_constraint_to_application_settings.rb new file mode 100644 index 00000000000000..b1d1c6ec4d4644 --- /dev/null +++ b/db/migrate/20251106223714_add_database_frameworks_settings_hash_constraint_to_application_settings.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddDatabaseFrameworksSettingsHashConstraintToApplicationSettings < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + + milestone '18.6' + + CONSTRAINT_NAME = 'check_application_settings_database_frameworks_settings_is_hash' + + def up + add_check_constraint( + :application_settings, + "(jsonb_typeof(database_frameworks_settings) = 'object')", + CONSTRAINT_NAME + ) + end + + def down + remove_check_constraint(:application_settings, CONSTRAINT_NAME) + end +end diff --git a/db/schema_migrations/20251106223701 b/db/schema_migrations/20251106223701 new file mode 100644 index 00000000000000..e300fbd4c0820f --- /dev/null +++ b/db/schema_migrations/20251106223701 @@ -0,0 +1 @@ +bc011af751c85d010c4ad739ed2a186b19f7fd81c66df952edbbbbb68ae0ad5a \ No newline at end of file diff --git a/db/schema_migrations/20251106223714 b/db/schema_migrations/20251106223714 new file mode 100644 index 00000000000000..81759b22a9a6d6 --- /dev/null +++ b/db/schema_migrations/20251106223714 @@ -0,0 +1 @@ +9d8c55a47e8beb19ec02fe6080ac54b6f7058de740a5437df03a19df1bd60e3f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a777a5a801bd28..a6842c4f768fca 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11757,6 +11757,7 @@ CREATE TABLE application_settings ( lock_duo_remote_flows_enabled boolean DEFAULT false NOT NULL, terraform_state_settings jsonb DEFAULT '{}'::jsonb NOT NULL, namespace_deletion_settings jsonb DEFAULT '{}'::jsonb NOT NULL, + database_frameworks_settings jsonb DEFAULT '{}'::jsonb NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), @@ -11810,6 +11811,7 @@ CREATE TABLE application_settings ( CONSTRAINT check_application_settings_clickhouse_is_hash CHECK ((jsonb_typeof(clickhouse) = 'object'::text)), CONSTRAINT check_application_settings_cluster_agents_is_hash CHECK ((jsonb_typeof(cluster_agents) = 'object'::text)), CONSTRAINT check_application_settings_code_creation_is_hash CHECK ((jsonb_typeof(code_creation) = 'object'::text)), + CONSTRAINT check_application_settings_database_frameworks_settings_is_hash CHECK ((jsonb_typeof(database_frameworks_settings) = 'object'::text)), CONSTRAINT check_application_settings_database_reindexing_is_hash CHECK ((jsonb_typeof(database_reindexing) = 'object'::text)), CONSTRAINT check_application_settings_duo_chat_is_hash CHECK ((jsonb_typeof(duo_chat) = 'object'::text)), CONSTRAINT check_application_settings_duo_workflow_is_hash CHECK ((jsonb_typeof(duo_workflow) = 'object'::text)), @@ -34169,6 +34171,9 @@ ALTER TABLE ONLY dora_performance_scores ALTER TABLE ONLY draft_notes ADD CONSTRAINT draft_notes_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dummies + ADD CONSTRAINT dummies_pkey PRIMARY KEY (id); + ALTER TABLE ONLY duo_workflows_checkpoint_writes ADD CONSTRAINT duo_workflows_checkpoint_writes_pkey PRIMARY KEY (id); diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 5ed3c37fa7816f..20ea3341543cc8 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -40,6 +40,7 @@ authorized_keys_enabled: true, autocomplete_users_limit: 300, autocomplete_users_unauthenticated_limit: 100, + background_operations_max_jobs: 10, bulk_import_concurrent_pipeline_batch_limit: 25, bulk_import_enabled: false, bulk_import_max_download_file_size: 5120, @@ -2702,6 +2703,27 @@ def expect_invalid it { expect(setting.ci_delete_pipelines_in_seconds_limit_human_readable_long).to eq('1 year') } end + describe '#database_frameworks_settings' do + let(:valid_settings) do + { + background_operations_max_jobs: 10 + } + end + + # valid json + it { is_expected.to allow_value({}).for(:database_frameworks_settings) } + it { is_expected.to allow_value(valid_settings).for(:database_frameworks_settings) } + + # invalid json + it { is_expected.not_to allow_value({ background_operations_max_jobs: 0 }).for(:database_frameworks_settings) } + it { is_expected.not_to allow_value({ background_operations_max_jobs: -1 }).for(:database_frameworks_settings) } + it { is_expected.not_to allow_value({ invalid_key: 10 }).for(:database_frameworks_settings) } + + it 'sets the correct default value' do + expect(setting.background_operations_max_jobs).to eq(10) + end + end + describe '#custom_default_search_scope_set?' do context 'when it is set to empty string' do it 'returns false' do -- GitLab From 46828b9dfc32c8e4b55de0c90dc9931ccc0553a1 Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Mon, 10 Nov 2025 11:48:48 +0100 Subject: [PATCH 3/6] Adds the FF schedule_background_operations --- .../ops/schedule_background_operations.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 config/feature_flags/ops/schedule_background_operations.yml diff --git a/config/feature_flags/ops/schedule_background_operations.yml b/config/feature_flags/ops/schedule_background_operations.yml new file mode 100644 index 00000000000000..1be7a0d04960aa --- /dev/null +++ b/config/feature_flags/ops/schedule_background_operations.yml @@ -0,0 +1,10 @@ +--- +name: schedule_background_operations +description: Controls scheduling of Background operations +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/578058 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/211385 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/580088 +milestone: '18.6' +group: group::database frameworks +type: ops +default_enabled: false -- GitLab From 132adac84b842b9d0e1d78ab4f84380688fd4557 Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Mon, 10 Nov 2025 12:09:24 +0100 Subject: [PATCH 4/6] Removes unwanted entry from schema --- db/schema_migrations/20251106223701 | 2 +- db/schema_migrations/20251106223714 | 2 +- db/structure.sql | 3 --- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/db/schema_migrations/20251106223701 b/db/schema_migrations/20251106223701 index e300fbd4c0820f..f7d07bb2867f70 100644 --- a/db/schema_migrations/20251106223701 +++ b/db/schema_migrations/20251106223701 @@ -1 +1 @@ -bc011af751c85d010c4ad739ed2a186b19f7fd81c66df952edbbbbb68ae0ad5a \ No newline at end of file +8839f6f80ef917dd812ee46c91be577a18bca6b0f42d6677c5dbcca08cb174b0 \ No newline at end of file diff --git a/db/schema_migrations/20251106223714 b/db/schema_migrations/20251106223714 index 81759b22a9a6d6..cb454808173dc7 100644 --- a/db/schema_migrations/20251106223714 +++ b/db/schema_migrations/20251106223714 @@ -1 +1 @@ -9d8c55a47e8beb19ec02fe6080ac54b6f7058de740a5437df03a19df1bd60e3f \ No newline at end of file +6b816ce89a6bc930ebbf17ef59b2dad466f5ef2b4403fc272675c119a3812b79 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a6842c4f768fca..c7c34766d7cd33 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -34171,9 +34171,6 @@ ALTER TABLE ONLY dora_performance_scores ALTER TABLE ONLY draft_notes ADD CONSTRAINT draft_notes_pkey PRIMARY KEY (id); -ALTER TABLE ONLY dummies - ADD CONSTRAINT dummies_pkey PRIMARY KEY (id); - ALTER TABLE ONLY duo_workflows_checkpoint_writes ADD CONSTRAINT duo_workflows_checkpoint_writes_pkey PRIMARY KEY (id); -- GitLab From 25e602d548bd0827a5c7bb9983db02aae7ac883b Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Fri, 14 Nov 2025 08:46:59 +0100 Subject: [PATCH 5/6] Adds specs --- app/workers/all_queues.yml | 2 +- .../base_scheduler_worker.rb | 2 +- .../orchestrator_worker.rb | 11 +- .../orchestrator_worker_spec.rb | 194 ++++++++++++++++++ spec/workers/every_sidekiq_worker_spec.rb | 1 + 5 files changed, 205 insertions(+), 5 deletions(-) create mode 100644 spec/workers/database/background_operation/orchestrator_worker_spec.rb diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 52cd9fcf6cf3d9..3b3d90184fb43e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -120,7 +120,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: true + :idempotent: false :tags: [] :queue_namespace: :background_operations - :name: batched_background_migrations:database_batched_background_migration_ci_execution diff --git a/app/workers/database/background_operation/base_scheduler_worker.rb b/app/workers/database/background_operation/base_scheduler_worker.rb index 597ca11d86a18d..bedf7b762c1bc1 100644 --- a/app/workers/database/background_operation/base_scheduler_worker.rb +++ b/app/workers/database/background_operation/base_scheduler_worker.rb @@ -35,7 +35,7 @@ def queue_workers_for_execution(workers) return unless workers.present? jobs_arguments = workers.map do |worker| - [worker.class.name, worker.id, worker.partition, tracking_database.to_s] + [worker.class.name, worker.partition, worker.id, tracking_database.to_s] end OrchestratorWorker.perform_with_capacity(jobs_arguments) diff --git a/app/workers/database/background_operation/orchestrator_worker.rb b/app/workers/database/background_operation/orchestrator_worker.rb index c9482abfa67a0b..4684fbcd3bf3d3 100644 --- a/app/workers/database/background_operation/orchestrator_worker.rb +++ b/app/workers/database/background_operation/orchestrator_worker.rb @@ -2,7 +2,7 @@ module Database # rubocop:disable Gitlab/BoundedContexts -- Database Framework module BackgroundOperation - class OrchestratorWorker + class OrchestratorWorker # rubocop:disable Scalability/IdempotentWorker -- A LimitedCapacity::Worker include ExclusiveLeaseGuard include Gitlab::Utils::StrongMemoize include ApplicationWorker @@ -11,7 +11,6 @@ class OrchestratorWorker INTERVAL_VARIANCE = 5.seconds.freeze LEASE_TIMEOUT_MULTIPLIER = 3 - idempotent! data_consistency :sticky feature_category :database prefer_calling_context_feature_category true @@ -36,6 +35,8 @@ def perform_work(worker_class, worker_partition, worker_id, database_name) self.database_name = database_name self.worker_class = worker_class + return if shares_db_config? + Gitlab::Database::SharedModel.using_connection(base_model.connection) do self.worker = find_worker(worker_partition, worker_id) @@ -66,7 +67,7 @@ def base_model # rubocop:disable CodeReuse/ActiveRecord -- Doesn't have to be a class method on the model def find_worker(partition, id) - worker_class.executable.where(partition: partition, id: id).first + worker_class.constantize.executable.where(partition: partition, id: id).first end # rubocop:enable CodeReuse/ActiveRecord @@ -93,6 +94,10 @@ def run_operation_job .new(connection: base_model.connection) .run_operation_job(worker) end + + def shares_db_config? + Gitlab::Database.db_config_share_with(base_model.connection_db_config).present? + end end end end diff --git a/spec/workers/database/background_operation/orchestrator_worker_spec.rb b/spec/workers/database/background_operation/orchestrator_worker_spec.rb new file mode 100644 index 00000000000000..acaaa8b56e5079 --- /dev/null +++ b/spec/workers/database/background_operation/orchestrator_worker_spec.rb @@ -0,0 +1,194 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Database::BackgroundOperation::OrchestratorWorker, :clean_gitlab_redis_shared_state, + feature_category: :database do + include ExclusiveLeaseHelpers + + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + + it 'is a limited capacity worker' do + expect(described_class.new).to be_a(LimitedCapacity::Worker) + end + + describe 'defining the job attributes' do + it 'defines the data_consistency as always' do + expect(described_class.get_data_consistency_per_database.values.uniq).to eq([:sticky]) + end + + it 'defines the feature_category as database' do + expect(described_class.get_feature_category).to eq(:database) + end + + it 'defines the idempotency as false' do + expect(described_class).not_to be_idempotent + end + + it 'does not retry failed jobs' do + expect(described_class.sidekiq_options['retry']).to eq(0) + end + + it 'does not deduplicate jobs' do + expect(described_class.get_deduplicate_strategy).to eq(:none) + end + + it 'defines the queue namespace' do + expect(described_class.queue_namespace).to eq('background_operations') + end + end + + describe '.perform_with_capacity' do + it 'enqueues jobs without modifying provided arguments' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:remove_failed_jobs) + end + + args = [['Gitlab::Database::BackgroundOperation::Worker', 1, 1, 'main']] + + expect(described_class).to receive(:bulk_perform_async).with(args) + + described_class.perform_with_capacity(args) + end + end + + describe '.max_running_jobs' do + it 'returns the default 10' do + expect(described_class.max_running_jobs).to eq(10) + end + + it 'returns updated background_operations_max_jobs from application setting' do + stub_application_setting(background_operations_max_jobs: 3) + + expect(described_class.max_running_jobs).to eq(3) + end + end + + describe '#remaining_work_count' do + it 'returns 0' do + expect(described_class.new.remaining_work_count).to eq(0) + end + end + + describe '#perform_work' do + let(:worker_class) { 'Gitlab::Database::BackgroundOperation::Worker' } + let(:partition) { 1 } + let(:database_name) { Gitlab::Database::MAIN_DATABASE_NAME.to_sym } + let(:base_model) { Gitlab::Database.database_base_models[database_name] } + let(:table_name) { :events } + let(:job_interval) { 5.minutes } + let(:lease_timeout) { job_interval * described_class::LEASE_TIMEOUT_MULTIPLIER } + let(:interval_variance) { described_class::INTERVAL_VARIANCE } + let(:worker_id) { worker.attributes['id'] } + let(:orchestrator) { described_class.new } + + subject(:perform_work) { orchestrator.perform_work(worker_class, partition, worker_id, database_name) } + + context 'when the provided database is sharing config' do + let(:worker_id) { 123 } + let(:database_name) { 'ci' } + + before do + skip_if_multiple_databases_not_setup(:ci) + end + + it 'does nothing' do + ci_model = Gitlab::Database.database_base_models['ci'] + expect(Gitlab::Database).to receive(:db_config_share_with) + .with(ci_model.connection_db_config).and_return('main') + + expect(orchestrator).not_to receive(:find_worker) + expect(orchestrator).not_to receive(:run_operation_job) + + perform_work + end + end + + context 'when operation does not exist' do + let(:worker_id) { non_existing_record_id } + + it 'does nothing' do + expect(orchestrator).not_to receive(:run_operation_job) + + perform_work + end + end + + context 'when operation exist' do + let(:worker) do + create(:background_operation_worker, :queued, table_name: table_name, interval: job_interval) + end + + context 'when the operation is no longer active' do + it 'does not run the operation' do + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield + + worker.hold! + + expect(orchestrator).not_to receive(:run_operation_job) + + perform_work + end + end + + context 'when worker is not runnable (elapsed interval)' do + it 'does not run the operation' do + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield + + expect(orchestrator).to receive(:runnable_worker?).and_return(false) + + expect(orchestrator).not_to receive(:run_operation_job) + + perform_work + end + end + + context 'when the operation is still active and the interval has elapsed' do + let(:lease_key) do + [ + database_name, + worker.class.name.underscore, + worker.table_name, + worker.id + ].join(':') + end + + context 'when can not obtain lease on the table name' do + it 'does nothing' do + stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) + + expect(orchestrator).not_to receive(:run_operation_job) + + perform_work + end + end + + it 'always cleans up the exclusive lease' do + expect_to_obtain_exclusive_lease(lease_key, 'uuid-table-name', timeout: lease_timeout) + expect_to_cancel_exclusive_lease(lease_key, 'uuid-table-name') + + expect(orchestrator).to receive(:run_operation_job).and_raise(RuntimeError, 'I broke') + + expect { perform_work }.to raise_error(RuntimeError, 'I broke') + end + + it 'runs the operation' do + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield + + expect_next_instance_of(Gitlab::Database::BackgroundOperation::Runner) do |instance| + expect(instance).to receive(:run_operation_job).with(worker) + end + + expect_to_obtain_exclusive_lease(lease_key, 'uuid-table-name', timeout: lease_timeout) + expect_to_cancel_exclusive_lease(lease_key, 'uuid-table-name') + + expect(orchestrator).to receive(:run_operation_job).and_call_original + + perform_work + end + end + end + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index d3b0932c4df07e..86772845d5d63c 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -208,6 +208,7 @@ 'Database::BatchedBackgroundMigration::CiExecutionWorker' => 0, 'Database::BatchedBackgroundMigration::MainExecutionWorker' => 0, 'Database::BatchedBackgroundMigration::SecExecutionWorker' => 0, + 'Database::BackgroundOperation::OrchestratorWorker' => 0, 'Database::CollationCheckerWorker' => false, 'Database::SchemaCheckerWorker' => false, 'DeleteDiffFilesWorker' => 3, -- GitLab From 44914129bc33ddfb1f9ee28f5d64175fe9657304 Mon Sep 17 00:00:00 2001 From: Prabakaran Murugesan Date: Sat, 15 Nov 2025 19:02:03 +0100 Subject: [PATCH 6/6] Makes FF non-dynamic to fix specs --- .../database/background_operation/base_scheduler_worker.rb | 4 ++-- app/workers/database/background_work_schedulable.rb | 4 +--- .../batched_background_migration/single_database_worker.rb | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/workers/database/background_operation/base_scheduler_worker.rb b/app/workers/database/background_operation/base_scheduler_worker.rb index bedf7b762c1bc1..da3240e04c735f 100644 --- a/app/workers/database/background_operation/base_scheduler_worker.rb +++ b/app/workers/database/background_operation/base_scheduler_worker.rb @@ -11,8 +11,8 @@ class BaseSchedulerWorker data_consistency :sticky feature_category :database - def self.schedule_feature_flag_name - :schedule_background_operations + def self.scheduler_feature_flag_enabled? + Feature.enabled?(:schedule_background_operations, type: :ops) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Global FF end def perform diff --git a/app/workers/database/background_work_schedulable.rb b/app/workers/database/background_work_schedulable.rb index 947cedb6bfd4b9..2448a03372465c 100644 --- a/app/workers/database/background_work_schedulable.rb +++ b/app/workers/database/background_work_schedulable.rb @@ -6,14 +6,12 @@ module BackgroundWorkSchedulable class_methods do # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Global FF - # rubocop:disable Gitlab/FeatureFlagKeyDynamic -- It's different for each sub-class. def enabled? return false if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) - schedule_feature_flag_name.present? ? Feature.enabled?(schedule_feature_flag_name, type: :ops) : true + scheduler_feature_flag_enabled? end # rubocop:enable Gitlab/FeatureFlagWithoutActor - # rubocop:enable Gitlab/FeatureFlagKeyDynamic end included do diff --git a/app/workers/database/batched_background_migration/single_database_worker.rb b/app/workers/database/batched_background_migration/single_database_worker.rb index 892ddc84986ea7..f562307478c492 100644 --- a/app/workers/database/batched_background_migration/single_database_worker.rb +++ b/app/workers/database/batched_background_migration/single_database_worker.rb @@ -10,8 +10,8 @@ module SingleDatabaseWorker include Database::BackgroundWorkSchedulable class_methods do - def schedule_feature_flag_name - :execute_batched_migrations_on_schedule + def scheduler_feature_flag_enabled? + Feature.enabled?(:execute_batched_migrations_on_schedule, type: :ops) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Global FF end end -- GitLab