diff options
Diffstat (limited to 'lib/gitlab/database')
24 files changed, 465 insertions, 220 deletions
diff --git a/lib/gitlab/database/background_migration/batch_metrics.rb b/lib/gitlab/database/background_migration/batch_metrics.rb index 3e6d7ac3c9f..14fe0c14c24 100644 --- a/lib/gitlab/database/background_migration/batch_metrics.rb +++ b/lib/gitlab/database/background_migration/batch_metrics.rb @@ -5,17 +5,24 @@ module Gitlab module BackgroundMigration class BatchMetrics attr_reader :timings + attr_reader :affected_rows def initialize @timings = {} + @affected_rows = {} end - def time_operation(label) + def time_operation(label, &blk) + instrument_operation(label, instrument_affected_rows: false, &blk) + end + + def instrument_operation(label, instrument_affected_rows: true) start_time = monotonic_time - yield + count = yield timings_for_label(label) << monotonic_time - start_time + affected_rows_for_label(label) << count if instrument_affected_rows && count.is_a?(Integer) end private @@ -24,6 +31,10 @@ module Gitlab timings[label] ||= [] end + def affected_rows_for_label(label) + affected_rows[label] ||= [] + end + def monotonic_time Gitlab::Metrics::System.monotonic_time end diff --git a/lib/gitlab/database/background_migration/batched_job.rb b/lib/gitlab/database/background_migration/batched_job.rb index f3160679d64..ebc3ee240bd 100644 --- a/lib/gitlab/database/background_migration/batched_job.rb +++ b/lib/gitlab/database/background_migration/batched_job.rb @@ -25,6 +25,7 @@ module Gitlab scope :except_succeeded, -> { without_status(:succeeded) } scope :successful_in_execution_order, -> { where.not(finished_at: nil).with_status(:succeeded).order(:finished_at) } scope :with_preloads, -> { preload(:batched_migration) } + scope :created_since, ->(date_time) { where('created_at >= ?', date_time) } state_machine :status, initial: :pending do state :pending, value: 0 @@ -62,7 +63,13 @@ module Gitlab job.split_and_retry! if job.can_split?(exception) rescue SplitAndRetryError => error - Gitlab::AppLogger.error(message: error.message, batched_job_id: job.id) + Gitlab::AppLogger.error( + message: error.message, + batched_job_id: job.id, + batched_migration_id: job.batched_migration.id, + job_class_name: job.migration_job_class_name, + job_arguments: job.migration_job_arguments + ) end after_transition do |job, transition| @@ -72,13 +79,23 @@ module Gitlab job.batched_job_transition_logs.create(previous_status: transition.from, next_status: transition.to, exception_class: exception&.class, exception_message: exception&.message) - Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id) if exception - - Gitlab::AppLogger.info(message: 'BatchedJob transition', batched_job_id: job.id, previous_state: transition.from_name, new_state: transition.to_name) + Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id, job_class_name: job.migration_job_class_name, job_arguments: job.migration_job_arguments) if exception + + Gitlab::AppLogger.info( + message: 'BatchedJob transition', + batched_job_id: job.id, + previous_state: transition.from_name, + new_state: transition.to_name, + batched_migration_id: job.batched_migration.id, + job_class_name: job.migration_job_class_name, + job_arguments: job.migration_job_arguments, + exception_class: exception&.class, + exception_message: exception&.message + ) end end - delegate :job_class, :table_name, :column_name, :job_arguments, + delegate :job_class, :table_name, :column_name, :job_arguments, :job_class_name, to: :batched_migration, prefix: :migration attribute :pause_ms, :integer, default: 100 diff --git a/lib/gitlab/database/background_migration/batched_migration.rb b/lib/gitlab/database/background_migration/batched_migration.rb index 65c15795de6..d94bf060d05 100644 --- a/lib/gitlab/database/background_migration/batched_migration.rb +++ b/lib/gitlab/database/background_migration/batched_migration.rb @@ -6,6 +6,8 @@ module Gitlab class BatchedMigration < SharedModel JOB_CLASS_MODULE = 'Gitlab::BackgroundMigration' BATCH_CLASS_MODULE = "#{JOB_CLASS_MODULE}::BatchingStrategies" + MAXIMUM_FAILED_RATIO = 0.5 + MINIMUM_JOBS = 50 self.table_name = :batched_background_migrations @@ -21,28 +23,60 @@ module Gitlab validate :validate_batched_jobs_status, if: -> { status_changed? && finished? } scope :queue_order, -> { order(id: :asc) } - scope :queued, -> { where(status: [:active, :paused]) } + scope :queued, -> { with_statuses(:active, :paused) } + + # on_hold_until is a temporary runtime status which puts execution "on hold" + scope :executable, -> { with_status(:active).where('on_hold_until IS NULL OR on_hold_until < NOW()') } + scope :for_configuration, ->(job_class_name, table_name, column_name, job_arguments) do where(job_class_name: job_class_name, table_name: table_name, column_name: column_name) .where("job_arguments = ?", job_arguments.to_json) # rubocop:disable Rails/WhereEquals end - enum status: { - paused: 0, - active: 1, - finished: 3, - failed: 4, - finalizing: 5 - } + state_machine :status, initial: :paused do + state :paused, value: 0 + state :active, value: 1 + state :finished, value: 3 + state :failed, value: 4 + state :finalizing, value: 5 + + event :pause do + transition any => :paused + end + + event :execute do + transition any => :active + end + + event :finish do + transition any => :finished + end + + event :failure do + transition any => :failed + end + + event :finalize do + transition any => :finalizing + end + + before_transition any => :active do |migration| + migration.started_at = Time.current if migration.respond_to?(:started_at) + end + end attribute :pause_ms, :integer, default: 100 + def self.valid_status + state_machine.states.map(&:name) + end + def self.find_for_configuration(job_class_name, table_name, column_name, job_arguments) for_configuration(job_class_name, table_name, column_name, job_arguments).first end def self.active_migration - active.queue_order.first + executable.queue_order.first end def self.successful_rows_counts(migrations) @@ -74,11 +108,23 @@ module Gitlab batched_jobs.with_status(:failed).each_batch(of: 100) do |batch| self.class.transaction do batch.lock.each(&:split_and_retry!) - self.active! + self.execute! end end - self.active! + self.execute! + end + + def should_stop? + return unless started_at + + total_jobs = batched_jobs.created_since(started_at).count + + return if total_jobs < MINIMUM_JOBS + + failed_jobs = batched_jobs.with_status(:failed).created_since(started_at).count + + failed_jobs.fdiv(total_jobs) > MAXIMUM_FAILED_RATIO end def next_min_value @@ -136,6 +182,10 @@ module Gitlab BatchOptimizer.new(self).optimize! end + def hold!(until_time: 10.minutes.from_now) + update!(on_hold_until: until_time) + end + private def validate_batched_jobs_status diff --git a/lib/gitlab/database/background_migration/batched_migration_runner.rb b/lib/gitlab/database/background_migration/batched_migration_runner.rb index 06cd40f1e06..59ff9a9744f 100644 --- a/lib/gitlab/database/background_migration/batched_migration_runner.rb +++ b/lib/gitlab/database/background_migration/batched_migration_runner.rb @@ -6,13 +6,13 @@ module Gitlab class BatchedMigrationRunner FailedToFinalize = Class.new(RuntimeError) - def self.finalize(job_class_name, table_name, column_name, job_arguments, connection: ApplicationRecord.connection) + def self.finalize(job_class_name, table_name, column_name, job_arguments, connection:) new(connection: connection).finalize(job_class_name, table_name, column_name, job_arguments) end - def initialize(migration_wrapper = BatchedMigrationWrapper.new, connection: ApplicationRecord.connection) - @migration_wrapper = migration_wrapper + def initialize(connection:, migration_wrapper: BatchedMigrationWrapper.new(connection: connection)) @connection = connection + @migration_wrapper = migration_wrapper end # Runs the next batched_job for a batched_background_migration. @@ -30,6 +30,7 @@ module Gitlab migration_wrapper.perform(next_batched_job) active_migration.optimize! + active_migration.failure! if next_batched_job.failed? && active_migration.should_stop? else finish_active_migration(active_migration) end @@ -67,7 +68,7 @@ module Gitlab elsif migration.finished? Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}" else - migration.finalizing! + migration.finalize! migration.batched_jobs.with_status(:pending).each { |job| migration_wrapper.perform(job) } run_migration_while(migration, :finalizing) @@ -78,7 +79,7 @@ module Gitlab private - attr_reader :migration_wrapper, :connection + attr_reader :connection, :migration_wrapper def find_or_create_next_batched_job(active_migration) if next_batch_range = find_next_batch_range(active_migration) @@ -118,14 +119,14 @@ module Gitlab return if active_migration.batched_jobs.active.exists? if active_migration.batched_jobs.with_status(:failed).exists? - active_migration.failed! + active_migration.failure! else - active_migration.finished! + active_migration.finish! end end def run_migration_while(migration, status) - while migration.status == status.to_s + while migration.status_name == status run_migration_job(migration) migration.reload_last_job diff --git a/lib/gitlab/database/background_migration/batched_migration_wrapper.rb b/lib/gitlab/database/background_migration/batched_migration_wrapper.rb index 057f856d859..ec68f401ca2 100644 --- a/lib/gitlab/database/background_migration/batched_migration_wrapper.rb +++ b/lib/gitlab/database/background_migration/batched_migration_wrapper.rb @@ -4,10 +4,9 @@ module Gitlab module Database module BackgroundMigration class BatchedMigrationWrapper - extend Gitlab::Utils::StrongMemoize - - def initialize(connection: ApplicationRecord.connection) + def initialize(connection:, metrics: PrometheusMetrics.new) @connection = connection + @metrics = metrics end # Wraps the execution of a batched_background_migration. @@ -28,12 +27,12 @@ module Gitlab raise ensure - track_prometheus_metrics(batch_tracking_record) + metrics.track(batch_tracking_record) end private - attr_reader :connection + attr_reader :connection, :metrics def start_tracking_execution(tracking_record) tracking_record.run! @@ -63,80 +62,6 @@ module Gitlab job_class.new end end - - def track_prometheus_metrics(tracking_record) - migration = tracking_record.batched_migration - base_labels = migration.prometheus_labels - - metric_for(:gauge_batch_size).set(base_labels, tracking_record.batch_size) - metric_for(:gauge_sub_batch_size).set(base_labels, tracking_record.sub_batch_size) - metric_for(:gauge_interval).set(base_labels, tracking_record.batched_migration.interval) - metric_for(:gauge_job_duration).set(base_labels, (tracking_record.finished_at - tracking_record.started_at).to_i) - metric_for(:counter_updated_tuples).increment(base_labels, tracking_record.batch_size) - metric_for(:gauge_migrated_tuples).set(base_labels, tracking_record.batched_migration.migrated_tuple_count) - metric_for(:gauge_total_tuple_count).set(base_labels, tracking_record.batched_migration.total_tuple_count) - metric_for(:gauge_last_update_time).set(base_labels, Time.current.to_i) - - if metrics = tracking_record.metrics - metrics['timings']&.each do |key, timings| - summary = metric_for(:histogram_timings) - labels = base_labels.merge(operation: key) - - timings.each do |timing| - summary.observe(labels, timing) - end - end - end - end - - def metric_for(name) - self.class.metrics[name] - end - - def self.metrics - strong_memoize(:metrics) do - { - gauge_batch_size: Gitlab::Metrics.gauge( - :batched_migration_job_batch_size, - 'Batch size for a batched migration job' - ), - gauge_sub_batch_size: Gitlab::Metrics.gauge( - :batched_migration_job_sub_batch_size, - 'Sub-batch size for a batched migration job' - ), - gauge_interval: Gitlab::Metrics.gauge( - :batched_migration_job_interval_seconds, - 'Interval for a batched migration job' - ), - gauge_job_duration: Gitlab::Metrics.gauge( - :batched_migration_job_duration_seconds, - 'Duration for a batched migration job' - ), - counter_updated_tuples: Gitlab::Metrics.counter( - :batched_migration_job_updated_tuples_total, - 'Number of tuples updated by batched migration job' - ), - gauge_migrated_tuples: Gitlab::Metrics.gauge( - :batched_migration_migrated_tuples_total, - 'Total number of tuples migrated by a batched migration' - ), - histogram_timings: Gitlab::Metrics.histogram( - :batched_migration_job_query_duration_seconds, - 'Query timings for a batched migration job', - {}, - [0.1, 0.25, 0.5, 1, 5].freeze - ), - gauge_total_tuple_count: Gitlab::Metrics.gauge( - :batched_migration_total_tuple_count, - 'Total tuple count the migration needs to touch' - ), - gauge_last_update_time: Gitlab::Metrics.gauge( - :batched_migration_last_update_time_seconds, - 'Unix epoch time in seconds' - ) - } - end - end end end end diff --git a/lib/gitlab/database/background_migration/prometheus_metrics.rb b/lib/gitlab/database/background_migration/prometheus_metrics.rb new file mode 100644 index 00000000000..ce1da4c59eb --- /dev/null +++ b/lib/gitlab/database/background_migration/prometheus_metrics.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module BackgroundMigration + class PrometheusMetrics + extend Gitlab::Utils::StrongMemoize + + QUERY_TIMING_BUCKETS = [0.1, 0.25, 0.5, 1, 5].freeze + + def track(job_record) + migration_record = job_record.batched_migration + base_labels = migration_record.prometheus_labels + + metric_for(:gauge_batch_size).set(base_labels, job_record.batch_size) + metric_for(:gauge_sub_batch_size).set(base_labels, job_record.sub_batch_size) + metric_for(:gauge_interval).set(base_labels, job_record.batched_migration.interval) + metric_for(:gauge_job_duration).set(base_labels, (job_record.finished_at - job_record.started_at).to_i) + metric_for(:counter_updated_tuples).increment(base_labels, job_record.batch_size) + metric_for(:gauge_migrated_tuples).set(base_labels, migration_record.migrated_tuple_count) + metric_for(:gauge_total_tuple_count).set(base_labels, migration_record.total_tuple_count) + metric_for(:gauge_last_update_time).set(base_labels, Time.current.to_i) + + track_timing_metrics(base_labels, job_record.metrics) + end + + def self.metrics + strong_memoize(:metrics) do + { + gauge_batch_size: Gitlab::Metrics.gauge( + :batched_migration_job_batch_size, + 'Batch size for a batched migration job' + ), + gauge_sub_batch_size: Gitlab::Metrics.gauge( + :batched_migration_job_sub_batch_size, + 'Sub-batch size for a batched migration job' + ), + gauge_interval: Gitlab::Metrics.gauge( + :batched_migration_job_interval_seconds, + 'Interval for a batched migration job' + ), + gauge_job_duration: Gitlab::Metrics.gauge( + :batched_migration_job_duration_seconds, + 'Duration for a batched migration job' + ), + counter_updated_tuples: Gitlab::Metrics.counter( + :batched_migration_job_updated_tuples_total, + 'Number of tuples updated by batched migration job' + ), + gauge_migrated_tuples: Gitlab::Metrics.gauge( + :batched_migration_migrated_tuples_total, + 'Total number of tuples migrated by a batched migration' + ), + histogram_timings: Gitlab::Metrics.histogram( + :batched_migration_job_query_duration_seconds, + 'Query timings for a batched migration job', + {}, + QUERY_TIMING_BUCKETS + ), + gauge_total_tuple_count: Gitlab::Metrics.gauge( + :batched_migration_total_tuple_count, + 'Total tuple count the migration needs to touch' + ), + gauge_last_update_time: Gitlab::Metrics.gauge( + :batched_migration_last_update_time_seconds, + 'Unix epoch time in seconds' + ) + } + end + end + + private + + def track_timing_metrics(base_labels, metrics) + return unless metrics && metrics['timings'] + + metrics['timings'].each do |key, timings| + summary = metric_for(:histogram_timings) + labels = base_labels.merge(operation: key) + + timings.each do |timing| + summary.observe(labels, timing) + end + end + end + + def metric_for(name) + self.class.metrics[name] + end + end + end + end +end diff --git a/lib/gitlab/database/consistency_checker.rb b/lib/gitlab/database/consistency_checker.rb new file mode 100644 index 00000000000..e398fef744c --- /dev/null +++ b/lib/gitlab/database/consistency_checker.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class ConsistencyChecker + BATCH_SIZE = 1000 + MAX_BATCHES = 25 + MAX_RUNTIME = 30.seconds # must be less than the scheduling frequency of the ConsistencyCheck jobs + + delegate :monotonic_time, to: :'Gitlab::Metrics::System' + + def initialize(source_model:, target_model:, source_columns:, target_columns:) + @source_model = source_model + @target_model = target_model + @source_columns = source_columns + @target_columns = target_columns + @source_sort_column = source_columns.first + @target_sort_column = target_columns.first + @result = { matches: 0, mismatches: 0, batches: 0, mismatches_details: [] } + end + + # rubocop:disable Metrics/AbcSize + def execute(start_id:) + current_start_id = start_id + + return build_result(next_start_id: nil) if max_id.nil? + return build_result(next_start_id: min_id) if current_start_id > max_id + + @start_time = monotonic_time + + MAX_BATCHES.times do + if (current_start_id <= max_id) && !over_time_limit? + ids_range = current_start_id...(current_start_id + BATCH_SIZE) + # rubocop: disable CodeReuse/ActiveRecord + source_data = source_model.where(source_sort_column => ids_range) + .order(source_sort_column => :asc).pluck(*source_columns) + target_data = target_model.where(target_sort_column => ids_range) + .order(target_sort_column => :asc).pluck(*target_columns) + # rubocop: enable CodeReuse/ActiveRecord + + current_start_id += BATCH_SIZE + result[:matches] += append_mismatches_details(source_data, target_data) + result[:batches] += 1 + else + break + end + end + + result[:mismatches] = result[:mismatches_details].length + metrics_counter.increment({ source_table: source_model.table_name, result: "match" }, result[:matches]) + metrics_counter.increment({ source_table: source_model.table_name, result: "mismatch" }, result[:mismatches]) + + build_result(next_start_id: current_start_id > max_id ? min_id : current_start_id) + end + # rubocop:enable Metrics/AbcSize + + private + + attr_reader :source_model, :target_model, :source_columns, :target_columns, + :source_sort_column, :target_sort_column, :start_time, :result + + def build_result(next_start_id:) + { next_start_id: next_start_id }.merge(result) + end + + def over_time_limit? + (monotonic_time - start_time) >= MAX_RUNTIME + end + + # This where comparing the items happen, and building the diff log + # It returns the number of matching elements + def append_mismatches_details(source_data, target_data) + # Mapping difference the sort key to the item values + # source - target + source_diff_hash = (source_data - target_data).index_by { |item| item.shift } + # target - source + target_diff_hash = (target_data - source_data).index_by { |item| item.shift } + + matches = source_data.length - source_diff_hash.length + + # Items that exist in the first table + Different items + source_diff_hash.each do |id, values| + result[:mismatches_details] << { + id: id, + source_table: values, + target_table: target_diff_hash[id] + } + end + + # Only the items that exist in the target table + target_diff_hash.each do |id, values| + next if source_diff_hash[id] # It's already added + + result[:mismatches_details] << { + id: id, + source_table: source_diff_hash[id], + target_table: values + } + end + + matches + end + + # rubocop: disable CodeReuse/ActiveRecord + def min_id + @min_id ||= source_model.minimum(source_sort_column) + end + + def max_id + @max_id ||= source_model.maximum(source_sort_column) + end + # rubocop: enable CodeReuse/ActiveRecord + + def metrics_counter + @metrics_counter ||= Gitlab::Metrics.counter( + :consistency_checks, + "Consistency Check Results" + ) + end + end + end +end diff --git a/lib/gitlab/database/each_database.rb b/lib/gitlab/database/each_database.rb index cccd4b48723..0d876f5124f 100644 --- a/lib/gitlab/database/each_database.rb +++ b/lib/gitlab/database/each_database.rb @@ -4,11 +4,13 @@ module Gitlab module Database module EachDatabase class << self - def each_database_connection(only: nil) + def each_database_connection(only: nil, include_shared: true) selected_names = Array.wrap(only) base_models = select_base_models(selected_names) base_models.each_pair do |connection_name, model| + next if !include_shared && Gitlab::Database.db_config_share_with(model.connection_db_config) + connection = model.connection with_shared_connection(connection, connection_name) do diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index dcd78bfd84f..ae0ea919b62 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -42,11 +42,11 @@ audit_events: :gitlab_main authentication_events: :gitlab_main award_emoji: :gitlab_main aws_roles: :gitlab_main -background_migration_jobs: :gitlab_main +background_migration_jobs: :gitlab_shared badges: :gitlab_main banned_users: :gitlab_main -batched_background_migration_jobs: :gitlab_main -batched_background_migrations: :gitlab_main +batched_background_migration_jobs: :gitlab_shared +batched_background_migrations: :gitlab_shared board_assignees: :gitlab_main board_group_recent_visits: :gitlab_main board_labels: :gitlab_main @@ -240,6 +240,7 @@ group_deletion_schedules: :gitlab_main group_deploy_keys: :gitlab_main group_deploy_keys_groups: :gitlab_main group_deploy_tokens: :gitlab_main +group_features: :gitlab_main group_group_links: :gitlab_main group_import_states: :gitlab_main group_merge_request_approval_settings: :gitlab_main @@ -393,7 +394,7 @@ postgres_indexes: :gitlab_shared postgres_partitioned_tables: :gitlab_shared postgres_partitions: :gitlab_shared postgres_reindex_actions: :gitlab_shared -postgres_reindex_queued_actions: :gitlab_main +postgres_reindex_queued_actions: :gitlab_shared product_analytics_events_experimental: :gitlab_main programming_languages: :gitlab_main project_access_tokens: :gitlab_main @@ -435,6 +436,7 @@ protected_branches: :gitlab_main protected_branch_merge_access_levels: :gitlab_main protected_branch_push_access_levels: :gitlab_main protected_branch_unprotect_access_levels: :gitlab_main +protected_environment_approval_rules: :gitlab_main protected_environment_deploy_access_levels: :gitlab_main protected_environments: :gitlab_main protected_tag_create_access_levels: :gitlab_main @@ -558,4 +560,4 @@ x509_commit_signatures: :gitlab_main x509_issuers: :gitlab_main zentao_tracker_data: :gitlab_main zoom_meetings: :gitlab_main -batched_background_migration_job_transition_logs: :gitlab_main +batched_background_migration_job_transition_logs: :gitlab_shared diff --git a/lib/gitlab/database/load_balancing/configuration.rb b/lib/gitlab/database/load_balancing/configuration.rb index 86b3afaa47b..3f03d9e2c12 100644 --- a/lib/gitlab/database/load_balancing/configuration.rb +++ b/lib/gitlab/database/load_balancing/configuration.rb @@ -78,15 +78,15 @@ module Gitlab end def primary_model_or_model_if_enabled - if force_no_sharing_primary_model? + if use_dedicated_connection? @model else @primary_model || @model end end - def force_no_sharing_primary_model? - return false unless @primary_model # Doesn't matter since we don't have an overriding primary model + def use_dedicated_connection? + return true unless @primary_model # We can only use dedicated connection, if re-use of connections is disabled return false unless ::Gitlab::SafeRequestStore.active? ::Gitlab::SafeRequestStore.fetch(:force_no_sharing_primary_model) do diff --git a/lib/gitlab/database/load_balancing/connection_proxy.rb b/lib/gitlab/database/load_balancing/connection_proxy.rb index a91df2eccdd..1be63da8896 100644 --- a/lib/gitlab/database/load_balancing/connection_proxy.rb +++ b/lib/gitlab/database/load_balancing/connection_proxy.rb @@ -13,13 +13,6 @@ module Gitlab WriteInsideReadOnlyTransactionError = Class.new(StandardError) READ_ONLY_TRANSACTION_KEY = :load_balacing_read_only_transaction - # The load balancer returned by connection might be different - # between `model.connection.load_balancer` vs `model.load_balancer` - # - # The used `model.connection` is dependent on `use_model_load_balancing`. - # See more in: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73949. - # - # Always use `model.load_balancer` or `model.sticking`. attr_reader :load_balancer # These methods perform writes after which we need to stick to the diff --git a/lib/gitlab/database/load_balancing/setup.rb b/lib/gitlab/database/load_balancing/setup.rb index 6d667e8ecf0..eceea1d8d9c 100644 --- a/lib/gitlab/database/load_balancing/setup.rb +++ b/lib/gitlab/database/load_balancing/setup.rb @@ -17,7 +17,12 @@ module Gitlab configure_connection setup_connection_proxy setup_service_discovery - setup_feature_flag_to_model_load_balancing + + ::Gitlab::Database::LoadBalancing::Logger.debug( + event: :setup, + model: model.name, + start_service_discovery: @start_service_discovery + ) end def configure_connection @@ -45,21 +50,6 @@ module Gitlab setup_class_attribute(:sticking, Sticking.new(load_balancer)) end - # TODO: This is temporary code to gradually redirect traffic to use - # a dedicated DB replicas, or DB primaries (depending on configuration) - # This implements a sticky behavior for the current request if enabled. - # - # This is needed for Phase 3 and Phase 4 of application rollout - # https://gitlab.com/groups/gitlab-org/-/epics/6160#progress - # - # If `GITLAB_USE_MODEL_LOAD_BALANCING` is set, its value is preferred - # Otherwise, a `use_model_load_balancing` FF value is used - def setup_feature_flag_to_model_load_balancing - return if active_record_base? - - @model.singleton_class.prepend(ModelLoadBalancingFeatureFlagMixin) - end - def setup_service_discovery return unless configuration.service_discovery_enabled? @@ -84,31 +74,6 @@ module Gitlab def active_record_base? @model == ActiveRecord::Base end - - module ModelLoadBalancingFeatureFlagMixin - extend ActiveSupport::Concern - - def use_model_load_balancing? - # Cache environment variable and return env variable first if defined - default_use_model_load_balancing_env = Gitlab.dev_or_test_env? || nil - use_model_load_balancing_env = Gitlab::Utils.to_boolean(ENV.fetch('GITLAB_USE_MODEL_LOAD_BALANCING', default_use_model_load_balancing_env)) - - unless use_model_load_balancing_env.nil? - return use_model_load_balancing_env - end - - # Check a feature flag using RequestStore (if active) - return false unless Gitlab::SafeRequestStore.active? - - Gitlab::SafeRequestStore.fetch(:use_model_load_balancing) do - Feature.enabled?(:use_model_load_balancing, default_enabled: :yaml) - end - end - - def connection - use_model_load_balancing? ? super : ApplicationRecord.connection - end - end end end end diff --git a/lib/gitlab/database/migration.rb b/lib/gitlab/database/migration.rb index b2248b0f4eb..dc695a74a4b 100644 --- a/lib/gitlab/database/migration.rb +++ b/lib/gitlab/database/migration.rb @@ -33,20 +33,33 @@ module Gitlab # We use major version bumps to indicate significant changes and minor version bumps # to indicate backwards-compatible or otherwise minor changes (e.g. a Rails version bump). # However, this hasn't been strictly formalized yet. - MIGRATION_CLASSES = { - 1.0 => Class.new(ActiveRecord::Migration[6.1]) do - include LockRetriesConcern - include Gitlab::Database::MigrationHelpers::V2 + + class V1_0 < ActiveRecord::Migration[6.1] # rubocop:disable Naming/ClassAndModuleCamelCase + include LockRetriesConcern + include Gitlab::Database::MigrationHelpers::V2 + end + + class V2_0 < V1_0 # rubocop:disable Naming/ClassAndModuleCamelCase + include Gitlab::Database::MigrationHelpers::RestrictGitlabSchema + + # When running migrations, the `db:migrate` switches connection of + # ActiveRecord::Base depending where the migration runs. + # This helper class is provided to avoid confusion using `ActiveRecord::Base` + class MigrationRecord < ActiveRecord::Base end - }.freeze + end def self.[](version) - MIGRATION_CLASSES[version] || raise(ArgumentError, "Unknown migration version: #{version}") + version = version.to_s + name = "V#{version.tr('.', '_')}" + raise ArgumentError, "Unknown migration version: #{version}" unless const_defined?(name, false) + + const_get(name, false) end # The current version to be used in new migrations def self.current_version - 1.0 + 2.0 end end end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 7602e09981a..d016dea224b 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -692,6 +692,8 @@ module Gitlab # batch_column_name - option for tables without a primary key, in this case # another unique integer column can be used. Example: :user_id def undo_cleanup_concurrent_column_type_change(table, column, old_type, type_cast_function: nil, batch_column_name: :id, limit: nil) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode! + temp_column = "#{column}_for_type_change" # Using a descriptive name that includes orinal column's name risks @@ -956,7 +958,7 @@ module Gitlab Gitlab::AppLogger.warn "Could not find batched background migration for the given configuration: #{configuration}" elsif !migration.finished? raise "Expected batched background migration for the given configuration to be marked as 'finished', " \ - "but it is '#{migration.status}':" \ + "but it is '#{migration.status_name}':" \ "\t#{configuration}" \ "\n\n" \ "Finalize it manualy by running" \ @@ -1639,7 +1641,9 @@ into similar problems in the future (e.g. when new tables are created). old_value = Arel::Nodes::NamedFunction.new(type_cast_function, [old_value]) end - update_column_in_batches(table, new, old_value, batch_column_name: batch_column_name) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do + update_column_in_batches(table, new, old_value, batch_column_name: batch_column_name) + end add_not_null_constraint(table, new) unless old_col.null diff --git a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb index b4e31565c60..5a25128f3a9 100644 --- a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb +++ b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb @@ -6,8 +6,6 @@ module Gitlab module RestrictGitlabSchema extend ActiveSupport::Concern - MigrationSkippedError = Class.new(StandardError) - included do class_attribute :allowed_gitlab_schemas end @@ -25,11 +23,8 @@ module Gitlab def migrate(direction) if unmatched_schemas.any? - # TODO: Today skipping migration would raise an exception. - # Ideally, skipped migration should be ignored (not loaded), or softly ignored. - # Read more in: https://gitlab.com/gitlab-org/gitlab/-/issues/355014 - raise MigrationSkippedError, "Current migration is skipped since it modifies "\ - "'#{self.class.allowed_gitlab_schemas}' which is outside of '#{allowed_schemas_for_connection}'" + migration_skipped + return end Gitlab::Database::QueryAnalyzer.instance.within([validator_class]) do @@ -41,6 +36,11 @@ module Gitlab private + def migration_skipped + say "Current migration is skipped since it modifies "\ + "'#{self.class.allowed_gitlab_schemas}' which is outside of '#{allowed_schemas_for_connection}'" + end + def validator_class Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas end diff --git a/lib/gitlab/database/migration_helpers/v2.rb b/lib/gitlab/database/migration_helpers/v2.rb index 0e7f6075196..dd426962033 100644 --- a/lib/gitlab/database/migration_helpers/v2.rb +++ b/lib/gitlab/database/migration_helpers/v2.rb @@ -134,6 +134,8 @@ module Gitlab # batch_column_name - option is for tables without primary key, in this # case another unique integer column can be used. Example: :user_id def rename_column_concurrently(table, old_column, new_column, type: nil, batch_column_name: :id) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode! + setup_renamed_column(__callee__, table, old_column, new_column, type, batch_column_name) with_lock_retries do @@ -181,6 +183,8 @@ module Gitlab # case another unique integer column can be used. Example: :user_id # def undo_cleanup_concurrent_column_rename(table, old_column, new_column, type: nil, batch_column_name: :id) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode! + setup_renamed_column(__callee__, table, new_column, old_column, type, batch_column_name) with_lock_retries do diff --git a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb index a2a4a37ab87..0261ade0fe7 100644 --- a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb +++ b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb @@ -84,7 +84,7 @@ module Gitlab FROM #{connection.quote_table_name(batch_table_name)} SQL - migration_status = batch_max_value.nil? ? :finished : :active + status_event = batch_max_value.nil? ? :finish : :execute batch_max_value ||= batch_min_value migration = Gitlab::Database::BackgroundMigration::BatchedMigration.new( @@ -98,7 +98,7 @@ module Gitlab batch_class_name: batch_class_name, batch_size: batch_size, sub_batch_size: sub_batch_size, - status: migration_status + status_event: status_event ) # Below `BatchedMigration` attributes were introduced after the diff --git a/lib/gitlab/database/migrations/instrumentation.rb b/lib/gitlab/database/migrations/instrumentation.rb index 9d28db6b886..7c21346007a 100644 --- a/lib/gitlab/database/migrations/instrumentation.rb +++ b/lib/gitlab/database/migrations/instrumentation.rb @@ -6,11 +6,8 @@ module Gitlab class Instrumentation STATS_FILENAME = 'migration-stats.json' - attr_reader :observations - def initialize(result_dir:, observer_classes: ::Gitlab::Database::Migrations::Observers.all_observers) @observer_classes = observer_classes - @observations = [] @result_dir = result_dir end @@ -38,15 +35,16 @@ module Gitlab on_each_observer(observers) { |observer| observer.after } on_each_observer(observers) { |observer| observer.record } - record_observation(observation) + record_observation(observation, destination_dir: per_migration_result_dir) end private attr_reader :observer_classes - def record_observation(observation) - @observations << observation + def record_observation(observation, destination_dir:) + stats_file_location = File.join(destination_dir, STATS_FILENAME) + File.write(stats_file_location, observation.to_json) end def on_each_observer(observers, &block) diff --git a/lib/gitlab/database/migrations/runner.rb b/lib/gitlab/database/migrations/runner.rb index 02645a0d452..3b6f52b43a8 100644 --- a/lib/gitlab/database/migrations/runner.rb +++ b/lib/gitlab/database/migrations/runner.rb @@ -6,7 +6,7 @@ module Gitlab class Runner BASE_RESULT_DIR = Rails.root.join('tmp', 'migration-testing').freeze METADATA_FILENAME = 'metadata.json' - SCHEMA_VERSION = 2 # Version of the output format produced by the runner + SCHEMA_VERSION = 3 # Version of the output format produced by the runner class << self def up @@ -17,6 +17,10 @@ module Gitlab Runner.new(direction: :down, migrations: migrations_for_down, result_dir: BASE_RESULT_DIR.join('down')) end + def background_migrations + TestBackgroundRunner.new(result_dir: BASE_RESULT_DIR.join('background_migrations')) + end + def migration_context @migration_context ||= ApplicationRecord.connection.migration_context end @@ -76,13 +80,8 @@ module Gitlab end end ensure - if instrumentation - stats_filename = File.join(result_dir, Gitlab::Database::Migrations::Instrumentation::STATS_FILENAME) - File.write(stats_filename, instrumentation.observations.to_json) - - metadata_filename = File.join(result_dir, METADATA_FILENAME) - File.write(metadata_filename, { version: SCHEMA_VERSION }.to_json) - end + metadata_filename = File.join(result_dir, METADATA_FILENAME) + File.write(metadata_filename, { version: SCHEMA_VERSION }.to_json) # We clear the cache here to mirror the cache clearing that happens at the end of `db:migrate` tasks # This clearing makes subsequent rake tasks in the same execution pick up database schema changes caused by diff --git a/lib/gitlab/database/migrations/test_background_runner.rb b/lib/gitlab/database/migrations/test_background_runner.rb index 821d68c06c9..74e54d62e05 100644 --- a/lib/gitlab/database/migrations/test_background_runner.rb +++ b/lib/gitlab/database/migrations/test_background_runner.rb @@ -4,12 +4,10 @@ module Gitlab module Database module Migrations class TestBackgroundRunner - # TODO - build a rake task to call this method, and support it in the gitlab-com-database-testing project. - # Until then, we will inject a migration with a very high timestamp during database testing - # that calls this class to run jobs - # See https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing/-/issues/41 for details + attr_reader :result_dir - def initialize + def initialize(result_dir:) + @result_dir = result_dir @job_coordinator = Gitlab::BackgroundMigration.coordinator_for_database(Gitlab::Database::MAIN_DATABASE_NAME) end @@ -24,18 +22,30 @@ module Gitlab # without .to_f, we do integer division # For example, 3.minutes / 2 == 1.minute whereas 3.minutes / 2.to_f == (1.minute + 30.seconds) duration_per_migration_type = for_duration / jobs_to_run.count.to_f - jobs_to_run.each do |_migration_name, jobs| + jobs_to_run.each do |migration_name, jobs| run_until = duration_per_migration_type.from_now - jobs.shuffle.each do |j| - break if run_until <= Time.current - run_job(j) - end + run_jobs_for_migration(migration_name: migration_name, jobs: jobs, run_until: run_until) end end private + def run_jobs_for_migration(migration_name:, jobs:, run_until:) + per_background_migration_result_dir = File.join(@result_dir, migration_name) + + instrumentation = Instrumentation.new(result_dir: per_background_migration_result_dir) + batch_names = (1..).each.lazy.map { |i| "batch_#{i}"} + + jobs.shuffle.each do |j| + break if run_until <= Time.current + + instrumentation.observe(version: nil, name: batch_names.next, connection: ActiveRecord::Migration.connection) do + run_job(j) + end + end + end + def run_job(job) Gitlab::BackgroundMigration.perform(job.args[0], job.args[1]) end diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index e56ffddac4f..034e18ec9f4 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -40,16 +40,20 @@ module Gitlab # 1. The minimum value for the partitioning column in the table # 2. If no data is present yet, the current month def partition_table_by_date(table_name, column_name, min_date: nil, max_date: nil) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode! + assert_table_is_allowed(table_name) assert_not_in_transaction_block(scope: ERROR_SCOPE) max_date ||= Date.today + 1.month - min_date ||= connection.select_one(<<~SQL)['minimum'] || max_date - 1.month - SELECT date_trunc('MONTH', MIN(#{column_name})) AS minimum - FROM #{table_name} - SQL + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do + min_date ||= connection.select_one(<<~SQL)['minimum'] || max_date - 1.month + SELECT date_trunc('MONTH', MIN(#{column_name})) AS minimum + FROM #{table_name} + SQL + end raise "max_date #{max_date} must be greater than min_date #{min_date}" if min_date >= max_date @@ -154,6 +158,8 @@ module Gitlab # finalize_backfilling_partitioned_table :audit_events # def finalize_backfilling_partitioned_table(table_name) + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_dml_mode! + assert_table_is_allowed(table_name) assert_not_in_transaction_block(scope: ERROR_SCOPE) @@ -170,8 +176,10 @@ module Gitlab primary_key = connection.primary_key(table_name) copy_missed_records(table_name, partitioned_table_name, primary_key) - disable_statement_timeout do - execute("VACUUM FREEZE ANALYZE #{partitioned_table_name}") + Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do + disable_statement_timeout do + execute("VACUUM FREEZE ANALYZE #{partitioned_table_name}") + end end end diff --git a/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics.rb b/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics.rb index 06e2b114c91..391375d472f 100644 --- a/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics.rb +++ b/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics.rb @@ -27,9 +27,15 @@ module Gitlab # to reduce amount of labels sort schemas used gitlab_schemas = gitlab_schemas.to_a.sort.join(",") + # Temporary feature to observe relation of `gitlab_schemas` to `db_config_name` + # depending on primary model + ci_dedicated_primary_connection = ::Ci::ApplicationRecord.connection_class? && + ::Ci::ApplicationRecord.load_balancer.configuration.use_dedicated_connection? + schemas_metrics.increment({ gitlab_schemas: gitlab_schemas, - db_config_name: db_config_name + db_config_name: db_config_name, + ci_dedicated_primary_connection: ci_dedicated_primary_connection }) end diff --git a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb index ab40ba5d59b..3f0176cb654 100644 --- a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb +++ b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb @@ -69,8 +69,10 @@ module Gitlab schemas = self.dml_schemas(tables) if (schemas - self.allowed_gitlab_schemas).any? - raise DMLAccessDeniedError, "Select/DML queries (SELECT/UPDATE/DELETE) do access '#{tables}' (#{schemas.to_a}) " \ - "which is outside of list of allowed schemas: '#{self.allowed_gitlab_schemas}'." + raise DMLAccessDeniedError, \ + "Select/DML queries (SELECT/UPDATE/DELETE) do access '#{tables}' (#{schemas.to_a}) " \ + "which is outside of list of allowed schemas: '#{self.allowed_gitlab_schemas}'. " \ + "#{documentation_url}" end end @@ -93,11 +95,19 @@ module Gitlab end def raise_dml_not_allowed_error(message) - raise DMLNotAllowedError, "Select/DML queries (SELECT/UPDATE/DELETE) are disallowed in the DDL (structure) mode. #{message}" + raise DMLNotAllowedError, \ + "Select/DML queries (SELECT/UPDATE/DELETE) are disallowed in the DDL (structure) mode. " \ + "#{message}. #{documentation_url}" \ end def raise_ddl_not_allowed_error(message) - raise DDLNotAllowedError, "DDL queries (structure) are disallowed in the Select/DML (SELECT/UPDATE/DELETE) mode. #{message}" + raise DDLNotAllowedError, \ + "DDL queries (structure) are disallowed in the Select/DML (SELECT/UPDATE/DELETE) mode. " \ + "#{message}. #{documentation_url}" + end + + def documentation_url + "For more information visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html" end end end diff --git a/lib/gitlab/database/reindexing/grafana_notifier.rb b/lib/gitlab/database/reindexing/grafana_notifier.rb index f4ea59deb50..ece9327b658 100644 --- a/lib/gitlab/database/reindexing/grafana_notifier.rb +++ b/lib/gitlab/database/reindexing/grafana_notifier.rb @@ -5,10 +5,10 @@ module Gitlab module Reindexing # This can be used to send annotations for reindexing to a Grafana API class GrafanaNotifier - def initialize(api_key = ENV['GITLAB_GRAFANA_API_KEY'], api_url = ENV['GITLAB_GRAFANA_API_URL'], additional_tag = ENV['GITLAB_REINDEXING_GRAFANA_TAG'] || Rails.env) - @api_key = api_key - @api_url = api_url - @additional_tag = additional_tag + def initialize(api_key: nil, api_url: nil, additional_tag: nil) + @api_key = api_key || default_api_key + @api_url = api_url || default_api_url + @additional_tag = additional_tag || default_additional_tag end def notify_start(action) @@ -35,10 +35,22 @@ module Gitlab private + def default_api_key + Gitlab::CurrentSettings.database_grafana_api_key || ENV['GITLAB_GRAFANA_API_KEY'] + end + + def default_api_url + Gitlab::CurrentSettings.database_grafana_api_url || ENV['GITLAB_GRAFANA_API_URL'] + end + + def default_additional_tag + Gitlab::CurrentSettings.database_grafana_tag || ENV['GITLAB_REINDEXING_GRAFANA_TAG'] || Rails.env + end + def base_payload(action) { time: (action.action_start.utc.to_f * 1000).to_i, - tags: ['reindex', @additional_tag, action.index.tablename, action.index.name].compact + tags: ['reindex', @additional_tag.presence, action.index.tablename, action.index.name].compact } end |