refactor: implement connection tracking in metrics#4672
refactor: implement connection tracking in metrics#4672mkleczek wants to merge 2 commits intoPostgREST:mainfrom
Conversation
Fix should have an entry on the CHANGELOG |
| SQL.ReadyForUseConnectionStatus -> atomically $ | ||
| SH.insert identity uuid connTrackConnected *> | ||
| SH.focus Focus.delete identity uuid connTrackInUse | ||
| SQL.TerminatedConnectionStatus _ -> atomically $ | ||
| SH.focus Focus.delete identity uuid connTrackConnected *> | ||
| SH.focus Focus.delete identity uuid connTrackInUse | ||
| SQL.InUseConnectionStatus -> atomically $ | ||
| SH.insert identity uuid connTrackInUse | ||
| _ -> mempty |
There was a problem hiding this comment.
Could there be noticeable overhead now that we have the insert/focus logic?
Doesn't look like it from the mixed loadtest but want to make sure.
| connectionCounts :: ConnTrack -> IO ConnStats | ||
| connectionCounts = atomically . fmap (uncurry ConnStats) . bisequenceA . both SH.size . (connTrackConnected &&& connTrackInUse) |
There was a problem hiding this comment.
This is purely used for testing right? If so, a comment here would be good for clarity.
There was a problem hiding this comment.
This is purely used for testing right? If so, a comment here would be good for clarity.
@steve-chavez It is not. The actual Metric is implementation is based on ConnTrack:
https://github.com/mkleczek/postgrest/blob/7e9df6416195b3d9c534c1f1c4aeb39865869f45/src/PostgREST/Metrics.hs#L59
Also - this function is crucial for #4643 - see: https://github.com/mkleczek/postgrest/blob/f3e06920544b88d26809e75d2652d2ed6a4ef8f3/src/PostgREST/AppState.hs#L421
0f475e3 to
592e5a3
Compare
592e5a3 to
1f3b4bc
Compare
DISCLAIMER: This commit was authored entirely by a human without the assistance of LLMs. Some helpers are provided for introspecting metrics already (used in JWT cache tests). This change provides facilities to additionally validate emited Observation events. A new Spec module is also implemented, adding basic tests of schema cache reloading - their main goal is to excercise the new infrastructure.
DISCLAIMER: This commit was authored entirely by a human without the assistance of LLMs. Right now metrics observation handler does not track database connections but updates a single Gauge based on HasqlPoolObs events. This is problematic because Hasql pool reports various connection events in multiple phases. The connection state machine is not simple and to precisely report the number of connections in various states, it is necessary to track their lifecycles. This change adds a ConnTrack data structure and logic to track database connections lifecycles. At the moment it supports "connected" and "inUse" connection counts precisely. The "pgrst_db_pool_available" metric is implemented on top of ConnTrack instead of a simple Gauge.
1f3b4bc to
bce1f6e
Compare
DISCLAIMER:
This commit was authored entirely by a human without the assistance of LLMs.Right now metrics observation handler does not track database connections but updates a single Gauge based on HasqlPoolObs events.
This is problematic because Hasql pool reports various connection events in various states that make it impossible to predict the state change from the received event. The connection state machine is not simple and to precisely report the number of connections in various states, it is necessary to track their lifecycles.
Fixes #4622