Fix retrying parent migration when child is retried

fatkodima · Jan 25, 2025 · 02a0445 · 02a0445
1 parent 867343e
commit 02a0445
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 20 deletions.
diff --git a/lib/online_migrations/background_migrations/migration.rb b/lib/online_migrations/background_migrations/migration.rb
@@ -189,15 +189,23 @@ def interval_elapsed?
       #
       def retry
         if composite? && failed?
-          children.failed.each(&:retry)
-          enqueued!
+          transaction do
+            enqueued!
+            children.failed.each(&:retry)
+          end
+
           true
         elsif failed?
-          iterator = BatchIterator.new(migration_jobs.failed)
-          iterator.each_batch(of: 100) do |batch|
-            batch.each(&:retry)
+          transaction do
+            parent.enqueued! if parent
+            enqueued!
+
+            iterator = BatchIterator.new(migration_jobs.failed)
+            iterator.each_batch(of: 100) do |batch|
+              batch.each(&:retry)
+            end
           end
-          enqueued!
+
           true
         else
           false

diff --git a/lib/online_migrations/background_schema_migrations/migration.rb b/lib/online_migrations/background_schema_migrations/migration.rb
@@ -139,22 +139,27 @@ def stuck?
       #
       def retry
         if composite? && failed?
-          children.failed.each(&:retry)
-          update!(
-            status: self.class.statuses[:enqueued],
-            finished_at: nil
-          )
+          transaction do
+            update!(status: :enqueued, finished_at: nil)
+            children.failed.each(&:retry)
+          end
+
           true
         elsif failed?
-          update!(
-            status: self.class.statuses[:enqueued],
-            attempts: 0,
-            started_at: nil,
-            finished_at: nil,
-            error_class: nil,
-            error_message: nil,
-            backtrace: nil
-          )
+          transaction do
+            parent.update!(status: :enqueued, finished_at: nil) if parent
+
+            update!(
+              status: :enqueued,
+              attempts: 0,
+              started_at: nil,
+              finished_at: nil,
+              error_class: nil,
+              error_message: nil,
+              backtrace: nil
+            )
+          end
+
           true
         else
           false

diff --git a/test/background_migrations/migration_test.rb b/test/background_migrations/migration_test.rb
@@ -301,6 +301,14 @@ def test_retry_composite
       assert child1.enqueued?
       assert child3.succeeded?
       assert child1.migration_jobs.all?(&:enqueued?)
+
+      # Retrying a child should retry a parent.
+      m.update_column(:status, "failed")
+      child1.update_column(:status, "failed")
+
+      assert child1.retry
+      assert child1.reload.enqueued?
+      assert m.reload.enqueued?
     end
 
     def test_started_at

diff --git a/test/background_schema_migrations/migration_test.rb b/test/background_schema_migrations/migration_test.rb
@@ -194,6 +194,14 @@ def test_retry_composite
       assert m.enqueued?
       assert child1.enqueued?
       assert child3.succeeded?
+
+      # Retrying a child should retry a parent.
+      m.update_column(:status, "failed")
+      child1.update_column(:status, "failed")
+
+      assert child1.retry
+      assert child1.reload.enqueued?
+      assert m.reload.enqueued?
     end
 
     private