Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DB metrics #141

Merged
merged 1 commit into from
Jan 10, 2024
Merged

Fix DB metrics #141

merged 1 commit into from
Jan 10, 2024

Conversation

joelmccracken
Copy link
Contributor

@joelmccracken joelmccracken commented Jan 5, 2024

  • fix active_pool_connections
  • start tracking queries_enqueued_and_processing where active_pool_connections was previously

@pbrisbin
Copy link
Member

pbrisbin commented Jan 8, 2024

🤦 I'm sorry. I got notified of this PR being opened and thought it was a review-request. My notifications on freckle-app are a little aggressive. I now see you just opened a draft. Obviously, ignore me.

@joelmccracken joelmccracken force-pushed the jnm/fix-db-metrics branch 3 times, most recently from ab1069e to fd4357f Compare January 10, 2024 15:48
@joelmccracken joelmccracken marked this pull request as ready for review January 10, 2024 15:59
Previously, the active_pool_connections metric was incremented _before_
the connection was obtained from the pool. This means that if there was
a queue of requests awaiting connection availablity, the metric value
would increase.

The number of enqueued requests is still valuable to know though so we
pick a metric with a better name (queries_enqueued_and_processing)
and use it in the place of active_pool_connections, and we set
active_pool_connections in another way which (I believe) will accurately
capture this.
@joelmccracken joelmccracken merged commit 866ce28 into main Jan 10, 2024
5 of 6 checks passed
@joelmccracken joelmccracken deleted the jnm/fix-db-metrics branch January 10, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants