Merge pull request rails-sqlserver#150 from ManageIQ/retry_deadlock_v…

…ictim_error

Retry deadlock victim error

CHANGELOG

@@ -1,6 +1,9 @@
 
 * master *
 
+* Renamed #with_auto_reconnect to #with_sqlserver_error_handling now that it handles both dropped
+  connections and deadlock victim errors. Fixes #150 [Joe Rafaniello]
+
 * Add activity_stats method that mimics the SQL Server Activity Monitor. Fixes #146 [Joe Rafaniello]
 
 * Add methods for sqlserver's #product_version, #product_level, #edition and include them in inspect.

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -45,6 +45,91 @@ def supports_statement_cache?
           true
         end
 
+        def transaction(options = {}, &block)
+          retry_deadlock_victim? ? transaction_with_retry_deadlock_victim(options, &block) : super(options, &block)
+        end
+
+        def transaction_with_retry_deadlock_victim(options = {})
+          options.assert_valid_keys :requires_new, :joinable
+
+          last_transaction_joinable = defined?(@transaction_joinable) ? @transaction_joinable : nil
+          if options.has_key?(:joinable)
+            @transaction_joinable = options[:joinable]
+          else
+            @transaction_joinable = true
+          end
+          requires_new = options[:requires_new] || !last_transaction_joinable
+
+          transaction_open = false
+          @_current_transaction_records ||= []
+
+          begin
+            if block_given?
+              if requires_new || open_transactions == 0
+                if open_transactions == 0
+                  begin_db_transaction
+                elsif requires_new
+                  create_savepoint
+                end
+                increment_open_transactions
+                transaction_open = true
+                @_current_transaction_records.push([])
+              end
+              yield
+            end
+          rescue Exception => database_transaction_rollback
+            if transaction_open && !outside_transaction?
+              transaction_open = false
+              decrement_open_transactions
+              # handle deadlock victim retries at the outermost transaction
+              if open_transactions == 0
+                if database_transaction_rollback.is_a?(DeadlockVictim)
+                  # SQL Server has already rolled back, so rollback activerecord's history
+                  rollback_transaction_records(true)
+                  retry
+                else
+                  rollback_db_transaction
+                  rollback_transaction_records(true)
+                end
+              else
+                rollback_to_savepoint
+                rollback_transaction_records(false)
+              end
+            end
+            raise unless database_transaction_rollback.is_a?(ActiveRecord::Rollback)
+          end
+        ensure
+          @transaction_joinable = last_transaction_joinable
+
+          if outside_transaction?
+            @open_transactions = 0
+          elsif transaction_open
+            decrement_open_transactions
+            begin
+              if open_transactions == 0
+                commit_db_transaction
+                commit_transaction_records
+              else
+                release_savepoint
+                save_point_records = @_current_transaction_records.pop
+                unless save_point_records.blank?
+                  @_current_transaction_records.push([]) if @_current_transaction_records.empty?
+                  @_current_transaction_records.last.concat(save_point_records)
+                end
+              end
+            rescue Exception => database_transaction_rollback
+              if open_transactions == 0
+                rollback_db_transaction
+                rollback_transaction_records(true)
+              else
+                rollback_to_savepoint
+                rollback_transaction_records(false)
+              end
+              raise
+            end
+          end
+        end
+
         def begin_db_transaction
           do_execute "BEGIN TRANSACTION"
         end
@@ -307,7 +392,7 @@ def valid_isolation_levels
         def do_execute(sql, name = nil)
           name ||= 'EXECUTE'
           log(sql, name) do
-            with_auto_reconnect { raw_connection_do(sql) }
+            with_sqlserver_error_handling { raw_connection_do(sql) }
           end
         end
 
@@ -365,7 +450,7 @@ def _raw_select(sql, options={})
         end
 
         def raw_connection_run(sql)
-          with_auto_reconnect do
+          with_sqlserver_error_handling do
             case @connection_options[:mode]
             when :dblib
               @connection.execute(sql)

lib/active_record/connection_adapters/sqlserver/errors.rb

@@ -3,6 +3,9 @@ module ActiveRecord
   class LostConnection < WrappedDatabaseException
   end
 
+  class DeadlockVictim < WrappedDatabaseException
+  end
+
   module ConnectionAdapters
     module Sqlserver
       module Errors

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -179,7 +179,7 @@ class SQLServerAdapter < AbstractAdapter
       attr_reader :database_version, :database_year, :spid, :product_level, :product_version, :edition
 
       cattr_accessor :native_text_database_type, :native_binary_database_type, :native_string_database_type,
-                     :log_info_schema_queries, :enable_default_unicode_types, :auto_connect,
+                     :log_info_schema_queries, :enable_default_unicode_types, :auto_connect, :retry_deadlock_victim,
                      :cs_equality_operator, :lowercase_schema_reflection, :auto_connect_duration
 
       self.enable_default_unicode_types = true
@@ -338,6 +338,11 @@ def auto_connect_duration
         @@auto_connect_duration ||= 10
       end
 
+      def retry_deadlock_victim
+        @@retry_deadlock_victim.is_a?(FalseClass) ? false : true
+      end
+      alias :retry_deadlock_victim? :retry_deadlock_victim
+
       def native_string_database_type
         @@native_string_database_type || (enable_default_unicode_types ? 'nvarchar' : 'varchar') 
       end
