From 7d5443be3de1c4f41d64e431ef7c4cba2e251b79 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 21 Nov 2011 12:25:27 -0500 Subject: [PATCH 1/6] Handle deadlock victim (1205) exceptions outside of a transactions by retrying the query. Add SQLServerAdapter.retry_deadlock_victim option to disable this handling. --- .../connection_adapters/sqlserver/errors.rb | 3 ++ .../connection_adapters/sqlserver_adapter.rb | 14 +++++++-- test/cases/connection_test_sqlserver.rb | 29 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/errors.rb b/lib/active_record/connection_adapters/sqlserver/errors.rb index c6da77382..9de1f8e19 100644 --- a/lib/active_record/connection_adapters/sqlserver/errors.rb +++ b/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 diff --git a/lib/active_record/connection_adapters/sqlserver_adapter.rb b/lib/active_record/connection_adapters/sqlserver_adapter.rb index b4a3521ef..bef702657 100644 --- a/lib/active_record/connection_adapters/sqlserver_adapter.rb +++ b/lib/active_record/connection_adapters/sqlserver_adapter.rb @@ -179,7 +179,7 @@ class SQLServerAdapter < AbstractAdapter attr_reader :database_version, :database_year, :spid 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 @@ -347,6 +347,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 @@ -381,6 +386,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 @@ -485,7 +492,10 @@ def with_auto_reconnect 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 diff --git a/test/cases/connection_test_sqlserver.rb b/test/cases/connection_test_sqlserver.rb index 31437d395..51c2337f0 100644 --- a/test/cases/connection_test_sqlserver.rb +++ b/test/cases/connection_test_sqlserver.rb @@ -101,6 +101,26 @@ def setup assert @connection.spid.nil? end + if connection_mode_dblib? + should 'handle deadlock victim exception (1205) outside a transaction by retrying the query' 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) + + result = nil + assert_nothing_raised do + result = @connection.execute(query) + end + assert_equal expected, result + end + end + should 'be able to disconnect and reconnect at will' do @connection.disconnect! assert !@connection.active? @@ -197,6 +217,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 From bb624aabcb3ac175aa3ccfb0b7f8769705c7c6bc Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 21 Nov 2011 12:44:53 -0500 Subject: [PATCH 2/6] Copied transaction method from rails 3-1-stable connection_adapters/abstract/database_statements.rb for adding in handling of deadlock victim exception. --- .../sqlserver/database_statements.rb | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index f320eaef3..e74471916 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -45,6 +45,80 @@ def supports_statement_cache? true end + def transaction(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 + if open_transactions == 0 + rollback_db_transaction + rollback_transaction_records(true) + 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 From f06beaa20877bf29aa5bcf0af20cc89148ba1e61 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 23 Nov 2011 11:40:15 -0500 Subject: [PATCH 3/6] Handle deadlock victim errors in the outermost transaction by skipping the rollback_db_transaction (since SQL Server issues it automatically) and retry the whole transaction if the retry_deadlock_victim flag is enabled (default). --- .../sqlserver/database_statements.rb | 11 ++- test/cases/connection_test_sqlserver.rb | 92 ++++++++++++++++--- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index e74471916..a99e789cf 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -77,9 +77,16 @@ def transaction(options = {}) if transaction_open && !outside_transaction? transaction_open = false decrement_open_transactions + # handle deadlock victim retries at the outermost transaction if open_transactions == 0 - rollback_db_transaction - rollback_transaction_records(true) + if translate_exception(database_transaction_rollback, database_transaction_rollback.message).is_a?(DeadlockVictim) + # SQL Server has already rolled back, so rollback activerecord's history + rollback_transaction_records(true) + retry if retry_deadlock_victim? + else + rollback_db_transaction + rollback_transaction_records(true) + end else rollback_to_savepoint rollback_transaction_records(false) diff --git a/test/cases/connection_test_sqlserver.rb b/test/cases/connection_test_sqlserver.rb index 51c2337f0..6e4436e0e 100644 --- a/test/cases/connection_test_sqlserver.rb +++ b/test/cases/connection_test_sqlserver.rb @@ -102,22 +102,88 @@ def setup end if connection_mode_dblib? - should 'handle deadlock victim exception (1205) outside a transaction by retrying the query' do - query = "SELECT 1 as [one]" - expected = @connection.execute(query) + 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) + # 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 - result = nil - assert_nothing_raised do - result = @connection.execute(query) + 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 - assert_equal expected, result end end From 89741f62be0677626e330276d4fcae34e25f0c18 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 28 Nov 2011 16:18:42 -0500 Subject: [PATCH 4/6] If retry_deadlock_victim flag is false, call the original transaction method, otherwise, call our version of the transcation method with deadlock_victim retry logic. --- .../connection_adapters/sqlserver/database_statements.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index a99e789cf..6f75cf703 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -45,7 +45,11 @@ def supports_statement_cache? true end - def transaction(options = {}) + 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 @@ -82,7 +86,7 @@ def transaction(options = {}) if translate_exception(database_transaction_rollback, database_transaction_rollback.message).is_a?(DeadlockVictim) # SQL Server has already rolled back, so rollback activerecord's history rollback_transaction_records(true) - retry if retry_deadlock_victim? + retry else rollback_db_transaction rollback_transaction_records(true) From be4074e420cd1aa20d5aaafcec6739bea38268f1 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 28 Nov 2011 16:26:41 -0500 Subject: [PATCH 5/6] Renamed #with_auto_reconnect to #with_sqlserver_error_handling now that it handles both dropped connections and deadlock victim errors. --- CHANGELOG | 3 +++ .../connection_adapters/sqlserver/database_statements.rb | 4 ++-- lib/active_record/connection_adapters/sqlserver_adapter.rb | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 13de290c0..c2b3ff45d 100644 --- a/CHANGELOG +++ b/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. diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 6f75cf703..0481bd70d 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -392,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 @@ -450,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) diff --git a/lib/active_record/connection_adapters/sqlserver_adapter.rb b/lib/active_record/connection_adapters/sqlserver_adapter.rb index bef702657..3726c7f25 100644 --- a/lib/active_record/connection_adapters/sqlserver_adapter.rb +++ b/lib/active_record/connection_adapters/sqlserver_adapter.rb @@ -488,7 +488,7 @@ 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 From ac52389e33578d39a9cd4c5a6f814cc8554d8dd0 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 28 Nov 2011 16:45:36 -0500 Subject: [PATCH 6/6] translate_exception is already called in the log block, which accounts for most if not all queries we want to retry, so assume it has been translated. --- .../connection_adapters/sqlserver/database_statements.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 0481bd70d..ba9043916 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -83,7 +83,7 @@ def transaction_with_retry_deadlock_victim(options = {}) decrement_open_transactions # handle deadlock victim retries at the outermost transaction if open_transactions == 0 - if translate_exception(database_transaction_rollback, database_transaction_rollback.message).is_a?(DeadlockVictim) + if database_transaction_rollback.is_a?(DeadlockVictim) # SQL Server has already rolled back, so rollback activerecord's history rollback_transaction_records(true) retry