From bfc42cdd8e79302b8ca38df95249aff58e7dba7c Mon Sep 17 00:00:00 2001 From: Christien Rioux Date: Wed, 6 Sep 2023 13:20:36 -0400 Subject: [PATCH] clean up ui pause routing table tasks when making routing domain changes --- veilid-core/src/network_manager/native/mod.rs | 16 +++- .../native/network_class_discovery.rs | 7 +- .../routing_table/routing_domain_editor.rs | 93 +++++++++++-------- .../src/routing_table/routing_table_inner.rs | 3 + veilid-core/src/routing_table/tasks/mod.rs | 33 ++++++- .../src/routing_table/tasks/ping_validator.rs | 18 ++-- .../tasks/private_route_management.rs | 5 - .../routing_table/tasks/relay_management.rs | 16 ++-- veilid-core/src/veilid_api/debug.rs | 3 +- 9 files changed, 122 insertions(+), 72 deletions(-) diff --git a/veilid-core/src/network_manager/native/mod.rs b/veilid-core/src/network_manager/native/mod.rs index b3bbe8d8..02335a6a 100644 --- a/veilid-core/src/network_manager/native/mod.rs +++ b/veilid-core/src/network_manager/native/mod.rs @@ -647,7 +647,11 @@ impl Network { let peer_socket_addr = dial_info.to_socket_addr(); let ph = match self.find_best_udp_protocol_handler(&peer_socket_addr, &None) { Some(ph) => ph, - None => bail!("no appropriate UDP protocol handler for dial_info"), + None => { + return Ok(NetworkResult::no_connection_other( + "no appropriate UDP protocol handler for dial_info", + )); + } }; connection_descriptor = network_result_try!(ph .send_message(data, peer_socket_addr) @@ -876,8 +880,8 @@ impl Network { } // commit routing table edits - editor_public_internet.commit(); - editor_local_network.commit(); + editor_public_internet.commit(true).await; + editor_local_network.commit(true).await; info!("network started"); self.inner.lock().network_started = true; @@ -932,14 +936,16 @@ impl Network { .clear_dial_info_details(None, None) .set_network_class(None) .clear_relay_node() - .commit(); + .commit(true) + .await; routing_table .edit_routing_domain(RoutingDomain::LocalNetwork) .clear_dial_info_details(None, None) .set_network_class(None) .clear_relay_node() - .commit(); + .commit(true) + .await; // Reset state including network class *self.inner.lock() = Self::new_inner(); diff --git a/veilid-core/src/network_manager/native/network_class_discovery.rs b/veilid-core/src/network_manager/native/network_class_discovery.rs index 60036fd8..62d0ba95 100644 --- a/veilid-core/src/network_manager/native/network_class_discovery.rs +++ b/veilid-core/src/network_manager/native/network_class_discovery.rs @@ -35,7 +35,7 @@ impl Network { editor.clear_dial_info_details(None, None); editor.set_network_class(Some(NetworkClass::OutboundOnly)); - editor.commit(); + editor.commit(true).await; } } DetectedDialInfo::Detected(did) => { @@ -85,7 +85,7 @@ impl Network { } editor.set_network_class(Some(NetworkClass::InboundCapable)); - editor.commit(); + editor.commit(true).await; } } } @@ -131,8 +131,7 @@ impl Network { editor.clear_dial_info_details(None, None); editor.set_network_class(None); editor.clear_relay_node(); - editor.commit(); - log_net!(debug "PublicInternet network class cleared"); + editor.commit(true).await; // Process all protocol and address combinations let mut unord = FuturesUnordered::new(); diff --git a/veilid-core/src/routing_table/routing_domain_editor.rs b/veilid-core/src/routing_table/routing_domain_editor.rs index 4f9c6ac7..1b22657e 100644 --- a/veilid-core/src/routing_table/routing_domain_editor.rs +++ b/veilid-core/src/routing_table/routing_domain_editor.rs @@ -122,16 +122,20 @@ impl RoutingDomainEditor { } #[instrument(level = "debug", skip(self))] - pub fn commit(&mut self) { + pub async fn commit(&mut self, pause_tasks: bool) { // No locking if we have nothing to do if self.changes.is_empty() { return; } + // Briefly pause routing table ticker while changes are made + if pause_tasks { + self.routing_table.pause_tasks(true).await; + } + + // Apply changes let mut changed = false; { - let node_ids = self.routing_table.node_ids(); - let mut inner = self.routing_table.inner.write(); inner.with_routing_domain_mut(self.routing_domain, |detail| { for change in self.changes.drain(..) { @@ -140,22 +144,32 @@ impl RoutingDomainEditor { address_type, protocol_type, } => { - debug!( - "[{:?}] cleared dial info details: at={:?} pt={:?}", - self.routing_domain, address_type, protocol_type - ); + if address_type.is_some() || protocol_type.is_some() { + info!( + "[{:?}] cleared dial info: {}:{}", + self.routing_domain, + address_type + .map(|at| format!("{:?}", at)) + .unwrap_or("---".to_string()), + protocol_type + .map(|at| format!("{:?}", at)) + .unwrap_or("---".to_string()), + ); + } else { + info!("[{:?}] cleared all dial info", self.routing_domain); + } detail .common_mut() .clear_dial_info_details(address_type, protocol_type); changed = true; } RoutingDomainChange::ClearRelayNode => { - debug!("[{:?}] cleared relay node", self.routing_domain); + info!("[{:?}] cleared relay node", self.routing_domain); detail.common_mut().set_relay_node(None); changed = true; } RoutingDomainChange::SetRelayNode { relay_node } => { - debug!("[{:?}] set relay node: {}", self.routing_domain, relay_node); + info!("[{:?}] set relay node: {}", self.routing_domain, relay_node); detail.common_mut().set_relay_node(Some(relay_node.clone())); changed = true; } @@ -165,18 +179,16 @@ impl RoutingDomainEditor { changed = true; } RoutingDomainChange::AddDialInfoDetail { dial_info_detail } => { - debug!( - "[{:?}] add dial info detail: {:?}", - self.routing_domain, dial_info_detail + info!( + "[{:?}] dial info: {:?}:{}", + self.routing_domain, + dial_info_detail.class, + dial_info_detail.dial_info ); detail .common_mut() .add_dial_info_detail(dial_info_detail.clone()); - info!( - "{:?} Dial Info: {}@{}", - self.routing_domain, node_ids, dial_info_detail.dial_info - ); changed = true; } RoutingDomainChange::SetupNetwork { @@ -195,22 +207,22 @@ impl RoutingDomainEditor { || old_address_types != address_types || old_capabilities != *capabilities; - debug!( - "[{:?}] setup network: {:?} {:?} {:?} {:?}", - self.routing_domain, - outbound_protocols, - inbound_protocols, - address_types, - capabilities - ); - - detail.common_mut().setup_network( - outbound_protocols, - inbound_protocols, - address_types, - capabilities.clone(), - ); if this_changed { + info!( + "[{:?}] setup network: {:?} {:?} {:?} {:?}", + self.routing_domain, + outbound_protocols, + inbound_protocols, + address_types, + capabilities + ); + + detail.common_mut().setup_network( + outbound_protocols, + inbound_protocols, + address_types, + capabilities.clone(), + ); changed = true; } } @@ -218,14 +230,16 @@ impl RoutingDomainEditor { let old_network_class = detail.common().network_class(); let this_changed = old_network_class != network_class; - - debug!( - "[{:?}] set network class: {:?}", - self.routing_domain, network_class, - ); - - detail.common_mut().set_network_class(network_class); if this_changed { + if let Some(network_class) = network_class { + info!( + "[{:?}] set network class: {:?}", + self.routing_domain, network_class, + ); + } else { + info!("[{:?}] cleared network class", self.routing_domain,); + } + detail.common_mut().set_network_class(network_class); changed = true; } } @@ -248,5 +262,8 @@ impl RoutingDomainEditor { rss.reset(); } } + + // Unpause routing table ticker + self.routing_table.pause_tasks(false).await; } } diff --git a/veilid-core/src/routing_table/routing_table_inner.rs b/veilid-core/src/routing_table/routing_table_inner.rs index d71e934a..8ba6cf88 100644 --- a/veilid-core/src/routing_table/routing_table_inner.rs +++ b/veilid-core/src/routing_table/routing_table_inner.rs @@ -34,6 +34,8 @@ pub struct RoutingTableInner { pub(super) recent_peers: LruCache, /// Storage for private/safety RouteSpecs pub(super) route_spec_store: Option, + /// Tick paused or not + pub(super) tick_paused: bool, } impl RoutingTableInner { @@ -50,6 +52,7 @@ impl RoutingTableInner { self_transfer_stats: TransferStatsDownUp::default(), recent_peers: LruCache::new(RECENT_PEERS_TABLE_SIZE), route_spec_store: None, + tick_paused: false, } } diff --git a/veilid-core/src/routing_table/tasks/mod.rs b/veilid-core/src/routing_table/tasks/mod.rs index cf7f7cdc..25a67cd0 100644 --- a/veilid-core/src/routing_table/tasks/mod.rs +++ b/veilid-core/src/routing_table/tasks/mod.rs @@ -125,6 +125,11 @@ impl RoutingTable { /// Ticks about once per second /// to run tick tasks which may run at slower tick rates as configured pub async fn tick(&self) -> EyreResult<()> { + // Don't tick if paused + if self.inner.read().tick_paused { + return Ok(()); + } + // Do rolling transfers every ROLLING_TRANSFERS_INTERVAL_SECS secs self.unlocked_inner.rolling_transfers_task.tick().await?; @@ -168,13 +173,33 @@ impl RoutingTable { self.unlocked_inner.relay_management_task.tick().await?; // Run the private route management task - self.unlocked_inner - .private_route_management_task - .tick() - .await?; + // If we don't know our network class then don't do this yet + if self.has_valid_network_class(RoutingDomain::PublicInternet) { + self.unlocked_inner + .private_route_management_task + .tick() + .await?; + } Ok(()) } + pub(crate) async fn pause_tasks(&self, paused: bool) { + let cancel = { + let mut inner = self.inner.write(); + if !inner.tick_paused && paused { + inner.tick_paused = true; + true + } else if inner.tick_paused && !paused { + inner.tick_paused = false; + false + } else { + false + } + }; + if cancel { + self.cancel_tasks().await; + } + } pub(crate) async fn cancel_tasks(&self) { // Cancel all tasks being ticked diff --git a/veilid-core/src/routing_table/tasks/ping_validator.rs b/veilid-core/src/routing_table/tasks/ping_validator.rs index 6f70b86b..bfc2fb24 100644 --- a/veilid-core/src/routing_table/tasks/ping_validator.rs +++ b/veilid-core/src/routing_table/tasks/ping_validator.rs @@ -12,7 +12,7 @@ impl RoutingTable { // Ping each node in the routing table if they need to be pinged // to determine their reliability #[instrument(level = "trace", skip(self), err)] - fn relay_keepalive_public_internet( + async fn relay_keepalive_public_internet( &self, cur_ts: Timestamp, relay_nr: NodeRef, @@ -41,7 +41,8 @@ impl RoutingTable { // Say we're doing this keepalive now self.edit_routing_domain(RoutingDomain::PublicInternet) .set_relay_node_keepalive(Some(cur_ts)) - .commit(); + .commit(false) + .await; // We need to keep-alive at one connection per ordering for relays // but also one per NAT mapping that we need to keep open for our inbound dial info @@ -119,7 +120,7 @@ impl RoutingTable { // Ping each node in the routing table if they need to be pinged // to determine their reliability #[instrument(level = "trace", skip(self), err)] - fn ping_validator_public_internet( + async fn ping_validator_public_internet( &self, cur_ts: Timestamp, unord: &mut FuturesUnordered< @@ -136,7 +137,8 @@ impl RoutingTable { // If this is our relay, let's check for NAT keepalives if let Some(relay_nr) = opt_relay_nr { - self.relay_keepalive_public_internet(cur_ts, relay_nr, unord)?; + self.relay_keepalive_public_internet(cur_ts, relay_nr, unord) + .await?; } // Just do a single ping with the best protocol for all the other nodes to check for liveness @@ -156,7 +158,7 @@ impl RoutingTable { // Ping each node in the LocalNetwork routing domain if they // need to be pinged to determine their reliability #[instrument(level = "trace", skip(self), err)] - fn ping_validator_local_network( + async fn ping_validator_local_network( &self, cur_ts: Timestamp, unord: &mut FuturesUnordered< @@ -195,10 +197,12 @@ impl RoutingTable { let mut unord = FuturesUnordered::new(); // PublicInternet - self.ping_validator_public_internet(cur_ts, &mut unord)?; + self.ping_validator_public_internet(cur_ts, &mut unord) + .await?; // LocalNetwork - self.ping_validator_local_network(cur_ts, &mut unord)?; + self.ping_validator_local_network(cur_ts, &mut unord) + .await?; // Wait for ping futures to complete in parallel while let Ok(Some(_)) = unord.next().timeout_at(stop_token.clone()).await {} diff --git a/veilid-core/src/routing_table/tasks/private_route_management.rs b/veilid-core/src/routing_table/tasks/private_route_management.rs index b8ddba51..dcf9fee1 100644 --- a/veilid-core/src/routing_table/tasks/private_route_management.rs +++ b/veilid-core/src/routing_table/tasks/private_route_management.rs @@ -169,11 +169,6 @@ impl RoutingTable { _last_ts: Timestamp, cur_ts: Timestamp, ) -> EyreResult<()> { - // If we don't know our network class then don't do this yet - if !self.has_valid_network_class(RoutingDomain::PublicInternet) { - return Ok(()); - } - // Test locally allocated routes first // This may remove dead routes let routes_needing_testing = self.get_allocated_routes_to_test(cur_ts); diff --git a/veilid-core/src/routing_table/tasks/relay_management.rs b/veilid-core/src/routing_table/tasks/relay_management.rs index 874200d6..b5b0bb02 100644 --- a/veilid-core/src/routing_table/tasks/relay_management.rs +++ b/veilid-core/src/routing_table/tasks/relay_management.rs @@ -23,13 +23,13 @@ impl RoutingTable { let state = relay_node.state(cur_ts); // Relay node is dead or no longer needed if matches!(state, BucketEntryState::Dead) { - info!("Relay node died, dropping relay {}", relay_node); + debug!("Relay node died, dropping relay {}", relay_node); editor.clear_relay_node(); false } // Relay node no longer can relay else if relay_node.operate(|_rti, e| !relay_node_filter(e)) { - info!( + debug!( "Relay node can no longer relay, dropping relay {}", relay_node ); @@ -38,7 +38,7 @@ impl RoutingTable { } // Relay node is no longer required else if !own_node_info.requires_relay() { - info!( + debug!( "Relay node no longer required, dropping relay {}", relay_node ); @@ -47,7 +47,7 @@ impl RoutingTable { } // Should not have relay for invalid network class else if !self.has_valid_network_class(RoutingDomain::PublicInternet) { - info!( + debug!( "Invalid network class does not get a relay, dropping relay {}", relay_node ); @@ -75,7 +75,7 @@ impl RoutingTable { false, ) { Ok(nr) => { - info!("Outbound relay node selected: {}", nr); + debug!("Outbound relay node selected: {}", nr); editor.set_relay_node(nr); got_outbound_relay = true; } @@ -84,20 +84,20 @@ impl RoutingTable { } } } else { - info!("Outbound relay desired but not available"); + debug!("Outbound relay desired but not available"); } } if !got_outbound_relay { // Find a node in our routing table that is an acceptable inbound relay if let Some(nr) = self.find_inbound_relay(RoutingDomain::PublicInternet, cur_ts) { - info!("Inbound relay node selected: {}", nr); + debug!("Inbound relay node selected: {}", nr); editor.set_relay_node(nr); } } } // Commit the changes - editor.commit(); + editor.commit(false).await; Ok(()) } diff --git a/veilid-core/src/veilid_api/debug.rs b/veilid-core/src/veilid_api/debug.rs index 9c169c5b..3e70cf04 100644 --- a/veilid-core/src/veilid_api/debug.rs +++ b/veilid-core/src/veilid_api/debug.rs @@ -570,7 +570,8 @@ impl VeilidAPI { routing_table .edit_routing_domain(routing_domain) .set_relay_node(relay_node) - .commit(); + .commit(true) + .await; Ok("Relay changed".to_owned()) }