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

Closing mdns agent #636

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Closing mdns agent #636

merged 1 commit into from
Nov 21, 2024

Conversation

ineiti
Copy link
Contributor

@ineiti ineiti commented Nov 20, 2024

I use the library with many open/close, and I get mdns storms where the webrtc library searches for other nodes over the network in a very aggressive manner.

This has been proposed by @r-byondlabs, and might very well be the problem: if I open/close many connections, and the mdns-agents are not cleaned up, there might be many requests.

Fixes #616

@ineiti ineiti mentioned this pull request Nov 20, 2024
Copy link
Member

@rainliu rainliu left a comment

Choose a reason for hiding this comment

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

Please fix the error

@ineiti
Copy link
Contributor Author

ineiti commented Nov 20, 2024

Oups - so sorry, this was a github-editor patch. I was hoping to have the time this evening to check the fix on my use-case, and wanted to give a heads-up to @r-byondlabs to look at it. But I didn't anticipate you being so fast!

Fixed the code. The tests pass on my machine.

@rainliu
Copy link
Member

rainliu commented Nov 20, 2024

Cargo fmt warning

I use the library with many open/close, and I get mdns storms where the webrtc library searches for other nodes over the network in a very aggressive manner.

This has been proposed by @r-byondlabs, and might very well be the problem: if I open/close many connections, and the mdns-agents are not cleaned up, there might be many requests.

Fixes webrtc-rs#616
@ineiti
Copy link
Contributor Author

ineiti commented Nov 21, 2024

Here you go. With my version of cargo-fmt, it also changes the formatting of two other files. Let me know if that's OK.

I tested it on my setup, and the mdns-storms seem to be effectively gone, which is great!

I also added devbox and a pre-commit hook here: #637 . I'm a recent devbox-convert and really think it improves the contributor experience a lot.

@ineiti ineiti requested a review from rainliu November 21, 2024 07:05
@rainliu rainliu merged commit ac61f20 into webrtc-rs:master Nov 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mdns storm
2 participants