From fb2f47c1e0f812af87825faa8a7a1052cf324a4b Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Fri, 11 Sep 2026 15:48:32 +0000 Subject: [PATCH] WebTransport: report the fate of outgoing datagrams via aggregate stats Adds SessionStats::datagrams_sent_outgoing/datagrams_dropped_outgoing, backed by the per-session sent count from the previous commit, so that once a session's queue is empty, every datagram a caller handed in has been counted as sent, expired or dropped exactly once. Dropped covers byte-budget evictions, datagrams refused at enqueue as the lowest priority, datagrams dropped at packet-build time as too big for the path MTU, and datagrams still queued when the session closes. The Capsule fallback counts as sent on write. close_session flushes the sent count and drops what is still queued before snapshotting the stats it returns, so the final values include the datagrams that call discards; a close or reset from the peer drops and counts the queue too. Both close paths expire stale datagrams first, so those count as expired rather than dropped. The per-tick sweep only visits the sessions the transport reports as having work. --- neqo-http3/src/connect_udp.rs | 3 +- neqo-http3/src/connection.rs | 31 +-- .../src/features/extended_connect/session.rs | 46 +++- .../src/features/extended_connect/stats.rs | 26 +- .../tests/webtransport/datagrams.rs | 258 +++++++++++++++--- .../extended_connect/webtransport_session.rs | 10 +- neqo-http3/src/webtransport.rs | 19 +- neqo-http3/tests/webtransport.rs | 6 +- neqo-transport/src/connection/mod.rs | 15 - .../src/connection/tests/datagram.rs | 6 - neqo-transport/src/datagram_queue.rs | 10 - neqo-transport/src/quic_datagrams.rs | 12 - 12 files changed, 306 insertions(+), 136 deletions(-) diff --git a/neqo-http3/src/connect_udp.rs b/neqo-http3/src/connect_udp.rs index 160db5647c..a8f5d5dbda 100644 --- a/neqo-http3/src/connect_udp.rs +++ b/neqo-http3/src/connect_udp.rs @@ -250,7 +250,8 @@ impl Handler for Http3Connection { error, message, now, - ) + )?; + Ok(()) } fn connect_udp_send_datagram>( diff --git a/neqo-http3/src/connection.rs b/neqo-http3/src/connection.rs index ddaf22dca8..830f942cc2 100644 --- a/neqo-http3/src/connection.rs +++ b/neqo-http3/src/connection.rs @@ -1447,13 +1447,9 @@ impl Http3Connection { error: u32, message: &str, now: Instant, - ) -> Res<()> { - if self - .get_extended_connect_session(session_id)? - .borrow() - .connect_type() - != connect_type - { + ) -> Res>> { + let session = self.get_extended_connect_session(session_id)?; + if session.borrow().connect_type() != connect_type { return Err(Error::InvalidStreamId); } let send_stream = self @@ -1468,7 +1464,7 @@ impl Http3Connection { } else if send_stream.has_data_to_send() { self.streams_with_pending_data.insert(session_id); } - Ok(()) + Ok(session) } fn get_extended_connect_session( @@ -1683,21 +1679,18 @@ impl Http3Connection { .datagram_queue_capacity(conn)) } - /// Expire stale outgoing datagrams on every active extended-CONNECT - /// session's queue and count them per session. Called once per - /// `process_http3` tick. The transport counts expiries on the queue - /// itself, whether its own timer sweep or this one sheds them, so this - /// sweep picks up the right number regardless of whether it runs before - /// or after `Connection::process_output` within a tick. - /// - /// Returns the total number of datagrams expired, for the caller to fold - /// into a stats counter. + /// Record expired, sent and too-big datagram counts for each active + /// extended-CONNECT session. Returns the number expired. pub(crate) fn expire_datagram_queues(&self, conn: &mut Connection, now: Instant) -> u64 { self.recv_streams .values() .filter_map(|s| s.extended_connect_session()) .filter(|s| s.borrow().is_active()) - .map(|s| s.borrow_mut().expire_datagrams(conn, now)) + .map(|s| { + let mut s = s.borrow_mut(); + s.report_sent_datagrams(conn); + s.expire_datagrams(conn, now) + }) .sum() } @@ -1884,7 +1877,7 @@ impl Http3Connection { wt: &Rc>, conn: &mut Connection, ) { - wt.borrow().drop_queued_datagrams(conn); + wt.borrow_mut().drop_queued_datagrams(conn); let (recv, send) = wt.borrow_mut().take_sub_streams(); diff --git a/neqo-http3/src/features/extended_connect/session.rs b/neqo-http3/src/features/extended_connect/session.rs index a34bf29124..aac182255d 100644 --- a/neqo-http3/src/features/extended_connect/session.rs +++ b/neqo-http3/src/features/extended_connect/session.rs @@ -13,7 +13,7 @@ use std::{ time::{Duration, Instant}, }; -use neqo_common::{Bytes, Encoder, Header, MessageType, Role, qdebug, qtrace}; +use neqo_common::{Bytes, Encoder, Header, MessageType, Role, qdebug, qtrace, to_u64}; #[cfg(test)] use neqo_transport::DatagramQueueCapacity; use neqo_transport::{ @@ -477,8 +477,8 @@ impl Session { self.datagram_capsule_blocked = true; } res?; - // This path never touches the queue, so it carries no - // backpressure signal. + // Bypasses the queue, so count it sent now. + self.protocol.record_sent_outgoing_datagrams(1); return Ok(DatagramQueueOutcome::Ok); } @@ -515,9 +515,31 @@ impl Session { send_order, )?; qtrace!("[{self}] enqueued datagram: {outcome:?}"); + match &outcome { + DatagramQueueOutcome::Ok | DatagramQueueOutcome::MaxBufferedReached => {} + DatagramQueueOutcome::Rejected => self.report_dropped(1), + DatagramQueueOutcome::Overflowed { dropped } => self.report_dropped(*dropped), + } Ok(outcome) } + fn report_dropped(&mut self, count: usize) { + self.protocol + .record_dropped_outgoing_datagrams(to_u64(count)); + } + + /// Drop this session's queue, recording its remaining datagrams and counts. + pub(crate) fn drop_queued_datagrams(&mut self, conn: &mut Connection) { + let dropped = conn.drop_session_datagrams(self.id); + self.protocol.record_sent_outgoing_datagrams(dropped.sent); + self.protocol + .record_expired_outgoing_datagrams(dropped.expired); + self.protocol + .record_dropped_outgoing_datagrams(dropped.too_big); + self.protocol + .record_dropped_outgoing_datagrams(dropped.queued); + } + /// Set the outgoing-datagram queue's `outgoingMaxBufferedDatagrams`, or /// clear it back to no limit with `None`. Ignored once closing: the /// queue is already dropped, and setting it would recreate it. @@ -572,10 +594,12 @@ impl Session { expired } - /// Drop this session's queue, which lives on `conn` and would otherwise - /// outlive the session. - pub(crate) fn drop_queued_datagrams(&self, conn: &mut Connection) { - _ = conn.drop_session_datagrams(self.id); + /// Record the sent and too-big counts since the last call. + pub(crate) fn report_sent_datagrams(&mut self, conn: &mut Connection) { + let sent = conn.take_session_sent_datagrams(self.id); + self.protocol.record_sent_outgoing_datagrams(sent); + let too_big = conn.take_session_too_big_datagrams(self.id); + self.protocol.record_dropped_outgoing_datagrams(too_big); } pub(crate) fn datagram(&self, datagram: Bytes) { @@ -782,10 +806,18 @@ pub(crate) trait Protocol: Debug + Display { None } + /// Record that `count` outgoing datagrams were sent. + /// A no-op default for protocols that don't track [`SessionStats`]. + fn record_sent_outgoing_datagrams(&mut self, _count: u64) {} + /// Record that `count` outgoing datagrams expired before being sent. /// A no-op default for protocols that don't track [`SessionStats`]. fn record_expired_outgoing_datagrams(&mut self, _count: u64) {} + /// Record that `count` outgoing datagrams were dropped unsent. + /// A no-op default for protocols that don't track [`SessionStats`]. + fn record_dropped_outgoing_datagrams(&mut self, _count: u64) {} + fn protocol(&self) -> Option<&str> { None } diff --git a/neqo-http3/src/features/extended_connect/stats.rs b/neqo-http3/src/features/extended_connect/stats.rs index 053da52544..442ee19b37 100644 --- a/neqo-http3/src/features/extended_connect/stats.rs +++ b/neqo-http3/src/features/extended_connect/stats.rs @@ -6,9 +6,9 @@ //! Per-session statistics exposed to the WebTransport API consumer. //! -//! Limited to the members of [`WebTransportDatagramStats`], which describe -//! per-session datagram queue behaviour and so cannot be derived from the -//! connection-level counters in [`neqo_transport::Stats`]. Everything else +//! Per-session datagram queue counters, which cannot be derived from +//! [`neqo_transport::Stats`]. Only [`OutgoingDatagramStats::expired`] is a +//! [`WebTransportDatagramStats`] member (`expiredOutgoing`). Everything else //! `getStats()` reports is scoped to the underlying connection, and is only //! exposed at all when that connection is dedicated to a single session. //! @@ -24,6 +24,22 @@ )] #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] pub struct SessionStats { - /// Outgoing datagrams that expired (per `outgoingMaxAge`) before being sent. - pub datagrams_expired_outgoing: u64, + pub outgoing_datagrams: OutgoingDatagramStats, +} + +/// What became of the datagrams handed to a session's outgoing queue. +#[expect( + clippy::module_name_repetitions, + reason = "stats::OutgoingDatagramStats is clearer than stats::OutgoingDatagram" +)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct OutgoingDatagramStats { + /// Written into a packet, or to the control stream for the capsule + /// fallback. Not a delivery guarantee. + pub sent: u64, + /// Expired (per `outgoingMaxAge`) before being sent. + pub expired: u64, + /// Discarded unsent and unexpired: evicted or refused by the queue, too + /// big for the path MTU, or still queued at close. + pub dropped: u64, } diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs index 97ff0aed6d..bb977dbd2d 100644 --- a/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs +++ b/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs @@ -4,16 +4,20 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::{num::NonZeroUsize, time::Duration}; +use std::{ + num::NonZeroUsize, + time::{Duration, Instant}, +}; use neqo_common::{Encoder, event::Provider as _, to_u64}; -use neqo_transport::{ConnectionParameters, DatagramQueueOutcome, streams::SendGroupId}; +use neqo_transport::{ConnectionParameters, DatagramQueueOutcome, StreamId, streams::SendGroupId}; use test_fixture::now; use crate::{ - Http3ClientEvent, Http3ServerEvent, SessionAcceptAction, WebTransportEvent, - features::extended_connect::tests::webtransport::{ - DATAGRAM_SIZE, WtTest, wt_default_parameters, + Http3ClientEvent, Http3ServerEvent, Res, SessionAcceptAction, WebTransportEvent, + features::extended_connect::{ + stats::SessionStats, + tests::webtransport::{DATAGRAM_SIZE, WtTest, wt_default_parameters}, }, frames::WebTransportFrame, webtransport::{ClientSession as _, ServerEvent, ServerSession}, @@ -114,7 +118,8 @@ fn datagram_expires_before_being_sent() { 0, "the stale datagram must be gone before it is ever handed to the QUIC layer" ); - assert_eq!(wt_session.stats().datagrams_expired_outgoing, 1); + assert_eq!(wt_session.stats().outgoing_datagrams.expired, 1); + assert_eq!(wt_session.stats().outgoing_datagrams.sent, 0); } #[test] @@ -154,6 +159,25 @@ fn datagram_of_exactly_the_peers_limit_is_accepted() { assert_eq!(wt_session.datagram_queue_capacity().queued_datagrams, 1); } +/// `OutgoingDatagramStats::sent` must count a datagram sent +/// without a tracking id. +#[test] +fn untracked_datagram_sent_is_counted_in_aggregate_stats() { + let mut wt = WtTest::new(); + let wt_session = wt.create_wt_session(); + let session_id = wt_session.stream_id(); + + assert_eq!( + client_send(&mut wt, session_id, DGRAM, None, now()), + Ok(DatagramQueueOutcome::Ok) + ); + wt.exchange_packets(); + + let stats = client_stats(&wt, session_id); + assert_eq!(stats.outgoing_datagrams.sent, 1); + assert_eq!(stats.outgoing_datagrams.dropped, 0); +} + /// With a mark of 2 already set, the first datagram `send` queues is `Ok` /// and the second crosses the mark. fn assert_second_datagram_crosses_a_mark_of_two( @@ -195,8 +219,7 @@ fn outgoing_datagram_space_available_forwarded() { .webtransport_set_max_buffered_datagrams(session_id, Some(NonZeroUsize::new(1).unwrap())) .unwrap(); assert_eq!( - wt.client - .webtransport_send_datagram(session_id, DGRAM, None, t0, SendGroupId::new(0), 0), + client_send(&mut wt, session_id, DGRAM, None, t0), Ok(DatagramQueueOutcome::MaxBufferedReached) ); assert!( @@ -253,7 +276,7 @@ fn close_session_stats_include_datagrams_expired_since_the_last_sweep() { let stats = wt_session .close_session(0, "", t0 + Duration::from_millis(10)) .unwrap(); - assert_eq!(stats.datagrams_expired_outgoing, 1); + assert_eq!(stats.outgoing_datagrams.expired, 1); } #[test] @@ -273,7 +296,7 @@ fn shrinking_max_age_counts_expired_datagrams_immediately() { ) .unwrap(); - assert_eq!(wt_session.stats().datagrams_expired_outgoing, 1); + assert_eq!(wt_session.stats().outgoing_datagrams.expired, 1); } /// A client that resets the session takes the server's queued datagrams @@ -369,6 +392,86 @@ fn datagram_send_order_controls_priority() { ); } +#[test] +fn session_close_counts_queued_datagrams_as_dropped() { + let mut wt = WtTest::new(); + let wt_session = wt.create_wt_session(); + let t0 = now(); + + _ = wt_session + .send_datagram(DGRAM, Some(3), t0, SendGroupId::new(0), 0) + .unwrap(); + let stats = wt_session.close_session(0, "bye", t0).unwrap(); + + assert_eq!( + stats.outgoing_datagrams.dropped, 1, + "the stats close_session returns must already count the drop" + ); +} + +/// Enqueueing only checks the peer's datagram limit, which can exceed the +/// path MTU; a datagram between the two is dropped at packet-build time. +#[test] +fn datagram_too_big_for_the_path_mtu_is_counted_as_dropped() { + let server_params = wt_default_parameters() + .connection_parameters(ConnectionParameters::default().datagram_size(2 * DATAGRAM_SIZE)); + let mut wt = WtTest::new_with_params(wt_default_parameters(), server_params); + let session_id = wt.create_wt_session().stream_id(); + let too_big = vec![0; usize::try_from(DATAGRAM_SIZE + 100).unwrap()]; + + assert_eq!( + client_send(&mut wt, session_id, &too_big, None, now()), + Ok(DatagramQueueOutcome::Ok) + ); + wt.exchange_packets(); + + let stats = client_stats(&wt, session_id); + assert_eq!(stats.outgoing_datagrams.dropped, 1); + assert_eq!(stats.outgoing_datagrams.sent, 0); +} + +#[test] +fn datagram_expires_on_the_implementation_defined_default_max_age() { + let mut wt = WtTest::new(); + let wt_session = wt.create_wt_session(); + let t0 = now(); + + // No set_datagram_max_age call: outgoingMaxAge is left at its default. + _ = wt_session + .send_datagram(DGRAM, Some(13), t0, SendGroupId::new(0), 0) + .unwrap(); + + // Well past the default, so it expires before it can be sent. + drop(wt.server.process_output(t0 + Duration::from_secs(1))); + + assert_eq!(wt_session.stats().outgoing_datagrams.expired, 1); + assert_eq!(wt_session.stats().outgoing_datagrams.sent, 0); +} + +#[test] +fn client_set_datagram_max_age_expires_queued_datagrams() { + let mut wt = WtTest::new(); + let wt_session = wt.create_wt_session(); + let session_id = wt_session.stream_id(); + let t0 = now(); + + assert_eq!( + client_send(&mut wt, session_id, DGRAM, Some(7), t0), + Ok(DatagramQueueOutcome::Ok) + ); + + let t1 = t0 + Duration::from_millis(200); + wt.client + .webtransport_set_datagram_max_age(session_id, Some(Duration::from_millis(100)), t1) + .unwrap(); + + assert_eq!( + client_stats(&wt, session_id).outgoing_datagrams.expired, + 1, + "shortening max_age past an already-queued datagram must expire it immediately" + ); +} + /// A burst exceeding the byte budget, with a mix of send-order priorities, /// must evict low-priority datagrams to make room for high-priority ones. #[test] @@ -385,17 +488,14 @@ fn datagram_burst_exceeding_byte_budget_preserves_priority_through_a_live_connec let mut total_evicted: usize = 0; let mut next_id: u64 = 0; loop { - let outcome = wt - .client - .webtransport_send_datagram( - session_id, - &next_id.to_le_bytes(), - Some(next_id), - now(), - SendGroupId::new(0), - 0, - ) - .unwrap(); + let outcome = client_send( + &mut wt, + session_id, + &next_id.to_le_bytes(), + Some(next_id), + now(), + ) + .unwrap(); low_priority_ids.push(next_id); next_id += 1; if let DatagramQueueOutcome::Overflowed { dropped } = outcome { @@ -493,14 +593,7 @@ fn client_set_max_buffered_datagrams_signals_backpressure() { .webtransport_set_max_buffered_datagrams(session_id, Some(NonZeroUsize::new(2).unwrap())) .unwrap(); assert_second_datagram_crosses_a_mark_of_two(|id| { - wt.client.webtransport_send_datagram( - session_id, - DGRAM, - Some(id), - now(), - SendGroupId::new(0), - 0, - ) + client_send(&mut wt, session_id, DGRAM, Some(id), now()) }); } @@ -517,11 +610,7 @@ fn client_changing_the_max_buffered_limit_resumes_only_a_blocked_queue() { .webtransport_set_max_buffered_datagrams(session_id, NonZeroUsize::new(n)) .unwrap(); }; - let send = |wt: &mut WtTest, id| { - wt.client - .webtransport_send_datagram(session_id, DGRAM, Some(id), t0, SendGroupId::new(0), 0) - .unwrap() - }; + let send = |wt: &mut WtTest, id| client_send(wt, session_id, DGRAM, Some(id), t0).unwrap(); // Read the transport's events directly: building a packet (as every // HTTP/3 `process_*` path would) sends the datagram, which resumes the // sender on its own. @@ -577,13 +666,98 @@ fn client_setters_work_before_the_connect_response() { wt.exchange_packets(); assert_second_datagram_crosses_a_mark_of_two(|id| { - wt.client.webtransport_send_datagram( - session_id, - DGRAM, - Some(id), - now(), - SendGroupId::new(0), - 0, - ) + client_send(&mut wt, session_id, DGRAM, Some(id), now()) }); } + +/// Send `buf` from the client, ungrouped at `send_order` 0. +fn client_send( + wt: &mut WtTest, + session_id: StreamId, + buf: &[u8], + id: Option, + now: Instant, +) -> Res { + wt.client + .webtransport_send_datagram(session_id, buf, id, now, SendGroupId::new(0), 0) +} + +fn client_stats(wt: &WtTest, session_id: StreamId) -> SessionStats { + wt.client.webtransport_session_stats(session_id).unwrap() +} + +/// Fill `session_id`'s queue with untracked `send_order` 10 datagrams until +/// the byte budget evicts one; returns how many were accepted. +fn fill_to_the_byte_budget(wt: &mut WtTest, session_id: StreamId) -> u64 { + for n in 1.. { + let outcome = wt + .client + .webtransport_send_datagram(session_id, DGRAM, None, now(), SendGroupId::new(0), 10) + .unwrap(); + if matches!(outcome, DatagramQueueOutcome::Overflowed { .. }) { + return n; + } + assert!(n < 1_000_000, "byte budget should have been hit by now"); + } + unreachable!() +} + +/// Untracked datagrams evicted at the byte budget, or refused for being the +/// lowest priority in a full queue, are counted as dropped. +#[test] +fn evicted_and_rejected_datagrams_are_counted_as_dropped() { + let mut wt = WtTest::new(); + let session_id = wt.create_wt_session().stream_id(); + + fill_to_the_byte_budget(&mut wt, session_id); + assert_eq!(client_stats(&wt, session_id).outgoing_datagrams.dropped, 1); + + assert_eq!( + client_send(&mut wt, session_id, DGRAM, None, now()), + Ok(DatagramQueueOutcome::Rejected) + ); + assert_eq!(client_stats(&wt, session_id).outgoing_datagrams.dropped, 2); +} + +/// Every accepted datagram ends up sent, expired or dropped exactly once. +#[test] +fn every_accepted_datagram_is_counted_exactly_once() { + let mut wt = WtTest::new(); + let session_id = wt.create_wt_session().stream_id(); + let t0 = now(); + + assert_eq!( + client_send(&mut wt, session_id, DGRAM, None, t0), + Ok(DatagramQueueOutcome::Ok) + ); + wt.exchange_packets(); + + assert_eq!( + client_send(&mut wt, session_id, DGRAM, None, t0), + Ok(DatagramQueueOutcome::Ok) + ); + let t1 = t0 + Duration::from_millis(10); + wt.client + .webtransport_set_datagram_max_age(session_id, Some(Duration::from_millis(1)), t1) + .unwrap(); + wt.client + .webtransport_set_datagram_max_age(session_id, None, t1) + .unwrap(); + + let queued = fill_to_the_byte_budget(&mut wt, session_id); + let accepted = 2 + queued; + + let stats = wt + .client + .webtransport_close_session(session_id, 0, "", now()) + .unwrap(); + assert_eq!(stats.outgoing_datagrams.sent, 1); + assert_eq!(stats.outgoing_datagrams.expired, 1); + assert_eq!(stats.outgoing_datagrams.dropped, queued); + assert_eq!( + stats.outgoing_datagrams.sent + + stats.outgoing_datagrams.expired + + stats.outgoing_datagrams.dropped, + accepted + ); +} diff --git a/neqo-http3/src/features/extended_connect/webtransport_session.rs b/neqo-http3/src/features/extended_connect/webtransport_session.rs index e648790caa..d873285a0c 100644 --- a/neqo-http3/src/features/extended_connect/webtransport_session.rs +++ b/neqo-http3/src/features/extended_connect/webtransport_session.rs @@ -247,8 +247,16 @@ impl Protocol for Session { Some(&self.stats) } + fn record_sent_outgoing_datagrams(&mut self, count: u64) { + self.stats.outgoing_datagrams.sent += count; + } + fn record_expired_outgoing_datagrams(&mut self, count: u64) { - self.stats.datagrams_expired_outgoing += count; + self.stats.outgoing_datagrams.expired += count; + } + + fn record_dropped_outgoing_datagrams(&mut self, count: u64) { + self.stats.outgoing_datagrams.dropped += count; } fn register_send_group(&mut self, id: SendGroupId) -> Res<()> { diff --git a/neqo-http3/src/webtransport.rs b/neqo-http3/src/webtransport.rs index b38407a4ee..11752f797a 100644 --- a/neqo-http3/src/webtransport.rs +++ b/neqo-http3/src/webtransport.rs @@ -552,18 +552,8 @@ impl Handler for Http3Connection { now: Instant, ) -> Res { qtrace!("Close WebTransport session {session_id:?}"); - // Collect expiries and snapshot the stats before tearing the session down, - // so the caller sees the final values. This also rejects non-WebTransport - // sessions. - // - // `extended_connect_close_session` then checks the type again. That is - // deliberate: it is shared with connect-udp, which needs the check for its own - // close path, so it cannot rely on this one having happened. - self.webtransport_session(session_id)? - .borrow_mut() - .expire_datagrams(conn, now); - let stats = self.webtransport_session_stats(session_id)?; - self.extended_connect_close_session( + // Read the stats after closing, which counts the queue's expired and dropped datagrams. + let session = self.extended_connect_close_session( conn, session_id, extended_connect::ExtendedConnectType::WebTransport, @@ -571,7 +561,8 @@ impl Handler for Http3Connection { message, now, )?; - Ok(stats) + let stats = session.borrow().stats(); + stats.ok_or(Error::InvalidStreamId) } fn webtransport_send_datagram>( @@ -938,7 +929,7 @@ impl ServerSession { ) } - /// This session's statistics, e.g. `datagrams_expired_outgoing`. + /// This session's statistics, e.g. `outgoing_datagrams.expired`. #[cfg(test)] pub(crate) fn stats(&self) -> extended_connect::stats::SessionStats { let session_id = self.stream_handler.stream_id(); diff --git a/neqo-http3/tests/webtransport.rs b/neqo-http3/tests/webtransport.rs index b37986e837..d3b5d12257 100644 --- a/neqo-http3/tests/webtransport.rs +++ b/neqo-http3/tests/webtransport.rs @@ -711,10 +711,8 @@ fn wt_export_keying_material_transport_error() { /// Covers the snapshot-before-teardown plumbing: `webtransport_close_session` must /// return the stats as they were before the session was dismantled. /// -/// The only counter in `SessionStats` so far is `datagrams_expired_outgoing`, and -/// nothing expires datagrams yet, so both snapshots are currently the default. The -/// datagram send below is scaffolding: once outgoing expiry is wired up it gives the -/// comparison something non-zero to catch. +/// The datagram sent below is counted in `outgoing_datagrams.sent`, so the +/// comparison has something non-zero to catch. #[test] fn wt_stats_at_session_close() { const DATAGRAM_DATA: &[u8] = &[1, 2, 3, 4, 5]; diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 5a13c30c4c..0e4fcc557d 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -4209,21 +4209,6 @@ impl Connection { self.quic_datagrams.take_session_expired_count(session) } - /// Whether any session has an expiry count waiting to be picked up by - /// [`Self::expire_session_datagrams`]. `process_timer` sheds stale - /// datagrams on its own schedule, so a caller that sweeps only when given - /// a reason to needs this to learn that there is now something to count. - /// - /// A queue's counts clear only when drained or when - /// [`Self::drop_session_datagrams`] removes the queue. A caller that - /// stops sweeping a session without dropping its queue therefore leaves - /// this `true` for the rest of the connection, and re-processes the - /// connection for nothing on every tick. - #[must_use] - pub fn has_pending_datagram_counts(&self) -> bool { - self.quic_datagrams.has_pending_counts() - } - /// Remove `session`'s queue, e.g. because the session is closing. Returns /// how many datagrams were still queued, and the sent, expiry and /// too-big counts not yet taken. diff --git a/neqo-transport/src/connection/tests/datagram.rs b/neqo-transport/src/connection/tests/datagram.rs index 6f47aa031b..1fa1861da2 100644 --- a/neqo-transport/src/connection/tests/datagram.rs +++ b/neqo-transport/src/connection/tests/datagram.rs @@ -948,18 +948,12 @@ fn timer_expiry_is_counted_by_the_next_session_sweep() { client.set_datagram_max_age(session, Some(Duration::from_millis(5)), now); _ = client.enqueue_datagram(session, vec![1], Some(1), now, SendGroupId::new(0), 0); - assert!(!client.has_pending_datagram_counts()); let later = now + Duration::from_millis(10); client.process_timer(later); assert_eq!(client.datagram_queue_capacity(session).queued_datagrams, 0); - assert!( - client.has_pending_datagram_counts(), - "the timer's expiry is waiting to be counted" - ); assert_eq!(client.expire_session_datagrams(session, later), 1); - assert!(!client.has_pending_datagram_counts()); assert_eq!( client.expire_session_datagrams(session, later), 0, diff --git a/neqo-transport/src/datagram_queue.rs b/neqo-transport/src/datagram_queue.rs index cbf17221de..d0540d6e12 100644 --- a/neqo-transport/src/datagram_queue.rs +++ b/neqo-transport/src/datagram_queue.rs @@ -525,13 +525,6 @@ impl DatagramQueue { std::mem::take(&mut self.expired) } - /// Whether [`Self::take_expired_count`] would return a nonzero count, - /// without consuming it. - #[must_use] - pub const fn has_expired(&self) -> bool { - self.expired > 0 - } - /// The `(send_order, group_id)` key of the globally lowest-priority /// occupied bucket: lowest `send_order` across all groups, ties broken by /// `group_id` (lowest first) for determinism. @@ -1584,14 +1577,11 @@ mod tests { q.set_max_age(Some(Duration::from_millis(10)), t0, NO_DEFAULT); enq(&mut q, vec![1], 1, t0, 0, 0); enq(&mut q, vec![2], 2, t0 + Duration::from_millis(5), 0, 0); - assert!(!q.has_expired()); assert_eq!(q.expire(t0 + Duration::from_millis(10), NO_DEFAULT), 1); - assert!(q.has_expired()); assert_eq!(q.expire(t0 + Duration::from_millis(15), NO_DEFAULT), 1); assert_eq!(q.take_expired_count(), 2); - assert!(!q.has_expired()); assert_eq!(q.take_expired_count(), 0); } diff --git a/neqo-transport/src/quic_datagrams.rs b/neqo-transport/src/quic_datagrams.rs index a6d638fdfa..eebe7e1357 100644 --- a/neqo-transport/src/quic_datagrams.rs +++ b/neqo-transport/src/quic_datagrams.rs @@ -429,18 +429,6 @@ impl QuicDatagrams { .map_or(0, DatagramQueue::take_expired_count) } - /// Whether any session has a count waiting to be picked up by - /// [`Self::expire_session_datagrams`] or - /// [`Self::take_session_expired_count`]. Only those, or - /// [`Self::drop_session_datagrams`], ever clear a queue's counts. - /// - /// Sent counts are not included yet: nothing drains them, so including - /// them would leave this `true` after the first send. - #[must_use] - pub fn has_pending_counts(&self) -> bool { - self.queues.values().any(DatagramQueue::has_expired) - } - /// Expire `queue`'s stale entries and, if that unblocks it, fire the /// resume event. Shared by [`Self::expire_datagrams`] (every session), /// [`Self::expire_session_datagrams`] (a single one) and