feat(multipath): add back path congestion metrics #3669

Frando · 2025-11-17T13:17:23Z

Description

Based on #3672

Add back some of the path congestion metrics added in #3491. @Arqu please check my port of these metrics and if the calculations are correct. I commented-out the metrics added in #3491 that I didn't know how to port. But I think I got the important ones.

Breaking Changes

Notes & open questions

Change checklist

github-actions · 2025-11-17T13:19:30Z

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3669/docs/iroh/

Last updated: 2025-11-18T10:49:06Z

dignifiedquire · 2025-11-17T14:12:20Z

iroh/src/magicsock/remote_map/remote_state.rs

@@ -220,6 +226,7 @@ impl EndpointStateActor {
        trace!("actor started");
        let idle_timeout = MaybeFuture::None;
        tokio::pin!(idle_timeout);
+        let mut metrics_interval = time::interval(METRICS_INTERVAL);


should this be cfged as well?

Failed to get the cfg be happy with tokio::select!. Maybe should be forever-pending then if the cfg is not active?

yeah, sth like that I have done in the past

flub

I'd suggest to limit this to figure out what the easy and interesting counters to add. Nothing more. If we need congestion metrics we can do that in a later pass.

flub · 2025-11-17T14:11:49Z

iroh/src/magicsock/remote_map/remote_state.rs

@@ -75,6 +78,9 @@ const APPLICATION_ABANDON_PATH: u8 = 30;
 /// in a high frequency, and to keep data about previous path around for subsequent connections.
 const ACTOR_MAX_IDLE_TIMEOUT: Duration = Duration::from_secs(60);

+/// Interval in which connection and path metrics are emitted.
+const METRICS_INTERVAL: Duration = Duration::from_secs(10);


Soo, I'm going to argue that this is fundamentally doing metrics wrong. Metrics increment counters or record historgrams at discreet points. If you are recording something on a timer you were supposed to provide the metrics that can compute the thing, and the metrics backend can compute it on the fly over the range it wants, using sampling intervals it wants etc etc.

What would be discreet points to record connection latency at? Open/close path events?

flub · 2025-11-17T14:12:17Z

iroh/src/magicsock/remote_map/remote_state/metrics.rs

This is metrics backend code. I'll have a much easier time with this PR if we don't try and do anything like that here.

This is present on main. It is not new, just ported from main to feat-multipath. See #3606 and #3491.

Happy to not do this here, but IIRC these are the metrics we are most interested in to compare feat-multipath to main via n0des? Correct me if I'm wrong please, @Arqu .

Edit: I've split the PR, the simple metrics are now here #3672 and this PR is about adding back the congestion metrics.

dignifiedquire · 2025-11-17T14:13:36Z

iroh/src/magicsock/remote_map/remote_state.rs

+    fn record_metrics(&mut self) {
+        #[cfg(feature = "metrics")]
+        for state in self.connections.values_mut() {
+            state.record_metrics_periodic(&self.metrics, self.selected_path.get());


sorry if I am missing sth, but should this be the path for the specific connection that we pass in here, this seems like a generic one?

We have a single selected path per ~~connection ~~remote endpoint. This is passed through here, because the metrics tracking code further down uses the RTT of the selected path as the connection's RTT. (There is no connection-level RTT exposed anymore with multipath on the quinn level).

but why is it the same for all connections?

Currently the selected path logic works on the level of the remote endpoint, and the result is then applied to all connections.

iroh/src/magicsock/remote_map/remote_state/metrics.rs

Frando · 2025-11-17T14:39:22Z

I'd suggest to limit this to figure out what the easy and interesting counters to add. Nothing more. If we need congestion metrics we can do that in a later pass.

This is present on main. It is not new, just ported from main to feat-multipath. See #3606 and #3491.

Happy to not do this here and only the simple counters initially, but IIRC these are the metrics we are most interested in to compare feat-multipath to main via n0des? Correct me if I'm wrong please, @Arqu .

Frando · 2025-11-17T15:23:30Z

I've split this PR: The basic, simple counter metrics are now in #3672. This PR is now based on 3672 and adds back the congestion metrics.

Frando requested review from Arqu, dignifiedquire and flub November 17, 2025 13:17

Frando force-pushed the Frando/mp-metrics branch from 2f4092f to 21448f3 Compare November 17, 2025 13:21

Frando linked an issue Nov 17, 2025 that may be closed by this pull request

Multipath metrics #3530

Open

n0bot bot added this to iroh Nov 17, 2025

github-project-automation bot moved this to 🏗 In progress in iroh Nov 17, 2025

dignifiedquire reviewed Nov 17, 2025

View reviewed changes

flub reviewed Nov 17, 2025

View reviewed changes

dignifiedquire reviewed Nov 17, 2025

View reviewed changes

iroh/src/magicsock/remote_map/remote_state/metrics.rs Outdated Show resolved Hide resolved

Frando force-pushed the Frando/mp-metrics branch from 112180e to 5bb726f Compare November 17, 2025 15:21

Frando changed the base branch from feat-multipath to Frando/mp-metrics-basics November 17, 2025 15:22

Frando changed the title ~~feat: add back metrics to multipath~~ feat: add back path congestion metris to multipath Nov 17, 2025

Frando changed the title ~~feat: add back path congestion metris to multipath~~ feat: add back path congestion metrics to multipath Nov 17, 2025

Frando changed the title ~~feat: add back path congestion metrics to multipath~~ feat(multipath): add back path congestion metrics Nov 17, 2025

feat: add back path congestion metrics

281e84b

Frando force-pushed the Frando/mp-metrics branch from 5bb726f to 281e84b Compare November 18, 2025 10:45

refactor: use if let not continue

66a61f3

feat(multipath): add back path congestion metrics #3669

Are you sure you want to change the base?

feat(multipath): add back path congestion metrics #3669

Conversation

Frando commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking Changes

Notes & open questions

Change checklist

Uh oh!

github-actions bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flub left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frando Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frando Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Frando commented Nov 17, 2025

Uh oh!

Frando commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Frando commented Nov 17, 2025 •

edited

Loading

github-actions bot commented Nov 17, 2025 •

edited

Loading

Frando Nov 17, 2025 •

edited

Loading

Frando Nov 17, 2025 •

edited

Loading

Frando commented Nov 17, 2025 •

edited

Loading