fix the increase lock timeout on retry

Author: CharlesDelannoy

lib/safe-pg-migrations/plugins/statement_retrier.rb

@@ -13,17 +13,14 @@ module StatementRetrier
     private
 
     def retry_if_lock_timeout
-      initial_lock_timeout = SafePgMigrations.config.lock_timeout
+      @lock_timeout = SafePgMigrations.config.lock_timeout
       number_of_retries = 0
       begin
         number_of_retries += 1
         yield
-      rescue ActiveRecord::LockWaitTimeout
+      rescue ActiveRecord::LockWaitTimeout => e
         # Retrying is useless if we're inside a transaction.
-        if transaction_open? || number_of_retries >= SafePgMigrations.config.max_tries
-          SafePgMigrations.config.lock_timeout = initial_lock_timeout
-          raise
-        end
+        raise e if transaction_open? || number_of_retries >= SafePgMigrations.config.max_tries
 
         retry_delay = SafePgMigrations.config.retry_delay
         Helpers::Logger.say "Retrying in #{retry_delay} seconds...", sub_item: true
@@ -39,20 +36,23 @@ def retry_if_lock_timeout
     end
 
     def increase_lock_timeout
-      Helpers::Logger.say "  Increasing the lock timeout... Currently set to #{SafePgMigrations.config.lock_timeout}",
+      Helpers::Logger.say "  Increasing the lock timeout... Currently set to #{@lock_timeout} seconds",
                           sub_item: true
-      SafePgMigrations.config.lock_timeout = (SafePgMigrations.config.lock_timeout + lock_timeout_step)
-      unless SafePgMigrations.config.lock_timeout < SafePgMigrations.config.max_lock_timeout_for_retry
-        SafePgMigrations.config.lock_timeout = SafePgMigrations.config.max_lock_timeout_for_retry
+      @lock_timeout += lock_timeout_step
+      unless @lock_timeout < SafePgMigrations.config.max_lock_timeout_for_retry
+        @lock_timeout = SafePgMigrations.config.max_lock_timeout_for_retry
       end
-      Helpers::Logger.say "  Lock timeout is now set to #{SafePgMigrations.config.lock_timeout}", sub_item: true
+      execute("SET lock_timeout TO '#{@lock_timeout}s'")
+      Helpers::Logger.say "  Lock timeout is now set to #{@lock_timeout} seconds", sub_item: true
     end
 
     def lock_timeout_step
+      return @lock_timeout_step if defined?(@lock_timeout_step)
+
       max_lock_timeout_for_retry = SafePgMigrations.config.max_lock_timeout_for_retry
       lock_timeout = SafePgMigrations.config.lock_timeout
       max_tries = SafePgMigrations.config.max_tries
-      @lock_timeout_step ||= (max_lock_timeout_for_retry - lock_timeout) / (max_tries - 1)
+      @lock_timeout_step = (max_lock_timeout_for_retry - lock_timeout) / (max_tries - 1)
     end
   end
 end

test/statement_retrier_test.rb

@@ -7,26 +7,28 @@ def test_lock_timeout_increase_on_retry
     SafePgMigrations.config.lock_timeout = 0.1.seconds
     SafePgMigrations.config.increase_lock_timeout_on_retry = true
 
-    calls = calls_for_lock_timeout_migration
+    2.times do
+      calls = calls_for_lock_timeout_migration
 
-    assert_equal [
-      '   -> Retrying in 60 seconds...',
-      '   ->   Increasing the lock timeout... Currently set to 0.1',
-      '   ->   Lock timeout is now set to 0.325',
-      '   -> Retrying now.',
-      '   -> Retrying in 60 seconds...',
-      '   ->   Increasing the lock timeout... Currently set to 0.325',
-      '   ->   Lock timeout is now set to 0.55',
-      '   -> Retrying now.',
-      '   -> Retrying in 60 seconds...',
-      '   ->   Increasing the lock timeout... Currently set to 0.55',
-      '   ->   Lock timeout is now set to 0.775',
-      '   -> Retrying now.',
-      '   -> Retrying in 60 seconds...',
-      '   ->   Increasing the lock timeout... Currently set to 0.775',
-      '   ->   Lock timeout is now set to 1',
-      '   -> Retrying now.',
-    ], calls[1..].map(&:first)
+      assert_equal [
+        '   -> Retrying in 60 seconds...',
+        '   ->   Increasing the lock timeout... Currently set to 0.1 seconds',
+        '   ->   Lock timeout is now set to 0.325 seconds',
+        '   -> Retrying now.',
+        '   -> Retrying in 60 seconds...',
+        '   ->   Increasing the lock timeout... Currently set to 0.325 seconds',
+        '   ->   Lock timeout is now set to 0.55 seconds',
+        '   -> Retrying now.',
+        '   -> Retrying in 60 seconds...',
+        '   ->   Increasing the lock timeout... Currently set to 0.55 seconds',
+        '   ->   Lock timeout is now set to 0.775 seconds',
+        '   -> Retrying now.',
+        '   -> Retrying in 60 seconds...',
+        '   ->   Increasing the lock timeout... Currently set to 0.775 seconds',
+        '   ->   Lock timeout is now set to 1 seconds',
+        '   -> Retrying now.',
+      ], calls[1..].map(&:first)
+    end
   end
 
   def test_no_lock_timeout_increase_on_retry_if_disabled