Permalink
Browse files

make active_connection? return true only if there is an open connecti…

…on in use for the current thread. fixes #5330
  • Loading branch information...
1 parent 263d842 commit cff19cf545de61ada8904d9d3daaaa594a9a931f @tenderlove tenderlove committed Mar 8, 2012
View
7 activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -95,10 +95,11 @@ def connection
@reserved_connections[current_connection_id] ||= checkout
end
- # Check to see if there is an active connection in this connection
- # pool.
+ # Is there an open connection that is being used for the current thread?
def active_connection?
- active_connections.any?
+ @reserved_connections.fetch(current_connection_id) {
+ return false
+ }.in_use?
end
# Signal that the thread is finished with the current connection.
View
42 activerecord/test/cases/connection_pool_test.rb
@@ -3,7 +3,11 @@
module ActiveRecord
module ConnectionAdapters
class ConnectionPoolTest < ActiveRecord::TestCase
+ attr_reader :pool
+
def setup
+ super
+
# Keep a duplicate pool so we do not bother others
@pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
@@ -18,6 +22,44 @@ def setup
end
end
+ def teardown
+ super
+ @pool.disconnect!
+ end
+
+ def active_connections(pool)
+ pool.connections.find_all(&:in_use?)
+ end
+
+ def test_with_connection
+ assert_equal 0, active_connections(pool).size
+
+ main_thread = pool.connection
+ assert_equal 1, active_connections(pool).size
+
+ Thread.new {
+ pool.with_connection do |conn|
+ assert conn
+ assert_equal 2, active_connections(pool).size
+ end
+ assert_equal 1, active_connections(pool).size
+ }.join
+
+ main_thread.close
+ assert_equal 0, active_connections(pool).size
+ end
+
+ def test_active_connection_in_use
+ assert !pool.active_connection?
+ main_thread = pool.connection
+
+ assert pool.active_connection?
+
+ main_thread.close
+
+ assert !pool.active_connection?
+ end
+
def test_active_connection?
assert !@pool.active_connection?
assert @pool.connection

3 comments on commit cff19cf

@brianmario

👍

curious if this will help any regarding brianmario/mysql2#66, brianmario/mysql2#209 or brianmario/mysql2#213

@tenderlove
Ruby on Rails member

@brianmario I don't think so... I've seen those errors before, but I can't remember exactly the problem. IIRC, it happens if you have a low traffic app where the connection timeout is not long enough, but I can't remember.

@jeremy do you recall issues like these? I seem to remember there was a fix in rails.

@fxn
Ruby on Rails member

I know of an application that reports lost connections often. It is a busy application (about 40K rpm). A priori seems strange because the connection pool does a mysql_ping on checkout, so you got a successful ping and just milliseconds later the connection is lost (or the server gone, also happens). The MySQL server on the other hand seems to be doing fine.

Not saying it is related to this patch, just a followup.

Please sign in to comment.