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

feat: introduce InflightProtocolDataQueue #4834

Closed
wants to merge 6 commits into from

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Nov 10, 2023

Description

This PR prototypes what I termed an InflightProtocolDataQueue. It is essentially a replacement for ConnectionHandler::OutboundOpenInfo, just slightly more generic. In essence, it allows users to submit a request to the queue, together with a piece of data and later submit responses.

Internally, the responses are matched with a pending piece of data and later returned. Assuming the functions are called in the right order (which is an invariant that libp2p-swarm should uphold), this frees users from having to worry about tracking state separately.

Resolves: #4510.
Related: #3268.

Notes & open questions

Opening this PR for early feedback. In a follow-up PR, I'd then deprecate InboundOpenInfo and OutboundOpenInfo and point users towards the new libp2p-protocol-utils crate and the InflightProtocolDataQueue.

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
Copy link
Contributor Author

@mxinden Ready for review. Only has draft status because I want to adapt more protocols if we agree on the design.

@thomaseizinger thomaseizinger added the internal-change Pull requests that make internal changes to crates and thus don't need to include a changelog entry. label Nov 14, 2023
@thomaseizinger
Copy link
Contributor Author

Adding internal-change for refactoring of libp2p-relay.

@thomaseizinger thomaseizinger marked this pull request as ready for review November 14, 2023 03:15
@thomaseizinger
Copy link
Contributor Author

cc @dgarus You might enjoy this change :)

Copy link

mergify bot commented Nov 15, 2023

This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏

@thomaseizinger
Copy link
Contributor Author

@mxinden The last commit experiments with a oneshot based design for libp2p-relay. I am actually liking it a lot! It works well in relay because the protocol doesn't need to be suspended there.

Comment on lines +182 to +191
async move {
let stream = receiver
.await
.map_err(|_| io::Error::from(io::ErrorKind::BrokenPipe))?
.map_err(into_reserve_error)?;

let reservation = outbound_hop::make_reservation(stream).await?;

Ok(reservation)
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an added benefit, the timeout of FuturesTupleSet now also applies to the entire stream opening.

@thomaseizinger
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-change Pull requests that make internal changes to crates and thus don't need to include a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

swarm: Design utility type for tracking pending stream openings
1 participant