Skip to content

fix(iroh): improve reusing of QAD reports#3512

Merged
dignifiedquire merged 7 commits intomainfrom
fix-net-report-qad-conns
Oct 14, 2025
Merged

fix(iroh): improve reusing of QAD reports#3512
dignifiedquire merged 7 commits intomainfrom
fix-net-report-qad-conns

Conversation

@dignifiedquire
Copy link
Copy Markdown
Contributor

Description

This splits v4 vs v6 QAD report reuse between runs, to reduce traffic and overall work

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 8, 2025

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

Last updated: 2025-10-14T14:11:09Z

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 8, 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: d307ca2

@n0bot n0bot bot added this to iroh Oct 8, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Oct 8, 2025
@dignifiedquire dignifiedquire force-pushed the fix-net-report-qad-conns branch 2 times, most recently from 0c4ada0 to 393ff7b Compare October 9, 2025 12:58
match (state.have_v4, state.have_v6) {
(true, true) => {
// Both IPv4 and IPv6 are expected to be available
if num_ipv4 > 1 && num_ipv6 > 1 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

>= perhaps? Or do you really want 2 each? same off-by-one question in the full report above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sigh, all of these should be >=....

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed..


Some(timeout)
// If we have at least one probe per relay, we are happy
if num_total >= num_relays {
Copy link
Copy Markdown
Contributor

@flub flub Oct 9, 2025

Choose a reason for hiding this comment

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

does this case need to consider state.have_v4 and state.have_v6? E.g. if you have both and above doesn't yet return because its still waiting on ipv4 then this will return and no longer require ipv4?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this needs to only look at https probes, you are right

@dignifiedquire dignifiedquire added this to the v0.94 milestone Oct 9, 2025
@dignifiedquire dignifiedquire self-assigned this Oct 9, 2025
@dignifiedquire dignifiedquire force-pushed the fix-net-report-qad-conns branch 2 times, most recently from 8ee3389 to 4debb5f Compare October 14, 2025 08:55
@dignifiedquire dignifiedquire force-pushed the fix-net-report-qad-conns branch from 4debb5f to 0c45244 Compare October 14, 2025 10:19
@dignifiedquire dignifiedquire added this pull request to the merge queue Oct 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2025
@dignifiedquire dignifiedquire force-pushed the fix-net-report-qad-conns branch from 7cd0f47 to 9079c10 Compare October 14, 2025 13:42
@dignifiedquire dignifiedquire added this pull request to the merge queue Oct 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2025
@dignifiedquire dignifiedquire added this pull request to the merge queue Oct 14, 2025
Merged via the queue into main with commit b2a55bf Oct 14, 2025
77 of 79 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Oct 14, 2025
@dignifiedquire dignifiedquire deleted the fix-net-report-qad-conns branch October 14, 2025 14:56
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.

2 participants