Permalink
Browse files

One hash is enough

We don't need separate @class_to_pool and @connection_pool hashes.
  • Loading branch information...
jonleighton committed Aug 31, 2012
1 parent 4a274ed commit ba1544d71628abff2777c9c514142d7e9a159111
@@ -496,40 +496,38 @@ def checkout_and_verify(c)
# ActiveRecord::Base.connection_handler. Active Record models use this to
# determine that connection pool that they should use.
class ConnectionHandler
- def initialize(pools = Hash.new { |h,k| h[k] = {} })
- @connection_pools = pools
- @class_to_pool = Hash.new { |h,k| h[k] = {} }
+ def initialize
+ @class_to_pool = Hash.new { |h,k| h[k] = {} }
end
def connection_pools
- @connection_pools[Process.pid]
+ class_to_pool.values.compact
end

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Nov 6, 2012

Member

This change broke some code. handler.connection_pools used to return a spec => pool map; now it returns an array of pools. This is in a gray area as far public API goes...

@jeremy

jeremy Nov 6, 2012

Member

This change broke some code. handler.connection_pools used to return a spec => pool map; now it returns an array of pools. This is in a gray area as far public API goes...

This comment has been minimized.

Show comment Hide comment
@jonleighton

jonleighton Nov 8, 2012

Member

Agree it's grey area. When you say it "broke some code", do you mean you were relying on this at 37s? I could fix it, I think at the time I thought it was sufficiently buried in AR internals that it didn't make a lot of difference to change. But can fix if you think we should.

@jonleighton

jonleighton Nov 8, 2012

Member

Agree it's grey area. When you say it "broke some code", do you mean you were relying on this at 37s? I could fix it, I think at the time I thought it was sufficiently buried in AR internals that it didn't make a lot of difference to change. But can fix if you think we should.

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Nov 8, 2012

Member

Yeah it broke some code we use to find the connection spec for a connection id. We send query timings to statsd by subscribing to AR notifications and looking up the database name from payload[:connection_id]. It's totally hax so a real api to find connection spec for a connection id would be welcome, too. In any case, we should probably be good citizens and leave this method alone.

@jeremy

jeremy Nov 8, 2012

Member

Yeah it broke some code we use to find the connection spec for a connection id. We send query timings to statsd by subscribing to AR notifications and looking up the database name from payload[:connection_id]. It's totally hax so a real api to find connection spec for a connection id would be welcome, too. In any case, we should probably be good citizens and leave this method alone.

This comment has been minimized.

Show comment Hide comment
@jonleighton

jonleighton Nov 9, 2012

Member

Ok: c3ca7ac

def establish_connection(klass, spec)
- class_to_pool[klass] =
- connection_pools[spec] = ConnectionAdapters::ConnectionPool.new(spec)
+ class_to_pool[klass] = ConnectionAdapters::ConnectionPool.new(spec)
end
# Returns true if there are any active connections among the connection
# pools that the ConnectionHandler is managing.
def active_connections?
- connection_pools.values.any? { |pool| pool.active_connection? }
+ connection_pools.any?(&:active_connection?)
end
# 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(&:release_connection)
end
# Clears the cache which maps classes.
def clear_reloadable_connections!
- connection_pools.each_value {|pool| pool.clear_reloadable_connections! }
+ connection_pools.each(&:clear_reloadable_connections!)
end
def clear_all_connections!
- connection_pools.each_value {|pool| pool.disconnect! }
+ connection_pools.each(&:disconnect!)
end
# Locate the connection of the nearest super class. This can be an
@@ -553,13 +551,11 @@ def connected?(klass)
# can be used as an argument for establish_connection, for easily
# re-establishing the connection.
def remove_connection(klass)
- pool = class_to_pool.delete(klass)
- return nil unless pool
-
- connection_pools.delete pool.spec
- pool.automatic_reconnect = false
- pool.disconnect!
- pool.spec.config
+ if pool = class_to_pool.delete(klass)
+ pool.automatic_reconnect = false
+ pool.disconnect!
+ pool.spec.config
+ end
end
def retrieve_connection_pool(klass)
@@ -879,7 +879,7 @@ def teardown_fixtures
end
def enlist_fixture_connections
- ActiveRecord::Base.connection_handler.connection_pools.values.map(&:connection)
+ ActiveRecord::Base.connection_handler.connection_pools.map(&:connection)
end
private

0 comments on commit ba1544d

Please sign in to comment.