Enhance open connection metrics#8124
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8124 +/- ##
=======================================
Coverage 82.9% 82.9%
=======================================
Files 823 823
Lines 361216 361241 +25
=======================================
+ Hits 299777 299809 +32
+ Misses 61439 61432 -7 🚀 New features to boost your workflow:
|
| struct ConnectionTable { | ||
| table: IndexMap<ConnectionTableKey, Vec<ConnectionEntry>>, | ||
| total_size: usize, | ||
| is_staked: bool, |
There was a problem hiding this comment.
For the readability, I personally would prefer enum with Staked/Unstaked. More like an opinion, not really insisting on it.
KirillLykov
left a comment
KirillLykov
left a comment
There was a problem hiding this comment.
looks good to me to distinguish these two metrics.
| @@ -210,6 +210,8 @@ pub struct StreamerStats { | |||
| pub(crate) connection_rate_limiter_length: AtomicUsize, | |||
| // All connections in various states such as Incoming, Connecting, Connection | |||
| pub(crate) open_connections: AtomicUsize, | |||
There was a problem hiding this comment.
Should we axe open_connections ? it gets updated differently and can be very confusing. If someone ever needs the sum, they can trivially add the two in SQL in metrics.
There was a problem hiding this comment.
@alexpyattaev, I reviewed the code again. I think there is a value of having open_connections which can track all open connections in various state -- even the connections which did not make into the connection table yet like those spawn into the async tasks before they were allowed into the connection table. This can help understand the worst case connection load in the system.
alexpyattaev
left a comment
alexpyattaev
left a comment
There was a problem hiding this comment.
The idea is great, thank you! However, I believe we should axe open_connections stat completely from code, it is extra complexity for no benefit. Can be another PR of course.
4 participants
Problem
We only have open_connections which include both staked and unstaked. It is hard to tell what is the usage for each.
Summary of Changes
introduced open_staked_connections and open_unstaked_connections metrics.
Fixes #