feat: congestion metrics #3491
feat: congestion metrics #3491
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3491/docs/iroh/ Last updated: 2025-10-07T18:23:25Z |
Arqu
force-pushed
the
arqu/congestion_metrics
branch
from
9e23616 to
a89e698
Compare
Arqu
changed the title
|
conflicts |
iroh/src/magicsock/metrics.rs
Outdated
| path_failure_resets: Counter::default(), | ||
| path_packet_loss_rate: packet_loss_buckets(), | ||
| path_rtt_variance_ms: rtt_variance_buckets(), | ||
| path_quality_score: quality_score_buckets(), |
There was a problem hiding this comment.
(not blocking) it's a bit sad we have to do this manually when including histograms now :( I wonder if we could adjust our macro or sth
cc @Frando
There was a problem hiding this comment.
I was looking for some magic syntax sugar with a default annotation or sth but couldn't work it out.
The problem is for each histogram you want to provide a specific set of buckets that make sense for that metric specifically, so you need to "adjust" it here. However if there was some magic #[default...] annotation I couldn't find where I pass in those funcs, it would be great.
There was a problem hiding this comment.
We can add such an annotation to the macro. Can do tomorrow.
There was a problem hiding this comment.
Here's the macro: n0-computer/iroh-metrics#37
There was a problem hiding this comment.
Nothing really against it if it helps somewhere. But be aware that so much of this will disappear soon. E.g. I just deleted the entire path_validity module in the feat-multipath branch.
I'm expecting to not try and merge any of the metrics changes into there and instead we'll have to give it a separate metrics review to get back at least what may still be relevant once it's merged.
iroh/src/magicsock/metrics.rs
Outdated
| } | ||
|
|
||
| fn packet_loss_buckets() -> Histogram { | ||
| Histogram::new(vec![0.0, 0.01, 0.05, 0.1, 0.2, 0.5, 1.0]) |
There was a problem hiding this comment.
I guess the buckets are:
- [0, 0.01)
- [0.01, 0.05)
- [0.1, 0.2)
- [0.2, 0.5)
- [0.5, 1.0)
- [1.0, infinity]
?
|
I think that's fine, this is mostly a short term exploration of the space. |
Arqu
force-pushed
the
arqu/congestion_metrics
branch
from
a89e698 to
bf1548c
Compare
|
Currently failing because of the dependency on n0-computer/net-tools#36 |
Description
Needs n0-computer/iroh-metrics#35 and updating the Cargo.toml's again.
Breaking Changes
Notes & open questions
Change checklist
quic-rpciroh-gossipiroh-blobsdumbpipesendme