Add tests for lock timeout increase

Author: CharlesDelannoy

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

@@ -25,25 +25,28 @@ def retry_if_lock_timeout
           raise
         end
 
-        increase_lock_timeout unless SafePgMigrations.config.lock_timeout.nil?
-
         retry_delay = SafePgMigrations.config.retry_delay
         Helpers::Logger.say "Retrying in #{retry_delay} seconds...", sub_item: true
+
+        increase_lock_timeout unless SafePgMigrations.config.lock_timeout.nil?
+
         sleep retry_delay
         Helpers::Logger.say 'Retrying now.', sub_item: true
         retry
       end
     end
 
     def increase_lock_timeout
-      SafePgMigrations.config.lock_timeout += lock_timeout_step
+      Helpers::Logger.say "  Increasing the lock timeout... Currently set to #{SafePgMigrations.config.lock_timeout}", 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
       end
+      Helpers::Logger.say "  Lock timeout is now set to #{SafePgMigrations.config.lock_timeout}", sub_item: true
     end
 
     def lock_timeout_step
-      (SafePgMigrations.config.max_lock_timeout_for_retry - SafePgMigrations.config.lock_timeout) / SafePgMigrations.config.max_tries
+      @lock_timeout_step ||= (SafePgMigrations.config.max_lock_timeout_for_retry - SafePgMigrations.config.lock_timeout) / (SafePgMigrations.config.max_tries - 1)
     end
   end
 end

test/statement_retrier_test.rb

@@ -4,24 +4,8 @@
 
 class StatementRetrierTest < Minitest::Test
   def test_retry_if_lock_timeout
-    @migration =
-      Class.new(ActiveRecord::Migration::Current) do
-        def up
-          connection.send(:retry_if_lock_timeout) do
-            raise ActiveRecord::LockWaitTimeout, 'PG::LockNotAvailable: ERROR:  canceling statement due to lock timeout'
-          end
-        end
-      end.new
+    calls = calls_for_lock_timeout_migration
 
-    @connection.expects(:sleep).times(4)
-    calls =
-      record_calls(@migration, :write) do
-        run_migration
-        flunk 'run_migration should raise'
-      rescue StandardError => e
-        assert_instance_of ActiveRecord::LockWaitTimeout, e.cause
-        assert_includes e.cause.message, 'canceling statement due to lock timeout'
-      end
     assert_equal [
       '   -> Retrying in 60 seconds...',
       '   -> Retrying now.',
@@ -49,7 +33,54 @@ def test_statement_retry
     ], calls[7..9]
   end
 
-  def lock_timout_increase_on_retry
+  def test_lock_timout_increase_on_retry
+    SafePgMigrations.config.lock_timeout = 0.1.seconds
+
+    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..-1].map(&:first)
+
+  end
+
+  private
+
+  def calls_for_lock_timeout_migration
+    @migration = Class.new(ActiveRecord::Migration::Current) do
+      def up
+        connection.send(:retry_if_lock_timeout) do
+          raise ActiveRecord::LockWaitTimeout, 'PG::LockNotAvailable: ERROR:  canceling statement due to lock timeout'
+        end
+      end
+    end.new
+
+    @connection.expects(:sleep).times(4)
+
+    calls =
+      record_calls(@migration, :write) do
+        run_migration
+        flunk 'run_migration should raise'
+      rescue StandardError => e
+        assert_instance_of ActiveRecord::LockWaitTimeout, e.cause
+        assert_includes e.cause.message, 'canceling statement due to lock timeout'
+      end
 
+    calls
   end
 end