Skip to content

Commit

Permalink
RUBY-1454 Modify connection lookup for makara
Browse files Browse the repository at this point in the history
Based on instacart/makara#59, there were perf
issues in looking up the connection from the pool when on ActiveRecord
4.x using the makara adapter.

While this was a problem with makara itself, @mnelson suggested
alternate lookup code which works with makara and actually allocates
less, so we're updating to use that.
  • Loading branch information
jasonrclark committed Mar 12, 2015
1 parent 8e381f2 commit 82d2777
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions lib/new_relic/agent/instrumentation/active_record_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,16 @@ def record_metrics(event, config) #THREAD_LOCAL_ACCESS
def active_record_config_for_event(event)
return unless event.payload[:connection_id]

connections = ::ActiveRecord::Base.connection_handler.connection_pool_list.map { |handler| handler.connections }.flatten
connection = connections.detect { |cnxn| cnxn.object_id == event.payload[:connection_id] }
connection = nil
connection_id = event.payload[:connection_id]

::ActiveRecord::Base.connection_handler.connection_pool_list.each do |handler|
connection = handler.connections.detect do |conn|
conn.object_id == connection_id
end

break if connection
end

connection.instance_variable_get(:@config) if connection
end
Expand Down

3 comments on commit 82d2777

@mnelson
Copy link

Choose a reason for hiding this comment

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

Nice, thanks @jasonrclark

@jasonrclark
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem @mnelson. Note, though, that this is only on the dev branch and not in the 3.11.0 release that just went out. I noticed our deploy script was pushing more than just master and release out, which is why this showed up.

You can certainly feel free to test against dev, and this change should show up for release in a new agent coming soon. Will ping you back for real when that happens.

@mnelson
Copy link

Choose a reason for hiding this comment

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

No worries, I'll continue working on a makara solution regardless of the rpm <> makara situation.

Please sign in to comment.