-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix: remove mdns_legacy #9048
fix: remove mdns_legacy #9048
Conversation
We've been running both implementations for a long, long time. It is time to remove legacy version and lower the number of LAN packets IPFS node produces.
797f60e
to
d3bcfa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's kill it. It's been a while.
Looking forward to removing it from libp2p as well.
core/node/libp2p/discovery.go
Outdated
@@ -49,12 +48,6 @@ func SetupDiscovery(useMdns bool, mdnsInterval int) func(helpers.MetricsCtx, fx. | |||
if mdnsInterval == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marten-seemann we have the implicit default documented here – is it the same in libp2p?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not configurable any more in the new mDNS implementation.
quick cc:
|
yes, I believe that's all rust-libp2p implements at this point |
Removed Replaced Ready for final review / merge – my hope is to ship this in 0.14. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix: remove mdns_legacy We've been running both implementations for a long, long time. It is time to remove legacy version and lower the number of LAN packets IPFS node produces. See ipfs/kubo#9048 (comment) for the Interval removal rational.
This PR removes the ancient
mdns_legacy
implementation.@marten-seemann @schomatis thoughts on removing old one? any concerns?