Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

ConnectionPool, unify exceptions, ConnectionTimeoutError

As a result of different commits, ConnectionPool had become
of two minds about exceptions, sometimes using PoolFullError
and sometimes using ConnectionTimeoutError. In fact, it was
using ConnectionTimeoutError internally, but then recueing
and re-raising as a PoolFullError.

There's no reason for this bifurcation, standardize on
ConnectionTimeoutError, which is the rails2 name and still
accurately describes semantics at this point.

History

In Rails2, ConnectionPool raises a ConnectionTimeoutError if
it can't get a connection within timeout.

Originally in master/rails3, @tenderlove had planned on removing
wait/blocking in connectionpool entirely, at that point he changed
exception to PoolFullError.

But then later wait/blocking came back, but exception remained
PoolFullError.

Then in 02b2335 pmahoney introduced fair waiting logic, and
brought back ConnectionTimeoutError, introducing the weird bifurcation.

ConnectionTimeoutError accurately describes semantics as of this
point, and is backwards compat with rails2, there's no reason
for PoolFullError to be introduced, and no reason for two
different exception types to be used internally, no reason
to rescue one and re-raise as another.  Unify!
  • Loading branch information...
commit 5b7cfc5eeae3c5aa8e2ab5e3a0b9bd63b8465168 1 parent 616ba15
@jrochkind authored
View
28 activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -5,14 +5,11 @@
module ActiveRecord
# Raised when a connection could not be obtained within the connection
- # acquisition timeout period.
+ # acquisition timeout period: because max connections in pool
+ # are in use.
class ConnectionTimeoutError < ConnectionNotEstablished
end
- # Raised when a connection pool is full and another connection is requested
- class PoolFullError < ConnectionNotEstablished
- end
-
module ConnectionAdapters
# Connection pool base class for managing Active Record database
# connections.
@@ -187,7 +184,11 @@ def wait_poll(timeout)
return remove if any?
elapsed = Time.now - t0
- raise ConnectionTimeoutError if elapsed >= timeout
+ if elapsed >= timeout
+ msg = 'could not obtain a database connection within %0.3f seconds (waited %0.3f seconds)' %
+ [timeout, elapsed]
+ raise ConnectionTimeoutError, msg
+ end
end
ensure
@num_waiting -= 1
@@ -350,12 +351,12 @@ def clear_stale_cached_connections! # :nodoc:
#
# If all connections are leased and the pool is at capacity (meaning the
# number of currently leased connections is greater than or equal to the
- # size limit set), an ActiveRecord::PoolFullError exception will be raised.
+ # size limit set), an ActiveRecord::ConnectionTimeoutError exception will be raised.
#
# Returns: an AbstractAdapter object.
#
# Raises:
- # - PoolFullError: no connection can be obtained from the pool.
+ # - ConnectionTimeoutError: no connection can be obtained from the pool.
def checkout
synchronize do
conn = acquire_connection
@@ -416,8 +417,7 @@ def reap
# queue for a connection to become available.
#
# Raises:
- # - PoolFullError if a connection could not be acquired (FIXME:
- # why not ConnectionTimeoutError?
+ # - ConnectionTimeoutError if a connection could not be acquired
def acquire_connection
if conn = @available.poll
conn
@@ -425,13 +425,7 @@ def acquire_connection
checkout_new_connection
else
t0 = Time.now
- begin
- @available.poll(@checkout_timeout)
- rescue ConnectionTimeoutError
- msg = 'could not obtain a database connection within %0.3f seconds (waited %0.3f seconds)' %
- [@checkout_timeout, Time.now - t0]
- raise PoolFullError, msg
- end
+ @available.poll(@checkout_timeout)
end
end
View
2  activerecord/test/cases/connection_pool_test.rb
@@ -89,7 +89,7 @@ def test_active_connection_in_use
end
def test_full_pool_exception
- assert_raises(PoolFullError) do
+ assert_raises(ConnectionTimeoutError) do
(@pool.size + 1).times do
@pool.checkout
end
Please sign in to comment.
Something went wrong with that request. Please try again.