Skip to content

Add ActiveRecord::Base.connection_pool.stat - #26988

Merged
rafaelfranca merged 1 commit into
rails:masterfrom
Paxa:connection_pool_stat
Nov 8, 2016
Merged

Add ActiveRecord::Base.connection_pool.stat#26988
rafaelfranca merged 1 commit into
rails:masterfrom
Paxa:connection_pool_stat

Conversation

@Paxa

@Paxa Paxa commented Nov 7, 2016

Copy link
Copy Markdown
Contributor

Example:

p ActiveRecord::Base.connection_pool.stat
{
  max: 25,
  total: 1,
  busy: 1,
  dead: 0,
  idle: 0,
  num_waiting: 0,
  checkout_timeout: 5
}

Related to #26898

@rails-bot

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @chancancode (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

@Paxa

Paxa commented Nov 7, 2016

Copy link
Copy Markdown
Contributor Author

@matthewd is it how you think or it?

@Paxa
Paxa force-pushed the connection_pool_stat branch from 395588b to 36b146c Compare November 7, 2016 10:57
@Paxa Paxa changed the title Add ActiveRecord::ConnectionAdapters::ConnectionPool.stat Add ActiveRecord::Base.connection_pool.stat Nov 7, 2016
@matthewd

matthewd commented Nov 8, 2016

Copy link
Copy Markdown
Member

👍

Wrap the method body in a synchronize do block to ensure the numbers are consistent.

@Paxa

Paxa commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

Done

@Paxa
Paxa force-pushed the connection_pool_stat branch 2 times, most recently from 08c91e2 to 72c3dfb Compare November 8, 2016 14:35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great idea! Personal preference but max and total are a bit confusing to me. Naming is hard. We instantiate a pool with a "size". A pool contains "connections". Therefore, my poor naming would be:

size: size
connections: @connections.size

Either way, this is helpful!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah. I like the naming suggested by @jrafanie

@Paxa

Paxa commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

Changed to

{ size: 15, connections: 1, busy: 1, dead: 0, idle: 0, waiting: 0, checkout_timeout: 5 }

Also waiting: feels odd to me, because all other values related to connections, but waiting are consumers. Will it be better to name it as waiting_in_queue or queue?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

# Example:
#
#    ActiveRecord::Base.connection_pool.stat # => { size: 15, connections: 1, busy: 1, dead: 0, idle: 0, waiting: 0, checkout_timeout: 5 }

@rafaelfranca

Copy link
Copy Markdown
Member

Could you add a CHANGELOG entry and squash your commits?

@Paxa
Paxa force-pushed the connection_pool_stat branch from f41319c to 35b6898 Compare November 8, 2016 17:11
@Paxa

Paxa commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

Done

@rafaelfranca
rafaelfranca merged commit 1b16e4c into rails:master Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants