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(swarm): allow both Disconnected and NotDialing condition combined #4225

Merged
merged 13 commits into from
Oct 20, 2023
Merged
12 changes: 10 additions & 2 deletions misc/connection-limits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,8 @@ impl NetworkBehaviour for Behaviour {
mod tests {
use super::*;
use libp2p_swarm::{
behaviour::toggle::Toggle, dial_opts::DialOpts, DialError, ListenError, Swarm, SwarmEvent,
behaviour::toggle::Toggle, dial_opts::DialOpts, dial_opts::PeerCondition, DialError,
ListenError, Swarm, SwarmEvent,
};
use libp2p_swarm_test::SwarmExt;
use quickcheck::*;
Expand All @@ -401,14 +402,21 @@ mod tests {
network
.dial(
DialOpts::peer_id(target)
// Dial always, even if already dialing or connected.
.condition(PeerCondition::Always)
.addresses(vec![addr.clone()])
.build(),
)
.expect("Unexpected connection limit.");
}

match network
.dial(DialOpts::peer_id(target).addresses(vec![addr]).build())
.dial(
DialOpts::peer_id(target)
.condition(PeerCondition::Always)
.addresses(vec![addr])
.build(),
)
.expect_err("Unexpected dialing success.")
{
DialError::Denied { cause } => {
Expand Down
9 changes: 8 additions & 1 deletion misc/memory-connection-limits/tests/max_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ fn max_bytes() {
network
.dial(
DialOpts::peer_id(target)
// Always dial, even if connected or already dialing.
.condition(libp2p_swarm::dial_opts::PeerCondition::Always)
.addresses(vec![addr.clone()])
.build(),
)
Expand All @@ -70,7 +72,12 @@ fn max_bytes() {
std::thread::sleep(Duration::from_millis(100)); // Memory stats are only updated every 100ms internally, ensure they are up-to-date when we try to exceed it.

match network
.dial(DialOpts::peer_id(target).addresses(vec![addr]).build())
.dial(
DialOpts::peer_id(target)
.condition(libp2p_swarm::dial_opts::PeerCondition::Always)
.addresses(vec![addr])
.build(),
)
.expect_err("Unexpected dialing success.")
{
DialError::Denied { cause } => {
Expand Down
14 changes: 12 additions & 2 deletions misc/memory-connection-limits/tests/max_percentage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ use std::time::Duration;
use sysinfo::{RefreshKind, SystemExt};
use util::*;

use libp2p_swarm::{dial_opts::DialOpts, DialError, Swarm};
use libp2p_swarm::{
dial_opts::{DialOpts, PeerCondition},
DialError, Swarm,
};
use libp2p_swarm_test::SwarmExt;

#[test]
Expand Down Expand Up @@ -63,6 +66,8 @@ fn max_percentage() {
network
.dial(
DialOpts::peer_id(target)
// Always dial, even if already dialing or connected.
.condition(PeerCondition::Always)
.addresses(vec![addr.clone()])
.build(),
)
Expand All @@ -72,7 +77,12 @@ fn max_percentage() {
std::thread::sleep(Duration::from_millis(100)); // Memory stats are only updated every 100ms internally, ensure they are up-to-date when we try to exceed it.

match network
.dial(DialOpts::peer_id(target).addresses(vec![addr]).build())
.dial(
DialOpts::peer_id(target)
.condition(PeerCondition::Always)
.addresses(vec![addr])
.build(),
)
.expect_err("Unexpected dialing success.")
{
DialError::Denied { cause } => {
Expand Down
4 changes: 3 additions & 1 deletion protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1995,7 +1995,9 @@ where
}
}
DialError::DialPeerConditionFalse(
dial_opts::PeerCondition::Disconnected | dial_opts::PeerCondition::NotDialing,
dial_opts::PeerCondition::Disconnected
| dial_opts::PeerCondition::NotDialing
| dial_opts::PeerCondition::DisconnectedAndNotDialing,
) => {
// We might (still) be connected, or about to be connected, thus do not report the
// failure to the queries.
Expand Down
4 changes: 4 additions & 0 deletions swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## 0.44.0 - unreleased

- Add `PeerCondition::DisconnectedAndNotDialing` variant, combining pre-existing conditions.
This is the new default.
A new dialing attempt is iniated _only if_ the peer is both considered disconnected and there is currently no ongoing dialing attempt.
See [PR 4225](https://github.com/libp2p/rust-libp2p/pull/4225).
- Remove deprecated `keep_alive_timeout` in `OneShotHandlerConfig`.
See [PR 4677](https://github.com/libp2p/rust-libp2p/pull/4677).

Expand Down
10 changes: 7 additions & 3 deletions swarm/src/dial_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,18 @@ impl WithoutPeerIdWithAddress {
#[derive(Debug, Copy, Clone, Default)]
pub enum PeerCondition {
/// A new dialing attempt is initiated _only if_ the peer is currently
/// considered disconnected, i.e. there is no established connection
/// and no ongoing dialing attempt.
#[default]
/// considered disconnected, i.e. there is no established connection.
Disconnected,
/// A new dialing attempt is initiated _only if_ there is currently
/// no ongoing dialing attempt, i.e. the peer is either considered
/// disconnected or connected but without an ongoing dialing attempt.
NotDialing,
/// A combination of [`Disconnected`](PeerCondition::Disconnected) and
/// [`NotDialing`](PeerCondition::NotDialing). A new dialing attempt is
/// iniated _only if_ the peer is both considered disconnected and there
/// is currently no ongoing dialing attempt.
#[default]
DisconnectedAndNotDialing,
/// A new dialing attempt is always initiated, only subject to the
/// configured connection limits.
Always,
Expand Down
7 changes: 5 additions & 2 deletions swarm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,11 +444,13 @@ where
let connection_id = dial_opts.connection_id();

let should_dial = match (condition, peer_id) {
(_, None) => true,
(PeerCondition::Always, _) => true,
(PeerCondition::Disconnected, None) => true,
(PeerCondition::NotDialing, None) => true,
(PeerCondition::Disconnected, Some(peer_id)) => !self.pool.is_connected(peer_id),
(PeerCondition::NotDialing, Some(peer_id)) => !self.pool.is_dialing(peer_id),
(PeerCondition::DisconnectedAndNotDialing, Some(peer_id)) => {
!self.pool.is_dialing(peer_id) && !self.pool.is_connected(peer_id)
}
};

if !should_dial {
Expand Down Expand Up @@ -1742,6 +1744,7 @@ impl fmt::Display for DialError {
),
DialError::DialPeerConditionFalse(PeerCondition::Disconnected) => write!(f, "Dial error: dial condition was configured to only happen when disconnected (`PeerCondition::Disconnected`), but node is already connected, thus cancelling new dial."),
DialError::DialPeerConditionFalse(PeerCondition::NotDialing) => write!(f, "Dial error: dial condition was configured to only happen if there is currently no ongoing dialing attempt (`PeerCondition::NotDialing`), but a dial is in progress, thus cancelling new dial."),
DialError::DialPeerConditionFalse(PeerCondition::DisconnectedAndNotDialing) => write!(f, "Dial error: dial condition was configured to only happen when both disconnected (`PeerCondition::Disconnected`) and there is currently no ongoing dialing attempt (`PeerCondition::NotDialing`), but node is already connected or dial is in progress, thus cancelling new dial."),
DialError::DialPeerConditionFalse(PeerCondition::Always) => unreachable!("Dial peer condition is by definition true."),
DialError::Aborted => write!(
f,
Expand Down