Retry deadlock victim error

[jrafanie]

No description provided.

[metaskills]

Real clean code! I Love the new WrappedDatabaseException too!

Now that I have looked at the monkey patch for #transaction, I am getting cold feet. Perhaps we could do the following to make this less brittle for the core adapter without this configuration. Here are a few suggestion, would love your thoughts on these:

1. Maybe we can super up if the option is not on and call our own copy if needed?

module ActiveRecord
  module ConnectionAdapters
    module Sqlserver
      module DatabaseStatements

        def transaction(options = {})
          retry_deadlock_victim? ? transaction_with_retry_deadlock_victim(options) : super
        end

        def transaction_with_retry_deadlock_victim(options = {})
          # Our copy of transaction here.
        end

      end
    end
  end
end

2. I think we should rename with_auto_reconnect to something like with_sqlserver_error_handling.

3. I wonder if we are doing too much work by using translate_exception ourselves. Since the block (renamed above) is always in the log block, it should be translated for us. Can we see if that is the case and use code like this? database_transaction_rollback.is_a(DeadlockVictim)?

Notes to self:

• Should the with_auto_reconnect block in the do_execute really be moved to the raw_connection_do so it syncs up with the raw_connection_run's usage? Will try that out and test.

[jrafanie]

@metaskills, I removed the extra translate_exception call in transaction_with_retry_deadlock_victim since log would raise to us the translated exception for nearly all of the queries we would ever want to retry as part of a transaction. I did not remove the translate_exception call in #with_sqlserver_error_handling since it's called within the log block and the exception would not yet be translated.

[metaskills]

Please QA my work. All tests still green.
b07f638

[jrafanie]

Looks good from a quick review, I'll do more testing in the next few days when I incorporate these changes and will let you know if I have any issues. Was there something that specifically needing AR ~> 3.1.3 instead of ~> 3.1.0? Just wondering since transaction does not appear to be different between those tags.

[metaskills]

Whoops, that gemspec change was a mistake on my part.