elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
309 stars 112 forks source link

Add metrics for connection pool usage #292

Closed peixian closed 5 months ago

peixian commented 1 year ago

This PR adds support for a DBConnection.ConnectionPool.get_metrics() function that returns the current state of active and waiting connections, to allow upstream callers to gain some visibility on their database usage. We're using a form of this in our internal forks.

josevalim commented 1 year ago

Hi @peixian! This looks good to me but I would consider using :counters instead, so we can read them decentralized. We would also need tests. :)

whatyouhide commented 1 year ago

counters is such a good idea for this 👏

peixian commented 1 year ago

@josevalim I'll change it to counters. For tests, it appears that the tests are by default already broken? I didn't add onto it because I wasn't sure what the state of tests are.

For example:

 ~/c/db_connection_master (master)> git log | head                                                                                             
commit 022393b7895f5bd3a7e884a35c7590f4fa549986
Author: Andrea Leopardi <an.leopardi@gmail.com>
Date:   Thu Jul 27 18:33:49 2023 +0200

    Add DBConnection.available_connection_options/0 (#291)

~/c/db_connection_master (master) [2]> mix test .     
.......
Finished in 1.7 seconds (1.7s async, 0.00s sync)
202 tests, 32 failures
josevalim commented 1 year ago

Instead of mix test ., please do mix test.all.

Also, when a new connection is started, it always checks itself in once ready to use, so you need to take that into account.

peixian commented 1 year ago

@josevalim this is good for re-review, I've got all the tests passing and added a few.

romul commented 8 months ago

@josevalim I'm interested in this feature. What's the current status? Any help required?