From a3ff4595d4c06fa0cd24c219f4e1bcf48f30e554 Mon Sep 17 00:00:00 2001
From: Prabakaran Murugesan <pmurugesan@gitlab.com>
Date: Sun, 10 Dec 2023 23:32:11 +0000
Subject: [PATCH] Safely assigns queued_migration_version on BBM enqueue
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/433529
---
...id_protected_branch_merge_access_levels.rb | 1 -
...lid_protected_branch_push_access_levels.rb | 1 -
...alid_protected_tag_create_access_levels.rb | 1 -
...queue_backfill_packages_tags_project_id.rb | 1 -
...backfill_merge_request_diffs_project_id.rb | 1 -
...has_remediations_of_vulnerability_reads.rb | 1 -
.../database/batched_background_migrations.md | 65 ++++++++-----------
.../database/not_null_constraints.md | 3 +-
...ueue_batched_background_migration.template | 1 -
.../batched_background_migration_helpers.rb | 11 ++--
.../queue_my_batched_migration.txt | 1 -
...tched_background_migration_helpers_spec.rb | 6 +-
12 files changed, 35 insertions(+), 58 deletions(-)
diff --git a/db/post_migrate/20231016173129_queue_delete_invalid_protected_branch_merge_access_levels.rb b/db/post_migrate/20231016173129_queue_delete_invalid_protected_branch_merge_access_levels.rb
index 3f4009d783c1a..8d6173fc7ca9b 100644
--- a/db/post_migrate/20231016173129_queue_delete_invalid_protected_branch_merge_access_levels.rb
+++ b/db/post_migrate/20231016173129_queue_delete_invalid_protected_branch_merge_access_levels.rb
@@ -16,7 +16,6 @@ def up
:protected_branch_merge_access_levels,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231016173129',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/db/post_migrate/20231016194927_queue_delete_invalid_protected_branch_push_access_levels.rb b/db/post_migrate/20231016194927_queue_delete_invalid_protected_branch_push_access_levels.rb
index 6accaa3296bdf..32022ff8be0aa 100644
--- a/db/post_migrate/20231016194927_queue_delete_invalid_protected_branch_push_access_levels.rb
+++ b/db/post_migrate/20231016194927_queue_delete_invalid_protected_branch_push_access_levels.rb
@@ -16,7 +16,6 @@ def up
:protected_branch_push_access_levels,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231016194927',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/db/post_migrate/20231016194943_queue_delete_invalid_protected_tag_create_access_levels.rb b/db/post_migrate/20231016194943_queue_delete_invalid_protected_tag_create_access_levels.rb
index 5880124d0a60d..f96f1c3b380ca 100644
--- a/db/post_migrate/20231016194943_queue_delete_invalid_protected_tag_create_access_levels.rb
+++ b/db/post_migrate/20231016194943_queue_delete_invalid_protected_tag_create_access_levels.rb
@@ -15,7 +15,6 @@ def up
:protected_tag_create_access_levels,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231016194943',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/db/post_migrate/20231030071209_queue_backfill_packages_tags_project_id.rb b/db/post_migrate/20231030071209_queue_backfill_packages_tags_project_id.rb
index 4984eb83263a6..1197e31dea8ae 100644
--- a/db/post_migrate/20231030071209_queue_backfill_packages_tags_project_id.rb
+++ b/db/post_migrate/20231030071209_queue_backfill_packages_tags_project_id.rb
@@ -16,7 +16,6 @@ def up
:packages_tags,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231030071209',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/db/post_migrate/20231114043522_queue_backfill_merge_request_diffs_project_id.rb b/db/post_migrate/20231114043522_queue_backfill_merge_request_diffs_project_id.rb
index 0dc15f5c56f97..19448243fdc0b 100644
--- a/db/post_migrate/20231114043522_queue_backfill_merge_request_diffs_project_id.rb
+++ b/db/post_migrate/20231114043522_queue_backfill_merge_request_diffs_project_id.rb
@@ -15,7 +15,6 @@ def up
:merge_request_diffs,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231114043522',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/db/post_migrate/20231201204712_requeue2_backfill_has_remediations_of_vulnerability_reads.rb b/db/post_migrate/20231201204712_requeue2_backfill_has_remediations_of_vulnerability_reads.rb
index e49a49d791262..35b6de08f1083 100644
--- a/db/post_migrate/20231201204712_requeue2_backfill_has_remediations_of_vulnerability_reads.rb
+++ b/db/post_migrate/20231201204712_requeue2_backfill_has_remediations_of_vulnerability_reads.rb
@@ -22,7 +22,6 @@ def up
:vulnerability_reads,
:vulnerability_id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231201204712',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/doc/development/database/batched_background_migrations.md b/doc/development/database/batched_background_migrations.md
index 4605447f25671..03f5eedb7f805 100644
--- a/doc/development/database/batched_background_migrations.md
+++ b/doc/development/database/batched_background_migrations.md
@@ -672,49 +672,37 @@ The following process has been configured to make dependencies more evident whil
Example:
```ruby
- class QueueBackfillRoutesNamespaceId < Gitlab::Database::Migration[2.1]
- MIGRATION = 'BackfillRouteNamespaceId'
-....
- restrict_gitlab_migration gitlab_schema: :gitlab_main
+# db/post_migrate/20231113120650_queue_backfill_routes_namespace_id.rb
+class QueueBackfillRoutesNamespaceId < Gitlab::Database::Migration[2.1]
+ MIGRATION = 'BackfillRouteNamespaceId'
- def up
- queue_batched_background_migration(
- ...
- queued_migration_version: '20231113120650',
- ...
- )
- end
-...
- end
- ```
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+ ...
+ ...
- ```ruby
- class NewQueueBackfillRoutesNamespaceId < Gitlab::Database::Migration[2.1]
- MIGRATION = 'NewBackfillRouteNamespaceId'
- DELAY_INTERVAL = 2.minutes
- BATCH_SIZE = 1000
- SUB_BATCH_SIZE = 100
- DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = ["20231113120650"]
+ def up
+ queue_batched_background_migration(
+ MIGRATION,
+ ...
+ )
+ end
+end
+```
- restrict_gitlab_migration gitlab_schema: :gitlab_main
+```ruby
+# This depends on the finalization of QueueBackfillRoutesNamespaceId BBM
+class AddNotNullToRoutesNamespaceId < Gitlab::Database::Migration[2.1]
+ DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = ["20231113120650"]
- def up
- queue_batched_background_migration(
- MIGRATION,
- :routes,
- :id,
- job_interval: DELAY_INTERVAL,
- queued_migration_version: '20241213120651',
- batch_size: BATCH_SIZE,
- sub_batch_size: SUB_BATCH_SIZE
- )
- end
+ def up
+ add_not_null_constraint :routes, :namespace_id
+ end
- def down
- delete_batched_background_migration(MIGRATION, :routes, :id, [])
- end
- end
- ```
+ def down
+ remove_not_null_constraint :routes, :namespace_id
+ end
+end
+```
#### Notes
@@ -1062,7 +1050,6 @@ background migration.
:routes,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231113120650',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/doc/development/database/not_null_constraints.md b/doc/development/database/not_null_constraints.md
index 7ffc1fba1a0b1..5615edfcc2a65 100644
--- a/doc/development/database/not_null_constraints.md
+++ b/doc/development/database/not_null_constraints.md
@@ -242,8 +242,7 @@ scheduled after the background migration has completed, which could be several r
MIGRATION,
:merge_request_diffs,
:id,
- job_interval: DELAY_INTERVAL,
- queued_migration_version: '20231114043522'
+ job_interval: DELAY_INTERVAL
)
end
diff --git a/lib/generators/batched_background_migration/templates/queue_batched_background_migration.template b/lib/generators/batched_background_migration/templates/queue_batched_background_migration.template
index df4c538274911..37d67194c591f 100644
--- a/lib/generators/batched_background_migration/templates/queue_batched_background_migration.template
+++ b/lib/generators/batched_background_migration/templates/queue_batched_background_migration.template
@@ -19,7 +19,6 @@ class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Data
:<%= table_name %>,
:<%= column_name %>,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '<%= migration_number %>',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb
index 2db03b1577004..39706582e3cab 100644
--- a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb
+++ b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb
@@ -38,10 +38,6 @@ module BatchedBackgroundMigrationHelpers
# batch_class_name - The name of the class that will be called to find the range of each next batch
# batch_size - The maximum number of rows per job
# sub_batch_size - The maximum number of rows processed per "iteration" within the job
- # queued_migration_version - Version of the migration that queues the BBM, this is used to establish dependecies
- #
- # queued_migration_version is made optional temporarily to allow prior migrations to not fail,
- # https://gitlab.com/gitlab-org/gitlab/-/issues/426417 will make it mandatory.
#
# *Returns the created BatchedMigration record*
#
@@ -67,7 +63,6 @@ def queue_batched_background_migration( # rubocop:disable Metrics/ParameterLists
batch_column_name,
*job_arguments,
job_interval:,
- queued_migration_version: nil,
batch_min_value: BATCH_MIN_VALUE,
batch_max_value: nil,
batch_class_name: BATCH_CLASS_NAME,
@@ -80,6 +75,8 @@ def queue_batched_background_migration( # rubocop:disable Metrics/ParameterLists
Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_dml_mode!
gitlab_schema ||= gitlab_schema_from_context
+ # Version of the migration that queued the BBM, this is used to establish dependencies
+ queued_migration_version = version
Gitlab::Database::BackgroundMigration::BatchedMigration.reset_column_information
@@ -120,7 +117,7 @@ def queue_batched_background_migration( # rubocop:disable Metrics/ParameterLists
"(given #{job_arguments.count}, expected #{migration.job_class.job_arguments_count})"
end
- assign_attribtues_safely(
+ assign_attributes_safely(
migration,
max_batch_size,
batch_table_name,
@@ -246,7 +243,7 @@ def ensure_batched_background_migration_is_finished(job_class_name:, table_name:
# about columns introduced later on because this model is not
# isolated in migrations, which is why we need to check for existence
# of these columns first.
- def assign_attribtues_safely(migration, max_batch_size, batch_table_name, gitlab_schema, queued_migration_version)
+ def assign_attributes_safely(migration, max_batch_size, batch_table_name, gitlab_schema, queued_migration_version)
# We keep track of the estimated number of tuples in 'total_tuple_count' to reason later
# about the overall progress of a migration.
safe_attributes_value = {
diff --git a/spec/lib/generators/batched_background_migration/expected_files/queue_my_batched_migration.txt b/spec/lib/generators/batched_background_migration/expected_files/queue_my_batched_migration.txt
index 36f7885b5919c..d1fab7cf4bd64 100644
--- a/spec/lib/generators/batched_background_migration/expected_files/queue_my_batched_migration.txt
+++ b/spec/lib/generators/batched_background_migration/expected_files/queue_my_batched_migration.txt
@@ -19,7 +19,6 @@ class QueueMyBatchedMigration < Gitlab::Database::Migration[2.2]
:projects,
:id,
job_interval: DELAY_INTERVAL,
- queued_migration_version: '<migration_version>',
batch_size: BATCH_SIZE,
sub_batch_size: SUB_BATCH_SIZE
)
diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
index a81ccf9583a56..5c98379d85265 100644
--- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
@@ -71,8 +71,11 @@ def self.name
end
context "when the migration doesn't exist already" do
+ let(:version) { '20231204101122' }
+
before do
allow(Gitlab::Database::PgClass).to receive(:for_table).with(:projects).and_return(pgclass_info)
+ allow(migration).to receive(:version).and_return(version)
end
subject(:enqueue_batched_background_migration) do
@@ -81,7 +84,6 @@ def self.name
:projects,
:id,
job_interval: 5.minutes,
- queued_migration_version: format("%.14d", 123),
batch_min_value: 5,
batch_max_value: 1000,
batch_class_name: 'MyBatchClass',
@@ -115,7 +117,7 @@ def self.name
status_name: :active,
total_tuple_count: pgclass_info.cardinality_estimate,
gitlab_schema: 'gitlab_ci',
- queued_migration_version: format("%.14d", 123)
+ queued_migration_version: version
)
end
end
--
GitLab