ref(iroh-net): Remove need for relay info in best_addr #2579
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2579/docs/iroh/ Last updated: 2024-08-02T15:08:22Z |
| // Update some metrics | ||
| if matches!(prev_typ, ConnectionType::Direct(_)) { | ||
| inc!(MagicsockMetrics, num_direct_conns_removed); | ||
| } | ||
| if matches!(typ, ConnectionType::Direct(_)) { | ||
| inc!(MagicsockMetrics, num_direct_conns_added); | ||
| } | ||
| if matches!(typ, ConnectionType::Relay(_) | ConnectionType::Mixed(_, _)) | ||
| && !matches!( | ||
| prev_typ, | ||
| ConnectionType::Relay(_) | ConnectionType::Mixed(_, _) | ||
| ) | ||
| { | ||
| inc!(MagicsockMetrics, num_relay_conns_added); | ||
| } | ||
| if matches!(typ, ConnectionType::Direct(_) | ConnectionType::None) | ||
| && matches!( | ||
| prev_typ, | ||
| ConnectionType::Relay(_) | ConnectionType::Mixed(_, _) | ||
| ) | ||
| { | ||
| inc!(MagicsockMetrics, num_relay_conns_removed); | ||
| } |
There was a problem hiding this comment.
I wouldn't mind if this were refactored into a match (prev_typ, typ) expression.
There was a problem hiding this comment.
Be careful what you wish for... This version is huge and contains a whole bunch of states that are unreachable. Admittedly the if version has to be read backwards, where you think about what conditions are needed for a certain state. So I guess the big match is less of a head-twister.
There was a problem hiding this comment.
I'd didn't necessarily want it to be an exhaustive match 😅 so yeah I guess I have to be careful in what I wish for haha!
Basically, having a last _ => {} case would've been fine by me :)
Description
This moves metrics for when switches between direct connections and
relayed connections to the NodeState. This is after all where this
decision is made. BestAddr only knows about the best UDP address, it
has no business keeping track of relays.
Cleaning this up enables further refactoring of BestAddr and PathState
which need to improve because they depend on each other, but are
hindered by the relay state sneaking into there.
Breaking Changes
None
Notes & open questions
These metrics ignore the fact that we have mixed as a possibility so
over-simplify the situation. This makes the logic to increment them
complex, which in turn makes it no one will really know what they
mean.
I need this to move on with #2546 so don't really want to get into
designing our metrics though. I believe this way the keep the same
behaviour.
Change checklist