mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Merge fix/peer-connectivity-from-idle into master, reporting a silent peer as stale
Carries the idle-time connectivity derivation for show_peers and the tick-published peer row up from the maint line. The discovery dial gate that master wraps as active_peer_link_is_live now reaches the same idle rule through peer_link_is_stale, and that helper's doc names the gate alongside the same-path re-dial. The only conflict was the doc comment and visibility of the heartbeat-interval test helper: master's doc text is kept with the pub(super) visibility the new control tests need.
This commit is contained in:
@@ -429,6 +429,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
link was created with. The counters cover authenticated link frames only, so
|
||||
they are not expected to match the transport totals in `show_transports`.
|
||||
The response shape is unchanged.
|
||||
- `show_peers` (`fipsctl show peers`) now reports a peer that has gone quiet
|
||||
as `stale`. Its `connectivity` was read from a state that nothing outside
|
||||
the tests ever changed, so every peer read `connected` until it was
|
||||
removed, including one that had stopped answering tens of seconds earlier.
|
||||
The value is now derived from how long the peer has been silent: `connected`
|
||||
while its idle time is at or below `heartbeat_interval_secs`, and `stale`
|
||||
above it, the same rule that decides whether discovery re-dials an active
|
||||
peer on the path it already has. The `reconnecting` and `disconnected`
|
||||
values the open-discovery tutorial described never occurred, and the
|
||||
tutorial no longer lists them. The response shape is unchanged.
|
||||
|
||||
### Changed
|
||||
|
||||
|
||||
@@ -284,7 +284,7 @@ sudo fipsctl show peers
|
||||
|
||||
In addition to your configured `test-us01` peer, you may see
|
||||
an entry for `test-us03` (the open-discovery test mesh node).
|
||||
It will have `connectivity` active and its own
|
||||
It will have `connectivity` `connected` and its own
|
||||
`transport_addr`. This peering appeared without you
|
||||
configuring anything — the test-mesh open-discovery node saw
|
||||
your advert, dialed the endpoint, and Noise IK established
|
||||
|
||||
@@ -200,11 +200,14 @@ You should see considerably more entries than before:
|
||||
Each entry has its own `connectivity` state, and every entry
|
||||
that appears here completed a handshake at least once: a peer
|
||||
whose advert was stale, or that NAT traversal never reached,
|
||||
produces no entry at all rather than a failed one. Healthy links
|
||||
read `connected`. A link not heard from recently reads `stale`
|
||||
and still carries traffic; one that dropped and is being retried
|
||||
reads `reconnecting`, and one explicitly torn down reads
|
||||
`disconnected`. Neither of the last two can send.
|
||||
produces no entry at all rather than a failed one. A link heard
|
||||
from within the last heartbeat interval
|
||||
(`node.heartbeat_interval_secs`, 10 seconds by default) reads
|
||||
`connected`. One silent for longer reads `stale`; it still
|
||||
carries traffic, and it reads `connected` again as soon as the
|
||||
peer is heard from. A link that stays silent until it is declared
|
||||
dead is removed, so its entry disappears rather than changing
|
||||
state.
|
||||
|
||||
To get a list of just the connected links:
|
||||
|
||||
|
||||
@@ -250,6 +250,7 @@ pub fn show_peers(node: &Node) -> Value {
|
||||
// start (no peer has SRTT) every peer uses the default link cost of 1.0.
|
||||
let any_peer_has_srtt = node.peers().any(|p| p.has_srtt());
|
||||
|
||||
let now = now_ms();
|
||||
let peers: Vec<Value> = node
|
||||
.peers()
|
||||
.map(|peer| {
|
||||
@@ -267,7 +268,7 @@ pub fn show_peers(node: &Node) -> Value {
|
||||
"npub": peer.npub(),
|
||||
"display_name": node.peer_display_name(&node_addr),
|
||||
"ipv6_addr": format!("{}", peer.address()),
|
||||
"connectivity": format!("{}", peer.connectivity()),
|
||||
"connectivity": format!("{}", node.peer_connectivity(peer, now)),
|
||||
"link_id": peer.link_id().as_u64(),
|
||||
"authenticated_at_ms": peer.authenticated_at(),
|
||||
"last_seen_ms": peer.last_seen(),
|
||||
|
||||
@@ -3297,13 +3297,7 @@ impl Node {
|
||||
let Some(peer) = self.peers.get(peer_node_addr) else {
|
||||
return false;
|
||||
};
|
||||
let stale_after_ms = self
|
||||
.config()
|
||||
.node
|
||||
.heartbeat_interval_secs
|
||||
.saturating_mul(1000)
|
||||
.max(1000);
|
||||
peer.idle_time(Self::now_ms()) > stale_after_ms
|
||||
self.peer_link_is_stale(peer, Self::now_ms())
|
||||
}
|
||||
|
||||
pub(in crate::node) fn active_peer_matches_candidate(
|
||||
|
||||
+34
-2
@@ -44,8 +44,8 @@ use self::reloadable::Reloadable;
|
||||
pub(crate) const REKEY_JITTER_SECS: i64 = 15;
|
||||
use crate::cache::CoordCache;
|
||||
use crate::node::session::SessionEntry;
|
||||
use crate::peer::ActivePeer;
|
||||
use crate::peer::machine::{PeerMachine, TimerKind};
|
||||
use crate::peer::{ActivePeer, ConnectivityState};
|
||||
use crate::proto::bloom::{BloomFilter, BloomState};
|
||||
use crate::proto::fmp::Fmp;
|
||||
use crate::proto::fmp::wire::{
|
||||
@@ -2271,6 +2271,7 @@ impl Node {
|
||||
// SRTT) every peer falls back to the default link cost of 1.0.
|
||||
let any_peer_has_srtt = self.peers().any(|p| p.has_srtt());
|
||||
|
||||
let now_ms = Self::now_ms();
|
||||
let peer_rows: Vec<snap::PeerRow> = self
|
||||
.peers()
|
||||
.map(|peer| {
|
||||
@@ -2323,7 +2324,7 @@ impl Node {
|
||||
npub: peer.npub(),
|
||||
display_name: self.peer_display_name(&node_addr),
|
||||
ipv6_addr: format!("{}", peer.address()),
|
||||
connectivity: format!("{}", peer.connectivity()),
|
||||
connectivity: format!("{}", self.peer_connectivity(peer, now_ms)),
|
||||
link_id: peer.link_id().as_u64(),
|
||||
authenticated_at_ms: peer.authenticated_at(),
|
||||
last_seen_ms: peer.last_seen(),
|
||||
@@ -3002,6 +3003,37 @@ impl Node {
|
||||
self.peers.values()
|
||||
}
|
||||
|
||||
/// Whether an active peer has been silent at `now_ms` for longer than the
|
||||
/// configured heartbeat interval, floored at one second.
|
||||
///
|
||||
/// The one idle-time liveness rule: the control socket reports such a peer
|
||||
/// as `stale`, the discovery dial gate
|
||||
/// [`Self::active_peer_link_is_live`] no longer holds its link as live, and
|
||||
/// discovery re-dials it on the path it already has.
|
||||
pub(in crate::node) fn peer_link_is_stale(&self, peer: &ActivePeer, now_ms: u64) -> bool {
|
||||
let stale_after_ms = self
|
||||
.config()
|
||||
.node
|
||||
.heartbeat_interval_secs
|
||||
.saturating_mul(1000)
|
||||
.max(1000);
|
||||
peer.idle_time(now_ms) > stale_after_ms
|
||||
}
|
||||
|
||||
/// Connectivity of an active peer as the control socket reports it:
|
||||
/// `Stale` when [`Self::peer_link_is_stale`] holds at `now_ms`, otherwise
|
||||
/// `Connected`.
|
||||
///
|
||||
/// Derived from idle time rather than read from the state stored on the
|
||||
/// peer, which nothing in production changes after promotion.
|
||||
pub(crate) fn peer_connectivity(&self, peer: &ActivePeer, now_ms: u64) -> ConnectivityState {
|
||||
if self.peer_link_is_stale(peer, now_ms) {
|
||||
ConnectivityState::Stale
|
||||
} else {
|
||||
ConnectivityState::Connected
|
||||
}
|
||||
}
|
||||
|
||||
/// Reference to the Nostr discovery handle if discovery is enabled.
|
||||
/// Used by control queries (`show_peers` per-peer Nostr-traversal
|
||||
/// state) to read failure-state without taking shared ownership.
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
//! socket framing.
|
||||
|
||||
use super::*;
|
||||
use heartbeat::set_heartbeat_interval;
|
||||
use spanning_tree::{
|
||||
TestNode, add_loopback_alias, cleanup_nodes, drain_all_packets, make_test_node,
|
||||
process_available_packets, run_tree_test,
|
||||
@@ -421,3 +422,110 @@ async fn show_links_reports_the_traffic_counters_of_the_peer_bound_to_each_link(
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// Insert an authenticated peer last heard from at `last_seen_ms` and return
|
||||
/// its address.
|
||||
fn insert_peer_last_seen_at(node: &mut Node, link: u64, last_seen_ms: u64) -> NodeAddr {
|
||||
let identity = PeerIdentity::from_pubkey_full(Identity::generate().pubkey_full());
|
||||
let addr = *identity.node_addr();
|
||||
node.peers.insert(
|
||||
addr,
|
||||
ActivePeer::new(identity, LinkId::new(link), last_seen_ms),
|
||||
);
|
||||
addr
|
||||
}
|
||||
|
||||
/// The `connectivity` string a `show_peers` response gives the peer at `addr`.
|
||||
fn connectivity_of(peers: &serde_json::Value, addr: &NodeAddr) -> String {
|
||||
let addr_hex = hex::encode(addr.as_bytes());
|
||||
peers["peers"]
|
||||
.as_array()
|
||||
.expect("show_peers returns a peers array")
|
||||
.iter()
|
||||
.find(|row| row["node_addr"] == addr_hex.as_str())
|
||||
.and_then(|row| row["connectivity"].as_str())
|
||||
.expect("show_peers lists the peer with a connectivity string")
|
||||
.to_string()
|
||||
}
|
||||
|
||||
/// Render `show_peers` on the loop, then publish a tick and render it again
|
||||
/// from the snapshot the control socket serves.
|
||||
fn show_peers_both_renders(node: &mut Node) -> [(&'static str, serde_json::Value); 2] {
|
||||
let on_loop = crate::control::queries::show_peers(node);
|
||||
node.record_stats_history();
|
||||
let off_loop = crate::control::queries::show_peers_from_handle(&node.control_read_handle());
|
||||
[("on-loop", on_loop), ("snapshot", off_loop)]
|
||||
}
|
||||
|
||||
/// `show_peers` reports a peer silent for longer than the heartbeat interval
|
||||
/// as `stale`, and a peer heard from just now as `connected`, on both the
|
||||
/// on-loop render and the tick-published snapshot render.
|
||||
#[test]
|
||||
fn show_peers_reports_a_peer_idle_past_the_heartbeat_interval_as_stale() {
|
||||
let mut node = make_node();
|
||||
let interval_ms = node.config().node.heartbeat_interval_secs * 1000;
|
||||
assert_eq!(interval_ms, 10_000, "the default heartbeat interval");
|
||||
let now = Node::now_ms();
|
||||
let fresh = insert_peer_last_seen_at(&mut node, 1, now);
|
||||
let idle = insert_peer_last_seen_at(&mut node, 2, now - interval_ms - 5_000);
|
||||
|
||||
for (render, peers) in show_peers_both_renders(&mut node) {
|
||||
assert_eq!(
|
||||
connectivity_of(&peers, &idle),
|
||||
"stale",
|
||||
"{render} render, peer silent for 15 s"
|
||||
);
|
||||
assert_eq!(
|
||||
connectivity_of(&peers, &fresh),
|
||||
"connected",
|
||||
"{render} render, peer heard from just now"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The `stale` threshold is the configured heartbeat interval rather than a
|
||||
/// fixed ten seconds: with a 30 s interval a peer silent for 15 s still reads
|
||||
/// `connected`, and one silent for 35 s reads `stale`.
|
||||
#[test]
|
||||
fn show_peers_stale_threshold_follows_the_configured_heartbeat_interval() {
|
||||
let mut node = make_node();
|
||||
set_heartbeat_interval(&mut node, 30);
|
||||
let now = Node::now_ms();
|
||||
let quiet = insert_peer_last_seen_at(&mut node, 1, now - 15_000);
|
||||
let idle = insert_peer_last_seen_at(&mut node, 2, now - 35_000);
|
||||
|
||||
for (render, peers) in show_peers_both_renders(&mut node) {
|
||||
assert_eq!(
|
||||
connectivity_of(&peers, &idle),
|
||||
"stale",
|
||||
"{render} render, peer silent for 35 s"
|
||||
);
|
||||
assert_eq!(
|
||||
connectivity_of(&peers, &quiet),
|
||||
"connected",
|
||||
"{render} render, peer silent for 15 s"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The derived connectivity changes at the heartbeat interval exactly: a peer
|
||||
/// silent for the whole interval still reads `connected`, and one millisecond
|
||||
/// more reads `stale`. A zero interval is floored at one second, the floor the
|
||||
/// discovery re-dial gate applies.
|
||||
#[test]
|
||||
fn peer_connectivity_turns_stale_one_millisecond_past_the_heartbeat_interval() {
|
||||
let mut node = make_node();
|
||||
let seen = 1_000_000;
|
||||
let addr = insert_peer_last_seen_at(&mut node, 1, seen);
|
||||
let at = |node: &Node, now_ms: u64| {
|
||||
let peer = node.get_peer(&addr).expect("the peer was inserted");
|
||||
node.peer_connectivity(peer, now_ms)
|
||||
};
|
||||
|
||||
assert_eq!(at(&node, seen + 10_000), ConnectivityState::Connected);
|
||||
assert_eq!(at(&node, seen + 10_001), ConnectivityState::Stale);
|
||||
|
||||
set_heartbeat_interval(&mut node, 0);
|
||||
assert_eq!(at(&node, seen + 1_000), ConnectivityState::Connected);
|
||||
assert_eq!(at(&node, seen + 1_001), ConnectivityState::Stale);
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@ fn set_link_dead_timeout(node: &mut crate::node::Node, secs: u64) {
|
||||
/// Set `node.heartbeat_interval_secs` on an already-constructed node, the same
|
||||
/// way `set_link_dead_timeout` does. This is the knob the retry gate must not
|
||||
/// floor.
|
||||
fn set_heartbeat_interval(node: &mut crate::node::Node, secs: u64) {
|
||||
pub(super) fn set_heartbeat_interval(node: &mut crate::node::Node, secs: u64) {
|
||||
node.replace_context(|ctx| {
|
||||
let mut cfg = (*ctx.config).clone();
|
||||
cfg.node.heartbeat_interval_secs = secs;
|
||||
|
||||
Reference in New Issue
Block a user