protocols/mdns: Allow users to choose between async-io and tokio runtime

[gallegogt]

Problem

When use the Tokio library with mdns module, it is polling every time the UDP socket; causing hight CPU usage.

Solution

When using Tokio library, use the UppSocket provided by the Tokio library instead of the Async-io wrapper.

Breaking Change

After this fix exist two features async-io, tokio for this module, for the general module must be use the features mdns-async-io or mdns-tokio for async-io or tokio libraries

Links to any relevant issues

• chat and chat-tokio examples: high CPU consumption (Ubuntu 20.04) #2675
• mdns: don't perform IO on main task #2591

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[thomaseizinger]

Thanks!

I think the feature itself is worthwhile having but we need to implement it slightly different. Commands like cargo build --all-features should continue to work across the workspace.

[gallegogt]

Hi @thomaseizinge, thanks for the feedback, I will check it

[thomaseizinger]

Thanks!

I gave this a more in-depth read, please see the inline comments :)

[mxinden]

Related issue on if-watch used in libp2p-mdns: libp2p/if-watch#21

//CC @dignifiedquire

[mxinden]

Thanks @gallegogt for the thorough patch and thanks @thomaseizinger for the detailed reviews!

[mxinden]

@gallegogt let us know once this is ready for another review.

[gallegogt]

Hi @mxinden it's ready for review

[thomaseizinger]

Thanks!

This looks really good now, I've left a couple more comments :)

[gallegogt]

Hi @mxinden @thomaseizinger it's ready for review

[thomaseizinger]

Thank you for incorporating the feedback :)

I think this is close to being mergeable. I made a few comments where we don't seem to be doing quite the same as previously. Please comment if that is on purpose, otherwise I'd prefer to stick to the previous logic.

[mxinden]

This looks good to me. Deferring final review to @thomaseizinger.

Thanks @gallegogt and @thomaseizinger for the solid work here!

[thomaseizinger]

Thanks!

I've found too more misalignments with the previous version. Thanks for sticking to this :)

[thomaseizinger]

Thanks for implementing all the feedback!

This is ready now :)

[thomaseizinger]

The test_expired_tokio test is failing. Can you look into that @gallegogt?

[gallegogt]

The test_expired_tokio test is failing. Can you look into that @gallegogt?

I'll check it, but in my test environments work, I will try in other PC.

[thomaseizinger]

The test_expired_tokio test is failing. Can you look into that @gallegogt?

I'll check it, but in my test environments work, I will try in other PC.

It also fails on my local computer. I am running Manjaro Linux.

[gallegogt]

The test_expired_tokio test is failing. Can you look into that @gallegogt?

I'll check it, but in my test environments work, I will try in other PC.

It also fails on my local computer. I am running Manjaro Linux.

@thomaseizinger , @mxinden
My environment:

ARM

Description:    Ubuntu 18.04.6 LTS
Release:        18.04
Codename:       bionic

rustc 1.62.0 (a8314ef7d 2022-06-27)

[mxinden]

Failing on my machine as well:

➜  cargo test --all-features
    Finished test [unoptimized + debuginfo] target(s) in 0.17s
     Running unittests src/lib.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/libp2p_mdns-aeea3dee3d0f3ca4)

running 5 tests
test behaviour::iface::dns::tests::build_service_discovery_response_correct ... ok
test behaviour::iface::dns::tests::build_query_correct ... ok
test behaviour::iface::dns::tests::test_random_string ... ok
test behaviour::iface::query::tests::test_create_mdns_peer ... ok
test behaviour::iface::dns::tests::build_query_response_correct ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/use-async-std.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/use_async_std-77e86062dfdf653f)

running 3 tests
test test_discovery_async_std_ipv4 ... ok
test test_discovery_async_std_ipv6 ... ok
test test_expired_async_std ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.03s

     Running tests/use-tokio.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/use_tokio-278cda36b01406d1)

running 3 tests
test test_expired_tokio ... FAILED
test test_discovery_tokio_ipv4 ... ok
test test_discovery_tokio_ipv6 ... ok

failures:

---- test_expired_tokio stdout ----
thread 'test_expired_tokio' panicked at 'called `Result::unwrap()` on an `Err` value: Elapsed(())', protocols/mdns/tests/use-tokio.rs:55:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_expired_tokio

test result: FAILED. 2 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.15s

error: test failed, to rerun pass '--test use-tokio'

[gallegogt]

Failing on my machine as well:

➜  cargo test --all-features

    Finished test [unoptimized + debuginfo] target(s) in 0.17s

     Running unittests src/lib.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/libp2p_mdns-aeea3dee3d0f3ca4)



running 5 tests

test behaviour::iface::dns::tests::build_service_discovery_response_correct ... ok

test behaviour::iface::dns::tests::build_query_correct ... ok

test behaviour::iface::dns::tests::test_random_string ... ok

test behaviour::iface::query::tests::test_create_mdns_peer ... ok

test behaviour::iface::dns::tests::build_query_response_correct ... ok



test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s



     Running tests/use-async-std.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/use_async_std-77e86062dfdf653f)



running 3 tests

test test_discovery_async_std_ipv4 ... ok

test test_discovery_async_std_ipv6 ... ok

test test_expired_async_std ... ok



test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.03s



     Running tests/use-tokio.rs (/home/mxinden/code/github.com/libp2p/rust-libp2p/target/debug/deps/use_tokio-278cda36b01406d1)



running 3 tests

test test_expired_tokio ... FAILED

test test_discovery_tokio_ipv4 ... ok

test test_discovery_tokio_ipv6 ... ok



failures:



---- test_expired_tokio stdout ----

thread 'test_expired_tokio' panicked at 'called `Result::unwrap()` on an `Err` value: Elapsed(())', protocols/mdns/tests/use-tokio.rs:55:10

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace





failures:

    test_expired_tokio



test result: FAILED. 2 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.15s



error: test failed, to rerun pass '--test use-tokio'

Very rare situation, I guess tokio (explicitly non_blocking) takes longer to find the peers than AsynIO. If so, I increase the expiration time and query request a little

[mxinden]

Very rare situation, I guess tokio (explicitly non_blocking) takes longer to find the peers than AsynIO. If so, I increase the expiration time and query request a little

That would be surprising to me, given that the timeout is on the order of seconds, still something worth trying @gallegogt.

[gallegogt]

Hi @mxinden and @thomaseizinger can you check if the error persist please ?

[thomaseizinger]

Hi @mxinden and @thomaseizinger can you check if the error persist please ?

Yep, CI is still failing.

[thomaseizinger]

I can't reproduce it locally anymore :/

[gallegogt]

@thomaseizinger done with last comments.

[thomaseizinger]

@thomaseizinger done with last comments.

Thanks!

Just one note for future PRs, we tend to not force-push on this repository because PRs will be squash-merged in the end so regular merge commits of e.g. master are completely fine :)

[thomaseizinger]

I've kicked CI again and will merge once it passes!

[mxinden]

🙏 Thanks @gallegogt and @thomaseizinger!

[tomaka]

When use the Tokio library with mdns module, it is polling every time the UDP socket; causing hight CPU usage.

I've just discovered this PR. Did we ever find out what causes this? If not, it seems to me that this PR was done to bypass the issue, not fix it.

[rkuhn]

See #2591 for my investigation into this: the short story is that epoll immediately returns.