Merge branch...

Merge branch '495649-solve-null-equality-in-bbm-that-deduplicates-lfs_objects_projects' into 'master' 

Solve null equality in BBM that deduplicates lfs_objects_projects

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167468



Merged-by: Andy Schoenen <asoiron@gitlab.com>
Approved-by: Naman Jagdish Gala <ngala@gitlab.com>
Approved-by: Jarka Košanová <jarka@gitlab.com>
Approved-by: Jerry Seto <jseto@gitlab.com>
Approved-by: Andy Schoenen <asoiron@gitlab.com>
Co-authored-by: Javiera Tapia <jtapia@gitlab.com>

db/docs/batched_background_migrations/deduplicate_lfs_objects_projects.yml

@@ -5,9 +5,13 @@ description: >-
   After the migration is finalized, we need to add a unique index on all three columns to ensure
   data consistency since the unique validation already exists at the model level.
 
-  Originally introduced by https://gitlab.com/gitlab-org/gitlab/-/merge_requests/154323
+  1) Originally introduced by https://gitlab.com/gitlab-org/gitlab/-/merge_requests/154323
   Re-enqueued because we found some duplicates were still being introduced
+
+  2) Originally re-enqueued in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167468
+  Re-enqueued a second time since we found some duplicates when repository_type is NULL
 feature_category: source_code_management
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164127
-milestone: '17.4'
-queued_migration_version: 20240827204855
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167468
+milestone: '17.5'
+queued_migration_version: 20240927202948
+finalize_after: '2024-10-17' # required upgrade stop due date

db/post_migrate/20240827204855_reenqueue_deduplicate_lfs_objects_projects.rb

@@ -13,20 +13,13 @@ class ReenqueueDeduplicateLfsObjectsProjects < Gitlab::Database::Migration[2.2]
 
   restrict_gitlab_migration gitlab_schema: :gitlab_main
 
+  # To be re-enqueued by:
+  # db/post_migrate/20240927202948_reenqueue_deduplicate_lfs_objects_projects_with_null_repository_types.rb
   def up
-    delete_batched_background_migration(MIGRATION, :lfs_objects_projects, :id, [])
-
-    queue_batched_background_migration(
-      MIGRATION,
-      :lfs_objects_projects,
-      :id,
-      job_interval: DELAY_INTERVAL,
-      batch_size: BATCH_SIZE,
-      sub_batch_size: SUB_BATCH_SIZE
-    )
+    # no-op
   end
 
   def down
-    delete_batched_background_migration(MIGRATION, :lfs_objects_projects, :id, [])
+    # no-op
   end
 end

db/post_migrate/20240927202948_reenqueue_deduplicate_lfs_objects_projects_with_null_repository_types.rb

+# frozen_string_literal: true
+
+class ReenqueueDeduplicateLfsObjectsProjectsWithNullRepositoryTypes < Gitlab::Database::Migration[2.2]
+  milestone '17.5'
+
+  MIGRATION = 'DeduplicateLfsObjectsProjects'
+  TABLE_NAME = :lfs_objects_projects
+  DELAY_INTERVAL = 100
+  BATCH_SIZE = 10_000
+  SUB_BATCH_SIZE = 2_500
+
+  disable_ddl_transaction!
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  def up
+    delete_batched_background_migration(MIGRATION, :lfs_objects_projects, :id, [])
+
+    queue_batched_background_migration(
+      MIGRATION,
+      :lfs_objects_projects,
+      :id,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      sub_batch_size: SUB_BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, :lfs_objects_projects, :id, [])
+  end
+end

db/schema_migrations/20240927202948

+b693c1b11d18d4dae7281af0e5784c91bd285d1e366baae96c00d22f3d0e05d8
\ No newline at end of file

lib/gitlab/background_migration/deduplicate_lfs_objects_projects.rb

@@ -34,7 +34,7 @@ def perform
             INNER JOIN (#{subquery.to_sql}) AS duplicates
             ON lfs_objects_projects.project_id = duplicates.project_id
             AND lfs_objects_projects.lfs_object_id = duplicates.lfs_object_id
-            AND lfs_objects_projects.repository_type = duplicates.repository_type
+            AND lfs_objects_projects.repository_type IS NOT DISTINCT FROM duplicates.repository_type
           SQL
 
           duplicated_lfs_objects_projects = LfsObjectsProject.joins(join_query).where.not(

spec/lib/gitlab/background_migration/deduplicate_lfs_objects_projects_spec.rb

@@ -68,15 +68,15 @@
   end
 
   let!(:duplicated_lfs_objects_project3) do
-    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: 0)
+    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: nil)
   end
 
   let!(:duplicated_lfs_objects_project4) do
-    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: 0)
+    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: nil)
   end
 
   let!(:lfs_objects_project5) do
-    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: 0)
+    lfs_objects_projects_table.create!(project_id: project2.id, lfs_object_id: lfs_object4.id, repository_type: nil)
   end
 
   let!(:lfs_objects_project6) do

spec/migrations/20240827204855_reenqueue_deduplicate_lfs_objects_projects_spec.rb

@@ -7,20 +7,7 @@
   let(:batched_migration) { described_class::MIGRATION }
 
   it 'schedules a new batched migration' do
-    reversible_migration do |migration|
-      migration.before -> {
-        expect(batched_migration).not_to have_scheduled_batched_migration
-      }
-
-      migration.after -> {
-        expect(batched_migration).to have_scheduled_batched_migration(
-          table_name: :lfs_objects_projects,
-          column_name: :id,
-          batch_class_name: described_class::BATCH_CLASS_NAME,
-          batch_size: described_class::BATCH_SIZE,
-          sub_batch_size: described_class::SUB_BATCH_SIZE
-        )
-      }
-    end
+    migrate!
+    expect(batched_migration).not_to have_scheduled_batched_migration
   end
 end

spec/migrations/20240927202948_reenqueue_deduplicate_lfs_objects_projects_with_null_repository_types_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe ReenqueueDeduplicateLfsObjectsProjectsWithNullRepositoryTypes, feature_category: :source_code_management do
+  let(:batched_migration) { described_class::MIGRATION }
+
+  it 'schedules a new batched migration' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(batched_migration).not_to have_scheduled_batched_migration
+      }
+
+      migration.after -> {
+        expect(batched_migration).to have_scheduled_batched_migration(
+          table_name: :lfs_objects_projects,
+          column_name: :id,
+          batch_class_name: described_class::BATCH_CLASS_NAME,
+          batch_size: described_class::BATCH_SIZE,
+          sub_batch_size: described_class::SUB_BATCH_SIZE
+        )
+      }
+    end
+  end
+end