Remove all auto recoonect & SQL retry logic.

This was getting too complicated and I believe it stood in the way of passing a few tests along with the strong likelyhood of causing other errors. For example, I found out that using the super strong method of going into single DB user mode caused potential DB lock outs and other grief when used with our connection pool #reset! hook. I would not be surprised to find out that we played a hand in causing users grief in odd to find places when we tried doing to much for them. Lets just allow ActiveRecord and the connection pool to do what it needs and if we are no longer #active? then so be it.

🏃🔥 So passes Autoreconnectathor, son of Notourproblemalion.

Author: metaskills

CHANGELOG.md

@@ -41,6 +41,7 @@
 * Removed tests for old issue #164. Handled by core types now.
 * The `activity_stats` method. Please put this in a gem if needed.
 * We no longger use regular expressions to fix identity inserts. Use ActiveRecord or public ID insert helper.
+* All auto reconnect and SQL retry logic. Got too complicated and stood in the way of AR's pool. Speed boost too.
 
 #### Fixed
 

README.md

@@ -77,15 +77,6 @@ ActiveRecord::Base.table_name_prefix = 'dbo.'
 ```
 
 
-#### Auto Connecting
-
-By default the adapter will auto connect to lost DB connections. For every query it will retry at intervals of 2, 4, 8, 16 and 32 seconds. During each retry it will callback out to ActiveRecord::Base.did_retry_sqlserver_connection(connection,count). When all retries fail, it will callback to ActiveRecord::Base.did_lose_sqlserver_connection(connection). Both implementations of these methods are to write to the rails logger, however, they make great override points for notifications like Hoptoad. If you want to disable automatic reconnections use the following in an initializer.
-
-```ruby
-ActiveRecord::ConnectionAdapters::SQLServerAdapter.auto_connect = false
-```
-
-
 #### Configure Connection & App Name
 
 We currently conform to an unpublished and non-standard AbstractAdapter interface to configure connections made to the database. To do so, just override the `configure_connection` method in an initializer like so. In this case below we are setting the `TEXTSIZE` to 64 megabytes. Also, TinyTDS supports an application name when it logs into SQL Server. This can be used to identify the connection in SQL Server's activity monitor. By default it will use the `appname` from your database.yml file or a lowercased version of your Rails::Application name. It is now possible to define a `configure_application_name` method that can give you per instance details. Below shows how you might use this to get the process id and thread id of the current connection.

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -53,21 +53,21 @@ def begin_db_transaction
         end
 
         def commit_db_transaction
-          disable_auto_reconnect { do_execute 'COMMIT TRANSACTION' }
+          do_execute 'COMMIT TRANSACTION'
         end
 
         def rollback_db_transaction
-          do_execute 'IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION'
+          do_execute 'ROLLBACK TRANSACTION'
         end
 
         include Savepoints
 
         def create_savepoint(name = current_savepoint_name)
-          disable_auto_reconnect { do_execute "SAVE TRANSACTION #{name}" }
+          do_execute "SAVE TRANSACTION #{name}"
         end
 
         def rollback_to_savepoint(name = current_savepoint_name)
-          disable_auto_reconnect { do_execute "ROLLBACK TRANSACTION #{name}" }
+          do_execute "ROLLBACK TRANSACTION #{name}"
         end
 
         def release_savepoint(name = current_savepoint_name)
@@ -307,9 +307,7 @@ def valid_isolation_levels
         # === SQLServer Specific (Executing) ============================ #
 
         def do_execute(sql, name = 'SQL')
-          log(sql, name) do
-            with_sqlserver_error_handling { raw_connection_do(sql) }
-          end
+          log(sql, name) { raw_connection_do(sql) }
         end
 
         def do_exec_query(sql, name, binds, options = {})
@@ -375,13 +373,11 @@ def _raw_select(sql, options = {})
         end
 
         def raw_connection_run(sql)
-          with_sqlserver_error_handling do
-            case @connection_options[:mode]
-            when :dblib
-              @connection.execute(sql)
-            when :odbc
-              block_given? ? @connection.run_block(sql) { |handle| yield(handle) } : @connection.run(sql)
-            end
+          case @connection_options[:mode]
+          when :dblib
+            @connection.execute(sql)
+          when :odbc
+            block_given? ? @connection.run_block(sql) { |handle| yield(handle) } : @connection.run(sql)
           end
         end
 

lib/active_record/connection_adapters/sqlserver/errors.rb

@@ -1,36 +1,7 @@
 module ActiveRecord
 
-  class LostConnection < WrappedDatabaseException
-  end
-
   class DeadlockVictim < WrappedDatabaseException
   end
 
-  module ConnectionAdapters
-    module SQLServer
-      module Errors
-
-        LOST_CONNECTION_EXCEPTIONS  = {
-          dblib: ['TinyTds::Error'],
-          odbc: ['ODBC::Error']
-        }.freeze
-
-        LOST_CONNECTION_MESSAGES    = {
-          dblib: [/closed connection/, /dead or not enabled/, /server failed/i],
-          odbc: [/link failure/, /server failed/, /connection was already closed/, /invalid handle/i]
-        }.freeze
-
-        def lost_connection_exceptions
-          exceptions = LOST_CONNECTION_EXCEPTIONS[@connection_options[:mode]]
-          @lost_connection_exceptions ||= exceptions ? exceptions.map { |e| e.constantize rescue nil }.compact : []
-        end
-
-        def lost_connection_messages
-          LOST_CONNECTION_MESSAGES[@connection_options[:mode]]
-        end
-
-      end
-    end
-  end
 
 end

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -30,14 +30,12 @@ class SQLServerAdapter < AbstractAdapter
               SQLServer::DatabaseStatements,
               SQLServer::Showplan,
               SQLServer::SchemaStatements,
-              SQLServer::DatabaseLimits,
-              SQLServer::Errors
+              SQLServer::DatabaseLimits
 
       ADAPTER_NAME = 'SQLServer'.freeze
 
       attr_reader :spid
 
-      cattr_accessor :auto_connect, :auto_connect_duration, instance_accessor: false
       cattr_accessor :cs_equality_operator, instance_accessor: false
       cattr_accessor :lowercase_schema_reflection, :showplan_option
 
@@ -115,13 +113,9 @@ def disable_referential_integrity
 
       def active?
         return false unless @connection
-        case @connection_options[:mode]
-        when :dblib
-          return @connection.active?
-        end
-        raw_connection_do('SELECT 1')
+        raw_connection_do 'SELECT 1'
         true
-      rescue *lost_connection_exceptions
+      rescue TinyTds::Error, ODBC::Error
         false
       end
 
@@ -144,7 +138,8 @@ def disconnect!
       end
 
       def reset!
-        remove_database_connections_and_rollback {}
+        reset_transaction
+        do_execute 'IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION'
       end
 
       # === Abstract Adapter (Misc Support) =========================== #
@@ -176,14 +171,6 @@ def inspect
         "#<#{self.class} version: #{version}, mode: #{@connection_options[:mode]}, azure: #{sqlserver_azure?.inspect}>"
       end
 
-      def auto_connect
-        self.class.auto_connect.is_a?(FalseClass) ? false : true
-      end
-
-      def auto_connect_duration
-        self.class.auto_connect_duration ||= 10
-      end
-
 
       protected
 
@@ -249,8 +236,6 @@ def translate_exception(e, message)
           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
           super
         end
@@ -268,8 +253,6 @@ def connect
                       end
         @spid = _raw_select('SELECT @@SPID', fetch: :rows).first.first
         configure_connection
-      rescue
-        raise unless @auto_connecting
       end
 
       def dblib_connect(config)
@@ -363,40 +346,6 @@ def remove_database_connections_and_rollback(database = nil)
         end if block_given?
       end
 
-      def with_sqlserver_error_handling
-        yield
-      rescue Exception => e
-        case translate_exception(e, e.message)
-        when LostConnection then retry if auto_reconnected?
-        end
-        raise
-      end
-
-      def disable_auto_reconnect
-        old_auto_connect, self.class.auto_connect = self.class.auto_connect, false
-        yield
-      ensure
-        self.class.auto_connect = old_auto_connect
-      end
-
-      def auto_reconnected?
-        return false unless auto_connect
-        @auto_connecting = true
-        count = 0
-        while count <= (auto_connect_duration / 2)
-          disconnect!
-          reconnect!
-          ActiveRecord::Base.did_retry_sqlserver_connection(self, count)
-          return true if active?
-          sleep 2**count
-          count += 1
-        end
-        ActiveRecord::Base.did_lose_sqlserver_connection(self)
-        false
-      ensure
-        @auto_connecting = false
-      end
-
     end
   end
 end

test/cases/connection_test_sqlserver.rb

@@ -109,53 +109,6 @@ class ConnectionTestSQLServer < ActiveRecord::TestCase
       assert connection.active?
     end
 
-    it 'auto reconnect when setting is on' do
-      with_auto_connect(true) do
-        disconnect_raw_connection!
-        assert_nothing_raised() { Topic.count }
-        assert connection.active?
-      end
-    end
-
-    it 'not auto reconnect when setting is off' do
-      with_auto_connect(false) do
-        disconnect_raw_connection!
-        assert_raise(ActiveRecord::LostConnection) { Topic.count }
-      end
-    end
-
-    it 'not auto reconnect on commit transaction' do
-      disconnect_raw_connection!
-      assert_raise(ActiveRecord::LostConnection) { connection.commit_db_transaction }
-    end
-
-    it 'gracefully ignore lost connections on rollback transaction' do
-      disconnect_raw_connection!
-      assert_nothing_raised { connection.rollback_db_transaction }
-    end
-
-    describe 'testing #disable_auto_reconnect' do
-
-      it 'when auto reconnect setting is on' do
-        with_auto_connect(true) do
-          connection.send(:disable_auto_reconnect) do
-            assert !connection.class.auto_connect
-          end
-          assert connection.class.auto_connect
-        end
-      end
-
-      it 'when auto reconnect setting is off' do
-        with_auto_connect(false) do
-          connection.send(:disable_auto_reconnect) do
-            assert !connection.class.auto_connect
-          end
-          assert !connection.class.auto_connect
-        end
-      end
-
-    end
-
     describe 'with a deadlock victim exception 1205' do
 
       describe 'outside a transaction' do

test/cases/helper_sqlserver.rb

@@ -28,14 +28,6 @@ def connection_mode_dblib? ; self.class.connection_mode_dblib? ; end
     def connection_mode_odbc? ; self.class.connection_mode_odbc? ; end
     def sqlserver_azure? ; self.class.sqlserver_azure? ; end
 
-    def with_auto_connect(boolean)
-      existing = connection.auto_connect
-      connection.class.auto_connect = boolean
-      yield
-    ensure
-      connection.class.auto_connect = existing
-    end
-
   end
 end