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

max_concurrent_outbound_dials does not restrict max concurrent outbound dials #6778

Closed
mattsse opened this issue Feb 24, 2024 · 4 comments · Fixed by #6860
Closed

max_concurrent_outbound_dials does not restrict max concurrent outbound dials #6778

mattsse opened this issue Feb 24, 2024 · 4 comments · Fixed by #6860
Assignees
Labels
A-networking Related to networking in general C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Feb 24, 2024

Describe the feature

The max_concurrent_outbound_dials

/// Maximum allowed concurrent outbound dials.
#[cfg_attr(feature = "serde", serde(default))]
max_concurrent_outbound_dials: usize,

only restricts how many new outbound dials we create at once but this does not respect how many are already in progress atm.

while self.connection_info.has_out_capacity() {

This should instead restrict how many outbound dials we allow at once and should slow down new connections if exceeded.

TODO

This can be solved by a new connection state and counter that is PendingOut

/// Counter for currently occupied slots for active outbound connections.
#[cfg_attr(feature = "serde", serde(skip))]
num_outbound: usize,

/// Connected via outgoing connection.
Out,

when we initiate an outgoing connection the peer's state is set to PendingOut and counter increased.

once the session is established or failed we need to update the state and counter for all possible cases:

fn decr_state(&mut self, state: PeerConnectionState) {

pub(crate) fn on_pending_session_gracefully_closed(&mut self, peer_id: &PeerId) {

and need a new function on_active_session_established that then shifts the peer's state from PendingOut to Out

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started A-networking Related to networking in general labels Feb 24, 2024
@abhijeetbhagat
Copy link
Contributor

@mattsse i can take this

@mattsse
Copy link
Collaborator Author

mattsse commented Feb 24, 2024

nice, lmk if anything's unclear

@abhijeetbhagat
Copy link
Contributor

@mattsse where should on_active_session_established be called from?

@mattsse
Copy link
Collaborator Author

mattsse commented Feb 26, 2024

from top level, we already have it for incoming, we also need it for outgoing:

if direction.is_incoming() {
this.swarm
.state_mut()
.peers_mut()
.on_incoming_session_established(peer_id, remote_addr);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants