Merge pull request qertoip#2 from Pixelapse/master

Add fuzz factor on delay

lib/transaction_retry.rb

@@ -38,4 +38,12 @@ def self.wait_times=( array_of_seconds )
     @@wait_times = array_of_seconds
   end
 
+  def self.fuzz
+    @@fuzz ||= true
+  end
+
+  def self.fuzz=( val )
+    @@fuzz = val
+  end
+
 end

lib/transaction_retry/active_record/base.rb

@@ -19,15 +19,32 @@ module ClassMethods
         def transaction_with_retry(*objects, &block)
           retry_count = 0
 
+          opts = if objects.last.is_a? Hash
+            objects.last
+          else
+            {}
+          end
+
+          retry_on = opts.delete(:retry_on)
+          max_retries = opts.delete(:max_retries) || TransactionRetry.max_retries
+
           begin
             transaction_without_retry(*objects, &block)
-          rescue ::ActiveRecord::TransactionIsolationConflict
-            raise if retry_count >= TransactionRetry.max_retries
+          rescue *[::ActiveRecord::TransactionIsolationConflict, *retry_on]
+            raise if retry_count >= max_retries
             raise if tr_in_nested_transaction?
 
             retry_count += 1
             postfix = { 1 => 'st', 2 => 'nd', 3 => 'rd' }[retry_count] || 'th'
-            logger.warn "Transaction isolation conflict detected. Retrying for the #{retry_count}-#{postfix} time..." if logger
+
+            type_s = case $!
+            when ::ActiveRecord::TransactionIsolationConflict
+              "Transaction isolation conflict"
+            else
+              $!.class.name
+            end
+
+            logger.warn "#{type_s} detected. Retrying for the #{retry_count}-#{postfix} time..." if logger
             tr_exponential_pause( retry_count )
             retry
           end
@@ -40,6 +57,13 @@ def transaction_with_retry(*objects, &block)
           # An ugly tr_ prefix is used to minimize the risk of method clash in the future.
           def tr_exponential_pause( count )
             seconds = TransactionRetry.wait_times[count-1] || 32
+
+            if TransactionRetry.fuzz
+              fuzz_factor = [seconds * 0.25, 1].max
+
+              seconds += rand * (fuzz_factor * 2) - fuzz_factor
+            end
+
             sleep( seconds ) if seconds > 0
           end
 

test/integration/active_record/base/transaction_with_retry_test.rb

@@ -3,6 +3,8 @@
 require 'test_helper'
 
 class TransactionWithRetryTest < MiniTest::Unit::TestCase
+  class CustomError < StandardError
+  end
 
   def setup
     @original_max_retries = TransactionRetry.max_retries
@@ -48,6 +50,38 @@ def test_retries_transaction_on_transaction_isolation_conflict
     QueuedJob.first.destroy
   end
 
+  def test_does_not_retry_on_unknown_error
+    first_run = true
+
+    assert_raises( CustomError ) do
+      ActiveRecord::Base.transaction do
+        if first_run
+          first_run = false
+          message = "Deadlock found when trying to get lock"
+          raise CustomError, "random error"
+        end
+        QueuedJob.create!( :job => 'is cool!' )
+      end
+    end
+    assert_equal( 0, QueuedJob.count )
+  end
+
+  def test_retries_on_custom_error
+    first_run = true
+
+    ActiveRecord::Base.transaction(retry_on: CustomError) do
+      if first_run
+        first_run = false
+        message = "Deadlock found when trying to get lock"
+        raise CustomError, "random error"
+      end
+      QueuedJob.create!( :job => 'is cool!' )
+    end
+    assert_equal( 1, QueuedJob.count )
+
+    QueuedJob.first.destroy
+  end
+
   def test_does_not_retry_transaction_more_than_max_retries_times
     TransactionRetry.max_retries = 1
     run = 0
@@ -61,6 +95,20 @@ def test_does_not_retry_transaction_more_than_max_retries_times
     end
 
     assert_equal( 2, run )  # normal run + one retry
+
+    TransactionRetry.max_retries = 3
+
+    run = 0
+
+    assert_raises( ActiveRecord::TransactionIsolationConflict ) do
+      ActiveRecord::Base.transaction(max_retries: 1) do
+        run += 1
+        message = "Deadlock found when trying to get lock"
+        raise ActiveRecord::TransactionIsolationConflict.new( ActiveRecord::StatementInvalid.new( message ), message )
+      end
+    end
+
+    assert_equal( 2, run )  # normal run + one retry
   end
 
   def test_does_not_retry_nested_transaction