ConnectionPool wait_timeout no longer used for different types of tim…

…eouts. rails#6441

An AR ConnectionSpec `wait_timeout` is pre-patch used for three
different things:

* mysql2 uses it for MySQL's own wait_timeout (how long MySQL
  should allow an idle connection before closing it), and
  defaults to 2592000 seconds.
* ConnectionPool uses it for "number of seconds to block and
  wait for a connection before giving up and raising a timeout error",
  default 5 seconds.
* ConnectionPool uses it for the Reaper, for deciding if a 'dead'
  connection can be reaped. Default 5 seconds.

Previously, if you want to change these from defaults, you need
to change them all together. This is problematic _especially_
for the mysql2/ConnectionPool conflict, you will generally _not_
want them to be the same, as evidenced by their wildly different
defaults. This has caused real problems for people rails#6441 rails#2894

But as long as we're changing this, forcing renaming the
ConnectionPool key to be more specific, it made sense
to seperate the two ConnectionPool uses too -- these two
types of ConnectionPool timeouts ought to be able to be
changed independently, you won't neccesarily want them
to be the same, even though the defaults are (currently)
the same.

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -55,19 +55,27 @@ module ConnectionAdapters
     #
     # == Options
     #
-    # There are two connection-pooling-related options that you can add to
+    # There are several connection-pooling-related options that you can add to
     # 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
+    # * +checkout_timeout+: number of seconds to block and wait for a connection
     #   before giving up and raising a timeout error (default 5 seconds).
+    # * +reaping_frequency+: frequency in seconds to periodically run the 
+    #   Reaper, which attempts to find and close dead connections, which can 
+    #   occur if a programmer forgets to close a connection at the end of a 
+    #   thread or a thread dies unexpectedly. (Default nil, which means don't
+    #   run the Reaper). 
+    # * +dead_connection_timeout+: number of seconds from last checkout
+    #   after which the Reaper will consider a connection reapable. (default
+    #   5 seconds). 
     class ConnectionPool
       # Every +frequency+ seconds, the reaper will call +reap+ on +pool+.
       # A reaper instantiated with a nil frequency will never reap the
       # connection pool.
       #
       # Configure the frequency by setting "reaping_frequency" in your
-      # database yaml file.
+      # database yaml file.      
       class Reaper
         attr_reader :pool, :frequency
 
@@ -89,7 +97,7 @@ def run
 
       include MonitorMixin
 
-      attr_accessor :automatic_reconnect, :timeout
+      attr_accessor :automatic_reconnect, :checkout_timeout, :dead_connection_timeout
       attr_reader :spec, :connections, :size, :reaper
 
       class Latch # :nodoc:
@@ -121,7 +129,8 @@ def initialize(spec)
         # The cache of reserved connections mapped to threads
         @reserved_connections = {}
 
-        @timeout = spec.config[:wait_timeout] || 5
+        @checkout_timeout = spec.config[:checkout_timeout] || 5
+        @dead_connection_timeout = spec.config[:dead_connection_timeout]
         @reaper  = Reaper.new self, spec.config[:reaping_frequency]
         @reaper.run
 
@@ -241,7 +250,7 @@ def checkout
             return checkout_and_verify(conn) if conn
           end
 
-          Timeout.timeout(@timeout, PoolFullError) { @latch.await }
+          Timeout.timeout(@checkout_timeout, PoolFullError) { @latch.await }
         end
       end
 
@@ -279,7 +288,7 @@ def remove(conn)
       # or a thread dies unexpectedly.
       def reap
         synchronize do
-          stale = Time.now - @timeout
+          stale = Time.now - @dead_connection_timeout
           connections.dup.each do |conn|
             remove conn if conn.in_use? && stale > conn.last_use && !conn.active?
           end

activerecord/test/cases/connection_pool_test.rb

@@ -124,7 +124,7 @@ def test_reap_and_active
         @pool.checkout
         @pool.checkout
         @pool.checkout
-        @pool.timeout = 0
+        @pool.dead_connection_timeout = 0
 
         connections = @pool.connections.dup
 
@@ -137,7 +137,7 @@ def test_reap_inactive
         @pool.checkout
         @pool.checkout
         @pool.checkout
-        @pool.timeout = 0
+        @pool.dead_connection_timeout = 0
 
         connections = @pool.connections.dup
         connections.each do |conn|

activerecord/test/cases/pooled_connections_test.rb

@@ -17,7 +17,7 @@ def teardown
   end
 
   def checkout_connections
-    ActiveRecord::Model.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
+    ActiveRecord::Model.establish_connection(@connection.merge({:pool => 2, :checkout_timeout => 0.3}))
     @connections = []
     @timed_out = 0
 
@@ -34,7 +34,7 @@ def checkout_connections
 
   # Will deadlock due to lack of Monitor timeouts in 1.9
   def checkout_checkin_connections(pool_size, threads)
-    ActiveRecord::Model.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
+    ActiveRecord::Model.establish_connection(@connection.merge({:pool => pool_size, :checkout_timeout => 0.5}))
     @connection_count = 0
     @timed_out = 0
     threads.times do

activerecord/test/cases/reaper_test.rb

@@ -64,7 +64,7 @@ def test_connection_pool_starts_reaper
         spec.config[:reaping_frequency] = 0.0001
 
         pool = ConnectionPool.new spec
-        pool.timeout = 0
+        pool.dead_connection_timeout = 0
 
         conn = pool.checkout
         count = pool.connections.length