Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
raise a pull full error when the connection pool is full and no conne…
…ction can be obtained
  • Loading branch information
tenderlove committed Dec 30, 2011
1 parent b1ac881 commit cceabe0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
Expand Up @@ -9,6 +9,13 @@ module ActiveRecord
class ConnectionTimeoutError < ConnectionNotEstablished
end

# Raised when a connection pool is full and another connection is requested
class PoolFullError < ConnectionNotEstablished
def initialize size, timeout
super("Connection pool of size #{size} and timeout #{timeout}s is full")
end
end

module ConnectionAdapters
# Connection pool base class for managing Active Record database
# connections.
Expand Down Expand Up @@ -60,7 +67,7 @@ class ConnectionPool
include MonitorMixin

attr_accessor :automatic_reconnect, :timeout
attr_reader :spec, :connections
attr_reader :spec, :connections, :size

# Creates a new ConnectionPool object. +spec+ is a ConnectionSpecification
# object which describes database connection information (e.g. adapter,
Expand Down Expand Up @@ -160,47 +167,42 @@ def verify_active_connections! #:nodoc:
end
end

# Return any checked-out connections back to the pool by threads that
# are no longer alive.
def clear_stale_cached_connections!
def clear_stale_cached_connections! # :nodoc:
end
deprecate :clear_stale_cached_connections!

# Check-out a database connection from the pool, indicating that you want
# to use it. You should call #checkin when you no longer need this.
#
# This is done by either returning an existing connection, or by creating
# a new connection. If the maximum number of connections for this pool has
# already been reached, but the pool is empty (i.e. they're all being used),
# then this method will wait until a thread has checked in a connection.
# The wait time is bounded however: if no connection can be checked out
# within the timeout specified for this pool, then a ConnectionTimeoutError
# exception will be raised.
# This is done by either returning and leasing existing connection, or by
# creating a new connection and leasing it.
#
# 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.
#
# Returns: an AbstractAdapter object.
#
# Raises:
# - ConnectionTimeoutError: no connection can be obtained from the pool
# within the timeout period.
# - PoolFullError: no connection can be obtained from the pool.
def checkout
# Checkout an available connection
synchronize do
# Try to find a connection that hasn't been leased
conn = @connections.find { |c| c.lease }
# Try to find a connection that hasn't been leased, and lease it
conn = connections.find { |c| c.lease }

# If all connections were leased, and we have room to expand,
# create a new connection and lease it.
if !conn && @connections.size < @size
if !conn && connections.size < size
conn = checkout_new_connection
conn.lease
end

if conn
checkout_and_verify conn
else
raise ConnectionTimeoutError, "could not obtain a database connection#{" within #{@timeout} seconds" if @timeout}. The max pool size is currently #{@size}; consider increasing it."
raise PoolFullError.new(size, timeout)
end

end
end

Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/connection_pool_test.rb
Expand Up @@ -25,6 +25,14 @@ def teardown
@pool.connections.each(&:close)
end

def test_full_pool_exception
assert_raises(PoolFullError) do
(@pool.size + 1).times do
@pool.checkout
end
end
end

def test_reap_and_active
@pool.checkout
@pool.checkout
Expand Down

7 comments on commit cceabe0

@alexeymuranov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this commit, on my machine i start getting an error when running

$ bundle exec rake test_postgresql

in activerecord:

  1) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    /Volumes/Data/Users/alexey/Development/rails/activerecord/test/cases/connection_pool_test.rb:84:in `ensure in test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/Development/rails/activerecord/test/cases/connection_pool_test.rb:84:in `test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

I do not get this error with

$ ARCONN=postgresql ruby -I test test/cases/connection_pool_test.rb

but on some later commits i do, but maybe it does not happen every time. On some of the later commits i get a message that "there are too many connections" for the same test file. Errors are in test_remove_connection and test_remove_connection_for_thread.

Maybe it is something about my local settings, but this is just to let you know.

Happy New Year! I am off.

@alexeymuranov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are more details. This is what has just happened on my machine, from activerecord directory:

§ bundle exec rake test_postgresql
...
Finished tests in 85.366136s, 39.0904 tests/s, 120.1765 assertions/s.

  1) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    /Volumes/Data/Users/alexey/Development/rails/activerecord/test/cases/connection_pool_test.rb:84:in `ensure in test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/Development/rails/activerecord/test/cases/connection_pool_test.rb:84:in `test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'
...
3337 tests, 10259 assertions, 0 failures, 1 errors, 10 skips
rake aborted!
...
§ ARCONN=postgresql ruby -I test test/cases/connection_pool_test.rb
Using postgresql with Identity Map off
Run options: 

# Running tests:

.......EE

Finished tests in 0.152619s, 58.9704 tests/s, 124.4930 assertions/s.

  1) Error:
test_remove_connection(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    test/cases/connection_pool_test.rb:76:in `ensure in test_remove_connection'
    test/cases/connection_pool_test.rb:76:in `test_remove_connection'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

  2) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    test/cases/connection_pool_test.rb:84:in `ensure in test_remove_connection_for_thread'
    test/cases/connection_pool_test.rb:84:in `test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

9 tests, 19 assertions, 0 failures, 2 errors, 0 skips

Same command again:

§ ARCONN=postgresql ruby -I test test/cases/connection_pool_test.rb
...
9 tests, 23 assertions, 0 failures, 0 errors, 0 skips

Like this 2 more times, then on the third time:

Using postgresql with Identity Map off
Run options: 

# Running tests:

........E

Finished tests in 0.139846s, 64.3565 tests/s, 157.3159 assertions/s.

  1) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    test/cases/connection_pool_test.rb:84:in `ensure in test_remove_connection_for_thread'
    test/cases/connection_pool_test.rb:84:in `test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

9 tests, 22 assertions, 0 failures, 1 errors, 0 skips

and on the fourth time:

Using postgresql with Identity Map off
Run options: 

# Running tests:

.......EE

Finished tests in 0.139984s, 64.2931 tests/s, 135.7298 assertions/s.

  1) Error:
test_remove_connection(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    test/cases/connection_pool_test.rb:76:in `ensure in test_remove_connection'
    test/cases/connection_pool_test.rb:76:in `test_remove_connection'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

  2) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
NoMethodError: undefined method `close' for nil:NilClass
    test/cases/connection_pool_test.rb:84:in `ensure in test_remove_connection_for_thread'
    test/cases/connection_pool_test.rb:84:in `test_remove_connection_for_thread'
    /Volumes/Data/Users/alexey/.rvm/gems/ruby-1.9.3-p0/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_230_to_251.rb:28:in `run'

9 tests, 19 assertions, 0 failures, 2 errors, 0 skips

In one of my previous experiments, i've also got another error:

Using postgresql with Identity Map off
Run options: 

# Running tests:

........E

Finished tests in 0.141029s, 63.8167 tests/s, 155.9963 assertions/s.

  1) Error:
test_remove_connection_for_thread(ActiveRecord::ConnectionAdapters::ConnectionPoolTest):
PGError: FATAL:  sorry, too many clients already

@tenderlove
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it happen against the tip?

@alexeymuranov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, starting from this commit, i always get either one or two errors NoMethodError: undefined method 'close' for nil:NilClass in those test cases when running

bundle exec rake test_postgresql

Last time just tried with 29f0f25.

@alexeymuranov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried again with the previous commit b1ac881, tests pass without errors.

@tenderlove
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed in 41be0fc

@alexeymuranov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it passes with the latest 49b6b49!

Please sign in to comment.