From 705f6b5569679a83f1899133a47a16e3d84a7b42 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 25 Sep 2026 05:26:57 -0700 Subject: [PATCH 01/15] quest: open the drain line Co-Authored-By: Claude Opus 5.5 From a40cc9a807b1050ab11df126cf2756276af7a45d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 25 Sep 2026 08:09:37 -0700 Subject: [PATCH 02/15] feat(relay): hand the drain signal to embedders and drain arrivals to one deadline (#4138) Co-authored-by: Claude Opus 5.5 --- doc/bin/relay/config.md | 16 ++ doc/bin/relay/index.md | 9 +- quest/m1/drain/README.md | 12 +- quest/m1/drain/drain-exit.md | 19 +++ quest/m1/drain/relay-drain-api.md | 22 --- rs/moq-relay/src/relay.rs | 53 +++++-- rs/moq-relay/src/shutdown.rs | 68 ++++++--- rs/moq-relay/tests/runtime_uring.rs | 68 +++++++++ rs/moq-relay/tests/shutdown_signal.rs | 207 ++++++++++++++++++++++++-- 9 files changed, 390 insertions(+), 84 deletions(-) create mode 100644 quest/m1/drain/drain-exit.md delete mode 100644 quest/m1/drain/relay-drain-api.md diff --git a/doc/bin/relay/config.md b/doc/bin/relay/config.md index 75d792c36f..178615089b 100644 --- a/doc/bin/relay/config.md +++ b/doc/bin/relay/config.md @@ -226,6 +226,22 @@ secret = "./iroh-secret.key" # Persist the key so the endpoint id surviv See [Transport](/concept/transport#iroh-peer-to-peer-experimental). +## Shutdown + +```toml +drain_timeout = "10s" # Top-level key, as --drain-timeout / MOQ_DRAIN_TIMEOUT. +``` + +The first SIGTERM or SIGINT starts a drain: every session is sent a GOAWAY +asking it to reconnect, and is force-closed if it is still connected when the +window ends. A session that connects during the drain, such as a client with a +cached DNS answer, is sent a GOAWAY immediately, with only the time left in +the window. The relay exits one second after the window ends, or immediately +on a second signal. `0` skips the GOAWAY and closes every session at once. +Only moq-lite-04+ and moq-transport clients act on a GOAWAY; older ones are +closed when the window ends. An embedder can take over the signals and start +the drain itself; see [Embed](/bin/relay/#embed). + ## \[log] ```toml diff --git a/doc/bin/relay/index.md b/doc/bin/relay/index.md index 0d449cb0bd..9e6393d557 100644 --- a/doc/bin/relay/index.md +++ b/doc/bin/relay/index.md @@ -77,9 +77,12 @@ TLS, and a certificate fingerprint for client pinning. The accessors borrow and `run` consumes the relay, so clone `cluster`, `auth`, `client`, `stats`, `shutdown`, and `shutdown_trigger` for application -tasks before calling it. `trigger.start()` drains every session with a GOAWAY -and `run` returns once the drain window elapses, with the listeners released -and the workers joined. Build routes from `web().routes()` (or +tasks before calling it. `trigger.start()` drains every session with a GOAWAY, +including any that connect afterwards, and `run` returns once the drain window +elapses, with the listeners released and the workers joined. `run` also starts +the drain on SIGTERM or SIGINT. An application that owns those signals, for +example to withdraw the node from DNS and wait out the TTL before draining, +calls `with_signals(false)` and fires the trigger itself. Build routes from `web().routes()` (or `internal().routes()`): `with_web` replaces the router, so `Router::new()` drops the built-in routes. Extra listeners (RTMP, SRT, ...) sit beside `run` in the application's `select!`. `runtime.workers` and `runtime.io_uring` stay diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md index 96d9195bb8..93428a5a8c 100644 --- a/quest/m1/drain/README.md +++ b/quest/m1/drain/README.md @@ -29,10 +29,9 @@ them (a planned-drain health state, the SIGTERM sequencing and stop timeouts, per-PoP serial deploys, a two-node PoP floor, and the gateway drain contract) is moq.pro's (downstream) fleet drain work, which consumes these quests. -**relay-drain-api.** A drain hook that GOAWAYs every established session and -immediately GOAWAYs any new arrival, so an embedding process can enter drain -on SIGTERM after the DNS window and still bound the total stop time. Expose -enough phase/session state to prove which bound ended a drain. +The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM +to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals +included, against one deadline. **client-goaway.** The JS reconnector migrates like the Rust one, preserving the app-visible session while resolving DNS again before dialing, and the Rust @@ -43,12 +42,11 @@ by the stop deadline and encoder reconnect. ## Quests -- [Relay drain api](/quest/m1/drain/relay-drain-api.md) - a drain hook that - GOAWAYs every session, including new arrivals, triggered by the embedding - process on SIGTERM - [Client goaway](/quest/m1/drain/client-goaway.md) - the JavaScript client migrates on GOAWAY with a handover and the guarded redirect the Rust client already has, and the Rust drain path gets its regression test +- [Drain exit](/quest/m1/drain/drain-exit.md) - a drain ends as soon as every + session has left, and reports whether that or the deadline ended it ## Related diff --git a/quest/m1/drain/drain-exit.md b/quest/m1/drain/drain-exit.md new file mode 100644 index 0000000000..5917a476dd --- /dev/null +++ b/quest/m1/drain/drain-exit.md @@ -0,0 +1,19 @@ +# [S] Drain exit + +## Goal + +`Relay::run` returns as soon as every session has left a drain, instead of +always waiting out the window, and the relay reports which bound ended it: +every session left, or the deadline force-closed the stragglers (and how +many). An orchestrator bounding its stop time can then prove from the log and +`/metrics` which one it hit. + +## Plan + +Today `drain` in `rs/moq-relay/src/relay.rs` sleeps the whole window plus a +second whatever the sessions do. Counting only needs the sessions that go +through `shutdown::Observer::drain_session` (QUIC on either runtime, and +WebSocket), not the `session::Registry`, which skips LAN peers. + +Open question: whether the relay's own outbound cluster sessions count, since +they are not drained by GOAWAY at all. diff --git a/quest/m1/drain/relay-drain-api.md b/quest/m1/drain/relay-drain-api.md deleted file mode 100644 index 7ff107aa3f..0000000000 --- a/quest/m1/drain/relay-drain-api.md +++ /dev/null @@ -1,22 +0,0 @@ -# [M] Relay drain api - -## Goal - -Expose a drain hook that sends GOAWAY on every relay session. Include new -arrivals while draining, so an embedding process can trigger it on SIGTERM. - -Identical behavior on both runtimes: a session served from an io_uring worker -drains the same as one served from a tokio worker. - -## Plan - -moq.pro's (downstream) fleet drain orchestration consumes this hook: its edge -process sheds the node from GeoDNS, waits out the TTL, then fires the drain. - -The signal path already reaches io_uring sessions: `drain_on_signal` fires the -broadcast, per-session `supervise` sends the GOAWAY from the shared runtime, -and `uring::Workers::shutdown` joins the threads only afterwards. What is -missing on both runtimes is the arrival half, and on the io_uring side nothing -proves any of it. Cover the new-arrival GOAWAY on the io_uring accept path -too, and add the drain case to `rs/moq-relay/tests/runtime_uring.rs`: a -session on a worker receives GOAWAY, closes, and the threads join cleanly. diff --git a/rs/moq-relay/src/relay.rs b/rs/moq-relay/src/relay.rs index ba04e67570..953111b772 100644 --- a/rs/moq-relay/src/relay.rs +++ b/rs/moq-relay/src/relay.rs @@ -84,6 +84,9 @@ pub struct Relay { addr: Option, shutdown: shutdown::Observer, shutdown_trigger: shutdown::Trigger, + /// Whether [`Self::run`] drains on SIGINT/SIGTERM itself, or leaves that to + /// the embedder firing [`Self::shutdown_trigger`]. + signals: bool, /// Replacement for the default public router. `None` serves [`web::Web::routes`]. web_routes: Option, /// Replacement for the default ops router. `None` serves [`internal::Internal::routes`]. @@ -315,6 +318,7 @@ impl Relay { addr, shutdown, shutdown_trigger, + signals: true, web_routes: None, internal_routes: None, sessions, @@ -389,9 +393,9 @@ impl Relay { &self.shutdown } - /// Starts graceful shutdown: every session drains with a GOAWAY and - /// [`Self::run`] returns once the drain window elapses. Clone it before - /// `run` consumes the relay. + /// Starts graceful shutdown: every session, including any accepted + /// afterwards, drains with a GOAWAY and [`Self::run`] returns once the drain + /// window elapses. Clone it before `run` consumes the relay. pub fn shutdown_trigger(&self) -> &shutdown::Trigger { &self.shutdown_trigger } @@ -447,11 +451,20 @@ impl Relay { self } + /// Whether [`Self::run`] starts the drain on SIGINT/SIGTERM. Defaults to + /// `true`; pass `false` when the application owns the signals and fires + /// [`Self::shutdown_trigger`] itself, e.g. after withdrawing the node from DNS. + #[must_use = "the relay with the signal choice is returned"] + pub fn with_signals(mut self, signals: bool) -> Self { + self.signals = signals; + self + } + /// Serve until something fails or shutdown completes: accept sessions, run /// the cluster, and serve both HTTP surfaces. Notifies systemd once /// everything is up. Returns once the drain window elapses after a signal - /// or [`shutdown::Trigger::start`], with every listener released and every - /// worker joined. + /// (see [`Self::with_signals`]) or [`shutdown::Trigger::start`], with every + /// listener released and every worker joined. /// /// This is also the embedding loop. Extra routes go on via [`Self::with_web`] /// / [`Self::with_internal`] before calling this; cloned handles outlive it. @@ -466,6 +479,7 @@ impl Relay { web, shutdown, shutdown_trigger, + signals, web_routes, internal_routes, sessions, @@ -621,7 +635,7 @@ impl Relay { Err(err) = quic_workers => Err(err).context("QUIC workers failed"), err = uring_failed => Err(err).context("io_uring QUIC workers failed"), Err(err) = jemalloc => Err(err).context("jemalloc profiler failed"), - res = drain(shutdown_trigger, shutdown.clone()) => res, + res = drain(shutdown_trigger, shutdown.clone(), signals) => res, else => Ok(()), }; @@ -642,12 +656,19 @@ impl Relay { /// Two-stage shutdown: the first signal, or an embedder firing /// [`shutdown::Trigger::start`], starts the drain broadcast (every session sends -/// GOAWAY and waits for its peer to leave); a second signal, or the drain -/// window elapsing, returns from [`Relay::run`]. -async fn drain(trigger: shutdown::Trigger, mut shutdown: shutdown::Observer) -> anyhow::Result<()> { +/// GOAWAY and waits for its peer to leave); a second signal, or that recorded +/// deadline plus one second, returns from [`Relay::run`]. Without `signals` +/// only the trigger and that deadline count. +async fn drain(trigger: shutdown::Trigger, mut shutdown: shutdown::Observer, signals: bool) -> anyhow::Result<()> { let window = shutdown.drain_timeout; + let signal = || async move { + match signals { + true => shutdown_signal().await, + false => std::future::pending().await, + } + }; tokio::select! { - res = shutdown_signal() => { + res = signal() => { res?; tracing::info!( ?window, @@ -658,11 +679,15 @@ async fn drain(trigger: shutdown::Trigger, mut shutdown: shutdown::Observer) -> _ = shutdown.started() => tracing::info!(?window, "shutdown requested; draining sessions"), } - // One extra second past the window so per-session force-closes fire first, - // giving every peer a proper GoawayTimeout instead of a dropped transport. - let grace = window + std::time::Duration::from_secs(1); + // One extra second past the deadline fixed when the trigger fired, so + // per-session force-closes fire first. That instant may be earlier than + // this future was polled (the embedder can start the drain during startup), + // and a fresh window here would keep the process up past the time sessions + // were told. + let deadline = shutdown.deadline().context("drain started without a deadline")?; + let grace = (deadline + std::time::Duration::from_secs(1)).saturating_duration_since(std::time::Instant::now()); tokio::select! { - res = shutdown_signal() => { + res = signal() => { res?; tracing::warn!("second shutdown signal; exiting immediately"); } diff --git a/rs/moq-relay/src/shutdown.rs b/rs/moq-relay/src/shutdown.rs index f7c309c9a1..a1987c104e 100644 --- a/rs/moq-relay/src/shutdown.rs +++ b/rs/moq-relay/src/shutdown.rs @@ -1,8 +1,12 @@ //! Graceful shutdown coordination: the first shutdown signal fires a broadcast //! that every accepted session observes, draining it with a GOAWAY before the //! process exits. +//! +//! The drain has one deadline, fixed when it starts. A session accepted after +//! that (a cached DNS resolve, a pool alias) is sent a GOAWAY at once, carrying +//! only the time left, so no session outlives the window the relay promised. -use std::time::Duration; +use std::time::{Duration, Instant}; use tokio::sync::watch; @@ -10,14 +14,23 @@ use tokio::sync::watch; /// signal path; an embedder clones one to stop the relay from its own task. #[derive(Clone)] pub struct Trigger { - tx: watch::Sender, + tx: watch::Sender>, + drain_timeout: Duration, } impl Trigger { /// Start the drain: every [`Observer`] handle's [`started`](Observer::started) - /// resolves and sessions begin sending GOAWAY. + /// resolves and sessions begin sending GOAWAY, including any accepted later. + /// Calling it again keeps the first deadline. pub fn start(&self) { - let _ = self.tx.send(true); + let deadline = Instant::now() + self.drain_timeout; + self.tx.send_if_modified(|started| { + let first = started.is_none(); + if first { + *started = Some(deadline); + } + first + }); } } @@ -27,7 +40,7 @@ impl Trigger { /// and drains itself via [`drain_session`](Self::drain_session) when it fires. #[derive(Clone)] pub struct Observer { - rx: watch::Receiver, + rx: watch::Receiver>, /// How long a drained session may keep running before it is force-closed. pub drain_timeout: Duration, } @@ -35,16 +48,16 @@ pub struct Observer { impl Observer { /// Create the trigger and its observer half. pub fn new(drain_timeout: Duration) -> (Trigger, Self) { - let (tx, rx) = watch::channel(false); - (Trigger { tx }, Self { rx, drain_timeout }) + let (tx, rx) = watch::channel(None); + (Trigger { tx, drain_timeout }, Self { rx, drain_timeout }) } /// A handle that never fires, for callers without shutdown coordination /// (tests, embedders that manage their own lifecycle). pub fn disabled() -> Self { - let (tx, rx) = watch::channel(false); + let (tx, rx) = watch::channel(None); // Leak-free: dropping the sender doesn't resolve `started` (it waits for - // a `true` value, not for channel closure). + // a deadline, not for channel closure). drop(tx); Self { rx, @@ -52,32 +65,43 @@ impl Observer { } } - /// Resolve once the shutdown broadcast fires. Never resolves for - /// [`disabled`](Self::disabled) handles. + /// Resolve once the shutdown broadcast fires, at once if it already has. + /// Never resolves for [`disabled`](Self::disabled) handles. pub async fn started(&mut self) { // wait_for returns Err once the sender is dropped without firing; park // forever in that case (a dropped trigger means no shutdown, not shutdown). - if self.rx.wait_for(|started| *started).await.is_err() { + if self.rx.wait_for(Option::is_some).await.is_err() { std::future::pending::<()>().await; } } + /// When the drain window ends, if [`Trigger::start`] has fired. + pub(crate) fn deadline(&self) -> Option { + *self.rx.borrow() + } + /// Drain `session` with an empty-URI GOAWAY ("reconnect to me"), waiting for /// the peer to leave. /// - /// The driver force-closes with [`moq_net::Error::GoawayTimeout`] once - /// [`drain_timeout`](Self::drain_timeout) passes, so this resolves either way, - /// on every version. A peer too old for GOAWAY (moq-lite-03 and earlier) keeps - /// being served for the same window and is then closed the same way; it simply - /// never learns why, which beats cutting it off with no warning and no grace. + /// The driver force-closes with [`moq_net::Error::GoawayTimeout`] at the + /// drain's deadline: [`drain_timeout`](Self::drain_timeout) after the trigger + /// fired, or after this call if it has not. So this resolves either way, on + /// every version. A peer too old for GOAWAY (moq-lite-03 and earlier) keeps + /// being served until then and is closed the same way; it simply never learns + /// why, which beats cutting it off with no warning and no grace. /// - /// A zero [`drain_timeout`](Self::drain_timeout) means no grace at all: the - /// session is closed at once. + /// With no time left (a zero [`drain_timeout`](Self::drain_timeout), or a + /// session accepted after the deadline) the session is closed at once. pub async fn drain_session(&self, session: &moq_net::Session) { - // No grace window, so there is nothing to drain. Close now rather than send a + let remaining = match *self.rx.borrow() { + Some(deadline) => deadline.saturating_duration_since(Instant::now()), + None => self.drain_timeout, + }; + + // No grace left, so there is nothing to drain. Close now rather than send a // GOAWAY: the peer would have no time to act on it, and a zero timeout means // "no deadline" on the wire, which is the opposite of what was asked for. - if self.drain_timeout.is_zero() { + if remaining.is_zero() { session.abort(moq_net::Error::GoingAway); return; } @@ -87,7 +111,7 @@ impl Observer { // is reachable; the deadline closes the session regardless. if let Err(err) = session .drain() - .send(moq_net::goaway::Goaway::new().with_timeout(self.drain_timeout)) + .send(moq_net::goaway::Goaway::new().with_timeout(remaining)) { tracing::warn!(%err, "failed to drain session"); } diff --git a/rs/moq-relay/tests/runtime_uring.rs b/rs/moq-relay/tests/runtime_uring.rs index 6fbebee5c8..e37fa835ed 100644 --- a/rs/moq-relay/tests/runtime_uring.rs +++ b/rs/moq-relay/tests/runtime_uring.rs @@ -245,6 +245,74 @@ async fn uring_workers_report_link_facts() { let _ = running.await; } +/// The shutdown trigger drains sessions the io_uring workers serve as it does +/// the shared runtime's: an established session and one arriving mid-drain +/// are each sent a GOAWAY and leave, and `run` then returns with the worker +/// threads joined and the port free. +/// +/// What an arrival is told is left of the window only reaches the wire on +/// moq-transport-17+, which the workers do not speak; `shutdown_signal.rs` +/// reads it through the shared runtime, whose supervision this path shares. +#[tokio::test] +async fn uring_workers_drain_on_the_trigger() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + if !supported() { + return; + } + + const DRAIN: Duration = Duration::from_secs(2); + + let dir = tempfile::tempdir().expect("tempdir"); + let (cert, key) = certificate(dir.path()); + let port = free_udp_port(); + + let mut config = uring_config(&cert, &key, port); + config.drain_timeout = DRAIN; + let relay = Relay::load(config).await.expect("load relay").with_signals(false); + let trigger = relay.shutdown_trigger().clone(); + let running = tokio::spawn(relay.run()); + + // One-shot (see `client`), so a session leaves on its GOAWAY rather than + // migrating, and closing cleanly shows it was one. + let client = client(); + let url: url::Url = format!("moql://127.0.0.1:{port}/drain").parse().expect("parse url"); + + let established = connect(client.clone(), url.clone()).await; + trigger.start(); + let goaway = tokio::time::timeout(TIMEOUT, established.draining().expect("connected").recv()) + .await + .expect("no GOAWAY after the trigger") + .expect("session closed without a GOAWAY"); + assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); + tokio::time::timeout(TIMEOUT, established.closed()) + .await + .expect("the drained session never closed") + .expect("a one-shot session leaves cleanly on GOAWAY"); + + // A straggler dialing mid-drain is admitted through a worker, then told to + // leave at once. + tokio::time::sleep(DRAIN / 2).await; + let arrival = connect(client, url).await; + let goaway = tokio::time::timeout(Duration::from_secs(1), arrival.draining().expect("connected").recv()) + .await + .expect("an arrival mid-drain was not sent a GOAWAY") + .expect("arrival closed without a GOAWAY"); + assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); + tokio::time::timeout(TIMEOUT, arrival.closed()) + .await + .expect("the arrival never closed") + .expect("a one-shot session leaves cleanly on GOAWAY"); + + // `run` joins the worker threads before returning, so returning at all is + // the clean join; the rebind proves every worker let go of the port. + tokio::time::timeout(TIMEOUT, running) + .await + .expect("run did not return after the drain window") + .expect("run panicked") + .expect("run returned an error after the drain"); + UdpSocket::bind(("127.0.0.1", port)).expect("a worker still holds the QUIC port"); +} + /// One HTTP/1.1 GET against `addr`, returning the response body. /// /// Hand-rolled because the relay has no HTTP client among its dev diff --git a/rs/moq-relay/tests/shutdown_signal.rs b/rs/moq-relay/tests/shutdown_signal.rs index 23cf66e264..b23ae043ea 100644 --- a/rs/moq-relay/tests/shutdown_signal.rs +++ b/rs/moq-relay/tests/shutdown_signal.rs @@ -11,6 +11,15 @@ //! against the drain future that deliberately waits out the window. SIGTERM was //! unaffected (only the drain future watches it), which is how the gap stayed //! hidden behind systemd while an operator's ctrl-C dropped every session. +//! +//! An embedder draining on its own schedule (withdraw from DNS, wait out the +//! TTL, then drain) turns the relay's signal handling off and fires the trigger +//! itself; a session that still arrives mid-drain is sent a GOAWAY at once, +//! carrying only what is left of the window. +//! +//! Each signal test raises or handles process signals, so they rely on +//! nextest's process-per-test isolation. `a_trigger_before_run_keeps_the_deadline` +//! fires the trigger before `run` instead of a signal. #![cfg(unix)] @@ -23,25 +32,45 @@ use moq_relay::{Config, Relay, auth}; /// one second before exiting. const DRAIN_TIMEOUT: Duration = Duration::from_secs(3); -#[test] -fn sigint_drains_sessions_before_exiting() { +/// Run `test` on a current-thread runtime with a large stack. +fn run_test + 'static>(test: fn() -> F) { // Same reason as the cluster tests: under `--all-features` a `Connection` // carries every transport backend, and holding one across awaits overflows // libtest's 2 MiB per-test stack in an unoptimized build. std::thread::Builder::new() .stack_size(32 * 1024 * 1024) - .spawn(|| { + .spawn(move || { tokio::runtime::Builder::new_current_thread() .enable_all() .build() .expect("build test runtime") - .block_on(sigint_drains_sessions_before_exiting_inner()); + .block_on(test()); }) .expect("spawn test thread") .join() .expect("test thread panicked"); } +#[test] +fn sigint_drains_sessions_before_exiting() { + run_test(sigint_drains_sessions_before_exiting_inner); +} + +#[test] +fn an_embedder_owns_the_signals() { + run_test(an_embedder_owns_the_signals_inner); +} + +#[test] +fn a_session_arriving_mid_drain_gets_what_is_left() { + run_test(a_session_arriving_mid_drain_gets_what_is_left_inner); +} + +#[test] +fn a_trigger_before_run_keeps_the_deadline() { + run_test(a_trigger_before_run_keeps_the_deadline_inner); +} + async fn sigint_drains_sessions_before_exiting_inner() { let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); @@ -55,19 +84,9 @@ async fn sigint_drains_sessions_before_exiting_inner() { let relay = Relay::load(config).await.expect("load relay"); let run = tokio::spawn(relay.run()); wait_listening(port).await; + let client = client(Vec::new()); - let mut client_config = moq_tokio::connect::Config::default(); - client_config.tls.insecure = Some(true); - let client = client_config.init(Default::default()).expect("client init"); - // One-shot: a reconnecting client would migrate on the GOAWAY and hide whether - // the original session outlived the signal. - let url: url::Url = format!("tcp://127.0.0.1:{port}/").parse().expect("parse url"); - let connection = client - .with_reconnect(false) - .connect(url) - .established() - .await - .expect("connect"); + let connection = connect(&client, port).await; let draining = connection.draining().expect("connected"); let signalled = std::time::Instant::now(); @@ -106,6 +125,162 @@ async fn sigint_drains_sessions_before_exiting_inner() { ); } +async fn an_embedder_owns_the_signals_inner() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + // The embedder's own handler, which is also what keeps SIGINT from killing + // the test process. + let mut interrupt = + tokio::signal::unix::signal(tokio::signal::unix::SignalKind::interrupt()).expect("register SIGINT"); + + let (port, config) = relay_config(); + let relay = Relay::load(config).await.expect("load relay").with_signals(false); + let trigger = relay.shutdown_trigger().clone(); + let run = tokio::spawn(relay.run()); + wait_listening(port).await; + let client = client(Vec::new()); + + let connection = connect(&client, port).await; + let draining = connection.draining().expect("connected"); + + // SAFETY: `raise` is async-signal-safe, and SIGINT's disposition is tokio's + // handler, registered above. + assert_eq!(unsafe { libc::raise(libc::SIGINT) }, 0, "failed to raise SIGINT"); + tokio::time::timeout(Duration::from_secs(5), interrupt.recv()) + .await + .expect("the embedder never saw SIGINT"); + + // The signal is the embedder's: the relay keeps serving without a GOAWAY. + assert!( + tokio::time::timeout(Duration::from_secs(1), draining.recv()) + .await + .is_err(), + "the relay drained on a signal the embedder owns" + ); + assert!(!run.is_finished(), "the relay exited on a signal the embedder owns"); + + // Until the embedder decides. + let started = std::time::Instant::now(); + trigger.start(); + let goaway = tokio::time::timeout(Duration::from_secs(5), draining.recv()) + .await + .expect("no GOAWAY within 5s of the trigger") + .expect("session closed without a GOAWAY"); + assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); + + tokio::time::timeout(Duration::from_secs(15), run) + .await + .expect("relay never exited after the drain window") + .expect("relay task panicked") + .expect("relay exited with an error"); + assert!( + started.elapsed() >= DRAIN_TIMEOUT, + "relay exited after {:?}, short of the {DRAIN_TIMEOUT:?} drain window", + started.elapsed() + ); +} + +async fn a_session_arriving_mid_drain_gets_what_is_left_inner() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let (port, config) = relay_config(); + let relay = Relay::load(config).await.expect("load relay").with_signals(false); + let trigger = relay.shutdown_trigger().clone(); + let run = tokio::spawn(relay.run()); + wait_listening(port).await; + // moq-transport-17 is the first version whose GOAWAY carries its deadline on + // the wire, which is what this test reads. + let client = client(vec!["moq-transport-17".parse().expect("parse version")]); + + let established = connect(&client, port).await; + trigger.start(); + let deadline = std::time::Instant::now() + DRAIN_TIMEOUT; + let goaway = tokio::time::timeout( + Duration::from_secs(5), + established.draining().expect("connected").recv(), + ) + .await + .expect("no GOAWAY within 5s of the trigger") + .expect("session closed without a GOAWAY"); + assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); + let timeout = goaway.timeout().expect("the GOAWAY carries its deadline"); + assert!( + timeout > DRAIN_TIMEOUT / 2 && timeout <= DRAIN_TIMEOUT, + "an established session gets the whole window, got {timeout:?}" + ); + + // A straggler dialing mid-drain (a cached DNS resolve) is still admitted, + // then told to leave at once. + tokio::time::sleep(DRAIN_TIMEOUT / 2).await; + let dialed = std::time::Instant::now(); + let arrival = connect(&client, port).await; + let goaway = tokio::time::timeout(Duration::from_secs(1), arrival.draining().expect("connected").recv()) + .await + .expect("an arrival mid-drain was not sent a GOAWAY") + .expect("arrival closed without a GOAWAY"); + assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); + + // Only what is left of the window, not a fresh one that would outlive the + // relay. The wire rounds up to whole milliseconds. + let left = deadline.saturating_duration_since(dialed) + Duration::from_millis(1); + let timeout = goaway.timeout().expect("the GOAWAY carries its deadline"); + assert!( + timeout <= left, + "an arrival mid-drain got {timeout:?}, past the {left:?} left of the window" + ); + + tokio::time::timeout(Duration::from_secs(15), run) + .await + .expect("relay never exited after the drain window") + .expect("relay task panicked") + .expect("relay exited with an error"); +} + +async fn a_trigger_before_run_keeps_the_deadline_inner() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let (_port, config) = relay_config(); + let relay = Relay::load(config).await.expect("load relay").with_signals(false); + let trigger = relay.shutdown_trigger().clone(); + + // Fired while `run` is still starting. The session deadline is this instant; + // `drain` must not open a fresh window when it finally observes the watch. + trigger.start(); + let deadline = std::time::Instant::now() + DRAIN_TIMEOUT; + tokio::time::sleep(DRAIN_TIMEOUT / 2).await; + + let run = tokio::spawn(relay.run()); + tokio::time::timeout(Duration::from_secs(15), run) + .await + .expect("relay never exited") + .expect("relay task panicked") + .expect("relay exited with an error"); + let over = std::time::Instant::now().saturating_duration_since(deadline); + assert!( + over <= Duration::from_secs(2), + "run returned {over:?} past the recorded deadline, not the one-second grace" + ); +} + +/// A one-shot client offering `version` (every version when empty): a +/// reconnecting one would migrate on the GOAWAY and hide whether the relay +/// closed the original session. +fn client(version: Vec) -> moq_tokio::Client { + let mut client_config = moq_tokio::connect::Config::default(); + client_config.tls.insecure = Some(true); + client_config.version = version; + client_config + .init(Default::default()) + .expect("client init") + .with_reconnect(false) +} + +/// A session to the relay on `port`. +async fn connect(client: &moq_tokio::Client, port: u16) -> moq_tokio::Connection { + let url: url::Url = format!("tcp://127.0.0.1:{port}/").parse().expect("parse url"); + client.connect(url).established().await.expect("connect") +} + /// A stream-only relay on a free loopback TCP port, fully public, with a short /// drain window. Returns the port and the config to hand [`Relay::load`]. fn relay_config() -> (u16, Config) { From 4c440096699fb34bed0c0234ea65f7b7985e8d2b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 25 Sep 2026 12:19:59 -0700 Subject: [PATCH 03/15] feat(net): migrate the JS connection on GOAWAY; refuse bad redirects in Rust (#4143) Co-authored-by: Claude Opus 5.5 Co-authored-by: Grok 4.7 --- doc/bin/relay/config.md | 8 +- doc/lib/js/net.md | 2 +- js/net/src/connection/forward.test.ts | 3 +- js/net/src/connection/goaway.test.ts | 110 ++++++++++ js/net/src/connection/goaway.ts | 224 ++++++++++++++++++++ js/net/src/connection/migrate.test.ts | 277 +++++++++++++++++++++++++ js/net/src/connection/pool.ts | 50 ++++- js/net/src/connection/reload.ts | 222 +++++++++++++++++--- js/net/src/error.ts | 15 ++ js/net/src/errors.ts | 1 + js/net/src/ietf/adapter.test.ts | 49 +++++ js/net/src/ietf/adapter.ts | 22 +- js/net/src/ietf/connection.ts | 39 +++- js/net/src/ietf/goaway.test.ts | 82 ++++++++ js/net/src/ietf/goaway.ts | 23 ++- js/net/src/lite/connection.test.ts | 45 +++- js/net/src/lite/connection.ts | 18 +- js/net/src/origin.test.ts | 46 +++++ js/net/src/origin.ts | 38 +++- js/net/src/wire.ts | 7 +- quest/m1/drain/README.md | 24 +-- quest/m1/drain/client-goaway.md | 87 -------- quest/m1/transport-upgrade/README.md | 7 +- quest/m1/transport-upgrade/js.md | 14 +- rs/moq-ffi/src/binary.rs | 6 +- rs/moq-tokio/src/client.rs | 4 + rs/moq-tokio/src/connection.rs | 287 ++++++++++++++++++++++---- rs/moq-tokio/src/error.rs | 5 + rs/moq-tokio/src/resolve.rs | 31 +++ rs/moq-tokio/tests/reconnect.rs | 55 +++-- 30 files changed, 1561 insertions(+), 240 deletions(-) create mode 100644 js/net/src/connection/goaway.test.ts create mode 100644 js/net/src/connection/goaway.ts create mode 100644 js/net/src/connection/migrate.test.ts create mode 100644 js/net/src/ietf/goaway.test.ts delete mode 100644 quest/m1/drain/client-goaway.md diff --git a/doc/bin/relay/config.md b/doc/bin/relay/config.md index 27563226ae..c2588abc45 100644 --- a/doc/bin/relay/config.md +++ b/doc/bin/relay/config.md @@ -160,9 +160,11 @@ A draining upstream may name a replacement URI. `same-host` follows it only onto the host we already dialed, so a peer moves us between ports and schemes; `follow` also lets it choose the host, which means trusting it not to point us into the local network, since a name it controls resolves wherever it likes; -`ignore` keeps the current address list. Empty, malformed, or refused redirects -also preserve caller-configured fallbacks; only an accepted redirect replaces -the list with the peer's URI. `handover` is a cap: a shorter deadline on +`ignore` keeps the current address list. An empty URI also keeps it, including +caller-configured fallbacks; an accepted redirect replaces the list with the +peer's URI. A malformed or refused redirect ends the connection with an error +rather than redialing the old address or a fallback, and so does one leaving +the host a `tls.fingerprint` pin verifies. `handover` is a cap: a shorter deadline on the received GOAWAY wins, a longer one does not extend it. ## \[cache] diff --git a/doc/lib/js/net.md b/doc/lib/js/net.md index f086c11cd9..7abdc2e542 100644 --- a/doc/lib/js/net.md +++ b/doc/lib/js/net.md @@ -48,7 +48,7 @@ for (;;) { ``` - **Origins** hold the broadcasts, not the connection: closing a session unannounces them but leaves them created for the next one. `origin.request(path)` resolves an announced local broadcast with no round trip, so a page that watches what it publishes reads its own copy, unless a cheaper route announces the same path. Create, populate, then `announce()` for an exact path; use `dynamic(prefix, route)` when the set of paths is not known: an exact-path subscribe before the tracks exist is refused, and nobody, local or remote, can see or reach a broadcast until it announces. -- **Connections** race WebTransport against WebSocket. `new Connection({ url })` pools one connection per relay URL and reconnects with backoff, which the elements use. Supplying WebTransport/WebSocket options, discovery, delay, or a caller-owned origin selects a private loop; explicit `share: true` refuses those options. `closed` settles when the handle is released (`null` on a clean close); the failure that stopped retrying the current URL is `error`, and a new URL recovers the same handle. A connection owns one send-rate sampler and one `Bandwidth.Allocator`; publishers reserve against it so their encoder targets sum to the estimate instead of each matching it. +- **Connections** race WebTransport against WebSocket. `new Connection({ url })` pools one connection per relay URL and reconnects with backoff, which the elements use. Supplying WebTransport/WebSocket options, discovery, delay, `goaway`, or a caller-owned origin selects a private loop; explicit `share: true` refuses those options. On a relay's GOAWAY the connection dials the replacement at once while the old session keeps serving its groups until it closes or the `goaway.handover` cap (default 10s, lowered to the relay's deadline) passes; the handle and its origin stay the same. An empty GOAWAY redials the same URL through a fresh DNS resolve. A redirect is followed onto the same host by default (`goaway.redirect: "follow"` lets the relay pick the host, `"ignore"` never moves), and moves the pooled entry to the new URL; a malformed or refused one, including a host change under a pinned certificate, ends the connection with `Error.RefusedRedirect` instead of reconnecting. `closed` settles when the handle is released (`null` on a clean close); the failure that stopped retrying the current URL is `error`, and a new URL recovers the same handle. A connection owns one send-rate sampler and one `Bandwidth.Allocator`; publishers reserve against it so their encoder targets sum to the estimate instead of each matching it. - **Bandwidth** (`Bandwidth.Allocator`) divides the connection's send-rate estimate by track priority, max-min fair within a tier. An idle track claims nothing. The receive side is untouched. - **Discovery** by any pattern scope (`origin.announced(scope)`, such as `room/*/chat`; default everything). Each event's `prefix` is the covered prefix relative to the origin, `captures` reports what the scope's wildcards matched when the prefix pins them, and `kind` says whether it was announced, updated, or retracted. The consumer is an async iterable. `origin.broadcasts(scope)` is a live `Getter>` of the same covered prefixes for UIs that need the current set. A borrowed `Connection.origin` also exposes `dynamic(prefix, route)` for serving paths on demand. - **Subscriptions** carry a priority, a `Time.Milli` max age, and optional `groups` bounds. Groups arrive out of order and are read frame by frame, with `Error.TooFarBehind` when a reader asks for a frame the group never held and `Error.GroupTooLarge` when a write exceeds the cache budget and aborts the group. diff --git a/js/net/src/connection/forward.test.ts b/js/net/src/connection/forward.test.ts index d9d9b74481..6039d312a8 100644 --- a/js/net/src/connection/forward.test.ts +++ b/js/net/src/connection/forward.test.ts @@ -1,4 +1,5 @@ import { expect, test } from "bun:test"; +import { Once } from "@moq/signals"; import * as Announce from "../announced.ts"; import { type Consumer as BroadcastConsumer, Producer as BroadcastProducer } from "../broadcast.ts"; import { Route } from "../hop.ts"; @@ -33,7 +34,7 @@ class FakeSession { constructor(discovery = true) { this.discovery = discovery; - registerWire(this, { consume: (path) => this.consume(path) }); + registerWire(this, { consume: (path) => this.consume(path), goaway: new Once() }); this.closed = new Promise((resolve) => { this.#die = resolve; }); diff --git a/js/net/src/connection/goaway.test.ts b/js/net/src/connection/goaway.test.ts new file mode 100644 index 0000000000..fb99bbe1c2 --- /dev/null +++ b/js/net/src/connection/goaway.test.ts @@ -0,0 +1,110 @@ +import { expect, test } from "bun:test"; +import { RefusedRedirect } from "../error.ts"; +import * as Time from "../time.ts"; +import { dialed, handover, isLocal, pinnedTransport, type Redirect, target } from "./goaway.ts"; + +const current = new URL("https://relay.example/room?jwt=secret"); + +test("no URI, or a policy that ignores it, keeps the current URL", () => { + expect(target("follow", "", current, false)).toBeUndefined(); + expect(target("same-host", "", current, false)).toBeUndefined(); + expect(target("ignore", "https://other.example/", current, false)).toBeUndefined(); +}); + +test("an explicit URI the policy will not follow is refused, not ignored", () => { + const refused: [Redirect, string][] = [ + ["same-host", "https://other.example/"], + ["follow", "not a url"], + ["ignore", "not a url"], + ["follow", "http://relay.example/"], + ["follow", "unix:///tmp/moq.sock"], + ["follow", "https://127.0.0.1/"], + ["follow", "https://[::ffff:127.0.0.1]/"], + ["follow", "https://[::ffff:7f00:1]/"], + ["follow", "moqt://169.254.169.254/"], + ]; + for (const [policy, uri] of refused) { + expect(() => target(policy, uri, current, false), `${policy}: ${uri}`).toThrow(RefusedRedirect); + } +}); + +test("a refusal never repeats the URI, which can carry credentials", () => { + try { + target("same-host", "https://other.example/?jwt=leaked", current, false); + throw new Error("not refused"); + } catch (err) { + expect(err).toBeInstanceOf(RefusedRedirect); + expect(String(err)).not.toContain("leaked"); + } +}); + +test("same-host follows a port or scheme move on the host already dialed", () => { + expect(target("same-host", "https://relay.example:5443/", current, false)?.port).toBe("5443"); + // An explicit assignment is still one when it names the current URL. + expect(target("same-host", current.href, current, false)?.href).toBe(current.href); + // An upgrade is fine; only a downgrade is refused. + const plain = new URL("http://relay.example/"); + expect(target("same-host", "https://relay.example/", plain, false)?.protocol).toBe("https:"); +}); + +test("follow lets the peer name another public host", () => { + expect(target("follow", "https://other.example/next", current, false)?.href).toBe("https://other.example/next"); +}); + +test("a local endpoint may redirect to another local one", () => { + const local = new URL("https://localhost:4443/"); + expect(target("follow", "https://127.0.0.1:9999/", local, false)?.port).toBe("9999"); +}); + +test("a certificate pin holds the host even under follow", () => { + expect(() => target("follow", "https://other.example/", current, true)).toThrow(RefusedRedirect); + expect(target("follow", "https://relay.example:5443/", current, true)?.port).toBe("5443"); +}); + +test("local literals are recognized in every spelling", () => { + const local = [ + "https://127.0.0.1/", + "https://localhost/", + "https://a.localhost/", + "https://[::1]/", + "https://[::]/", + "https://10.0.0.1/", + "https://172.16.0.1/", + "https://192.168.1.1/", + "https://169.254.1.1/", + "https://0.0.0.0/", + "https://[::ffff:10.0.0.1]/", + "https://[fe80::1]/", + "https://[fc00::1]/", + "moqt://127.0.0.1/", + "moqt://[::ffff:127.0.0.1]/", + "unix:///tmp/moq.sock", + ]; + for (const url of local) expect(isLocal(new URL(url)), url).toBe(true); + + for (const url of ["https://example.com/", "https://8.8.8.8/", "https://172.32.0.1/", "https://[2606:4700::1]/"]) { + expect(isLocal(new URL(url)), url).toBe(false); + } +}); + +test("a WebSocket fallback that connected is the host a redirect is judged against", () => { + const primary = new URL("https://relay.example/"); + const socket = new URL("wss://edge.example/moq"); + expect(dialed(primary, "websocket", socket).href).toBe(socket.href); + expect(dialed(primary, "webtransport", socket).href).toBe(primary.href); + expect(dialed(primary, "websocket").href).toBe(primary.href); +}); + +test("a certificate pin holds only the WebTransport session that used it", () => { + expect(pinnedTransport("webtransport", true)).toBe(true); + expect(pinnedTransport("websocket", true)).toBe(false); + expect(pinnedTransport("webtransport", false)).toBe(false); +}); + +test("the handover is the cap, lowered only by a positive peer deadline", () => { + const cap = Time.Milli(10_000); + expect(handover(cap)).toBe(cap); + expect(handover(cap, Time.Milli(0))).toBe(cap); + expect(handover(cap, Time.Milli(3_000))).toBe(Time.Milli(3_000)); + expect(handover(cap, Time.Milli(3_600_000))).toBe(cap); +}); diff --git a/js/net/src/connection/goaway.ts b/js/net/src/connection/goaway.ts new file mode 100644 index 0000000000..a304e45838 --- /dev/null +++ b/js/net/src/connection/goaway.ts @@ -0,0 +1,224 @@ +/** + * GOAWAY handling for the reconnect loop: the peer's drain signal and the policy for the + * redirect it may name. Mirrors `Goaway` and `Redirect` in `moq_tokio::connection`. + * + * @module + */ +import { RefusedRedirect } from "../error.ts"; +import * as Time from "../time.ts"; + +/** A peer's GOAWAY as a session received it. */ +export interface Drain { + /** Where to reconnect, including any credentials it needs. Empty means the same endpoint. */ + readonly uri: string; + /** When the peer force-closes the session. Undefined when the wire carried none. */ + readonly timeout?: Time.Milli; +} + +/** + * What to do with the URI a peer names in its GOAWAY. + * + * - `same-host` (the default) follows it only onto the host already dialed, so a peer can move + * the connection between ports or schemes but not to another host. + * - `follow` also lets the peer name the host. The name is dialed as written, so only use it + * with a relay trusted not to point the connection into the local network. + * - `ignore` reconnects to the current URL whatever the peer names. + * + * A malformed or refused URI ends the connection with {@link RefusedRedirect}. + */ +export type Redirect = "follow" | "same-host" | "ignore"; + +/** How a reconnecting connection reacts to a peer's GOAWAY. */ +export interface GoawayProps { + /** What to do with the URI the peer names (default: `same-host`). */ + redirect?: Redirect; + + /** + * How long the old session keeps serving after the GOAWAY while the replacement dials + * (default: 10000ms). A cap: a shorter deadline from the peer wins, a longer one does not + * extend it. + */ + handover?: Time.Milli; +} + +/** The handover cap when the caller names none. */ +export const DEFAULT_HANDOVER = Time.Milli(10_000); + +/** + * How long a drained session may keep serving: the cap, lowered to the peer's deadline when + * it named a positive one. Absence is not a zero-length handover. + * + * @internal + */ +export function handover(cap: Time.Milli, timeout?: Time.Milli): Time.Milli { + return timeout !== undefined && timeout > 0 ? Time.Milli(Math.min(cap, timeout)) : cap; +} + +/** + * The URL a redirect is judged against. A WebSocket fallback that won the race is the host + * we dialed; the primary URL was not, so same-host must not treat it as the current peer. + * + * @internal + */ +export function dialed(primary: URL, transport: "webtransport" | "websocket", websocket?: URL): URL { + return transport === "websocket" && websocket ? websocket : primary; +} + +/** + * Whether a certificate pin constrains this session. The pin is a WebTransport option, so it + * holds the host only when that transport is the one that connected. + * + * @internal + */ +export function pinnedTransport(transport: "webtransport" | "websocket", configured: boolean): boolean { + return configured && transport === "webtransport"; +} + +/** + * The URL a GOAWAY assigns: `undefined` keeps the current URL (the peer named none, or the + * policy ignores a URI it could parse), and a URL replaces it. Throws {@link RefusedRedirect} + * for an explicit URI the policy will not follow, including a malformed one under `ignore`. + * + * `pinned` is a certificate pin on the connection, which can only verify the host it was + * configured for, so it refuses a host change even under `follow`. + * + * @internal + */ +export function target(policy: Redirect, uri: string, current: URL, pinned: boolean): URL | undefined { + if (uri === "") return undefined; + + // The URI can carry credentials, so the error names the reason, never the URI. + // Parse before `ignore`: a malformed redirect is terminal even when the policy + // would otherwise stay on the current URL. + let next: URL; + try { + next = new URL(uri); + } catch { + throw new RefusedRedirect("the GOAWAY URI is malformed"); + } + if (policy === "ignore") return undefined; + + if (schemeTier(next.protocol) < schemeTier(current.protocol)) { + throw new RefusedRedirect("the GOAWAY redirect downgrades the scheme"); + } + + // Only as far as the URL itself says: a name is dialed, never resolved here, so this + // refuses a peer naming a local address outright, not one hiding it behind a hostname. + // That gap is why `same-host` is the default. + if (isLocal(next) && !isLocal(current)) { + throw new RefusedRedirect("the GOAWAY redirect widens reachability to a local address"); + } + + // Host only, not the full authority: the port is what a peer legitimately moves us + // across when it hands off to a sibling process on the same box. + const sameHost = next.hostname === current.hostname; + if (policy === "same-host" && !sameHost) { + throw new RefusedRedirect("the GOAWAY redirect leaves the current host"); + } + if (pinned && !sameHost) { + throw new RefusedRedirect("the GOAWAY redirect leaves the host a certificate pin verifies"); + } + + return next; +} + +/** Rank a scheme so a redirect cannot silently drop encryption; unknown schemes rank lowest. */ +function schemeTier(protocol: string): number { + switch (protocol) { + case "https:": + case "wss:": + case "moqt:": + case "moql:": + return 2; + case "http:": + case "ws:": + case "tcp:": + return 1; + default: + return 0; + } +} + +/** + * Whether a URL says it names something only reachable from this host or network. A judgement + * about the URL, not about where a dial lands: `false` means "not local on its face". + * + * @internal + */ +export function isLocal(url: URL): boolean { + const host = url.hostname.toLowerCase(); + // No host at all, e.g. a `unix:` socket path. + if (host === "") return true; + if (host === "localhost" || host.endsWith(".localhost")) return true; + + if (host.startsWith("[") && host.endsWith("]")) { + const segments = parseIpv6(host.slice(1, -1)); + return segments !== undefined && isLocalV6(segments); + } + + const v4 = parseIpv4(host); + return v4 !== undefined && isLocalV4(v4); +} + +function parseIpv4(host: string): number[] | undefined { + const parts = host.split("."); + if (parts.length !== 4) return undefined; + const octets = parts.map((part) => (/^\d{1,3}$/.test(part) ? Number(part) : Number.NaN)); + return octets.every((octet) => octet <= 255) ? octets : undefined; +} + +function isLocalV4([a, b, c, d]: number[]): boolean { + return ( + a === 127 || + a === 10 || + (a === 172 && b >= 16 && b <= 31) || + (a === 192 && b === 168) || + (a === 169 && b === 254) || + (a === 0 && b === 0 && c === 0 && d === 0) + ); +} + +/** Parse an IPv6 literal (without brackets) into its eight 16-bit segments. */ +function parseIpv6(host: string): number[] | undefined { + // Drop a zone id; it scopes the address without changing it. + let text = host.split("%")[0] ?? ""; + + // A trailing dotted quad (`::ffff:127.0.0.1`) spells the last two segments. + const lastColon = text.lastIndexOf(":"); + const quad = parseIpv4(text.slice(lastColon + 1)); + if (quad) { + const [a = 0, b = 0, c = 0, d = 0] = quad; + const word = (hi: number, lo: number) => ((hi << 8) | lo).toString(16); + text = `${text.slice(0, lastColon + 1)}${word(a, b)}:${word(c, d)}`; + } + + const halves = text.split("::"); + if (halves.length > 2) return undefined; + const words = (part: string) => (part === "" ? [] : part.split(":")); + const head = words(halves[0] ?? ""); + const rest = halves.length === 2 ? words(halves[1] ?? "") : []; + const missing = 8 - head.length - rest.length; + if (halves.length === 2 ? missing < 0 : missing !== 0) return undefined; + + const all = [...head, ...Array(missing).fill("0"), ...rest]; + const segments = all.map((word) => (/^[0-9a-f]{1,4}$/i.test(word) ? Number.parseInt(word, 16) : Number.NaN)); + return segments.some(Number.isNaN) ? undefined : segments; +} + +function isLocalV6(segments: number[]): boolean { + // An IPv4-mapped address reaches the same host as the v4 it wraps. + if (segments.slice(0, 5).every((s) => s === 0) && segments[5] === 0xffff) { + const [hi = 0, lo = 0] = segments.slice(6); + return isLocalV4([hi >> 8, hi & 0xff, lo >> 8, lo & 0xff]); + } + const first = segments[0] ?? 0; + const zeroPrefix = segments.slice(0, 7).every((s) => s === 0); + const last = segments[7] ?? 0; + return ( + // Loopback (::1) and unspecified (::). + (zeroPrefix && (last === 1 || last === 0)) || + // Unique local (fc00::/7) and link local (fe80::/10). + (first & 0xfe00) === 0xfc00 || + (first & 0xffc0) === 0xfe80 + ); +} diff --git a/js/net/src/connection/migrate.test.ts b/js/net/src/connection/migrate.test.ts new file mode 100644 index 0000000000..a4af8d032f --- /dev/null +++ b/js/net/src/connection/migrate.test.ts @@ -0,0 +1,277 @@ +import { afterEach, expect, test } from "bun:test"; +import { RefusedRedirect } from "../error.ts"; +import * as Lite from "../lite/index.ts"; +import { createMockTransportPair } from "../mock.ts"; +import { Producer as OriginProducer } from "../origin.ts"; +import * as Path from "../path.ts"; +import { Stream } from "../stream.ts"; +import * as Time from "../time.ts"; +import { accept } from "./accept.ts"; +import { Connection, resetShared } from "./pool.ts"; +import { Reload } from "./reload.ts"; + +const original = globalThis.WebTransport; + +afterEach(() => { + resetShared(); + globalThis.WebTransport = original; +}); + +async function settle() { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +// Polls until `pred` holds, so a regression fails the test instead of hanging it. +async function waitUntil(pred: () => boolean, ms = 2000): Promise { + const deadline = Date.now() + ms; + for (;;) { + if (pred()) return; + if (Date.now() > deadline) throw new Error("timed out waiting for condition"); + await settle(); + } +} + +/** One accepted session on the fake fleet: the URL it was dialed at, and its server side. */ +interface Dial { + url: string; + server: WebTransport; + closed: boolean; +} + +/** + * Stand in for a fleet of relays behind any URL: every dial gets a fresh session serving + * `origin`, so each one publishes the same broadcasts the way siblings of a fleet do. + */ +function fleet(origin = new OriginProducer()): { dials: Dial[]; origin: OriginProducer } { + const dials: Dial[] = []; + const stub = function StubWebTransport(url: string | URL) { + const pair = createMockTransportPair(Lite.ALPN_06); + const dial: Dial = { url: new URL(url).href, server: pair.server, closed: false }; + dials.push(dial); + void pair.server.closed.then( + () => { + dial.closed = true; + }, + () => { + dial.closed = true; + }, + ); + void accept({ transport: pair.server, url: new URL(url), publish: origin.consume() }); + return pair.client; + }; + globalThis.WebTransport = stub as unknown as typeof WebTransport; + return { dials, origin }; +} + +/** Send a moq-lite GOAWAY from the server side of a session. */ +async function goaway(server: WebTransport, uri: string): Promise { + const stream = await Stream.open(server); + await stream.writer.u53(Lite.StreamId.Goaway); + await new Lite.Goaway(uri).encode(stream.writer, Lite.Version.DRAFT_06); +} + +const url = new URL("https://relay.example/room"); + +// Short enough to watch, long enough that a handover visibly overlaps the replacement. +const handover = Time.Milli(200); + +test("an empty GOAWAY migrates without unrouting the path", async () => { + const { dials, origin } = fleet(); + const broadcast = origin.createBroadcast(Path.from("cam")); + broadcast.announce(); + + const consume = new OriginProducer(); + const reload = new Reload({ + url, + websocket: { enabled: false }, + consume, + goaway: { handover }, + // A session younger than this counts as redirected immediately and backs off first. + delay: { initial: Time.Milli(1) }, + }); + const watched = consume.request(Path.from("cam"), { announced: true }); + + try { + await waitUntil(() => watched.active.peek() !== undefined); + const first = reload.established.peek(); + + // Record every moment the path had no route, from here on. + let gaps = 0; + const stop = watched.active.subscribe((active) => { + if (active === undefined) gaps++; + }); + + const drained = dials[0]; + if (!drained) throw new Error("no first dial"); + await goaway(drained.server, ""); + + // The replacement dials the configured URL at once, while the old session still serves. + await waitUntil(() => dials.length === 2); + expect(dials[1]?.url).toBe(url.href); + await waitUntil(() => reload.established.peek() !== first && reload.established.peek() !== undefined); + expect(drained.closed).toBe(false); + expect(reload.status.peek()).toBe("connected"); + + // The old session closes at the handover cap, and the path never went unrouted. + await waitUntil(() => drained.closed, handover * 10); + await settle(); + expect(watched.active.peek()).not.toBeUndefined(); + expect(gaps).toBe(0); + expect(dials.length).toBe(2); + stop(); + } finally { + watched.close(); + reload.close(); + consume.close(); + broadcast.close(); + } +}); + +test("a GOAWAY without a timeout hands over at the configured cap", async () => { + const { dials } = fleet(); + const reload = new Reload({ + url, + websocket: { enabled: false }, + goaway: { handover }, + delay: { initial: Time.Milli(1) }, + }); + + try { + await waitUntil(() => reload.status.peek() === "connected"); + const drained = dials[0]; + if (!drained) throw new Error("no first dial"); + + const sent = performance.now(); + await goaway(drained.server, ""); + await waitUntil(() => dials.length === 2 && reload.status.peek() === "connected"); + + // Lite carries no deadline, which must read as "the cap", never as a zero handover. + await waitUntil(() => drained.closed, handover * 10); + expect(performance.now() - sent).toBeGreaterThanOrEqual(handover * 0.9); + } finally { + reload.close(); + } +}); + +test("a refused redirect ends the connection instead of reconnecting", async () => { + const refused = ["https://other.example/", "not a url", "http://relay.example/", "https://127.0.0.1/"]; + for (const uri of refused) { + const { dials } = fleet(); + const reload = new Reload({ url, websocket: { enabled: false }, delay: { initial: Time.Milli(1) } }); + + try { + await waitUntil(() => reload.status.peek() === "connected"); + const drained = dials[0]; + if (!drained) throw new Error("no first dial"); + + await goaway(drained.server, uri); + await waitUntil(() => reload.error.peek() !== undefined); + expect(reload.error.peek(), uri).toBeInstanceOf(RefusedRedirect); + expect(reload.status.peek()).toBe("disconnected"); + await waitUntil(() => drained.closed); + + // Nothing redials: not the original URL, not anything else. + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(dials.length, uri).toBe(1); + } finally { + reload.close(); + } + } +}); + +test("a certificate pin refuses a redirect to another host even under follow", async () => { + const { dials } = fleet(); + const reload = new Reload({ + url, + websocket: { enabled: false }, + webtransport: { serverCertificateHashes: [{ value: "00".repeat(32) }] }, + goaway: { redirect: "follow" }, + delay: { initial: Time.Milli(1) }, + }); + + try { + await waitUntil(() => reload.status.peek() === "connected"); + await goaway(dials[0]?.server as WebTransport, "https://other.example/"); + await waitUntil(() => reload.error.peek() !== undefined); + expect(reload.error.peek()).toBeInstanceOf(RefusedRedirect); + expect(dials.length).toBe(1); + } finally { + reload.close(); + } +}); + +// A same-host move to another port: what `same-host` exists to allow. +const moved = new URL("https://relay.example:5443/room"); + +test("a redirect moves the pool key while the handle keeps its origin", async () => { + const { dials } = fleet(); + + const handle = new Connection({ url }); + try { + await waitUntil(() => handle.status.peek() === "connected"); + const origin = handle.origin.peek(); + + await goaway(dials[0]?.server as WebTransport, moved.href); + await waitUntil(() => dials.length === 2); + expect(dials[1]?.url).toBe(moved.href); + expect(handle.origin.peek()).toBe(origin); + + // A caller configured with the target shares the migrated connection. + const joined = new Connection({ url: moved }); + await waitUntil(() => joined.status.peek() === "connected"); + expect(joined.origin.peek()).toBe(origin); + expect(dials.length).toBe(2); + + // One still asking for the original URL gets a fresh entry. + const fresh = new Connection({ url }); + await waitUntil(() => fresh.status.peek() === "connected"); + expect(fresh.origin.peek()).not.toBe(origin); + expect(dials.length).toBe(3); + + joined.close(); + fresh.close(); + } finally { + handle.close(); + } +}); + +test("a redirect onto an already pooled key leaves both entries to their handles", async () => { + const { dials } = fleet(); + + const redirected = new Connection({ url }); + const resident = new Connection({ url: moved }); + try { + await waitUntil(() => redirected.status.peek() === "connected" && resident.status.peek() === "connected"); + const migrated = redirected.origin.peek(); + const target = resident.origin.peek(); + expect(migrated).not.toBe(target); + + const drained = dials.find((dial) => dial.url === url.href); + await goaway(drained?.server as WebTransport, moved.href); + await waitUntil(() => dials.filter((dial) => dial.url === moved.href).length === 2); + await waitUntil(() => redirected.status.peek() === "connected"); + + // Existing handles keep their own entries. + expect(redirected.origin.peek()).toBe(migrated); + expect(resident.origin.peek()).toBe(target); + + // The target key still belongs to the entry that was there. + const joined = new Connection({ url: moved }); + await waitUntil(() => joined.origin.peek() !== undefined); + expect(joined.origin.peek()).toBe(target); + + // The original key was vacated, so a new caller dials fresh. + const before = dials.length; + const fresh = new Connection({ url }); + await waitUntil(() => fresh.status.peek() === "connected"); + expect(fresh.origin.peek()).not.toBe(migrated); + expect(fresh.origin.peek()).not.toBe(target); + expect(dials.length).toBe(before + 1); + + joined.close(); + fresh.close(); + } finally { + redirected.close(); + resident.close(); + } +}); diff --git a/js/net/src/connection/pool.ts b/js/net/src/connection/pool.ts index 959697ca09..b7da0472bf 100644 --- a/js/net/src/connection/pool.ts +++ b/js/net/src/connection/pool.ts @@ -20,6 +20,7 @@ import { type WebTransportProps as WebTransportPropsType, } from "./connect.ts"; import type { Established as EstablishedType } from "./established.ts"; +import type { GoawayProps, Redirect as RedirectType } from "./goaway.ts"; import { Reload, type ReloadDelay, type ReloadStatus } from "./reload.ts"; import type { Probe as ProbeType, Stats as StatsType } from "./stats.ts"; import type { Transport as TransportType } from "./transport.ts"; @@ -69,6 +70,13 @@ export interface ConnectionProps { /** Backoff settings for the reconnect loop; an unset field uses its default. */ delay?: ReloadDelay; + /** + * How to react to the relay's GOAWAY; an unset field uses its default. Every connection + * migrates on GOAWAY, dialing the replacement while the old session finishes its groups. + * This only tunes where it may go and for how long the old session serves. + */ + goaway?: GoawayProps; + /** A Connection owns the abort signal for each connection attempt. */ signal?: never; @@ -93,7 +101,11 @@ export interface ConnectionProps { * connection so the next handle dials fresh; a new URL on this handle starts another * sequence. {@link closed} settles only when this handle is released. * - * Options the pool cannot honor (transport options, discovery, delay, a pinned + * A relay's GOAWAY migrates rather than drops: the replacement dials at once while the old + * session finishes its groups, and the handle and its origin carry across. A redirect the + * {@link ConnectionProps.goaway} policy refuses stops the loop like an auth rejection does. + * + * Options the pool cannot honor (transport options, discovery, delay, goaway, a pinned * certificate, caller-owned origins) use a private loop. An explicit `share: true` * refuses them. A supplied transport cannot reconnect at all; pass it to {@link Connection.connect} * instead. @@ -246,6 +258,7 @@ export class Connection { // A handle nobody watches wants unlimited retries; an auth rejection still // stops this URL, and a new one starts another sequence. delay: { timeout: Time.Milli(0), ...props.delay }, + goaway: props.goaway, }); this.#signals.cleanup(() => loop.close()); @@ -339,6 +352,10 @@ export namespace Connection { export type AcceptProps = AcceptPropsType; /** Backoff settings for a private reconnect loop. */ export type Backoff = ReloadDelay; + /** How a connection reacts to the relay's GOAWAY. */ + export type Goaway = GoawayProps; + /** What to do with the URI a relay names in its GOAWAY. */ + export type Redirect = RedirectType; /** Current state of a {@link Connection}. */ export type Status = ReloadStatus; /** The current connection's PROBE estimates. */ @@ -389,6 +406,9 @@ function refuse(props?: ConnectionProps): void { if (props.delay !== undefined) { throw new Error("delay cannot be shared; pass share: false"); } + if (props.goaway !== undefined) { + throw new Error("goaway cannot be shared; pass share: false"); + } } /** Options tied to one handle cannot be represented by a URL-keyed shared entry. */ @@ -398,6 +418,7 @@ function requiresPrivate(props: ConnectionProps): boolean { props.websocket !== undefined || props.discovery !== undefined || props.delay !== undefined || + props.goaway !== undefined || props.publish !== undefined || props.consume !== undefined ); @@ -405,6 +426,8 @@ function requiresPrivate(props: ConnectionProps): boolean { /** One shared connection and the handles keeping it alive. */ interface Entry { + /** The pool key: the dialed URL, which a GOAWAY redirect moves. */ + key: string; origin: Origin.Producer; connection: Reload; refs: number; @@ -431,14 +454,25 @@ function acquire(key: string, linger?: Time.Milli): Entry & { release: () => voi delay: { timeout: Time.Milli(0) }, }); - const created: Entry = { origin, connection, refs: 0, linger: linger ?? LINGER_MS }; + const created: Entry = { key, origin, connection, refs: 0, linger: linger ?? LINGER_MS }; - // The loop only stops on a peer saying these credentials will never work. Drop the - // entry so a later handle dials fresh rather than joining a loop that has stopped; - // handles already on it keep it until they release, since a redial would be refused - // the same way. + // The loop only stops on a peer saying these credentials will never work, or on a + // GOAWAY redirect it refused. Drop the entry so a later handle dials fresh rather + // than joining a loop that has stopped; handles already on it keep it until they + // release. connection.error.subscribe((err) => { - if (err !== undefined && pool.get(key) === created) pool.delete(key); + if (err !== undefined && pool.get(created.key) === created) pool.delete(created.key); + }); + + // An accepted redirect moves the entry to the URL it now dials, so a later handle + // configured with that URL shares it, and one asking for the old URL dials fresh. + // A live entry already at the target wins: this one leaves the pool and serves only + // the handles it has until they release it. + connection.redirect.subscribe((redirect) => { + if (!redirect || redirect.href === created.key) return; + if (pool.get(created.key) === created) pool.delete(created.key); + created.key = redirect.href; + if (!pool.has(created.key)) pool.set(created.key, created); }); entry = created; @@ -464,7 +498,7 @@ function acquire(key: string, linger?: Time.Milli): Entry & { release: () => voi if (taken.refs > 0) return; taken.timer = setTimeout(() => { - if (pool.get(key) === taken) pool.delete(key); + if (pool.get(taken.key) === taken) pool.delete(taken.key); taken.connection.close(); taken.origin.close(); }, taken.linger); diff --git a/js/net/src/connection/reload.ts b/js/net/src/connection/reload.ts index b7b77bcaef..a555ff9bf3 100644 --- a/js/net/src/connection/reload.ts +++ b/js/net/src/connection/reload.ts @@ -8,6 +8,7 @@ import * as Time from "../time.ts"; import { wireOf } from "../wire.ts"; import { type ConnectProps, connect, type WebSocketProps, type WebTransportProps } from "./connect.ts"; import type { Established } from "./established.ts"; +import { DEFAULT_HANDOVER, type Drain, dialed, type GoawayProps, handover, pinnedTransport, target } from "./goaway.ts"; import type { Probe, Stats } from "./stats.ts"; /** @@ -59,6 +60,9 @@ export type ReloadProps = Omit & { /** Backoff settings for the reconnect loop; every field falls back to its default. */ delay?: ReloadDelay; + + /** How to react to the peer's GOAWAY; every field falls back to its default. */ + goaway?: GoawayProps; }; /** @@ -162,6 +166,16 @@ export class Reload { /** Backoff settings for the reconnect loop; an unset field uses its default. */ delay: ReloadDelay; + /** How to react to the peer's GOAWAY (not reactive). */ + goaway: GoawayProps; + + /** + * The URL an accepted GOAWAY redirect assigned, or undefined while the loop dials + * {@link Reload.url}. Sticky across reconnects: a redirect is an assignment, not a + * detour. Cleared when a new URL or a disable/re-enable starts another sequence. + */ + readonly redirect: Getter; + /** The reactive effect scope driving the connect loop; closed by {@link Reload.close}. */ #signals = new Effect(); @@ -179,6 +193,16 @@ export class Reload { #closed = new Once(); #error = new Signal(undefined); + #redirect = new Signal(undefined); + // The configured href the redirect was assigned for. + #redirectHref: string | undefined; + + // A session the peer sent GOAWAY on, serving its groups in flight while the replacement + // dials. Retired when it closes, at its handover cap, or when the sequence ends. + #draining: Draining | undefined; + + // Whether a connect attempt is in flight, so a retiring predecessor reports the right status. + #dialing = false; // The current wait between attempts, doubling per failure, and when the retry window expires. // Both are undefined between sequences, so a later edit to `delay` applies to the next one. @@ -207,6 +231,8 @@ export class Reload { this.url = Signal.from(props?.url); this.enabled = Signal.from(props?.enabled ?? true); this.delay = props?.delay ?? {}; + this.goaway = props?.goaway ?? {}; + this.redirect = this.#redirect; this.webtransport = props?.webtransport; this.websocket = props?.websocket; this.discovery = props?.discovery; @@ -279,6 +305,7 @@ export class Reload { if (!enabled) { this.#givenUpHref = undefined; this.#resetSequence(); + this.#redirect.set(undefined); return; } @@ -293,6 +320,7 @@ export class Reload { if (!href) { this.#givenUpHref = undefined; this.#resetSequence(); + this.#redirect.set(undefined); return; } const url = new URL(href); @@ -303,49 +331,100 @@ export class Reload { if (this.#sequenceHref !== href) { this.#resetSequence(); + // A redirect was assigned for another URL; this one starts from itself. A page + // hide/show resumes the same URL, so it keeps the assignment. + if (this.#redirectHref !== href) this.#redirect.set(undefined); this.#sequenceHref = href; this.#error.set(undefined); this.#givenUpHref = undefined; } - effect.set(this.status, "connecting", "disconnected"); + // A drained predecessor still serves while its replacement dials. + if (!this.#draining) this.status.set("connecting"); // This run's teardown, handed to connect() so a rerun cancels the attempt in flight. const signal = effect.abort; + // The session this run serves, closed with the run. A drained one leaves this slot + // for #draining, which outlives the run. + let current: Established | undefined; + effect.cleanup(() => { + if (current) { + current.close(); + if (this.established.peek() === current) this.established.set(undefined); + current = undefined; + } + if (this.established.peek() === undefined) this.status.set("disconnected"); + }); + effect.spawn(async () => { // Set once the session is live, so #retry can tell a healthy session that // later dropped from a connect failure or a peer that flaps immediately. let connected: DOMHighResTimeStamp | undefined; try { - const connection = await connect({ - url, - websocket: this.websocket, - webtransport: this.webtransport, - discovery: this.discovery, - publish: this.publish, - consume: this.consume, - signal, - }); - - // Hand the connection to the effect, which closes it now if this run is already over. - effect.cleanup(() => connection.close()); - if (signal.aborted) return; - - effect.set(this.established, connection); - effect.set(this.status, "connected", "disconnected"); - - connected = performance.now(); + // Loops only to migrate: a GOAWAY off a healthy session dials its replacement + // straight away, with no backoff. + for (;;) { + const dialing = this.#redirect.peek() ?? url; + + this.#dialing = true; + let connection: Established; + try { + connection = await connect({ + url: dialing, + // A redirect names the relay; a fallback URL pinned for the old one does not follow. + websocket: this.#redirect.peek() ? { ...this.websocket, url: undefined } : this.websocket, + webtransport: this.webtransport, + discovery: this.discovery, + publish: this.publish, + consume: this.consume, + signal, + }); + } finally { + this.#dialing = false; + } - // A cancelled effect resolves undefined, so the sentinel tells the session - // closing (null for clean, an Error otherwise) apart from this run being - // torn down. - const closed = await effect.race(connection.closed); - if (closed === undefined) return; + // Hand the connection to the effect, which closes it now if this run is already over. + if (signal.aborted) { + connection.close(); + return; + } + current = connection; + + // The replacement serves now; a predecessor keeps draining its groups in flight. + this.established.set(connection); + this.status.set("connected"); + connected = performance.now(); + + // A cancelled effect resolves undefined, so the sentinel tells the session + // closing (null for clean, an Error otherwise) apart from this run being + // torn down. Anything else is the peer's GOAWAY. + const ended = await effect.race(connection.closed, wireOf(connection).goaway); + if (ended === undefined) return; + if (ended === null || ended instanceof Error) { + console.warn("connection closed, reconnecting"); + if (this.established.peek() === connection) this.established.set(undefined); + this.#retry(effect, connected, ended ?? undefined); + return; + } - console.warn("connection closed, reconnecting"); - this.#retry(effect, connected, closed ?? undefined); + current = undefined; + // A pinned WebSocket URL can win the race against the primary. Judge the + // redirect against that endpoint, not the primary we never reached. + const socket = this.#redirect.peek() ? undefined : this.websocket?.url; + if (!this.#migrate(connection, dialed(dialing, connection.transport, socket), ended)) return; + + // A session that outlived the initial delay was healthy, so its handover is not a + // failure. One redirected almost at once still migrates, but through the backoff, + // so two peers bouncing us between them escalate and eventually give up. + if (performance.now() - connected < this.#initial()) { + this.#retry(effect, connected, new Error("peer redirected immediately")); + return; + } + this.#delay = undefined; + this.#deadline = undefined; + } } catch (err) { // Treat teardown as cancellation, not a connection failure. if (signal.aborted) return; @@ -356,6 +435,56 @@ export class Reload { }); } + /** + * Act on the peer's GOAWAY for `connection`, which was dialed at `dialing`: resolve where + * to go next and leave the old session serving until it drains. Returns false when the + * redirect is refused, which ends the sequence rather than redialing. + */ + #migrate(connection: Established, dialing: URL, drain: Drain): boolean { + const hashes = this.webtransport?.serverCertificateHashes?.length ?? 0; + const configured = hashes > 0 || this.webtransport?.serverCertificate !== undefined; + // The pin is a WebTransport option. A WebSocket that won the race never used it. + const pinned = pinnedTransport(connection.transport, configured); + + let next: URL | undefined; + try { + next = target(this.goaway.redirect ?? "same-host", drain.uri, dialing, pinned); + } catch (err) { + // The peer is leaving and named somewhere we won't go: redialing the old address + // would ignore it, so stop here. + console.warn("GOAWAY redirect refused:", err); + connection.close(); + this.established.set(undefined); + this.status.set("disconnected"); + this.#giveUp(error(err)); + return false; + } + + // Only an accepted redirect replaces the URL; an empty one keeps it. + if (next) { + this.#redirect.set(next); + this.#redirectHref = this.#sequenceHref; + } + + console.info("GOAWAY received; migrating"); + // A newer GOAWAY retires an older predecessor rather than holding two open. + this.#draining?.retire(); + const cap = handover(this.goaway.handover ?? DEFAULT_HANDOVER, drain.timeout); + const draining = new Draining(connection, cap, () => { + if (this.#draining === draining) this.#draining = undefined; + // If nothing replaced it yet, nothing is serving. + if (this.established.peek() !== connection) return; + this.established.set(undefined); + this.status.set(this.#dialing ? "connecting" : "disconnected"); + }); + this.#draining = draining; + return true; + } + + #initial(): Time.Milli { + return this.delay?.initial ?? DEFAULT_DELAY.initial; + } + /** * Schedule the next connect attempt after the current backoff, or stop once the retry window * has expired. `connected` is when the dead session was established, if it ever was, and @@ -368,15 +497,14 @@ export class Reload { // optional value passes an explicit undefined, which a spread would take as the // answer, turning the backoff into NaN or the window into forever. const delay = this.delay ?? {}; - const initial = delay.initial ?? DEFAULT_DELAY.initial; + const initial = this.#initial(); const multiplier = delay.multiplier ?? DEFAULT_DELAY.multiplier; const max = delay.max ?? DEFAULT_DELAY.max; const timeout = delay.timeout ?? DEFAULT_DELAY.timeout; - // Any session is dead now: report disconnected during the backoff rather than - // when the retry reruns the effect. - this.established.set(undefined); - this.status.set("disconnected"); + // Report disconnected during the backoff rather than when the retry reruns the + // effect, unless a drained predecessor still serves until it retires. + if (this.established.peek() === undefined) this.status.set("disconnected"); // A session that outlived the initial delay was healthy, so clear the backoff and // start a fresh retry window: a one-off drop should reconnect promptly. Anything @@ -432,6 +560,7 @@ export class Reload { this.#delay = undefined; this.#deadline = undefined; this.#sequenceHref = undefined; + this.#draining?.retire(); } /** @@ -507,6 +636,37 @@ export class Reload { /** Stop reconnecting, close the current connection, and settle {@link Reload.closed}. Idempotent. */ close(abort?: Error) { this.#signals.close(); + this.#draining?.retire(); if (this.#closed.peek() === undefined) this.#closed.set(abort ?? null); } } + +/** + * A session the peer sent GOAWAY on, left serving so its groups in flight finish. It retires + * when it closes on its own or overstays its handover window, and `onRetire` runs once either way. + */ +class Draining { + #connection: Established; + #timer: ReturnType; + #onRetire: () => void; + #retired = false; + + constructor(connection: Established, handover: Time.Milli, onRetire: () => void) { + this.#connection = connection; + this.#onRetire = onRetire; + this.#timer = setTimeout(() => { + console.warn("old session did not drain in time; closing"); + this.retire(); + }, handover); + void connection.closed.then(() => this.retire()); + } + + /** Close the old session now, whatever remains of its window. Idempotent. */ + retire(): void { + if (this.#retired) return; + this.#retired = true; + clearTimeout(this.#timer); + this.#connection.close(); + this.#onRetire(); + } +} diff --git a/js/net/src/error.ts b/js/net/src/error.ts index 912fae64cd..dc048d19fe 100644 --- a/js/net/src/error.ts +++ b/js/net/src/error.ts @@ -246,6 +246,21 @@ export class NotFound extends Stream { } } +/** + * A peer's GOAWAY named a redirect the connection refuses, or one it could not parse. + * + * Terminal: the peer is leaving, so the connection stops rather than redialing the old + * address. Mirrors the Rust `Error::RefusedRedirect`. + * + * @public + */ +export class RefusedRedirect extends Error { + constructor(reason: string) { + super(`GOAWAY redirect refused: ${reason}`); + this.name = "RefusedRedirect"; + } +} + /** * A peer broke the protocol in a way the spec says must end the session. * diff --git a/js/net/src/errors.ts b/js/net/src/errors.ts index 09cc4dd606..d84bf02d1e 100644 --- a/js/net/src/errors.ts +++ b/js/net/src/errors.ts @@ -8,6 +8,7 @@ export { GroupTooLarge, NotFound, ProtocolViolation, + RefusedRedirect, Session, Stream, type StreamOptions, diff --git a/js/net/src/ietf/adapter.test.ts b/js/net/src/ietf/adapter.test.ts index 1945cef825..ebde36a91f 100644 --- a/js/net/src/ietf/adapter.test.ts +++ b/js/net/src/ietf/adapter.test.ts @@ -4,6 +4,7 @@ import * as Path from "../path.ts"; import { Stream } from "../stream.ts"; import { ControlStreamAdapter } from "./adapter.ts"; import { toRequestCode } from "./error.ts"; +import { GoAway } from "./goaway.ts"; import { PublishNamespace, PublishNamespaceCancel, PublishNamespaceDone } from "./publish_namespace.ts"; import { RequestError } from "./request.ts"; import { ALPN, Version } from "./version.ts"; @@ -184,3 +185,51 @@ test("withdrawals distinguish the same namespace by direction", async () => { await cancel(peer, namespace); expect(await closed(outgoing)).toBe(true); }); + +/** + * Draft-14 to -16 carry GOAWAY on the shared control stream. The adapter decodes it and keeps + * routing, so the session serves its groups in flight while the caller migrates. + */ +test("the control stream adapter decodes a GOAWAY and keeps running", async () => { + const { adapter, peer } = await connect(); + + await peer.writer.u53(GoAway.id); + await new GoAway({ newSessionUri: "https://relay.example/next" }).encode(peer.writer, VERSION); + + const drain = await adapter.goaway; + expect(drain.uri).toBe("https://relay.example/next"); + // These drafts carry no timeout: absence means the caller's cap, never a zero handover. + expect(drain.timeout).toBeUndefined(); + + // Still routing: a later announcement opens its virtual stream. + await announce(peer, 1n, Path.from("still")); + await accept(adapter); +}); + +test("a server adapter rejects a client GOAWAY that names a redirect", async () => { + const pair = createMockTransportPair(ALPN.DRAFT_15); + const control = await Stream.open(pair.server, { version: VERSION }); + const adapter = new ControlStreamAdapter(pair.server, control, VERSION, 100n, false); + const running = adapter.run(); + const peer = await Stream.accept(pair.client, VERSION); + if (!peer) throw new Error("no control stream"); + + await peer.writer.u53(GoAway.id); + await new GoAway({ newSessionUri: "https://other.example/" }).encode(peer.writer, VERSION); + await expect(running).rejects.toThrow("client GOAWAY must not name a redirect"); +}); + +test("a second GOAWAY on the control stream closes the session", async () => { + const pair = createMockTransportPair(ALPN.DRAFT_15); + const control = await Stream.open(pair.server, { version: VERSION }); + const adapter = new ControlStreamAdapter(pair.server, control, VERSION, 100n, true); + const running = adapter.run(); + const peer = await Stream.accept(pair.client, VERSION); + if (!peer) throw new Error("no control stream"); + + for (let i = 0; i < 2; i++) { + await peer.writer.u53(GoAway.id); + await new GoAway({ newSessionUri: "" }).encode(peer.writer, VERSION); + } + await expect(running).rejects.toThrow("duplicate GOAWAY"); +}); diff --git a/js/net/src/ietf/adapter.ts b/js/net/src/ietf/adapter.ts index fd16a5349d..ad2ae94e2e 100644 --- a/js/net/src/ietf/adapter.ts +++ b/js/net/src/ietf/adapter.ts @@ -1,6 +1,10 @@ +import { Once } from "@moq/signals"; import { Mutex } from "async-mutex"; +import type { Drain } from "../connection/goaway.ts"; +import { ProtocolViolation } from "../error.ts"; import { Reader, Stream, type Writer } from "../stream.ts"; import * as Varint from "../varint.ts"; +import { GoAway } from "./goaway.ts"; import * as Namespace from "./namespace.ts"; import { type IetfVersion, Version } from "./version.ts"; @@ -90,6 +94,9 @@ export class ControlStreamAdapter implements Session { #writeMutex = new Mutex(); readonly version: IetfVersion; + /** The peer's GOAWAY, which draft-14 to -16 carry on the shared control stream. */ + readonly goaway = new Once(); + // Virtual streams keyed by requestId #streams = new Map(); @@ -123,6 +130,9 @@ export class ControlStreamAdapter implements Session { #closed = false; + // Whether this side opened the session. Only a server may name a redirect. + #client: boolean; + constructor( quic: WebTransport, controlStream: Stream, @@ -138,6 +148,7 @@ export class ControlStreamAdapter implements Session { this.version = version; this.#maxRequestId = maxRequestId; this.#requestId = client ? 0n : 1n; + this.#client = client; } /** @@ -263,8 +274,15 @@ export class ControlStreamAdapter implements Session { const classified = await this.#classify(typeId, body); if (classified.route === Route.GoAway) { - console.warn("received GOAWAY on control stream"); - return; + // The session keeps serving: a GOAWAY asks us to migrate, not to stop reading. + const msg = await GoAway.decodeBody(body, this.version); + if (this.goaway.peek() !== undefined) throw new ProtocolViolation("duplicate GOAWAY"); + // A client may leave, but only the server may name where to go. + if (!this.#client && msg.newSessionUri !== "") { + throw new ProtocolViolation("client GOAWAY must not name a redirect"); + } + this.goaway.set(msg.drain()); + continue; } const { route, requestId } = classified; diff --git a/js/net/src/ietf/connection.ts b/js/net/src/ietf/connection.ts index e1d22f9bf8..7869b48188 100644 --- a/js/net/src/ietf/connection.ts +++ b/js/net/src/ietf/connection.ts @@ -1,6 +1,7 @@ -import { type Getter, Signal } from "@moq/signals"; +import { type Getter, Once, Signal } from "@moq/signals"; import type * as announce from "../announced.ts"; import type { Established } from "../connection/established.ts"; +import type { Drain } from "../connection/goaway.ts"; import { type Probe, type Stats, transportStats } from "../connection/stats.ts"; import { type Transport, transportOf } from "../connection/transport.ts"; import { error, fromClose, ProtocolViolation, StreamCode, StreamError } from "../error.ts"; @@ -45,6 +46,9 @@ export class Connection implements Established { // The established WebTransport session. #quic: WebTransport; + // Whether this side opened the session. Only a server may name a redirect. + #client: boolean; + // Session abstraction: adapter for v14-v16, native for v17. #session: Session; @@ -60,6 +64,9 @@ export class Connection implements Established { // The Hop IDs this session declared; see {@link Cluster}. #cluster?: Cluster.Hops; + // The peer's GOAWAY: read here on v17+, by the control stream adapter before that. + #goaway: Once; + // Just to avoid logging when `close()` is called. #closed = false; @@ -116,15 +123,18 @@ export class Connection implements Established { this.version = versionName(version); this.transport = transportOf(quic); this.#quic = quic; + this.#client = client; // Two-path dispatch: v14-v16 uses adapter, v17+ uses native bidi streams if (version >= Version.DRAFT_17) { this.#session = new NativeSession(quic, version, client); + this.#goaway = new Once(); // v17+: control/setup stream only carries GoAway void this.#runGoAway(control, version); } else { const adapter = new ControlStreamAdapter(quic, control, version, maxRequestId, client); this.#session = adapter; + this.#goaway = adapter.goaway; // Start the adapter read loop (routes control messages to virtual streams) void adapter.run().catch((err: unknown) => { if (!this.#closed) console.error("adapter error", err); @@ -142,7 +152,7 @@ export class Connection implements Established { this.#solicit = solicit; this.#cluster = cluster; this.#subscriber = new Subscriber({ session: this.#session, cluster, hidden }); - registerWire(this, { consume: (path) => this.#subscriber.consume(path) }); + registerWire(this, { consume: (path) => this.#subscriber.consume(path), goaway: this.#goaway }); void this.#run(); } @@ -317,18 +327,29 @@ export class Connection implements Established { /** * v17+ only: reads GoAway from the setup/control stream. + * + * The session keeps serving after a GOAWAY so its groups in flight can finish while the + * caller migrates; only the stream ending, or a second GOAWAY, closes it here. */ async #runGoAway(controlStream: Stream, version: IetfVersion) { try { - const done = await controlStream.reader.done(); - if (done) return; + for (;;) { + const done = await controlStream.reader.done(); + if (done) return; + + const typeId = await controlStream.reader.u53(); + if (typeId !== GoAway.id) { + console.warn(`unexpected message on setup stream: 0x${typeId.toString(16)}`); + return; + } - const typeId = await controlStream.reader.u53(); - if (typeId === GoAway.id) { const msg = await GoAway.decode(controlStream.reader, version); - console.warn(`received GOAWAY with redirect URI: ${msg.newSessionUri}`); - } else { - console.warn(`unexpected message on setup stream: 0x${typeId.toString(16)}`); + if (this.#goaway.peek() !== undefined) throw new ProtocolViolation("duplicate GOAWAY"); + // A client may leave, but only the server may name where to go. + if (!this.#client && msg.newSessionUri !== "") { + throw new ProtocolViolation("client GOAWAY must not name a redirect"); + } + this.#goaway.set(msg.drain()); } } catch (err) { if (!this.#closed) { diff --git a/js/net/src/ietf/goaway.test.ts b/js/net/src/ietf/goaway.test.ts new file mode 100644 index 0000000000..22c35e5eef --- /dev/null +++ b/js/net/src/ietf/goaway.test.ts @@ -0,0 +1,82 @@ +import { expect, test } from "bun:test"; +import { createMockTransportPair } from "../mock.ts"; +import { Stream } from "../stream.ts"; +import * as Time from "../time.ts"; +import { wireOf } from "../wire.ts"; +import { Connection } from "./connection.ts"; +import { GoAway } from "./goaway.ts"; +import { ALPN, Version } from "./version.ts"; + +// Draft-17+ carry GOAWAY on the setup stream, with the peer's deadline. The session keeps +// serving afterwards so its groups in flight finish while the caller migrates. +test("a draft-17 GOAWAY surfaces its URI and deadline without closing the session", async () => { + const version = Version.DRAFT_17; + const pair = createMockTransportPair(ALPN.DRAFT_17); + const control = await Stream.open(pair.client, { version }); + const peer = await Stream.accept(pair.server, version); + if (!peer) throw new Error("no setup stream"); + + const connection = new Connection({ + url: new URL("https://relay.example/"), + quic: pair.client, + control, + maxRequestId: 100n, + version, + client: true, + }); + + let closed = false; + void connection.closed.then(() => { + closed = true; + }); + + try { + await peer.writer.u53(GoAway.id); + await new GoAway({ newSessionUri: "", timeout: 5000n }).encode(peer.writer, version); + + const drain = await wireOf(connection).goaway; + expect(drain.uri).toBe(""); + expect(drain.timeout).toBe(Time.Milli(5000)); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(closed).toBe(false); + } finally { + connection.close(); + } +}); + +test("a server rejects a client GOAWAY that names a redirect", async () => { + const version = Version.DRAFT_17; + const pair = createMockTransportPair(ALPN.DRAFT_17); + const control = await Stream.open(pair.client, { version }); + const peer = await Stream.accept(pair.server, version); + if (!peer) throw new Error("no setup stream"); + + const connection = new Connection({ + url: new URL("https://relay.example/"), + quic: pair.client, + control, + maxRequestId: 100n, + version, + client: false, + }); + + let closed = false; + void connection.closed.then(() => { + closed = true; + }); + + try { + await peer.writer.u53(GoAway.id); + await new GoAway({ newSessionUri: "https://other.example/", timeout: 0n }).encode(peer.writer, version); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(closed).toBe(true); + } finally { + connection.close(); + } +}); + +test("a zero GOAWAY timeout reads as no deadline", async () => { + const msg = new GoAway({ newSessionUri: "https://relay.example/next", timeout: 0n }); + expect(msg.drain()).toEqual({ uri: "https://relay.example/next", timeout: undefined }); +}); diff --git a/js/net/src/ietf/goaway.ts b/js/net/src/ietf/goaway.ts index 9619b6ab4a..d77f188cfe 100644 --- a/js/net/src/ietf/goaway.ts +++ b/js/net/src/ietf/goaway.ts @@ -1,4 +1,6 @@ -import type { Reader, Writer } from "../stream.ts"; +import type { Drain } from "../connection/goaway.ts"; +import { Reader, type Writer } from "../stream.ts"; +import * as Time from "../time.ts"; import * as Message from "./message.ts"; import { type IetfVersion, Version } from "./version.ts"; @@ -35,6 +37,25 @@ export class GoAway { return Message.decode(r, (mr) => GoAway.#decode(mr, version)); } + /** + * Decode a body the caller already unframed, as the draft-14 to -16 control stream + * adapter does before routing a message. + */ + static async decodeBody(body: Uint8Array, version: IetfVersion): Promise { + const r = new Reader(undefined, body, version); + const msg = await GoAway.#decode(r, version); + if (!(await r.done())) throw new Error("GOAWAY has trailing bytes"); + return msg; + } + + /** The drain signal this message carries. A zero timeout is the wire saying "none". */ + drain(): Drain { + return { + uri: this.newSessionUri, + timeout: this.timeout > 0n ? Time.Milli(Number(this.timeout)) : undefined, + }; + } + static async #decode(r: Reader, version: IetfVersion): Promise { const newSessionUri = await r.string(); // All drafts cap the New Session URI at 8,192 bytes; a longer one is a diff --git a/js/net/src/lite/connection.test.ts b/js/net/src/lite/connection.test.ts index bf7717fefb..703a6702ae 100644 --- a/js/net/src/lite/connection.test.ts +++ b/js/net/src/lite/connection.test.ts @@ -1,7 +1,12 @@ import { expect, test } from "bun:test"; -import { probeLevel } from "./connection.ts"; +import { createMockTransportPair } from "../mock.ts"; +import { Stream } from "../stream.ts"; +import { wireOf } from "../wire.ts"; +import { Connection, probeLevel } from "./connection.ts"; +import { Goaway } from "./goaway.ts"; import { ProbeLevel } from "./setup.ts"; -import { Version } from "./version.ts"; +import { StreamId } from "./stream.ts"; +import { ALPN_04, Version } from "./version.ts"; /** A transport whose `getStats` behaves as described, or is absent entirely. */ function transport(getStats?: () => Promise): WebTransport { @@ -45,3 +50,39 @@ test("a throwing getStats advertises None rather than propagating", async () => }); expect(await probeLevel(quic, Version.DRAFT_05)).toBe(ProbeLevel.None); }); + +async function sendGoaway(server: WebTransport, uri: string): Promise { + const stream = await Stream.open(server); + await stream.writer.u53(StreamId.Goaway); + await new Goaway(uri).encode(stream.writer, Version.DRAFT_04); + stream.writer.close(); +} + +test("a lite GOAWAY keeps the session open, and a second one closes it", async () => { + const pair = createMockTransportPair(ALPN_04); + const connection = new Connection({ + url: new URL("https://relay.example/"), + quic: pair.client, + version: Version.DRAFT_04, + }); + + let closed = false; + void connection.closed.then(() => { + closed = true; + }); + + try { + await sendGoaway(pair.server, ""); + const drain = await wireOf(connection).goaway; + expect(drain.uri).toBe(""); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(closed).toBe(false); + + await sendGoaway(pair.server, "https://other.example/"); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(closed).toBe(true); + } finally { + connection.close(); + } +}); diff --git a/js/net/src/lite/connection.ts b/js/net/src/lite/connection.ts index 75e681d6f8..85b48684cf 100644 --- a/js/net/src/lite/connection.ts +++ b/js/net/src/lite/connection.ts @@ -1,9 +1,10 @@ -import { type Getter, Signal } from "@moq/signals"; +import { type Getter, Once, Signal } from "@moq/signals"; import type * as announce from "../announced.ts"; import type { Established } from "../connection/established.ts"; +import type { Drain } from "../connection/goaway.ts"; import { type Probe, type Stats, transportStats } from "../connection/stats.ts"; import { type Transport, transportOf } from "../connection/transport.ts"; -import { error, fromClose, StreamCode, StreamError } from "../error.ts"; +import { error, fromClose, ProtocolViolation, StreamCode, StreamError } from "../error.ts"; import { type Hop, randomHop } from "../hop.ts"; import type { Consumer as OriginConsumer } from "../origin.ts"; import type * as Path from "../path.ts"; @@ -94,6 +95,9 @@ export class Connection implements Established { // Written by the Subscriber as PROBE messages arrive. #probe = new Signal({}); + // The peer's GOAWAY. Lite carries no deadline, so only the URI is set. + #goaway = new Once(); + /** * The {@link Role} the peer advertised in its SETUP, for a server deciding whether the * peer's authorization grants the direction it intends to use. @@ -126,7 +130,7 @@ export class Connection implements Established { this.hop = randomHop(); this.#publisher = new Publisher(this.#quic, this.#version, this.hop, publish); this.#subscriber = new Subscriber(this.#quic, this.#version, this.hop, this.#probe, this.#peerSetup); - registerWire(this, { consume: (path) => this.#subscriber.consume(path) }); + registerWire(this, { consume: (path) => this.#subscriber.consume(path), goaway: this.#goaway }); void this.#run(); } @@ -218,6 +222,10 @@ export class Connection implements Established { this.#runBidi(stream) .catch((err: unknown) => { stream.writer.reset(err); + // A protocol violation on one stream is the peer breaking the session. + // Resetting that stream leaves it free to repeat the violation; a duplicate + // GOAWAY is the one this dispatcher raises. + if (err instanceof ProtocolViolation) this.close(); }) .finally(() => { stream.writer.close(); @@ -246,7 +254,9 @@ export class Connection implements Established { await this.#publisher.runProbe(stream); } else if (typ === StreamId.Goaway) { const msg = await Goaway.decode(stream.reader, this.#version); - console.info("received goaway:", msg.uri); + // A peer sends at most one; a second is a protocol violation. + if (this.#goaway.peek() !== undefined) throw new ProtocolViolation("duplicate GOAWAY"); + this.#goaway.set({ uri: msg.uri }); } else { throw new Error(`unknown stream type: ${typ.toString()}`); } diff --git a/js/net/src/origin.test.ts b/js/net/src/origin.test.ts index 47d044158e..bc7427169c 100644 --- a/js/net/src/origin.test.ts +++ b/js/net/src/origin.test.ts @@ -1300,6 +1300,52 @@ test("a rejected request is not asked of the same route again", async () => { origin.close(); }); +// A relay migration lands the replacement session's route next to the draining one's. The +// request must hand over to it, not drop to nothing while the new session answers. +test("an outranked route keeps serving until its replacement answers", async () => { + const origin = new Producer(); + const consumer = origin.consume(); + const path = Path.from("migrating"); + + const older = new BroadcastProducer(); + const keepOlder = older.consume(); + const disposeOlder = serve(origin, path, () => keepOlder.clone()); + + const request = consumer.request(path); + await settle(); + const first = request.active.peek(); + expect(first).toBeDefined(); + + let gaps = 0; + const stop = request.active.subscribe((active) => { + if (active === undefined) gaps++; + }); + + // The newer route wins the tie but has not answered yet. + const newer = wireOf(origin).receive(path); + const asked = newer.requested().next(); + await settle(); + expect(request.active.peek()).toBe(first); + + // Once it answers, the request swaps straight across. + const replacement = new BroadcastProducer(); + const { value: req } = await asked; + req?.accept(replacement.consume()); + await settle(); + expect(request.active.peek()).toBeDefined(); + expect(request.active.peek()).not.toBe(first); + expect(gaps).toBe(0); + + stop(); + request.close(); + newer.close(); + disposeOlder(); + keepOlder.close(); + older.close(); + replacement.close(); + origin.close(); +}); + test("a rejected request falls through to the next-best route", async () => { const origin = new Producer(); const consumer = origin.consume(); diff --git a/js/net/src/origin.ts b/js/net/src/origin.ts index 28dd06e62d..9d5a604623 100644 --- a/js/net/src/origin.ts +++ b/js/net/src/origin.ts @@ -321,11 +321,10 @@ class OriginState { const requests = this.requests.peek(); for (const [path, cached] of [...this.materialized]) { if (!Path.hasPrefix(prefix, path)) continue; - const refused = requests?.get(path)?.refused; - if (cached.entry !== this.bestEntry(path, (entry) => refused?.has(entry) ?? false)) { - this.materialized.delete(path); - cached.front.close(); - } + // A merely outranked provider is left to `route`, which holds it until the new one serves. + if (this.present(cached.entry)) continue; + this.materialized.delete(path); + cached.front.close(); } for (const [path, slot] of requests ?? []) { if (Path.hasPrefix(prefix, path)) slot.route.set(this.route(path, slot)); @@ -369,6 +368,14 @@ class OriginState { cached.front.close(); } + /** Whether `entry` is still in the table, rather than retracted. */ + present(entry: RouteEntry): boolean { + for (const entries of this.routes.peek()?.values() ?? []) { + if (entries.includes(entry)) return true; + } + return false; + } + /** The preferred entry on the most specific route covering `path`, ignoring skipped entries, if any. */ bestEntry(path: Path.Valid, skip?: (entry: RouteEntry) => boolean): RouteEntry | undefined { let bestPrefix: Path.Valid | undefined; @@ -405,7 +412,9 @@ class OriginState { * * Materialization is lazy and cached per path: the first request under a route opens * the providing session's subscription, repeats share it, and a provider change (the - * route retracting, a better session taking over) swaps it out. + * route retracting, a better session taking over) swaps it out. A route that was only + * outranked keeps serving until its replacement answers, so the swap never leaves the + * path unrouted in between: a relay migration hands over rather than dropping out. */ route(path: Path.Valid, slot: Pick): broadcast.Consumer | undefined { const entry = this.bestEntry(path, (candidate) => slot.refused.has(candidate)); @@ -416,23 +425,34 @@ class OriginState { return local; } - const cached = this.materialized.get(path); + let cached = this.materialized.get(path); if (cached && cached.entry === entry) { if (cached.front.closed.peek() === undefined) return cached.front; this.materialized.delete(path); - } else if (cached) { + cached = undefined; + } + // Whatever `cached` holds now belongs to another provider, and goes once this one serves. + const replace = () => { + if (!cached) return; this.materialized.delete(path); cached.front.close(); + }; + if (!entry?.server) { + replace(); + return slot.answer; } - if (!entry?.server) return slot.answer; const served = entry.server.served.get(path); if (served && served.closed.peek() === undefined) { + replace(); this.materialized.set(path, { entry, front: served }); return served; } entry.server.enqueue(path); + const standby = cached && !slot.refused.has(cached.entry) && this.present(cached.entry); + if (standby && cached?.front.closed.peek() === undefined) return cached?.front; + replace(); return undefined; } } diff --git a/js/net/src/wire.ts b/js/net/src/wire.ts index 3348b172bc..f4eb0a86d0 100644 --- a/js/net/src/wire.ts +++ b/js/net/src/wire.ts @@ -7,8 +7,9 @@ * * @module */ -import type { Dispose, Getter } from "@moq/signals"; +import type { Dispose, GetPromise, Getter } from "@moq/signals"; import type * as broadcast from "./broadcast.ts"; +import type { Drain } from "./connection/goaway.ts"; import type { Consumer as GroupConsumer } from "./group.ts"; import type { Route } from "./hop.ts"; import type * as origin from "./origin.ts"; @@ -53,9 +54,11 @@ export interface Advertised { readonly route: Route; } -/** The protocol-facing operation behind an established session. */ +/** The protocol-facing operations behind an established session. */ export interface Established { consume(path: Path.Valid): broadcast.Consumer; + /** Settles with the peer's GOAWAY; the session keeps serving until it closes. */ + readonly goaway: GetPromise; } type View = Broadcast | OriginProducer | OriginConsumer | Established; diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md index 93428a5a8c..c8e0a45e04 100644 --- a/quest/m1/drain/README.md +++ b/quest/m1/drain/README.md @@ -11,13 +11,12 @@ node anyway (a cached resolve, or a pool alias) just gets another GOAWAY. Only after sessions drain or the stop deadline expires does the process exit and the new software boot. -GOAWAY only reaches MoQ sessions, and only the Rust client acts on it today: -`moq_tokio::Connection` migrates, while `js/net` decodes and logs the message -and at most closes the session afterwards (the IETF path does, the lite path -does not), leaving any reconnect to the ordinary close-triggered backoff -rather than migrating. The wire message is lite04+/IETF only besides. So the stop deadline is the +GOAWAY only reaches MoQ sessions. Both clients migrate on it: +`moq_tokio::Connection` and the `js/net` `Connection` dial the replacement +through a fresh resolve while the old session drains. The wire message is +lite04+/IETF only besides. So the stop deadline is the real backstop - for pre-lite04 versions, for client SDKs deployed before -client-goaway ships, and for in-process ingest gateways (RTMP/SRT/WHIP/WHEP), +the JS migration shipped, and for in-process ingest gateways (RTMP/SRT/WHIP/WHEP), which have no GOAWAY equivalent at all: their grace is the DNS-drain window stopping new arrivals plus the encoder's own reconnect. The DNS-drain-first ordering is what keeps that hard-close window small. @@ -33,18 +32,19 @@ The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals included, against one deadline. -**client-goaway.** The JS reconnector migrates like the Rust one, preserving -the app-visible session while resolving DNS again before dialing, and the Rust -path gains the regression test it lacks. This is a +**Clients (landed).** The JS reconnector migrates like the Rust one, +preserving the app-visible session while resolving DNS again before dialing. +Both are covered against stand-in servers. This is a scale-down prerequisite, not merely a deploy improvement. RTMP/SRT/WHIP/WHEP cannot receive MoQ GOAWAY, so their contract remains DNS withdrawal followed by the stop deadline and encoder reconnect. +**End to end.** The line's own remaining work: a JS client watching a live +track through an in-tree relay drained with the drain hook migrates to a +second relay behind the same name without a dropped group. + ## Quests -- [Client goaway](/quest/m1/drain/client-goaway.md) - the JavaScript client - migrates on GOAWAY with a handover and the guarded redirect the Rust client - already has, and the Rust drain path gets its regression test - [Drain exit](/quest/m1/drain/drain-exit.md) - a drain ends as soon as every session has left, and reports whether that or the deadline ended it diff --git a/quest/m1/drain/client-goaway.md b/quest/m1/drain/client-goaway.md deleted file mode 100644 index 5d611d75dc..0000000000 --- a/quest/m1/drain/client-goaway.md +++ /dev/null @@ -1,87 +0,0 @@ -# [L] Client goaway - -## Goal - -The JavaScript client migrates on GOAWAY the way the Rust client already -does: it dials the replacement while the old session keeps serving, follows a -redirect URI under the same guard, and keeps the app-visible handle and its -origins across the swap. The Rust client's fleet-drain path (an empty-URI -GOAWAY redialed through a fresh DNS resolve) gains the regression test it -lacks today. - -## Plan - -moq.pro's (downstream) fleet drain orchestration relies on this behavior: a -drained node is already out of DNS when GOAWAY fires, so a re-resolve is what -lands clients on a healthy relay. - -Everything below describes `dev`, which is where this quest lands: `main` -still has `moq-native`'s close-only `Reconnect`. The drain loop is written -once into `Connection` and its pool. - -### Rust policy and proof - -`moq_tokio::Connection` already handles GOAWAY: the session loop returns the -message, `Redirect::resolve` guards the URI (scheme tier never drops, the host -is pinned to the configured one by default, and `follow` is the opt-in that -lets a peer name another), the loop redials while a -`Draining` handle keeps the old session serving until it closes or overstays -the handover cap, and `Status::Migrating` is visible to callers. Every dial -resolves DNS again, since `Addrs` holds URLs and each backend resolves at dial -time, so nothing is pinned. The relay always sends an empty URI, and the only -end-to-end coverage is the cluster sibling test with a redirect. - -Add the fleet-drain regression test: a relay GOAWAYs with an empty URI and a -timeout, the client redials the configured URL through a fresh resolve (a -resolver the test can repoint), live tracks hand over at a group boundary, -and the old session closes within the handover cap. - -### JavaScript is greenfield - -`js/net` logs the lite GOAWAY URI and keeps that session open, -logs the IETF draft-17+ URI and then closes the session when its control -loop ends, and on the draft-14 to -16 shared control stream reads the message -body but returns without decoding it. Nothing migrates: `Connection` -reconnects only after `closed` fires, through its backoff, tears the old -connection down in its effect cleanup, and the pool -(`js/net/src/connection/pool.ts`) keys by URL href. - -- Surface the peer's GOAWAY on the live session as a drain signal carrying the - resolved URI and the timeout, decoded on every wire the client speaks, - including the draft-14 to -16 adapter route that currently returns without - decoding the body it has already read. -- `Connection` mirrors `Draining`: on GOAWAY it dials the target immediately, - swaps the origin wiring (`forwardAnnounced`, `publish`, `subscribe`) once - the replacement is established, and leaves the old session to close on its - own or at a handover cap: the configured cap, lowered to the peer's timeout - when the wire carried a positive one. Lite and IETF drafts 14 to 16 carry no - timeout, and the IETF decoder reads an absent one as zero, so absence means - the cap and never a zero-length handover. Groups in flight finish. A GOAWAY does not go through the backoff delay; a failed - replacement dial does. -- Port the guard: same-host by default, refuse a scheme-tier drop or a - widening to a local host, and offer the follow mode. An empty URI preserves - the current address list, including caller-selected fallbacks, and starts - normal migration. A malformed or policy-refused explicit redirect ends the - connection with a typed terminal error; it must not trigger a retry against - the original address or another configured fallback. Only an accepted - redirect replaces the list. Apply and test this policy in Rust as well as JS; - do not assume the existing Rust behavior already satisfies it. A redirect with a certificate pin (`serverCertificateHashes`) - is refused unless the host is unchanged, since the pin cannot verify another - relay; the pool already refuses to share pinned connections. -- The pool re-keys its entry to the redirect target, so a later - caller configured with that URL shares the migrated connection. The app's - handle and shared origin are unchanged; only the pool key moves, and a - caller still asking for the original URL gets a fresh entry. When the - target key already holds a live entry, that entry wins: the migrating entry - is removed from the pool without replacing the target entry. Existing - handles keep its migrated connection, but a new lookup for the original URL - dials fresh and a target lookup joins the target entry. The migrated entry - retires when its last existing handle releases it. Two connections to one - relay for that overlap is the honest cost; entry removal is identity-guarded - so neither cleanup can delete another entry. -- Tests against the in-tree relay: an empty-URI drain migrates without a - dropped group, a GOAWAY without a timeout hands over at the configured cap, - a redirect moves the pool key and origins, a redirect onto an already - pooled key keeps existing handles on both entries but makes a new caller for - the original URL dial fresh, each guard refusal closes rather than - reconnects, and the draft-14 to -16 route decodes the URI. diff --git a/quest/m1/transport-upgrade/README.md b/quest/m1/transport-upgrade/README.md index 7e88b12e6f..194c13b0bd 100644 --- a/quest/m1/transport-upgrade/README.md +++ b/quest/m1/transport-upgrade/README.md @@ -33,9 +33,8 @@ every version (a client may send one with an empty URI; only a redirect URI is forbidden to a moq-transport client). The origin's multi-route front prefers the newest of two equal routes and `resume` splices each track at a group boundary, capping the old segment so the old session's subscription ends at the -boundary on its own. The JavaScript handover is the -[client goaway](/quest/m1/drain/client-goaway.md) quest's, so the JS half -requires it. +boundary on its own. The JavaScript handover ships with the +[drain](/quest/m1/drain/README.md) line, so the JS half requires it. Shared decisions: @@ -58,7 +57,7 @@ Shared decisions: ## Quests - [Rust](/quest/m1/transport-upgrade/rust.md) - moq-tokio keeps the QUIC dial after WebSocket wins and migrates through the existing Draining path -- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through the client-goaway handover +- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through the drain line's GOAWAY handover ## Related diff --git a/quest/m1/transport-upgrade/js.md b/quest/m1/transport-upgrade/js.md index 4cb58a3c59..f98faccec7 100644 --- a/quest/m1/transport-upgrade/js.md +++ b/quest/m1/transport-upgrade/js.md @@ -11,10 +11,12 @@ nothing changes. ## Plan -Lands in `js/net`, after the [client goaway](/quest/m1/drain/client-goaway.md) -quest ships the handover it reuses: dial the replacement while the old session -keeps serving, swap the origin wiring once it is established, leave the old -session to close on its own or at the handover cap. See the +Lands in `js/net`, after the [drain](/quest/m1/drain/README.md) line ships the +GOAWAY handover it reuses (`Reload`'s migration in +`js/net/src/connection/reload.ts`): dial the replacement while the old session +keeps serving, let both feed the origin (a request holds the outranked route +until the new one answers), leave the old session to close on its own or at the +handover cap. See the [questline](/quest/m1/transport-upgrade/README.md) for the shared decisions. - `connectInner` (`js/net/src/connection/connect.ts`) currently resolves @@ -37,10 +39,10 @@ session to close on its own or at the handover cap. See the group across the upgrade, the WebSocket session closes within the cap, and the next connect to the same URL gives WebTransport the head start again; with no delay, WebTransport wins and no WebSocket session is ever opened. -- Public API: none beyond what client-goaway adds; `transportOf` already +- Public API: none beyond what the drain line adds; `transportOf` already reports the live transport. Update `doc/lib/js` where the fallback race is described. ## Required -- [Client goaway](/quest/m1/drain/client-goaway.md) - the handover this upgrade reuses +- [Graceful relay drains](/quest/m1/drain/README.md) - ships the JS GOAWAY handover this upgrade reuses diff --git a/rs/moq-ffi/src/binary.rs b/rs/moq-ffi/src/binary.rs index 8e343bd2ad..1ffa8ca881 100644 --- a/rs/moq-ffi/src/binary.rs +++ b/rs/moq-ffi/src/binary.rs @@ -48,7 +48,8 @@ impl MoqBroadcastProducer { let _guard = crate::ffi::enter(); self.with_state(|state| { let track = state.broadcast.create_track(name, None)?; - let producer = state.catalog.binary_snapshot(track, config.into())?; + let config: moq_mux::binary::Config = config.into(); + let producer = state.catalog.binary_snapshot(track, config)?; Ok(Arc::new(MoqBinarySnapshotProducer { inner: std::sync::Mutex::new(Some(producer)), })) @@ -66,7 +67,8 @@ impl MoqBroadcastProducer { let _guard = crate::ffi::enter(); self.with_state(|state| { let track = state.broadcast.create_track(name, None)?; - let producer = state.catalog.binary_stream(track, config.into())?; + let config: moq_mux::binary::Config = config.into(); + let producer = state.catalog.binary_stream(track, config)?; Ok(Arc::new(MoqBinaryStreamProducer { inner: std::sync::Mutex::new(Some(producer)), })) diff --git a/rs/moq-tokio/src/client.rs b/rs/moq-tokio/src/client.rs index e9133a8e89..bc1744e633 100644 --- a/rs/moq-tokio/src/client.rs +++ b/rs/moq-tokio/src/client.rs @@ -57,6 +57,9 @@ pub struct Client { pub(crate) reconnect: bool, pub(crate) backoff: Backoff, pub(crate) goaway: Goaway, + /// Whether the TLS config pins a certificate fingerprint, which only verifies + /// the configured host, so a GOAWAY may not redirect elsewhere. + pub(crate) pinned: bool, /// The resolved Happy Eyeballs timings, used by the `tcp://` dial here; the /// QUIC backend captures its own copy from the config. #[cfg(feature = "tcp")] @@ -135,6 +138,7 @@ impl Client { reconnect: !config.once.unwrap_or(false), backoff: config.backoff, goaway: config.goaway, + pinned: !config.tls.fingerprint.is_empty(), #[cfg(feature = "tcp")] failover_delay, #[cfg(feature = "tcp")] diff --git a/rs/moq-tokio/src/connection.rs b/rs/moq-tokio/src/connection.rs index 13f3fb4e2a..6038f285af 100644 --- a/rs/moq-tokio/src/connection.rs +++ b/rs/moq-tokio/src/connection.rs @@ -209,27 +209,42 @@ pub enum Redirect { impl Redirect { /// Resolve the URL to dial after a GOAWAY, falling back to `current` when the /// redirect is empty ("reconnect to me"), malformed, or refused by policy. + /// + /// Lenient on purpose, for a caller that only wants somewhere to dial. A + /// [`Connection`] is stricter: it ends with [`Error::RefusedRedirect`] rather + /// than redialing after a malformed or refused URI. pub fn resolve(&self, uri: &str, current: &Url) -> Url { - self.target(uri, current).unwrap_or_else(|| current.clone()) + self.target(uri, current, false) + .ok() + .flatten() + .unwrap_or_else(|| current.clone()) } - // Absence keeps the caller's address list; only an accepted URI replaces it. - fn target(&self, uri: &str, current: &Url) -> Option { - if uri.is_empty() || matches!(self, Self::Ignore) { - return None; + /// The URL a GOAWAY assigns. `Ok(None)` keeps the current address list (the + /// peer named no URI, or the policy ignores a URI it could parse), `Ok(Some)` + /// replaces it, and `Err` is an explicit URI this policy refuses. A malformed + /// URI is refused even under [`Self::Ignore`]. + /// + /// `pinned` is a certificate pin on the connection, which can only verify the + /// host it was configured for, so it refuses a host change even under + /// [`Self::Follow`]. + fn target(&self, uri: &str, current: &Url, pinned: bool) -> crate::Result> { + if uri.is_empty() { + return Ok(None); } - let Ok(target) = uri.parse::() else { - tracing::warn!(uri, "malformed GOAWAY URI; keeping the current addresses"); - return None; - }; + // The URI can carry credentials, so the error names the reason, never the URI. + // Parse before `Ignore`: a malformed redirect is terminal even when the policy + // would otherwise stay on the current address list. + let refuse = |reason: &str| Error::RefusedRedirect(reason.to_string()); + + let target = uri.parse::().map_err(|_| refuse("the GOAWAY URI is malformed"))?; + if matches!(self, Self::Ignore) { + return Ok(None); + } if scheme_tier(target.scheme()) < scheme_tier(current.scheme()) { - tracing::warn!( - uri, - "GOAWAY redirect downgrades the scheme; keeping the current addresses" - ); - return None; + return Err(refuse("the GOAWAY redirect downgrades the scheme")); } // Only as far as the URL itself says: a name is dialed, never resolved here, @@ -237,27 +252,29 @@ impl Redirect { // nothing about one that hides the same address behind a hostname. That gap // is why [`Self::SameHost`] is the default; see [`is_local`]. if is_local(&target) && !is_local(current) { - tracing::warn!( - uri, - "GOAWAY redirect widens reachability; keeping the current addresses" - ); - return None; + return Err(refuse("the GOAWAY redirect widens reachability to a local address")); } // Host only, not the full authority: the port is what a peer legitimately // moves us across when it hands off to a sibling process on the same box. - if matches!(self, Self::SameHost) && target.host_str() != current.host_str() { - tracing::warn!( - uri, - "GOAWAY redirect leaves the current host; keeping the current addresses" - ); - return None; + let same_host = target.host_str() == current.host_str(); + if matches!(self, Self::SameHost) && !same_host { + return Err(refuse("the GOAWAY redirect leaves the current host")); + } + if pinned && !same_host { + return Err(refuse("the GOAWAY redirect leaves the host a certificate pin verifies")); } - Some(target) + Ok(Some(target)) } } +/// Whether this scheme's dial installs the Rustls verifier, so a configured +/// fingerprint actually checked the peer. Plain and non-Rustls transports do not. +fn fingerprint_pins(scheme: &str) -> bool { + matches!(scheme, "https" | "wss" | "moqt" | "moql") +} + /// Rank a scheme so a peer-supplied redirect cannot silently drop encryption. /// Unknown schemes rank lowest, so a forgotten classification is refused. fn scheme_tier(scheme: &str) -> u8 { @@ -762,11 +779,20 @@ impl Connection { // ended sooner counts as a failed attempt however it ended. let healthy = connected.elapsed() >= initial; - // The connected target owns the policy, including in one-shot mode. - if let Ended::Goaway(msg) = &ended - && addr.addresses().is_some() - && goaway.redirect.target(msg.uri(), &url).is_some() - { + // The connected target owns the policy, including in one-shot mode. A + // refused redirect is terminal: the peer is leaving and named somewhere we + // won't go, so redialing the old address or a fallback would ignore it. + let assigned = match &ended { + // A fingerprint only checked the dial that installed the Rustls + // verifier. tcp, unix, iroh, and plaintext WebSocket never consult it. + Ended::Goaway(msg) => { + goaway + .redirect + .target(msg.uri(), &url, client.pinned && fingerprint_pins(url.scheme()))? + } + Ended::Closed(_) => None, + }; + if assigned.is_some() && addr.addresses().is_some() { return Err(Error::PinnedRedirect); } @@ -784,7 +810,7 @@ impl Connection { // An accepted redirect is an assignment: keep dialing it from here on, and // only it. The peer named exactly one place to go, which retires // whatever other addresses got us to this session. - let url = if let Some(target) = goaway.redirect.target(msg.uri(), &url) { + let url = if let Some(target) = assigned { addrs = Addrs::new(target.clone()); target } else { @@ -1495,8 +1521,8 @@ mod tests { assert_eq!(Redirect::Follow.resolve("https://other.example/", &plain), same); } - /// The three ways a redirect resolves to "redial what we already had": the peer - /// naming no URI, a URI we cannot parse, and a policy that ignores it outright. + /// `resolve` is the lenient form: every way a redirect can fail to assign a + /// new URL, refusals included, lands back on the current one. #[test] fn resolve_falls_back_to_the_current_url() { let current: Url = "https://relay.example/".parse().unwrap(); @@ -1506,11 +1532,7 @@ mod tests { current, "empty means 'reconnect to me'" ); - assert_eq!( - Redirect::Follow.resolve("not a url", ¤t), - current, - "a malformed URI is not a reason to stop reconnecting" - ); + assert_eq!(Redirect::Follow.resolve("not a url", ¤t), current); assert_eq!( Redirect::Ignore.resolve("https://other.example/", ¤t), current, @@ -1575,26 +1597,73 @@ mod tests { ); } + /// Only an explicit URI can assign, and one the policy will not follow is an + /// error rather than a quiet fallback: the loop ends on it instead of redialing. #[test] fn only_an_accepted_redirect_replaces_the_address_list() { let current: Url = "https://relay.example/".parse().unwrap(); + + // No URI, or a policy that ignores it: keep the current address list. + assert_eq!(Redirect::Follow.target("", ¤t, false).unwrap(), None); + assert_eq!( + Redirect::Ignore + .target("https://relay.example:5443/", ¤t, false) + .unwrap(), + None + ); + + // An explicit URI the policy will not follow is refused. for (policy, uri) in [ (Redirect::SameHost, "https://other.example/"), - (Redirect::Follow, ""), (Redirect::Follow, "not a url"), - (Redirect::Ignore, "https://relay.example:5443/"), + (Redirect::Ignore, "not a url"), (Redirect::Follow, "http://relay.example/"), (Redirect::Follow, "https://127.0.0.1/"), ] { - assert_eq!(policy.target(uri, ¤t), None, "{policy:?}: {uri}"); + assert!( + matches!(policy.target(uri, ¤t, false), Err(Error::RefusedRedirect(_))), + "{policy:?}: {uri}" + ); } + // An explicit assignment remains an assignment even if its URL is unchanged. assert_eq!( - Redirect::SameHost.target(current.as_str(), ¤t), + Redirect::SameHost.target(current.as_str(), ¤t, false).unwrap(), Some(current.clone()) ); let moved = "https://relay.example:5443/"; - assert_eq!(Redirect::SameHost.target(moved, ¤t), Some(moved.parse().unwrap())); + assert_eq!( + Redirect::SameHost.target(moved, ¤t, false).unwrap(), + Some(moved.parse().unwrap()) + ); + } + + /// A fingerprint is a Rustls check. Schemes that never install that verifier + /// must not inherit the pin. + #[test] + fn a_fingerprint_pin_only_covers_rustls_schemes() { + for scheme in ["https", "wss", "moqt", "moql"] { + assert!(fingerprint_pins(scheme), "{scheme}"); + } + for scheme in ["http", "ws", "tcp", "unix", "iroh"] { + assert!(!fingerprint_pins(scheme), "{scheme}"); + } + } + + /// A certificate pin verifies only the host it was configured for, so it + /// refuses a host change even when the policy would follow one. + #[test] + fn a_certificate_pin_holds_the_host() { + let current: Url = "https://relay.example/".parse().unwrap(); + assert!(matches!( + Redirect::Follow.target("https://other.example/", ¤t, true), + Err(Error::RefusedRedirect(_)) + )); + let moved = "https://relay.example:5443/"; + assert_eq!( + Redirect::Follow.target(moved, ¤t, true).unwrap(), + Some(moved.parse().unwrap()) + ); } /// `SameHost` lets a peer move us between ports or schemes on the endpoint we @@ -1950,4 +2019,134 @@ mod tests { assert_eq!(attempt_timeout(1, 3), Some(CONNECT_ATTEMPT), "still one more"); assert_eq!(attempt_timeout(2, 3), None, "the last of three"); } + + /// A stream-only server on a free loopback port, publishing `origin`. + /// + /// Returns its address, a receiver yielding each accepted session (so the test + /// can drain it), and the listener task. Probing for a free port races other + /// tests between the probe closing and the real bind, so this retries. + #[cfg(feature = "tcp")] + async fn serve( + origin: moq_net::origin::Producer, + ) -> ( + std::net::SocketAddr, + tokio::sync::mpsc::UnboundedReceiver, + tokio::task::JoinHandle<()>, + ) { + for _ in 0..20 { + let probe = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = probe.local_addr().unwrap(); + drop(probe); + + let mut config = crate::listen::Config::default(); + config.tcp.bind = Some(addr); + let Ok(mut server) = config.init(Default::default()).unwrap().listen().await else { + continue; + }; + + let (accepted, sessions) = tokio::sync::mpsc::unbounded_channel(); + let task = tokio::spawn(async move { + while let Some(request) = server.accept().await { + if let Ok(session) = request.with_publisher(&origin).ok().await { + let _ = accepted.send(session); + } + } + }); + return (addr, sessions, task); + } + panic!("could not bind a free TCP port after 20 attempts"); + } + + /// The fleet drain: a relay withdrawn from DNS sends an empty-URI GOAWAY with a + /// deadline, and the client lands on a healthy relay by resolving the configured + /// name again. The drained relay still accepts, so a cached resolve would land + /// right back on it. The live track hands over at a group boundary, and the old + /// session closes at our handover cap, well before the peer's own deadline. + #[cfg(feature = "tcp")] + #[tokio::test] + async fn a_fleet_drain_redials_through_a_fresh_resolve() { + const WAIT: Duration = Duration::from_secs(10); + const HANDOVER: Duration = Duration::from_millis(500); + const DEADLINE: Duration = Duration::from_secs(30); + + // Two relays of one fleet, serving the same live broadcast. + let origin = crate::origin::spawn(); + let broadcast = origin.create_broadcast("cam").unwrap(); + broadcast.announce(Default::default()).unwrap(); + let track = broadcast.create_track("video", None).unwrap(); + let (addr_a, mut accepted_a, _task_a) = serve(origin.clone()).await; + let (addr_b, mut accepted_b, _task_b) = serve(origin.clone()).await; + + // Unique to this test: the table is process-wide. + const HOST: &str = "fleet-drain.test"; + crate::resolve::hosts::point(HOST, [addr_a]); + + let subscriber = crate::origin::spawn(); + let mut config = crate::connect::Config::default(); + config.goaway.handover = HANDOVER; + // A session younger than the initial delay counts as redirected immediately and + // waits out a backoff, which this test is not about. + config.backoff.initial = MIN_BACKOFF; + let client = config + .init(Default::default()) + .unwrap() + .with_subscriber(subscriber.clone()); + let url: Url = format!("tcp://{HOST}:1/").parse().unwrap(); + let _connection = client.connect(url); + + let session_a = tokio::time::timeout(WAIT, accepted_a.recv()).await.unwrap().unwrap(); + + let consumer = subscriber.consume(); + let cam = tokio::time::timeout(WAIT, consumer.routed_broadcast("cam")) + .await + .unwrap() + .unwrap(); + let mut sub = cam.track("video").unwrap().subscribe(None).await.unwrap(); + + let mut group = track.append_group().unwrap(); + group.write_frame(moq_net::Timestamp::ZERO, b"g0".as_ref()).unwrap(); + group.finish().unwrap(); + let g0 = tokio::time::timeout(WAIT, sub.recv_group()) + .await + .unwrap() + .unwrap() + .unwrap(); + assert_eq!(g0.sequence, 0); + + // Withdraw A from DNS, then drain it. + crate::resolve::hosts::point(HOST, [addr_b]); + let drained = tokio::time::Instant::now(); + session_a + .drain() + .send(moq_net::goaway::Goaway::new().with_timeout(DEADLINE)) + .unwrap(); + + let _session_b = tokio::time::timeout(WAIT, accepted_b.recv()) + .await + .expect("never redialed through the fresh resolve") + .unwrap(); + + let mut group = track.append_group().unwrap(); + group.write_frame(moq_net::Timestamp::ZERO, b"g1".as_ref()).unwrap(); + group.finish().unwrap(); + let mut g1 = tokio::time::timeout(WAIT, sub.recv_group()) + .await + .unwrap() + .unwrap() + .unwrap(); + assert_eq!(g1.sequence, 1, "delivery resumes at the next group after the swap"); + assert_eq!(g1.read_frame().await.unwrap().unwrap().payload[..], b"g1"[..]); + + tokio::time::timeout(WAIT, session_a.closed()) + .await + .expect("the drained session never closed"); + assert!( + drained.elapsed() < DEADLINE, + "the old session outlived the handover cap and waited for the peer's deadline" + ); + assert!( + accepted_a.try_recv().is_err(), + "a cached resolve redialed the drained relay" + ); + } } diff --git a/rs/moq-tokio/src/error.rs b/rs/moq-tokio/src/error.rs index 209306d96a..f603fc1307 100644 --- a/rs/moq-tokio/src/error.rs +++ b/rs/moq-tokio/src/error.rs @@ -58,6 +58,11 @@ pub enum Error { #[error("peer redirect refused for a connection with fixed addresses")] PinnedRedirect, + /// A peer's GOAWAY named a redirect the connection's policy refuses, or one it + /// could not parse. Terminal: the peer is leaving, so redialing is ignoring it. + #[error("GOAWAY redirect refused: {0}")] + RefusedRedirect(String), + /// Reading or writing a socket, certificate, or key file failed. #[error(transparent)] Io(Arc), diff --git a/rs/moq-tokio/src/resolve.rs b/rs/moq-tokio/src/resolve.rs index 7410cae79a..3f58d119fa 100644 --- a/rs/moq-tokio/src/resolve.rs +++ b/rs/moq-tokio/src/resolve.rs @@ -249,6 +249,11 @@ impl Candidates { }, }; + #[cfg(test)] + if let Some(addrs) = hosts::lookup(domain) { + return Self::fixed(addrs); + } + Self { full: Query::start(domain, port, Lookup::Full), ipv4: Query::start(domain, port, Lookup::Ipv4), @@ -549,6 +554,32 @@ impl Candidates { } } +/// A name table tests can repoint, consulted before the system resolver. +/// +/// DNS is what moves a client off a drained node, so proving a redial resolves +/// afresh needs a name whose answer changes between dials. Entries carry the +/// port too, so two servers on one loopback address can stand in for two hosts. +#[cfg(test)] +pub(crate) mod hosts { + use std::collections::HashMap; + use std::net::SocketAddr; + use std::sync::Mutex; + + static HOSTS: Mutex>>> = Mutex::new(None); + + /// Answer every later lookup of `host` with `addrs`, ports included. + pub(crate) fn point(host: &str, addrs: impl IntoIterator) { + let mut hosts = HOSTS.lock().unwrap(); + hosts + .get_or_insert_with(HashMap::new) + .insert(host.to_string(), addrs.into_iter().collect()); + } + + pub(super) fn lookup(host: &str) -> Option> { + HOSTS.lock().unwrap().as_ref()?.get(host).cloned() + } +} + #[cfg(test)] impl Query { /// A lookup that answers with `addrs` after `delay`. diff --git a/rs/moq-tokio/tests/reconnect.rs b/rs/moq-tokio/tests/reconnect.rs index a3532e354c..c2ecf04bf7 100644 --- a/rs/moq-tokio/tests/reconnect.rs +++ b/rs/moq-tokio/tests/reconnect.rs @@ -133,8 +133,7 @@ async fn spawn_server() -> ( (port, sessions, handle) } -/// A client that redials fast, so a refused redirect lands back on the original -/// server inside the test's patience. +/// A client that redials fast, so a migration lands inside the test's patience. fn quick_client(redirect: moq_tokio::Redirect) -> moq_tokio::Client { let mut config = moq_tokio::connect::Config::default(); config.backoff.initial = Duration::from_millis(20); @@ -144,14 +143,15 @@ fn quick_client(redirect: moq_tokio::Redirect) -> moq_tokio::Client { config.init(Default::default()).expect("failed to init client") } -/// The default refuses peer-selected host changes and redials the configured URL. +/// The default refuses a peer-selected host change, and the refusal ends the +/// connection: the peer is leaving, so redialing the configured URL would ignore it. #[tokio::test] async fn a_redirect_to_another_host_is_refused_by_default() { let (port_a, mut sessions_a, _task_a) = spawn_server().await; let (port_b, mut sessions_b, _task_b) = spawn_server().await; let url: url::Url = format!("tcp://localhost:{port_a}/").parse().expect("parse url"); - let _connection = quick_client(Default::default()).connect(url); + let connection = quick_client(Default::default()).connect(url); let first = tokio::time::timeout(Duration::from_secs(10), sessions_a.recv()) .await @@ -163,12 +163,13 @@ async fn a_redirect_to_another_host_is_refused_by_default() { .send(moq_net::goaway::Goaway::redirect(format!("tcp://127.0.0.1:{port_b}/"))) .expect("send goaway"); - // Refused, so the redial goes back to A rather than to the host the peer named. - tokio::time::timeout(Duration::from_secs(10), sessions_a.recv()) + let err = tokio::time::timeout(Duration::from_secs(10), connection.closed()) .await - .expect("never redialed the configured URL") - .expect("server A stopped accepting"); + .expect("the refusal never ended the connection") + .expect_err("a refused redirect must end with an error"); + assert!(matches!(err, moq_tokio::Error::RefusedRedirect(_)), "ended with {err}"); + assert!(sessions_a.try_recv().is_err(), "redialed the configured URL"); assert!( sessions_b.try_recv().is_err(), "the peer moved us onto the host it named" @@ -202,21 +203,21 @@ async fn follow_still_honors_a_cross_host_redirect() { .expect("server B stopped accepting"); } -/// Refusing a peer-selected host must not discard caller-selected fallbacks. +/// A refused redirect must not fall through to a caller-selected fallback either. #[tokio::test] -async fn a_refused_redirect_preserves_configured_fallbacks() { +async fn a_refused_redirect_skips_configured_fallbacks() { let (port_a, mut sessions_a, task_a) = spawn_server().await; let (port_b, mut sessions_b, _task_b) = spawn_server().await; let primary: url::Url = format!("tcp://localhost:{port_a}/").parse().expect("primary URL"); let fallback: url::Url = format!("tcp://127.0.0.1:{port_b}/").parse().expect("fallback URL"); let addrs = moq_tokio::connect::Addrs::new(primary).or(fallback); - let _connection = quick_client(Default::default()).connect(addrs); + let connection = quick_client(Default::default()).connect(addrs); let first = tokio::time::timeout(Duration::from_secs(10), sessions_a.recv()) .await .expect("first dial timed out") .expect("server A stopped accepting"); - // Stop accepting before GOAWAY so reconnect must use the configured fallback. + // Stop accepting before GOAWAY so any redial could only land on the fallback. task_a.abort(); assert!(task_a.await.expect_err("listener was aborted").is_cancelled()); first @@ -224,6 +225,36 @@ async fn a_refused_redirect_preserves_configured_fallbacks() { .send(moq_net::goaway::Goaway::redirect("tcp://127.0.0.1:1/")) .expect("send goaway"); + let err = tokio::time::timeout(Duration::from_secs(10), connection.closed()) + .await + .expect("the refusal never ended the connection") + .expect_err("a refused redirect must end with an error"); + assert!(matches!(err, moq_tokio::Error::RefusedRedirect(_)), "ended with {err}"); + assert!( + sessions_b.try_recv().is_err(), + "fell through to the configured fallback" + ); +} + +/// An empty GOAWAY is "reconnect to me", which keeps every caller-selected fallback. +#[tokio::test] +async fn an_empty_goaway_preserves_configured_fallbacks() { + let (port_a, mut sessions_a, task_a) = spawn_server().await; + let (port_b, mut sessions_b, _task_b) = spawn_server().await; + let primary: url::Url = format!("tcp://localhost:{port_a}/").parse().expect("primary URL"); + let fallback: url::Url = format!("tcp://127.0.0.1:{port_b}/").parse().expect("fallback URL"); + let addrs = moq_tokio::connect::Addrs::new(primary).or(fallback); + let _connection = quick_client(Default::default()).connect(addrs); + let first = tokio::time::timeout(Duration::from_secs(10), sessions_a.recv()) + .await + .expect("first dial timed out") + .expect("server A stopped accepting"); + + // Stop accepting before GOAWAY so the migration must use the configured fallback. + task_a.abort(); + assert!(task_a.await.expect_err("listener was aborted").is_cancelled()); + first.drain().send(moq_net::goaway::Goaway::new()).expect("send goaway"); + tokio::time::timeout(Duration::from_secs(10), sessions_b.recv()) .await .expect("configured fallback was discarded") From 5e04fdb410cf030b8e49d7f271b51ad30d50bdc3 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 25 Sep 2026 15:28:48 -0700 Subject: [PATCH 04/15] feat(relay): end a drain once every session has left (#4186) Co-authored-by: Claude Opus 5.5 --- doc/bin/relay/config.md | 10 +- doc/bin/relay/http.md | 4 +- doc/bin/relay/index.md | 5 +- quest/m1/drain/README.md | 11 +- quest/m1/drain/drain-exit.md | 19 ---- rs/moq-relay/src/config.rs | 5 +- rs/moq-relay/src/connection.rs | 3 +- rs/moq-relay/src/internal.rs | 35 ++++++- rs/moq-relay/src/relay.rs | 58 ++++++++--- rs/moq-relay/src/shutdown.rs | 145 ++++++++++++++++++++++++-- rs/moq-relay/src/websocket.rs | 1 + rs/moq-relay/tests/shutdown_signal.rs | 101 +++++++++++++++--- 12 files changed, 322 insertions(+), 75 deletions(-) delete mode 100644 quest/m1/drain/drain-exit.md diff --git a/doc/bin/relay/config.md b/doc/bin/relay/config.md index c2588abc45..4b9680d03f 100644 --- a/doc/bin/relay/config.md +++ b/doc/bin/relay/config.md @@ -231,8 +231,14 @@ The first SIGTERM or SIGINT starts a drain: every session is sent a GOAWAY asking it to reconnect, and is force-closed if it is still connected when the window ends. A session that connects during the drain, such as a client with a cached DNS answer, is sent a GOAWAY immediately, with only the time left in -the window. The relay exits one second after the window ends, or immediately -on a second signal. `0` skips the GOAWAY and closes every session at once. +the window. The relay exits as soon as every session has left, when the window +ends, or immediately on a second signal. `0` skips the GOAWAY and closes every +session at once. + +The exit is logged with how long the drain took, as either +`drain complete: every session left` or `drain deadline force-closed sessions` +with the number `forced`. A session still in its handshake when the last one +leaves is not waited for. Only moq-lite-04+ and moq-transport clients act on a GOAWAY; older ones are closed when the window ends. An embedder can take over the signals and start the drain itself; see [Embed](/bin/relay/#embed). diff --git a/doc/bin/relay/http.md b/doc/bin/relay/http.md index 28cad8de5c..1ace5b461c 100644 --- a/doc/bin/relay/http.md +++ b/doc/bin/relay/http.md @@ -52,7 +52,9 @@ split by `tier` and `role`, plus accept-loop counters per TCP listener. Alert on `moq_relay_accept_failures_total{class="exhausted"}`, which means the process ran out of a resource `accept` needs. Content dropped for drifting past a subscriber's budget is counted separately as `moq_relay_stale_bytes_total` -and friends. Host CPU and memory belong to a node exporter. +and friends. During a [shutdown drain](/bin/relay/config#shutdown), +`moq_relay_draining_sessions` counts the sessions sent a GOAWAY that have not +left yet. Host CPU and memory belong to a node exporter. With `--runtime-io-uring`, each QUIC worker thread also reports its own `moq_relay_uring_*` counters under a `worker` label: datagrams and syscalls diff --git a/doc/bin/relay/index.md b/doc/bin/relay/index.md index 9e6393d557..6c18cf5186 100644 --- a/doc/bin/relay/index.md +++ b/doc/bin/relay/index.md @@ -78,8 +78,9 @@ TLS, and a certificate fingerprint for client pinning. The accessors borrow and `run` consumes the relay, so clone `cluster`, `auth`, `client`, `stats`, `shutdown`, and `shutdown_trigger` for application tasks before calling it. `trigger.start()` drains every session with a GOAWAY, -including any that connect afterwards, and `run` returns once the drain window -elapses, with the listeners released and the workers joined. `run` also starts +including any that connect afterwards, and `run` returns once every session +has left or the drain window elapses, with the listeners released and the +workers joined. `run` also starts the drain on SIGTERM or SIGINT. An application that owns those signals, for example to withdraw the node from DNS and wait out the TTL before draining, calls `with_signals(false)` and fires the trigger itself. Build routes from `web().routes()` (or diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md index c8e0a45e04..c5396c5d24 100644 --- a/quest/m1/drain/README.md +++ b/quest/m1/drain/README.md @@ -1,4 +1,4 @@ -# Graceful relay drains (GOAWAY) +# [M] Graceful relay drains (GOAWAY) ## Goal @@ -30,7 +30,9 @@ is moq.pro's (downstream) fleet drain work, which consumes these quests. The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals -included, against one deadline. +included, against one deadline. `Relay::run` returns as soon as every session +has left, logging whether the deadline force-closed any, and +`moq_relay_draining_sessions` shows the drain's progress. **Clients (landed).** The JS reconnector migrates like the Rust one, preserving the app-visible session while resolving DNS again before dialing. @@ -43,11 +45,6 @@ by the stop deadline and encoder reconnect. track through an in-tree relay drained with the drain hook migrates to a second relay behind the same name without a dropped group. -## Quests - -- [Drain exit](/quest/m1/drain/drain-exit.md) - a drain ends as soon as every - session has left, and reports whether that or the deadline ended it - ## Related - [pop-skipping](/quest/m1/pop-skipping/README.md) - its same-PoP link price and full eligible pairing become important when a deployment adds a second relay per PoP diff --git a/quest/m1/drain/drain-exit.md b/quest/m1/drain/drain-exit.md deleted file mode 100644 index 5917a476dd..0000000000 --- a/quest/m1/drain/drain-exit.md +++ /dev/null @@ -1,19 +0,0 @@ -# [S] Drain exit - -## Goal - -`Relay::run` returns as soon as every session has left a drain, instead of -always waiting out the window, and the relay reports which bound ended it: -every session left, or the deadline force-closed the stragglers (and how -many). An orchestrator bounding its stop time can then prove from the log and -`/metrics` which one it hit. - -## Plan - -Today `drain` in `rs/moq-relay/src/relay.rs` sleeps the whole window plus a -second whatever the sessions do. Counting only needs the sessions that go -through `shutdown::Observer::drain_session` (QUIC on either runtime, and -WebSocket), not the `session::Registry`, which skips LAN peers. - -Open question: whether the relay's own outbound cluster sessions count, since -they are not drained by GOAWAY at all. diff --git a/rs/moq-relay/src/config.rs b/rs/moq-relay/src/config.rs index 3fca841c59..69ef3ccc4a 100644 --- a/rs/moq-relay/src/config.rs +++ b/rs/moq-relay/src/config.rs @@ -91,8 +91,9 @@ pub struct Config { /// How long accepted sessions may keep running after a shutdown signal, e.g. /// "10s" or "500ms". The first signal sends every session a GOAWAY and waits - /// this long for clients to reconnect elsewhere before force-closing them; a - /// second signal exits immediately. Zero closes them at once, with no GOAWAY + /// up to this long for clients to reconnect elsewhere before force-closing + /// them, exiting as soon as they have all left; a second signal exits + /// immediately. Zero closes them at once, with no GOAWAY /// they would have no time to act on. Defaults to 10 seconds. #[usage(skip)] #[serde(with = "crate::duration::serde_duration")] diff --git a/rs/moq-relay/src/connection.rs b/rs/moq-relay/src/connection.rs index 8d5e460c84..5faa19dbea 100644 --- a/rs/moq-relay/src/connection.rs +++ b/rs/moq-relay/src/connection.rs @@ -279,7 +279,7 @@ pub(crate) fn authorize( /// the session ([`auth::Lease::ended`]) the session closes with the reason, and /// the session's own close is reported back through the lease as the `end` event. /// Either way, a relay shutdown drains the session with a GOAWAY instead of -/// cutting it off. +/// cutting it off, and does not exit before this returns or the drain deadline. /// /// The session handle is `Send + Sync` whatever transport carries it, so this /// runs on the shared runtime even for sessions a pinned QUIC worker drives. @@ -289,6 +289,7 @@ pub async fn supervise( mut shutdown: crate::shutdown::Observer, registration: Option, ) -> anyhow::Result<()> { + let _serving = shutdown.serve(); loop { let nudged = async { match ®istration { diff --git a/rs/moq-relay/src/internal.rs b/rs/moq-relay/src/internal.rs index c3e576a8a8..493fa77472 100644 --- a/rs/moq-relay/src/internal.rs +++ b/rs/moq-relay/src/internal.rs @@ -9,7 +9,8 @@ //! - `/metrics` - this node's own traffic counters as Prometheus text //! exposition, plus the accept-loop health of its TCP listeners //! ([`with_listeners`](Internal::with_listeners)) and the per-worker health of -//! its io_uring runtime ([`with_uring`](Internal::with_uring)). A distinct plane +//! its io_uring runtime ([`with_uring`](Internal::with_uring)), and the +//! progress of a shutdown drain ([`with_shutdown`](Internal::with_shutdown)). A distinct plane //! from both the customer `web` surface and the MoQ `.stats` broadcast: the same //! atomics, but a different transport and audience (an ops scraper, not a //! customer or the dashboard/billing aggregators). The runtime counters are @@ -89,6 +90,7 @@ pub struct Internal { health: moq_tokio::accept::Health, listeners: Vec, uring: Vec, + shutdown: Option, } #[derive(Clone)] @@ -98,6 +100,7 @@ struct InternalState { sessions: crate::session::Registry, listeners: Vec, uring: Vec, + shutdown: Option, } impl Internal { @@ -122,6 +125,7 @@ impl Internal { health, listeners, uring: Vec::new(), + shutdown: None, } } @@ -172,6 +176,12 @@ impl Internal { self } + /// Report the sessions a shutdown drain is still waiting on at `/metrics`. + pub fn with_shutdown(mut self, shutdown: crate::shutdown::Observer) -> Self { + self.shutdown = Some(shutdown); + self + } + /// Attach the relay cluster used to serve the `/nodes` topology snapshot. pub fn with_cluster(mut self, cluster: &crate::cluster::Cluster) -> Self { self.nodes = Some(cluster.nodes.clone()); @@ -205,6 +215,7 @@ impl Internal { sessions: self.sessions.clone(), listeners: self.listeners.clone(), uring: self.uring.clone(), + shutdown: self.shutdown.clone(), }) } @@ -275,7 +286,10 @@ async fn serve_health() -> Response { /// current cumulative snapshot; a downstream scraper derives rates and live /// counts (`open - closed`). async fn serve_metrics(State(state): State) -> Response { - let body = render_metrics(&state.stats.snapshot(), &state.listeners, &state.uring); + let mut body = render_metrics(&state.stats.snapshot(), &state.listeners, &state.uring); + if let Some(shutdown) = &state.shutdown { + render_drain(&mut body, shutdown.tally()); + } ([(http::header::CONTENT_TYPE, "text/plain; version=0.0.4")], body).into_response() } @@ -454,6 +468,21 @@ fn render_metrics( out } +/// The sessions a shutdown drain is still waiting on: 0 until the drain starts, +/// and back to 0 when every session has left, which is when the relay exits. +/// A scrape that last saw it above 0 shortly before the deadline means the +/// deadline force-closed the rest; the exit log records how many. +fn render_drain(out: &mut String, tally: crate::shutdown::Tally) { + use std::fmt::Write as _; + + let _ = writeln!( + out, + "# HELP moq_relay_draining_sessions Sessions sent a shutdown GOAWAY that have not left yet." + ); + let _ = writeln!(out, "# TYPE moq_relay_draining_sessions gauge"); + let _ = writeln!(out, "moq_relay_draining_sessions {}", tally.draining); +} + /// The accept-loop health of every listener on the node. /// /// The counters are the load-bearing half: a process out of descriptors cannot @@ -863,6 +892,7 @@ mod tests { sessions: crate::session::Registry::new(), listeners: Vec::new(), uring: Vec::new(), + shutdown: None, }; let Json(snapshot) = serve_nodes(State(state)).await; @@ -880,6 +910,7 @@ mod tests { sessions: crate::session::Registry::new(), listeners: Vec::new(), uring: Vec::new(), + shutdown: None, }; let Json(snapshot) = serve_nodes(State(state)).await; diff --git a/rs/moq-relay/src/relay.rs b/rs/moq-relay/src/relay.rs index 953111b772..3b62122cbe 100644 --- a/rs/moq-relay/src/relay.rs +++ b/rs/moq-relay/src/relay.rs @@ -268,7 +268,8 @@ impl Relay { let cluster = cluster.with_stats(stats.clone()); // Graceful shutdown: the first signal drains every accepted session with a - // GOAWAY; a second signal (or the drain window elapsing) exits. + // GOAWAY; the relay exits once they have all left, at the drain deadline, + // or on a second signal. let (shutdown_trigger, shutdown) = shutdown::Observer::new(drain_timeout); let sessions = crate::session::Registry::new(); let (ready, _) = tokio::sync::watch::channel(false); @@ -286,6 +287,7 @@ impl Relay { let internal = internal::Internal::new(config.internal, cluster.stats.clone()) .with_cluster(&cluster) .with_sessions(sessions.clone()) + .with_shutdown(shutdown.clone()) .with_listeners(web.accept_health()) .with_listeners(server.accept_health()); // Bound but not yet serving: registering here (rather than after the @@ -394,8 +396,9 @@ impl Relay { } /// Starts graceful shutdown: every session, including any accepted - /// afterwards, drains with a GOAWAY and [`Self::run`] returns once the drain - /// window elapses. Clone it before `run` consumes the relay. + /// afterwards, drains with a GOAWAY and [`Self::run`] returns once every + /// session has left or the drain window elapses. Clone it before `run` + /// consumes the relay. pub fn shutdown_trigger(&self) -> &shutdown::Trigger { &self.shutdown_trigger } @@ -462,9 +465,10 @@ impl Relay { /// Serve until something fails or shutdown completes: accept sessions, run /// the cluster, and serve both HTTP surfaces. Notifies systemd once - /// everything is up. Returns once the drain window elapses after a signal - /// (see [`Self::with_signals`]) or [`shutdown::Trigger::start`], with every - /// listener released and every worker joined. + /// everything is up. Returns once a drain started by a signal (see + /// [`Self::with_signals`]) or [`shutdown::Trigger::start`] ends, as soon as + /// every session has left or at the drain deadline, with every listener + /// released and every worker joined. /// /// This is also the embedding loop. Extra routes go on via [`Self::with_web`] /// / [`Self::with_internal`] before calling this; cloned handles outlive it. @@ -656,9 +660,10 @@ impl Relay { /// Two-stage shutdown: the first signal, or an embedder firing /// [`shutdown::Trigger::start`], starts the drain broadcast (every session sends -/// GOAWAY and waits for its peer to leave); a second signal, or that recorded -/// deadline plus one second, returns from [`Relay::run`]. Without `signals` -/// only the trigger and that deadline count. +/// GOAWAY and waits for its peer to leave). Returns from [`Relay::run`] once +/// every session has left, which the drain deadline forces, or on a second +/// signal, logging which ended it. +/// Without `signals` only the trigger and the sessions count. async fn drain(trigger: shutdown::Trigger, mut shutdown: shutdown::Observer, signals: bool) -> anyhow::Result<()> { let window = shutdown.drain_timeout; let signal = || async move { @@ -679,19 +684,38 @@ async fn drain(trigger: shutdown::Trigger, mut shutdown: shutdown::Observer, sig _ = shutdown.started() => tracing::info!(?window, "shutdown requested; draining sessions"), } - // One extra second past the deadline fixed when the trigger fired, so - // per-session force-closes fire first. That instant may be earlier than - // this future was polled (the embedder can start the drain during startup), - // and a fresh window here would keep the process up past the time sessions - // were told. + // The deadline fixed when the trigger fired, which may be earlier than this + // future was polled (the embedder can start the drain during startup); a + // fresh window here would keep the process up past the time sessions were + // told. Each session is force-closed at it, so `drained` resolves by then; + // the extra second only bounds a session whose close never completes. let deadline = shutdown.deadline().context("drain started without a deadline")?; - let grace = (deadline + std::time::Duration::from_secs(1)).saturating_duration_since(std::time::Instant::now()); tokio::select! { res = signal() => { res?; - tracing::warn!("second shutdown signal; exiting immediately"); + tracing::warn!(open = shutdown.tally().live, "second shutdown signal; exiting immediately"); + return Ok(()); + } + _ = shutdown.drained() => {} + _ = tokio::time::sleep_until(deadline + std::time::Duration::from_secs(1)) => {} + } + + let elapsed = tokio::time::Instant::now().saturating_duration_since(deadline - window); + match shutdown.tally() { + shutdown::Tally { live: 0, forced: 0, .. } => { + tracing::info!(?elapsed, "drain complete: every session left; exiting") + } + shutdown::Tally { live: 0, forced, .. } => { + tracing::warn!(?elapsed, forced, "drain deadline force-closed sessions; exiting") + } + shutdown::Tally { live, forced, .. } => { + tracing::warn!( + ?elapsed, + forced, + open = live, + "drain deadline passed with sessions still open; exiting" + ) } - _ = tokio::time::sleep(grace) => tracing::info!("drain window elapsed; exiting"), } Ok(()) } diff --git a/rs/moq-relay/src/shutdown.rs b/rs/moq-relay/src/shutdown.rs index a1987c104e..cf7db81675 100644 --- a/rs/moq-relay/src/shutdown.rs +++ b/rs/moq-relay/src/shutdown.rs @@ -5,10 +5,15 @@ //! The drain has one deadline, fixed when it starts. A session accepted after //! that (a cached DNS resolve, a pool alias) is sent a GOAWAY at once, carrying //! only the time left, so no session outlives the window the relay promised. +//! +//! The drain ends as soon as every session has left, or at that deadline. +//! Sessions are counted once established, so one still in +//! its handshake when the last established session leaves is cut off with the +//! process. -use std::time::{Duration, Instant}; +use std::{sync::Arc, time::Duration}; -use tokio::sync::watch; +use tokio::{sync::watch, time::Instant}; /// Fires the relay-wide shutdown broadcast. Held by `Relay::run` for the OS /// signal path; an embedder clones one to stop the relay from its own task. @@ -34,6 +39,17 @@ impl Trigger { } } +/// The sessions a drain is waiting on. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct Tally { + /// Established sessions still open, drained or not. + pub live: usize, + /// Sessions sent a GOAWAY that have not left yet. + pub draining: usize, + /// Sessions still open when their drain deadline passed, and so closed by it. + pub forced: usize, +} + /// A per-connection handle observing the relay-wide shutdown broadcast. /// /// Cheap to clone; each accepted session waits on [`started`](Self::started) @@ -41,6 +57,7 @@ impl Trigger { #[derive(Clone)] pub struct Observer { rx: watch::Receiver>, + tally: Arc>, /// How long a drained session may keep running before it is force-closed. pub drain_timeout: Duration, } @@ -49,7 +66,7 @@ impl Observer { /// Create the trigger and its observer half. pub fn new(drain_timeout: Duration) -> (Trigger, Self) { let (tx, rx) = watch::channel(None); - (Trigger { tx, drain_timeout }, Self { rx, drain_timeout }) + (Trigger { tx, drain_timeout }, Self::from(rx, drain_timeout)) } /// A handle that never fires, for callers without shutdown coordination @@ -59,9 +76,14 @@ impl Observer { // Leak-free: dropping the sender doesn't resolve `started` (it waits for // a deadline, not for channel closure). drop(tx); + Self::from(rx, crate::DEFAULT_DRAIN_TIMEOUT) + } + + fn from(rx: watch::Receiver>, drain_timeout: Duration) -> Self { Self { rx, - drain_timeout: crate::DEFAULT_DRAIN_TIMEOUT, + tally: Arc::new(watch::channel(Tally::default()).0), + drain_timeout, } } @@ -80,6 +102,36 @@ impl Observer { *self.rx.borrow() } + /// Count an established session until the returned guard drops, so the drain + /// waits for it to leave. + pub(crate) fn serve(&self) -> Serving { + self.tally.send_modify(|tally| tally.live += 1); + Serving { + tally: self.tally.clone(), + } + } + + /// The sessions counted so far. + pub(crate) fn tally(&self) -> Tally { + *self.tally.borrow() + } + + /// Resolve once no [`serve`](Self::serve) guard is left. + pub(crate) async fn drained(&self) { + // The sender lives in `self`, so the channel cannot close under the wait. + let _ = self.tally.subscribe().wait_for(|tally| tally.live == 0).await; + } + + /// Count a session as draining until the returned guard drops, and as forced + /// if it is still open at `deadline`. + fn draining(&self, deadline: Instant) -> Draining { + self.tally.send_modify(|tally| tally.draining += 1); + Draining { + tally: self.tally.clone(), + deadline, + } + } + /// Drain `session` with an empty-URI GOAWAY ("reconnect to me"), waiting for /// the peer to leave. /// @@ -93,10 +145,12 @@ impl Observer { /// With no time left (a zero [`drain_timeout`](Self::drain_timeout), or a /// session accepted after the deadline) the session is closed at once. pub async fn drain_session(&self, session: &moq_net::Session) { - let remaining = match *self.rx.borrow() { - Some(deadline) => deadline.saturating_duration_since(Instant::now()), - None => self.drain_timeout, - }; + let now = Instant::now(); + let deadline = self.deadline().unwrap_or(now + self.drain_timeout); + let remaining = deadline.saturating_duration_since(now); + // Dropped when this returns or is cancelled (a WebSocket driver ending + // first), so the count holds whichever way the session goes. + let _draining = self.draining(deadline); // No grace left, so there is nothing to drain. Close now rather than send a // GOAWAY: the peer would have no time to act on it, and a zero timeout means @@ -119,3 +173,78 @@ impl Observer { session.closed().await; } } + +/// An established session the drain waits on; see [`Observer::serve`]. +pub(crate) struct Serving { + tally: Arc>, +} + +impl Drop for Serving { + fn drop(&mut self) { + self.tally.send_modify(|tally| tally.live -= 1); + } +} + +/// A session sent a GOAWAY; see [`Observer::draining`]. +struct Draining { + tally: Arc>, + deadline: Instant, +} + +impl Drop for Draining { + fn drop(&mut self) { + // The driver arms its force-close after sending the GOAWAY, so it never + // fires before `deadline` and every session it closes is counted. A peer + // leaving in that sliver was still there at the deadline too. + let forced = Instant::now() >= self.deadline; + self.tally.send_modify(|tally| { + tally.draining -= 1; + tally.forced += usize::from(forced); + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test(start_paused = true)] + async fn drained_waits_for_every_session() { + let (trigger, shutdown) = Observer::new(Duration::from_secs(10)); + let first = shutdown.serve(); + let second = shutdown.serve(); + trigger.start(); + + let drained = tokio::spawn({ + let shutdown = shutdown.clone(); + async move { shutdown.drained().await } + }); + drop(first); + tokio::task::yield_now().await; + assert!(!drained.is_finished(), "drained with a session still open"); + + drop(second); + drained.await.expect("drained task"); + assert_eq!(shutdown.tally(), Tally::default()); + } + + #[tokio::test(start_paused = true)] + async fn a_session_open_at_the_deadline_is_forced() { + let window = Duration::from_secs(10); + let (trigger, shutdown) = Observer::new(window); + trigger.start(); + let deadline = shutdown.deadline().expect("started"); + + let left = shutdown.draining(deadline); + let straggler = shutdown.draining(deadline); + assert_eq!(shutdown.tally().draining, 2); + + drop(left); + tokio::time::sleep(window).await; + drop(straggler); + + let tally = shutdown.tally(); + assert_eq!(tally.draining, 0); + assert_eq!(tally.forced, 1, "only the session open at the deadline is forced"); + } +} diff --git a/rs/moq-relay/src/websocket.rs b/rs/moq-relay/src/websocket.rs index 6093d0ba95..b2ebba9428 100644 --- a/rs/moq-relay/src/websocket.rs +++ b/rs/moq-relay/src/websocket.rs @@ -197,6 +197,7 @@ where // The handshake is done, so this is a MoQ session now: only now can a push // be serviced, and only now does the session appear in the live table. let registration = pending.map(|(sessions, request)| sessions.register(request)); + let _serving = shutdown.serve(); loop { let nudged = async { diff --git a/rs/moq-relay/tests/shutdown_signal.rs b/rs/moq-relay/tests/shutdown_signal.rs index b23ae043ea..6e505cd3fa 100644 --- a/rs/moq-relay/tests/shutdown_signal.rs +++ b/rs/moq-relay/tests/shutdown_signal.rs @@ -17,6 +17,9 @@ //! itself; a session that still arrives mid-drain is sent a GOAWAY at once, //! carrying only what is left of the window. //! +//! The drain ends as soon as every session has left rather than waiting out the +//! window, which a stop-time budget depends on. +//! //! Each signal test raises or handles process signals, so they rely on //! nextest's process-per-test isolation. `a_trigger_before_run_keeps_the_deadline` //! fires the trigger before `run` instead of a signal. @@ -28,8 +31,8 @@ use std::{net::TcpListener, time::Duration}; use moq_relay::{Config, Relay, auth}; /// Long enough that "exited immediately" and "waited out the window" cannot be -/// confused, short enough to keep the test quick: `Relay::run` sleeps this plus -/// one second before exiting. +/// confused, short enough to keep the test quick: a session that never leaves +/// keeps `Relay::run` up this long. const DRAIN_TIMEOUT: Duration = Duration::from_secs(3); /// Run `test` on a current-thread runtime with a large stack. @@ -66,6 +69,11 @@ fn a_session_arriving_mid_drain_gets_what_is_left() { run_test(a_session_arriving_mid_drain_gets_what_is_left_inner); } +#[test] +fn a_drain_ends_once_every_session_leaves() { + run_test(a_drain_ends_once_every_session_leaves_inner); +} + #[test] fn a_trigger_before_run_keeps_the_deadline() { run_test(a_trigger_before_run_keeps_the_deadline_inner); @@ -88,6 +96,9 @@ async fn sigint_drains_sessions_before_exiting_inner() { let connection = connect(&client, port).await; let draining = connection.draining().expect("connected"); + // The one-shot client leaves on the GOAWAY; this peer is what keeps the drain + // open for its whole window. + let _straggler = straggler(port).await; let signalled = std::time::Instant::now(); // SAFETY: `raise` is async-signal-safe, and SIGINT's disposition is tokio's @@ -142,6 +153,7 @@ async fn an_embedder_owns_the_signals_inner() { let connection = connect(&client, port).await; let draining = connection.draining().expect("connected"); + let _straggler = straggler(port).await; // SAFETY: `raise` is async-signal-safe, and SIGINT's disposition is tokio's // handler, registered above. @@ -193,15 +205,15 @@ async fn a_session_arriving_mid_drain_gets_what_is_left_inner() { let client = client(vec!["moq-transport-17".parse().expect("parse version")]); let established = connect(&client, port).await; + let draining = established.draining().expect("connected"); + // Keeps the relay up for the arrival below once `established` leaves. + let _straggler = straggler(port).await; trigger.start(); let deadline = std::time::Instant::now() + DRAIN_TIMEOUT; - let goaway = tokio::time::timeout( - Duration::from_secs(5), - established.draining().expect("connected").recv(), - ) - .await - .expect("no GOAWAY within 5s of the trigger") - .expect("session closed without a GOAWAY"); + let goaway = tokio::time::timeout(Duration::from_secs(5), draining.recv()) + .await + .expect("no GOAWAY within 5s of the trigger") + .expect("session closed without a GOAWAY"); assert_eq!(goaway.uri(), "", "expected a reconnect-to-me GOAWAY"); let timeout = goaway.timeout().expect("the GOAWAY carries its deadline"); assert!( @@ -236,6 +248,39 @@ async fn a_session_arriving_mid_drain_gets_what_is_left_inner() { .expect("relay exited with an error"); } +async fn a_drain_ends_once_every_session_leaves_inner() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + // A window the test would time out long before, so only an early exit passes. + let (port, mut config) = relay_config(); + config.drain_timeout = Duration::from_secs(600); + let internal = free_port(); + config.internal.listen = Some(format!("127.0.0.1:{internal}").parse().expect("parse addr")); + let relay = Relay::load(config).await.expect("load relay").with_signals(false); + let trigger = relay.shutdown_trigger().clone(); + let run = tokio::spawn(relay.run()); + wait_listening(port).await; + + let left = connect(&client(Vec::new()), port).await; + let straggler = straggler(port).await; + trigger.start(); + + // The one-shot client leaves on its GOAWAY; the straggler is still draining. + tokio::time::timeout(Duration::from_secs(5), left.closed()) + .await + .expect("the one-shot client did not leave on the GOAWAY") + .expect("the one-shot client failed"); + wait_metric(internal, "moq_relay_draining_sessions 1").await; + assert!(!run.is_finished(), "the relay exited with a session still draining"); + + drop(straggler); + tokio::time::timeout(Duration::from_secs(5), run) + .await + .expect("relay kept running after every session left") + .expect("relay task panicked") + .expect("relay exited with an error"); +} + async fn a_trigger_before_run_keeps_the_deadline_inner() { let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); @@ -275,20 +320,30 @@ fn client(version: Vec) -> moq_tokio::Client { .with_reconnect(false) } +/// A session that stays until the drain deadline closes it: moq-lite-03 has no +/// GOAWAY message, so the relay drains it without the peer ever knowing. +async fn straggler(port: u16) -> moq_tokio::Connection { + connect(&client(vec!["moq-lite-03".parse().expect("parse version")]), port).await +} + /// A session to the relay on `port`. async fn connect(client: &moq_tokio::Client, port: u16) -> moq_tokio::Connection { let url: url::Url = format!("tcp://127.0.0.1:{port}/").parse().expect("parse url"); client.connect(url).established().await.expect("connect") } +/// A free loopback TCP port. The listener is bound by `Relay::run`, not here, +/// so this leaves the usual probe/bind gap; on loopback it is not worth +/// retrying around. +fn free_port() -> u16 { + let probe = TcpListener::bind("127.0.0.1:0").expect("bind probe"); + probe.local_addr().expect("local addr").port() +} + /// A stream-only relay on a free loopback TCP port, fully public, with a short /// drain window. Returns the port and the config to hand [`Relay::load`]. fn relay_config() -> (u16, Config) { - // The listener is bound by `Relay::run`, not here, so this leaves the usual - // probe/bind gap; on loopback it is not worth retrying around. - let probe = TcpListener::bind("127.0.0.1:0").expect("bind probe"); - let port = probe.local_addr().expect("local addr").port(); - drop(probe); + let port = free_port(); // Fully public auth: any no-JWT stream client gets the whole root. let mut auth = auth::Config::default(); @@ -302,6 +357,24 @@ fn relay_config() -> (u16, Config) { (port, config) } +/// Wait until the internal listener on `port` reports `line` at `/metrics`. +async fn wait_metric(port: u16, line: &str) { + let deadline = std::time::Instant::now() + Duration::from_secs(5); + loop { + let metrics = reqwest::get(format!("http://127.0.0.1:{port}/metrics")) + .await + .expect("scrape metrics") + .text() + .await + .expect("read metrics"); + if metrics.lines().any(|l| l == line) { + break; + } + assert!(std::time::Instant::now() < deadline, "never saw {line} in:\n{metrics}"); + tokio::time::sleep(Duration::from_millis(25)).await; + } +} + async fn wait_listening(port: u16) { let deadline = std::time::Instant::now() + Duration::from_secs(5); loop { From 4665c13befe865caa4d1785e582b74f08fe3e464 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 26 Sep 2026 13:57:11 -0700 Subject: [PATCH 05/15] test(relay): hold an io_uring drain open with a straggler The drain now ends once every counted session leaves (#4186), so the io_uring drain test raced its own relay: a lite-06 client sees its session before the relay counts it, the trigger found nothing to wait for, and the relay exited before sending a GOAWAY. Hold a moq-lite-03 straggler as shutdown_signal.rs does, and trigger only once the relay lists both sessions. Co-Authored-By: Claude Opus 5.5 --- rs/moq-relay/tests/runtime_uring.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/rs/moq-relay/tests/runtime_uring.rs b/rs/moq-relay/tests/runtime_uring.rs index d5f8821754..0565b06ab0 100644 --- a/rs/moq-relay/tests/runtime_uring.rs +++ b/rs/moq-relay/tests/runtime_uring.rs @@ -234,8 +234,8 @@ async fn uring_workers_report_link_facts() { /// The shutdown trigger drains sessions the io_uring workers serve as it does /// the shared runtime's: an established session and one arriving mid-drain -/// are each sent a GOAWAY and leave, and `run` then returns with the worker -/// threads joined and the port free. +/// are each sent a GOAWAY and leave, and `run` then returns at the deadline +/// with the worker threads joined and the port free. /// /// What an arrival is told is left of the window only reaches the wire on /// moq-transport-17+, which the workers do not speak; `shutdown_signal.rs` @@ -257,6 +257,7 @@ async fn uring_workers_drain_on_the_trigger() { let relay = Relay::load(config).await.expect("load relay").with_signals(false); let port = relay.addr().expect("workers bound an address").port(); let trigger = relay.shutdown_trigger().clone(); + let sessions = relay.sessions().clone(); let running = tokio::spawn(relay.run()); // One-shot (see `client`), so a session leaves on its GOAWAY rather than @@ -265,6 +266,22 @@ async fn uring_workers_drain_on_the_trigger() { let url: url::Url = format!("moql://127.0.0.1:{port}/drain").parse().expect("parse url"); let established = connect(client.clone(), url.clone()).await; + // moq-lite-03 has no GOAWAY, so this peer stays until the deadline closes it, + // keeping the drain open for the arrival below once `established` leaves. + let mut straggler = moq_tokio::connect::Config::default(); + straggler.tls.insecure = Some(true); + straggler.once = Some(true); + straggler.bind = Some("127.0.0.1:0".parse().expect("parse bind")); + straggler.version = vec!["moq-lite-03".parse().expect("parse version")]; + let _straggler = connect(straggler.init(Default::default()).expect("client init"), url.clone()).await; + + // A client can see its session established before the relay counts it, and a + // drain with nothing counted ends at once. + let deadline = std::time::Instant::now() + TIMEOUT; + while sessions.list(&Default::default()).len() < 2 { + assert!(std::time::Instant::now() < deadline, "the relay never listed both sessions"); + tokio::time::sleep(Duration::from_millis(25)).await; + } trigger.start(); let goaway = tokio::time::timeout(TIMEOUT, established.draining().expect("connected").recv()) .await From 14519f4ed59d455f7d9028fd505aaa9eb439406e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 26 Sep 2026 14:08:09 -0700 Subject: [PATCH 06/15] test(drain): a JS viewer migrates off a draining relay without a dropped group Completes the drain line: `just test drain` stands up relay B with a publisher and relay A clustered to it, points a stand-in for DNS at A, and has a @moq/net viewer watch the track through it. The driver then withdraws A from the name, sends it SIGTERM, and requires the viewer to read fresh groups on B within the handover window with none missing, to dial A only once, and A to exit on its own once every session left. Runs nightly. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/nightly.yml | 5 +- bun.lock | 8 + package.json | 1 + quest/m1/README.md | 1 - quest/m1/drain/README.md | 50 ------ quest/m1/pop-skipping/README.md | 1 - quest/m1/transport-upgrade/README.md | 10 +- quest/m1/transport-upgrade/js.md | 9 +- quest/m2/quic-careful-resume.md | 1 - test/README.md | 3 +- test/drain/README.md | 52 ++++++ test/drain/drain.ts | 234 +++++++++++++++++++++++++++ test/drain/package.json | 8 + test/drain/relay.toml | 27 ++++ test/drain/run.sh | 137 ++++++++++++++++ test/justfile | 8 + 16 files changed, 486 insertions(+), 69 deletions(-) delete mode 100644 quest/m1/drain/README.md create mode 100644 test/drain/README.md create mode 100644 test/drain/drain.ts create mode 100644 test/drain/package.json create mode 100644 test/drain/relay.toml create mode 100755 test/drain/run.sh diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 055ba9711f..b67f9b2f9e 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -135,7 +135,10 @@ jobs: matrix: # `rs uring` is a separate feature compile (off the default set) and # kernel-gated below 6.12. The embedding tests for io_uring live there. - recipe: ["rs doctest --workspace", "rs loom", "test drill-sensitivity", "rs uring"] + # `test drain` stands up two relays and drains one under a live JS + # viewer: diff-independent, since a break can arrive through the relay, + # js/net, or the wire between them. + recipe: ["rs doctest --workspace", "rs loom", "test drill-sensitivity", "rs uring", "test drain"] steps: - name: Free disk space uses: jlumbroso/free-disk-space@54081f138730dfa15788a46383842cd2f914a1be # main diff --git a/bun.lock b/bun.lock index 4005df50c8..d730db935d 100644 --- a/bun.lock +++ b/bun.lock @@ -343,6 +343,12 @@ "vite": "^8.3.0", }, }, + "test/drain": { + "name": "@moq/drain-test", + "dependencies": { + "@moq/net": "workspace:*", + }, + }, "test/interop/clients/js": { "name": "@moq/interop-browser", "dependencies": { @@ -652,6 +658,8 @@ "@moq/demo-boy": ["@moq/demo-boy@workspace:demo/boy"], + "@moq/drain-test": ["@moq/drain-test@workspace:test/drain"], + "@moq/flate": ["@moq/flate@workspace:js/flate"], "@moq/hang": ["@moq/hang@workspace:js/hang"], diff --git a/package.json b/package.json index bffe7256c3..2ab0266273 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "js/moq-boy", "test/interop/clients/js", "test/interop/clients/js-native", + "test/drain", "test/wasm" ] } diff --git a/quest/m1/README.md b/quest/m1/README.md index b97087b518..6e54791eb5 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -62,7 +62,6 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Keyframe trigger](/quest/m1/keyframe-trigger.md) - an application can ask the built-in capture encoder for a keyframe - [Video keyframe flag](/quest/m1/video-keyframe-flag.md) - encoded video marks its keyframes, so a requested cut never forces an extra one after a cadence keyframe - [QoS](/quest/m1/qos/README.md) - broadcast health: relay starvation and timeliness histograms, and client stats broadcasts from publishers and viewers -- [Drain](/quest/m1/drain/README.md) - relay restarts drain sessions over GOAWAY instead of hard-dropping them - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - a session that came up over WebSocket moves to QUIC once the QUIC dial lands, handing over at a group boundary - [Own the QUIC stack](/quest/m1/quic/README.md) - the moq-noq fork carries ACK progress, reliable reset, hierarchical scheduling, deadlines, probing, diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md deleted file mode 100644 index c5396c5d24..0000000000 --- a/quest/m1/drain/README.md +++ /dev/null @@ -1,50 +0,0 @@ -# [M] Graceful relay drains (GOAWAY) - -## Goal - -Relay restarts drain sessions instead of hard-dropping them. The end state: a -draining node is first withdrawn from DNS (marked unhealthy so resolvers stop -handing it out), waits out the DNS TTL plus a margin for monitor detection, -THEN sends GOAWAY on every MoQ session. Clients reconnect through a fresh DNS -resolve and land on a different relay, and a straggler that dials the draining -node anyway (a cached resolve, or a pool alias) just gets another GOAWAY. Only -after sessions drain or the stop deadline expires does the process exit and -the new software boot. - -GOAWAY only reaches MoQ sessions. Both clients migrate on it: -`moq_tokio::Connection` and the `js/net` `Connection` dial the replacement -through a fresh resolve while the old session drains. The wire message is -lite04+/IETF only besides. So the stop deadline is the -real backstop - for pre-lite04 versions, for client SDKs deployed before -the JS migration shipped, and for in-process ingest gateways (RTMP/SRT/WHIP/WHEP), -which have no GOAWAY equivalent at all: their grace is the DNS-drain window -stopping new arrivals plus the encoder's own reconnect. The DNS-drain-first -ordering is what keeps that hard-close window small. - -## Plan - -This questline holds the two relay/client halves. The orchestration around -them (a planned-drain health state, the SIGTERM sequencing and stop timeouts, -per-PoP serial deploys, a two-node PoP floor, and the gateway drain contract) -is moq.pro's (downstream) fleet drain work, which consumes these quests. - -The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM -to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals -included, against one deadline. `Relay::run` returns as soon as every session -has left, logging whether the deadline force-closed any, and -`moq_relay_draining_sessions` shows the drain's progress. - -**Clients (landed).** The JS reconnector migrates like the Rust one, -preserving the app-visible session while resolving DNS again before dialing. -Both are covered against stand-in servers. This is a -scale-down prerequisite, not merely a deploy improvement. RTMP/SRT/WHIP/WHEP -cannot receive MoQ GOAWAY, so their contract remains DNS withdrawal followed -by the stop deadline and encoder reconnect. - -**End to end.** The line's own remaining work: a JS client watching a live -track through an in-tree relay drained with the drain hook migrates to a -second relay behind the same name without a dropped group. - -## Related - -- [pop-skipping](/quest/m1/pop-skipping/README.md) - its same-PoP link price and full eligible pairing become important when a deployment adds a second relay per PoP diff --git a/quest/m1/pop-skipping/README.md b/quest/m1/pop-skipping/README.md index c1399c0b19..eeccd643e7 100644 --- a/quest/m1/pop-skipping/README.md +++ b/quest/m1/pop-skipping/README.md @@ -153,6 +153,5 @@ costs of one bidirectional session, which one `?cost=` cannot split. ## Related -- [drain](/quest/m1/drain/README.md) - a second relay per PoP makes the same-PoP link price and its connection cardinality operationally important - [wildcard](/quest/m1/wildcard/README.md) - it reuses this questline's route cost, and needs a cluster on Lite06 - [relay-memory](/quest/m1/relay-memory.md) - a denser mesh multiplies whatever a non-selected route costs diff --git a/quest/m1/transport-upgrade/README.md b/quest/m1/transport-upgrade/README.md index 194c13b0bd..57ae183627 100644 --- a/quest/m1/transport-upgrade/README.md +++ b/quest/m1/transport-upgrade/README.md @@ -33,8 +33,8 @@ every version (a client may send one with an empty URI; only a redirect URI is forbidden to a moq-transport client). The origin's multi-route front prefers the newest of two equal routes and `resume` splices each track at a group boundary, capping the old segment so the old session's subscription ends at the -boundary on its own. The JavaScript handover ships with the -[drain](/quest/m1/drain/README.md) line, so the JS half requires it. +boundary on its own. The JavaScript half reuses `Reload`'s GOAWAY migration in +`js/net`. Shared decisions: @@ -57,8 +57,4 @@ Shared decisions: ## Quests - [Rust](/quest/m1/transport-upgrade/rust.md) - moq-tokio keeps the QUIC dial after WebSocket wins and migrates through the existing Draining path -- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through the drain line's GOAWAY handover - -## Related - -- [Drain](/quest/m1/drain/README.md) - the peer-initiated half of the same handover +- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through its GOAWAY handover diff --git a/quest/m1/transport-upgrade/js.md b/quest/m1/transport-upgrade/js.md index f98faccec7..719c8284d3 100644 --- a/quest/m1/transport-upgrade/js.md +++ b/quest/m1/transport-upgrade/js.md @@ -11,8 +11,7 @@ nothing changes. ## Plan -Lands in `js/net`, after the [drain](/quest/m1/drain/README.md) line ships the -GOAWAY handover it reuses (`Reload`'s migration in +Lands in `js/net`, reusing the GOAWAY handover (`Reload`'s migration in `js/net/src/connection/reload.ts`): dial the replacement while the old session keeps serving, let both feed the origin (a request holds the outranked route until the new one answers), leave the old session to close on its own or at the @@ -39,10 +38,6 @@ handover cap. See the group across the upgrade, the WebSocket session closes within the cap, and the next connect to the same URL gives WebTransport the head start again; with no delay, WebTransport wins and no WebSocket session is ever opened. -- Public API: none beyond what the drain line adds; `transportOf` already +- Public API: none; `transportOf` already reports the live transport. Update `doc/lib/js` where the fallback race is described. - -## Required - -- [Graceful relay drains](/quest/m1/drain/README.md) - ships the JS GOAWAY handover this upgrade reuses diff --git a/quest/m2/quic-careful-resume.md b/quest/m2/quic-careful-resume.md index 30d906eb0a..cf43686780 100644 --- a/quest/m2/quic-careful-resume.md +++ b/quest/m2/quic-careful-resume.md @@ -34,4 +34,3 @@ only when the jump never makes the first second worse than slow start. - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - one of the reconnects this speeds up -- [Drain](/quest/m1/drain/README.md) - GOAWAY redials are the other diff --git a/test/README.md b/test/README.md index 5c4f994c43..1333b765f7 100644 --- a/test/README.md +++ b/test/README.md @@ -8,8 +8,9 @@ tests live in each language's own justfile. | [interop](interop/README.md) | `just test interop` | every client built from this checkout interoperates | | [wasm](wasm/README.md) | `just test wasm` | the `@moq/wasm` bindings work in a real browser | | [ts](ts/README.md) | `just test ts` | the subscriber's `export ts` output is IRD-compliant | +| [drain](drain/README.md) | `just test drain` | a JS viewer migrates off a draining relay without a dropped group | -All three stand up a `moq-relay` and clients, so two of them running at once, or +All four stand up a `moq-relay` and clients, so two of them running at once, or the same one running from two worktrees, would otherwise collide. `lib/harness.sh` is what keeps them apart. diff --git a/test/drain/README.md b/test/drain/README.md new file mode 100644 index 0000000000..92eebde688 --- /dev/null +++ b/test/drain/README.md @@ -0,0 +1,52 @@ +# Relay drain harness + +A viewer watching a live track through a relay that drains moves to a sibling +relay behind the same name without missing a group. This is the whole drain +story at once: the relay's GOAWAY, the JS client's migration on it, and the +origin handing the path over while the old session still serves. + +## Running + +```bash +just test drain +``` + +```bash +just test drain --timeout 120 +``` + +`cargo build -p moq-relay` builds the relay from this checkout. `DRAIN_PROFILE` +picks its cargo profile, and `RELAY_BIN` points at a prebuilt relay instead. +Ports come from the shared reservation (see [the harness contract](../README.md)). +A failing run keeps its run directory with both relay logs; a passing run +deletes its own. + +## Shape + +`run.sh` starts relay B, then relay A clustered to B, and hands both to +`drain.ts`, which plays every other part: + +- **The name.** A TCP proxy stands in for DNS. The viewer dials it and lands on + A. Pointing it at B before draining A is the fleet's DNS withdrawal: nothing + new resolves to A, while the sessions already there stay. +- **The publisher** sits on B and writes one group every 100 ms, each carrying + its own sequence number. A pulls the track through the cluster, so both relays + serve the same groups. +- **The viewer** is `@moq/net` over WebSocket, following whichever broadcast the + path routes to the way a player does: subscribe to the new one, drop the old. + +Once the viewer has read groups through A, the driver withdraws A from the name +and sends it SIGTERM, which fires the same trigger an embedder's drain hook +does. The run passes when: + +- the viewer reads fresh groups through B within a few seconds, well inside A's + 20 s drain window, having dialed A exactly once; +- every group from the first read through the last arrived, from one relay or + the other; +- the viewer leaves A at its 2 s handover cap, and A then exits on its own, + logging that every session left rather than that its deadline forced one out. + +The viewer subscribes with a 1 s latency budget, as the interop subscribers do. +After the swap, B has to subscribe upstream afresh once A drops its pull, and the +budget is what reaches back to a group in flight across the swap. With no budget, +a group boundary that lands inside the swap loses that group. diff --git a/test/drain/drain.ts b/test/drain/drain.ts new file mode 100644 index 0000000000..9daf6e243c --- /dev/null +++ b/test/drain/drain.ts @@ -0,0 +1,234 @@ +/** + * Drives one relay drain end to end: a viewer watching a live track through relay A, behind a + * stand-in for DNS, migrates to relay B when A drains, without missing a group. + * + * The "name" the viewer dials is a TCP proxy owned by this script. Pointing it at B before A + * drains is the fleet's DNS withdrawal: nothing new resolves to A, while sessions already on A + * stay put. A then gets SIGTERM, sends every session a GOAWAY, and the viewer redials the same + * URL, which now lands on B. The publisher sits on B and A pulls it through the cluster, so both + * relays carry the same track with the same group numbers. + * + * bun drain.ts --a-port 4470 --b-port 4471 --proxy-port 4472 --a-pid 1234 --timeout 60 + * + * Exits 0 once the viewer has migrated, read groups on both relays with none missing in + * between, and left A on its own; 1 otherwise. + * + * @module + */ +import * as net from "node:net"; +import { parseArgs } from "node:util"; +import * as Moq from "@moq/net"; + +const { values } = parseArgs({ + options: { + "a-port": { type: "string" }, + "b-port": { type: "string" }, + "proxy-port": { type: "string" }, + "a-pid": { type: "string" }, + timeout: { type: "string", default: "60" }, + }, +}); + +const aPort = Number(values["a-port"]); +const bPort = Number(values["b-port"]); +const proxyPort = Number(values["proxy-port"]); +const aPid = Number(values["a-pid"]); +const timeoutMs = Number(values.timeout) * 1000; +if (![aPort, bPort, proxyPort, aPid, timeoutMs].every((n) => Number.isInteger(n) && n > 0)) { + console.error("usage: drain.ts --a-port P --b-port P --proxy-port P --a-pid PID [--timeout S]"); + process.exit(2); +} + +// Groups the viewer must read on each relay: enough to prove the track is live there, +// not merely that one group slipped through. +const GROUPS_PER_RELAY = 10; +// A new group every 100ms, one frame each carrying its own sequence number. +const GROUP_INTERVAL_MS = 100; +// How long the viewer's old session may keep serving after the GOAWAY. Well inside the relay's +// 20s drain window, so the relay's exit log can prove the viewer left on its own. +const HANDOVER = Moq.Time.Milli(2000); +// The viewer's latency budget, as a player sets one (the interop subscribers use the same). +// Following the route means resubscribing on B, and B has to subscribe upstream afresh once A +// drops its pull; the budget is what lets that resubscribe reach back to a group that was in +// flight across the swap instead of starting at the next one. With none, a group boundary +// landing inside the swap drops that group. +const MAX_AGE = Moq.Time.Milli(1000); + +const path = Moq.Path.from("drain"); +const trackName = "seq"; + +function log(message: string) { + console.error(`[drain] ${message}`); +} + +// Resolve once `pred` holds, re-checking every tick; fails with `what` at the deadline. +async function until(what: string, pred: () => boolean, ms = timeoutMs): Promise { + const deadline = performance.now() + ms; + while (!pred()) { + if (performance.now() > deadline) throw new Error(`timed out waiting for ${what}`); + await new Promise((resolve) => setTimeout(resolve, 20)); + } +} + +// ── the name: a TCP proxy standing in for DNS ───────────────────────────────── +interface Backend { + port: number; + /** Connections ever sent here. */ + dialed: number; + /** Connections still open. */ + open: number; +} +const a: Backend = { port: aPort, dialed: 0, open: 0 }; +const b: Backend = { port: bPort, dialed: 0, open: 0 }; +let resolved = a; + +const proxy = net.createServer((client) => { + const backend = resolved; + backend.dialed++; + backend.open++; + const upstream = net.connect(backend.port, "127.0.0.1"); + client.pipe(upstream).pipe(client); + let closed = false; + const close = () => { + if (closed) return; + closed = true; + backend.open--; + client.destroy(); + upstream.destroy(); + }; + for (const socket of [client, upstream]) { + socket.on("error", close); + socket.on("close", close); + } +}); +await new Promise((resolve, reject) => { + proxy.once("error", reject); + proxy.listen(proxyPort, "127.0.0.1", resolve); +}); + +// ── publisher on B ──────────────────────────────────────────────────────────── +const published = new Moq.Origin.Producer(); +const broadcast = published.createBroadcast(path); +const track = broadcast.createTrack(trackName); +broadcast.announce(); +const publisher = new Moq.Connection({ url: new URL(`http://127.0.0.1:${bPort}/`), publish: published.consume() }); + +let lastPublished = -1; +const ticker = setInterval(() => { + const group = track.appendGroup(); + group.writeString(String(group.sequence)); + group.close(); + lastPublished = group.sequence; +}, GROUP_INTERVAL_MS); + +// ── viewer through the name ─────────────────────────────────────────────────── +/** Which relay delivered each group: the generation of the broadcast it was read from. */ +const seen = new Map>(); +let generation = -1; + +const watched = new Moq.Origin.Producer(); +const viewer = new Moq.Connection({ + url: new URL(`http://127.0.0.1:${proxyPort}/`), + consume: watched, + goaway: { handover: HANDOVER }, +}); +const request = watched.request(path, { announced: true }); + +async function read(sub: Moq.Track.Subscriber, gen: number): Promise { + try { + for (;;) { + const group = await sub.recvGroup(); + if (!group) return; + const text = await group.readString(); + if (text !== String(group.sequence)) throw new Error(`group ${group.sequence} carried ${text}`); + let gens = seen.get(group.sequence); + if (!gens) { + gens = new Set(); + seen.set(group.sequence, gens); + } + gens.add(gen); + } + } catch (err) { + // The broadcast it was read from went away; its successor picks up from here. + log(`generation ${gen} ended: ${err instanceof Error ? err.message : String(err)}`); + } +} + +// Follow whichever broadcast the path routes to, as a player does: subscribe to the new one and +// drop the old one each time the route changes. +let watching = true; +const follow = (async () => { + let current: { broadcast: Moq.Broadcast.Consumer; sub: Moq.Track.Subscriber } | undefined; + while (watching) { + const active = request.active.peek(); + if (active && active !== current?.broadcast) { + generation++; + log(`watching generation ${generation}`); + const sub = active.track(trackName).subscribe({ maxAge: MAX_AGE }); + void read(sub, generation); + current?.sub.close(); + current = { broadcast: active, sub }; + } + await request.active.changed(); + } + current?.sub.close(); +})(); + +const readOn = (gen: number) => [...seen.values()].filter((gens) => gens.has(gen)).length; +const newestOn = (gen: number) => Math.max(-1, ...[...seen].filter(([, gens]) => gens.has(gen)).map(([seq]) => seq)); + +let failure: Error | undefined; +try { + await until(`${GROUPS_PER_RELAY} groups through relay A`, () => generation === 0 && readOn(0) >= GROUPS_PER_RELAY); + if (a.dialed !== 1 || b.dialed !== 0) throw new Error(`expected one dial to A, saw A=${a.dialed} B=${b.dialed}`); + + log("withdrawing A from the name, then draining it"); + resolved = b; + process.kill(aPid, "SIGTERM"); + + // Counted past the last group A delivered: B's first answer can include recent groups the + // viewer already had, which prove nothing about B being live. + // Bounded well inside A's 20s drain window: a viewer that only reconnects once A cuts it off + // at the deadline would otherwise pass, just late. + await until( + `${GROUPS_PER_RELAY} new groups through relay B`, + () => generation >= 1 && newestOn(generation) >= newestOn(0) + GROUPS_PER_RELAY, + HANDOVER * 3, + ); + // The old session leaves at the handover cap, before A's own deadline would cut it. + await until("the viewer to leave relay A", () => a.open === 0, HANDOVER * 3); + + if (generation !== 1) throw new Error(`expected exactly one migration, saw ${generation}`); + if (a.dialed !== 1) throw new Error(`the viewer redialed the withdrawn relay (${a.dialed} dials)`); + if (b.dialed < 1) throw new Error("the viewer never dialed relay B"); + + const sequences = [...seen.keys()].sort((x, y) => x - y); + const first = sequences[0] ?? 0; + const last = sequences[sequences.length - 1] ?? 0; + const missing: number[] = []; + for (let seq = first; seq <= last; seq++) { + if (!seen.has(seq)) missing.push(seq); + } + const both = sequences.filter((seq) => seen.get(seq)?.size === 2).length; + log(`read groups ${first}..${last} (published up to ${lastPublished}); ${both} arrived from both relays`); + if (missing.length > 0) throw new Error(`dropped groups across the migration: ${missing.join(", ")}`); + log("migrated without a dropped group"); +} catch (err) { + failure = err instanceof Error ? err : new Error(String(err)); +} finally { + watching = false; + clearInterval(ticker); + request.close(); + viewer.close(); + publisher.close(); + broadcast.close(); + proxy.close(); +} +// `follow` parks on a route change that may never come once the request closes. +void follow; + +if (failure) { + console.error(`error: ${failure.message}`); + process.exit(1); +} +process.exit(0); diff --git a/test/drain/package.json b/test/drain/package.json new file mode 100644 index 0000000000..ed14027cc4 --- /dev/null +++ b/test/drain/package.json @@ -0,0 +1,8 @@ +{ + "name": "@moq/drain-test", + "private": true, + "type": "module", + "dependencies": { + "@moq/net": "workspace:*" + } +} diff --git a/test/drain/relay.toml b/test/drain/relay.toml new file mode 100644 index 0000000000..f64e909029 --- /dev/null +++ b/test/drain/relay.toml @@ -0,0 +1,27 @@ +# Relay config for the drain harness. +# Anonymous access, self-signed localhost cert, QUIC + HTTP on 127.0.0.1. +# +# run.sh starts two relays from this file, overriding the ports on the command +# line (`--listen` / `--web-http-listen`), and clusters the draining one to the +# other with `--cluster-connect`. Only the settings both share live here. + +# Long enough that a client still on the draining relay at the deadline is a +# failure the harness reports, not a close it races. +drain_timeout = "20s" + +[log] +level = "info" + +[listen] +# QUIC on UDP. 127.0.0.1 avoids IPv6 flakiness on CI runners. +bind = "127.0.0.1:4470" +tls.generate = ["localhost", "127.0.0.1"] + +[web.http] +# HTTP and WebSocket on TCP, also serving /certificate.sha256, which is how the +# cluster dial over `http://` pins the peer's self-signed certificate. +listen = "127.0.0.1:4470" + +[auth] +# Allow anonymous access to everything. +public = "**" diff --git a/test/drain/run.sh b/test/drain/run.sh new file mode 100755 index 0000000000..d70344cb4a --- /dev/null +++ b/test/drain/run.sh @@ -0,0 +1,137 @@ +#!/usr/bin/env bash +# A relay drain, end to end: a JS viewer watching a live track through relay A +# migrates to relay B when A drains, without a dropped group. See README.md. +set -euo pipefail + +DRAIN_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +WORKSPACE=$(cd "$DRAIN_DIR/../.." && pwd) + +# Run directory, reserved ports, and process-group ownership. See test/README.md. +# shellcheck source-path=SCRIPTDIR source=../lib/harness.sh +source "$DRAIN_DIR/../lib/harness.sh" + +# Captured before the parse below consumes it, so the rerun command carries every +# flag and every environment override this run was actually given. +RERUN="$(harness_env DRAIN_TIMEOUT DRAIN_PROFILE RELAY_BIN)just test drain$(harness_argv "$@")" + +TIMEOUT="${DRAIN_TIMEOUT:-60}" +PROFILE="${DRAIN_PROFILE:-debug}" +RELAY="${RELAY_BIN:-}" + +while [[ $# -gt 0 ]]; do + case "$1" in + --timeout) + if [[ $# -lt 2 || -z "${2:-}" || "$2" == -* ]]; then + echo "error: --timeout requires a value" >&2 + exit 2 + fi + TIMEOUT="$2" + shift 2 + ;; + *) + echo "unknown arg: $1" >&2 + exit 2 + ;; + esac +done + +if [[ ! "$TIMEOUT" =~ ^[1-9][0-9]*$ ]]; then + echo "error: timeout must be a positive whole number of seconds (got '$TIMEOUT')" >&2 + exit 2 +fi + +harness_begin drain "$RERUN" + +for tool in cargo bun curl; do + command -v "$tool" >/dev/null 2>&1 || { + echo "error: $tool not found; run inside 'nix develop'" >&2 + exit 1 + } +done + +# ── build ─────────────────────────────────────────────────────────────────── +echo "building moq-relay ($PROFILE)..." +flag=() +[[ "$PROFILE" == "debug" ]] || flag=(--profile "$PROFILE") +(cd "$WORKSPACE" && cargo build --locked ${flag[@]+"${flag[@]}"} -p moq-relay) +TARGET_BASE="${CARGO_TARGET_DIR:-$WORKSPACE/target}" +[[ -n "$RELAY" ]] || RELAY="$TARGET_BASE/$PROFILE/moq-relay" + +(cd "$WORKSPACE" && bun install --frozen-lockfile) + +# ── relays ────────────────────────────────────────────────────────────────── +# Start one relay on a reserved port: `start [extra relay args...]`. +# Sets PID and PORT for the caller. +start() { + local name="$1" + shift + harness_port "$name" + PORT="$HARNESS_PORT" + local url="http://127.0.0.1:$PORT" + # The reservation covers other harness runs, not the rest of the machine. + if harness_probe "$url/certificate.sha256"; then + echo "error: something is already listening on 127.0.0.1:$PORT (stale relay?)" >&2 + exit 1 + fi + + echo "starting relay $name on 127.0.0.1:$PORT..." + harness_spawn "relay-$name" "$HARNESS_RUN/relay-$name.log" "$RELAY" "$DRAIN_DIR/relay.toml" \ + --listen "127.0.0.1:$PORT" --web-http-listen "127.0.0.1:$PORT" "$@" + PID="$HARNESS_PID" + if ! harness_ready "$url/certificate.sha256" 30 "$PID"; then + echo "relay $name never became ready" >&2 + sed 's/^/ relay: /' "$HARNESS_RUN/relay-$name.log" >&2 || true + exit 1 + fi + harness_endpoint "relay-$name" "$url" +} + +# B survives and holds the publisher; A drains and serves the track by pulling it +# from B, the way a sibling in a fleet does. +start b +B_PORT="$PORT" +start a --cluster-connect "http://127.0.0.1:$B_PORT/" +A_PORT="$PORT" +A_PID="$PID" + +harness_port proxy +PROXY_PORT="$HARNESS_PORT" +harness_endpoint name "http://127.0.0.1:$PROXY_PORT" + +# ── run ───────────────────────────────────────────────────────────────────── +# Spawned rather than run in the foreground so a SIGTERM lands while the shell is +# in `wait`, where a trap can run. +status=0 +harness_spawn driver - bun "$DRAIN_DIR/drain.ts" \ + --a-port "$A_PORT" --b-port "$B_PORT" --proxy-port "$PROXY_PORT" --a-pid "$A_PID" --timeout "$TIMEOUT" +harness_wait "$HARNESS_PID" || status=$? + +# The driver saw the viewer leave A; A itself has to notice every session is gone +# and exit cleanly, well before its 20s deadline would have forced anyone out. +if [[ $status -eq 0 ]]; then + deadline=$((SECONDS + 10)) + while ! harness_exited "$A_PID" && ((SECONDS < deadline)); do + sleep 0.1 + done + if ! harness_exited "$A_PID"; then + echo "error: relay A was still running 10s after its last session left" >&2 + status=1 + elif ! harness_wait "$A_PID"; then + echo "error: relay A exited with a failure" >&2 + status=1 + elif ! grep -q "drain complete: every session left" "$HARNESS_RUN/relay-a.log"; then + echo "error: relay A did not report that every session left on its own" >&2 + status=1 + else + echo "relay A drained: every session left" + fi +fi + +if [[ $status -ne 0 ]]; then + for name in a b; do + echo "── relay $name log ──" >&2 + sed 's/^/ /' "$HARNESS_RUN/relay-$name.log" >&2 || true + done +fi + +exit $status diff --git a/test/justfile b/test/justfile index 5cb8ba2342..fc02f77c4e 100644 --- a/test/justfile +++ b/test/justfile @@ -89,6 +89,14 @@ ts *args: ts-eit *args: ./ts/eit-roundtrip.sh {{ args }} +# A JS viewer watching a live track through a relay that drains (SIGTERM, after +# a stand-in for DNS stops pointing at it) migrates to a second relay behind the +# same name without a dropped group. See drain/README.md. + +# Relay drain migration, end to end. +drain *args: + ./drain/run.sh {{ args }} + # Runs the @moq/wasm bindings in headless Chromium against a real relay, one per # protocol flavour. The crate is `#![cfg(target_arch = "wasm32")]`, so nothing in # `just check` or `just rs wasm` gets past compiling it. See wasm/README.md. From 944f864af9320c1d22b1121d251ef7ced91448a9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 26 Sep 2026 14:08:32 -0700 Subject: [PATCH 07/15] style(net): format the merged error import Co-Authored-By: Claude Opus 5.5 --- js/net/src/lite/connection.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/js/net/src/lite/connection.ts b/js/net/src/lite/connection.ts index 8b1ce2cc0b..ef46eba330 100644 --- a/js/net/src/lite/connection.ts +++ b/js/net/src/lite/connection.ts @@ -4,15 +4,7 @@ import type { Established } from "../connection/established.ts"; import type { Drain } from "../connection/goaway.ts"; import { type Probe, type Stats, transportStats } from "../connection/stats.ts"; import { type Transport, transportOf } from "../connection/transport.ts"; -import { - closeError, - error, - fromClose, - ProtocolViolation, - StreamCode, - StreamError, - sessionCause, -} from "../error.ts"; +import { closeError, error, fromClose, ProtocolViolation, StreamCode, StreamError, sessionCause } from "../error.ts"; import { type Hop, randomHop } from "../hop.ts"; import type { Consumer as OriginConsumer } from "../origin.ts"; import type * as Path from "../path.ts"; From 22b7d3cd34bd3775bb4494189700f680c785c79f Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 26 Sep 2026 14:13:06 -0700 Subject: [PATCH 08/15] style(relay): format the drain tests Co-Authored-By: Claude Opus 5.5 --- rs/moq-relay/tests/runtime_uring.rs | 5 ++++- rs/moq-relay/tests/shutdown_signal.rs | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/rs/moq-relay/tests/runtime_uring.rs b/rs/moq-relay/tests/runtime_uring.rs index 0565b06ab0..0a993eda89 100644 --- a/rs/moq-relay/tests/runtime_uring.rs +++ b/rs/moq-relay/tests/runtime_uring.rs @@ -279,7 +279,10 @@ async fn uring_workers_drain_on_the_trigger() { // drain with nothing counted ends at once. let deadline = std::time::Instant::now() + TIMEOUT; while sessions.list(&Default::default()).len() < 2 { - assert!(std::time::Instant::now() < deadline, "the relay never listed both sessions"); + assert!( + std::time::Instant::now() < deadline, + "the relay never listed both sessions" + ); tokio::time::sleep(Duration::from_millis(25)).await; } trigger.start(); diff --git a/rs/moq-relay/tests/shutdown_signal.rs b/rs/moq-relay/tests/shutdown_signal.rs index 1d6a070243..9e70682570 100644 --- a/rs/moq-relay/tests/shutdown_signal.rs +++ b/rs/moq-relay/tests/shutdown_signal.rs @@ -143,7 +143,10 @@ async fn an_embedder_owns_the_signals_inner() { let mut interrupt = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::interrupt()).expect("register SIGINT"); - let relay = Relay::load(relay_config()).await.expect("load relay").with_signals(false); + let relay = Relay::load(relay_config()) + .await + .expect("load relay") + .with_signals(false); let port = relay.tcp_addr().expect("TCP listener bound").port(); let trigger = relay.shutdown_trigger().clone(); let run = tokio::spawn(relay.run()); @@ -193,7 +196,10 @@ async fn an_embedder_owns_the_signals_inner() { async fn a_session_arriving_mid_drain_gets_what_is_left_inner() { let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - let relay = Relay::load(relay_config()).await.expect("load relay").with_signals(false); + let relay = Relay::load(relay_config()) + .await + .expect("load relay") + .with_signals(false); let port = relay.tcp_addr().expect("TCP listener bound").port(); let trigger = relay.shutdown_trigger().clone(); let run = tokio::spawn(relay.run()); @@ -281,7 +287,10 @@ async fn a_drain_ends_once_every_session_leaves_inner() { async fn a_trigger_before_run_keeps_the_deadline_inner() { let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - let relay = Relay::load(relay_config()).await.expect("load relay").with_signals(false); + let relay = Relay::load(relay_config()) + .await + .expect("load relay") + .with_signals(false); let trigger = relay.shutdown_trigger().clone(); // Fired while `run` is still starting. The session deadline is this instant; From 5139fa8b4ed9f4c03930e7bc3bb64d920d5bddfe Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 26 Sep 2026 14:48:51 -0700 Subject: [PATCH 09/15] quest(drain): keep the line open for a zero-budget JS handover The maintainer requires the drain e2e to pass with no latency budget before the line completes. Restore the line with one child, JS group-boundary handover, which flips test/drain to zero budget; the transport upgrade's JS half now requires it. Add two m1 follow-ups: drain in-flight handshakes, and run the io_uring tests on a 6.12+ self-hosted runner. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 3 ++ quest/m1/drain-handshakes.md | 29 ++++++++++++++ quest/m1/drain/README.md | 57 ++++++++++++++++++++++++++++ quest/m1/drain/js-group-handover.md | 47 +++++++++++++++++++++++ quest/m1/pop-skipping/README.md | 1 + quest/m1/transport-upgrade/README.md | 11 ++++-- quest/m1/transport-upgrade/js.md | 12 ++++-- quest/m1/uring-runner.md | 28 ++++++++++++++ quest/m2/quic-careful-resume.md | 1 + test/drain/README.md | 2 + 10 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 quest/m1/drain-handshakes.md create mode 100644 quest/m1/drain/README.md create mode 100644 quest/m1/drain/js-group-handover.md create mode 100644 quest/m1/uring-runner.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 6e54791eb5..0e703126a4 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -62,6 +62,9 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Keyframe trigger](/quest/m1/keyframe-trigger.md) - an application can ask the built-in capture encoder for a keyframe - [Video keyframe flag](/quest/m1/video-keyframe-flag.md) - encoded video marks its keyframes, so a requested cut never forces an extra one after a cadence keyframe - [QoS](/quest/m1/qos/README.md) - broadcast health: relay starvation and timeliness histograms, and client stats broadcasts from publishers and viewers +- [Drain](/quest/m1/drain/README.md) - relay restarts drain sessions over GOAWAY instead of hard-dropping them +- [Drain handshakes](/quest/m1/drain-handshakes.md) - a drain GOAWAYs and waits for sessions still in their handshake instead of exiting under them +- [io_uring runner](/quest/m1/uring-runner.md) - the io_uring relay tests run nightly on a 6.12+ self-hosted runner instead of skipping - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - a session that came up over WebSocket moves to QUIC once the QUIC dial lands, handing over at a group boundary - [Own the QUIC stack](/quest/m1/quic/README.md) - the moq-noq fork carries ACK progress, reliable reset, hierarchical scheduling, deadlines, probing, diff --git a/quest/m1/drain-handshakes.md b/quest/m1/drain-handshakes.md new file mode 100644 index 0000000000..037d72f9f7 --- /dev/null +++ b/quest/m1/drain-handshakes.md @@ -0,0 +1,29 @@ +# [S] Drain in-flight handshakes + +## Goal + +A relay drain counts a session from admission, not only once it is +established, so a client still in its handshake when the drain starts is sent +a GOAWAY and waited for instead of being cut off by the relay exiting. The +drain still ends at its deadline whatever a handshake does. + +## Plan + +`shutdown::Observer::serve` is taken in `supervise`, after the handshake. A +drain that starts before any session reaches it sees nothing to wait for and +`Relay::run` returns at once. #4186 chose this on purpose, reasoning that with +DNS withdrawn first, arrivals are rare. The io_uring drain test hit it anyway: a +moq-lite-06 client sees its session before the relay has admitted it, so the +test now holds a straggler to keep the drain open. + +Take the count where the relay first commits to a session (the accept or +registration point on each of the QUIC, io_uring, WebSocket, and iroh paths), +and hand it to the session so the guard spans auth and the handshake. A +session admitted during a drain already gets a GOAWAY carrying the time left, +so the change should be the count alone. Revisit whether a handshake that fails +auth, or never completes, should hold a drain until the deadline. + +Add a regression test that triggers the drain between a client's connect and +the relay's admission and expects a GOAWAY, then drop the straggler workaround +in `rs/moq-relay/tests/runtime_uring.rs`. Update `doc/bin/relay/config.md`, +which documents the current behavior. diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md new file mode 100644 index 0000000000..e856f92fd8 --- /dev/null +++ b/quest/m1/drain/README.md @@ -0,0 +1,57 @@ +# Graceful relay drains (GOAWAY) + +## Goal + +Relay restarts drain sessions instead of hard-dropping them. The end state: a +draining node is first withdrawn from DNS (marked unhealthy so resolvers stop +handing it out), waits out the DNS TTL plus a margin for monitor detection, +THEN sends GOAWAY on every MoQ session. Clients reconnect through a fresh DNS +resolve and land on a different relay, and a straggler that dials the draining +node anyway (a cached resolve, or a pool alias) just gets another GOAWAY. Only +after sessions drain or the stop deadline expires does the process exit and +the new software boot. + +GOAWAY only reaches MoQ sessions. Both clients migrate on it: +`moq_tokio::Connection` and the `js/net` `Connection` dial the replacement +through a fresh resolve while the old session drains. The wire message is +lite04+/IETF only besides. So the stop deadline is the +real backstop - for pre-lite04 versions, for client SDKs deployed before +the JS migration shipped, and for in-process ingest gateways (RTMP/SRT/WHIP/WHEP), +which have no GOAWAY equivalent at all: their grace is the DNS-drain window +stopping new arrivals plus the encoder's own reconnect. The DNS-drain-first +ordering is what keeps that hard-close window small. + +## Plan + +This questline holds the two relay/client halves. The orchestration around +them (a planned-drain health state, the SIGTERM sequencing and stop timeouts, +per-PoP serial deploys, a two-node PoP floor, and the gateway drain contract) +is moq.pro's (downstream) fleet drain work, which consumes these quests. + +The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM +to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals +included, against one deadline. `Relay::run` returns as soon as every session +has left, logging whether the deadline force-closed any, and +`moq_relay_draining_sessions` shows the drain's progress. + +**Clients (landed).** The JS reconnector migrates like the Rust one, +preserving the app-visible session while resolving DNS again before dialing. +Both are covered against stand-in servers. This is a +scale-down prerequisite, not merely a deploy improvement. RTMP/SRT/WHIP/WHEP +cannot receive MoQ GOAWAY, so their contract remains DNS withdrawal followed +by the stop deadline and encoder reconnect. + +**End to end (landed).** `just test drain` (`test/drain/`, nightly) has a JS +viewer watch a live track through relay A behind a stand-in for DNS, withdraws +A, SIGTERMs it, and requires the viewer to move to relay B without a dropped +group. It passes today only with a 1s latency budget: following the route +means resubscribing on B, and a group boundary inside the swap loses that +group. The line completes once the viewer needs no budget. + +## Quests + +- [JS group-boundary handover](/quest/m1/drain/js-group-handover.md) - a JS track subscription carries across a route swap at a group boundary, so `test/drain` passes at zero latency budget + +## Related + +- [pop-skipping](/quest/m1/pop-skipping/README.md) - its same-PoP link price and full eligible pairing become important when a deployment adds a second relay per PoP diff --git a/quest/m1/drain/js-group-handover.md b/quest/m1/drain/js-group-handover.md new file mode 100644 index 0000000000..42cc600ba1 --- /dev/null +++ b/quest/m1/drain/js-group-handover.md @@ -0,0 +1,47 @@ +# [L] JS group-boundary handover + +## Goal + +A `js/net` track subscription survives its broadcast's route swapping to +another provider, such as a relay migration after a GOAWAY, and resumes from +the new provider at the first group it has not delivered. A viewer at the live +edge with no latency budget never loses a group across the swap, and never +has to notice the swap to keep reading. + +Done when `test/drain` passes with the viewer's `MAX_AGE` at zero, the +viewer subscribes once instead of following `request.active`, and the run is +stable enough for the nightly. + +## Plan + +Today the origin (`js/net/src/origin.ts`) holds the outranked route until the +new one answers, then closes the old front. Every track read through that +front ends, and the app has to subscribe again on the new broadcast. Once the +old relay drops its upstream pull, the new relay subscribes upstream from +scratch at the live edge. A group boundary that lands inside that window loses +the group in flight: about 1 run in 5 at 100 ms groups. + +Rust already solves this. `moq-net`'s origin fronts are spliced broadcasts, and +`model/resume.rs` resumes each track from the replacement at the first missing +group, capping the old segment so it ends at the boundary on its own. Mirror +that shape and naming in JS rather than inventing a second model. Things to +settle along the way: + +- Whether the request's `active` broadcast stays the same object across a swap, + with its tracks re-sourced underneath. That is the Rust behavior and the + simplest for players. It is also a behavior change for code that watches + `active` to resubscribe. +- The resumed subscription names where it left off (the `groups` floor) so the + new provider serves the missing group from its cache or upstream instead of + starting at its own live edge. +- Failover compatibility: Rust refuses to splice a source whose track + properties differ (timescale, retention, priority, order). Match it. +- `js/watch` and `js/hang` consumers that re-subscribe on `active` changes. + Check whether they still need to. + +Add unit coverage at the origin level against stand-in sessions, then flip +`test/drain` to zero budget (drop the resubscribe loop in `drain.ts` and the +budget note in its README). + +Public API: likely a behavior change to `Origin.Requesting.active` and track +subscriptions across a swap. Report it in the PR. diff --git a/quest/m1/pop-skipping/README.md b/quest/m1/pop-skipping/README.md index eeccd643e7..c1399c0b19 100644 --- a/quest/m1/pop-skipping/README.md +++ b/quest/m1/pop-skipping/README.md @@ -153,5 +153,6 @@ costs of one bidirectional session, which one `?cost=` cannot split. ## Related +- [drain](/quest/m1/drain/README.md) - a second relay per PoP makes the same-PoP link price and its connection cardinality operationally important - [wildcard](/quest/m1/wildcard/README.md) - it reuses this questline's route cost, and needs a cluster on Lite06 - [relay-memory](/quest/m1/relay-memory.md) - a denser mesh multiplies whatever a non-selected route costs diff --git a/quest/m1/transport-upgrade/README.md b/quest/m1/transport-upgrade/README.md index 57ae183627..0f53bd72ab 100644 --- a/quest/m1/transport-upgrade/README.md +++ b/quest/m1/transport-upgrade/README.md @@ -33,8 +33,9 @@ every version (a client may send one with an empty URI; only a redirect URI is forbidden to a moq-transport client). The origin's multi-route front prefers the newest of two equal routes and `resume` splices each track at a group boundary, capping the old segment so the old session's subscription ends at the -boundary on its own. The JavaScript half reuses `Reload`'s GOAWAY migration in -`js/net`. +boundary on its own. The JavaScript GOAWAY handover has shipped, but its track +subscriptions do not yet carry across the route swap; the JS half requires the +[drain](/quest/m1/drain/README.md) line's group-boundary handover for that. Shared decisions: @@ -57,4 +58,8 @@ Shared decisions: ## Quests - [Rust](/quest/m1/transport-upgrade/rust.md) - moq-tokio keeps the QUIC dial after WebSocket wins and migrates through the existing Draining path -- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through its GOAWAY handover +- [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through the drain line's GOAWAY handover + +## Related + +- [Drain](/quest/m1/drain/README.md) - the peer-initiated half of the same handover diff --git a/quest/m1/transport-upgrade/js.md b/quest/m1/transport-upgrade/js.md index 719c8284d3..61ae88c297 100644 --- a/quest/m1/transport-upgrade/js.md +++ b/quest/m1/transport-upgrade/js.md @@ -11,8 +11,10 @@ nothing changes. ## Plan -Lands in `js/net`, reusing the GOAWAY handover (`Reload`'s migration in -`js/net/src/connection/reload.ts`): dial the replacement while the old session +Lands in `js/net`, reusing the [drain](/quest/m1/drain/README.md) line's +GOAWAY handover (`Reload`'s migration in +`js/net/src/connection/reload.ts`), once its group-boundary handover keeps a +watched track's groups across the route swap: dial the replacement while the old session keeps serving, let both feed the origin (a request holds the outranked route until the new one answers), leave the old session to close on its own or at the handover cap. See the @@ -38,6 +40,10 @@ handover cap. See the group across the upgrade, the WebSocket session closes within the cap, and the next connect to the same URL gives WebTransport the head start again; with no delay, WebTransport wins and no WebSocket session is ever opened. -- Public API: none; `transportOf` already +- Public API: none beyond what the drain line adds; `transportOf` already reports the live transport. Update `doc/lib/js` where the fallback race is described. + +## Required + +- [JS group-boundary handover](/quest/m1/drain/js-group-handover.md) - a watched track keeps every group across a route swap, which the upgrade is one of diff --git a/quest/m1/uring-runner.md b/quest/m1/uring-runner.md new file mode 100644 index 0000000000..612840661a --- /dev/null +++ b/quest/m1/uring-runner.md @@ -0,0 +1,28 @@ +# [S] io_uring tests on a 6.12+ runner + +## Goal + +The io_uring relay tests actually run nightly instead of skipping. Today +GitHub-hosted runners are below the 6.12 kernel floor, so `just rs uring` +compiles the tests and every one of them skips. The io_uring drain test was +broken on the drain line and nothing noticed. + +## Plan + +Run the nightly `rs uring` recipe on a self-hosted Linux runner with kernel +6.12+, registered on the maintainer's host. Keep the GitHub-hosted entry only +if it still catches something the self-hosted one does not. Otherwise move it. + +A self-hosted runner on a public repository must never run untrusted code. +Gate the job to `schedule` and `workflow_dispatch` on `main` (never +`pull_request` from forks), give it a label only that job selects, and keep its +permissions read-only. Check GitHub's self-hosted runner security guidance +before wiring it. + +Fail loudly when the runner's kernel is below the floor instead of letting the +tests skip, so a runner that regresses reports it. Mind the host's memlock limit, +which `a05a2d5a5` sized the io_uring tests against. + +## Required + +- A self-hosted runner with Linux 6.12+ is registered for moq-dev/moq on the maintainer's host diff --git a/quest/m2/quic-careful-resume.md b/quest/m2/quic-careful-resume.md index cf43686780..30d906eb0a 100644 --- a/quest/m2/quic-careful-resume.md +++ b/quest/m2/quic-careful-resume.md @@ -34,3 +34,4 @@ only when the jump never makes the first second worse than slow start. - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - one of the reconnects this speeds up +- [Drain](/quest/m1/drain/README.md) - GOAWAY redials are the other diff --git a/test/drain/README.md b/test/drain/README.md index 92eebde688..1919258d5a 100644 --- a/test/drain/README.md +++ b/test/drain/README.md @@ -50,3 +50,5 @@ The viewer subscribes with a 1 s latency budget, as the interop subscribers do. After the swap, B has to subscribe upstream afresh once A drops its pull, and the budget is what reaches back to a group in flight across the swap. With no budget, a group boundary that lands inside the swap loses that group. +[JS group-boundary handover](/quest/m1/drain/js-group-handover.md) removes the +budget by carrying the subscription across the swap. From c273150aa3b04e483177360211546406bf869af9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 07:19:57 -0700 Subject: [PATCH 10/15] quest(drain): finish the line; promote the JS handover quests to m1 js-group-handover and js-goaway-requests become standalone m1 quests, and every reference to the drain line moves to them or drops. The relay embed docs note that a drain only reaches MoQ sessions. Co-Authored-By: Claude Opus 5.5 --- doc/bin/relay/index.md | 4 +- quest/m1/README.md | 3 +- quest/m1/drain/README.md | 63 ---------------------- quest/m1/{drain => }/js-goaway-requests.md | 0 quest/m1/{drain => }/js-group-handover.md | 0 quest/m1/redirect-resolve.md | 7 +-- quest/m1/transport-upgrade/README.md | 10 ++-- quest/m1/transport-upgrade/js.md | 13 +++-- quest/m2/firefox-155-webtransport.md | 4 +- quest/m2/quic-careful-resume.md | 3 +- test/drain/README.md | 2 +- 11 files changed, 21 insertions(+), 88 deletions(-) delete mode 100644 quest/m1/drain/README.md rename quest/m1/{drain => }/js-goaway-requests.md (100%) rename quest/m1/{drain => }/js-group-handover.md (100%) diff --git a/doc/bin/relay/index.md b/doc/bin/relay/index.md index a25613488b..9223249492 100644 --- a/doc/bin/relay/index.md +++ b/doc/bin/relay/index.md @@ -90,7 +90,9 @@ example to withdraw the node from DNS and wait out the TTL before draining, calls `with_signals(false)` and fires the trigger itself. Build routes from `web().routes()` (or `internal().routes()`): `with_web` replaces the router, so `Router::new()` drops the built-in routes. Extra listeners (RTMP, SRT, ...) sit beside `run` -in the application's `select!`. `runtime.workers` and `runtime.io_uring` stay +in the application's `select!`. The drain reaches only MoQ sessions: an +extra listener has no GOAWAY, so the application stops it on its own deadline +and leaves the encoder to reconnect. `runtime.workers` and `runtime.io_uring` stay inside the owner; do not split the worker group yourself. An application that decides admissions itself leaves `[auth]` empty and answers `relay.admissions()`; see [Authentication](/bin/relay/auth#in-process). See diff --git a/quest/m1/README.md b/quest/m1/README.md index 85c8843d3b..54a8b25452 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -84,7 +84,8 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [QoS](/quest/m1/qos/README.md) - broadcast health: relay starvation and timeliness histograms, on dev - [Catalog track alias](/quest/m1/catalog-track-alias.md) - catalog rendition keys become aliases with an optional `track` name, so one catalog lists renditions from several broadcasts - [Media stats](/quest/m1/stats/README.md) - publishers announce a stats track in the catalog, viewers answer a soliciting catalog through a per-catalog `.echo` broadcast, and a Rust encoder adapts to them -- [Drain](/quest/m1/drain/README.md) - relay restarts drain sessions over GOAWAY instead of hard-dropping them +- [JS group-boundary handover](/quest/m1/js-group-handover.md) - a JS track subscription carries across a route swap at a group boundary, so `test/drain` passes at zero latency budget +- [JS GOAWAY requests](/quest/m1/js-goaway-requests.md) - after GOAWAY the JS client opens no new request on the old session, like Rust - [Drain handshakes](/quest/m1/drain-handshakes.md) - a drain GOAWAYs and waits for sessions still in their handshake instead of exiting under them - [io_uring runner](/quest/m1/uring-runner.md) - the io_uring relay tests run nightly on a 6.12+ self-hosted runner instead of skipping - [Strict Redirect::resolve](/quest/m1/redirect-resolve.md) - on dev, `Redirect::resolve` can no longer quietly turn a refused redirect into a redial diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md deleted file mode 100644 index 1c21ad96a5..0000000000 --- a/quest/m1/drain/README.md +++ /dev/null @@ -1,63 +0,0 @@ -# Graceful relay drains (GOAWAY) - -## Goal - -Relay restarts drain sessions instead of hard-dropping them. The end state: a -draining node is first withdrawn from DNS (marked unhealthy so resolvers stop -handing it out), waits out the DNS TTL plus a margin for monitor detection, -THEN sends GOAWAY on every MoQ session. Clients reconnect through a fresh DNS -resolve and land on a different relay, and a straggler that dials the draining -node anyway (a cached resolve, or a pool alias) just gets another GOAWAY. Only -after sessions drain or the stop deadline expires does the process exit and -the new software boot. - -GOAWAY only reaches MoQ sessions. Both clients migrate on it: -`moq_tokio::Connection` and the `js/net` `Connection` dial the replacement -through a fresh resolve while the old session drains. The wire message is -lite04+/IETF only besides. So the stop deadline is the -real backstop - for pre-lite04 versions, for client SDKs deployed before -the JS migration shipped, and for in-process ingest gateways (RTMP/SRT/WHIP/WHEP), -which have no GOAWAY equivalent at all: their grace is the DNS-drain window -stopping new arrivals plus the encoder's own reconnect. The DNS-drain-first -ordering is what keeps that hard-close window small. - -## Plan - -This questline holds the two relay/client halves. The orchestration around -them (a planned-drain health state, the SIGTERM sequencing and stop timeouts, -per-PoP serial deploys, a two-node PoP floor, and the gateway drain contract) -is moq.pro's (downstream) fleet drain work, which consumes these quests. - -Drain stays at the MoQ layer. WebTransport's `WT_DRAIN_SESSION` capsule and -the browser `draining` promise are advisory and carry no redirect URI or -timeout, and qmux and WebSocket have no equivalent, so neither the relay nor -the clients send or act on them (decided 2026-09-26). - -The relay's drain hook has landed: `Relay::with_signals(false)` hands SIGTERM -to the embedder, and its `shutdown_trigger` GOAWAYs every session, arrivals -included, against one deadline. `Relay::run` returns as soon as every session -has left, logging whether the deadline force-closed any, and -`moq_relay_draining_sessions` shows the drain's progress. - -**Clients (landed).** The JS reconnector migrates like the Rust one, -preserving the app-visible session while resolving DNS again before dialing. -Both are covered against stand-in servers. This is a -scale-down prerequisite, not merely a deploy improvement. RTMP/SRT/WHIP/WHEP -cannot receive MoQ GOAWAY, so their contract remains DNS withdrawal followed -by the stop deadline and encoder reconnect. - -**End to end (landed).** `just test drain` (`test/drain/`, nightly) has a JS -viewer watch a live track through relay A behind a stand-in for DNS, withdraws -A, SIGTERMs it, and requires the viewer to move to relay B without a dropped -group. It passes today only with a 1s latency budget: following the route -means resubscribing on B, and a group boundary inside the swap loses that -group. The line completes once the viewer needs no budget. - -## Required - -- [JS group-boundary handover](/quest/m1/drain/js-group-handover.md) - a JS track subscription carries across a route swap at a group boundary, so `test/drain` passes at zero latency budget -- [JS GOAWAY requests](/quest/m1/drain/js-goaway-requests.md) - after GOAWAY the JS client opens no new request on the old session, like Rust - -## Related - -- [Cluster routing](/quest/m1/cluster-routing.md) - the configured topology and link costs a second relay per PoP joins diff --git a/quest/m1/drain/js-goaway-requests.md b/quest/m1/js-goaway-requests.md similarity index 100% rename from quest/m1/drain/js-goaway-requests.md rename to quest/m1/js-goaway-requests.md diff --git a/quest/m1/drain/js-group-handover.md b/quest/m1/js-group-handover.md similarity index 100% rename from quest/m1/drain/js-group-handover.md rename to quest/m1/js-group-handover.md diff --git a/quest/m1/redirect-resolve.md b/quest/m1/redirect-resolve.md index 7ee9ef7014..37f4e8460c 100644 --- a/quest/m1/redirect-resolve.md +++ b/quest/m1/redirect-resolve.md @@ -19,12 +19,9 @@ the connection it documents. Recommendation: make it private. Nothing outside `moq-tokio` calls it (only its own unit tests), and the repo keeps things private until a consumer needs them. If a consumer turns up, the alternative is returning the same -`Result>` as the internal `target` does once the drain line lands -(on `main` it is still `Option`, folding a refusal into "keep the -current addresses"), so empty and refused stay distinct. Removing or changing a published method is a break, so this +`Result>` as the internal `target` does, so empty and refused stay distinct. Removing or changing a published method is a break, so this targets `dev`; update `doc/lib/rs` if it mentions the method. ## Required -- [Graceful relay drains](/quest/m1/drain/README.md) - the stricter `Connection` lands with the line -- `dev` has merged `main` after the drain line lands +- `dev` has merged `main` since the drain line (moq-dev/moq#4132) landed diff --git a/quest/m1/transport-upgrade/README.md b/quest/m1/transport-upgrade/README.md index 094817c6e0..21b834645d 100644 --- a/quest/m1/transport-upgrade/README.md +++ b/quest/m1/transport-upgrade/README.md @@ -33,9 +33,9 @@ every version (a client may send one with an empty URI; only a redirect URI is forbidden to a moq-transport client). The origin's multi-route front prefers the newest of two equal routes and `resume` splices each track at a group boundary, capping the old segment so the old session's subscription ends at the -boundary on its own. The JavaScript handover is the -[drain](/quest/m1/drain/README.md) line's (client goaway, done there), so the -JS half requires it. +boundary on its own. The JavaScript GOAWAY handover landed with the drain line, but it does not +splice tracks yet; the JS half requires +[JS group-boundary handover](/quest/m1/js-group-handover.md) for that. Shared decisions: @@ -59,7 +59,3 @@ Shared decisions: - [Rust](/quest/m1/transport-upgrade/rust.md) - moq-tokio keeps the QUIC dial after WebSocket wins and migrates through the existing Draining path - [JavaScript](/quest/m1/transport-upgrade/js.md) - js/net keeps the WebTransport dial after WebSocket wins and migrates through the client-goaway handover - -## Related - -- [Drain](/quest/m1/drain/README.md) - the peer-initiated half of the same handover diff --git a/quest/m1/transport-upgrade/js.md b/quest/m1/transport-upgrade/js.md index f5c8562035..eefefe9002 100644 --- a/quest/m1/transport-upgrade/js.md +++ b/quest/m1/transport-upgrade/js.md @@ -11,10 +11,9 @@ nothing changes. ## Plan -Lands in `js/net`, after the [drain](/quest/m1/drain/README.md) line ships -the GOAWAY handover it reuses (client goaway is done on the line, JS -group-boundary handover is its remaining child): dial the replacement while the old session -keeps serving, swap the origin wiring once it is established, leave the old +Lands in `js/net`, reusing the GOAWAY handover the drain line shipped once +[JS group-boundary handover](/quest/m1/js-group-handover.md) splices tracks +across it: dial the replacement while the old session keeps serving, swap the origin wiring once it is established, leave the old session to close on its own or at the handover cap. See the [questline](/quest/m1/transport-upgrade/README.md) for the shared decisions. @@ -33,7 +32,7 @@ session to close on its own or at the handover cap. See the configured cap. A WebTransport attempt that fails after WebSocket won is logged at debug and the session stays on WebSocket. - A self-sent GOAWAY must gate new requests on the old session too, not only - a received one: [JS GOAWAY requests](/quest/m1/drain/js-goaway-requests.md) + a received one: [JS GOAWAY requests](/quest/m1/js-goaway-requests.md) covers the received case, so check it also covers this path. - On a successful upgrade delete the URL from `websocketWon`. - Tests in the browser harness against the in-tree relay: with the @@ -47,5 +46,5 @@ session to close on its own or at the handover cap. See the ## Required -- [Drain](/quest/m1/drain/README.md) - the GOAWAY handover this upgrade reuses -- [JS GOAWAY requests](/quest/m1/drain/js-goaway-requests.md) - no new request opens on a session that is going away +- [JS group-boundary handover](/quest/m1/js-group-handover.md) - tracks carry across the handover this upgrade reuses without a dropped group +- [JS GOAWAY requests](/quest/m1/js-goaway-requests.md) - no new request opens on a session that is going away diff --git a/quest/m2/firefox-155-webtransport.md b/quest/m2/firefox-155-webtransport.md index dc8af1eb6d..399b438473 100644 --- a/quest/m2/firefox-155-webtransport.md +++ b/quest/m2/firefox-155-webtransport.md @@ -39,8 +39,8 @@ Firefox 155 shipped five WebTransport features; decided 2026-09-26: - **`draining`** is not used. Drain stays at the MoQ layer: GOAWAY carries a redirect URI (and a timeout on moq-transport draft-17+; elsewhere the deadline is sender-local) and works over qmux and WebSocket, while - `WT_DRAIN_SESSION` is advisory and carries neither (see - [drain](/quest/m1/drain/README.md)). + `WT_DRAIN_SESSION` is advisory and carries neither (decided + 2026-09-26 in the drain line, moq-dev/moq#4132). - **`exportKeyingMaterial()`** has no consumer: the exporter is per hop, so it cannot key e2ee, which is end to end. Binding auth tokens to the TLS session is the plausible future use; moq-noq already exposes the exporter, but diff --git a/quest/m2/quic-careful-resume.md b/quest/m2/quic-careful-resume.md index 30d906eb0a..1bc6ec9f11 100644 --- a/quest/m2/quic-careful-resume.md +++ b/quest/m2/quic-careful-resume.md @@ -34,4 +34,5 @@ only when the jump never makes the first second worse than slow start. - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - one of the reconnects this speeds up -- [Drain](/quest/m1/drain/README.md) - GOAWAY redials are the other +- [JS group-boundary handover](/quest/m1/js-group-handover.md) - GOAWAY + redials are the other diff --git a/test/drain/README.md b/test/drain/README.md index 1919258d5a..a5b821233b 100644 --- a/test/drain/README.md +++ b/test/drain/README.md @@ -50,5 +50,5 @@ The viewer subscribes with a 1 s latency budget, as the interop subscribers do. After the swap, B has to subscribe upstream afresh once A drops its pull, and the budget is what reaches back to a group in flight across the swap. With no budget, a group boundary that lands inside the swap loses that group. -[JS group-boundary handover](/quest/m1/drain/js-group-handover.md) removes the +[JS group-boundary handover](/quest/m1/js-group-handover.md) removes the budget by carrying the subscription across the swap. From cba7744261ae9e4ccb65b09f5d462e6ba037ec4a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 07:27:56 -0700 Subject: [PATCH 11/15] fix(relay): close the shared listener before run returns Once a drain ends as soon as every session leaves, run no longer sleeps out the window, so dropping the listener left its QUIC socket open while the endpoint's closing connections finished in the background. Main's embed test caught it: the owner's sockets outlived run. Co-Authored-By: Claude Opus 5.5 --- rs/moq-relay/src/relay.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rs/moq-relay/src/relay.rs b/rs/moq-relay/src/relay.rs index cb115974f5..f0e6d6181d 100644 --- a/rs/moq-relay/src/relay.rs +++ b/rs/moq-relay/src/relay.rs @@ -488,7 +488,7 @@ impl Relay { pub async fn run(self) -> anyhow::Result<()> { let Relay { ready, - server, + mut server, auth, admissions, cluster, @@ -628,6 +628,7 @@ impl Relay { let has_iroh = false; let serve_shared = { let idle = quic_on_workers && server.accept_health().is_empty() && !has_iroh; + let server = &mut server; let cluster = cluster.clone(); let auth = auth.clone(); let shutdown = shutdown.clone(); @@ -652,6 +653,10 @@ impl Relay { else => Ok(()), }; + // Dropping the listener would leave its QUIC socket open until the + // endpoint's closing connections finish in the background, past `run`. + server.close().await; + // Explicitly, so the joins land on the blocking pool rather than on the // executor thread this future happens to be running on. #[cfg(feature = "_quic")] @@ -763,12 +768,12 @@ async fn serve( ) -> anyhow::Result<()> { // Each QUIC worker binds here; Relay::run binds the shared listener before // readiness and passes it to the same accept loop. - let listener = server.listen().await.context("failed to bind listeners")?; - serve_listening(listener, cluster, auth, shutdown, sessions).await + let mut listener = server.listen().await.context("failed to bind listeners")?; + serve_listening(&mut listener, cluster, auth, shutdown, sessions).await } async fn serve_listening( - mut listener: moq_tokio::Listener, + listener: &mut moq_tokio::Listener, cluster: cluster::Cluster, auth: auth::Auth, shutdown: shutdown::Observer, From b063f0c7a7b292ea10f4b167df1017d28c53b6f7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 08:41:08 -0700 Subject: [PATCH 12/15] fix(drain): address review on the drain line - The drain harness fails on a corrupt group payload instead of logging it. - moq-tokio's reconnect test binds port 0 instead of probing for a port. - Drop quest/m1/uring-runner.md: the nightly rs uring job on the ARM runner (kernel 6.17) already runs the io_uring tests. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 1 - quest/m1/gpu-ci.md | 4 ---- quest/m1/uring-runner.md | 28 --------------------------- rs/moq-tokio/src/connection.rs | 35 +++++++++++++--------------------- test/drain/drain.ts | 8 +++++++- 5 files changed, 20 insertions(+), 56 deletions(-) delete mode 100644 quest/m1/uring-runner.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 18f8245120..bb36a1348f 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -85,7 +85,6 @@ QUIC studies there on that rule. - [JS group-boundary handover](/quest/m1/js-group-handover.md) - a JS track subscription carries across a route swap at a group boundary, so `test/drain` passes at zero latency budget - [JS GOAWAY requests](/quest/m1/js-goaway-requests.md) - after GOAWAY the JS client opens no new request on the old session, like Rust - [Drain handshakes](/quest/m1/drain-handshakes.md) - a drain GOAWAYs and waits for sessions still in their handshake instead of exiting under them -- [io_uring runner](/quest/m1/uring-runner.md) - the io_uring relay tests run nightly on a 6.12+ self-hosted runner instead of skipping - [Strict Redirect::resolve](/quest/m1/redirect-resolve.md) - on dev, `Redirect::resolve` can no longer quietly turn a refused redirect into a redial - [Transport upgrade](/quest/m1/transport-upgrade/README.md) - a session that came up over WebSocket moves to QUIC once the QUIC dial lands, handing over at a group boundary - [Scope track priority](/quest/m1/track-priority-scope.md) - priority orders one owner's streams, and a shared cluster session is fair across tenants diff --git a/quest/m1/gpu-ci.md b/quest/m1/gpu-ci.md index e294915c56..d6b3fba521 100644 --- a/quest/m1/gpu-ci.md +++ b/quest/m1/gpu-ci.md @@ -43,10 +43,6 @@ skipping inside the Nix shell. job gated to `refs/heads/main`, never `pull_request`; a dedicated label only this job selects; read-only `permissions`. Read GitHub's self-hosted runner hardening guidance before wiring it. -- Share the runner with the io_uring one that #4132 plans - (`quest/m1/uring-runner.md` on the drain line, which wants a 6.12+ kernel on - the same host): one registration and one security posture, a label per - capability. Whichever quest lands second reuses the first's job shape. Public API: none. Wire: none. diff --git a/quest/m1/uring-runner.md b/quest/m1/uring-runner.md deleted file mode 100644 index 612840661a..0000000000 --- a/quest/m1/uring-runner.md +++ /dev/null @@ -1,28 +0,0 @@ -# [S] io_uring tests on a 6.12+ runner - -## Goal - -The io_uring relay tests actually run nightly instead of skipping. Today -GitHub-hosted runners are below the 6.12 kernel floor, so `just rs uring` -compiles the tests and every one of them skips. The io_uring drain test was -broken on the drain line and nothing noticed. - -## Plan - -Run the nightly `rs uring` recipe on a self-hosted Linux runner with kernel -6.12+, registered on the maintainer's host. Keep the GitHub-hosted entry only -if it still catches something the self-hosted one does not. Otherwise move it. - -A self-hosted runner on a public repository must never run untrusted code. -Gate the job to `schedule` and `workflow_dispatch` on `main` (never -`pull_request` from forks), give it a label only that job selects, and keep its -permissions read-only. Check GitHub's self-hosted runner security guidance -before wiring it. - -Fail loudly when the runner's kernel is below the floor instead of letting the -tests skip, so a runner that regresses reports it. Mind the host's memlock limit, -which `a05a2d5a5` sized the io_uring tests against. - -## Required - -- A self-hosted runner with Linux 6.12+ is registered for moq-dev/moq on the maintainer's host diff --git a/rs/moq-tokio/src/connection.rs b/rs/moq-tokio/src/connection.rs index e6b6d17623..cbffa9c49d 100644 --- a/rs/moq-tokio/src/connection.rs +++ b/rs/moq-tokio/src/connection.rs @@ -2092,8 +2092,7 @@ mod tests { /// A stream-only server on a free loopback port, publishing `origin`. /// /// Returns its address, a receiver yielding each accepted session (so the test - /// can drain it), and the listener task. Probing for a free port races other - /// tests between the probe closing and the real bind, so this retries. + /// can drain it), and the listener task. #[cfg(feature = "tcp")] async fn serve( origin: moq_net::origin::Producer, @@ -2102,28 +2101,20 @@ mod tests { tokio::sync::mpsc::UnboundedReceiver, tokio::task::JoinHandle<()>, ) { - for _ in 0..20 { - let probe = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = probe.local_addr().unwrap(); - drop(probe); - - let mut config = crate::listen::Config::default(); - config.tcp.bind = Some(addr); - let Ok(mut server) = config.init(Default::default()).unwrap().listen().await else { - continue; - }; + let mut config = crate::listen::Config::default(); + config.tcp.bind = Some("127.0.0.1:0".parse().unwrap()); + let mut server = config.init(Default::default()).unwrap().listen().await.unwrap(); + let addr = server.tcp_local_addr().expect("tcp listener bound"); - let (accepted, sessions) = tokio::sync::mpsc::unbounded_channel(); - let task = tokio::spawn(async move { - while let Some(request) = server.accept().await { - if let Ok(session) = request.with_publisher(&origin).ok().await { - let _ = accepted.send(session); - } + let (accepted, sessions) = tokio::sync::mpsc::unbounded_channel(); + let task = tokio::spawn(async move { + while let Some(request) = server.accept().await { + if let Ok(session) = request.with_publisher(&origin).ok().await { + let _ = accepted.send(session); } - }); - return (addr, sessions, task); - } - panic!("could not bind a free TCP port after 20 attempts"); + } + }); + (addr, sessions, task) } /// The fleet drain: a relay withdrawn from DNS sends an empty-URI GOAWAY with a diff --git a/test/drain/drain.ts b/test/drain/drain.ts index 9daf6e243c..40512d8f12 100644 --- a/test/drain/drain.ts +++ b/test/drain/drain.ts @@ -125,6 +125,8 @@ const ticker = setInterval(() => { /** Which relay delivered each group: the generation of the broadcast it was read from. */ const seen = new Map>(); let generation = -1; +/** A group whose payload is not its sequence number: fails the run however the swap goes. */ +let corrupt: string | undefined; const watched = new Moq.Origin.Producer(); const viewer = new Moq.Connection({ @@ -140,7 +142,10 @@ async function read(sub: Moq.Track.Subscriber, gen: number): Promise { const group = await sub.recvGroup(); if (!group) return; const text = await group.readString(); - if (text !== String(group.sequence)) throw new Error(`group ${group.sequence} carried ${text}`); + if (text !== String(group.sequence)) { + corrupt ??= `generation ${gen}: group ${group.sequence} carried ${text}`; + return; + } let gens = seen.get(group.sequence); if (!gens) { gens = new Set(); @@ -212,6 +217,7 @@ try { const both = sequences.filter((seq) => seen.get(seq)?.size === 2).length; log(`read groups ${first}..${last} (published up to ${lastPublished}); ${both} arrived from both relays`); if (missing.length > 0) throw new Error(`dropped groups across the migration: ${missing.join(", ")}`); + if (corrupt) throw new Error(`corrupt group from ${corrupt}`); log("migrated without a dropped group"); } catch (err) { failure = err instanceof Error ? err : new Error(String(err)); From 714bb03c4e7fb0dbb0eddd099077596ac0052a9b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 09:12:14 -0700 Subject: [PATCH 13/15] test(net): open GOAWAY streams with a version after the lite-07 varint change Co-Authored-By: Claude Opus 5.5 --- js/net/src/connection/migrate.test.ts | 2 +- js/net/src/lite/connection.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/net/src/connection/migrate.test.ts b/js/net/src/connection/migrate.test.ts index a4af8d032f..2d6969764c 100644 --- a/js/net/src/connection/migrate.test.ts +++ b/js/net/src/connection/migrate.test.ts @@ -65,7 +65,7 @@ function fleet(origin = new OriginProducer()): { dials: Dial[]; origin: OriginPr /** Send a moq-lite GOAWAY from the server side of a session. */ async function goaway(server: WebTransport, uri: string): Promise { - const stream = await Stream.open(server); + const stream = await Stream.open(server, { version: Lite.Version.DRAFT_06 }); await stream.writer.u53(Lite.StreamId.Goaway); await new Lite.Goaway(uri).encode(stream.writer, Lite.Version.DRAFT_06); } diff --git a/js/net/src/lite/connection.test.ts b/js/net/src/lite/connection.test.ts index 703a6702ae..fc11239b1f 100644 --- a/js/net/src/lite/connection.test.ts +++ b/js/net/src/lite/connection.test.ts @@ -52,7 +52,7 @@ test("a throwing getStats advertises None rather than propagating", async () => }); async function sendGoaway(server: WebTransport, uri: string): Promise { - const stream = await Stream.open(server); + const stream = await Stream.open(server, { version: Version.DRAFT_04 }); await stream.writer.u53(StreamId.Goaway); await new Goaway(uri).encode(stream.writer, Version.DRAFT_04); stream.writer.close(); From 3a867323a923ed26d4805bbfc84b80a2db52303c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 21:38:41 -0700 Subject: [PATCH 14/15] test(drain): subscribe to the route before peeking it The follow loop peeked request.active and then awaited changed(), so a route change could in principle slip between the two. Subscribe first, then apply the current value once. Co-Authored-By: Claude Opus 5.5 --- test/drain/drain.ts | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/test/drain/drain.ts b/test/drain/drain.ts index 40512d8f12..59716364eb 100644 --- a/test/drain/drain.ts +++ b/test/drain/drain.ts @@ -161,23 +161,19 @@ async function read(sub: Moq.Track.Subscriber, gen: number): Promise { // Follow whichever broadcast the path routes to, as a player does: subscribe to the new one and // drop the old one each time the route changes. -let watching = true; -const follow = (async () => { - let current: { broadcast: Moq.Broadcast.Consumer; sub: Moq.Track.Subscriber } | undefined; - while (watching) { - const active = request.active.peek(); - if (active && active !== current?.broadcast) { - generation++; - log(`watching generation ${generation}`); - const sub = active.track(trackName).subscribe({ maxAge: MAX_AGE }); - void read(sub, generation); - current?.sub.close(); - current = { broadcast: active, sub }; - } - await request.active.changed(); - } +// Subscribed before the first peek, so no route change can land between reading it and listening. +let current: { broadcast: Moq.Broadcast.Consumer; sub: Moq.Track.Subscriber } | undefined; +const follow = (active: Moq.Broadcast.Consumer | undefined) => { + if (!active || active === current?.broadcast) return; + generation++; + log(`watching generation ${generation}`); + const sub = active.track(trackName).subscribe({ maxAge: MAX_AGE }); + void read(sub, generation); current?.sub.close(); -})(); + current = { broadcast: active, sub }; +}; +const unfollow = request.active.subscribe(follow); +follow(request.active.peek()); const readOn = (gen: number) => [...seen.values()].filter((gens) => gens.has(gen)).length; const newestOn = (gen: number) => Math.max(-1, ...[...seen].filter(([, gens]) => gens.has(gen)).map(([seq]) => seq)); @@ -222,7 +218,8 @@ try { } catch (err) { failure = err instanceof Error ? err : new Error(String(err)); } finally { - watching = false; + unfollow(); + current?.sub.close(); clearInterval(ticker); request.close(); viewer.close(); @@ -230,8 +227,6 @@ try { broadcast.close(); proxy.close(); } -// `follow` parks on a route change that may never come once the request closes. -void follow; if (failure) { console.error(`error: ${failure.message}`); From e24fdb82d9bca4907a84647403d8a9527ea4f3db Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 1 Oct 2026 00:13:07 -0700 Subject: [PATCH 15/15] quest: drop the plain-text blocker from redirect-resolve Co-Authored-By: Claude Opus 5.5 --- quest/m1/redirect-resolve.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/quest/m1/redirect-resolve.md b/quest/m1/redirect-resolve.md index 37f4e8460c..8e31590936 100644 --- a/quest/m1/redirect-resolve.md +++ b/quest/m1/redirect-resolve.md @@ -22,6 +22,4 @@ needs them. If a consumer turns up, the alternative is returning the same `Result>` as the internal `target` does, so empty and refused stay distinct. Removing or changing a published method is a break, so this targets `dev`; update `doc/lib/rs` if it mentions the method. -## Required - -- `dev` has merged `main` since the drain line (moq-dev/moq#4132) landed +Start by merging `main` into `dev` if `dev` does not have the drain line yet.