From 5ab2f3b879e79955dabac5c01f4019e0da07f49e Mon Sep 17 00:00:00 2001 From: Maxime Orefice Date: Wed, 8 Feb 2023 11:14:52 +0100 Subject: [PATCH 1/2] Fix partition_id for ci_pipeline_variables Changelog: fixed --- ..._partition_ids_for_ci_pipeline_variable.rb | 23 ++++++++++ db/schema_migrations/20230208100917 | 1 + ...ition_ids_for_ci_pipeline_variable_spec.rb | 42 +++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb create mode 100644 db/schema_migrations/20230208100917 create mode 100644 spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb diff --git a/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb b/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb new file mode 100644 index 00000000000000..1a266593821d63 --- /dev/null +++ b/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class FixPartitionIdsForCiPipelineVariable < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_ci + + BATCH_SIZE = 100 + + def up + return unless Gitlab.com? + + define_batchable_model(:ci_pipeline_variables) + .where(partition_id: 101) + .each_batch(of: BATCH_SIZE) do |batch| + batch.update_all(partition_id: 100) + sleep rand + end + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20230208100917 b/db/schema_migrations/20230208100917 new file mode 100644 index 00000000000000..1862dcf6738d93 --- /dev/null +++ b/db/schema_migrations/20230208100917 @@ -0,0 +1 @@ +55d9815e9590681cedf6e5817ef6419db882837325b9d3c86ae34419339b090d \ No newline at end of file diff --git a/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb b/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb new file mode 100644 index 00000000000000..3004b1f060dfb1 --- /dev/null +++ b/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe FixPartitionIdsForCiPipelineVariable, migration: :gitlab_ci, feature_category: :continuous_integration do + let(:pipeline_variables) { table(:ci_pipeline_variables, database: :ci) } + let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } + + before do + pipeline = ci_pipelines.create!(partition_id: 100) + + pipeline_variables.insert_all!([ + { pipeline_id: pipeline.id, partition_id: 100, key: 'variable-100' }, + { pipeline_id: pipeline.id, partition_id: 101, key: 'variable-101' } + ]) + end + + describe '#up' do + context 'when on sass' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'fixes partition_id' do + expect { migrate! }.not_to raise_error + + expect(pipeline_variables.where(partition_id: 100).count).to eq(2) + expect(pipeline_variables.where(partition_id: 101).count).to eq(0) + end + end + + context 'when on self managed' do + it 'does not change partition_id' do + expect { migrate! }.not_to raise_error + + expect(pipeline_variables.where(partition_id: 100).count).to eq(1) + expect(pipeline_variables.where(partition_id: 101).count).to eq(1) + end + end + end +end -- GitLab From b1d55867e72866ae9919ddd9107f52e9c146c655 Mon Sep 17 00:00:00 2001 From: Maxime Orefice Date: Wed, 8 Feb 2023 14:22:06 +0100 Subject: [PATCH 2/2] Use batched migration --- ..._partition_ids_for_ci_pipeline_variable.rb | 27 +++++---- ...ition_ids_for_ci_pipeline_variable_spec.rb | 60 ++++++++++++------- 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb b/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb index 1a266593821d63..9901e6af4aee0a 100644 --- a/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb +++ b/db/post_migrate/20230208100917_fix_partition_ids_for_ci_pipeline_variable.rb @@ -1,23 +1,30 @@ # frozen_string_literal: true class FixPartitionIdsForCiPipelineVariable < Gitlab::Database::Migration[2.1] - disable_ddl_transaction! - restrict_gitlab_migration gitlab_schema: :gitlab_ci + MIGRATION = 'RebalancePartitionId' + DELAY_INTERVAL = 2.minutes.freeze + TABLE = :ci_pipeline_variables + BATCH_SIZE = 2_000 + SUB_BATCH_SIZE = 200 - BATCH_SIZE = 100 + restrict_gitlab_migration gitlab_schema: :gitlab_ci def up return unless Gitlab.com? - define_batchable_model(:ci_pipeline_variables) - .where(partition_id: 101) - .each_batch(of: BATCH_SIZE) do |batch| - batch.update_all(partition_id: 100) - sleep rand - end + queue_batched_background_migration( + MIGRATION, + TABLE, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) end def down - # no-op + return unless Gitlab.com? + + delete_batched_background_migration(MIGRATION, TABLE, :id, []) end end diff --git a/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb b/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb index 3004b1f060dfb1..fb0e1fe17ec4c1 100644 --- a/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb +++ b/spec/migrations/20230208100917_fix_partition_ids_for_ci_pipeline_variable_spec.rb @@ -4,38 +4,54 @@ require_migration! RSpec.describe FixPartitionIdsForCiPipelineVariable, migration: :gitlab_ci, feature_category: :continuous_integration do - let(:pipeline_variables) { table(:ci_pipeline_variables, database: :ci) } - let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } + let(:migration) { described_class::MIGRATION } - before do - pipeline = ci_pipelines.create!(partition_id: 100) + context 'when on saas' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end - pipeline_variables.insert_all!([ - { pipeline_id: pipeline.id, partition_id: 100, key: 'variable-100' }, - { pipeline_id: pipeline.id, partition_id: 101, key: 'variable-101' } - ]) - end + describe '#up' do + it 'schedules background jobs for each batch of ci_pipeline_variables' do + migrate! + + expect(migration).to have_scheduled_batched_migration( + gitlab_schema: :gitlab_ci, + table_name: :ci_pipeline_variables, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + end + end - describe '#up' do - context 'when on sass' do - before do - allow(Gitlab).to receive(:com?).and_return(true) + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration end + end + end + + context 'when on self-managed instance' do + let(:migration) { described_class.new } - it 'fixes partition_id' do - expect { migrate! }.not_to raise_error + describe '#up' do + it 'does not schedule background job' do + expect(migration).not_to receive(:queue_batched_background_migration) - expect(pipeline_variables.where(partition_id: 100).count).to eq(2) - expect(pipeline_variables.where(partition_id: 101).count).to eq(0) + migration.up end end - context 'when on self managed' do - it 'does not change partition_id' do - expect { migrate! }.not_to raise_error + describe '#down' do + it 'does not delete background job' do + expect(migration).not_to receive(:delete_batched_background_migration) - expect(pipeline_variables.where(partition_id: 100).count).to eq(1) - expect(pipeline_variables.where(partition_id: 101).count).to eq(1) + migration.down end end end -- GitLab