Do not wait before running retried background migrations

fatkodima · Sep 26, 2024 · 16628dc · 16628dc
1 parent abd776b
commit 16628dc
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## master (unreleased)
 
+- Do not wait before running retried background migrations
+
 ## 0.19.6 (2024-09-26)
 
 - Fix `add_refernce_concurrently` when adding non polymorphic references

diff --git a/lib/online_migrations/background_migrations/migration_job.rb b/lib/online_migrations/background_migrations/migration_job.rb
@@ -17,7 +17,7 @@ class MigrationJob < ApplicationRecord
       scope :completed, -> { where(status: [:failed, :succeeded]) }
       scope :stuck, -> do
         timeout = OnlineMigrations.config.background_migrations.stuck_jobs_timeout
-        active.where("updated_at <= ?", timeout.seconds.ago)
+        running.where("updated_at <= ?", timeout.seconds.ago)
       end
 
       scope :retriable, -> do

diff --git a/lib/online_migrations/background_migrations/migration_runner.rb b/lib/online_migrations/background_migrations/migration_runner.rb
@@ -115,7 +115,7 @@ def find_or_create_next_migration_job
           if min_value && max_value
             create_migration_job!(min_value, max_value)
           else
-            migration.migration_jobs.retriable.first
+            migration.migration_jobs.enqueued.first || migration.migration_jobs.retriable.first
           end
         end
 

diff --git a/lib/online_migrations/background_schema_migrations/migration.rb b/lib/online_migrations/background_schema_migrations/migration.rb
@@ -123,7 +123,7 @@ def stuck?
         return false if composite?
 
         stuck_timeout = (statement_timeout || 1.day) + 10.minutes
-        (enqueued? || running?) && updated_at <= stuck_timeout.seconds.ago
+        running? && updated_at <= stuck_timeout.seconds.ago
       end
 
       # Mark this migration as ready to be processed again.
@@ -134,7 +134,7 @@ def retry
         if composite? && failed?
           children.failed.each(&:retry)
           update!(
-            status: self.class.statuses[:running],
+            status: self.class.statuses[:enqueued],
             finished_at: nil
           )
           true

diff --git a/test/background_schema_migrations/migration_test.rb b/test/background_schema_migrations/migration_test.rb
@@ -191,7 +191,7 @@ def test_retry_composite
       assert_nil m.finished_at
 
       migrations.each(&:reload)
-      assert m.running?
+      assert m.enqueued?
       assert child1.enqueued?
       assert child3.succeeded?
     end