From d07a6b1a4a234908959650197f596329ca08b4f0 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 14:11:44 -0500 Subject: [PATCH] Make clear_active_connections! also return stale connections back to the pool - also clean up some cruft remaining from per-thread connection cache --- .../abstract/connection_pool.rb | 53 +++++------ .../abstract/connection_specification.rb | 5 +- activerecord/test/cases/locking_test.rb | 2 - .../test/cases/pooled_connections_test.rb | 87 ++++++++++++++++++ .../test/cases/threaded_connections_test.rb | 91 ------------------- activerecord/test/cases/transactions_test.rb | 3 - 6 files changed, 112 insertions(+), 129 deletions(-) create mode 100644 activerecord/test/cases/pooled_connections_test.rb delete mode 100644 activerecord/test/cases/threaded_connections_test.rb diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 1b908c3113..472cf64c54 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -60,11 +60,11 @@ module ActiveRecord # #connection can be called any number of times; the connection is # held in a hash keyed by the thread id. def connection - if conn = @reserved_connections[active_connection_name] + if conn = @reserved_connections[current_connection_id] conn.verify!(verification_timeout) conn else - @reserved_connections[active_connection_name] = checkout + @reserved_connections[current_connection_id] = checkout end end @@ -72,7 +72,7 @@ module ActiveRecord # #release_connection releases the connection-thread association # and returns the connection to the pool. def release_connection - conn = @reserved_connections.delete(active_connection_name) + conn = @reserved_connections.delete(current_connection_id) checkin conn if conn end @@ -118,14 +118,20 @@ module ActiveRecord # Verify active connections and remove and disconnect connections # associated with stale threads. def verify_active_connections! #:nodoc: - remove_stale_cached_threads!(@reserved_connections) do |name, conn| - checkin conn - end + clear_stale_cached_connections! connections.each do |connection| connection.verify!(verification_timeout) end end + # Return any checked-out connections back to the pool by threads that + # are no longer alive. + def clear_stale_cached_connections! + remove_stale_cached_threads!(@reserved_connections) do |name, conn| + checkin conn + end + end + # Check-out a database connection from the pool. def checkout raise NotImplementedError, "checkout is an abstract method" @@ -156,7 +162,7 @@ module ActiveRecord ActiveRecord::Base.send(spec.adapter_method, config) end - def active_connection_name #:nodoc: + def current_connection_id #:nodoc: Thread.current.object_id end @@ -178,12 +184,6 @@ module ActiveRecord # NewConnectionEveryTime is a simple implementation: always # create/disconnect on checkout/checkin. class NewConnectionEveryTime < ConnectionPool - def active_connection - @reserved_connections[active_connection_name] - end - - def active_connections; @reserved_connections; end - def checkout new_connection end @@ -288,19 +288,14 @@ module ActiveRecord @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) end - # for internal use only and for testing; - # only works with ConnectionPerThread pool class - def active_connections #:nodoc: - @connection_pools.inject({}) do |hash,kv| - hash[kv.first] = kv.last.active_connection - hash.delete(kv.first) unless hash[kv.first] - hash - end - end - - # Clears the cache which maps classes to connections. + # Returns any connections in use by the current thread back to the pool, + # and also returns connections to the pool cached by threads that are no + # longer alive. def clear_active_connections! - @connection_pools.each_value {|pool| pool.release_connection } + @connection_pools.each_value do |pool| + pool.release_connection + pool.clear_stale_cached_connections! + end end # Clears the cache which maps classes @@ -353,11 +348,9 @@ module ActiveRecord end # Apply monitor to all public methods that access the pool. - synchronize :establish_connection, :retrieve_connection, - :connected?, :remove_connection, :active_connections, - :clear_active_connections!, :clear_reloadable_connections!, - :clear_all_connections!, :verify_active_connections!, - :with => :connection_pools_lock + synchronize :establish_connection, :retrieve_connection, :connected?, :remove_connection, + :clear_active_connections!, :clear_reloadable_connections!, :clear_all_connections!, + :verify_active_connections!, :with => :connection_pools_lock end end end \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 47fc11a620..417a333aab 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -124,9 +124,8 @@ module ActiveRecord connection_handler.remove_connection(klass) end - delegate :active_connections, :clear_active_connections!, - :clear_reloadable_connections!, :clear_all_connections!, - :verify_active_connections!, :to => :connection_handler + delegate :clear_active_connections!, :clear_reloadable_connections!, + :clear_all_connections!,:verify_active_connections!, :to => :connection_handler end end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index df9829195c..bbe8582466 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -280,7 +280,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter) sleep zzz # block thread 2 for zzz seconds end t1 = Time.now - Person.clear_active_connections! end b = Thread.new do @@ -288,7 +287,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter) t2 = Time.now Person.transaction { yield } t3 = Time.now - Person.clear_active_connections! end a.join diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb new file mode 100644 index 0000000000..078ca1d679 --- /dev/null +++ b/activerecord/test/cases/pooled_connections_test.rb @@ -0,0 +1,87 @@ +require "cases/helper" + +class PooledConnectionsTest < ActiveRecord::TestCase + def setup + super + @connection = ActiveRecord::Base.remove_connection + end + + def teardown + ActiveRecord::Base.clear_all_connections! + ActiveRecord::Base.establish_connection(@connection) + super + end + + def checkout_connections + ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3})) + @connections = [] + @timed_out = 0 + + 4.times do + Thread.new do + begin + @connections << ActiveRecord::Base.connection_pool.checkout + rescue ActiveRecord::ConnectionTimeoutError + @timed_out += 1 + end + end.join + end + end + + def test_pooled_connection_checkout + checkout_connections + assert_equal @connections.length, 2 + assert_equal @timed_out, 2 + end + + def checkout_checkin_connections(pool_size, threads) + ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5})) + @connection_count = 0 + @timed_out = 0 + threads.times do + Thread.new do + begin + conn = ActiveRecord::Base.connection_pool.checkout + sleep 0.1 + ActiveRecord::Base.connection_pool.checkin conn + @connection_count += 1 + rescue ActiveRecord::ConnectionTimeoutError + @timed_out += 1 + end + end.join + end + end + + def test_pooled_connection_checkin_one + checkout_checkin_connections 1, 2 + assert_equal 2, @connection_count + assert_equal 0, @timed_out + end + + def test_pooled_connection_checkin_two + checkout_checkin_connections 2, 3 + assert_equal 3, @connection_count + assert_equal 0, @timed_out + end + + def test_pooled_connection_checkout_existing_first + ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1})) + conn_pool = ActiveRecord::Base.connection_pool + conn = conn_pool.checkout + conn_pool.checkin(conn) + conn = conn_pool.checkout + assert ActiveRecord::ConnectionAdapters::AbstractAdapter === conn + conn_pool.checkin(conn) + end +end unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name + +class AllowConcurrencyDeprecatedTest < ActiveRecord::TestCase + def test_allow_concurrency_is_deprecated + assert_deprecated('ActiveRecord::Base.allow_concurrency') do + ActiveRecord::Base.allow_concurrency + end + assert_deprecated('ActiveRecord::Base.allow_concurrency=') do + ActiveRecord::Base.allow_concurrency = true + end + end +end diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb deleted file mode 100644 index 54c1470e69..0000000000 --- a/activerecord/test/cases/threaded_connections_test.rb +++ /dev/null @@ -1,91 +0,0 @@ -require "cases/helper" -require 'models/topic' -require 'models/reply' - -unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name - class ThreadedConnectionsTest < ActiveRecord::TestCase - def test_allow_concurrency_is_deprecated - assert_deprecated('ActiveRecord::Base.allow_concurrency') do - ActiveRecord::Base.allow_concurrency - end - assert_deprecated('ActiveRecord::Base.allow_concurrency=') do - ActiveRecord::Base.allow_concurrency = true - end - end - end - - class PooledConnectionsTest < ActiveRecord::TestCase - def setup - super - @connection = ActiveRecord::Base.remove_connection - end - - def teardown - ActiveRecord::Base.clear_all_connections! - ActiveRecord::Base.establish_connection(@connection) - super - end - - def checkout_connections - ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3})) - @connections = [] - @timed_out = 0 - - 4.times do - Thread.new do - begin - @connections << ActiveRecord::Base.connection_pool.checkout - rescue ActiveRecord::ConnectionTimeoutError - @timed_out += 1 - end - end.join - end - end - - def test_pooled_connection_checkout - checkout_connections - assert_equal @connections.length, 2 - assert_equal @timed_out, 2 - end - - def checkout_checkin_connections(pool_size, threads) - ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5})) - @connection_count = 0 - @timed_out = 0 - threads.times do - Thread.new do - begin - conn = ActiveRecord::Base.connection_pool.checkout - sleep 0.1 - ActiveRecord::Base.connection_pool.checkin conn - @connection_count += 1 - rescue ActiveRecord::ConnectionTimeoutError - @timed_out += 1 - end - end.join - end - end - - def test_pooled_connection_checkin_one - checkout_checkin_connections 1, 2 - assert_equal 2, @connection_count - assert_equal 0, @timed_out - end - - def test_pooled_connection_checkin_two - checkout_checkin_connections 2, 3 - assert_equal 3, @connection_count - assert_equal 0, @timed_out - end - - def test_pooled_connection_checkout_existing_first - ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1})) - conn_pool = ActiveRecord::Base.connection_pool - conn = conn_pool.checkout - conn_pool.checkin(conn) - conn = conn_pool.checkout - assert ActiveRecord::ConnectionAdapters::AbstractAdapter === conn - conn_pool.checkin(conn) - end - end -end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index d3cc69507a..8383ba58e9 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -296,7 +296,6 @@ if current_adapter?(:PostgreSQLAdapter) topic.approved = !topic.approved? topic.save! end - Topic.clear_active_connections! end end @@ -332,7 +331,6 @@ if current_adapter?(:PostgreSQLAdapter) dev = Developer.find(1) assert_equal original_salary, dev.salary end - Developer.clear_active_connections! end end @@ -344,7 +342,6 @@ if current_adapter?(:PostgreSQLAdapter) # Always expect original salary. assert_equal original_salary, Developer.find(1).salary end - Developer.clear_active_connections! end end