Adds new statistics attributes for tracking connections closed #205
Conversation
bb8/src/internals.rs
Outdated
| @@ -290,3 +305,8 @@ pub(crate) enum StatsGetKind { | |||
| Waited, | |||
| TimedOut, | |||
| } | |||
|
|
|||
| pub(crate) enum StatsConnectionClosedKind { | |||
There was a problem hiding this comment.
Since there's nothing "special" about these values, let's just call this StatsKind again.
There was a problem hiding this comment.
TBH i would prefer to keep the ConnectionClosed since this is used for recording closed connections, but Ill make the change
There was a problem hiding this comment.
It can be used for any kind of simple increment of AtomicU64. There's nothing in the record() logic that is specific to ConnectionClosed events.
There was a problem hiding this comment.
Its ok, lets see how record evolves .... BTW added the changes that you asked
There was a problem hiding this comment.
If we rename the enum might also rename the variants to be like ClosedBroken, ClosedInvalid.
bb8/src/internals.rs
Outdated
|
|
||
| pub(crate) fn record(&self, kind: StatsConnectionClosedKind) { | ||
| match kind { | ||
| StatsConnectionClosedKind::Broken => self |
There was a problem hiding this comment.
Maybe yield &self.connections_closed_broken from the match arm and then move the .fetch_add(1, Ordering::SeqCst) call out of the `match?
There was a problem hiding this comment.
I can try it, but to me seems to overcomplicate things for just trying to make the code less verbose but making it less readable.
There was a problem hiding this comment.
I prefer the conciseness and reduced duplication.
pfreixes
force-pushed
the
record_connections_closed_2
branch
from
2b787ac to
5f8886b
Compare
|
@djc squashed requested changes into my latest commit PTAL one you have a moment |
|
I'm not seeing the changes I suggested appear in your commit, I think? Are you sure your squashing worked as expected? |
pfreixes
force-pushed
the
record_connections_closed_2
branch
from
5f8886b to
c22505a
Compare
|
@djc, forgot to add the files changed 🤦 |
bb8/src/internals.rs
Outdated
| StatsKind::ClosedBroken => &self.connections_closed_broken, | ||
| StatsKind::ClosedInvalid => &self.connections_closed_invalid, | ||
| }; | ||
| stats.fetch_add(1, Ordering::SeqCst); |
There was a problem hiding this comment.
Sorry, I meant you can chain the fetch_add() directly onto the match { .. } without binding it.
There was a problem hiding this comment.
No problemo, I guess that behind the scenes the compiler will do the same right? In any case less code is better, feedback applied with squashing.
The two new attributes `connections_closed_broken` and `connections_closed_invalid` can be used for respectively understand how many conections were closed due to be considered broken or invalid.
pfreixes
force-pushed
the
record_connections_closed_2
branch
from
c22505a to
3e35964
Compare
The two new attributes
connections_closed_brokenandconnections_closed_invalidcan be used for respectively understand how many connections were closed due to be considered broken or invalid.The
StatsKindinternal enum was also renamed toStatsGetKindfor better clarity.Note: A new PR for tracking the reaped connections will be created on top of these changes.