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.

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

hack: Prioritise UDP disco messages #3237

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

flub
Copy link
Contributor

@flub flub commented Mar 17, 2025

Description

This should help with disco messages being dropped because the AsyncUdpSocket is hogging the socket's internal lock because it being asked to send so much data.

The unbounded queue is essentially bounded by the concurrency of the writers to the socket, so will stay small.

Breaking Changes

Notes & open questions

The code duplication, the unbounded queue (which is bounded really).... It's a quick hack right now. Though I'm not sure how nice it can be made right now.

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:

This should help with disco messages being dropped because the
AsyncUdpSocket is hogging the socket's internal lock because it being
asked to send so much data.

The unbounded queue is essentially bounded by the concurrency of the
writers to the socket, so will stay small.
@@ -1783,7 +1860,7 @@ impl Handle {
let _ = actor_tasks.spawn({
let msock = msock.clone();
async move {
while let Some((dst, dst_key, msg)) = udp_disco_receiver.recv().await {
while let Ok((dst, dst_key, msg)) = udp_disco_receiver.recv().await {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this task was previously the only place which would send these queued disco messages. Internally in the actual socket it is taking a lock which is contended with the same socket with the same internal lock that is called from try_send whcih is modified above to also read from this same udp_disco_receiver now.

Copy link

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

Last updated: 2025-03-17T18:28:02Z

Copy link

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: 1eb3ad8

@n0bot n0bot bot added this to iroh Mar 17, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

1 participant