gsadmin/noq
1
0
Fork 0
mirror of https://github.com/n0-computer/noq.git synced 2026-10-04 05:25:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: split path stats from connection stats (#168)

Browse Source 
This separates the path stats from the connection stats. The advantage
is that accessing the stats doesn't involve an allocation (for cloning
the map of the path stats).
This commit is contained in:
Franz Heinzmann
2025-11-07 10:45:46 +01:00
committed by GitHub
parent 25c75bf574 c232f40b99
commit dced59ee0a
5 changed files with 63 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quinn-proto/src/connection/mod.rs
+20 -17
View File
@@ -253,6 +253,8 @@ pub struct Connection {
datagrams: DatagramState,
/// Connection level statistics
stats: ConnectionStats,
/// Path level statistics
path_stats: FxHashMap<PathId, PathStats>,
/// QUIC version used for the connection.
version: u32,
@@ -430,6 +432,7 @@ impl Connection {
rem_cids: FxHashMap::from_iter([(PathId::ZERO, CidQueue::new(rem_cid))]),
rng,
stats: ConnectionStats::default(),
path_stats: Default::default(),
version,
// peer params are not yet known, so multipath is not enabled
@@ -1435,8 +1438,7 @@ impl Connection {
PadDatagram::ToSize(probe_size),
);
self.stats
.paths
self.path_stats
.entry(path_id)
.or_default()
.sent_plpmtud_probes += 1;
@@ -1920,15 +1922,19 @@ impl Connection {
/// Returns connection statistics
pub fn stats(&mut self) -> ConnectionStats {
for (path_id, path) in self.paths.iter() {
let stats = self.stats.paths.entry(*path_id).or_default();
stats.rtt = path.data.rtt.get();
stats.cwnd = path.data.congestion.window();
stats.current_mtu = path.data.mtud.current_mtu();
}
self.stats.clone()
}
/// Returns path statistics
pub fn path_stats(&mut self, path_id: PathId) -> Option<PathStats> {
let path = self.paths.get(&path_id)?;
let stats = self.path_stats.entry(path_id).or_default();
stats.rtt = path.data.rtt.get();
stats.cwnd = path.data.congestion.window();
stats.current_mtu = path.data.mtud.current_mtu();
Some(*stats)
}
/// Ping the remote endpoint
///
/// Causes an ACK-eliciting packet to be transmitted on the connection.
@@ -2355,7 +2361,7 @@ impl Connection {
}
Ok(false) => {}
Ok(true) => {
self.stats.paths.entry(path).or_default().congestion_events += 1;
self.path_stats.entry(path).or_default().congestion_events += 1;
self.path_data_mut(path).congestion.on_congestion_event(
now,
largest_sent_time,
@@ -2621,7 +2627,7 @@ impl Connection {
self.paths.remove(&path_id);
self.spaces[SpaceId::Data].number_spaces.remove(&path_id);
let path_stats = self.stats.paths.remove(&path_id).unwrap_or_default();
let path_stats = self.path_stats.remove(&path_id).unwrap_or_default();
self.events.push_back(
PathEvent::Abandoned {
id: path_id,
@@ -2655,7 +2661,7 @@ impl Connection {
let old_bytes_in_flight = self.path_data_mut(path_id).in_flight.bytes;
let largest_lost_sent =
self.spaces[pn_space].for_path(path_id).sent_packets[&largest_lost].time_sent;
let path_stats = self.stats.paths.entry(path_id).or_default();
let path_stats = self.path_stats.entry(path_id).or_default();
path_stats.lost_packets += lost_packets.len() as u64;
path_stats.lost_bytes += size_of_lost_packets;
trace!(
@@ -2707,8 +2713,7 @@ impl Connection {
if let Some(max_datagram_size) = self.datagrams().max_size() {
self.datagrams.drop_oversized(max_datagram_size);
}
self.stats
.paths
self.path_stats
.entry(path_id)
.or_default()
.black_holes_detected += 1;
@@ -2719,8 +2724,7 @@ impl Connection {
old_bytes_in_flight != self.path_data_mut(path_id).in_flight.bytes;
if lost_ack_eliciting {
self.stats
.paths
self.path_stats
.entry(path_id)
.or_default()
.congestion_events += 1;
@@ -2745,8 +2749,7 @@ impl Connection {
.unwrap()
.remove_in_flight(&info);
self.path_data_mut(path_id).mtud.on_probe_lost();
self.stats
.paths
self.path_stats
.entry(path_id)
.or_default()
.lost_plpmtud_probes += 1;
quinn-proto/src/connection/packet_builder.rs
+1 -1
View File
@@ -236,7 +236,7 @@ impl<'a, 'b> PacketBuilder<'a, 'b> {
packet,
conn.spaces[space_id].for_path(path_id),
);
conn.stats.paths.entry(path_id).or_default().sent_packets += 1;
conn.path_stats.entry(path_id).or_default().sent_packets += 1;
conn.reset_keep_alive(path_id, now);
if size != 0 {
if ack_eliciting {
quinn-proto/src/connection/stats.rs
-6
View File
@@ -1,12 +1,8 @@
//! Connection statistics
use rustc_hash::FxHashMap;
use crate::FrameType;
use crate::{Dir, Duration, frame::Frame};
use super::PathId;
/// Statistics about UDP datagrams transmitted or received on a connection
///
/// All QUIC packets are carried by UDP datagrams. Hence, these statistics cover all traffic on a connection.
@@ -216,6 +212,4 @@ pub struct ConnectionStats {
pub frame_tx: FrameStats,
/// Statistics about frames received on a connection
pub frame_rx: FrameStats,
/// Statistics related to the current transmission path
pub paths: FxHashMap<PathId, PathStats>,
}
quinn-proto/src/tests/mod.rs
+36 -44
View File
@@ -599,9 +599,7 @@ fn zero_rtt_happypath() {
let _ = chunks.finalize();
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
@@ -682,9 +680,7 @@ fn zero_rtt_rejection() {
let _ = chunks.finalize();
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
@@ -780,9 +776,7 @@ fn test_zero_rtt_incoming_limit<F: FnOnce(&mut ServerConfig)>(configure_server:
let _ = chunks.finalize();
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
EXPECTED_DROPPED
@@ -1035,18 +1029,14 @@ fn key_update_simple() {
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
);
assert_eq!(
pair.server_conn_mut(server_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
@@ -1084,9 +1074,7 @@ fn key_update_reordered() {
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
@@ -1107,18 +1095,14 @@ fn key_update_reordered() {
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
);
assert_eq!(
pair.server_conn_mut(server_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
@@ -1765,9 +1749,7 @@ fn handshake_1rtt_handling() {
assert!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets
!= 0
@@ -2369,14 +2351,17 @@ fn connect_lost_mtu_probes_do_not_trigger_congestion_control() {
let (client_ch, server_ch) = pair.connect();
pair.drive();
let client_stats = pair.client_conn_mut(client_ch).stats();
let server_stats = pair.server_conn_mut(server_ch).stats();
// Sanity check (all MTU probes should have been lost)
let client_path_stats = client_stats.paths.get(&PathId::ZERO).unwrap();
let client_path_stats = pair
.client_conn_mut(client_ch)
.path_stats(PathId::ZERO)
.unwrap();
assert_eq!(client_path_stats.sent_plpmtud_probes, 9);
assert_eq!(client_path_stats.lost_plpmtud_probes, 9);
let server_path_stats = server_stats.paths.get(&PathId::ZERO).unwrap();
let server_path_stats = pair
.server_conn_mut(server_ch)
.path_stats(PathId::ZERO)
.unwrap();
assert_eq!(server_path_stats.sent_plpmtud_probes, 9);
assert_eq!(server_path_stats.lost_plpmtud_probes, 9);
@@ -2481,14 +2466,12 @@ fn connect_runs_mtud_again_after_600_seconds() {
// Sanity check: the mtu has been discovered
let client_conn = pair.client_conn_mut(client_ch);
let client_stats = client_conn.stats();
let client_path_stats = client_stats.paths.get(&PathId::ZERO).unwrap();
let client_path_stats = client_conn.path_stats(PathId::ZERO).unwrap();
assert_eq!(client_conn.path_mtu(), 1389);
assert_eq!(client_path_stats.sent_plpmtud_probes, 5);
assert_eq!(client_path_stats.lost_plpmtud_probes, 3);
let server_conn = pair.server_conn_mut(server_ch);
let server_stats = server_conn.stats();
let server_path_stats = server_stats.paths.get(&PathId::ZERO).unwrap();
let server_path_stats = server_conn.path_stats(PathId::ZERO).unwrap();
assert_eq!(server_conn.path_mtu(), 1389);
assert_eq!(server_path_stats.sent_plpmtud_probes, 5);
assert_eq!(server_path_stats.lost_plpmtud_probes, 3);
@@ -2542,8 +2525,10 @@ fn blackhole_after_mtu_change_repairs_itself() {
assert_eq!(buf.len(), 1300);
// Sanity checks (black hole detected after 3 lost packets)
let client_stats = pair.client_conn_mut(client_ch).stats();
let client_path_stats = client_stats.paths.get(&PathId::ZERO).unwrap();
let client_path_stats = pair
.client_conn_mut(client_ch)
.path_stats(PathId::ZERO)
.unwrap();
assert!(client_path_stats.lost_packets >= 3);
assert!(client_path_stats.congestion_events >= 3);
assert_eq!(client_path_stats.black_holes_detected, 1);
@@ -2557,7 +2542,10 @@ fn mtud_probes_include_immediate_ack() {
pair.drive();
let stats = pair.client_conn_mut(client_ch).stats();
let path_stats = stats.paths.get(&PathId::ZERO).unwrap();
let path_stats = pair
.client_conn_mut(client_ch)
.path_stats(PathId::ZERO)
.unwrap();
assert_eq!(path_stats.sent_plpmtud_probes, 4);
// Each probe contains a ping and an immediate ack
@@ -2758,6 +2746,10 @@ fn single_ack_eliciting_packet_with_ce_bit_triggers_immediate_ack() {
pair.drive();
let stats_after_connect = pair.client_conn_mut(client_ch).stats();
let after_connect_path_stats = pair
.client_conn_mut(client_ch)
.path_stats(PathId::ZERO)
.unwrap();
let start = pair.time;
@@ -2782,8 +2774,10 @@ fn single_ack_eliciting_packet_with_ce_bit_triggers_immediate_ack() {
stats_after_ping.frame_rx.acks - stats_after_connect.frame_rx.acks,
1
);
let after_ping_path_stats = stats_after_ping.paths.get(&PathId::ZERO).unwrap();
let after_connect_path_stats = stats_after_connect.paths.get(&PathId::ZERO).unwrap();
let after_ping_path_stats = pair
.client_conn_mut(client_ch)
.path_stats(PathId::ZERO)
.unwrap();
assert_eq!(
after_ping_path_stats.congestion_events - after_connect_path_stats.congestion_events,
1
@@ -3548,9 +3542,7 @@ fn address_discovery_zero_rtt_accepted() {
let _ = chunks.finalize();
assert_eq!(
pair.client_conn_mut(client_ch)
.stats()
.paths
.get(&PathId::ZERO)
.path_stats(PathId::ZERO)
.unwrap()
.lost_packets,
0
quinn/src/connection.rs
+6 -1
View File
@@ -28,7 +28,7 @@ use crate::{
};
use proto::{
ConnectionError, ConnectionHandle, ConnectionStats, Dir, EndpointEvent, PathError, PathEvent,
PathId, PathStatus, Side, StreamEvent, StreamId, congestion::Controller,
PathId, PathStats, PathStatus, Side, StreamEvent, StreamId, congestion::Controller,
};
/// In-progress connection attempt future
@@ -706,6 +706,11 @@ impl Connection {
self.0.state.lock("stats").inner.stats()
}
/// Returns path statistics
pub fn path_stats(&self, path_id: PathId) -> Option<PathStats> {
self.0.state.lock("path_stats").inner.path_stats(path_id)
}
/// Current state of the congestion control algorithm, for debugging purposes
pub fn congestion_state(&self) -> Box<dyn Controller> {
self.0