Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Netcheck https & icmp probes are scheduled too late for subsequent relay servers #2444

Closed
flub opened this issue Jul 2, 2024 · 0 comments · Fixed by #2445
Closed

Netcheck https & icmp probes are scheduled too late for subsequent relay servers #2444

flub opened this issue Jul 2, 2024 · 0 comments · Fixed by #2445
Assignees
@flub
Labels
bug Something isn't working c-iroh-net
Milestone
v0.20.0

Comments

@flub
Copy link
Contributor

flub commented Jul 2, 2024

When you have a RelayMap with multiple relay servers the ICMP and HTTPS probes for the 2nd or higher servers are scheduled too late:

https://github.com/n0-computer/iroh/blob/main/iroh-net/src/netcheck/reportgen/probes.rs#L237

For the 2nd, 3rd etc relay server this line results in probes being further out in the future than they need to be.
@flub flub added bug Something isn't working c-iroh-net labels Jul 2, 2024
@flub flub added this to the v0.20.0 milestone Jul 2, 2024
@flub flub self-assigned this Jul 2, 2024
@flub flub added this to iroh Jul 2, 2024
@flub flub mentioned this issue Jul 2, 2024
Merged
2 tasks
@ramfox ramfox moved this to 👀 In review in iroh Jul 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 3, 2024
@flub
fix(iroh-net): delays of non-stun probes for subsequent relays (#2445)
b34587f 
## Description

The idea of the probe plan is that first the STUN probes happen, if
those don't work we add the other probes.  However when there are
multiple relay servers we accidentally started the subsequent probes
for all but the first relay server too late.

This makes sure to globally record when the last STUN probe was sent
and re-uses this value for all the relay servers which are probed.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

Fixes #2444

See #2221 for flaky tests.

## Change checklist

- [x] Self-review.
- ~~[ ] Documentation updates if relevant.~~
- [x] Tests if relevant.
- ~~[ ] All breaking changes documented.~~
@flub flub closed this as completed in #2445 Jul 3, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in iroh Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@flub flub

Labels
bug Something isn't working c-iroh-net
Projects
iroh
Archived in project
Milestone
v0.20.0
Development

Successfully merging a pull request may close this issue.

fix(iroh-net): delays of non-stun probes for subsequent relays n0-computer/iroh
1 participant
@flub