feat: Add pool connection info to __lbheartbeat__ for ops #985
Conversation
This will add several fields to `__lbheartbeat__` if
`database_pool_max_size` is specified. These include:
```json
{
"active_connections": ... /* Number of active connections */,
"idle_connections": ... /* number of idle connections */,
"duration": ... /* how long no idle connections have been availble */,
}
```
Note that "duration" will only be present if `idle_connections` has been
zero since the last time a check was performed.
* this also adds `database_pool_max_size` as a config option.
Issue: #945
…nto feat/better_heartbeat
src/web/handlers.rs
Outdated
|
|
||
| if let Some(max_size) = deadman.max_size { | ||
| if db_state.connections >= max_size && db_state.idle_connections == 0 { |
There was a problem hiding this comment.
Here and below when we include it in json: connections isn't the active count
| if let Some(max_size) = deadman.max_size { | |
| if db_state.connections >= max_size && db_state.idle_connections == 0 { | |
| let active = db_state.connections - db_state.idle_connections; | |
| if let Some(max_size) = deadman.max_size { | |
| if active >= max_size && db_state.idle_connections == 0 { |
src/settings.rs
Outdated
| @@ -114,6 +121,7 @@ impl Settings { | |||
| s.set_default("database_pool_connection_timeout", Some(30))?; | |||
| s.set_default("database_use_test_transactions", false)?; | |||
| s.set_default("master_secret", "")?; | |||
| s.set_default::<Option<String>>("database_pool_max_size", Some("10".to_owned()))?; | |||
There was a problem hiding this comment.
probably want to kill this (was it for debugging?). I'm surprised it even works as a String (it expects u32 here)
There was a problem hiding this comment.
This can be simplified to just Some(10), rather than letting things auto-parse. There was some old debugging that is the source. I do note that we have database_pool_max_size set, but weren't including it as an option?
There was a problem hiding this comment.
We just didn't fill in a default here, so it defaults to None. The different db backends do their own default when it's None -- hence suggesting we kill it because it isn't really needed but it also doesn't match the Default impl setting it to None (this method's defaults values setup here and the Default impl values should really match and hopefully one day we won't need both versions of setting defaults #253)
There was a problem hiding this comment.
I'll add a comment in that respect so it's clearer. Thanks!
There was a problem hiding this comment.
Heh, although I see the value is 10 for both mysql and spanner.
| }; | ||
|
|
||
| let deadarc = state.deadman.clone(); | ||
| let mut deadman = *deadarc.read().await; |
There was a problem hiding this comment.
This confused me for a while (getting a mut here) -- finally realized it's a Copy type
…yncstorage-rs into feat/better_heartbeat
Description
This will add several fields to
__lbheartbeat__ifdatabase_pool_max_sizeis specified. These include:{ "active_connections": ... /* Number of active connections */, "idle_connections": ... /* number of idle connections */, "duration": ... /* how long no idle connections have been availble */, }Note that "duration" will only be present if
idle_connectionshas beenzero since the last time a check was performed.
database_pool_max_sizeas a config option.Testing
curl http://{server}/__lbheartbeatshould return200 with a body of
{"active_connections":0,"idle_connections":0}(values may differ depending on the server being queried.
A saturated server should return something like:
{"active_connections":0,"idle_connections":0, "duration_ms": 1000}indicating that the server has been saturated for 1 second.
Note the saturation is only checked on each call to
__lbheartbeat__, however that should be sufficient for most of the instances we've seen.Issue(s)
Issue: #945