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
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ libp2p-rendezvous = { version = "0.13.0", path = "protocols/rendezvous" }
libp2p-upnp = { version = "0.1.0", path = "protocols/upnp" }
libp2p-request-response = { version = "0.25.1", path = "protocols/request-response" }
libp2p-server = { version = "0.12.3", path = "misc/server" }
libp2p-swarm = { version = "0.43.4", path = "swarm" }
libp2p-swarm = { version = "0.44.0", path = "swarm" }
libp2p-swarm-derive = { version = "0.33.0", path = "swarm-derive" }
libp2p-swarm-test = { version = "0.2.0", path = "swarm-test" }
libp2p-tcp = { version = "0.40.0", path = "transports/tcp" }
Expand Down
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 @@ -2018,7 +2018,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
7 changes: 6 additions & 1 deletion swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
## 0.43.4
## 0.44.0 - unreleased

- Add `PeerCondition` variant `DisconnectedAndNotDialing`, combining pre-existing conditions. This is the new default.
thomaseizinger marked this conversation as resolved.
Show resolved Hide resolved
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].
thomaseizinger marked this conversation as resolved.
Show resolved Hide resolved

- Implement `Debug` for event structs.
See [PR 4426].
Expand All @@ -9,6 +13,7 @@
- Introduce `SwarmBuilder::idle_conncetion_timeout` and deprecate `keep_alive::Behaviour` as a result.
See [PR 4161].

[PR 4225]: https://github.com/libp2p/rust-libp2p/pull/4225
thomaseizinger marked this conversation as resolved.
Show resolved Hide resolved
[PR 4426]: https://github.com/libp2p/rust-libp2p/pull/4426
[PR 4409]: https://github.com/libp2p/rust-libp2p/pull/4409
[PR 4161]: https://github.com/libp2p/rust-libp2p/pull/4161
Expand Down
2 changes: 1 addition & 1 deletion swarm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-swarm"
edition = "2021"
rust-version = { workspace = true }
description = "The libp2p swarm"
version = "0.43.4"
version = "0.44.0"
authors = ["Parity Technologies <admin@parity.io>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
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 @@ -422,11 +422,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 @@ -1595,6 +1597,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