Merge branch 'bvl-fix-concurrent-fork-network-migrations' into 'master'

changelogs/unreleased/bvl-fix-concurrent-fork-network-migrations.yml

+---
+title: Avoid running `PopulateForkNetworksRange`-migration multiple times
+merge_request: 16988
+author:
+type: fixed

db/migrate/20170929131201_populate_fork_networks.rb

@@ -6,22 +6,8 @@ class PopulateForkNetworks < ActiveRecord::Migration
 
   DOWNTIME = false
 
-  MIGRATION = 'PopulateForkNetworksRange'.freeze
-  BATCH_SIZE = 100
-  DELAY_INTERVAL = 15.seconds
-
-  disable_ddl_transaction!
-
-  class ForkedProjectLink < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'forked_project_links'
-  end
-
   def up
-    say 'Populating the `fork_networks` based on existing `forked_project_links`'
-
-    queue_background_migration_jobs_by_range_at_intervals(ForkedProjectLink, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+    say 'Fork networks will be populated in 20171205190711 - RescheduleForkNetworkCreationCaller'
   end
 
   def down

db/post_migrate/20171124150326_reschedule_fork_network_creation.rb

@@ -3,22 +3,8 @@ class RescheduleForkNetworkCreation < ActiveRecord::Migration
 
   DOWNTIME = false
 
-  MIGRATION = 'PopulateForkNetworksRange'.freeze
-  BATCH_SIZE = 100
-  DELAY_INTERVAL = 15.seconds
-
-  disable_ddl_transaction!
-
-  class ForkedProjectLink < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'forked_project_links'
-  end
-
   def up
-    say 'Populating the `fork_networks` based on existing `forked_project_links`'
-
-    queue_background_migration_jobs_by_range_at_intervals(ForkedProjectLink, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+    say 'Fork networks will be populated in 20171205190711 - RescheduleForkNetworkCreationCaller'
   end
 
   def down

doc/development/background_migrations.md

@@ -94,6 +94,18 @@ jobs = [['BackgroundMigrationClassName', [1]],
 BackgroundMigrationWorker.bulk_perform_in(5.minutes, jobs)
 ```
 
+### Rescheduling background migrations
+
+If one of the background migrations contains a bug that is fixed in a patch
+release, the background migration needs to be rescheduled so the migration would
+be repeated on systems that already performed the initial migration.
+
+When you reschedule the background migration, make sure to turn the original
+scheduling into a no-op by clearing up the `#up` and `#down` methods of the
+migration performing the scheduling. Otherwise the background migration would be
+scheduled multiple times on systems that are upgrading multiple patch releases at
+once.
+
 ## Cleaning Up
 
 >**Note:**

lib/gitlab/background_migration/create_fork_network_memberships_range.rb

@@ -14,6 +14,14 @@ module Gitlab
       def perform(start_id, end_id)
         log("Creating memberships for forks: #{start_id} - #{end_id}")
 
+        insert_members(start_id, end_id)
+
+        if missing_members?(start_id, end_id)
+          BackgroundMigrationWorker.perform_in(RESCHEDULE_DELAY, "CreateForkNetworkMembershipsRange", [start_id, end_id])
+        end
+      end
+
+      def insert_members(start_id, end_id)
         ActiveRecord::Base.connection.execute <<~INSERT_MEMBERS
           INSERT INTO fork_network_members (fork_network_id, project_id, forked_from_project_id)
 
@@ -33,10 +41,9 @@ module Gitlab
             WHERE existing_members.project_id = forked_project_links.forked_to_project_id
           )
         INSERT_MEMBERS
-
-        if missing_members?(start_id, end_id)
-          BackgroundMigrationWorker.perform_in(RESCHEDULE_DELAY, "CreateForkNetworkMembershipsRange", [start_id, end_id])
-        end
+      rescue ActiveRecord::RecordNotUnique => e
+        # `fork_network_member` was created concurrently in another migration
+        log(e.message)
       end
 
       def missing_members?(start_id, end_id)