Skip to content

Conversation

rklaehn
Copy link
Contributor

@rklaehn rklaehn commented Aug 14, 2025

Description

Fixes very slow (~1s) initial connection establishment

Also adds a test for initial connection delay.

Initial connection was very slow even with relays disabled and static discovery. There was a fixed delay.

Breaking Changes

None

Notes & open questions

Question: not sure how we can turn this into a proper test without making it super flaky, since it is a timing test. Maybe measure the ratio between first and second round and have some very conservative bounds?

Changed the test name and put the timings in the assertion. I think the test is good to go as is. The ratio is extremely conservative, so it should rarely flake out.

Note: I do 32 connections as an attempt to make the timings more reliable, but strictly speaking it would also work with 1 connection.

thread 'endpoint::tests::initial_connection_time' panicked at iroh/src/endpoint.rs:3269:9:
First round: 32.230450542s, second round 0.063144708s

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.
    • List all breaking changes in the above "Breaking Changes" section.
    • Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are:

@rklaehn rklaehn force-pushed the connection-speed-test branch from 422a364 to 3718ef1 Compare August 14, 2025 08:46
Copy link

github-actions bot commented Aug 14, 2025

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

Last updated: 2025-08-14T12:18:56Z

@rklaehn rklaehn force-pushed the connection-speed-test branch from 1162d92 to e5771b1 Compare August 14, 2025 09:00
Copy link

github-actions bot commented Aug 14, 2025

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: b0d9c28

@n0bot n0bot bot added this to iroh Aug 14, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Aug 14, 2025
@rklaehn rklaehn marked this pull request as ready for review August 14, 2025 09:43
@rklaehn rklaehn requested a review from matheus23 August 14, 2025 12:19
Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super happy with the code duplication, I think it's probably possible to factor out common code for constructing a Transmit from send_ping_actions and try_send_ping_actions, but it's fine, especially since this code won't live that long.

@rklaehn rklaehn changed the title test: add test to measure initial connection speed without relay. fix(iroh): fix very slow initial connection establishment Aug 15, 2025
@rklaehn rklaehn added this pull request to the merge queue Aug 15, 2025
Merged via the queue into main with commit 59d1432 Aug 15, 2025
31 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Aug 15, 2025
@matheus23 matheus23 deleted the connection-speed-test branch August 15, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants