From 3d0321d45dd477676f48c60d1aa66c8b2a75f6d1 Mon Sep 17 00:00:00 2001
From: Lucas Charles <me@lucascharles.me>
Date: Tue, 17 Dec 2024 00:08:50 +0000
Subject: [PATCH] fix: Update DB healthcheck to use conn.load_balancer for
deriving DB
Change PrometheusAlertIndicator (and all inheriting indicators) to rely
on connection.load_balancer for deriving current context SLI/SLOs
Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/501105
---
.../import/reassign_placeholder_user_records_service.rb | 6 ++----
.../database/background_migration/batched_migration.rb | 3 +--
lib/gitlab/database/health_status/context.rb | 5 ++---
.../indicators/prometheus_alert_indicator.rb | 8 ++++----
lib/gitlab/sidekiq_middleware/skip_jobs.rb | 3 +--
.../indicators/autovacuum_active_on_table_spec.rb | 3 +--
.../indicators/prometheus_alert_indicator_spec.rb | 3 +--
.../health_status/indicators/write_ahead_log_spec.rb | 3 +--
spec/lib/gitlab/database/health_status_spec.rb | 1 -
spec/lib/gitlab/sidekiq_middleware/skip_jobs_spec.rb | 2 +-
.../prometheus_alert_based_shared_examples.rb | 3 +--
11 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb
index 4e76ffff21be0..0714e5859232b 100644
--- a/app/services/import/reassign_placeholder_user_records_service.rb
+++ b/app/services/import/reassign_placeholder_user_records_service.rb
@@ -236,8 +236,7 @@ def db_table_health_check!(model)
health_context = Gitlab::Database::HealthStatus::Context.new(
DatabaseHealthStatusChecker.new(import_source_user.id, self.class.name),
nil,
- [model.table_name],
- nil
+ [model.table_name]
)
stop_signal = Gitlab::Database::HealthStatus
@@ -253,8 +252,7 @@ def db_health_check!
health_context = Gitlab::Database::HealthStatus::Context.new(
DatabaseHealthStatusChecker.new(import_source_user.id, self.class.name),
Gitlab::Database.schemas_to_base_models[gitlab_schema].first,
- nil,
- gitlab_schema
+ nil
)
Gitlab::Database::HealthStatus
diff --git a/lib/gitlab/database/background_migration/batched_migration.rb b/lib/gitlab/database/background_migration/batched_migration.rb
index 567f99d034ea7..2e11abdb378b5 100644
--- a/lib/gitlab/database/background_migration/batched_migration.rb
+++ b/lib/gitlab/database/background_migration/batched_migration.rb
@@ -279,8 +279,7 @@ def health_context
@health_context ||= Gitlab::Database::HealthStatus::Context.new(
self,
connection,
- [table_name],
- gitlab_schema.to_sym
+ [table_name]
)
end
diff --git a/lib/gitlab/database/health_status/context.rb b/lib/gitlab/database/health_status/context.rb
index 717257a84ad36..639149908cb5c 100644
--- a/lib/gitlab/database/health_status/context.rb
+++ b/lib/gitlab/database/health_status/context.rb
@@ -4,15 +4,14 @@ module Gitlab
module Database
module HealthStatus
class Context
- attr_reader :status_checker, :connection, :tables, :gitlab_schema
+ attr_reader :status_checker, :connection, :tables
# status_checker: the caller object which checks for database health status
# eg: BackgroundMigration::BatchedMigration or DeferJobs::DatabaseHealthStatusChecker
- def initialize(status_checker, connection, tables, gitlab_schema)
+ def initialize(status_checker, connection, tables)
@status_checker = status_checker
@connection = connection
@tables = tables
- @gitlab_schema = gitlab_schema
end
def status_checker_info
diff --git a/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator.rb b/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator.rb
index ecfb61014753b..b57e8bdda593d 100644
--- a/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator.rb
+++ b/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator.rb
@@ -13,7 +13,7 @@ class PrometheusAlertIndicator
}.freeze
def initialize(context)
- @gitlab_schema = context.gitlab_schema.to_sym
+ @connection = context.connection
end
def evaluate
@@ -34,7 +34,7 @@ def evaluate
private
- attr_reader :gitlab_schema
+ attr_reader :connection
def indicator_name
self.class.name.demodulize
@@ -107,7 +107,7 @@ def sli_query
gitlab_main_cell: prometheus_alert_db_indicators_settings[sli_query_key][:main_cell],
gitlab_ci: prometheus_alert_db_indicators_settings[sli_query_key][:ci],
gitlab_sec: gitlab_sec_query
- }.fetch(gitlab_schema)
+ }.fetch(:"gitlab_#{connection.load_balancer.name}", nil)
end
strong_memoize_attr :sli_query
@@ -121,7 +121,7 @@ def slo
gitlab_main_cell: prometheus_alert_db_indicators_settings[slo_key][:main_cell],
gitlab_ci: prometheus_alert_db_indicators_settings[slo_key][:ci],
gitlab_sec: gitlab_sec_query
- }.fetch(gitlab_schema)
+ }.fetch(:"gitlab_#{connection.load_balancer.name}", nil)
end
strong_memoize_attr :slo
diff --git a/lib/gitlab/sidekiq_middleware/skip_jobs.rb b/lib/gitlab/sidekiq_middleware/skip_jobs.rb
index e12d3bd079b2d..942834357aaa7 100644
--- a/lib/gitlab/sidekiq_middleware/skip_jobs.rb
+++ b/lib/gitlab/sidekiq_middleware/skip_jobs.rb
@@ -96,8 +96,7 @@ def defer_job_by_database_health_signal?(job, worker_class)
health_context = Gitlab::Database::HealthStatus::Context.new(
DatabaseHealthStatusChecker.new(job['jid'], worker_class.name),
job_base_model.connection,
- tables,
- schema
+ tables
)
Gitlab::Database::HealthStatus.evaluate(health_context).any?(&:stop?)
diff --git a/spec/lib/gitlab/database/health_status/indicators/autovacuum_active_on_table_spec.rb b/spec/lib/gitlab/database/health_status/indicators/autovacuum_active_on_table_spec.rb
index edf452575b14f..a05433e63c1ca 100644
--- a/spec/lib/gitlab/database/health_status/indicators/autovacuum_active_on_table_spec.rb
+++ b/spec/lib/gitlab/database/health_status/indicators/autovacuum_active_on_table_spec.rb
@@ -27,8 +27,7 @@
Gitlab::Database::HealthStatus::Context.new(
described_class,
connection,
- tables,
- :gitlab_main
+ tables
)
end
diff --git a/spec/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator_spec.rb b/spec/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator_spec.rb
index 2bd7639875cd4..4d593860c0b91 100644
--- a/spec/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator_spec.rb
+++ b/spec/lib/gitlab/database/health_status/indicators/prometheus_alert_indicator_spec.rb
@@ -10,8 +10,7 @@
Gitlab::Database::HealthStatus::Context.new(
described_class,
connection,
- ['users'],
- :gitlab_main
+ ['users']
)
end
diff --git a/spec/lib/gitlab/database/health_status/indicators/write_ahead_log_spec.rb b/spec/lib/gitlab/database/health_status/indicators/write_ahead_log_spec.rb
index cb849a7662156..58ef8d6dce254 100644
--- a/spec/lib/gitlab/database/health_status/indicators/write_ahead_log_spec.rb
+++ b/spec/lib/gitlab/database/health_status/indicators/write_ahead_log_spec.rb
@@ -18,8 +18,7 @@
Gitlab::Database::HealthStatus::Context.new(
described_class,
connection,
- tables,
- :gitlab_main
+ tables
)
end
diff --git a/spec/lib/gitlab/database/health_status_spec.rb b/spec/lib/gitlab/database/health_status_spec.rb
index 8e9cd1c0fc9c8..d25e4e301eb24 100644
--- a/spec/lib/gitlab/database/health_status_spec.rb
+++ b/spec/lib/gitlab/database/health_status_spec.rb
@@ -123,7 +123,6 @@ def self.name
Gitlab::Database::HealthStatus::Context.new(
deferred_worker_health_checker,
ActiveRecord::Base.connection,
- :gitlab_main,
[:users]
)
end
diff --git a/spec/lib/gitlab/sidekiq_middleware/skip_jobs_spec.rb b/spec/lib/gitlab/sidekiq_middleware/skip_jobs_spec.rb
index ecdc620c01710..0383791b82518 100644
--- a/spec/lib/gitlab/sidekiq_middleware/skip_jobs_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/skip_jobs_spec.rb
@@ -208,7 +208,7 @@ def self.name
it 'uses the lazy evaluated schema and tables returned by the block' do
expect(Gitlab::Database::HealthStatus::Context).to receive(:new)
- .with(anything, anything, [:ci_pipelines], :gitlab_ci).and_call_original
+ .with(anything, anything, [:ci_pipelines]).and_call_original
expect { |b| subject.call(TestWorker.new, job, queue, &b) }.to yield_control
end
diff --git a/spec/support/shared_examples/database_health_status_indicators/prometheus_alert_based_shared_examples.rb b/spec/support/shared_examples/database_health_status_indicators/prometheus_alert_based_shared_examples.rb
index b3c9efa0cfa24..09897f3551125 100644
--- a/spec/support/shared_examples/database_health_status_indicators/prometheus_alert_based_shared_examples.rb
+++ b/spec/support/shared_examples/database_health_status_indicators/prometheus_alert_based_shared_examples.rb
@@ -11,8 +11,7 @@
end
describe '#evaluate' do
- let(:context) { Gitlab::Database::HealthStatus::Context.new(described_class, connection, ['users'], gitlab_schema) }
- let(:gitlab_schema) { "gitlab_#{schema}" }
+ let(:context) { Gitlab::Database::HealthStatus::Context.new(described_class, connection, ['users']) }
let(:client_ready) { true }
let(:indicator_name) { described_class.name.demodulize }
let(:indicator) { described_class.new(context) }
--
GitLab