Skip to content

G
gitlab-ce
Commit f53a3e5a authored by Mayra Cabrera's avatar Mayra Cabrera Committed by Robert Speicher
Browse files
Options

Revert "Merge branch 'fix_migration_helpers_final_delay' into 'master'" 

This reverts commit ff949207, reversing
changes made to 106260b1.
parent e4d69634
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 146 deletions
changelogs/unreleased/28560_cleanup_optimistic_locking_db_pt2.yml deleted 100644 → 0
View file @ e4d69634
---
title: Set NULL `lock_version` values to 0 for CI objects
merge_request: 25396
author:
type: fixed
db/post_migrate/20200128210353_cleanup_optimistic_locking_nulls.rb
View file @ f53a3e5a
......@@ -31,7 +31,7 @@ class CleanupOptimisticLockingNulls < ActiveRecord::Migration[5.2]
'CleanupOptimisticLockingNulls',
2.minutes,
batch_size: BATCH_SIZE,
other_job_arguments: [table]
other_arguments: [table]
)
end
end
......
db/post_migrate/20200217210353_cleanup_optimistic_locking_nulls_pt2.rb deleted 100644 → 0
View file @ e4d69634
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class CleanupOptimisticLockingNullsPt2 < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
TABLES = %w(ci_stages ci_builds ci_pipelines).freeze
BATCH_SIZE = 10_000
def declare_class(table)
Class.new(ActiveRecord::Base) do
include EachBatch
self.table_name = table
self.inheritance_column = :_type_disabled # Disable STI
end
end
def up
last_table_final_delay = 0
TABLES.each do |table|
add_concurrent_index table.to_sym, :lock_version, where: "lock_version IS NULL"
last_table_final_delay = queue_background_migration_jobs_by_range_at_intervals(
declare_class(table).where(lock_version: nil),
'CleanupOptimisticLockingNulls',
2.minutes,
batch_size: BATCH_SIZE,
other_job_arguments: [table],
initial_delay: last_table_final_delay
)
end
end
def down
TABLES.each do |table|
remove_concurrent_index table.to_sym, :lock_version, where: "lock_version IS NULL"
end
end
end
db/structure.sql
View file @ f53a3e5a
......@@ -8862,8 +8862,6 @@ CREATE INDEX index_ci_builds_on_commit_id_and_type_and_name_and_ref ON public.ci
CREATE INDEX index_ci_builds_on_commit_id_and_type_and_ref ON public.ci_builds USING btree (commit_id, type, ref);
CREATE INDEX index_ci_builds_on_lock_version ON public.ci_builds USING btree (lock_version) WHERE (lock_version IS NULL);
CREATE INDEX index_ci_builds_on_name_and_security_type_eq_ci_build ON public.ci_builds USING btree (name, id) WHERE (((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text, ('license_scanning'::character varying)::text])) AND ((type)::text = 'Ci::Build'::text));
CREATE INDEX index_ci_builds_on_project_id_and_id ON public.ci_builds USING btree (project_id, id);
......@@ -8940,8 +8938,6 @@ CREATE INDEX index_ci_pipelines_on_auto_canceled_by_id ON public.ci_pipelines US
CREATE INDEX index_ci_pipelines_on_external_pull_request_id ON public.ci_pipelines USING btree (external_pull_request_id) WHERE (external_pull_request_id IS NOT NULL);
CREATE INDEX index_ci_pipelines_on_lock_version ON public.ci_pipelines USING btree (lock_version) WHERE (lock_version IS NULL);
CREATE INDEX index_ci_pipelines_on_merge_request_id ON public.ci_pipelines USING btree (merge_request_id) WHERE (merge_request_id IS NOT NULL);
CREATE INDEX index_ci_pipelines_on_pipeline_schedule_id ON public.ci_pipelines USING btree (pipeline_schedule_id);
......@@ -9010,8 +9006,6 @@ CREATE INDEX index_ci_sources_projects_on_pipeline_id ON public.ci_sources_proje
CREATE UNIQUE INDEX index_ci_sources_projects_on_source_project_id_and_pipeline_id ON public.ci_sources_projects USING btree (source_project_id, pipeline_id);
CREATE INDEX index_ci_stages_on_lock_version ON public.ci_stages USING btree (lock_version) WHERE (lock_version IS NULL);
CREATE INDEX index_ci_stages_on_pipeline_id ON public.ci_stages USING btree (pipeline_id);
CREATE UNIQUE INDEX index_ci_stages_on_pipeline_id_and_name ON public.ci_stages USING btree (pipeline_id, name);
......@@ -13070,7 +13064,6 @@ COPY "schema_migrations" (version) FROM STDIN;
20200214214934
20200215222507
20200215225103
20200217210353
20200217223651
20200217225719
20200218113721
......
lib/gitlab/database/migration_helpers.rb
View file @ f53a3e5a
......@@ -1063,8 +1063,6 @@ into similar problems in the future (e.g. when new tables are created).
# batch_size - The maximum number of rows per job
# other_arguments - Other arguments to send to the job
#
# *Returns the final migration delay*
#
# Example:
#
# class Route < ActiveRecord::Base
......@@ -1081,7 +1079,7 @@ into similar problems in the future (e.g. when new tables are created).
# # do something
# end
# end
def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE, other_job_arguments: [], initial_delay: 0)
def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE, other_arguments: [])
raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id')
# To not overload the worker too much we enforce a minimum interval both
......@@ -1090,19 +1088,14 @@ into similar problems in the future (e.g. when new tables are created).
delay_interval = BackgroundMigrationWorker.minimum_interval
end
final_delay = 0
model_class.each_batch(of: batch_size) do |relation, index|
start_id, end_id = relation.pluck(Arel.sql('MIN(id), MAX(id)')).first
# `BackgroundMigrationWorker.bulk_perform_in` schedules all jobs for
# the same time, which is not helpful in most cases where we wish to
# spread the work over time.
final_delay = initial_delay + delay_interval * index
migrate_in(final_delay, job_class_name, [start_id, end_id] + other_job_arguments)
migrate_in(delay_interval * index, job_class_name, [start_id, end_id] + other_arguments)
end
final_delay
end
# Fetches indexes on a column by name for postgres.
......
spec/lib/gitlab/database/migration_helpers_spec.rb
View file @ f53a3e5a
......@@ -1365,22 +1365,6 @@ describe Gitlab::Database::MigrationHelpers do
end
end
it 'returns the final expected delay' do
Sidekiq::Testing.fake! do
final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2)
expect(final_delay.to_f).to eq(20.minutes.to_f)
end
end
it 'returns zero when nothing gets queued' do
Sidekiq::Testing.fake! do
final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User.none, 'FooJob', 10.minutes)
expect(final_delay).to eq(0)
end
end
context 'with batch_size option' do
it 'queues jobs correctly' do
Sidekiq::Testing.fake! do
......@@ -1405,25 +1389,12 @@ describe Gitlab::Database::MigrationHelpers do
end
end
context 'with other_job_arguments option' do
it 'queues jobs correctly' do
Sidekiq::Testing.fake! do
model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2])
expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]])
expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f)
end
end
end
context 'with initial_delay option' do
context 'with other_arguments option' do
it 'queues jobs correctly' do
Sidekiq::Testing.fake! do
model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2], initial_delay: 10.minutes)
model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_arguments: [1, 2])
expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]])
expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(20.minutes.from_now.to_f)
end
expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]])
expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f)
end
end
end
......
spec/migrations/cleanup_optimistic_locking_nulls_pt2_spec.rb deleted 100644 → 0
View file @ e4d69634
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20200217210353_cleanup_optimistic_locking_nulls_pt2')
describe CleanupOptimisticLockingNullsPt2, :migration do
let(:ci_stages) { table(:ci_stages) }
let(:ci_builds) { table(:ci_builds) }
let(:ci_pipelines) { table(:ci_pipelines) }
let(:tables) { [ci_stages, ci_builds, ci_pipelines] }
before do
# Create necessary rows
ci_stages.create!
ci_builds.create!
ci_pipelines.create!
# Nullify `lock_version` column for all rows
# Needs to be done with a SQL fragment, otherwise Rails will coerce it to 0
tables.each do |table|
table.update_all('lock_version = NULL')
end
end
it 'correctly migrates nullified lock_version column', :sidekiq_might_not_need_inline do
tables.each do |table|
expect(table.where(lock_version: nil).count).to eq(1)
end
tables.each do |table|
expect(table.where(lock_version: 0).count).to eq(0)
end
migrate!
tables.each do |table|
expect(table.where(lock_version: nil).count).to eq(0)
end
tables.each do |table|
expect(table.where(lock_version: 0).count).to eq(1)
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