Skip to content

G
gitlab-ce
Commit a2c40922 authored by Stan Hu's avatar Stan Hu Committed by Mikolaj Wawrzyniak
Browse files
Options

Revert broken background migration 

This revert PatchPrometheusServicesForSharedClusterApplications
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22974
because it has the potential of blowing up Sidekiq queues.
Background jobs are intended to process many rows for
a large batch of records, but this migration tends to process
a single row within a second.

Mikolaj W: With exception for adding temporary index
as plain revert of this migration can potentially
leave some databases in inconsistent state.

Identified in
https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/9064
parent d6bb8322
Hide whitespace changes
Inline Side-by-side
Showing with 1 addition and 374 deletions
db/post_migrate/20200114113341_patch_prometheus_services_for_shared_cluster_applications.rb
View file @ a2c40922
# frozen_string_literal: true
class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
MIGRATION = 'ActivatePrometheusServicesForSharedClusterApplications'.freeze
BATCH_SIZE = 500
DELAY = 2.minutes
disable_ddl_transaction!
module Migratable
module Applications
class Prometheus < ActiveRecord::Base
self.table_name = 'clusters_applications_prometheus'
enum status: {
errored: -1,
installed: 3,
updated: 5
}
end
end
class Project < ActiveRecord::Base
self.table_name = 'projects'
include ::EachBatch
scope :with_application_on_group_clusters, -> {
joins("INNER JOIN namespaces ON namespaces.id = projects.namespace_id")
.joins("INNER JOIN cluster_groups ON cluster_groups.group_id = namespaces.id")
.joins("INNER JOIN clusters ON clusters.id = cluster_groups.cluster_id AND clusters.cluster_type = #{Cluster.cluster_types['group_type']}")
.joins("INNER JOIN clusters_applications_prometheus ON clusters_applications_prometheus.cluster_id = clusters.id
AND clusters_applications_prometheus.status IN (#{Applications::Prometheus.statuses[:installed]}, #{Applications::Prometheus.statuses[:updated]})")
}
scope :without_active_prometheus_services, -> {
joins("LEFT JOIN services ON services.project_id = projects.id AND services.type = 'PrometheusService'")
.where("services.id IS NULL OR (services.active = FALSE AND services.properties = '{}')")
}
end
class Cluster < ActiveRecord::Base
self.table_name = 'clusters'
enum cluster_type: {
instance_type: 1,
group_type: 2
}
def self.has_prometheus_application?
joins("INNER JOIN clusters_applications_prometheus ON clusters_applications_prometheus.cluster_id = clusters.id
AND clusters_applications_prometheus.status IN (#{Applications::Prometheus.statuses[:installed]}, #{Applications::Prometheus.statuses[:updated]})").exists?
end
end
end
def up
projects_without_active_prometheus_service.group('projects.id').each_batch(of: BATCH_SIZE) do |batch, index|
bg_migrations_batch = batch.select('projects.id').map { |project| [MIGRATION, project.id] }
delay = index * DELAY
BackgroundMigrationWorker.bulk_perform_in(delay.seconds, bg_migrations_batch)
end
# no-op
end
def down
# no-op
end
private
def projects_without_active_prometheus_service
scope = Migratable::Project.without_active_prometheus_services
return scope if migrate_instance_cluster?
scope.with_application_on_group_clusters
end
def migrate_instance_cluster?
if instance_variable_defined?('@migrate_instance_cluster')
@migrate_instance_cluster
else
@migrate_instance_cluster = Migratable::Cluster.instance_type.has_prometheus_application?
end
end
end
lib/gitlab/background_migration/activate_prometheus_services_for_shared_cluster_applications.rb deleted 100644 → 0
View file @ d6bb8322
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# Create missing PrometheusServices records or sets active attribute to true
# for all projects which belongs to cluster with Prometheus Application installed.
class ActivatePrometheusServicesForSharedClusterApplications
module Migratable
# Migration model namespace isolated from application code.
class PrometheusService < ActiveRecord::Base
self.inheritance_column = :_type_disabled
self.table_name = 'services'
default_scope { where("services.type = 'PrometheusService'") }
def self.for_project(project_id)
new(
project_id: project_id,
active: true,
properties: '{}',
type: 'PrometheusService',
template: false,
push_events: true,
issues_events: true,
merge_requests_events: true,
tag_push_events: true,
note_events: true,
category: 'monitoring',
default: false,
wiki_page_events: true,
pipeline_events: true,
confidential_issues_events: true,
commit_events: true,
job_events: true,
confidential_note_events: true,
deployment_events: false
)
end
def managed?
properties == '{}'
end
end
end
def perform(project_id)
service = Migratable::PrometheusService.find_by(project_id: project_id) || Migratable::PrometheusService.for_project(project_id)
service.update!(active: true) if service.managed?
end
end
end
end
spec/lib/gitlab/background_migration/activate_prometheus_services_for_shared_cluster_applications_spec.rb deleted 100644 → 0
View file @ d6bb8322
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BackgroundMigration::ActivatePrometheusServicesForSharedClusterApplications, :migration, schema: 2020_01_14_113341 do
include MigrationHelpers::PrometheusServiceHelpers
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:services) { table(:services) }
let(:namespace) { namespaces.create(name: 'user', path: 'user') }
let(:project) { projects.create(namespace_id: namespace.id) }
let(:columns) do
%w(project_id active properties type template push_events
issues_events merge_requests_events tag_push_events
note_events category default wiki_page_events pipeline_events
confidential_issues_events commit_events job_events
confidential_note_events deployment_events)
end
describe '#perform' do
it 'is idempotent' do
expect { subject.perform(project.id) }.to change { services.order(:id).map { |row| row.attributes } }
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
context 'non prometheus services' do
it 'does not change them' do
other_type = 'SomeOtherService'
services.create(service_params_for(project.id, active: true, type: other_type))
expect { subject.perform(project.id) }.not_to change { services.where(type: other_type).order(:id).map { |row| row.attributes } }
end
end
context 'prometheus services are configured manually ' do
it 'does not change them' do
properties = '{"api_url":"http://test.dev","manual_configuration":"1"}'
services.create(service_params_for(project.id, properties: properties, active: false))
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
end
context 'prometheus integration services do not exist' do
it 'creates missing services entries' do
subject.perform(project.id)
rows = services.order(:id).map { |row| row.attributes.slice(*columns).symbolize_keys }
expect([service_params_for(project.id, active: true)]).to eq rows
end
end
context 'prometheus integration services exist' do
context 'in active state' do
it 'does not change them' do
services.create(service_params_for(project.id, active: true))
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
end
context 'not in active state' do
it 'sets active attribute to true' do
service = services.create(service_params_for(project.id))
expect { subject.perform(project.id) }.to change { service.reload.active? }.from(false).to(true)
end
end
end
end
end
spec/migrations/patch_prometheus_services_for_shared_cluster_applications_spec.rb deleted 100644 → 0
View file @ d6bb8322
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20200114113341_patch_prometheus_services_for_shared_cluster_applications.rb')
describe PatchPrometheusServicesForSharedClusterApplications, :migration do
include MigrationHelpers::PrometheusServiceHelpers
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:services) { table(:services) }
let(:clusters) { table(:clusters) }
let(:cluster_groups) { table(:cluster_groups) }
let(:clusters_applications_prometheus) { table(:clusters_applications_prometheus) }
let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
let(:application_statuses) do
{
errored: -1,
installed: 3,
updated: 5
}
end
let(:cluster_types) do
{
instance_type: 1,
group_type: 2
}
end
describe '#up' do
let!(:project_with_missing_service) { projects.create!(name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) }
let(:project_with_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_manual_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_manual_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_active_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_inactive_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
before do
services.create(service_params_for(project_with_inactive_service.id, active: false))
services.create(service_params_for(project_with_active_service.id, active: true))
services.create(service_params_for(project_with_active_not_prometheus_service.id, active: true, type: 'other'))
services.create(service_params_for(project_with_inactive_not_prometheus_service.id, active: false, type: 'other'))
services.create(service_params_for(project_with_manual_inactive_service.id, active: false, properties: { some: 'data' }.to_json))
services.create(service_params_for(project_with_manual_active_service.id, active: true, properties: { some: 'data' }.to_json))
end
shared_examples 'patch prometheus services post migration' do
context 'prometheus application is installed on the cluster' do
it 'schedules a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:installed], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
migrate!
enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] }
expect(enqueued_migrations).to match_array(background_migrations)
end
end
end
end
context 'prometheus application was recently updated on the cluster' do
it 'schedules a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:updated], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
migrate!
enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] }
expect(enqueued_migrations).to match_array(background_migrations)
end
end
end
end
context 'prometheus application failed to install on the cluster' do
it 'does not schedule a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:errored], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(BackgroundMigrationWorker.jobs.size).to eq 0
end
end
end
end
context 'prometheus application is NOT installed on the cluster' do
it 'does not schedule a background migration' do
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(BackgroundMigrationWorker.jobs.size).to eq 0
end
end
end
end
end
context 'Cluster is group_type' do
let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:group_type]) }
before do
cluster_groups.create(group_id: namespace.id, cluster_id: cluster.id)
end
it_behaves_like 'patch prometheus services post migration'
end
context 'Cluster is instance_type' do
let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:instance_type]) }
it_behaves_like 'patch prometheus services post migration'
end
end
end
spec/support/migrations_helpers/prometheus_service_helpers.rb deleted 100644 → 0
View file @ d6bb8322
# frozen_string_literal: true
module MigrationHelpers
module PrometheusServiceHelpers
def service_params_for(project_id, params = {})
{
project_id: project_id,
active: false,
properties: '{}',
type: 'PrometheusService',
template: false,
push_events: true,
issues_events: true,
merge_requests_events: true,
tag_push_events: true,
note_events: true,
category: 'monitoring',
default: false,
wiki_page_events: true,
pipeline_events: true,
confidential_issues_events: true,
commit_events: true,
job_events: true,
confidential_note_events: true,
deployment_events: false
}.merge(params)
end
def row_attributes(entity)
entity.attributes.with_indifferent_access.tap do |hash|
hash.merge!(hash.slice(:created_at, :updated_at).transform_values { |v| v.to_s(:db) })
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7