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.

Sign up for GitHub

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

Change DSN bootstrapping. #1690

Merged
merged 10 commits into from
Jul 26, 2023
Merged

Change DSN bootstrapping. #1690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3c9fc2c
Introduce Kademlia bootstrap operation.
shamil-gadelshin Jul 25, 2023
e26c6d3
Remove bootstrap-nodes from redialing.
shamil-gadelshin Jul 25, 2023
8f53b21
networking: Refactor bootstrapping.
shamil-gadelshin Jul 26, 2023
5b29807
networking: Refactor networking parameters manager.
shamil-gadelshin Jul 26, 2023
64fecdf
networking: Change bootstrapping algorithm.
shamil-gadelshin Jul 26, 2023
b7de051
networking: Ensure a single bootstrapping for node_runner.run().
shamil-gadelshin Jul 26, 2023
76e464a
Simplify bootstrapping process
nazar-pc Jul 26, 2023
e52d6fd
Merge pull request #1699 from subspace/change_bootstrapping-simplific…
shamil-gadelshin Jul 26, 2023
589017c
networking: Refactor node_runner.
shamil-gadelshin Jul 26, 2023
876aaf9
Merge branch 'main' into change_bootstrapping
shamil-gadelshin Jul 26, 2023
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
16 changes: 0 additions & 16 deletions crates/subspace-farmer/src/bin/subspace-farmer/commands/farm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::commands::shared::print_disk_farm_info;
use crate::utils::{get_required_plot_space_with_overhead, shutdown_signal};
use crate::{DiskFarm, FarmingArgs};
use anyhow::{anyhow, Context, Result};
use futures::future::pending;
use futures::stream::FuturesUnordered;
use futures::{FutureExt, StreamExt};
use lru::LruCache;
Expand Down Expand Up @@ -352,22 +351,7 @@ where
)?;
let mut networking_fut = Box::pin(networking_fut).fuse();

let bootstrap_fut = Box::pin({
let node = node.clone();

async move {
if let Err(err) = node.bootstrap().await {
warn!(?err, "DSN bootstrap failed.");
}

pending::<()>().await;
}
});

futures::select!(
// Network bootstrapping future
_ = bootstrap_fut.fuse() => {},

// Signal future
_ = signal.fuse() => {},

Expand Down
24 changes: 2 additions & 22 deletions crates/subspace-networking/src/bin/subspace-bootstrap-node/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use anyhow::anyhow;
use bytesize::ByteSize;
use clap::{Parser, ValueHint};
use either::Either;
use futures::future::pending;
use futures::FutureExt;
use libp2p::identity::ed25519::Keypair;
use libp2p::{identity, Multiaddr, PeerId};
use serde::{Deserialize, Serialize};
Expand All @@ -20,7 +18,7 @@ use subspace_networking::{
peer_id, Config, NetworkingParametersManager, ParityDbProviderStorage, PeerInfoProvider,
VoidProviderStorage,
};
use tracing::{debug, info, warn, Level};
use tracing::{debug, info, Level};
use tracing_subscriber::fmt::Subscriber;
use tracing_subscriber::util::SubscriberInitExt;
use tracing_subscriber::EnvFilter;
Expand Down Expand Up @@ -205,25 +203,7 @@ async fn main() -> anyhow::Result<()> {
.detach();

info!("Subspace Bootstrap Node started");
let bootstrap_fut = Box::pin({
let node = node.clone();

async move {
if let Err(err) = node.bootstrap().await {
warn!(?err, "DSN bootstrap failed.");
}

pending::<()>().await;
}
});

futures::select!(
// Network bootstrapping future
_ = bootstrap_fut.fuse() => {},

// Networking runner
_ = node_runner.run().fuse() => {},
);
node_runner.run().await;
}
Command::GenerateKeypair { json } => {
let output = KeypairOutput::new(Keypair::generate());
Expand Down
36 changes: 36 additions & 0 deletions crates/subspace-networking/src/node_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ where

/// Drives the main networking future forward.
pub async fn run(&mut self) {
self.bootstrap().await;

loop {
futures::select! {
_ = &mut self.random_query_timeout => {
Expand Down Expand Up @@ -237,6 +239,40 @@ where
}
}

/// Bootstraps Kademlia network
pub async fn bootstrap(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

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

This will work as expected most of the time, but not all the time.

NodeRunner::run() can be called, driven for some time and then dropped. When it it called again, it could be that you're triggering second, third and so on bootstrapping process.

Command::Bootstrap should be sent once in constructor with receiver stored in NodeRunner. Then here you need to check if it is still present and only run this function if it is.

Just remember that any async function can be interrupted at any time.

Also I noticed handle_command is async, which is not desired and not necessary if you change remove_all_known_peer_addresses to not be async anymore. Otherwise you have even more edge cases to potentially deal with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether we have this case in practice but this feature could be achieved using just a bool.

Copy link
Member

Choose a reason for hiding this comment

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

No, it is not. There is absolutely no guarantee that bootstrapping has finished last time you called it. There is a guarantee that it has started bootstrapping process, but not more than that. It will not prevent other commands from being processed in the meantime.

let (result_sender, mut result_receiver) = mpsc::unbounded();

debug!("Bootstrap started.");

self.handle_command(Command::Bootstrap { result_sender })
.await;

let mut bootstrap_step = 0;
loop {
futures::select! {
swarm_event = self.swarm.next() => {
if let Some(swarm_event) = swarm_event {
self.register_event_metrics(&swarm_event);
self.handle_swarm_event(swarm_event).await;
} else {
break;
}
},
result = result_receiver.next() => {
if result.is_some() {
debug!(%bootstrap_step, "Kademlia bootstrapping...");
bootstrap_step += 1;
} else {
break;
}
}
}
}

debug!("Bootstrap finished.");
}

/// Handles periodical tasks.
async fn handle_periodical_tasks(&mut self) {
// Log current connections.
Expand Down
18 changes: 1 addition & 17 deletions crates/subspace-service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ use subspace_runtime_primitives::opaque::Block;
use subspace_runtime_primitives::{AccountId, Balance, Hash, Index as Nonce};
use subspace_transaction_pool::bundle_validator::BundleValidator;
use subspace_transaction_pool::{FullPool, PreValidateTransaction};
use tracing::{debug, error, info, warn, Instrument};
use tracing::{debug, error, info, Instrument};

/// Error type for Subspace service.
#[derive(thiserror::Error, Debug)]
Expand Down Expand Up @@ -652,22 +652,6 @@ where
),
);

task_manager.spawn_handle().spawn(
"node-runner",
Some("subspace-networking-bootstrapping"),
Box::pin(
{
let node = node.clone();
async move {
if let Err(err) = node.bootstrap().await {
warn!(?err, "DSN bootstrap failed.");
}
}
}
.in_current_span(),
),
);

(node, dsn_config.bootstrap_nodes, Some(piece_cache))
}
};
Expand Down