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 02b23355 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!

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
 

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