@@ -372,6 +377,8 @@ def translate_exception(e, message)
           RecordNotUnique.new(message,e)
         when /conflicted with the foreign key constraint/i
           InvalidForeignKey.new(message,e)
+        when /has been chosen as the deadlock victim/i
+          DeadlockVictim.new(message,e)
         when *lost_connection_messages
           LostConnection.new(message,e)
         else
@@ -472,11 +479,14 @@ def remove_database_connections_and_rollback(database=nil)
         end if block_given?
       end
 
-      def with_auto_reconnect
+      def with_sqlserver_error_handling
         begin
           yield
         rescue Exception => e
-          retry if translate_exception(e,e.message).is_a?(LostConnection) && auto_reconnected?
+          case translate_exception(e,e.message)
+            when LostConnection; retry if auto_reconnected?
+            when DeadlockVictim; retry if retry_deadlock_victim? && self.open_transactions == 0
+          end
           raise
         end
       end

test/cases/connection_test_sqlserver.rb

@@ -101,6 +101,92 @@ def setup
       assert @connection.spid.nil?
     end
 
+    if connection_mode_dblib?
+      context 'with a deadlock victim exception (1205) outside a transaction' do
+        setup do
+          @query = "SELECT 1 as [one]"
+          @expected = @connection.execute(@query)
+
+          # Execute the query to get a handle of the expected result, which will
+          # be returned after a simulated deadlock victim (1205).
+          raw_conn = @connection.instance_variable_get(:@connection)
+          stubbed_handle = raw_conn.execute(@query)
+          @connection.send(:finish_statement_handle, stubbed_handle)
+          raw_conn.stubs(:execute).raises(deadlock_victim_exception(@query)).then.returns(stubbed_handle)
+        end
+
+        teardown do
+          @connection.class.retry_deadlock_victim = nil
+        end
+
+        should 'retry by default' do
+          assert_nothing_raised do
+            assert_equal @expected, @connection.execute(@query)
+          end
+        end
+
+        should 'raise ActiveRecord::DeadlockVictim if retry is disabled' do
+          @connection.class.retry_deadlock_victim = false
+          assert_raise(ActiveRecord::DeadlockVictim) do
+            assert_equal @expected, @connection.execute(@query)
+          end
+        end
+      end
+
+      context 'with a deadlock victim exception (1205) within a transaction' do
+        setup do
+          @query = "SELECT 1 as [one]"
+          @expected = @connection.execute(@query)
+
+          # "stub" the execute method to simulate raising a deadlock victim exception once
+          @connection.class.class_eval do
+            def execute_with_deadlock_exception(sql, *args)
+              if !@raised_deadlock_exception && sql == "SELECT 1 as [one]"
+                sql = "RAISERROR('Transaction (Process ID #{Process.pid}) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.: #{sql}', 13, 1)"
+                @raised_deadlock_exception = true
+              elsif @raised_deadlock_exception == true && sql =~ /RAISERROR\('Transaction \(Process ID \d+\) was deadlocked on lock resources with another process and has been chosen as the deadlock victim\. Rerun the transaction\.: SELECT 1 as \[one\]', 13, 1\)/
+                sql = "SELECT 1 as [one]"
+              end
+
+              execute_without_deadlock_exception(sql, *args)
+            end
+
+            alias :execute_without_deadlock_exception :execute
+            alias :execute :execute_with_deadlock_exception
+          end
+        end
+
+        teardown do
+          # Cleanup the "stubbed" execute method
+          @connection.class.class_eval do
+            alias :execute :execute_without_deadlock_exception
+            remove_method :execute_with_deadlock_exception
+            remove_method :execute_without_deadlock_exception
+          end
+
+          @connection.send(:remove_instance_variable, :@raised_deadlock_exception)
+          @connection.class.retry_deadlock_victim = nil
+        end
+
+        should 'retry by default' do
+          assert_nothing_raised do
+            ActiveRecord::Base.transaction do
+              assert_equal @expected, @connection.execute(@query)
+            end
+          end
+        end
+
+        should 'raise ActiveRecord::DeadlockVictim if retry disabled' do
+          @connection.class.retry_deadlock_victim = false
+          assert_raise(ActiveRecord::DeadlockVictim) do
+            ActiveRecord::Base.transaction do
+              assert_equal @expected, @connection.execute(@query)
+            end
+          end
+        end
+      end
+    end
+
     should 'be able to disconnect and reconnect at will' do
       @connection.disconnect!
       assert !@connection.active?
@@ -197,6 +283,15 @@ def assert_all_odbc_statements_used_are_closed(&block)
     GC.enable
   end
 
+  def deadlock_victim_exception(sql)
+    require 'tiny_tds/error'
+    error = TinyTds::Error.new("Transaction (Process ID #{Process.pid}) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.: #{sql}")
+    error.severity = 13
+    error.db_error_number = 1205
+    error
+  end
+
+
   def with_auto_connect(boolean)
     existing = ActiveRecord::ConnectionAdapters::SQLServerAdapter.auto_connect
     ActiveRecord::ConnectionAdapters::SQLServerAdapter.auto_connect = boolean