Skip to content
This repository
Browse code

ConnectionPool wait_timeout no longer used for different types of tim…

…eouts. #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 #6441 #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.
  • Loading branch information...
commit cb6f839359bd894feb0a1105b79af72b336aa41e 1 parent c1487f6
Jonathan Rochkind authored May 23, 2012
23  activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -55,19 +55,27 @@ module ConnectionAdapters
55 55
     #
56 56
     # == Options
57 57
     #
58  
-    # There are two connection-pooling-related options that you can add to
  58
+    # There are several connection-pooling-related options that you can add to
59 59
     # your database connection configuration:
60 60
     #
61 61
     # * +pool+: number indicating size of connection pool (default 5)
62  
-    # * +wait_timeout+: number of seconds to block and wait for a connection
  62
+    # * +checkout_timeout+: number of seconds to block and wait for a connection
63 63
     #   before giving up and raising a timeout error (default 5 seconds).
  64
+    # * +reaping_frequency+: frequency in seconds to periodically run the 
  65
+    #   Reaper, which attempts to find and close dead connections, which can 
  66
+    #   occur if a programmer forgets to close a connection at the end of a 
  67
+    #   thread or a thread dies unexpectedly. (Default nil, which means don't
  68
+    #   run the Reaper). 
  69
+    # * +dead_connection_timeout+: number of seconds from last checkout
  70
+    #   after which the Reaper will consider a connection reapable. (default
  71
+    #   5 seconds). 
64 72
     class ConnectionPool
65 73
       # Every +frequency+ seconds, the reaper will call +reap+ on +pool+.
66 74
       # A reaper instantiated with a nil frequency will never reap the
67 75
       # connection pool.
68 76
       #
69 77
       # Configure the frequency by setting "reaping_frequency" in your
70  
-      # database yaml file.
  78
+      # database yaml file.      
71 79
       class Reaper
72 80
         attr_reader :pool, :frequency
73 81
 
@@ -89,7 +97,7 @@ def run
89 97
 
90 98
       include MonitorMixin
91 99
 
92  
-      attr_accessor :automatic_reconnect, :timeout
  100
+      attr_accessor :automatic_reconnect, :checkout_timeout, :dead_connection_timeout
93 101
       attr_reader :spec, :connections, :size, :reaper
94 102
 
95 103
       class Latch # :nodoc:
@@ -121,7 +129,8 @@ def initialize(spec)
121 129
         # The cache of reserved connections mapped to threads
122 130
         @reserved_connections = {}
123 131
 
124  
-        @timeout = spec.config[:wait_timeout] || 5
  132
+        @checkout_timeout = spec.config[:checkout_timeout] || 5
  133
+        @dead_connection_timeout = spec.config[:dead_connection_timeout]
125 134
         @reaper  = Reaper.new self, spec.config[:reaping_frequency]
126 135
         @reaper.run
127 136
 
@@ -241,7 +250,7 @@ def checkout
241 250
             return checkout_and_verify(conn) if conn
242 251
           end
243 252
 
244  
-          Timeout.timeout(@timeout, PoolFullError) { @latch.await }
  253
+          Timeout.timeout(@checkout_timeout, PoolFullError) { @latch.await }
245 254
         end
246 255
       end
247 256
 
@@ -279,7 +288,7 @@ def remove(conn)
279 288
       # or a thread dies unexpectedly.
280 289
       def reap
281 290
         synchronize do
282  
-          stale = Time.now - @timeout
  291
+          stale = Time.now - @dead_connection_timeout
283 292
           connections.dup.each do |conn|
284 293
             remove conn if conn.in_use? && stale > conn.last_use && !conn.active?
285 294
           end
4  activerecord/test/cases/connection_pool_test.rb
@@ -124,7 +124,7 @@ def test_reap_and_active
124 124
         @pool.checkout
125 125
         @pool.checkout
126 126
         @pool.checkout
127  
-        @pool.timeout = 0
  127
+        @pool.dead_connection_timeout = 0
128 128
 
129 129
         connections = @pool.connections.dup
130 130
 
@@ -137,7 +137,7 @@ def test_reap_inactive
137 137
         @pool.checkout
138 138
         @pool.checkout
139 139
         @pool.checkout
140  
-        @pool.timeout = 0
  140
+        @pool.dead_connection_timeout = 0
141 141
 
142 142
         connections = @pool.connections.dup
143 143
         connections.each do |conn|
4  activerecord/test/cases/pooled_connections_test.rb
@@ -17,7 +17,7 @@ def teardown
17 17
   end
18 18
 
19 19
   def checkout_connections
20  
-    ActiveRecord::Model.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
  20
+    ActiveRecord::Model.establish_connection(@connection.merge({:pool => 2, :checkout_timeout => 0.3}))
21 21
     @connections = []
22 22
     @timed_out = 0
23 23
 
@@ -34,7 +34,7 @@ def checkout_connections
34 34
 
35 35
   # Will deadlock due to lack of Monitor timeouts in 1.9
36 36
   def checkout_checkin_connections(pool_size, threads)
37  
-    ActiveRecord::Model.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
  37
+    ActiveRecord::Model.establish_connection(@connection.merge({:pool => pool_size, :checkout_timeout => 0.5}))
38 38
     @connection_count = 0
39 39
     @timed_out = 0
40 40
     threads.times do
2  activerecord/test/cases/reaper_test.rb
@@ -64,7 +64,7 @@ def test_connection_pool_starts_reaper
64 64
         spec.config[:reaping_frequency] = 0.0001
65 65
 
66 66
         pool = ConnectionPool.new spec
67  
-        pool.timeout = 0
  67
+        pool.dead_connection_timeout = 0
68 68
 
69 69
         conn = pool.checkout
70 70
         count = pool.connections.length

0 notes on commit cb6f839

Please sign in to comment.
Something went wrong with that request. Please try again.