feat(kad): Limit number of active outbound streams

[nazar-pc]

Description

Limit number of active outbound streams to not exceed configured number of streams.

Resolves #3236.

Notes

The approach taken here restricts number of active outbound substreams and all extra are put into the queue.
Since some outbounds streams might not actually exist in the handler temporarily if request for outbound stream was issued, a separate variable is used to keep track of those as well.

As the result we maintain a bounded number of outbound streams that matches the limit for inbound streams on the other side.

Links to any relevant issues

#3235
#3236

Open Questions

Not entirely sure how to test higher-level behavior here as I'm new to the codebase, though changes are somewhat straightforward.
Also there is no support for cancelling of outbound streams, though it didn't seem to be possible before either, so reasonable timeouts before retries are your friends.

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

[melekes]

👍

[melekes]

lgtm

[mxinden]

Thanks for looking into this @nazar-pc!

[mxinden]

Note that in my opinion we should get rid of the manually written state machines long-term in libp2p-kad handler.rs. See #3130. Though that should be a separate pull request.

[nazar-pc]

Simplified implementation a bit based on review comments

[thomaseizinger]

LGTM

A few minor suggestions but not blocking!

[thomaseizinger]

Thanks, this is good to merge from my end. We will have to wait until the interoperability tests are fixed. See libp2p/test-plans#99 (comment).

[mxinden]

Wonderful. This looks good to me. Thanks for the work. Thanks for the follow-ups.

Next step would be to propagate backpressure to the NetworkBehaviour implementation. Let me know in case you are interested in implementing that. I would expect this patch alone to suffice for the issues (#3236) you are facing.

Approvals have been dismissed because the PR was updated after the send-it label was applied.

[mxinden]

Once #3331 is merged we can proceed here.

[nazar-pc]

On our small testnet results are very promising so far

[thomaseizinger]

Let's go!

[mxinden]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed

[mxinden]

Seems like Mergify is stuck on the initial failure on the interoperability tests. They are green on the second run. Will merge master in the hope for mergify to forget the initial failure.

Approvals have been dismissed because the PR was updated after the send-it label was applied.