Merge pull request #7684 from jrochkind/connection_pool_timeout_key_backport

ConnectionPool accepts spec key 'checkout_timeout' (Backport)

4 files changed, 39 insertions, 5 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 589c3d71b0..7b60b31c7d 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,11 @@
 ## Rails 3.2.9 (unreleased)
 
+*   ConnectionPool recognizes checkout_timeout spec key as taking
+    precedence over legacy wait_timeout spec key, can be used to avoid
+    conflict with mysql2 use of wait_timeout.  Closes #7684.
+
+    *jrochkind*
+
 *   Rename field_changed? to _field_changed? so that users can create a field named field
 
     *Akira Matsuda*, backported by *Steve Klabnik*
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index d4649102df..ef176a1959 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -54,8 +54,11 @@ module ActiveRecord
     # your database connection configuration:
     #
     # * +pool+: number indicating size of connection pool (default 5)
-    # * +wait_timeout+: number of seconds to block and wait for a connection
-    #   before giving up and raising a timeout error (default 5 seconds).
+    # * +checkout _timeout+: number of seconds to block and wait for a 
+    #   connection before giving up and raising a timeout error 
+    #   (default 5 seconds). ('wait_timeout' supported for backwards
+    #   compatibility, but conflicts with key used for different purpose
+    #   by mysql2 adapter). 
     class ConnectionPool
       include MonitorMixin
 
@@ -77,7 +80,10 @@ module ActiveRecord
         @reserved_connections = {}
 
         @queue = new_cond
-        @timeout = spec.config[:wait_timeout] || 5
+        # 'wait_timeout', the backward-compatible key, conflicts with spec key 
+        # used by mysql2 for something entirely different, checkout_timeout
+        # preferred to avoid conflict and allow independent values. 
+        @timeout = spec.config[:checkout_timeout] || spec.config[:wait_timeout] || 5
 
         # default max pool size to 5
         @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index 5cecfa90e7..98759544b6 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -151,6 +151,28 @@ module ActiveRecord
       def test_pool_sets_connection_visitor
         assert @pool.connection.visitor.is_a?(Arel::Visitors::ToSql)
       end
+      
+      def test_timeout_spec_keys
+        # 'wait_timeout' is supported for backwards compat, 
+        # 'checkout_timeout' is preferred to avoid conflicting
+        #  with mysql2 adapters key of name 'wait_timeout' but
+        #  different meaning.
+        config = ActiveRecord::Base.connection_pool.spec.config.merge(:wait_timeout => nil, :connection_timeout => nil)
+        method = ActiveRecord::Base.connection_pool.spec.adapter_method        
+        
+        pool = ConnectionPool.new ActiveRecord::Base::ConnectionSpecification.new(config.merge(:wait_timeout => 1), method)
+        assert_equal 1, pool.instance_variable_get(:@timeout)
+        pool.disconnect!
+        
+        pool = ConnectionPool.new ActiveRecord::Base::ConnectionSpecification.new(config.merge(:checkout_timeout => 1), method)
+        assert_equal 1, pool.instance_variable_get(:@timeout)
+        pool.disconnect!
+        
+        pool = ConnectionPool.new ActiveRecord::Base::ConnectionSpecification.new(config.merge(:wait_timeout => 6000, :checkout_timeout => 1), method)
+        assert_equal 1, pool.instance_variable_get(:@timeout)
+        pool.disconnect!
+      end
+      
     end
   end
 end
diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb
index bc3dfb1078..e69243a537 100644
--- a/activerecord/test/cases/pooled_connections_test.rb
+++ b/activerecord/test/cases/pooled_connections_test.rb
@@ -17,7 +17,7 @@ class PooledConnectionsTest < ActiveRecord::TestCase
   end
 
   def checkout_connections
-    ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :checkout_timeout => 0.3}))
     @connections = []
     @timed_out = 0
 
@@ -42,7 +42,7 @@ class PooledConnectionsTest < ActiveRecord::TestCase
   end
 
   def checkout_checkin_connections(pool_size, threads)
-    ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :checkout_timeout => 0.5}))
     @connection_count = 0
     @timed_out = 0
     threads.times do