-
Notifications
You must be signed in to change notification settings - Fork 36
transport: Limit dial concurrency and bound total dialing time #538
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
12f000b
tcp: Support address dialing with a concurrency cap of 8
lexnv b73a866
tcp: Bound total dialing time
lexnv 1fd872c
websocket: Apply changes to the websocket transport
lexnv 7e92614
config: Add max parallel dials to config and default to litep2p config
lexnv 4c5e63c
transport: Lift the limit from transport builder config
lexnv 9789aad
Apply clippy
lexnv fa51c6e
Apply fmt
lexnv a1da5f0
config: Dont use 0 parallel addresses
lexnv 0f823ca
transport: Hide the max parallel from configs
lexnv 735998b
transport: Log properly the deadline
lexnv f798961
transport: Document the new fields better
lexnv 8e423ac
Merge branch 'master' into lexnv/impr-dialing
lexnv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ use crate::{ | |
|
|
||
| use futures::{ | ||
| future::BoxFuture, | ||
| stream::{AbortHandle, FuturesUnordered, Stream, StreamExt}, | ||
| stream::{AbortHandle, Stream, StreamExt}, | ||
| TryFutureExt, | ||
| }; | ||
| use hickory_resolver::TokioResolver; | ||
|
|
@@ -434,74 +434,108 @@ impl Transport for TcpTransport { | |
| addresses: Vec<Multiaddr>, | ||
| ) -> crate::Result<()> { | ||
| let num_addresses = addresses.len(); | ||
| let mut futures: FuturesUnordered<_> = addresses | ||
| .into_iter() | ||
| .map(|address| { | ||
| let yamux_config = self.config.yamux_config.clone(); | ||
| let max_read_ahead_factor = self.config.noise_read_ahead_frame_count; | ||
| let max_write_buffer_size = self.config.noise_write_buffer_size; | ||
| let connection_open_timeout = self.config.connection_open_timeout; | ||
| let substream_open_timeout = self.config.substream_open_timeout; | ||
| let dial_addresses = self.dial_addresses.clone(); | ||
| let keypair = self.context.keypair.clone(); | ||
| let nodelay = self.config.nodelay; | ||
| let resolver = self.resolver.clone(); | ||
|
|
||
| async move { | ||
| let (address, stream) = TcpTransport::dial_peer( | ||
| address.clone(), | ||
| dial_addresses, | ||
| connection_open_timeout, | ||
| nodelay, | ||
| resolver, | ||
| ) | ||
| .await | ||
| .map_err(|error| (address, error))?; | ||
|
|
||
| let open_address = address.clone(); | ||
| let (socket_address, peer) = TcpAddress::multiaddr_to_socket_address(&address) | ||
| .map_err(|error| (address, error.into()))?; | ||
| let yamux_config = self.config.yamux_config.clone(); | ||
| let max_read_ahead_factor = self.config.noise_read_ahead_frame_count; | ||
| let max_write_buffer_size = self.config.noise_write_buffer_size; | ||
| let connection_open_timeout = self.config.connection_open_timeout; | ||
| let substream_open_timeout = self.config.substream_open_timeout; | ||
| let max_parallel_dials = self.config.max_parallel_dials; | ||
| let dial_addresses = self.dial_addresses.clone(); | ||
| let keypair = self.context.keypair.clone(); | ||
| let nodelay = self.config.nodelay; | ||
| let resolver = self.resolver.clone(); | ||
|
|
||
| TcpConnection::open_connection( | ||
| connection_id, | ||
| keypair, | ||
| stream, | ||
| socket_address, | ||
| peer, | ||
| yamux_config, | ||
| max_read_ahead_factor, | ||
| max_write_buffer_size, | ||
| connection_open_timeout, | ||
| substream_open_timeout, | ||
| ) | ||
| .await | ||
| .map_err(|error| (open_address, error.into())) | ||
| } | ||
| }) | ||
| .collect(); | ||
| let futures = futures::stream::iter(addresses.into_iter().map(move |address| { | ||
| let yamux_config = yamux_config.clone(); | ||
| let dial_addresses = dial_addresses.clone(); | ||
| let keypair = keypair.clone(); | ||
| let resolver = resolver.clone(); | ||
|
|
||
| async move { | ||
| let (address, stream) = TcpTransport::dial_peer( | ||
| address.clone(), | ||
| dial_addresses, | ||
| connection_open_timeout, | ||
| nodelay, | ||
| resolver, | ||
| ) | ||
| .await | ||
| .map_err(|error| (address, error))?; | ||
|
|
||
| let open_address = address.clone(); | ||
| let (socket_address, peer) = TcpAddress::multiaddr_to_socket_address(&address) | ||
| .map_err(|error| (address, error.into()))?; | ||
|
|
||
| TcpConnection::open_connection( | ||
| connection_id, | ||
| keypair, | ||
| stream, | ||
| socket_address, | ||
| peer, | ||
| yamux_config, | ||
| max_read_ahead_factor, | ||
| max_write_buffer_size, | ||
| connection_open_timeout, | ||
| substream_open_timeout, | ||
| ) | ||
| .await | ||
| .map_err(|error| (open_address, error.into())) | ||
| } | ||
| })) | ||
| .buffer_unordered(max_parallel_dials); | ||
|
|
||
| // Future that will resolve to the first successful connection. | ||
| let future = async move { | ||
| let mut errors = Vec::with_capacity(num_addresses); | ||
| while let Some(result) = futures.next().await { | ||
| match result { | ||
| Ok(negotiated) => return RawConnectionResult::Connected { negotiated, errors }, | ||
| Err(error) => { | ||
| // Deadline for the overall dial attempt, including all retries. This is to prevent | ||
| // retry attempts from indefinitely delaying the dial result. | ||
| let deadline = tokio::time::sleep(2 * connection_open_timeout); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make this a constant? |
||
|
|
||
| tokio::pin!(deadline); | ||
| tokio::pin!(futures); | ||
|
|
||
| loop { | ||
| tokio::select! { | ||
| result = futures.next() => { | ||
| match result { | ||
| Some(Ok(negotiated)) => { | ||
| return RawConnectionResult::Connected { | ||
| negotiated, | ||
| errors, | ||
| }; | ||
| } | ||
| Some(Err(error)) => { | ||
| tracing::debug!( | ||
| target: LOG_TARGET, | ||
| ?connection_id, | ||
| ?error, | ||
| "failed to open connection", | ||
| ); | ||
| errors.push(error); | ||
| } | ||
| None => { | ||
| return RawConnectionResult::Failed { | ||
| connection_id, | ||
| errors, | ||
| }; | ||
| } | ||
| } | ||
| } | ||
| _ = &mut deadline => { | ||
| tracing::debug!( | ||
| target: LOG_TARGET, | ||
| ?connection_id, | ||
| ?error, | ||
| "failed to open connection", | ||
| ?connection_open_timeout, | ||
| "overall dial timeout exceeded", | ||
| ); | ||
| errors.push(error) | ||
| return RawConnectionResult::Failed { | ||
| connection_id, | ||
| errors, | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| RawConnectionResult::Failed { | ||
| connection_id, | ||
| errors, | ||
| } | ||
| }; | ||
|
|
||
| let (fut, handle) = futures::future::abortable(future); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm do we not limit the max adresses already here?