diff --git a/serf-compio/Cargo.toml b/serf-compio/Cargo.toml index a20dbc5a..7e51f0c0 100644 --- a/serf-compio/Cargo.toml +++ b/serf-compio/Cargo.toml @@ -11,8 +11,12 @@ description = "compio-based async driver for the Sans-I/O serf machine" default = ["tcp", "tag-regex"] # Plain-TCP reliable coordinator. tcp = ["serf-proto/tcp", "serf-driver/tcp"] -# TLS-over-TCP reliable coordinator (implies tcp). -tls = ["tcp", "serf-proto/tls", "compio/rustls"] +# TLS-over-TCP reliable coordinator (implies tcp). The record layer comes from +# serf-proto; a crypto-backend feature (tls-rustls-*) supplies the rustls +# provider via memberlist-proto (serf-proto exposes no rustls provider itself). +tls = ["tcp", "serf-proto/tls", "serf-driver/tls", "compio/rustls"] +tls-rustls-ring = ["tls", "memberlist-proto/tls-rustls-ring"] +tls-rustls-aws-lc-rs = ["tls", "memberlist-proto/tls-rustls-aws-lc-rs"] # QUIC coordinator. quic = ["serf-proto/quic", "serf-driver/quic"] quic-rustls-ring = ["quic", "serf-proto/quic-rustls-ring", "serf-driver/quic-rustls-ring"] @@ -28,6 +32,10 @@ chacha20-poly1305 = [ ] # Regex-backed tag-filter matching. tag-regex = ["serf-proto/tag-regex", "serf-driver/tag-regex"] +# Test-only fault-injection surface (forwards serf-proto's `test` feature: a +# `MessageDropper` installed via `VoidDelegate::with_message_dropper`). Compiled +# out of every non-test build. +test = ["serf-proto/test"] # Emit `tracing` spans around the public driver operations. tracing = ["dep:tracing", "serf-driver/tracing"] # Optional config layering: `serde` adds Serialize/Deserialize; `clap` adds CLI flags. @@ -112,7 +120,7 @@ required-features = ["tcp"] # delegate, snapshot persistence, user event, query round-trip, graceful leave). [[test]] name = "tls" -required-features = ["tls"] +required-features = ["tls-rustls-ring"] # The real-node QUIC test suite: loopback nodes over a quinn config bundle # driving the QUIC pump's whole command surface (join/converge, user event, query @@ -123,5 +131,19 @@ name = "quic" required-features = ["quic-rustls-ring"] [package.metadata.docs.rs] -all-features = true +# A single coherent crypto-provider set (ring, not aws-lc-rs) so the doc build +# documents every gated item without pulling conflicting rustls providers. +features = [ + "tls-rustls-ring", + "quic-rustls-ring", + "coordinates", + "aes-gcm", + "chacha20-poly1305", + "tag-regex", + "tracing", + "serde", + "clap", + "dns", + "getifs", +] rustdoc-args = ["--cfg", "docsrs"] diff --git a/serf-compio/src/delegate/mod.rs b/serf-compio/src/delegate/mod.rs index e8fd0e63..5fab6901 100644 --- a/serf-compio/src/delegate/mod.rs +++ b/serf-compio/src/delegate/mod.rs @@ -144,6 +144,16 @@ pub trait Delegate: type Id; /// Address type — always `SocketAddr` in compio. type Address; + + /// Test-only inbound message-drop hook. The driver installs the returned + /// [`MessageDropper`](serf_proto::MessageDropper) on the machine so a test can + /// drop selected inbound membership messages; `None` (the default) drops + /// nothing. Gated behind the `test` feature — no production use. + #[cfg(feature = "test")] + #[cfg_attr(docsrs, doc(cfg(feature = "test")))] + fn message_dropper(&self) -> Option> { + None + } } /// Observer the driver notifies after it rotates the LIVE wire keyring, so an diff --git a/serf-compio/src/delegate/tests.rs b/serf-compio/src/delegate/tests.rs index 2cf474d3..0b755a36 100644 --- a/serf-compio/src/delegate/tests.rs +++ b/serf-compio/src/delegate/tests.rs @@ -45,3 +45,34 @@ fn a_sync_predicate_satisfies_the_merge_delegate() { "a permit-all predicate admits the exchange" ); } + +/// A BOXED machine merge delegate satisfies the trait — the shape the node +/// constructors' `Option>>` slot takes, so this pins +/// that the re-exported trait and its `Box` blanket impl compose. +#[cfg(any(feature = "tcp", feature = "quic"))] +#[test] +fn boxed_merge_delegate_satisfies_trait() { + struct AcceptAll; + impl MergeDelegate for AcceptAll { + fn notify_merge( + &self, + _peers: memberlist_proto::MaybeOwned< + '_, + [memberlist_proto::typed::NodeState], + >, + ) -> bool { + true + } + } + fn assert_merge(t: &T) -> bool + where + T: MergeDelegate, + { + t.notify_merge(memberlist_proto::MaybeOwned::Borrowed(&[])) + } + let boxed: Box> = Box::new(AcceptAll); + assert!( + assert_merge(&boxed), + "the boxed predicate's verdict is the one the machine acts on" + ); +} diff --git a/serf-compio/src/delegate/void.rs b/serf-compio/src/delegate/void.rs index 581895d6..24a3515b 100644 --- a/serf-compio/src/delegate/void.rs +++ b/serf-compio/src/delegate/void.rs @@ -21,6 +21,10 @@ use super::KeyringDelegate; #[cfg_attr(docsrs, doc(cfg(any(feature = "tcp", feature = "quic"))))] pub struct VoidDelegate { _phantom: PhantomData, + /// Test-only inbound message-drop hook returned via + /// [`Delegate::message_dropper`]; `None` in every real build. + #[cfg(feature = "test")] + message_dropper: Option>, } #[cfg(any(feature = "tcp", feature = "quic"))] @@ -30,8 +34,24 @@ impl VoidDelegate { pub const fn new() -> Self { Self { _phantom: PhantomData, + #[cfg(feature = "test")] + message_dropper: None, } } + + /// Attach a test-only [`MessageDropper`](serf_proto::MessageDropper) surfaced + /// through [`Delegate::message_dropper`], so a test node drops selected + /// inbound membership messages. Test fault injection only. + #[cfg(feature = "test")] + #[cfg_attr(docsrs, doc(cfg(feature = "test")))] + #[must_use] + pub fn with_message_dropper( + mut self, + dropper: std::sync::Arc, + ) -> Self { + self.message_dropper = Some(dropper); + self + } } #[cfg(any(feature = "tcp", feature = "quic"))] @@ -77,6 +97,11 @@ where { type Id = I; type Address = A; + + #[cfg(feature = "test")] + fn message_dropper(&self) -> Option> { + self.message_dropper.clone() + } } /// A keyring delegate that persists nothing. diff --git a/serf-compio/src/driver/options/tests.rs b/serf-compio/src/driver/options/tests.rs index 2d9ef3ec..14b4c687 100644 --- a/serf-compio/src/driver/options/tests.rs +++ b/serf-compio/src/driver/options/tests.rs @@ -476,3 +476,21 @@ fn tracing_forwards_to_the_shared_engines() { ) }; } + +/// `Default` and `new()` are the same source of truth for the stream knobs, and +/// the defaults they produce are themselves admissible — a node built with no +/// explicit stream configuration passes the same `validate` gate `Transport::new` +/// applies. +#[test] +fn stream_transport_options_default_matches_new() { + let d = StreamTransportOptions::default(); + let n = StreamTransportOptions::new(); + assert_eq!(d.dial_timeout(), n.dial_timeout()); + assert_eq!(d.close_timeout(), n.close_timeout()); + assert_eq!(d.bridge_inbound_cap(), n.bridge_inbound_cap()); + assert_eq!(d.bridge_recv_buf_len(), n.bridge_recv_buf_len()); + assert!( + d.validate().is_ok(), + "the default stream knobs are admissible" + ); +} diff --git a/serf-compio/src/driver/quic/mod.rs b/serf-compio/src/driver/quic/mod.rs index 9af2e24a..645e4b89 100644 --- a/serf-compio/src/driver/quic/mod.rs +++ b/serf-compio/src/driver/quic/mod.rs @@ -34,7 +34,7 @@ use futures_channel::oneshot; use futures_util::{FutureExt, pin_mut, select_biased}; use lochan::mpsc; use memberlist_proto::{ - Instant, Rng, SeedableRng, StreamId, Transmit, + DatagramSendStatus, Instant, Rng, SeedableRng, StreamId, Transmit, UnreliableTransport, codec::{ DecodeOptions, EncodeOptions, decode_incoming, encode_outgoing, encode_outgoing_compound, parse_messages, @@ -59,8 +59,9 @@ use crate::{ driver::{ options::RuntimeOptions, shared::{ - ExchangeId, add_obs_payload, dispatch_event_delegate, drain_past_due_udp, - observation_payload_bytes, yield_once, + ExchangeId, Farewell, ShutdownComplete, add_obs_payload, dispatch_event_delegate, + drain_past_due_udp, leave_outcome, observation_payload_bytes, send_gossip_datagram, + trace_leave_transform_error, yield_once, }, }, drop_counter::CompioDropCounter, @@ -303,8 +304,16 @@ pub(crate) async fn quic_driver_loop( events_tx: Sender>, events_dropped: Rc>, observation_dropped: Rc>, + // Cumulative gossip payloads accepted onto the QUIC datagram plane. Shares the + // cell a `Serf` handle reads through `datagrams_sent()`, so the count is visible + // with no publish step. + datagrams_sent: Rc>, snapshot: SnapshotCell, shutdown_flag: Rc>, + // The driver half of the teardown-completion latch. Dropped at the very end of + // the cleanup below — once the gossip socket is closed — so a `shutdown()` + // caller parked on the waiter returns to a free bind address. + shutdown_complete: ShutdownComplete, driver_opts: RuntimeOptions, delegate: D, // Cluster label applied to both gossip encode and decode. `None` accepts @@ -346,16 +355,25 @@ pub(crate) async fn quic_driver_loop( )) .detach(); - // Stash for the [`Command::Shutdown`] reply — acked AFTER the post-loop cleanup - // closes the gossip socket so the bound port is free when the caller resumes - // from `shutdown.await`. - let mut shutdown_reply: Option>> = None; + // Stash for every [`Command::Shutdown`] reply — each acked AFTER the post-loop + // cleanup closes the gossip socket, so the bound port is free when the caller + // resumes from `shutdown.await`. A vector, not a single slot: `shutdown()` is + // idempotent, so racing callers (cloned handles, or a straggler command drained + // during teardown) all park here and all resolve `Ok(())` together once the port + // is released. + let mut shutdown_reply: Vec>> = Vec::new(); #[cfg(encryption)] let mut pending_key_responses: Vec> = Vec::new(); let mut pending = PendingCommands { joins: Vec::new(), leave: None, }; + // Graceful-leave fan-out accounting. Until `leave()` is initiated the gossip + // egress rides the configured unreliable transport best-effort; from then on the + // fan-out is routed over plain UDP and every send is classified, so a local + // delivery failure resolves the parked leave with an error rather than a false + // `Ok`. + let mut farewell = Farewell::new(); // Per-pump UDP recv buffer size, derived once at entry as the larger of the // gossip plane (`gossip_mtu` + AEAD wrapper) and the raw-QUIC plane (quinn's @@ -397,6 +415,7 @@ pub(crate) async fn quic_driver_loop( &mut endpoint, &mut shutdown_reply, &mut pending, + &mut farewell, driver_opts.leave_timeout(), c, now, @@ -439,6 +458,8 @@ pub(crate) async fn quic_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, + &datagrams_sent, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -509,6 +530,8 @@ pub(crate) async fn quic_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, + &datagrams_sent, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -544,6 +567,8 @@ pub(crate) async fn quic_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, + &datagrams_sent, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -610,6 +635,7 @@ pub(crate) async fn quic_driver_loop( &mut endpoint, &mut shutdown_reply, &mut pending, + &mut farewell, driver_opts.leave_timeout(), c, now, @@ -654,6 +680,8 @@ pub(crate) async fn quic_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, + &datagrams_sent, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -677,11 +705,18 @@ pub(crate) async fn quic_driver_loop( // Cleanup. Order: flip the shutdown flag so a racing clone observes it on // entry, drain queued commands with Err(Shutdown), drop the command receiver - // so a late send fails fast, resolve any parked leave, close the bound socket - // (awaited so its port is released), then ack the observed shutdown caller. + // so a late send parks on the completion latch instead, resolve any parked + // leave, close the bound socket (awaited so its port is released), then ack + // every parked shutdown caller and fire the latch. shutdown_flag.set(true); while let Ok(c) = commands.try_recv() { - reply_shutdown(c); + match c { + // A straggler `Shutdown` raced the teardown into the queue. Park it beside + // the caller the loop already observed rather than failing it: `shutdown()` + // is idempotent and must not resolve while the port is still bound. + Command::Shutdown(ShutdownCmd { reply }) => shutdown_reply.push(reply), + other => reply_shutdown(other), + } } drop(commands); // Reply Err(Shutdown) to every parked await-result join waiter whose reply has @@ -708,10 +743,14 @@ pub(crate) async fn quic_driver_loop( // asynchronously, so a plain drop could race a same-port rebind into AddrInUse). let _ = gossip_socket.close().await; - if let Some(reply) = shutdown_reply { + // The bind address is free. Ack every parked caller, then fire the completion + // latch so a `shutdown()` the closed queue turned away also returns — both + // paths therefore resolve only once an immediate rebind would succeed. + for reply in shutdown_reply { // Ignoring Err: caller dropped the reply receiver. let _ = reply.send(Ok(())); } + drop(shutdown_complete); } /// Reply `Err(Shutdown)` to a command drained during teardown. @@ -760,13 +799,14 @@ fn reply_shutdown(c: Command) { /// the per-command reply channel — a dropped reply receiver means the caller /// gave up. /// -/// The [`Command::Shutdown`] reply is NOT acked inline; it is stashed into +/// The [`Command::Shutdown`] reply is NOT acked inline; it is parked in /// `shutdown_reply` so the pump acks the caller only AFTER the socket drops in /// the post-loop cleanup. async fn dispatch_command( endpoint: &mut QuicEndpoint, - shutdown_reply: &mut Option>>, + shutdown_reply: &mut Vec>>, pending: &mut PendingCommands, + farewell: &mut Farewell, leave_timeout: Duration, cmd: Command, now: Instant, @@ -866,6 +906,11 @@ async fn dispatch_command( let res: Result<()> = endpoint.leave(now).map_err(SerfError::from); match res { Ok(()) if was_alive => { + // The leave mutated and the fan-out is queued: route it over plain UDP + // (exact socket-handoff semantics, even in `Datagram` mode) and classify + // every send, so a farewell the local socket refuses fails the leave + // instead of vanishing. + farewell.initiated = true; pending.leave = Some(PendingLeave { repliers: vec![reply], deadline: now + leave_timeout, @@ -1019,9 +1064,9 @@ async fn dispatch_command( let _ = reply.send(Ok(endpoint.cached_coordinate(&id))); } Command::Shutdown(ShutdownCmd { reply }) => { - // Do NOT ack the caller here — the socket is still bound; stash the reply + // Do NOT ack the caller here — the socket is still bound; park the reply // and let the post-loop cleanup ack AFTER it drops. - *shutdown_reply = Some(reply); + shutdown_reply.push(reply); } } } @@ -1156,14 +1201,36 @@ where progress } -/// Drain every queued unreliable (gossip-plane) [`Transmit`] and send it on the -/// shared UDP socket. Outbound gossip is label-stamped (`encode_outgoing` / -/// `encode_outgoing_compound`), then — with an encryption backend built in — -/// wrapped in the encryption layer (`encrypt_gossip`) before it hits the wire. +/// Drain every queued unreliable (gossip-plane) [`Transmit`], encode it +/// (`encode_outgoing` / `encode_outgoing_compound`) and — with an encryption +/// backend built in — seal it (`encrypt_gossip`), then route it onto the +/// unreliable wire the endpoint is CONFIGURED for: a QUIC datagram over the +/// peer's pooled (quinn-TLS-protected) connection in +/// [`Datagram`](UnreliableTransport::Datagram) mode, or the shared UDP socket in +/// [`Udp`](UnreliableTransport::Udp) mode. +/// +/// A datagram that quinn cannot take right now — the connection is not yet +/// established ([`NotReady`](DatagramSendStatus::NotReady)) or the payload +/// exceeds the peer's negotiated datagram limit +/// ([`TooLarge`](DatagramSendStatus::TooLarge)) — falls back to the plain-UDP +/// path, which every peer demuxes in both modes. `NotReady` may mean the queue +/// just initiated a cold dial, so it also flushes quinn's outbound this pass to +/// get the connection's Initial onto the wire. +/// +/// Popping the last transmit is the endpoint's leave-completion fence (it emits +/// `LeftCluster`), so the leave fan-out reaches the wire before that fence fires. +/// Once `leave()` has been initiated the fan-out rides plain UDP in BOTH modes: a +/// frame queued into quinn is emitted only when congestion control and pacing +/// allow, so an accepted queue-handoff would not prove the farewell reached the +/// socket — and the fan-out has no retry round to absorb that loss. The plain-UDP +/// send gives exact socket-handoff semantics (completed, or error-classified into +/// `farewell`). async fn drain_transmits( endpoint: &mut QuicEndpoint, gossip_socket: &UdpSocket, label: Option, + farewell: &mut Farewell, + datagrams_sent: &Cell, ) -> bool where I: memberlist_proto::Id + Clone, @@ -1171,7 +1238,13 @@ where R: Rng + SeedableRng, { let encode_opts = EncodeOptions::new(label); + let unreliable = endpoint.unreliable_transport(); + // One timestamp for the whole pass: every datagram this drain queues into quinn + // is stamped with the same instant the flush below advances to, so a + // datagram-borne probe's queue time and its flush cannot straddle a clock read. + let now = Instant::now(); let mut progress = false; + let mut needs_flush = false; while let Some(transmit) = endpoint.poll_memberlist_transmit() { progress = true; let (peer, plain): (SocketAddr, Bytes) = match transmit { @@ -1181,14 +1254,20 @@ where Ok(b) => (to, b), // A locally-built message that fails to encode is dropped so one bad // codec invocation cannot wedge the pump. - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, to); + continue; + } } } Transmit::Compound(cmp) => { let (to, msgs) = cmp.into_parts(); match encode_outgoing_compound(&msgs, &encode_opts) { Ok(b) => (to, b), - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, to); + continue; + } } } }; @@ -1203,17 +1282,76 @@ where { on_wire = match endpoint.encrypt_gossip(&on_wire) { Ok(bytes) => bytes, - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, peer); + continue; + } }; } - let BufResult(res, _buf) = gossip_socket.send_to(on_wire, peer).await; - // Ignoring Err: a transient send error is non-fatal — gossip is lossy and - // the next probe/gossip round recovers. - let _ = res; + match unreliable { + // Plain-UDP gossip: best-effort for periodic rounds, classified once the + // leave fan-out is in flight. + UnreliableTransport::Udp => { + send_gossip_datagram(gossip_socket, peer, on_wire, farewell).await; + } + // The leave carve-out (see the doc comment): the departure fan-out takes + // the plain-UDP path even in `Datagram` mode, for exact socket-handoff + // semantics. Peers demux plain gossip datagrams in every mode — the + // `NotReady` / `TooLarge` fallbacks below rely on exactly that. + UnreliableTransport::Datagram if farewell.initiated => { + send_gossip_datagram(gossip_socket, peer, on_wire, farewell).await; + } + UnreliableTransport::Datagram => { + // `Vec` → `Bytes` moves the allocation; the fallback arms below pay a + // copy only when quinn declines the datagram. + let on_wire = Bytes::from(on_wire); + match endpoint.queue_unreliable_datagram(peer, on_wire.clone(), now) { + // Accepted onto an established QUIC connection: flush it into + // `poll_transmit` this pass (below) so a datagram-borne probe leaves on + // the tick its timeout is armed. + DatagramSendStatus::Queued => { + needs_flush = true; + datagrams_sent.set(datagrams_sent.get().saturating_add(1)); + } + // NotReady may mean the queue just initiated a cold dial: flush this + // pass so the connection's Initial is emitted now (else it does not warm + // until the next driver wake). The gossip itself still goes out + // immediately over the plain-UDP fallback. + DatagramSendStatus::NotReady => { + needs_flush = true; + send_gossip_datagram(gossip_socket, peer, on_wire.to_vec(), farewell).await; + } + // TooLarge: the connection is already Established (its datagram limit is + // known), so there is no pending Initial to flush; fall back to plain UDP. + DatagramSendStatus::TooLarge => { + send_gossip_datagram(gossip_socket, peer, on_wire.to_vec(), farewell).await; + } + } + } + } + } + // Flush the datagrams queued above into `poll_transmit` NOW, so the raw-QUIC + // drain that runs next in `drain_outputs` sends them this pass — a + // datagram-borne probe whose timeout is armed on this same tick must not wait + // for the next driver wake (that wake can be the timeout). `needs_flush` is + // only ever set from inside the loop above, which has already reported progress. + if needs_flush { + endpoint.flush_outbound_transmits(now); } progress } +/// Record a gossip datagram that could not be encoded or encrypted. Best-effort +/// periodic gossip drops it silently (the next round rebuilds it); a datagram of +/// the leave fan-out has no next round, so the failure is logged and fails the +/// parked leave. +fn note_farewell_transform_error(farewell: &mut Farewell, peer: SocketAddr) { + if farewell.initiated { + trace_leave_transform_error(peer); + farewell.send_failed = true; + } +} + /// Drain every raw outbound QUIC datagram (handshake, acks, reliable stream data) /// the coordinator queued, and send it on the shared UDP socket. These are /// already framed by quinn-proto, so no codec wrap is applied. @@ -1255,6 +1393,7 @@ async fn drain_events( pending: &mut PendingCommands, snapshotter: &mut Option>, terminal: &mut bool, + farewell: &Farewell, #[cfg(encryption)] keyring: &dyn KeyringDelegate, #[cfg(encryption)] pending_key_responses: &mut Vec>, ) -> bool @@ -1323,11 +1462,16 @@ where // Leave-completion resolution. `LeftCluster` fires once the leave notices // have drained to the wire; resolving the parked waiter here — on this pump // task, ahead of the observation task's `notify_leave` — is what makes - // `leave()` return promptly once the flush is done. + // `leave()` return promptly once the flush is done. `Ok` means every farewell + // datagram reached the socket: the fan-out was popped (and awaited over plain + // UDP) by the gossip drain that runs ahead of this one, so a local send or + // transform failure has already been recorded and downgrades the reply to + // `LeaveFarewellUndelivered` rather than a false success. if matches!(ev, Event::LeftCluster) && let Some(pl) = pending.leave.take() { - pl.resolve_all(|| Ok(())).await; + let failed = farewell.send_failed; + pl.resolve_all(|| leave_outcome(failed)).await; } // Conflict-shutdown enforcement. `Event::Shutdown` means the local node lost // an id-conflict vote and MUST stop, exactly as for a `Command::Shutdown`. @@ -1413,6 +1557,8 @@ async fn drain_outputs( obs_payload_budget: Option, pending: &mut PendingCommands, snapshotter: &mut Option>, + farewell: &mut Farewell, + datagrams_sent: &Cell, #[cfg(encryption)] keyring: &dyn KeyringDelegate, #[cfg(encryption)] pending_key_responses: &mut Vec>, ) -> bool @@ -1424,7 +1570,18 @@ where let mut terminal = false; loop { let did_ingress = drain_ingress::(endpoint, label); - let did_transmits = drain_transmits::(endpoint, gossip_socket, label.clone()).await; + // Gossip egress runs BEFORE the raw-QUIC drain (so a datagram queued into + // quinn leaves this pass) and before the event drain (so a leave fan-out has + // reached the socket, and recorded any failure in `farewell`, by the time the + // `LeftCluster` fence it releases is drained below). + let did_transmits = drain_transmits::( + endpoint, + gossip_socket, + label.clone(), + farewell, + datagrams_sent, + ) + .await; let did_quic = drain_quic_transmits::(endpoint, gossip_socket).await; let did_events = drain_events::( endpoint, @@ -1435,6 +1592,7 @@ where pending, snapshotter, &mut terminal, + farewell, #[cfg(encryption)] keyring, #[cfg(encryption)] diff --git a/serf-compio/src/driver/shared/mod.rs b/serf-compio/src/driver/shared/mod.rs index b1d018f4..fb73ab16 100644 --- a/serf-compio/src/driver/shared/mod.rs +++ b/serf-compio/src/driver/shared/mod.rs @@ -36,12 +36,56 @@ pub(crate) async fn yield_once() { /// Coordinator-allocated handle for one in-flight reliable exchange. /// -/// Shared by the TCP driver and the per-bridge task so they agree on the -/// same opaque id without the rest of the crate naming the machine's -/// streams module. -#[cfg(feature = "tcp")] +/// Shared by both reliable planes — the TCP driver with its per-bridge tasks, and +/// the QUIC driver with its per-connection streams — so each agrees on the same +/// opaque id without the rest of the crate naming the machine's streams module. +#[cfg(any(feature = "tcp", feature = "quic"))] pub(crate) type ExchangeId = memberlist_proto::event::ExchangeId; +/// Driver half of the teardown-completion latch. +/// +/// The pump owns this for its whole life and drops it — firing the latch — only +/// once it has released its bind sockets and acked every parked +/// [`shutdown`](crate::Serf::shutdown) caller. Nothing is ever sent on it: the +/// latch fires by sender-disconnect, so a pump that is torn down without +/// reaching its cleanup still releases every waiter rather than stranding it. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) struct ShutdownComplete { + /// Held solely for its `Drop`; a value is never sent. + _tx: flume::Sender<()>, +} + +/// Handle half of the teardown-completion latch. +/// +/// A `shutdown()` whose command the driver can no longer accept (the pump is +/// already tearing down, so the flag is set or the queue is gone) parks here +/// instead of returning into a still-bound port. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) struct ShutdownWaiter { + rx: flume::Receiver<()>, +} + +#[cfg(any(feature = "tcp", feature = "quic"))] +impl ShutdownWaiter { + /// Resolve once the driver has released its bind sockets — i.e. once the bind + /// address is free for an immediate rebind, NOT once every connected stream fd + /// has closed. Resolves immediately if the driver has already finished. + pub(crate) async fn wait(&self) { + // Ignoring Err: the latch fires by sender-disconnect, never by a sent value, + // so `recv_async` resolves to `Err` exactly once teardown completes. + let _ = self.rx.recv_async().await; + } +} + +/// Mint a teardown-completion latch: the [`ShutdownComplete`] the driver holds +/// until its bind sockets are released, and the [`ShutdownWaiter`] every `Serf` +/// clone shares. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) fn shutdown_latch() -> (ShutdownComplete, ShutdownWaiter) { + let (tx, rx) = flume::bounded(0); + (ShutdownComplete { _tx: tx }, ShutdownWaiter { rx }) +} + /// Dispatch the matching [`Delegate`] hook for one drained serf [`Event`]. /// /// Member hooks run once per affected member in the batch; user-event and @@ -98,6 +142,214 @@ pub(crate) fn add_obs_payload(counter: &std::cell::Cell, bytes: Option #[cfg(any(feature = "tcp", feature = "quic"))] pub(crate) use serf_driver::observation_payload_bytes; +// ── leave-farewell delivery ─────────────────────────────────────────────────── +// +// Periodic gossip is best-effort: a failed UDP send drops the datagram, and SWIM +// re-sends on the next round. The graceful-leave fan-out has NO next round — a +// dropped farewell leaves peers to classify the intentional departure as a +// failure while `leave()` reported success — so once `leave()` has been initiated +// the pump CLASSIFIES each fan-out send's outcome instead of discarding it, and a +// local delivery failure resolves the parked leave with +// `LeaveFarewellUndelivered` rather than a false `Ok`. +// +// Completion I/O has no backpressure state to park on: `send_to().await` resolves +// only once the operation itself completed, and compio's backends fold a +// would-block into an internal re-arm and an interrupt into a syscall retry. So +// each awaited send either handed the datagram to the socket or answered with a +// real error — there is nothing to retain for a later writable wake. +// +// An errored send never accepted the CURRENT datagram, and on a shared +// unconnected UDP socket the kernel may surface an asynchronous error left by an +// EARLIER packet to a DIFFERENT peer — so an ICMP-reflection-class error +// (reset/refused) is disambiguated by bounded re-send: each re-send is a distinct +// syscall that both drains the stale error slot and re-hands this datagram to the +// socket, and an error that persists across the re-sends is credibly this +// destination's own answer (a peer that is itself gone), which the reference +// implementation logs and proceeds past. Every other error kind — aborts (a local +// software abort on Windows), unreachables (usually the LOCAL routing table's +// answer), a closed or invalid socket — is a local delivery failure that fails the +// leave. + +/// Per-pump accounting for the graceful-leave farewell fan-out. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) struct Farewell { + /// `leave()` has been initiated, so the gossip egress carries the departure + /// fan-out: a send that fails is classified rather than dropped best-effort. + pub(crate) initiated: bool, + /// A LOCAL send or transform failure occurred — the farewell (or part of it) + /// never left this host — so the parked leave resolves + /// [`LeaveFarewellUndelivered`](crate::error::SerfError::LeaveFarewellUndelivered) + /// instead of a false `Ok`. + pub(crate) send_failed: bool, +} + +#[cfg(any(feature = "tcp", feature = "quic"))] +impl Farewell { + /// A pump that has not yet initiated a leave. + pub(crate) const fn new() -> Self { + Self { + initiated: false, + send_failed: false, + } + } +} + +/// Total ICMP-class (`ConnectionReset` / `ConnectionRefused`) send errors one +/// farewell datagram absorbs before it is dropped as answered-by-the-network. +/// The first errors are ambiguous (a stale asynchronous error from an earlier +/// packet to a different peer may occupy the socket's error slot), so the +/// datagram is re-sent; at the limit the answer is attributed to this destination +/// itself. +#[cfg(any(feature = "tcp", feature = "quic"))] +const FAREWELL_ICMP_ERROR_LIMIT: u8 = 3; + +/// The final outcome of one graceful leave: `Ok` when every farewell datagram +/// reached the socket, [`LeaveFarewellUndelivered`] when a local send or +/// transform failure lost part of the fan-out. +/// +/// [`LeaveFarewellUndelivered`]: crate::error::SerfError::LeaveFarewellUndelivered +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) fn leave_outcome(send_failed: bool) -> crate::error::Result<()> { + if send_failed { + Err(crate::error::SerfError::LeaveFarewellUndelivered) + } else { + Ok(()) + } +} + +/// The disposition of one errored leave-farewell send. +#[cfg(any(feature = "tcp", feature = "quic"))] +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum ErroredFarewell { + /// Ambiguous ICMP-class error under the limit: re-send the datagram (with the + /// bumped absorb count) — the re-send drains a possibly-stale asynchronous + /// error slot and re-hands the datagram to the socket. + Resend(u8), + /// ICMP-class errors persisted to [`FAREWELL_ICMP_ERROR_LIMIT`]: the answer is + /// credibly this destination's own (the peer is itself gone). Logged and + /// dropped without failing the leave, as the reference implementation does. + PeerAnswered, + /// Any other error kind: a LOCAL delivery failure — the farewell never left + /// this host — so the leave resolves + /// [`LeaveFarewellUndelivered`](crate::error::SerfError::LeaveFarewellUndelivered). + LocalFailure, +} + +/// Classify one errored farewell send, given how many ICMP-class errors this +/// datagram has already absorbed. +/// +/// Only `ConnectionReset` / `ConnectionRefused` are the ambiguous +/// ICMP-reflection class: they are what a gone peer's ICMP answer surfaces on +/// every platform (and what Windows reflects routinely after a send to a closed +/// port), and on a shared unconnected socket they may equally be a stale answer +/// to an EARLIER packet for a different peer — hence bounded re-send rather than +/// trusting either reading. `ConnectionAborted` is a local software abort on +/// Windows, the unreachables usually report the LOCAL routing table's answer, and +/// everything else (a closed or invalid socket, a vanished source address, a +/// broken pipe) is unambiguously local. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) fn classify_errored_farewell(icmp_errors: u8, err: &std::io::Error) -> ErroredFarewell { + if matches!( + err.kind(), + std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::ConnectionRefused + ) { + let absorbed = icmp_errors.saturating_add(1); + if absorbed < FAREWELL_ICMP_ERROR_LIMIT { + ErroredFarewell::Resend(absorbed) + } else { + ErroredFarewell::PeerAnswered + } + } else { + ErroredFarewell::LocalFailure + } +} + +/// Surface a leave-farewell datagram the driver could not deliver. Unlike +/// best-effort periodic gossip the drop is logged (the send error is +/// deterministic, config/IO-class, and the fan-out has no next round to mask it). +/// A no-op without the `tracing` feature. +#[cfg(any(feature = "tcp", feature = "quic"))] +fn trace_leave_send_error(_peer: std::net::SocketAddr, _err: &std::io::Error) { + #[cfg(feature = "tracing")] + tracing::debug!(peer = %_peer, error = %_err, "serf leave farewell datagram send failed"); +} + +/// Surface a leave-farewell datagram dropped after absorbing +/// [`FAREWELL_ICMP_ERROR_LIMIT`] ICMP-class send errors: the network's answer is +/// attributed to this destination (a peer that is itself gone), and the reference +/// implementation logs such peers and proceeds. A no-op without the `tracing` +/// feature. +#[cfg(any(feature = "tcp", feature = "quic"))] +fn trace_leave_peer_answered(_peer: std::net::SocketAddr) { + #[cfg(feature = "tracing")] + tracing::debug!( + peer = %_peer, + "serf leave farewell dropped after repeated ICMP-class send errors; peer presumed gone" + ); +} + +/// Surface a leave-farewell datagram dropped because it could not be encoded or +/// encrypted — a deterministic config-class failure, unlike a transient +/// best-effort gossip drop. A no-op without the `tracing` feature. +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) fn trace_leave_transform_error(_peer: std::net::SocketAddr) { + #[cfg(feature = "tracing")] + tracing::debug!(peer = %_peer, "serf leave farewell datagram could not be encoded or encrypted"); +} + +/// Send one already-transformed gossip datagram over the plain-UDP socket. +/// +/// Periodic gossip (`acct.initiated == false`) is best-effort: a failed send +/// drops the datagram and the next round recovers it. Once `leave()` has been +/// initiated the datagram is the departure fan-out, so a failed send is +/// classified by [`classify_errored_farewell`]: an ambiguous ICMP-class answer is +/// re-sent (bounded), and a LOCAL failure records `acct.send_failed` so the parked +/// leave resolves [`LeaveFarewellUndelivered`] instead of a false `Ok`. +/// +/// [`LeaveFarewellUndelivered`]: crate::error::SerfError::LeaveFarewellUndelivered +#[cfg(any(feature = "tcp", feature = "quic"))] +pub(crate) async fn send_gossip_datagram( + socket: &compio::net::UdpSocket, + peer: std::net::SocketAddr, + on_wire: Vec, + acct: &mut Farewell, +) { + use compio::buf::BufResult; + + if !acct.initiated { + let BufResult(res, _buf) = socket.send_to(on_wire, peer).await; + // Ignoring Err: a transient send error is non-fatal for periodic gossip — + // gossip is lossy and the next probe/gossip round recovers. + let _ = res; + return; + } + + // The leave fan-out. Bounded by the ICMP absorb allowance: each pass either + // hands the datagram to the socket, re-sends it against a possibly-stale error + // slot, or reaches a terminal classification, so the loop always ends. + let mut buf = on_wire; + let mut icmp_errors = 0u8; + loop { + let BufResult(res, returned) = socket.send_to(buf, peer).await; + buf = returned; + let Err(err) = res else { + return; + }; + trace_leave_send_error(peer, &err); + match classify_errored_farewell(icmp_errors, &err) { + ErroredFarewell::Resend(absorbed) => icmp_errors = absorbed, + ErroredFarewell::PeerAnswered => { + trace_leave_peer_answered(peer); + return; + } + ErroredFarewell::LocalFailure => { + acct.send_failed = true; + return; + } + } + } +} + /// Bounded past-due UDP drain shared by both driver pumps. /// /// When the coordinator's wake deadline is already past, the driver must not diff --git a/serf-compio/src/driver/shared/tests.rs b/serf-compio/src/driver/shared/tests.rs index a017f851..f2d252d5 100644 --- a/serf-compio/src/driver/shared/tests.rs +++ b/serf-compio/src/driver/shared/tests.rs @@ -153,3 +153,231 @@ async fn empty_socket_drains_nothing() { // Ignoring Err: test cleanup of the probe socket. let _ = driver.close().await; } + +// ── leave-farewell classification ───────────────────────────────────────────── + +/// A fresh pump has not initiated a leave and has recorded no failure, so its +/// leave outcome is a clean success. +#[test] +fn a_fresh_farewell_is_clean() { + let acct = Farewell::new(); + assert!(!acct.initiated); + assert!(!acct.send_failed); + assert!(leave_outcome(acct.send_failed).is_ok()); +} + +/// A local send or transform failure during the fan-out resolves the leave with +/// `LeaveFarewellUndelivered`, never a false `Ok` — the caller learns that at +/// least one peer will read the departure as a failure. +#[test] +fn a_local_failure_fails_the_leave() { + let err = leave_outcome(true).expect_err("a local send failure must fail the leave"); + assert!(matches!( + err, + crate::error::SerfError::LeaveFarewellUndelivered + )); +} + +/// The ICMP-reflection class (`ConnectionReset` / `ConnectionRefused`) is +/// AMBIGUOUS on a shared unconnected UDP socket — the error slot may hold a stale +/// asynchronous answer to an earlier packet for a DIFFERENT peer — so the first +/// errors are absorbed and the datagram is re-sent, each re-send both draining the +/// stale slot and re-handing the datagram to the socket. +#[test] +fn an_icmp_class_error_under_the_limit_is_resent() { + for kind in [ + std::io::ErrorKind::ConnectionReset, + std::io::ErrorKind::ConnectionRefused, + ] { + assert_eq!( + classify_errored_farewell(0, &std::io::Error::from(kind)), + ErroredFarewell::Resend(1), + "the first {kind:?} is ambiguous and is re-sent" + ); + assert_eq!( + classify_errored_farewell(1, &std::io::Error::from(kind)), + ErroredFarewell::Resend(2), + ); + } +} + +/// ICMP-class errors that PERSIST to the absorb limit are credibly this +/// destination's own answer (a peer that is itself gone). The reference +/// implementation logs such peers and proceeds, so the datagram is dropped and the +/// leave still succeeds — a departing node is not held back by peers that already +/// left. +#[test] +fn persistent_icmp_class_errors_are_the_peer_answering() { + assert_eq!( + classify_errored_farewell( + FAREWELL_ICMP_ERROR_LIMIT - 1, + &std::io::Error::from(std::io::ErrorKind::ConnectionReset) + ), + ErroredFarewell::PeerAnswered, + ); + // PeerAnswered does not record a failure, so the leave still resolves `Ok`. + assert!(leave_outcome(false).is_ok()); +} + +/// Every non-ICMP error kind is a LOCAL delivery failure — the farewell never left +/// this host — so it fails the leave. `ConnectionAborted` is a local software abort +/// on Windows, the unreachables usually report the LOCAL routing table's answer, +/// and a closed/invalid socket or broken pipe is unambiguously local. +#[test] +fn every_other_error_kind_is_a_local_failure() { + for kind in [ + std::io::ErrorKind::ConnectionAborted, + std::io::ErrorKind::HostUnreachable, + std::io::ErrorKind::NetworkUnreachable, + std::io::ErrorKind::BrokenPipe, + std::io::ErrorKind::NotConnected, + std::io::ErrorKind::InvalidInput, + std::io::ErrorKind::PermissionDenied, + ] { + assert_eq!( + classify_errored_farewell(0, &std::io::Error::from(kind)), + ErroredFarewell::LocalFailure, + "{kind:?} is a local delivery failure and must fail the leave" + ); + } +} + +/// A periodic-gossip send (no leave in flight) is best-effort: a send to an +/// unroutable destination records NO failure, so a later leave is not poisoned by +/// an unrelated dropped gossip datagram. +#[compio::test] +async fn a_pre_leave_gossip_send_never_records_a_failure() { + let (driver, _peer, dst) = socket_pair().await; + let mut acct = Farewell::new(); + + send_gossip_datagram(&driver, dst, b"gossip".to_vec(), &mut acct).await; + + assert!( + !acct.send_failed, + "periodic gossip is best-effort and must never record a farewell failure" + ); + assert!(leave_outcome(acct.send_failed).is_ok()); + + // Ignoring Err: test cleanup of the probe socket. + let _ = driver.close().await; +} + +/// Once `leave()` is initiated, a farewell datagram that the local socket ACCEPTS +/// records no failure, so the leave resolves `Ok`. +#[compio::test] +async fn a_delivered_farewell_keeps_the_leave_ok() { + let (driver, _peer, dst) = socket_pair().await; + let mut acct = Farewell::new(); + acct.initiated = true; + + send_gossip_datagram(&driver, dst, b"farewell".to_vec(), &mut acct).await; + + assert!( + !acct.send_failed, + "a farewell the socket accepted must not fail the leave" + ); + assert!(leave_outcome(acct.send_failed).is_ok()); + + // Ignoring Err: test cleanup of the probe socket. + let _ = driver.close().await; +} + +/// A farewell the local socket REFUSES records the failure, so the parked leave +/// resolves `LeaveFarewellUndelivered` instead of a false `Ok`. +/// +/// The refusal is forced deterministically by an address-family mismatch: an +/// IPv4-bound socket cannot send to an IPv6 destination, and the OS rejects it +/// locally (never an ICMP-class reflection), which is exactly the "the farewell +/// never left this host" class. This is the raise path end to end: socket error → +/// classification → `send_failed` → the leave's error. +#[compio::test] +async fn a_refused_farewell_raises_leave_farewell_undelivered() { + let v4: SocketAddr = "127.0.0.1:0".parse().expect("v4 loopback"); + let socket = UdpSocket::bind(v4).await.expect("bind a v4 gossip socket"); + let v6_dst: SocketAddr = "[::1]:9".parse().expect("v6 destination"); + + let mut acct = Farewell::new(); + acct.initiated = true; + + send_gossip_datagram(&socket, v6_dst, b"farewell".to_vec(), &mut acct).await; + + assert!( + acct.send_failed, + "a farewell the local socket refused must record the delivery failure" + ); + let err = leave_outcome(acct.send_failed).expect_err("the leave must not report a false success"); + assert!(matches!( + err, + crate::error::SerfError::LeaveFarewellUndelivered + )); + + // Ignoring Err: test cleanup of the probe socket. + let _ = socket.close().await; +} + +/// The SAME refused send on a pump that has NOT initiated a leave is best-effort: +/// it records nothing, so an unrelated gossip failure can never poison a later +/// leave into a false `LeaveFarewellUndelivered`. +#[compio::test] +async fn a_refused_pre_leave_gossip_send_does_not_poison_a_later_leave() { + let v4: SocketAddr = "127.0.0.1:0".parse().expect("v4 loopback"); + let socket = UdpSocket::bind(v4).await.expect("bind a v4 gossip socket"); + let v6_dst: SocketAddr = "[::1]:9".parse().expect("v6 destination"); + + let mut acct = Farewell::new(); + + send_gossip_datagram(&socket, v6_dst, b"gossip".to_vec(), &mut acct).await; + + assert!( + !acct.send_failed, + "periodic gossip is best-effort; a failed send must not be charged to a leave" + ); + assert!(leave_outcome(acct.send_failed).is_ok()); + + // Ignoring Err: test cleanup of the probe socket. + let _ = socket.close().await; +} + +/// The rotation-durability acknowledgement contract both pumps park a key +/// response on: an unacknowledged rotation keeps the response parked; a persisted +/// rotation sends it unchanged; a persistence failure — or a worker that vanished +/// without acknowledging — downgrades it to `result = false` carrying the error, +/// so a caller is never told a rotation was durable when it was not. +#[cfg(encryption)] +#[test] +fn parked_key_responses_settle_by_acknowledgement_outcome() { + use std::sync::mpsc; + + use serf_driver::settle_parked_key_response; + + let ok_resp = serf_proto::event::KeyResponseArgs { + result: true, + message: "".into(), + ..Default::default() + }; + + // Still pending: stays parked. + let (tx, rx) = mpsc::channel(); + assert!(settle_parked_key_response(&rx, &ok_resp).is_none()); + + // Persisted: the response goes out unchanged. + tx.send(Ok(())).expect("ack sends"); + let settled = settle_parked_key_response(&rx, &ok_resp).expect("resolved"); + assert!(settled.result); + assert!(settled.message.is_empty()); + + // Persistence failure: downgraded, carrying the error. + let (tx, rx) = mpsc::channel(); + tx.send(Err(std::io::Error::other("disk gone").into())) + .expect("ack sends"); + let settled = settle_parked_key_response(&rx, &ok_resp).expect("resolved"); + assert!(!settled.result); + assert!(settled.message.contains("disk gone")); + + // Worker vanished without acknowledging: a failure, not a silent success. + let (tx, rx) = mpsc::channel::>(); + drop(tx); + let settled = settle_parked_key_response(&rx, &ok_resp).expect("resolved"); + assert!(!settled.result); + assert!(settled.message.contains("without acknowledging")); +} diff --git a/serf-compio/src/driver/stream/mod.rs b/serf-compio/src/driver/stream/mod.rs index 333c510a..86a34b9a 100644 --- a/serf-compio/src/driver/stream/mod.rs +++ b/serf-compio/src/driver/stream/mod.rs @@ -67,8 +67,9 @@ use crate::{ driver::{ options::{RuntimeOptions, StreamTransportOptions}, shared::{ - ExchangeId, add_obs_payload, dispatch_event_delegate, drain_past_due_udp, - observation_payload_bytes, yield_once, + ExchangeId, Farewell, ShutdownComplete, add_obs_payload, dispatch_event_delegate, + drain_past_due_udp, leave_outcome, observation_payload_bytes, send_gossip_datagram, + trace_leave_transform_error, yield_once, }, }, drop_counter::CompioDropCounter, @@ -123,7 +124,9 @@ struct PendingJoin { /// denominator on a zero-contact resolution. requested: usize, /// Wall-clock instant past which the driver replies with whatever `contacted` - /// set it has accumulated even if `pending` is non-empty. + /// set it has accumulated even if `pending` is non-empty. Reconciled against the + /// coordinator's own per-exchange deadline at construction via + /// [`clamp_join_deadline`], which caps it at the exchange deadline. deadline: Instant, /// One-shot reply channel back to the caller, taken when the reply resolves /// (all-exchanges-done or `deadline`). `None` once resolved; the waiter then @@ -132,6 +135,27 @@ struct PendingJoin { reply: Option>, } +/// Reconcile a caller's await-result join deadline with the machine's push/pull +/// exchange deadline (`now + stream_timeout`), returning the effective +/// [`PendingJoin::deadline`]. +/// +/// A join we initiate carries two clocks: this driver-local fallback deadline and +/// the coordinator's own per-exchange deadline (`now + stream_timeout`). A caller +/// deadline LATER than the exchange deadline lets an elapsed exchange emit a +/// terminal `ExchangeCompleted(Failed)` — reaping a `JoinAllFailed` for a +/// `join_deadline` that has not elapsed. Clamping the driver deadline so it never +/// exceeds the exchange deadline keeps the two clocks consistent: the join can +/// only resolve all-failed once the exchange that would fail it has actually run +/// out of time. Plumbing the caller deadline INTO the exchange (collapsing the two +/// clocks) is a serf-proto follow-up. +fn clamp_join_deadline( + caller_deadline: Instant, + now: Instant, + stream_timeout: Duration, +) -> Instant { + caller_deadline.min(now + stream_timeout) +} + impl PendingJoin { /// Resolve the caller's reply once, from the current `contacted` set. Idempotent: /// after the first call `reply` is `None` and this is a no-op, so the deadline @@ -427,12 +451,21 @@ pub(crate) async fn stream_driver_loop( observation_dropped: Rc>, snapshot: SnapshotCell, shutdown_flag: Rc>, + // The driver half of the teardown-completion latch. Dropped at the very end of + // the cleanup below — once the listener and gossip socket are closed — so a + // `shutdown()` caller parked on the waiter returns to a free bind address. + shutdown_complete: ShutdownComplete, driver_opts: RuntimeOptions, stream_opts: StreamTransportOptions, delegate: D, // Cluster label applied to both gossip encode and decode. `None` accepts // datagrams from any cluster. label: Option, + // The coordinator's per-exchange push/pull deadline window, snapshotted in + // `Transport::run` from the SAME `EndpointOptions` the coordinator was built + // from. An await-result join's caller deadline is reconciled against it in + // `clamp_join_deadline`. + stream_timeout: Duration, // The snapshot writer the pump appends membership records to; `None` // disables persistence. mut snapshotter: Option>, @@ -475,16 +508,24 @@ pub(crate) async fn stream_driver_loop( )) .detach(); - // Stash for the [`Command::Shutdown`] reply — acked AFTER the post-loop - // cleanup closes the listener and gossip socket so the bound ports are free - // when the caller resumes from `shutdown.await`. - let mut shutdown_reply: Option>> = None; + // Stash for every [`Command::Shutdown`] reply — each acked AFTER the post-loop + // cleanup closes the listener and gossip socket, so the bound ports are free + // when the caller resumes from `shutdown.await`. A vector, not a single slot: + // `shutdown()` is idempotent, so racing callers (cloned handles, or a straggler + // command drained during teardown) all park here and all resolve `Ok(())` + // together once the ports are released. + let mut shutdown_reply: Vec>> = Vec::new(); #[cfg(encryption)] let mut pending_key_responses: Vec> = Vec::new(); let mut pending = PendingCommands { joins: Vec::new(), leave: None, }; + // Graceful-leave fan-out accounting. Until `leave()` is initiated the gossip + // egress is best-effort; from then on every farewell send is classified, so a + // local delivery failure resolves the parked leave with an error rather than a + // false `Ok`. + let mut farewell = Farewell::new(); // Per-pump UDP recv buffer size, derived once at entry from the coordinator's // `gossip_mtu` (fixed for the endpoint lifetime). @@ -563,7 +604,9 @@ pub(crate) async fn stream_driver_loop( stream_opts, &mut shutdown_reply, &mut pending, + &mut farewell, driver_opts.leave_timeout(), + stream_timeout, c, now, ) @@ -641,6 +684,7 @@ pub(crate) async fn stream_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -719,6 +763,7 @@ pub(crate) async fn stream_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -757,6 +802,7 @@ pub(crate) async fn stream_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -836,7 +882,9 @@ pub(crate) async fn stream_driver_loop( stream_opts, &mut shutdown_reply, &mut pending, + &mut farewell, driver_opts.leave_timeout(), + stream_timeout, c, now, ).await; @@ -912,6 +960,7 @@ pub(crate) async fn stream_driver_loop( obs_payload_budget, &mut pending, &mut snapshotter, + &mut farewell, #[cfg(encryption)] &*keyring, #[cfg(encryption)] @@ -935,12 +984,18 @@ pub(crate) async fn stream_driver_loop( // Cleanup. Order: flip the shutdown flag so a racing clone observes it on // entry, drain queued commands with Err(Shutdown), drop the command receiver - // so a late send fails fast, signal every live bridge to close, close the - // bound sockets (awaited so their ports are released), then ack the observed - // shutdown caller. + // so a late send parks on the completion latch instead, signal every live + // bridge to close, close the bound sockets (awaited so their ports are + // released), then ack every parked shutdown caller and fire the latch. shutdown_flag.set(true); while let Ok(c) = commands.try_recv() { - reply_shutdown(c); + match c { + // A straggler `Shutdown` raced the teardown into the queue. Park it beside + // the caller the loop already observed rather than failing it: `shutdown()` + // is idempotent and must not resolve while the ports are still bound. + Command::Shutdown(ShutdownCmd { reply }) => shutdown_reply.push(reply), + other => reply_shutdown(other), + } } drop(commands); // Reply Err(Shutdown) to every parked await-result join waiter whose reply has @@ -982,10 +1037,14 @@ pub(crate) async fn stream_driver_loop( // released before the stashed reply fires. let _ = gossip_socket.close().await; - if let Some(reply) = shutdown_reply { + // The bind address is free. Ack every parked caller, then fire the completion + // latch so a `shutdown()` the closed queue turned away also returns — both + // paths therefore resolve only once an immediate rebind would succeed. + for reply in shutdown_reply { // Ignoring Err: caller dropped the reply receiver. let _ = reply.send(Ok(())); } + drop(shutdown_complete); } /// Reply `Err(Shutdown)` to a command drained during teardown. @@ -1043,9 +1102,11 @@ async fn dispatch_command( bridges: &mut HashMap, bridge_ready_tx: &Sender, stream_opts: StreamTransportOptions, - shutdown_reply: &mut Option>>, + shutdown_reply: &mut Vec>>, pending: &mut PendingCommands, + farewell: &mut Farewell, leave_timeout: Duration, + stream_timeout: Duration, cmd: Command, now: Instant, ) where @@ -1147,7 +1208,7 @@ async fn dispatch_command( contacted: SmallVec::new(), ignore_streams, requested, - deadline, + deadline: clamp_join_deadline(deadline, now, stream_timeout), reply: Some(reply), }); } @@ -1167,6 +1228,10 @@ async fn dispatch_command( let res: Result<()> = endpoint.leave(now).map_err(SerfError::from); match res { Ok(()) if was_alive => { + // The leave mutated and the fan-out is queued: switch the gossip + // egress from best-effort to classified, so a farewell the local + // socket refuses fails the leave instead of vanishing. + farewell.initiated = true; pending.leave = Some(PendingLeave { repliers: vec![reply], deadline: now + leave_timeout, @@ -1321,13 +1386,13 @@ async fn dispatch_command( } Command::Shutdown(ShutdownCmd { reply }) => { // Drain every live bridge so the byte-movers observe the close and exit. - // Do NOT ack the caller here — the sockets are still bound; stash the + // Do NOT ack the caller here — the sockets are still bound; park the // reply and let the post-loop cleanup ack AFTER they drop. for (_eid, handle) in bridges.drain() { // Ignoring Err: bridge may have already exited; close is best-effort. let _ = handle.out_tx.try_send(BridgeOut::Close); } - *shutdown_reply = Some(reply); + shutdown_reply.push(reply); } } } @@ -1575,10 +1640,18 @@ where /// gossip socket. Outbound gossip is label-stamped (`encode_outgoing` / /// `encode_outgoing_compound`), then — with an encryption backend built in — /// wrapped in the encryption layer (`encrypt_gossip`) before it hits the wire. +/// +/// Popping the last transmit is the endpoint's leave-completion fence (it emits +/// `LeftCluster`), so once `leave()` has been initiated these are the departure +/// fan-out and it reaches the socket before that fence fires. `farewell` carries +/// that state: a transform or LOCAL send failure on the fan-out is recorded there +/// (rather than dropped best-effort) so the parked leave resolves +/// [`SerfError::LeaveFarewellUndelivered`] instead of a false `Ok`. async fn drain_transmits( endpoint: &mut StreamEndpoint, gossip_socket: &UdpSocket, label: Option, + farewell: &mut Farewell, ) -> bool where I: memberlist_proto::Id + Clone, @@ -1597,14 +1670,20 @@ where Ok(b) => (to, b), // A locally-built message that fails to encode is dropped so one bad // codec invocation cannot wedge the pump. - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, to); + continue; + } } } Transmit::Compound(cmp) => { let (to, msgs) = cmp.into_parts(); match encode_outgoing_compound(&msgs, &encode_opts) { Ok(b) => (to, b), - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, to); + continue; + } } } }; @@ -1619,17 +1698,28 @@ where { on_wire = match endpoint.encrypt_gossip(&on_wire) { Ok(bytes) => bytes, - Err(_) => continue, + Err(_) => { + note_farewell_transform_error(farewell, peer); + continue; + } }; } - let BufResult(res, _buf) = gossip_socket.send_to(on_wire, peer).await; - // Ignoring Err: a transient send error is non-fatal — gossip is lossy and - // the next probe/gossip round recovers. - let _ = res; + send_gossip_datagram(gossip_socket, peer, on_wire, farewell).await; } progress } +/// Record a gossip datagram that could not be encoded or encrypted. Best-effort +/// periodic gossip drops it silently (the next round rebuilds it); a datagram of +/// the leave fan-out has no next round, so the failure is logged and fails the +/// parked leave. +fn note_farewell_transform_error(farewell: &mut Farewell, peer: SocketAddr) { + if farewell.initiated { + trace_leave_transform_error(peer); + farewell.send_failed = true; + } +} + /// Read-modify-write `endpoint`'s LIVE wire keyring for one inbound [`KeyRequest`], /// returning the [`KeyResponseArgs`] built from the post-op live state. /// @@ -1696,6 +1786,7 @@ async fn drain_events( pending: &mut PendingCommands, snapshotter: &mut Option>, terminal: &mut bool, + farewell: &Farewell, #[cfg(encryption)] keyring: &dyn KeyringDelegate, #[cfg(encryption)] pending_key_responses: &mut Vec>, ) -> bool @@ -1765,11 +1856,16 @@ where // Leave-completion resolution. `LeftCluster` fires once the leave notices // have drained to the wire; resolving the parked waiter here — on this pump // task, ahead of the observation task's `notify_leave` — is what makes - // `leave()` return promptly once the flush is done. + // `leave()` return promptly once the flush is done. `Ok` means every farewell + // datagram reached the socket: the fan-out was popped (and awaited) by the + // gossip drain that runs ahead of this one, so a local send or transform + // failure has already been recorded and downgrades the reply to + // `LeaveFarewellUndelivered` rather than a false success. if matches!(ev, Event::LeftCluster) && let Some(pl) = pending.leave.take() { - pl.resolve_all(|| Ok(())).await; + let failed = farewell.send_failed; + pl.resolve_all(|| leave_outcome(failed)).await; } // Conflict-shutdown enforcement. `Event::Shutdown` means the local node lost // an id-conflict vote and MUST stop, exactly as for a `Command::Shutdown`. @@ -1858,6 +1954,7 @@ async fn drain_outputs( obs_payload_budget: Option, pending: &mut PendingCommands, snapshotter: &mut Option>, + farewell: &mut Farewell, #[cfg(encryption)] keyring: &dyn KeyringDelegate, #[cfg(encryption)] pending_key_responses: &mut Vec>, ) -> bool @@ -1871,8 +1968,11 @@ where loop { let did_actions = drain_actions::(endpoint, bridges, bridge_ready_tx, stream_opts); let did_transports = drain_transport_transmits::(endpoint, bridges); + // Gossip egress runs BEFORE the event drain, so a leave fan-out has reached + // the socket (and recorded any failure in `farewell`) by the time the + // `LeftCluster` fence it releases is drained below. let did_transmits = - drain_transmits::(endpoint, gossip_socket, label.clone()).await; + drain_transmits::(endpoint, gossip_socket, label.clone(), farewell).await; let did_events = drain_events::( endpoint, obs_tx, @@ -1882,6 +1982,7 @@ where pending, snapshotter, &mut terminal, + farewell, #[cfg(encryption)] keyring, #[cfg(encryption)] diff --git a/serf-compio/src/driver/stream/tests.rs b/serf-compio/src/driver/stream/tests.rs index 1b201568..7f73881d 100644 --- a/serf-compio/src/driver/stream/tests.rs +++ b/serf-compio/src/driver/stream/tests.rs @@ -326,3 +326,59 @@ async fn all_exchanges_done_resolves_and_reaps() { other => panic!("a successful exchange must reply Ok(contacted), got {other:?}"), } } + +// ── await-result join deadline reconciliation ───────────────────────────────── + +/// A caller join deadline LATER than the coordinator's per-exchange deadline is +/// clamped down to the exchange deadline. +/// +/// The two clocks are independent: the driver's fallback deadline and the +/// coordinator's `now + stream_timeout` exchange deadline. Left unclamped, an +/// elapsed exchange emits a terminal `ExchangeCompleted(Failed)` while the +/// caller's deadline has NOT elapsed, so the join reaps a premature +/// `JoinAllFailed`. Clamping keeps the driver deadline from ever outliving the +/// exchange that would fail it. +#[test] +fn a_caller_deadline_past_the_exchange_deadline_is_clamped() { + let now = Instant::now(); + let stream_timeout = Duration::from_secs(10); + let caller = now + Duration::from_secs(60); + + let effective = clamp_join_deadline(caller, now, stream_timeout); + + assert_eq!( + effective, + now + stream_timeout, + "a caller deadline beyond the exchange deadline must clamp to the exchange deadline" + ); + assert!( + effective < caller, + "the clamp must actually pull the deadline in" + ); +} + +/// A caller join deadline EARLIER than the exchange deadline is the binding one +/// and passes through untouched — the clamp is a ceiling, not a floor, so a short +/// `join_deadline` still resolves on the caller's own schedule. +#[test] +fn a_caller_deadline_inside_the_exchange_window_is_kept() { + let now = Instant::now(); + let stream_timeout = Duration::from_secs(10); + let caller = now + Duration::from_secs(2); + + assert_eq!( + clamp_join_deadline(caller, now, stream_timeout), + caller, + "a caller deadline inside the exchange window is kept as-is" + ); +} + +/// The two clocks coinciding is the boundary case: the clamp is idempotent there. +#[test] +fn a_caller_deadline_equal_to_the_exchange_deadline_is_stable() { + let now = Instant::now(); + let stream_timeout = Duration::from_secs(10); + let caller = now + stream_timeout; + + assert_eq!(clamp_join_deadline(caller, now, stream_timeout), caller); +} diff --git a/serf-compio/src/error/mod.rs b/serf-compio/src/error/mod.rs index 059b1a22..95eb6694 100644 --- a/serf-compio/src/error/mod.rs +++ b/serf-compio/src/error/mod.rs @@ -147,6 +147,17 @@ pub enum SerfError { #[error("leave did not complete within the configured leave timeout")] LeaveTimeout, + /// A graceful [`leave`](crate::Serf::leave) completed locally but the local + /// socket failed while sending the departure fan-out, so at least one peer + /// was never handed the farewell and will classify the departure as a + /// failure. Per-peer network signals (a reset or unreachable reflected for a + /// peer that is itself gone) do NOT raise this — only a local send failure + /// does. + #[error( + "the local socket failed while sending the leave fan-out; peers may classify the departure as a failure" + )] + LeaveFarewellUndelivered, + /// The driver task has shut down and is no longer accepting commands. #[error("driver shut down")] Shutdown, diff --git a/serf-compio/src/error/tests.rs b/serf-compio/src/error/tests.rs index 2ee94e47..940208e4 100644 --- a/serf-compio/src/error/tests.rs +++ b/serf-compio/src/error/tests.rs @@ -52,6 +52,7 @@ fn every_variant_displays_and_debugs() { SerfError::Entropy(io::Error::other("entropy")), SerfError::Resolve(io::Error::other("dns")), SerfError::LeaveTimeout, + SerfError::LeaveFarewellUndelivered, SerfError::Shutdown, SerfError::NotRunning, SerfError::JoinAllFailed(JoinFailed::new(3, 0)), diff --git a/serf-compio/src/lib.rs b/serf-compio/src/lib.rs index 44e75d3f..c6f4e7eb 100644 --- a/serf-compio/src/lib.rs +++ b/serf-compio/src/lib.rs @@ -3,6 +3,7 @@ #![deny(missing_docs)] #![cfg_attr(docsrs, feature(doc_cfg))] #![cfg_attr(docsrs, allow(unused_attributes))] +#![forbid(unsafe_code)] #[cfg(feature = "tcp")] mod bridge; @@ -65,9 +66,15 @@ pub(crate) fn os_seeded_std_rng() -> crate::Result { } pub use error::{ - GossipMtuTooSmall, InvalidAdvertiseAddr, InvalidGossipMtu, InvalidOption, Result, SerfError, + GossipMtuTooSmall, InvalidAdvertiseAddr, InvalidGossipMtu, InvalidOption, JoinFailed, Result, + SerfError, }; +/// The seed/advertise address form re-exported from `memberlist-proto`: either an +/// already-`Resolved` wire [`std::net::SocketAddr`] or an `Unresolved` user +/// address the caller's [`Resolver`] resolves at the boundary. +pub use memberlist_proto::MaybeResolved; + #[cfg(any(feature = "tcp", feature = "quic"))] #[cfg_attr(docsrs, doc(cfg(any(feature = "tcp", feature = "quic"))))] pub use delegate::{ @@ -118,7 +125,7 @@ pub use resolver::{ #[cfg(feature = "dns")] #[cfg_attr(docsrs, doc(cfg(feature = "dns")))] -pub use resolver::{DEFAULT_DNS_TIMEOUT, DnsResolver}; +pub use resolver::{DEFAULT_DNS_TIMEOUT, DnsError, DnsResolver}; #[cfg(feature = "getifs")] #[cfg_attr(docsrs, doc(cfg(feature = "getifs")))] @@ -151,7 +158,7 @@ pub use serf::Serf; pub use driver::options::{ Channel, DEFAULT_BRIDGE_INBOUND_CAP, DEFAULT_BRIDGE_RECV_BUF_LEN, DEFAULT_CLOSE_TIMEOUT, DEFAULT_CMD_FAIRNESS_BUDGET, DEFAULT_DIAL_TIMEOUT, DEFAULT_EVENT_QUEUE_CAP, - DEFAULT_IDLE_WAKE_INTERVAL, DEFAULT_ITER_DRAIN_CAP, DEFAULT_LEAVE_TIMEOUT, + DEFAULT_IDLE_WAKE_INTERVAL, DEFAULT_ITER_DRAIN_CAP, DEFAULT_JOIN_DEADLINE, DEFAULT_LEAVE_TIMEOUT, DEFAULT_OBSERVATION_CHANNEL, DEFAULT_SNAPSHOT_COMPACT_THRESHOLD, ParseChannelError, RuntimeOptions, SnapshotOptions, StreamTransportOptions, }; diff --git a/serf-compio/src/quic/mod.rs b/serf-compio/src/quic/mod.rs index 541d96da..dd90fd8b 100644 --- a/serf-compio/src/quic/mod.rs +++ b/serf-compio/src/quic/mod.rs @@ -17,13 +17,13 @@ #![cfg(feature = "quic")] -use core::num::NonZeroU8; +use core::{num::NonZeroU8, time::Duration}; use std::{io::ErrorKind, net::SocketAddr}; use compio::net::UdpSocket; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, EndpointOptions, Id, MaybeResolved, QuicEndpoint as Coordinator, + CheapClone, EndpointOptions, Id, MaybeResolved, QuicEndpoint as Coordinator, }; use rand::rngs::StdRng; use smol_str::SmolStr; @@ -54,6 +54,29 @@ pub struct QuicTransportOptions> { local_id: Option, advertise_addr: Option>, quic_config: Option, + /// Override for the memberlist anti-entropy push/pull interval. `None` keeps the + /// coordinator default; `Some(Duration::ZERO)` disables periodic push/pull + /// entirely. See [`with_push_pull_interval`](Self::with_push_pull_interval). + push_pull_interval: Option, + /// SWIM probe interval override. `None` keeps the coordinator default. See + /// [`with_probe_interval`](Self::with_probe_interval). + probe_interval: Option, + /// SWIM direct-ping timeout override. `None` keeps the coordinator default. See + /// [`with_probe_timeout`](Self::with_probe_timeout). + probe_timeout: Option, + /// Gossip interval override. `None` keeps the coordinator default. See + /// [`with_gossip_interval`](Self::with_gossip_interval). + gossip_interval: Option, + /// SWIM suspicion multiplier override. `None` keeps the coordinator default. See + /// [`with_suspicion_mult`](Self::with_suspicion_mult). + suspicion_mult: Option, + /// Reclaim window for a same-name member returning at a NEW address: a dead + /// member older than this is revived in place of a conflict. See + /// [`with_dead_node_reclaim_time`](Self::with_dead_node_reclaim_time). + dead_node_reclaim_time: Option, + /// SWIM suspicion max-timeout multiplier override. `None` keeps the coordinator + /// default. See [`with_suspicion_max_timeout_mult`](Self::with_suspicion_max_timeout_mult). + suspicion_max_timeout_mult: Option, /// Gossip-encryption policy. The default (no keyring) leaves the gossip /// datagrams plaintext; attaching a keyring via /// [`with_encryption`](Self::with_encryption) makes the coordinator's @@ -74,6 +97,13 @@ impl QuicTransportOptions { local_id: None, advertise_addr: None, quic_config: None, + push_pull_interval: None, + probe_interval: None, + probe_timeout: None, + gossip_interval: None, + suspicion_mult: None, + dead_node_reclaim_time: None, + suspicion_max_timeout_mult: None, #[cfg(encryption)] encryption: EncryptionOptions::new(), } @@ -103,6 +133,90 @@ impl QuicTransportOptions { self } + /// Builder: override the memberlist anti-entropy push/pull interval. + /// + /// `None` (the default) keeps the coordinator's built-in interval. A positive + /// duration re-tunes the periodic full-state sync; `Duration::ZERO` disables + /// periodic push/pull entirely — join-time and explicit exchanges still run, but + /// no background anti-entropy is scheduled. + #[must_use] + #[inline] + pub const fn with_push_pull_interval(mut self, interval: Duration) -> Self { + self.push_pull_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM probe interval — how often the + /// coordinator probes a random peer for liveness. + /// + /// `None` (the default) keeps the coordinator default (~1s). A shorter interval + /// speeds failure detection at the cost of more probe traffic; it also shortens + /// the suspicion timeout, which scales with the probe interval. + #[must_use] + #[inline] + pub const fn with_probe_interval(mut self, interval: Duration) -> Self { + self.probe_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM direct-ping timeout — how long the + /// coordinator waits for a probe ack before escalating to indirect probes. + /// + /// `None` (the default) keeps the coordinator default (~500ms). It must + /// comfortably exceed the real network round-trip, or a live peer whose ack is + /// merely slow is falsely suspected. + #[must_use] + #[inline] + pub const fn with_probe_timeout(mut self, timeout: Duration) -> Self { + self.probe_timeout = Some(timeout); + self + } + + /// Builder: override the memberlist gossip interval — how often the coordinator + /// flushes queued gossip to a random subset of peers. + /// + /// `None` (the default) keeps the coordinator default (~200ms). + #[must_use] + #[inline] + pub const fn with_gossip_interval(mut self, interval: Duration) -> Self { + self.gossip_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM suspicion multiplier — how long a + /// suspected peer is held in the Suspect state before being declared Failed. + /// + /// The minimum suspicion timeout is `suspicion_mult * log10(N+1) * probe_interval`. + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_mult(mut self, mult: u32) -> Self { + self.suspicion_mult = Some(mult); + self + } + + /// Builder: allow a dead member to be revived under the SAME id at a NEW + /// address once it has been dead longer than `window` — the reference + /// implementation's dead-node reclaim. Left unset (the default), a same-name + /// Alive from a different address is a name conflict, never a revival. + #[must_use] + #[inline] + pub const fn with_dead_node_reclaim_time(mut self, window: Duration) -> Self { + self.dead_node_reclaim_time = Some(window); + self + } + + /// Builder: override the memberlist SWIM suspicion max-timeout multiplier — the + /// upper bound on the suspicion timeout as a multiple of the minimum. + /// + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_max_timeout_mult(mut self, mult: u32) -> Self { + self.suspicion_max_timeout_mult = Some(mult); + self + } + /// Builder: gossip-encryption policy. /// /// The default (no keyring) keeps the gossip datagrams plaintext, so an @@ -141,6 +255,49 @@ impl QuicTransportOptions { self.quic_config.as_ref() } + /// The push/pull interval override, if set. + #[inline] + pub const fn push_pull_interval(&self) -> Option { + self.push_pull_interval + } + + /// The SWIM probe-interval override, if set. + #[inline] + pub const fn probe_interval(&self) -> Option { + self.probe_interval + } + + /// The SWIM probe-timeout override, if set. + #[inline] + pub const fn probe_timeout(&self) -> Option { + self.probe_timeout + } + + /// The gossip-interval override, if set. + #[inline] + pub const fn gossip_interval(&self) -> Option { + self.gossip_interval + } + + /// The SWIM suspicion-multiplier override, if set. + #[inline] + pub const fn suspicion_mult(&self) -> Option { + self.suspicion_mult + } + + /// The configured dead-node reclaim window, if overridden. + #[must_use] + #[inline] + pub const fn dead_node_reclaim_time(&self) -> Option { + self.dead_node_reclaim_time + } + + /// The SWIM suspicion max-timeout-multiplier override, if set. + #[inline] + pub const fn suspicion_max_timeout_mult(&self) -> Option { + self.suspicion_max_timeout_mult + } + /// Gossip-encryption policy. #[cfg(encryption)] #[cfg_attr( @@ -174,6 +331,19 @@ pub struct QuicTransport> { advertise_socket: SocketAddr, gossip_socket: UdpSocket, quic_config: QuicOptions, + /// Push/pull interval override, applied to the coordinator's `EndpointOptions` + /// in [`Transport::run`]. `None` keeps the default; `Some(Duration::ZERO)` + /// disables periodic anti-entropy. + push_pull_interval: Option, + /// SWIM failure-detection overrides applied to the coordinator's + /// `EndpointOptions` in [`Transport::run`]. Each `None` keeps the coordinator + /// default. + probe_interval: Option, + probe_timeout: Option, + gossip_interval: Option, + suspicion_mult: Option, + dead_node_reclaim_time: Option, + suspicion_max_timeout_mult: Option, /// Independent OS-seeded seed for the serf core's RNG, drawn once per node in /// [`Transport::new`] and consumed when [`Transport::run`] builds the /// endpoint via `new_with_rng`. Distinct from the coordinator's gossip RNG so @@ -188,7 +358,7 @@ pub struct QuicTransport> { impl Transport for QuicTransport where I: Id + CheapClone + core::fmt::Debug + core::fmt::Display + Send + Sync + 'static, - A: Data + Clone + Send + 'static, + A: Clone + Send + 'static, { type Error = SerfError; type Id = I; @@ -274,6 +444,13 @@ where advertise_socket, gossip_socket, quic_config, + push_pull_interval: options.push_pull_interval, + probe_interval: options.probe_interval, + probe_timeout: options.probe_timeout, + gossip_interval: options.gossip_interval, + suspicion_mult: options.suspicion_mult, + dead_node_reclaim_time: options.dead_node_reclaim_time, + suspicion_max_timeout_mult: options.suspicion_max_timeout_mult, serf_rng, #[cfg(encryption)] encryption: options.encryption, @@ -305,8 +482,36 @@ where // config. Serf ranks its user broadcasts on three tiers (intent / event / // query → ranks 0 / 1 / 2), so the inner memberlist endpoint needs at least // three broadcast tiers. - let inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) + let mut inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) .with_user_broadcast_tiers(NonZeroU8::new(3).expect("3 is nonzero")); + // A caller-supplied push/pull interval re-tunes (or, at `Duration::ZERO`, + // disables) the periodic anti-entropy full-state sync. Left unset, the + // coordinator keeps its own default. + if let Some(interval) = self.push_pull_interval { + inner_opts = inner_opts.with_push_pull_interval(interval); + } + // Caller-supplied SWIM failure-detection overrides: each left unset keeps the + // coordinator's own default. Lowering these speeds up failure detection (probe + // cadence, ack timeout, gossip cadence, and the suspicion timeout that scales + // with the probe interval). + if let Some(v) = self.probe_interval { + inner_opts = inner_opts.with_probe_interval(v); + } + if let Some(v) = self.probe_timeout { + inner_opts = inner_opts.with_probe_timeout(v); + } + if let Some(v) = self.gossip_interval { + inner_opts = inner_opts.with_gossip_interval(v); + } + if let Some(v) = self.suspicion_mult { + inner_opts = inner_opts.with_suspicion_mult(v); + } + if let Some(v) = self.dead_node_reclaim_time { + inner_opts = inner_opts.with_dead_node_reclaim_time(v); + } + if let Some(v) = self.suspicion_max_timeout_mult { + inner_opts = inner_opts.with_suspicion_max_timeout_mult(v); + } let inner = memberlist_proto::Endpoint::new(inner_opts, gossip_rng); // The shared UDP socket also carries raw QUIC packets, whose size is governed // by the quinn `EndpointConfig`'s accepted max UDP payload — which a caller @@ -365,8 +570,10 @@ where runtime.events_tx, runtime.events_dropped, runtime.observation_dropped, + runtime.datagrams_sent, runtime.snapshot, runtime.shutdown_flag, + runtime.shutdown_complete, runtime.driver_options, runtime.delegate, None, diff --git a/serf-compio/src/quic/tests.rs b/serf-compio/src/quic/tests.rs index deb6f893..b02d7758 100644 --- a/serf-compio/src/quic/tests.rs +++ b/serf-compio/src/quic/tests.rs @@ -167,12 +167,12 @@ fn test_quic_options_jumbo() -> QuicOptions { } /// Build and spawn a QUIC serf node bound to an ephemeral loopback port. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { spawn_node_with(id, test_quic_options()).await } /// Build and spawn a QUIC serf node from a caller-supplied [`QuicOptions`]. -async fn spawn_node_with(id: &str, quic: QuicOptions) -> Serf { +async fn spawn_node_with(id: &str, quic: QuicOptions) -> Serf { try_spawn_node_at(id, quic, "127.0.0.1:0".parse().expect("loopback addr")) .await .expect("spawn serf node") @@ -185,19 +185,18 @@ async fn try_spawn_node_at( id: &str, quic: QuicOptions, bind: SocketAddr, -) -> Result, SerfError> { +) -> Result, SerfError> { let opts = QuicTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_quic_config(quic); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::quic( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -218,22 +217,20 @@ async fn assert_quic_new_rejects(runtime: RuntimeOptions) { .with_local_id(SmolStr::new("bad-opt-node")) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_quic_config(test_quic_options()); - let res = - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( - opts, - &SocketAddrResolver, - &FirstAddrResolver, - VoidDelegate::::new(), - runtime, - SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), - None, - None, - None, - #[cfg(encryption)] - std::rc::Rc::new(VoidKeyringDelegate), - ) - .await; + let res = Serf::quic( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + runtime, + SerfOptions::new(), + None, + None, + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await; match res { Err(SerfError::InvalidOption(_)) => {} Err(other) => panic!("expected InvalidOption, got {other:?}"), @@ -357,6 +354,52 @@ async fn quic_shutdown_releases_bound_address_for_rebind() { second.shutdown().await.expect("second node shuts down"); } +/// A second `shutdown()` — issued once the QUIC driver has already exited and +/// closed its command queue — still resolves `Ok`, and only AFTER the bound UDP +/// address is free: the late caller parks on the teardown-completion latch rather +/// than returning into a still-bound port. The freed address is proven rebindable +/// immediately after. +#[compio::test] +async fn quic_second_shutdown_awaits_teardown_completion() { + let node = spawn_node("twice-a").await; + let addr = node.advertise_address(); + + node.shutdown().await.expect("the first shutdown resolves"); + node + .shutdown() + .await + .expect("a second shutdown after teardown still resolves Ok"); + + let reborn = try_spawn_node_at("twice-b", test_quic_options(), addr) + .await + .expect("the freed address rebinds after the awaited teardown"); + assert_eq!(reborn.advertise_address(), addr); + reborn.shutdown().await.expect("twice-b shuts down"); +} + +/// Two `shutdown()` calls issued CONCURRENTLY both land in the command queue +/// before the QUIC pump observes either. It dispatches the first and drains the +/// second during teardown; both must resolve `Ok` — and both only once the bound +/// UDP socket is released, which the immediate same-address rebind proves. +#[compio::test] +async fn quic_concurrent_shutdowns_resolve_ok_after_the_port_is_freed() { + let node = spawn_node("concurrent-shutdown-a").await; + let addr = node.advertise_address(); + + let (first, second) = futures_util::future::join(node.shutdown(), node.shutdown()).await; + first.expect("the observed shutdown resolves Ok"); + second.expect("the shutdown drained during teardown also resolves Ok"); + + let reborn = try_spawn_node_at("concurrent-shutdown-b", test_quic_options(), addr) + .await + .expect("both shutdowns resolved only after the port was freed"); + assert_eq!(reborn.advertise_address(), addr); + reborn + .shutdown() + .await + .expect("concurrent-shutdown-b shuts down"); +} + /// All `Serf` handles dropping under a continuous gossip flood must still shut the /// QUIC driver down. Under the flood the higher-priority recv arm starves the main /// select's command arm, so the command-channel disconnect is observable ONLY by @@ -536,21 +579,23 @@ fn test_secret_key(fill: u8) -> SecretKey { /// Build and spawn a QUIC serf node on an ephemeral loopback port with /// `encryption` installed as its gossip keyring policy. #[cfg(encryption)] -async fn spawn_encrypted_node(id: &str, encryption: EncryptionOptions) -> Serf { +async fn spawn_encrypted_node( + id: &str, + encryption: EncryptionOptions, +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = QuicTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_quic_config(test_quic_options()) .with_encryption(encryption); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::quic( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -610,13 +655,13 @@ async fn two_node_quic_join_observes_membership_encrypted() { /// blackhole join tests to shorten the await-join deadline — a QUIC dial to a /// closed UDP port has no fast reset, so its exchange resolves only at the /// deadline reaper). -async fn spawn_node_with_runtime(id: &str, runtime: RuntimeOptions) -> Serf { +async fn spawn_node_with_runtime(id: &str, runtime: RuntimeOptions) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = QuicTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_quic_config(test_quic_options()); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::quic_with_rng( opts, &SocketAddrResolver, &FirstAddrResolver, @@ -726,3 +771,144 @@ async fn quic_join_zero_resolution_surfaces_join_all_failed() { a.shutdown().await.expect("joiner shuts down"); } + +// ── SWIM / timing knobs ─────────────────────────────────────────────────────── + +/// Every SWIM knob starts UNSET, so a caller that sets none keeps the +/// coordinator's own defaults — `Transport::run` applies an override only when +/// it is `Some`. +#[test] +fn swim_knobs_start_unset() { + let opts = crate::QuicTransportOptions::::new(); + assert!(opts.push_pull_interval().is_none()); + assert!(opts.probe_interval().is_none()); + assert!(opts.probe_timeout().is_none()); + assert!(opts.gossip_interval().is_none()); + assert!(opts.suspicion_mult().is_none()); + assert!(opts.dead_node_reclaim_time().is_none()); + assert!(opts.suspicion_max_timeout_mult().is_none()); +} + +/// Every builder writes its OWN field: the accessors read back exactly what was +/// set, with distinct values per knob so a crossed assignment surfaces. +#[test] +fn swim_knob_builders_round_trip_each_knob() { + let opts = crate::QuicTransportOptions::::new() + .with_push_pull_interval(Duration::from_millis(1)) + .with_probe_interval(Duration::from_millis(2)) + .with_probe_timeout(Duration::from_millis(3)) + .with_gossip_interval(Duration::from_millis(4)) + .with_suspicion_mult(5) + .with_dead_node_reclaim_time(Duration::from_millis(6)) + .with_suspicion_max_timeout_mult(7); + + assert_eq!(opts.push_pull_interval(), Some(Duration::from_millis(1))); + assert_eq!(opts.probe_interval(), Some(Duration::from_millis(2))); + assert_eq!(opts.probe_timeout(), Some(Duration::from_millis(3))); + assert_eq!(opts.gossip_interval(), Some(Duration::from_millis(4))); + assert_eq!(opts.suspicion_mult(), Some(5)); + assert_eq!( + opts.dead_node_reclaim_time(), + Some(Duration::from_millis(6)) + ); + assert_eq!(opts.suspicion_max_timeout_mult(), Some(7)); +} + +/// A zero push/pull interval is a MEANINGFUL setting (it disables periodic +/// anti-entropy, isolating the gossip plane), so it must round-trip as +/// `Some(ZERO)` — never collapse back to the `None` that means "keep the +/// coordinator default". +#[test] +fn zero_push_pull_interval_is_set_not_unset() { + let opts = crate::QuicTransportOptions::::new() + .with_push_pull_interval(Duration::ZERO); + assert_eq!(opts.push_pull_interval(), Some(Duration::ZERO)); +} + +/// The identity builders round-trip through the options block. +#[test] +fn builders_round_trip() { + let addr: SocketAddr = "127.0.0.1:8300".parse().expect("addr"); + let opts = QuicTransportOptions::::new() + .with_local_id(SmolStr::new("node-a")) + .with_advertise_addr(MaybeResolved::Resolved(addr)); + + assert_eq!(opts.local_id(), Some(&SmolStr::new("node-a"))); + match opts.advertise_addr() { + Some(MaybeResolved::Resolved(s)) => assert_eq!(*s, addr), + other => panic!("expected a resolved advertise addr, got {other:?}"), + } +} + +/// An unresolved advertise input round-trips through the options block in its +/// ORIGINAL form: resolution happens once at construction, not in the builder. +#[test] +fn unresolved_advertise_addr_round_trips_unresolved() { + let host: hostaddr::HostAddr = "example.com:7946".parse().expect("host addr"); + let opts = QuicTransportOptions::>::new() + .with_advertise_addr(MaybeResolved::Unresolved(host.clone())); + match opts.advertise_addr() { + Some(MaybeResolved::Unresolved(h)) => assert_eq!(*h, host), + other => panic!("expected an unresolved advertise addr, got {other:?}"), + } +} + +/// The gossip keyring reaches the options block through the builder. On QUIC the +/// reliable plane rides quinn's own TLS, so this keyring protects the datagram +/// gossip plane. +#[cfg(encryption)] +#[test] +fn encryption_policy_round_trips() { + let key = test_secret_key(0x21); + let opts = QuicTransportOptions::::new() + .with_encryption(EncryptionOptions::new().with_keyring(Keyring::new(key))); + let keyring = opts + .encryption() + .keyring() + .expect("the configured keyring reaches the options block"); + assert_eq!( + keyring.primary_ref(), + &key, + "the primary key is the one that was configured" + ); +} + +/// `Default` delegates to `new` — the empty starting point, with no accidental +/// pre-set id or quinn bundle. +#[test] +fn default_matches_new() { + let opts = QuicTransportOptions::::default(); + assert!(opts.local_id().is_none()); + assert!(opts.quic_config().is_none()); +} + +/// A constructed transport reports the identity it was built with: the local id, +/// the advertise input in the ORIGINAL form the caller supplied, and the concrete +/// bound contact the node will gossip (its single UDP socket's readback). +#[compio::test] +async fn transport_reports_its_identity_and_bound_contact() { + let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); + let transport = QuicTransport::::new( + QuicTransportOptions::new() + .with_local_id(SmolStr::new("ident")) + .with_advertise_addr(MaybeResolved::Unresolved(bind)) + .with_quic_config(test_quic_options()), + &SocketAddrResolver, + &FirstAddrResolver, + ) + .await + .expect("the transport binds an ephemeral loopback UDP port"); + + assert_eq!(transport.local_id(), &SmolStr::new("ident")); + match transport.local_address() { + MaybeResolved::Unresolved(a) => assert_eq!(*a, bind), + other => panic!("the advertise INPUT form must be retained, got {other:?}"), + } + let advertise = *transport.advertise_address(); + assert!(advertise.ip().is_loopback()); + assert_ne!( + advertise.port(), + 0, + "the bound contact carries the OS-assigned port, not the ephemeral `:0`" + ); +} diff --git a/serf-compio/src/resolver/dns/mod.rs b/serf-compio/src/resolver/dns/mod.rs index 257fed35..75a5a0a5 100644 --- a/serf-compio/src/resolver/dns/mod.rs +++ b/serf-compio/src/resolver/dns/mod.rs @@ -140,9 +140,12 @@ impl DnsResolver { /// Bounded by `self.timeout` (default [`DEFAULT_DNS_TIMEOUT`]): a /// slow or hostile nameserver cannot hang the caller's `join` /// future beyond this wall-clock budget. On timeout returns - /// [`DnsError::Io`]`(io::ErrorKind::TimedOut)`; the resolver's - /// caller falls back to the OS resolver via the standard error - /// path (see [`Resolver::resolve`] below). + /// [`DnsError::Io`]`(io::ErrorKind::TimedOut)`, which + /// [`Resolver::resolve`] surfaces WITHOUT the OS fallback — that + /// fallback runs outside this deadline, so escalating a timeout into + /// it would defeat the bound. A genuine unavailability (connect + /// refused, unreachable nameserver, malformed response) or an empty + /// answer does fall through to the OS resolver. async fn tcp_query(&self, host: &str, port: u16) -> Result, DnsError> { let query = self.tcp_query_inner(host, port).fuse(); let timeout = compio::time::sleep(self.timeout).fuse(); @@ -238,15 +241,25 @@ impl Resolver for DnsResolver { // and only when we have at least one nameserver configured. Short // names will be resolved through the OS resolver's search-domain list. if host_str.contains('.') && !self.servers.is_empty() { - // Ignoring Err: TCP-first is best-effort per the upstream spec - // ("If this fails it's not fatal since this isn't a standard way to - // query DNS, and we have a fallback below.", memberlist.go:404). - // We unconditionally fall through to the OS resolver on any error - // or empty answer. - if let Ok(addrs) = self.tcp_query(host_str, port).await - && !addrs.is_empty() - { - return Ok(addrs); + // TCP-first is best-effort per the upstream spec ("If this fails it's not + // fatal since this isn't a standard way to query DNS, and we have a fallback + // below.", memberlist.go:404), so a genuine unavailability (connect refused, + // unreachable nameserver, malformed response) or an empty answer falls + // through to the OS resolver below. + match self.tcp_query(host_str, port).await { + // A productive TCP answer wins outright; no fallback needed. + Ok(addrs) if !addrs.is_empty() => return Ok(addrs), + // A configured-resolver TIMEOUT must NOT escalate into the OS resolver: the + // OS path runs OUTSIDE `self.timeout` (its DNS is unbounded / + // runtime-dependent), so falling through would let a slow or hostile + // nameserver burn the TCP deadline and THEN hang bootstrap in unbounded OS + // DNS — contradicting the timeout contract. Surface the timeout so the + // configured budget bounds the whole resolution. + Err(DnsError::Io(err)) if err.kind() == io::ErrorKind::TimedOut => { + return Err(DnsError::Io(err)); + } + // Empty answer or a genuine unavailability: fall through to the OS resolver. + Ok(_) | Err(_) => {} } } diff --git a/serf-compio/src/resolver/dns/tests.rs b/serf-compio/src/resolver/dns/tests.rs index 4c219e2e..2374c571 100644 --- a/serf-compio/src/resolver/dns/tests.rs +++ b/serf-compio/src/resolver/dns/tests.rs @@ -410,3 +410,82 @@ async fn tcp_query_empty_answers_falls_through_to_os_fallback() { handle.await.expect("DNS server task"); } + +/// A loopback TCP nameserver that ACCEPTS the query and then never answers: it +/// drains the 2-byte length prefix and the query body so the resolver's write +/// completes, then parks forever holding the connection open, so the resolver's +/// response read blocks until its OWN configured timeout fires. +async fn spawn_stalling_tcp_dns_server() -> (SocketAddr, compio::runtime::JoinHandle<()>) { + use compio::{buf::BufResult, io::AsyncReadExt, net::TcpListener}; + + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind loopback DNS server"); + let addr = listener.local_addr().expect("server local_addr"); + + let handle = compio::runtime::spawn(async move { + let Ok((mut stream, _)) = listener.accept().await else { + return; + }; + let len_buf = vec![0u8; 2]; + let BufResult(r, len_buf) = stream.read_exact(len_buf).await; + if r.is_err() { + return; + } + let qlen = u16::from_be_bytes([len_buf[0], len_buf[1]]) as usize; + // Ignoring Err: a truncated read just means the client hung up early; the + // assertion still holds because the resolver received no response either way. + let BufResult(_r, _q) = stream.read_exact(vec![0u8; qlen]).await; + // Park forever, keeping `stream` (and thus the open connection) alive. The + // test cancels this task by dropping its JoinHandle once the resolver has + // timed out. + core::future::pending::<()>().await; + }); + + (addr, handle) +} + +/// A configured nameserver that ACCEPTS the TCP-DNS query and then STALLS (never +/// writes a response) must not hang the caller: the per-query `timeout` bounds the +/// WHOLE resolution, and the timeout is surfaced as an error rather than silently +/// escalating into the unbounded OS-resolver fallback (which runs outside the +/// deadline). A hostile resolver must not be able to burn the TCP budget and THEN +/// hang bootstrap in OS DNS. +#[compio::test] +async fn stalling_server_times_out_without_os_fallback() { + let (server_addr, handle) = spawn_stalling_tcp_dns_server().await; + + let timeout = Duration::from_millis(100); + let r = DnsResolver::from_servers(vec![server_addr]).with_timeout(timeout); + // A fully-qualified name (contains a `.`) so the TCP-first branch is taken; the + // reserved `.test` TLD (RFC 6761) never resolves, so were the OS fallback reached + // it would surface a DIFFERENT lookup error (or hang) rather than this synthetic + // timeout — the assertion below discriminates the two. + let addr: Address = "seed.cluster.test:8300".parse().expect("parse FQDN:port"); + assert!(matches!(addr.host(), Host::Domain(_))); + + let start = std::time::Instant::now(); + let err = r + .resolve(&addr) + .await + .map(|_| ()) + .expect_err("a stalling resolver must surface the timeout, not fall through to OS DNS"); + let elapsed = start.elapsed(); + + // The synthetic TCP-DNS timeout is surfaced verbatim, proving the resolution did + // NOT escalate into the OS fallback (which does not produce a `TimedOut`). + assert!( + matches!(&err, DnsError::Io(io_err) if io_err.kind() == io::ErrorKind::TimedOut), + "expected a TimedOut error bounding the whole resolution, got: {err:?}" + ); + // And it returned promptly — bounded by the configured timeout, not the unbounded + // OS resolver. A generous ceiling (20x the 100ms budget) stays robust under CI + // load while still separating a bounded timeout from a fall-through hang. + assert!( + elapsed < Duration::from_secs(2), + "resolve must return within the configured timeout budget, took {elapsed:?}" + ); + + // Dropping the handle cancels the parked server task. + drop(handle); +} diff --git a/serf-compio/src/resolver/getifs/tests.rs b/serf-compio/src/resolver/getifs/tests.rs index 1a25d1e8..3274496d 100644 --- a/serf-compio/src/resolver/getifs/tests.rs +++ b/serf-compio/src/resolver/getifs/tests.rs @@ -36,6 +36,21 @@ async fn wildcard_respects_address_family() { assert!(v6.iter().all(|s| s.is_ipv6()), "[::] must yield only IPv6"); } +/// Each named constructor carries its own scope, and the `Default` impl is the +/// `private` scope — the conservative choice for a node advertising itself on an +/// unknown host. +#[test] +fn named_constructors_carry_their_scope() { + assert_eq!(LocalAddrResolver::private().scope, LocalAddrScope::Private); + assert_eq!(LocalAddrResolver::public().scope, LocalAddrScope::Public); + assert_eq!(LocalAddrResolver::all().scope, LocalAddrScope::All); + assert_eq!( + LocalAddrResolver::default().scope, + LocalAddrScope::Private, + "the default scope is private" + ); +} + #[test] fn local_advertise_attaches_port() { // Best-effort: a host may have no address in a given scope, but when one is diff --git a/serf-compio/src/resolver/mod.rs b/serf-compio/src/resolver/mod.rs index 95e51c36..c65ad71d 100644 --- a/serf-compio/src/resolver/mod.rs +++ b/serf-compio/src/resolver/mod.rs @@ -32,7 +32,7 @@ pub use socket_addr::SocketAddrResolver; #[cfg(feature = "dns")] #[cfg_attr(docsrs, doc(cfg(feature = "dns")))] -pub use dns::{DEFAULT_DNS_TIMEOUT, DnsResolver}; +pub use dns::{DEFAULT_DNS_TIMEOUT, DnsError, DnsResolver}; #[cfg(feature = "getifs")] #[cfg_attr(docsrs, doc(cfg(feature = "getifs")))] @@ -61,3 +61,6 @@ pub trait Resolver: 'static { /// Resolve `addr` to its concrete socket addresses. async fn resolve(&self, addr: &Self::Address) -> Result, Self::Error>; } + +#[cfg(test)] +mod tests; diff --git a/serf-compio/src/resolver/tests.rs b/serf-compio/src/resolver/tests.rs new file mode 100644 index 00000000..95ad17c6 --- /dev/null +++ b/serf-compio/src/resolver/tests.rs @@ -0,0 +1,151 @@ +//! Unit tests for the built-in resolvers: the advertise-candidate pickers (the +//! policy `Transport::new` applies when an unresolved advertise address resolves +//! to more than one candidate) and the address resolvers themselves. + +use std::net::SocketAddr; + +use super::{ + AdvertiseAddrResolver, AdvertiseResolutionError, FirstAddrResolver, Ipv4PreferringResolver, + Ipv6PreferringResolver, +}; + +fn addr(s: &str) -> SocketAddr { + s.parse().expect("socket addr") +} + +/// The default picker takes the candidate set's FIRST address, whatever its +/// family — the order the resolver returned is the policy. +#[test] +fn first_addr_resolver_takes_the_head_of_the_candidate_set() { + let picked = FirstAddrResolver + .pick(vec![addr("[::1]:7946"), addr("127.0.0.1:7946")]) + .expect("a non-empty candidate set resolves"); + assert_eq!(picked, addr("[::1]:7946"), "the head candidate is picked"); + + let picked = FirstAddrResolver + .pick(vec![addr("10.0.0.1:1"), addr("10.0.0.2:2")]) + .expect("a non-empty candidate set resolves"); + assert_eq!(picked, addr("10.0.0.1:1")); +} + +/// The IPv4-preferring picker skips past IPv6 candidates to the first IPv4 one, +/// and falls back to the head of the set when the resolution returned no IPv4 +/// address at all (an IPv6-only host still gets a contact). +#[test] +fn ipv4_preferring_resolver_prefers_v4_then_falls_back() { + let picked = Ipv4PreferringResolver + .pick(vec![ + addr("[::1]:7946"), + addr("[2001:db8::1]:7946"), + addr("127.0.0.1:7946"), + addr("10.0.0.1:7946"), + ]) + .expect("a non-empty candidate set resolves"); + assert_eq!( + picked, + addr("127.0.0.1:7946"), + "the FIRST IPv4 candidate wins over any IPv6 candidate ahead of it" + ); + + let picked = Ipv4PreferringResolver + .pick(vec![addr("[::1]:7946"), addr("[2001:db8::1]:7946")]) + .expect("an IPv6-only candidate set still resolves"); + assert_eq!( + picked, + addr("[::1]:7946"), + "with no IPv4 candidate the preference falls back to the head of the set" + ); +} + +/// The IPv6-preferring picker is the mirror image: the first IPv6 candidate +/// wins, and an IPv4-only set falls back to the head. +#[test] +fn ipv6_preferring_resolver_prefers_v6_then_falls_back() { + let picked = Ipv6PreferringResolver + .pick(vec![ + addr("127.0.0.1:7946"), + addr("10.0.0.1:7946"), + addr("[2001:db8::1]:7946"), + addr("[::1]:7946"), + ]) + .expect("a non-empty candidate set resolves"); + assert_eq!( + picked, + addr("[2001:db8::1]:7946"), + "the FIRST IPv6 candidate wins over any IPv4 candidate ahead of it" + ); + + let picked = Ipv6PreferringResolver + .pick(vec![addr("127.0.0.1:7946"), addr("10.0.0.1:7946")]) + .expect("an IPv4-only candidate set still resolves"); + assert_eq!( + picked, + addr("127.0.0.1:7946"), + "with no IPv6 candidate the preference falls back to the head of the set" + ); +} + +/// An empty candidate set is a resolution FAILURE on every picker, never a +/// silent default: a node with no resolvable advertise address must not boot. +#[test] +fn every_picker_rejects_an_empty_candidate_set() { + assert!(matches!( + FirstAddrResolver.pick(Vec::new()), + Err(AdvertiseResolutionError::Empty) + )); + assert!(matches!( + Ipv4PreferringResolver.pick(Vec::new()), + Err(AdvertiseResolutionError::Empty) + )); + assert!(matches!( + Ipv6PreferringResolver.pick(Vec::new()), + Err(AdvertiseResolutionError::Empty) + )); +} + +/// The identity resolver returns its already-concrete input verbatim as the sole +/// candidate — the pass-through the `SocketAddr`-addressed node type relies on. +#[compio::test] +async fn socket_addr_resolver_passes_its_input_through() { + use super::{Resolver, SocketAddrResolver}; + + let input = addr("192.0.2.7:7946"); + let out = SocketAddrResolver + .resolve(&input) + .await + .expect("the identity resolver cannot fail"); + assert_eq!( + out, + vec![input], + "the identity resolver yields exactly its input" + ); +} + +/// The OS resolver resolves a literal-IP host without a DNS round-trip, keeps +/// the port, and reports a lookup failure for an unresolvable name rather than +/// yielding an empty candidate set. +#[compio::test] +async fn os_resolver_resolves_a_literal_ip_host() { + use hostaddr::HostAddr; + + use super::{OsResolver, Resolver}; + + let host: HostAddr = "127.0.0.1:7946".parse().expect("literal-IP host addr"); + let out = OsResolver + .resolve(&host) + .await + .expect("a literal IP resolves without DNS"); + assert_eq!( + out, + vec![addr("127.0.0.1:7946")], + "the literal IP and its port pass through" + ); + + let bad: HostAddr = "no-such-host.invalid:7946" + .parse() + .expect("domain host addr"); + assert!( + OsResolver.resolve(&bad).await.is_err(), + "an unresolvable name is a resolution error, never an empty candidate set" + ); +} diff --git a/serf-compio/src/serf/mod.rs b/serf-compio/src/serf/mod.rs index 6eed72c7..5c2bf5ed 100644 --- a/serf-compio/src/serf/mod.rs +++ b/serf-compio/src/serf/mod.rs @@ -11,7 +11,7 @@ //! ergonomics — typed query/response futures, builder-style construction — are a //! follow-up; the protocol surface here is complete. -use core::time::Duration; +use core::{marker::PhantomData, time::Duration}; use std::{ cell::{Cell, RefCell}, net::SocketAddr, @@ -35,13 +35,22 @@ use smol_str::SmolStr; #[cfg(encryption)] use crate::command::{KeyCmd, ListKeysCmd}; +#[cfg(feature = "quic")] +use crate::quic::{QuicTransport, QuicTransportOptions}; +#[cfg(feature = "tcp")] +use crate::tcp::{TcpTransport, TcpTransportOptions}; +#[cfg(feature = "tls")] +use crate::tls::{TlsTransport, TlsTransportOptions}; use crate::{ command::{ Command, ForceLeaveCmd, JoinCmd, JoinKind, JoinReply, LeaveCmd, QueryCmd, RespondCmd, SetTagsCmd, ShutdownCmd, UserEventCmd, WaitForCompletionArgs, }, delegate::Delegate, - driver::options::RuntimeOptions, + driver::{ + options::RuntimeOptions, + shared::{ShutdownWaiter, shutdown_latch}, + }, drop_counter::DropReader, error::{InvalidOption, JoinFailed, Result, SerfError}, events::EventStream, @@ -68,6 +77,11 @@ struct Shared { /// at the bounded internal observation channel when the delegate dispatch /// loop falls behind. Monotonically increasing. observation_dropped: Rc>, + /// Shares the same `Rc` the QUIC driver pump increments. Counts gossip payloads + /// that rode the QUIC datagram plane (a `DatagramSendStatus::Queued`), as + /// opposed to the plain-UDP fallback. Stays zero on the stream transports and on + /// a QUIC endpoint in `UnreliableTransport::Udp` mode. + datagrams_sent: Rc>, /// Read-only view of the endpoint's cumulative user-coalescer drop count, over /// the SAME cell the endpoint's writer increments. The driver owns the /// endpoint, so a handle reads the shed count here with no publish step. @@ -76,6 +90,11 @@ struct Shared { coalesced_member_events_dropped: DropReader, snapshot: SnapshotCell, shutdown_flag: Rc>, + /// Handle half of the driver's teardown-completion latch. A `shutdown()` the + /// driver can no longer accept (it is already tearing down, so `shutdown_flag` + /// is set or the command queue is gone) parks here rather than returning into a + /// still-bound port. + shutdown_complete: ShutdownWaiter, local_id: I, advertise: SocketAddr, /// Per-call deadline applied to await-result joins, cached from @@ -90,20 +109,46 @@ struct Shared { /// A cheaply-clonable handle to a running serf node. /// -/// Construct one with [`Serf::new`]; clone it freely — every clone shares the -/// single driver task. Requires a stream or QUIC transport feature. +/// Construct one with a per-backend constructor — [`Serf::tcp`], [`Serf::tls`], +/// [`Serf::quic`] — or with the generic [`Serf::new`]; clone it freely, since +/// every clone shares the single driver task. Requires a stream or QUIC transport +/// feature. +/// +/// `Serf` carries the wire id type `I` and the unresolved address type `A` +/// the node's ADVERTISE address was configured in. `I` flows into the snapshot and +/// the events channel (both ``). There is no runtime parameter: +/// compio IS the runtime, so a compio handle is `!Send` and its driver is spawned +/// on the current thread's compio runtime. +/// +/// `A` is a brand, not a wire type: the advertise address is resolved to a +/// [`SocketAddr`] once at construction, and every membership address from then on +/// is a `SocketAddr`. It is therefore free to be a hostname type — e.g. the +/// [`hostaddr::HostAddr`](hostaddr::HostAddr) that +/// [`OsResolver`](crate::OsResolver) consumes. +/// +/// `A` does NOT constrain the seeds accepted by [`join`](Serf::join) / +/// [`join_many`](Serf::join_many) / [`dispatch_join`](Serf::dispatch_join): each is +/// generic over the [`Resolver`](crate::Resolver) it is handed and takes its seeds +/// in THAT resolver's address domain. Joining by hostname while advertising a +/// resolved `SocketAddr` (or the reverse) is deliberately allowed. #[cfg(any(feature = "tcp", feature = "quic"))] #[cfg_attr(docsrs, doc(cfg(any(feature = "tcp", feature = "quic"))))] -pub struct Serf { +pub struct Serf { shared: Rc>, + /// Brands the handle with the unresolved address type the node's advertise + /// address was configured in. No `A` value survives construction (it is resolved + /// to a `SocketAddr` before the driver starts), and the `join` family is generic + /// over the resolver it is handed, so this constrains no seed. + _a: PhantomData, } #[cfg(any(feature = "tcp", feature = "quic"))] -impl Clone for Serf { +impl Clone for Serf { #[inline] fn clone(&self) -> Self { Self { shared: self.shared.clone(), + _a: PhantomData, } } } @@ -131,7 +176,7 @@ where } #[cfg(any(feature = "tcp", feature = "quic"))] -impl Serf +impl Serf where I: Clone + PartialEq + 'static, { @@ -212,12 +257,17 @@ where flume::bounded::>(runtime_options.event_queue_cap()); let events_dropped = Rc::new(Cell::new(0u64)); let observation_dropped = Rc::new(Cell::new(0u64)); + let datagrams_sent = Rc::new(Cell::new(0u64)); // Mint the two shed counters as (writer, reader) pairs: the driver injects the // writers into the endpoint, the handle keeps the readers, both over the same // backing cell so no publish step exists. let (user_drop_writer, user_drop_reader) = crate::drop_counter::drop_channel(); let (member_drop_writer, member_drop_reader) = crate::drop_counter::drop_channel(); let shutdown_flag = Rc::new(Cell::new(false)); + // Mint the teardown-completion latch: the driver holds the sender until its + // bind sockets are released, and the handle keeps the waiter so a `shutdown()` + // the driver can no longer accept still returns only once the ports are free. + let (shutdown_complete, shutdown_waiter) = shutdown_latch(); let snapshot: SnapshotCell = Rc::new(RefCell::new(Rc::new(initial_snapshot( &local_id, advertise, )))); @@ -227,6 +277,7 @@ where // always reflect the driver's live count. let events_dropped_handle = events_dropped.clone(); let observation_dropped_handle = observation_dropped.clone(); + let datagrams_sent_handle = datagrams_sent.clone(); // Clone the serf options before they are moved into the driver so the // handle can compute `default_query_timeout` / `default_query_param` // without a driver round-trip. @@ -238,10 +289,12 @@ where events_tx, events_dropped, observation_dropped, + datagrams_sent, user_drop_writer, member_drop_writer, snapshot.clone(), shutdown_flag.clone(), + shutdown_complete, runtime_options, serf_options, reconnect_delegate, @@ -261,15 +314,18 @@ where events_rx, events_dropped: events_dropped_handle, observation_dropped: observation_dropped_handle, + datagrams_sent: datagrams_sent_handle, coalesced_user_events_dropped: user_drop_reader, coalesced_member_events_dropped: member_drop_reader, snapshot, shutdown_flag, + shutdown_complete: shutdown_waiter, local_id, advertise, join_deadline, serf_options: serf_options_handle, }), + _a: PhantomData, }) } @@ -489,6 +545,17 @@ where self.shared.coalesced_member_events_dropped.get() } + /// Cumulative number of gossip payloads sent over the QUIC datagram plane (a + /// datagram queued onto the peer's pooled, TLS-protected connection) rather than + /// the plain-UDP fallback. + /// + /// Always `0` on the stream transports and on a QUIC endpoint configured for + /// `UnreliableTransport::Udp`. Lifetime total, saturating. + #[inline] + pub fn datagrams_sent(&self) -> u64 { + self.shared.datagrams_sent.get() + } + /// Subscribe to the serf [`Event`] stream. Multiple subscribers round-robin /// (the channel is MPMC, not broadcast). #[inline] @@ -847,15 +914,328 @@ where await_reply(rx).await } - /// Gracefully shut the driver down, releasing the bound ports before this - /// resolves so an immediate rebind on the same address succeeds. + /// Stop the driver and release its bound ports, so an immediate rebind on the + /// same address succeeds with no grace period. Returns once those ports are + /// released; it aborts in-flight reliable-stream exchanges but does not block on + /// their connection cleanup. + /// + /// Idempotent: a second — or concurrent — call resolves `Ok(())` too, and it + /// resolves at the same instant the first one does. A caller whose command the + /// tearing-down driver can no longer accept parks on the driver's completion + /// latch instead of failing, so "returns once the ports are free" holds for + /// EVERY caller, not just the one the driver observed. pub async fn shutdown(&self) -> Result<()> { let (tx, rx) = oneshot::channel(); - self.send(Command::Shutdown(ShutdownCmd { reply: tx }))?; + if self + .send(Command::Shutdown(ShutdownCmd { reply: tx })) + .is_err() + { + // The driver is already tearing down (or has finished), so it will never + // read this command. It may still hold its bind sockets, so await teardown + // completion before reporting success rather than returning into a + // still-bound port. + self.shared.shutdown_complete.wait().await; + return Ok(()); + } await_reply(rx).await } } +// Ergonomic per-backend constructors: instantiate the transport for the caller so +// a node can be built without naming the generic `Serf::new::` machinery. +#[cfg(feature = "tcp")] +#[cfg_attr(docsrs, doc(cfg(feature = "tcp")))] +impl Serf +where + I: memberlist_proto::Id, + A: Clone + Send + 'static, +{ + /// Build a TCP-backed serf node and spawn its driver on the compio runtime. + /// + /// The ergonomic wrapper over [`Serf::new`] that instantiates the + /// [`TcpTransport`](crate::TcpTransport) for the caller: it binds a UDP gossip + /// socket and a TCP reliable listener on the advertise address (resolved once + /// via `resolver` / `advertise_resolver`), then spawns the stream driver. The + /// gossip RNG is drawn from OS entropy via [`gossip_rng`](crate::gossip_rng); + /// use [`tcp_with_rng`](Self::tcp_with_rng) to supply your own. + /// + /// Under an encryption backend, pass an + /// [`Rc`](crate::KeyringDelegate) + /// (`Rc::new(VoidKeyringDelegate)` for a node that manages no keys). + #[allow(clippy::too_many_arguments)] + pub async fn tcp( + options: TcpTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + { + Self::tcp_with_rng( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + crate::gossip_rng()?, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } + + /// Like [`tcp`](Self::tcp) but with a caller-supplied gossip RNG `G` — draw it + /// via [`gossip_rng`](crate::gossip_rng) for fork-safe OS entropy. + #[allow(clippy::too_many_arguments)] + pub async fn tcp_with_rng( + options: TcpTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + gossip_rng: G, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + G: rand::Rng + Send + Unpin + 'static, + { + Self::new::, RES, AR, D, G>( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + gossip_rng, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } +} + +// Ergonomic TLS constructor. TLS rides the same stream driver as plain TCP, +// differing only in the record layer. +#[cfg(feature = "tls")] +#[cfg_attr(docsrs, doc(cfg(feature = "tls")))] +impl Serf +where + I: memberlist_proto::Id, + A: Clone + Send + 'static, +{ + /// Build a TLS-backed serf node and spawn its driver on the compio runtime. + /// + /// The ergonomic wrapper over [`Serf::new`] that instantiates the + /// [`TlsTransport`](crate::TlsTransport) for the caller: it binds a UDP gossip + /// socket and a TCP reliable listener on the advertise address (resolved once via + /// `resolver` / `advertise_resolver`), then spawns the stream driver whose + /// reliable record layer drives rustls over the plain compio TCP stream. The + /// caller supplies the rustls server/client bundle and the per-peer SNI provider + /// through [`TlsTransportOptions`](crate::TlsTransportOptions). The gossip RNG is + /// drawn from OS entropy via [`gossip_rng`](crate::gossip_rng); use + /// [`tls_with_rng`](Self::tls_with_rng) to supply your own. + /// + /// Under an encryption backend, pass an + /// [`Rc`](crate::KeyringDelegate) + /// (`Rc::new(VoidKeyringDelegate)` for a node that manages no keys); the keyring + /// AEAD-protects the gossip datagrams (the reliable plane rides the TLS session). + #[allow(clippy::too_many_arguments)] + pub async fn tls( + options: TlsTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + { + Self::tls_with_rng( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + crate::gossip_rng()?, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } + + /// Like [`tls`](Self::tls) but with a caller-supplied gossip RNG `G` — draw it via + /// [`gossip_rng`](crate::gossip_rng) for fork-safe OS entropy. + #[allow(clippy::too_many_arguments)] + pub async fn tls_with_rng( + options: TlsTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + gossip_rng: G, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + G: rand::Rng + Send + Unpin + 'static, + { + Self::new::, RES, AR, D, G>( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + gossip_rng, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } +} + +// Ergonomic QUIC constructor. +#[cfg(feature = "quic")] +#[cfg_attr(docsrs, doc(cfg(feature = "quic")))] +impl Serf +where + I: memberlist_proto::Id, + A: Clone + Send + 'static, +{ + /// Build a QUIC-backed serf node and spawn its driver on the compio runtime. + /// + /// The ergonomic wrapper over [`Serf::new`] that instantiates the + /// [`QuicTransport`](crate::QuicTransport) for the caller: it binds a single UDP + /// socket on the advertise address (resolved once via `resolver` / + /// `advertise_resolver`) over which the coordinator multiplexes the reliable + /// push/pull streams and serf's datagram gossip, then spawns the QUIC driver. The + /// caller supplies the quinn-proto config bundle through + /// [`QuicTransportOptions::with_quic_config`](crate::QuicTransportOptions::with_quic_config). + /// The gossip RNG is drawn from OS entropy via [`gossip_rng`](crate::gossip_rng); + /// use [`quic_with_rng`](Self::quic_with_rng) to supply your own. + /// + /// Under an encryption backend, pass an + /// [`Rc`](crate::KeyringDelegate) + /// (`Rc::new(VoidKeyringDelegate)` for a node that manages no keys); the keyring + /// AEAD-protects the gossip datagrams (the reliable plane rides quinn's own TLS). + #[allow(clippy::too_many_arguments)] + pub async fn quic( + options: QuicTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + { + Self::quic_with_rng( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + crate::gossip_rng()?, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } + + /// Like [`quic`](Self::quic) but with a caller-supplied gossip RNG `G` — draw it + /// via [`gossip_rng`](crate::gossip_rng) for fork-safe OS entropy. + #[allow(clippy::too_many_arguments)] + pub async fn quic_with_rng( + options: QuicTransportOptions, + resolver: &RES, + advertise_resolver: &AR, + delegate: D, + runtime_options: RuntimeOptions, + serf_options: SerfOptions, + gossip_rng: G, + reconnect_delegate: Option>>, + merge_delegate: Option>>, + snapshot: Option, + #[cfg(encryption)] keyring: Rc, + ) -> Result + where + RES: Resolver
, + AR: AdvertiseAddrResolver, + D: Delegate + 'static, + G: rand::Rng + Send + Unpin + 'static, + { + Self::new::, RES, AR, D, G>( + options, + resolver, + advertise_resolver, + delegate, + runtime_options, + serf_options, + gossip_rng, + reconnect_delegate, + merge_delegate, + snapshot, + #[cfg(encryption)] + keyring, + ) + .await + } +} + /// Await a driver reply, mapping a dropped reply channel to /// [`SerfError::ReplyClosed`]. async fn await_reply(rx: oneshot::Receiver>) -> Result { diff --git a/serf-compio/src/serf/tests.rs b/serf-compio/src/serf/tests.rs index fe69287b..385f0aeb 100644 --- a/serf-compio/src/serf/tests.rs +++ b/serf-compio/src/serf/tests.rs @@ -18,7 +18,7 @@ use smol_str::SmolStr; use crate::{ Channel, FirstAddrResolver, Resolver, RuntimeOptions, Serf, SerfError, SocketAddrResolver, - TcpTransport, TcpTransportOptions, VoidDelegate, gossip_rng, + TcpTransportOptions, VoidDelegate, gossip_rng, }; /// A loopback address with a port nothing listens on — `connect()` returns @@ -46,7 +46,7 @@ impl Resolver for EmptyResolver { use crate::{EncryptionOptions, Keyring, KeyringDelegate, SecretKey, VoidKeyringDelegate}; /// Build and spawn a TCP serf node bound to an ephemeral loopback port. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { try_spawn_node_at(id, "127.0.0.1:0".parse().expect("loopback addr")) .await .expect("spawn serf node") @@ -55,18 +55,20 @@ async fn spawn_node(id: &str) -> Serf { /// Build a TCP serf node bound to a specific advertise address, returning the /// construction result so the same-address rebind regression can assert a freed /// port accepts an immediate rebind. -async fn try_spawn_node_at(id: &str, bind: SocketAddr) -> Result, SerfError> { +async fn try_spawn_node_at( + id: &str, + bind: SocketAddr, +) -> Result, SerfError> { let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -99,6 +101,65 @@ async fn tcp_shutdown_releases_bound_address_for_rebind() { second.shutdown().await.expect("second node shuts down"); } +/// A second `shutdown()` — issued once the driver has already exited and closed +/// its command queue — still resolves `Ok`, and only AFTER the bind address is +/// actually free: the late caller parks on the teardown-completion latch rather +/// than returning into a still-bound port. The freed address is proven rebindable +/// immediately after. +#[compio::test] +async fn tcp_second_shutdown_awaits_teardown_completion() { + let node = spawn_node("twice-a").await; + let addr = node.advertise_address(); + + node.shutdown().await.expect("the first shutdown resolves"); + node + .shutdown() + .await + .expect("a second shutdown after teardown still resolves Ok"); + + // Every other command path still fails fast once the queue is closed, rather + // than hanging: only `shutdown` is idempotent. + let err = node + .user_event("post", Bytes::from_static(b"x"), false) + .await + .expect_err("a shut-down node accepts no commands"); + assert!( + matches!(err, SerfError::Shutdown), + "a post-shutdown command reports Shutdown, got {err:?}" + ); + + let reborn = try_spawn_node_at("twice-b", addr) + .await + .expect("the freed address rebinds after the awaited teardown"); + assert_eq!(reborn.advertise_address(), addr); + reborn.shutdown().await.expect("twice-b shuts down"); +} + +/// Two `shutdown()` calls issued CONCURRENTLY both land in the command queue +/// before the driver observes either. The pump dispatches the first and drains the +/// second during teardown; both must resolve `Ok` — and both only once the bound +/// ports are released, which the immediate same-address rebind proves. A straggler +/// shutdown failed at the teardown drain would resolve `Err` while the listener +/// and gossip socket were still bound. +#[compio::test] +async fn tcp_concurrent_shutdowns_resolve_ok_after_the_ports_are_freed() { + let node = spawn_node("concurrent-shutdown-a").await; + let addr = node.advertise_address(); + + let (first, second) = future::join(node.shutdown(), node.shutdown()).await; + first.expect("the observed shutdown resolves Ok"); + second.expect("the shutdown drained during teardown also resolves Ok"); + + let reborn = try_spawn_node_at("concurrent-shutdown-b", addr) + .await + .expect("both shutdowns resolved only after the ports were freed"); + assert_eq!(reborn.advertise_address(), addr); + reborn + .shutdown() + .await + .expect("concurrent-shutdown-b shuts down"); +} + /// All `Serf` handles dropping under a continuous gossip flood must still shut the /// driver down. Under the flood the higher-priority recv arm starves the main /// select's command arm, so the command-channel disconnect is observable ONLY by @@ -177,22 +238,20 @@ async fn assert_tcp_new_rejects(runtime: RuntimeOptions) { let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new("bad-opt-node")) .with_advertise_addr(MaybeResolved::Resolved(bind)); - let res = - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( - opts, - &SocketAddrResolver, - &FirstAddrResolver, - VoidDelegate::::new(), - runtime, - SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), - None, - None, - None, - #[cfg(encryption)] - std::rc::Rc::new(VoidKeyringDelegate), - ) - .await; + let res = Serf::tcp( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + runtime, + SerfOptions::new(), + None, + None, + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await; match res { Err(SerfError::InvalidOption(_)) => {} Err(other) => panic!("expected InvalidOption, got {other:?}"), @@ -224,22 +283,20 @@ async fn tcp_new_rejects_over_ceiling_user_event_size() { .with_advertise_addr(MaybeResolved::Resolved(bind)); let serf = SerfOptions::new().with_max_user_event_size(SerfOptions::DEFAULT_USER_EVENT_SIZE_LIMIT + 1); - let res = - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( - opts, - &SocketAddrResolver, - &FirstAddrResolver, - VoidDelegate::::new(), - RuntimeOptions::new(), - serf, - gossip_rng().expect("seed gossip rng"), - None, - None, - None, - #[cfg(encryption)] - std::rc::Rc::new(VoidKeyringDelegate), - ) - .await; + let res = Serf::tcp( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + serf, + None, + None, + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await; match res { Err(SerfError::InvalidOption(_)) => {} Err(other) => panic!("expected InvalidOption, got {other:?}"), @@ -342,20 +399,22 @@ fn test_secret_key(fill: u8) -> SecretKey { /// Build and spawn a TCP serf node on an ephemeral loopback port with `encryption` /// installed as its gossip-and-reliable keyring policy. #[cfg(encryption)] -async fn spawn_encrypted_node(id: &str, encryption: EncryptionOptions) -> Serf { +async fn spawn_encrypted_node( + id: &str, + encryption: EncryptionOptions, +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_encryption(encryption); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -472,12 +531,15 @@ async fn mismatched_keyring_nodes_do_not_exchange_membership() { } /// Build a TCP serf node with a custom `RuntimeOptions`. -async fn spawn_node_with_runtime(id: &str, runtime_options: RuntimeOptions) -> Serf { +async fn spawn_node_with_runtime( + id: &str, + runtime_options: RuntimeOptions, +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp_with_rng( opts, &SocketAddrResolver, &FirstAddrResolver, @@ -565,19 +627,21 @@ async fn tcp_events_dropped_counter_observable_under_backpressure() { } /// Build a TCP serf node with a custom `SerfOptions` (runtime options at defaults). -async fn spawn_node_with_serf_options(id: &str, serf_options: SerfOptions) -> Serf { +async fn spawn_node_with_serf_options( + id: &str, + serf_options: SerfOptions, +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), serf_options, - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -1075,20 +1139,19 @@ async fn spawn_encrypted_node_with_keyring( id: &str, encryption: EncryptionOptions, keyring: std::rc::Rc, -) -> Serf { +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_encryption(encryption); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -1297,20 +1360,19 @@ async fn spawn_node_with_snapshot( id: &str, snapshot: crate::SnapshotOptions, rejoin_after_leave: bool, -) -> Serf { +) -> Serf { let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved( "127.0.0.1:0".parse().expect("loopback addr"), )); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new().with_rejoin_after_leave(rejoin_after_leave), - gossip_rng().expect("seed gossip rng"), None, None, Some(snapshot), @@ -1331,7 +1393,7 @@ fn snapshot_path(name: &str) -> std::path::PathBuf { } /// Poll both nodes until each reports the full two-member cluster. -async fn converge(a: &Serf, b: &Serf) { +async fn converge(a: &Serf, b: &Serf) { compio::time::timeout(Duration::from_secs(20), async { loop { if a.num_members() == 2 && b.num_members() == 2 { @@ -1443,26 +1505,24 @@ async fn merge_delegate_is_consulted_on_join() { .with_advertise_addr(MaybeResolved::Resolved( "127.0.0.1:0".parse().expect("loopback addr"), )); - let b = - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( - opts, - &SocketAddrResolver, - &FirstAddrResolver, - VoidDelegate::::new(), - RuntimeOptions::new(), - SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), - None, - Some(Box::new(RecordingMerge { - hits: hits.clone(), - saw_peer: saw_peer.clone(), - })), - None, - #[cfg(encryption)] - std::rc::Rc::new(VoidKeyringDelegate), - ) - .await - .expect("spawn merge-recording serf node"); + let b = Serf::tcp( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + SerfOptions::new(), + None, + Some(Box::new(RecordingMerge { + hits: hits.clone(), + saw_peer: saw_peer.clone(), + })), + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await + .expect("spawn merge-recording serf node"); let a = spawn_node("cmerge-a").await; let b_addr = b.advertise_address(); @@ -1591,3 +1651,252 @@ async fn file_backed_rotation_gates_the_response_on_persistence() { // Ignoring Err: best-effort test-file cleanup. let _ = std::fs::remove_file(&path); } + +/// Poll `cond` until it holds, failing the test on the fixture ceiling so a +/// convergence regression surfaces as a bounded timeout rather than a hang. +async fn await_condition(what: &str, mut cond: impl FnMut() -> bool) { + compio::time::timeout(Duration::from_secs(20), async { + loop { + if cond() { + break; + } + compio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap_or_else(|_| panic!("timed out waiting for {what}")); +} + +/// The handle derives the default query timeout from its live snapshot member +/// count without a driver round-trip, and `default_query_param` carries exactly +/// that timeout with the neutral filter / relay / ack posture. +#[compio::test] +async fn tcp_default_query_param_defaults() { + let a = spawn_node("dqp-node").await; + + let qt = a.default_query_timeout(); + assert!( + qt > Duration::ZERO, + "default_query_timeout must be positive" + ); + + let qp = a.default_query_param(); + assert_eq!(qp.timeout, qt, "default_query_param timeout matches"); + assert!(qp.filters.is_empty(), "no filters"); + assert!(!qp.request_ack, "no ack"); + assert_eq!(qp.relay_factor, 0, "no relay"); + + a.shutdown().await.expect("node shuts down"); +} + +/// A node whose advertise address domain is `HostAddr` — the type the +/// built-in name resolvers produce — rather than a wire `SocketAddr`. +type HostNode = Serf>; + +/// Build a node whose advertise address is the UNRESOLVED host `host`, resolved +/// through `resolver` at construction — so the node's address domain `A` is +/// `HostAddr`. The advertise candidate set is narrowed by +/// [`Ipv4PreferringResolver`], which still falls back to the head of the set on an +/// IPv6-only host. +async fn try_spawn_host_node( + id: &str, + host: &str, + resolver: &RES, +) -> Result +where + RES: Resolver
>, +{ + let advertise: hostaddr::HostAddr = host.parse().expect("host addr"); + let opts = TcpTransportOptions::>::new() + .with_local_id(SmolStr::new(id)) + .with_advertise_addr(MaybeResolved::Unresolved(advertise)); + Serf::tcp( + opts, + resolver, + &crate::Ipv4PreferringResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + SerfOptions::new(), + None, + None, + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await +} + +/// A node can advertise a HOSTNAME: built over the `HostAddr` address domain and an +/// [`OsResolver`](crate::OsResolver), it resolves `localhost:0` at construction, +/// binds the resolved loopback address, and publishes the concrete `SocketAddr` it +/// bound as its contact. A second such node then joins the first BY HOSTNAME through +/// the same resolver — proving the address brand `A` constrains only the advertise +/// domain, never the seeds `join` accepts. +#[compio::test] +async fn tcp_advertise_and_join_through_os_resolver() { + let resolver = crate::OsResolver; + + let a = try_spawn_host_node("os-host-a", "localhost:0", &resolver) + .await + .expect("a hostname advertise address resolves and binds"); + let a_addr = a.advertise_address(); + assert!( + a_addr.ip().is_loopback(), + "the node advertises the RESOLVED loopback IP, not the hostname it was given: {a_addr}" + ); + assert_ne!( + a_addr.port(), + 0, + "the ephemeral `:0` resolved to the concrete port the listener bound" + ); + + let b = try_spawn_host_node("os-host-b", "localhost:0", &resolver) + .await + .expect("a second hostname-addressed node binds"); + assert_ne!( + b.advertise_address(), + a_addr, + "the two nodes bind distinct ephemeral ports" + ); + + // The seed is a HOSTNAME (`localhost:`), never a `SocketAddr`: `join` + // resolves it through the same resolver and reports the seed it actually reached. + let seed: hostaddr::HostAddr = format!("localhost:{}", a_addr.port()) + .parse() + .expect("hostname seed"); + let reached = b + .join(&resolver, MaybeResolved::Unresolved(seed), false) + .await + .expect("the hostname seed resolves and its node is contacted"); + assert_eq!( + reached, a_addr, + "the contacted seed is the first node's bound advertise address" + ); + + await_condition("both hostname-addressed nodes to see 2 members", || { + a.num_members() == 2 && b.num_members() == 2 + }) + .await; + + a.shutdown().await.expect("node a shuts down"); + b.shutdown().await.expect("node b shuts down"); +} + +/// The advertise name the fixture nameserver answers for. Its `.invalid` TLD is +/// reserved never to resolve (RFC 6761 §6.4), so the OS fallback inside +/// `DnsResolver` CANNOT produce an address for it — a node that comes up on +/// loopback therefore did so on the nameserver's `A` record, not on a fallback. +#[cfg(feature = "dns")] +const ADVERTISE_NAME_FQDN: &str = "seed.cluster.invalid."; + +/// A loopback TCP nameserver answering exactly ONE query with a single `A` record +/// for `127.0.0.1`. Speaks just enough of TCP-DNS (RFC 1035 §4.2.2: a 2-byte +/// big-endian length prefix, then the message) for the resolver's TCP-first path; +/// the resolver harvests A/AAAA answers without validating the question, so a fixed +/// answer needs no query parsing. Returns the bound address to point a resolver at. +/// Nothing leaves loopback. +#[cfg(feature = "dns")] +async fn spawn_loopback_nameserver() -> (SocketAddr, compio::runtime::JoinHandle<()>) { + use std::net::Ipv4Addr; + + use compio::{ + buf::BufResult, + io::{AsyncReadExt, AsyncWriteExt}, + net::TcpListener, + }; + use hickory_proto::{ + op::{Message, OpCode}, + rr::{Name, RData, Record, rdata::A}, + }; + + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind loopback nameserver"); + let addr = listener.local_addr().expect("nameserver local_addr"); + + let handle = compio::runtime::spawn(async move { + let Ok((mut stream, _)) = listener.accept().await else { + return; + }; + // The 2-byte big-endian length prefix, then the query body it announces. + let len_buf = vec![0u8; 2]; + let BufResult(r, len_buf) = stream.read_exact(len_buf).await; + if r.is_err() { + return; + } + let qlen = u16::from_be_bytes([len_buf[0], len_buf[1]]) as usize; + let BufResult(r, _q) = stream.read_exact(vec![0u8; qlen]).await; + if r.is_err() { + return; + } + + let mut resp = Message::response(0, OpCode::Query); + resp.add_answer(Record::from_rdata( + Name::from_ascii(ADVERTISE_NAME_FQDN).expect("answer name"), + 60, + RData::A(A(Ipv4Addr::LOCALHOST)), + )); + let body = resp.to_vec().expect("encode DNS response"); + let mut framed = Vec::with_capacity(2 + body.len()); + framed.extend_from_slice(&(body.len() as u16).to_be_bytes()); + framed.extend_from_slice(&body); + // Ignoring Err: a best-effort single write from a one-shot fixture; a client + // that hung up surfaces as the resolving node's construction failure instead. + let BufResult(_w, _b) = stream.write_all(framed).await; + }); + + (addr, handle) +} + +/// A node can advertise a DNS name: built over the `HostAddr` address domain and a +/// [`DnsResolver`](crate::DnsResolver) pointed at the loopback fixture nameserver, +/// it resolves `seed.cluster.invalid:0` at construction and binds the `127.0.0.1` +/// its `A` record carried. Because that name is unresolvable by the OS fallback, +/// binding loopback proves the DNS answer drove the bind. A plain peer then joins +/// the DNS-derived contact and both converge, proving it is genuinely reachable. +#[cfg(feature = "dns")] +#[compio::test] +async fn tcp_advertise_resolved_through_dns_resolver() { + use std::net::{IpAddr, Ipv4Addr}; + + use crate::DnsResolver; + + let (nameserver, server) = spawn_loopback_nameserver().await; + let resolver = DnsResolver::from_servers(vec![nameserver]).with_timeout(Duration::from_secs(5)); + + let a = try_spawn_host_node("dns-host-a", "seed.cluster.invalid:0", &resolver) + .await + .expect("the fixture nameserver's answer resolves the advertise address"); + server.await.expect("the nameserver answered one query"); + + let a_addr = a.advertise_address(); + assert_eq!( + a_addr.ip(), + IpAddr::V4(Ipv4Addr::LOCALHOST), + "the node bound the IP its `A` record carried" + ); + assert_ne!( + a_addr.port(), + 0, + "the ephemeral `:0` resolved to the concrete port the listener bound" + ); + + // The DNS-derived contact is real: a peer dials it and the two converge. + let b = spawn_node("dns-peer-b").await; + let reached = b + .join(&SocketAddrResolver, MaybeResolved::Resolved(a_addr), false) + .await + .expect("the DNS-advertised node is reachable at the address it published"); + assert_eq!(reached, a_addr, "the peer contacted the advertised address"); + + await_condition( + "the DNS-advertised node and its peer to see 2 members", + || a.num_members() == 2 && b.num_members() == 2, + ) + .await; + + a.shutdown() + .await + .expect("the DNS-advertised node shuts down"); + b.shutdown().await.expect("the peer shuts down"); +} diff --git a/serf-compio/src/tcp/mod.rs b/serf-compio/src/tcp/mod.rs index 24cc4620..0f8a2094 100644 --- a/serf-compio/src/tcp/mod.rs +++ b/serf-compio/src/tcp/mod.rs @@ -8,13 +8,13 @@ #![cfg(feature = "tcp")] -use core::num::NonZeroU8; +use core::{num::NonZeroU8, time::Duration}; use std::{io::ErrorKind, net::SocketAddr}; use compio::net::{TcpListener, UdpSocket}; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, Endpoint, EndpointOptions, Id, MaybeResolved, RawRecords, + CheapClone, Endpoint, EndpointOptions, Id, MaybeResolved, RawRecords, streams::{LabelOptions, StreamEndpoint as Coordinator}, }; use rand::rngs::StdRng; @@ -42,6 +42,29 @@ pub struct TcpTransportOptions> { local_id: Option, advertise_addr: Option>, stream: StreamTransportOptions, + /// Override for the memberlist anti-entropy push/pull interval. `None` keeps the + /// coordinator default; `Some(Duration::ZERO)` disables periodic push/pull + /// entirely. See [`with_push_pull_interval`](Self::with_push_pull_interval). + push_pull_interval: Option, + /// SWIM probe interval override. `None` keeps the coordinator default. See + /// [`with_probe_interval`](Self::with_probe_interval). + probe_interval: Option, + /// SWIM direct-ping timeout override. `None` keeps the coordinator default. See + /// [`with_probe_timeout`](Self::with_probe_timeout). + probe_timeout: Option, + /// Gossip interval override. `None` keeps the coordinator default. See + /// [`with_gossip_interval`](Self::with_gossip_interval). + gossip_interval: Option, + /// SWIM suspicion multiplier override. `None` keeps the coordinator default. See + /// [`with_suspicion_mult`](Self::with_suspicion_mult). + suspicion_mult: Option, + /// Reclaim window for a same-name member returning at a NEW address: a dead + /// member older than this is revived in place of a conflict. See + /// [`with_dead_node_reclaim_time`](Self::with_dead_node_reclaim_time). + dead_node_reclaim_time: Option, + /// SWIM suspicion max-timeout multiplier override. `None` keeps the coordinator + /// default. See [`with_suspicion_max_timeout_mult`](Self::with_suspicion_max_timeout_mult). + suspicion_max_timeout_mult: Option, /// Gossip-and-reliable encryption policy. The default (no keyring) leaves /// both planes plaintext; attaching a keyring via /// [`with_encryption`](Self::with_encryption) makes the coordinator's @@ -61,6 +84,13 @@ impl TcpTransportOptions { local_id: None, advertise_addr: None, stream: StreamTransportOptions::new(), + push_pull_interval: None, + probe_interval: None, + probe_timeout: None, + gossip_interval: None, + suspicion_mult: None, + dead_node_reclaim_time: None, + suspicion_max_timeout_mult: None, #[cfg(encryption)] encryption: EncryptionOptions::new(), } @@ -90,6 +120,90 @@ impl TcpTransportOptions { self } + /// Builder: override the memberlist anti-entropy push/pull interval. + /// + /// `None` (the default) keeps the coordinator's built-in interval. A positive + /// duration re-tunes the periodic full-state sync; `Duration::ZERO` disables + /// periodic push/pull entirely — join-time and explicit exchanges still run, but + /// no background anti-entropy is scheduled. + #[must_use] + #[inline] + pub const fn with_push_pull_interval(mut self, interval: Duration) -> Self { + self.push_pull_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM probe interval — how often the + /// coordinator probes a random peer for liveness. + /// + /// `None` (the default) keeps the coordinator default (~1s). A shorter interval + /// speeds failure detection at the cost of more probe traffic; it also shortens + /// the suspicion timeout, which scales with the probe interval. + #[must_use] + #[inline] + pub const fn with_probe_interval(mut self, interval: Duration) -> Self { + self.probe_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM direct-ping timeout — how long the + /// coordinator waits for a probe ack before escalating to indirect probes. + /// + /// `None` (the default) keeps the coordinator default (~500ms). It must + /// comfortably exceed the real network round-trip, or a live peer whose ack is + /// merely slow is falsely suspected. + #[must_use] + #[inline] + pub const fn with_probe_timeout(mut self, timeout: Duration) -> Self { + self.probe_timeout = Some(timeout); + self + } + + /// Builder: override the memberlist gossip interval — how often the coordinator + /// flushes queued gossip to a random subset of peers. + /// + /// `None` (the default) keeps the coordinator default (~200ms). + #[must_use] + #[inline] + pub const fn with_gossip_interval(mut self, interval: Duration) -> Self { + self.gossip_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM suspicion multiplier — how long a + /// suspected peer is held in the Suspect state before being declared Failed. + /// + /// The minimum suspicion timeout is `suspicion_mult * log10(N+1) * probe_interval`. + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_mult(mut self, mult: u32) -> Self { + self.suspicion_mult = Some(mult); + self + } + + /// Builder: allow a dead member to be revived under the SAME id at a NEW + /// address once it has been dead longer than `window` — the reference + /// implementation's dead-node reclaim. Left unset (the default), a same-name + /// Alive from a different address is a name conflict, never a revival. + #[must_use] + #[inline] + pub const fn with_dead_node_reclaim_time(mut self, window: Duration) -> Self { + self.dead_node_reclaim_time = Some(window); + self + } + + /// Builder: override the memberlist SWIM suspicion max-timeout multiplier — the + /// upper bound on the suspicion timeout as a multiple of the minimum. + /// + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_max_timeout_mult(mut self, mult: u32) -> Self { + self.suspicion_max_timeout_mult = Some(mult); + self + } + /// Builder: gossip-and-reliable encryption policy. /// /// The default (no keyring) keeps both planes plaintext, so an unencrypted @@ -127,6 +241,49 @@ impl TcpTransportOptions { &self.stream } + /// The push/pull interval override, if set. + #[inline] + pub const fn push_pull_interval(&self) -> Option { + self.push_pull_interval + } + + /// The SWIM probe-interval override, if set. + #[inline] + pub const fn probe_interval(&self) -> Option { + self.probe_interval + } + + /// The SWIM probe-timeout override, if set. + #[inline] + pub const fn probe_timeout(&self) -> Option { + self.probe_timeout + } + + /// The gossip-interval override, if set. + #[inline] + pub const fn gossip_interval(&self) -> Option { + self.gossip_interval + } + + /// The SWIM suspicion-multiplier override, if set. + #[inline] + pub const fn suspicion_mult(&self) -> Option { + self.suspicion_mult + } + + /// The configured dead-node reclaim window, if overridden. + #[must_use] + #[inline] + pub const fn dead_node_reclaim_time(&self) -> Option { + self.dead_node_reclaim_time + } + + /// The SWIM suspicion max-timeout-multiplier override, if set. + #[inline] + pub const fn suspicion_max_timeout_mult(&self) -> Option { + self.suspicion_max_timeout_mult + } + /// Gossip-and-reliable encryption policy. #[cfg(encryption)] #[cfg_attr( @@ -160,6 +317,19 @@ pub struct TcpTransport> { gossip_socket: UdpSocket, tcp_listener: TcpListener, stream_options: StreamTransportOptions, + /// Push/pull interval override, applied to the coordinator's `EndpointOptions` + /// in [`Transport::run`]. `None` keeps the default; `Some(Duration::ZERO)` + /// disables periodic anti-entropy. + push_pull_interval: Option, + /// SWIM failure-detection overrides applied to the coordinator's + /// `EndpointOptions` in [`Transport::run`]. Each `None` keeps the coordinator + /// default. + probe_interval: Option, + probe_timeout: Option, + gossip_interval: Option, + suspicion_mult: Option, + dead_node_reclaim_time: Option, + suspicion_max_timeout_mult: Option, /// Independent OS-seeded seed for the serf core's RNG, drawn once per node in /// [`Transport::new`] and consumed when [`Transport::run`] builds the /// endpoint via `new_with_rng`. Distinct from the coordinator's gossip RNG so @@ -174,7 +344,7 @@ pub struct TcpTransport> { impl Transport for TcpTransport where I: Id + CheapClone + core::fmt::Debug + core::fmt::Display + Send + Sync + 'static, - A: Data + Clone + Send + 'static, + A: Clone + Send + 'static, { type Error = SerfError; type Id = I; @@ -302,6 +472,13 @@ where gossip_socket, tcp_listener, stream_options: options.stream, + push_pull_interval: options.push_pull_interval, + probe_interval: options.probe_interval, + probe_timeout: options.probe_timeout, + gossip_interval: options.gossip_interval, + suspicion_mult: options.suspicion_mult, + dead_node_reclaim_time: options.dead_node_reclaim_time, + suspicion_max_timeout_mult: options.suspicion_max_timeout_mult, serf_rng, #[cfg(encryption)] encryption: options.encryption, @@ -332,8 +509,40 @@ where // endpoint; build it here from `self`'s stored config. Serf ranks its user // broadcasts on three tiers (intent / event / query → ranks 0 / 1 / 2), so // the inner memberlist endpoint needs at least three broadcast tiers. - let inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) + let mut inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) .with_user_broadcast_tiers(NonZeroU8::new(3).expect("3 is nonzero")); + // A caller-supplied push/pull interval re-tunes (or, at `Duration::ZERO`, + // disables) the periodic anti-entropy full-state sync. Left unset, the + // coordinator keeps its own default. + if let Some(interval) = self.push_pull_interval { + inner_opts = inner_opts.with_push_pull_interval(interval); + } + // Caller-supplied SWIM failure-detection overrides: each left unset keeps the + // coordinator's own default. Lowering these speeds up failure detection (probe + // cadence, ack timeout, gossip cadence, and the suspicion timeout that scales + // with the probe interval). + if let Some(v) = self.probe_interval { + inner_opts = inner_opts.with_probe_interval(v); + } + if let Some(v) = self.probe_timeout { + inner_opts = inner_opts.with_probe_timeout(v); + } + if let Some(v) = self.gossip_interval { + inner_opts = inner_opts.with_gossip_interval(v); + } + if let Some(v) = self.suspicion_mult { + inner_opts = inner_opts.with_suspicion_mult(v); + } + if let Some(v) = self.dead_node_reclaim_time { + inner_opts = inner_opts.with_dead_node_reclaim_time(v); + } + if let Some(v) = self.suspicion_max_timeout_mult { + inner_opts = inner_opts.with_suspicion_max_timeout_mult(v); + } + // Snapshot the reliable push/pull exchange timeout from the SAME options the + // coordinator is built from, so the driver reconciles an await-result join's + // caller deadline against the exact deadline the coordinator will stamp. + let stream_timeout = inner_opts.stream_timeout(); let inner = Endpoint::new(inner_opts, gossip_rng); // Plain TCP has no SNI (`|_| None`) and a membership address that IS the // transport socket (`|addr| *addr`). No cluster label at this stage. @@ -372,6 +581,11 @@ where if let Some(md) = runtime.merge_delegate { endpoint.set_merge_delegate(md); } + // Test-only: install the delegate's inbound message-dropper on the machine. + #[cfg(feature = "test")] + if let Some(dropper) = runtime.delegate.message_dropper() { + endpoint.set_message_dropper(dropper); + } let snapshotter = match runtime.snapshot_file { Some((writer, records)) => { let replay = serf_proto::snapshot::ReplayResult::replay(records, rejoin_after_leave); @@ -393,10 +607,12 @@ where runtime.observation_dropped, runtime.snapshot, runtime.shutdown_flag, + runtime.shutdown_complete, runtime.driver_options, self.stream_options, runtime.delegate, None, + stream_timeout, snapshotter, #[cfg(encryption)] runtime.keyring, diff --git a/serf-compio/src/tcp/tests.rs b/serf-compio/src/tcp/tests.rs index d3547577..42e1da72 100644 --- a/serf-compio/src/tcp/tests.rs +++ b/serf-compio/src/tcp/tests.rs @@ -123,3 +123,135 @@ async fn freshly_constructed_nodes_have_independent_serf_rngs() { "two fresh nodes must hold independently OS-seeded serf RNGs, not a shared stream" ); } + +// ── SWIM / timing knobs ─────────────────────────────────────────────────────── + +/// Every SWIM knob starts UNSET, so a caller that sets none keeps the +/// coordinator's own defaults — `Transport::run` applies an override only when +/// it is `Some`. +#[test] +fn swim_knobs_start_unset() { + let opts = crate::TcpTransportOptions::::new(); + assert!(opts.push_pull_interval().is_none()); + assert!(opts.probe_interval().is_none()); + assert!(opts.probe_timeout().is_none()); + assert!(opts.gossip_interval().is_none()); + assert!(opts.suspicion_mult().is_none()); + assert!(opts.dead_node_reclaim_time().is_none()); + assert!(opts.suspicion_max_timeout_mult().is_none()); +} + +/// Every builder writes its OWN field: the accessors read back exactly what was +/// set, with distinct values per knob so a crossed assignment surfaces. +#[test] +fn swim_knob_builders_round_trip_each_knob() { + let opts = crate::TcpTransportOptions::::new() + .with_push_pull_interval(Duration::from_millis(1)) + .with_probe_interval(Duration::from_millis(2)) + .with_probe_timeout(Duration::from_millis(3)) + .with_gossip_interval(Duration::from_millis(4)) + .with_suspicion_mult(5) + .with_dead_node_reclaim_time(Duration::from_millis(6)) + .with_suspicion_max_timeout_mult(7); + + assert_eq!(opts.push_pull_interval(), Some(Duration::from_millis(1))); + assert_eq!(opts.probe_interval(), Some(Duration::from_millis(2))); + assert_eq!(opts.probe_timeout(), Some(Duration::from_millis(3))); + assert_eq!(opts.gossip_interval(), Some(Duration::from_millis(4))); + assert_eq!(opts.suspicion_mult(), Some(5)); + assert_eq!( + opts.dead_node_reclaim_time(), + Some(Duration::from_millis(6)) + ); + assert_eq!(opts.suspicion_max_timeout_mult(), Some(7)); +} + +/// A zero push/pull interval is a MEANINGFUL setting (it disables periodic +/// anti-entropy, isolating the gossip plane), so it must round-trip as +/// `Some(ZERO)` — never collapse back to the `None` that means "keep the +/// coordinator default". +#[test] +fn zero_push_pull_interval_is_set_not_unset() { + let opts = crate::TcpTransportOptions::::new() + .with_push_pull_interval(Duration::ZERO); + assert_eq!(opts.push_pull_interval(), Some(Duration::ZERO)); +} + +/// An unresolved advertise input round-trips through the options block in its +/// ORIGINAL form: resolution happens once at construction, not in the builder, so +/// the accessor must not silently pre-resolve a hostname. +#[test] +fn unresolved_advertise_addr_round_trips_unresolved() { + let host: hostaddr::HostAddr = "example.com:7946".parse().expect("host addr"); + let opts = TcpTransportOptions::>::new() + .with_advertise_addr(MaybeResolved::Unresolved(host.clone())); + match opts.advertise_addr() { + Some(MaybeResolved::Unresolved(h)) => assert_eq!(*h, host), + other => panic!("expected an unresolved advertise addr, got {other:?}"), + } +} + +/// The gossip-and-reliable keyring reaches the options block through the builder. +#[cfg(encryption)] +#[test] +fn encryption_policy_round_trips() { + use memberlist_proto::{EncryptionOptions, Keyring, SecretKey}; + + #[cfg(feature = "aes-gcm")] + let key = SecretKey::Aes256([0x21; 32]); + #[cfg(all(not(feature = "aes-gcm"), feature = "chacha20-poly1305"))] + let key = SecretKey::ChaCha20Poly1305([0x21; 32]); + + let opts = TcpTransportOptions::::new() + .with_encryption(EncryptionOptions::new().with_keyring(Keyring::new(key))); + let keyring = opts + .encryption() + .keyring() + .expect("the configured keyring reaches the options block"); + assert_eq!( + keyring.primary_ref(), + &key, + "the primary key is the one that was configured" + ); +} + +/// `Default` is the `new()` state — one source of truth, so a node built from +/// `Default` carries no accidental pre-set id, advertise address, or SWIM override. +#[test] +fn default_matches_new() { + let d = TcpTransportOptions::::default(); + assert!(d.local_id().is_none()); + assert!(d.advertise_addr().is_none()); + assert!(d.probe_interval().is_none()); +} + +/// A constructed transport reports the identity it was built with: the local id, +/// the advertise input in the ORIGINAL form the caller supplied (an unresolved +/// input stays unresolved — resolution happens for the bind, not for this +/// accessor), and the concrete bound contact the node will gossip. +#[compio::test] +async fn transport_reports_its_identity_and_bound_contact() { + let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); + let transport = TcpTransport::::new( + TcpTransportOptions::new() + .with_local_id(SmolStr::new("ident")) + .with_advertise_addr(MaybeResolved::Unresolved(bind)), + &SocketAddrResolver, + &FirstAddrResolver, + ) + .await + .expect("the transport binds an ephemeral loopback port"); + + assert_eq!(transport.local_id(), &SmolStr::new("ident")); + match transport.local_address() { + MaybeResolved::Unresolved(a) => assert_eq!(*a, bind), + other => panic!("the advertise INPUT form must be retained, got {other:?}"), + } + let advertise = *transport.advertise_address(); + assert!(advertise.ip().is_loopback()); + assert_ne!( + advertise.port(), + 0, + "the bound contact carries the OS-assigned port, not the ephemeral `:0`" + ); +} diff --git a/serf-compio/src/tls/mod.rs b/serf-compio/src/tls/mod.rs index 69627eca..60eef857 100644 --- a/serf-compio/src/tls/mod.rs +++ b/serf-compio/src/tls/mod.rs @@ -26,13 +26,13 @@ #![cfg(feature = "tls")] -use core::num::NonZeroU8; +use core::{num::NonZeroU8, time::Duration}; use std::{io::ErrorKind, net::SocketAddr}; use compio::net::{TcpListener, UdpSocket}; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, Endpoint, EndpointOptions, Id, MaybeResolved, TlsRecords, + CheapClone, Endpoint, EndpointOptions, Id, MaybeResolved, TlsRecords, streams::{LabelOptions, Labeled, StreamEndpoint as Coordinator}, }; use rand::rngs::StdRng; @@ -74,6 +74,29 @@ pub struct TlsTransportOptions> { stream: StreamTransportOptions, sni_provider: SniProvider, tls_options: Option, + /// Override for the memberlist anti-entropy push/pull interval. `None` keeps the + /// coordinator default; `Some(Duration::ZERO)` disables periodic push/pull + /// entirely. See [`with_push_pull_interval`](Self::with_push_pull_interval). + push_pull_interval: Option, + /// SWIM probe interval override. `None` keeps the coordinator default. See + /// [`with_probe_interval`](Self::with_probe_interval). + probe_interval: Option, + /// SWIM direct-ping timeout override. `None` keeps the coordinator default. See + /// [`with_probe_timeout`](Self::with_probe_timeout). + probe_timeout: Option, + /// Gossip interval override. `None` keeps the coordinator default. See + /// [`with_gossip_interval`](Self::with_gossip_interval). + gossip_interval: Option, + /// SWIM suspicion multiplier override. `None` keeps the coordinator default. See + /// [`with_suspicion_mult`](Self::with_suspicion_mult). + suspicion_mult: Option, + /// Reclaim window for a same-name member returning at a NEW address: a dead + /// member older than this is revived in place of a conflict. See + /// [`with_dead_node_reclaim_time`](Self::with_dead_node_reclaim_time). + dead_node_reclaim_time: Option, + /// SWIM suspicion max-timeout multiplier override. `None` keeps the coordinator + /// default. See [`with_suspicion_max_timeout_mult`](Self::with_suspicion_max_timeout_mult). + suspicion_max_timeout_mult: Option, /// Gossip encryption policy. The default (no keyring) leaves the gossip /// datagrams plaintext; attaching a keyring via /// [`with_encryption`](Self::with_encryption) makes the coordinator's @@ -98,6 +121,13 @@ impl TlsTransportOptions { stream: StreamTransportOptions::new(), sni_provider: Box::new(|_addr: &SocketAddr| Some("localhost".to_string())), tls_options: None, + push_pull_interval: None, + probe_interval: None, + probe_timeout: None, + gossip_interval: None, + suspicion_mult: None, + dead_node_reclaim_time: None, + suspicion_max_timeout_mult: None, #[cfg(encryption)] encryption: EncryptionOptions::new(), } @@ -148,6 +178,90 @@ impl TlsTransportOptions { self } + /// Builder: override the memberlist anti-entropy push/pull interval. + /// + /// `None` (the default) keeps the coordinator's built-in interval. A positive + /// duration re-tunes the periodic full-state sync; `Duration::ZERO` disables + /// periodic push/pull entirely — join-time and explicit exchanges still run, but + /// no background anti-entropy is scheduled. + #[must_use] + #[inline] + pub const fn with_push_pull_interval(mut self, interval: Duration) -> Self { + self.push_pull_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM probe interval — how often the + /// coordinator probes a random peer for liveness. + /// + /// `None` (the default) keeps the coordinator default (~1s). A shorter interval + /// speeds failure detection at the cost of more probe traffic; it also shortens + /// the suspicion timeout, which scales with the probe interval. + #[must_use] + #[inline] + pub const fn with_probe_interval(mut self, interval: Duration) -> Self { + self.probe_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM direct-ping timeout — how long the + /// coordinator waits for a probe ack before escalating to indirect probes. + /// + /// `None` (the default) keeps the coordinator default (~500ms). It must + /// comfortably exceed the real network round-trip, or a live peer whose ack is + /// merely slow is falsely suspected. + #[must_use] + #[inline] + pub const fn with_probe_timeout(mut self, timeout: Duration) -> Self { + self.probe_timeout = Some(timeout); + self + } + + /// Builder: override the memberlist gossip interval — how often the coordinator + /// flushes queued gossip to a random subset of peers. + /// + /// `None` (the default) keeps the coordinator default (~200ms). + #[must_use] + #[inline] + pub const fn with_gossip_interval(mut self, interval: Duration) -> Self { + self.gossip_interval = Some(interval); + self + } + + /// Builder: override the memberlist SWIM suspicion multiplier — how long a + /// suspected peer is held in the Suspect state before being declared Failed. + /// + /// The minimum suspicion timeout is `suspicion_mult * log10(N+1) * probe_interval`. + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_mult(mut self, mult: u32) -> Self { + self.suspicion_mult = Some(mult); + self + } + + /// Builder: allow a dead member to be revived under the SAME id at a NEW + /// address once it has been dead longer than `window` — the reference + /// implementation's dead-node reclaim. Left unset (the default), a same-name + /// Alive from a different address is a name conflict, never a revival. + #[must_use] + #[inline] + pub const fn with_dead_node_reclaim_time(mut self, window: Duration) -> Self { + self.dead_node_reclaim_time = Some(window); + self + } + + /// Builder: override the memberlist SWIM suspicion max-timeout multiplier — the + /// upper bound on the suspicion timeout as a multiple of the minimum. + /// + /// `None` (the default) keeps the coordinator default. + #[must_use] + #[inline] + pub const fn with_suspicion_max_timeout_mult(mut self, mult: u32) -> Self { + self.suspicion_max_timeout_mult = Some(mult); + self + } + /// Builder: gossip-encryption policy. /// /// The default (no keyring) keeps the gossip datagrams plaintext, so an @@ -198,6 +312,49 @@ impl TlsTransportOptions { self.tls_options.as_ref() } + /// The push/pull interval override, if set. + #[inline] + pub const fn push_pull_interval(&self) -> Option { + self.push_pull_interval + } + + /// The SWIM probe-interval override, if set. + #[inline] + pub const fn probe_interval(&self) -> Option { + self.probe_interval + } + + /// The SWIM probe-timeout override, if set. + #[inline] + pub const fn probe_timeout(&self) -> Option { + self.probe_timeout + } + + /// The gossip-interval override, if set. + #[inline] + pub const fn gossip_interval(&self) -> Option { + self.gossip_interval + } + + /// The SWIM suspicion-multiplier override, if set. + #[inline] + pub const fn suspicion_mult(&self) -> Option { + self.suspicion_mult + } + + /// The configured dead-node reclaim window, if overridden. + #[must_use] + #[inline] + pub const fn dead_node_reclaim_time(&self) -> Option { + self.dead_node_reclaim_time + } + + /// The SWIM suspicion max-timeout-multiplier override, if set. + #[inline] + pub const fn suspicion_max_timeout_mult(&self) -> Option { + self.suspicion_max_timeout_mult + } + /// Gossip-encryption policy. #[cfg(encryption)] #[cfg_attr( @@ -234,6 +391,19 @@ pub struct TlsTransport> { stream_options: StreamTransportOptions, sni_provider: SniProvider, tls_options: TlsOptions, + /// Push/pull interval override, applied to the coordinator's `EndpointOptions` + /// in [`Transport::run`]. `None` keeps the default; `Some(Duration::ZERO)` + /// disables periodic anti-entropy. + push_pull_interval: Option, + /// SWIM failure-detection overrides applied to the coordinator's + /// `EndpointOptions` in [`Transport::run`]. Each `None` keeps the coordinator + /// default. + probe_interval: Option, + probe_timeout: Option, + gossip_interval: Option, + suspicion_mult: Option, + dead_node_reclaim_time: Option, + suspicion_max_timeout_mult: Option, /// Independent OS-seeded seed for the serf core's RNG, drawn once per node in /// [`Transport::new`] and consumed when [`Transport::run`] builds the /// endpoint via `new_with_rng`. Distinct from the coordinator's gossip RNG so @@ -248,7 +418,7 @@ pub struct TlsTransport> { impl Transport for TlsTransport where I: Id + CheapClone + core::fmt::Debug + core::fmt::Display + Send + Sync + 'static, - A: Data + Clone + Send + 'static, + A: Clone + Send + 'static, { type Error = SerfError; type Id = I; @@ -384,6 +554,13 @@ where stream_options: options.stream, sni_provider: options.sni_provider, tls_options, + push_pull_interval: options.push_pull_interval, + probe_interval: options.probe_interval, + probe_timeout: options.probe_timeout, + gossip_interval: options.gossip_interval, + suspicion_mult: options.suspicion_mult, + dead_node_reclaim_time: options.dead_node_reclaim_time, + suspicion_max_timeout_mult: options.suspicion_max_timeout_mult, serf_rng, #[cfg(encryption)] encryption: options.encryption, @@ -414,8 +591,40 @@ where // endpoint; build it here from `self`'s stored config. Serf ranks its user // broadcasts on three tiers (intent / event / query → ranks 0 / 1 / 2), so // the inner memberlist endpoint needs at least three broadcast tiers. - let inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) + let mut inner_opts = EndpointOptions::new(self.local_id, self.advertise_socket) .with_user_broadcast_tiers(NonZeroU8::new(3).expect("3 is nonzero")); + // A caller-supplied push/pull interval re-tunes (or, at `Duration::ZERO`, + // disables) the periodic anti-entropy full-state sync. Left unset, the + // coordinator keeps its own default. + if let Some(interval) = self.push_pull_interval { + inner_opts = inner_opts.with_push_pull_interval(interval); + } + // Caller-supplied SWIM failure-detection overrides: each left unset keeps the + // coordinator's own default. Lowering these speeds up failure detection (probe + // cadence, ack timeout, gossip cadence, and the suspicion timeout that scales + // with the probe interval). + if let Some(v) = self.probe_interval { + inner_opts = inner_opts.with_probe_interval(v); + } + if let Some(v) = self.probe_timeout { + inner_opts = inner_opts.with_probe_timeout(v); + } + if let Some(v) = self.gossip_interval { + inner_opts = inner_opts.with_gossip_interval(v); + } + if let Some(v) = self.suspicion_mult { + inner_opts = inner_opts.with_suspicion_mult(v); + } + if let Some(v) = self.dead_node_reclaim_time { + inner_opts = inner_opts.with_dead_node_reclaim_time(v); + } + if let Some(v) = self.suspicion_max_timeout_mult { + inner_opts = inner_opts.with_suspicion_max_timeout_mult(v); + } + // Snapshot the reliable push/pull exchange timeout from the SAME options the + // coordinator is built from, so the driver reconciles an await-result join's + // caller deadline against the exact deadline the coordinator will stamp. + let stream_timeout = inner_opts.stream_timeout(); let inner = Endpoint::new(inner_opts, gossip_rng); // The TLS coordinator carries the per-peer SNI provider and the cert/key // bundle (ridden as the inner options on `LabelOptions`); the membership @@ -479,10 +688,12 @@ where runtime.observation_dropped, runtime.snapshot, runtime.shutdown_flag, + runtime.shutdown_complete, runtime.driver_options, self.stream_options, runtime.delegate, None, + stream_timeout, snapshotter, #[cfg(encryption)] runtime.keyring, diff --git a/serf-compio/src/tls/tests.rs b/serf-compio/src/tls/tests.rs index 5f76ff8c..2a43aa14 100644 --- a/serf-compio/src/tls/tests.rs +++ b/serf-compio/src/tls/tests.rs @@ -104,7 +104,7 @@ fn test_tls_options() -> TlsOptions { /// Build and spawn a TLS serf node bound to an ephemeral loopback port. The /// default SNI provider (`Some("localhost")`) matches the self-signed cert SAN. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { try_spawn_node_at(id, "127.0.0.1:0".parse().expect("loopback addr")) .await .expect("spawn serf node") @@ -114,19 +114,21 @@ async fn spawn_node(id: &str) -> Serf { /// construction result so the same-address rebind regression can assert a freed /// port accepts an immediate rebind. A fresh self-signed bundle is built per /// node, matching `spawn_node`. -async fn try_spawn_node_at(id: &str, bind: SocketAddr) -> Result, SerfError> { +async fn try_spawn_node_at( + id: &str, + bind: SocketAddr, +) -> Result, SerfError> { let opts = TlsTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_tls_options(test_tls_options()); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tls( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, None, None, @@ -170,22 +172,20 @@ async fn tls_new_rejects_zero_observation_channel() { .with_local_id(SmolStr::new("bad-opt-node")) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_tls_options(test_tls_options()); - let res = - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( - opts, - &SocketAddrResolver, - &FirstAddrResolver, - VoidDelegate::::new(), - RuntimeOptions::new().with_observation_channel(Channel::Bounded(0)), - SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), - None, - None, - None, - #[cfg(encryption)] - std::rc::Rc::new(VoidKeyringDelegate), - ) - .await; + let res = Serf::tls( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + RuntimeOptions::new().with_observation_channel(Channel::Bounded(0)), + SerfOptions::new(), + None, + None, + None, + #[cfg(encryption)] + std::rc::Rc::new(VoidKeyringDelegate), + ) + .await; match res { Err(SerfError::InvalidOption(_)) => {} Err(other) => panic!("expected InvalidOption, got {other:?}"), @@ -401,14 +401,17 @@ fn test_secret_key(fill: u8) -> SecretKey { /// Build and spawn a TLS serf node on an ephemeral loopback port with /// `encryption` installed as its gossip keyring policy. #[cfg(encryption)] -async fn spawn_encrypted_node(id: &str, encryption: EncryptionOptions) -> Serf { +async fn spawn_encrypted_node( + id: &str, + encryption: EncryptionOptions, +) -> Serf { let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); let opts = TlsTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(bind)) .with_tls_options(test_tls_options()) .with_encryption(encryption); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tls_with_rng( opts, &SocketAddrResolver, &FirstAddrResolver, @@ -470,3 +473,173 @@ async fn two_node_tls_join_observes_membership_encrypted() { a.shutdown().await.expect("node A shuts down"); b.shutdown().await.expect("node B shuts down"); } + +// ── SWIM / timing knobs ─────────────────────────────────────────────────────── + +/// Every SWIM knob starts UNSET, so a caller that sets none keeps the +/// coordinator's own defaults — `Transport::run` applies an override only when +/// it is `Some`. +#[test] +fn swim_knobs_start_unset() { + let opts = crate::TlsTransportOptions::::new(); + assert!(opts.push_pull_interval().is_none()); + assert!(opts.probe_interval().is_none()); + assert!(opts.probe_timeout().is_none()); + assert!(opts.gossip_interval().is_none()); + assert!(opts.suspicion_mult().is_none()); + assert!(opts.dead_node_reclaim_time().is_none()); + assert!(opts.suspicion_max_timeout_mult().is_none()); +} + +/// Every builder writes its OWN field: the accessors read back exactly what was +/// set, with distinct values per knob so a crossed assignment surfaces. +#[test] +fn swim_knob_builders_round_trip_each_knob() { + let opts = crate::TlsTransportOptions::::new() + .with_push_pull_interval(Duration::from_millis(1)) + .with_probe_interval(Duration::from_millis(2)) + .with_probe_timeout(Duration::from_millis(3)) + .with_gossip_interval(Duration::from_millis(4)) + .with_suspicion_mult(5) + .with_dead_node_reclaim_time(Duration::from_millis(6)) + .with_suspicion_max_timeout_mult(7); + + assert_eq!(opts.push_pull_interval(), Some(Duration::from_millis(1))); + assert_eq!(opts.probe_interval(), Some(Duration::from_millis(2))); + assert_eq!(opts.probe_timeout(), Some(Duration::from_millis(3))); + assert_eq!(opts.gossip_interval(), Some(Duration::from_millis(4))); + assert_eq!(opts.suspicion_mult(), Some(5)); + assert_eq!( + opts.dead_node_reclaim_time(), + Some(Duration::from_millis(6)) + ); + assert_eq!(opts.suspicion_max_timeout_mult(), Some(7)); +} + +/// A zero push/pull interval is a MEANINGFUL setting (it disables periodic +/// anti-entropy, isolating the gossip plane), so it must round-trip as +/// `Some(ZERO)` — never collapse back to the `None` that means "keep the +/// coordinator default". +#[test] +fn zero_push_pull_interval_is_set_not_unset() { + let opts = crate::TlsTransportOptions::::new() + .with_push_pull_interval(Duration::ZERO); + assert_eq!(opts.push_pull_interval(), Some(Duration::ZERO)); +} + +/// The SNI provider is consulted PER PEER: the dial-time server name is derived +/// from the peer's address, and a provider that refuses a peer (returns `None`) +/// aborts that dial before the handshake rather than falling back to a default +/// name. +#[test] +fn sni_provider_is_consulted_per_peer() { + let opts = TlsTransportOptions::::new().with_sni_provider(Box::new(|a| { + (a.port() != 9).then(|| format!("peer-{}.example", a.port())) + })); + let sni = opts.sni_provider(); + assert_eq!( + sni(&"127.0.0.1:1".parse().unwrap()), + Some("peer-1.example".to_string()) + ); + assert_eq!( + sni(&"127.0.0.1:2".parse().unwrap()), + Some("peer-2.example".to_string()) + ); + assert_eq!( + sni(&"127.0.0.1:9".parse().unwrap()), + None, + "a provider may refuse a peer, which aborts the dial before the handshake" + ); +} + +/// The gossip keyring reaches the options block through the builder. On TLS it +/// seals only the gossip datagrams — the reliable plane rides the TLS session. +#[cfg(encryption)] +#[test] +fn encryption_policy_round_trips() { + let key = test_secret_key(0x31); + let opts = TlsTransportOptions::::new() + .with_encryption(EncryptionOptions::new().with_keyring(Keyring::new(key))); + let keyring = opts + .encryption() + .keyring() + .expect("the configured keyring reaches the options block"); + assert_eq!( + keyring.primary_ref(), + &key, + "the primary key is the one that was configured" + ); +} + +/// The reliable listener and the gossip socket share one port. A port whose UDP +/// half is already taken must fail construction — a node cannot come up without +/// its gossip plane — and releasing the squatter must make the very same +/// construction succeed, proving the squatter (not the address) was the failure. +#[compio::test] +async fn taken_gossip_port_fails_construction() { + let squatter = std::net::UdpSocket::bind("127.0.0.1:0").expect("squat a UDP port"); + let taken = squatter.local_addr().expect("the squatted address"); + + let err = TlsTransport::::new( + TlsTransportOptions::new() + .with_local_id(SmolStr::new("squatted")) + .with_advertise_addr(MaybeResolved::Resolved(taken)) + .with_tls_options(test_tls_options()), + &SocketAddrResolver, + &FirstAddrResolver, + ) + .await + .err() + .expect("a node cannot come up without its gossip plane"); + assert!( + matches!(err, SerfError::Io(_)), + "a taken gossip port is an I/O failure, got {err:?}" + ); + + drop(squatter); + let transport = TlsTransport::::new( + TlsTransportOptions::new() + .with_local_id(SmolStr::new("unsquatted")) + .with_advertise_addr(MaybeResolved::Resolved(taken)) + .with_tls_options(test_tls_options()), + &SocketAddrResolver, + &FirstAddrResolver, + ) + .await + .expect("the released gossip port lets the transport bind"); + assert_eq!(*transport.advertise_address(), taken); +} + +/// An advertise address that resolves to NO candidate fails construction rather +/// than booting a node with no reachable contact. +#[compio::test] +async fn advertise_resolution_failure_fails_construction() { + /// Resolves nothing — a bootstrap outage the advertise picker must refuse. + struct EmptyResolver; + + impl crate::Resolver for EmptyResolver { + type Address = SocketAddr; + type Error = std::io::Error; + + async fn resolve(&self, _addr: &SocketAddr) -> std::io::Result> { + Ok(Vec::new()) + } + } + + let bind: SocketAddr = "127.0.0.1:0".parse().expect("loopback addr"); + let err = TlsTransport::::new( + TlsTransportOptions::new() + .with_local_id(SmolStr::new("unresolvable")) + .with_advertise_addr(MaybeResolved::Unresolved(bind)) + .with_tls_options(test_tls_options()), + &EmptyResolver, + &FirstAddrResolver, + ) + .await + .err() + .expect("an advertise address that resolves to nothing cannot boot a node"); + assert!( + matches!(err, SerfError::Resolve(_)), + "an empty candidate set is a resolution failure, got {err:?}" + ); +} diff --git a/serf-compio/src/transport/runtime.rs b/serf-compio/src/transport/runtime.rs index a1a39e52..0deb8099 100644 --- a/serf-compio/src/transport/runtime.rs +++ b/serf-compio/src/transport/runtime.rs @@ -13,8 +13,12 @@ use flume::{Receiver, Sender}; use serf_proto::{event::Event, options::Options as SerfOptions}; use crate::{ - command::Command, delegate::Delegate, driver::options::RuntimeOptions, - drop_counter::CompioDropCounter, snapshot::SnapshotCell, transport::Transport, + command::Command, + delegate::Delegate, + driver::{options::RuntimeOptions, shared::ShutdownComplete}, + drop_counter::CompioDropCounter, + snapshot::SnapshotCell, + transport::Transport, }; #[cfg(encryption)] @@ -49,6 +53,10 @@ where /// Counter for events dropped at the delegate observation channel when the /// delegate fell behind — may include unrecoverable app-data. pub(crate) observation_dropped: Rc>, + /// Counter for gossip payloads accepted onto the QUIC datagram plane (as + /// opposed to the plain-UDP fallback). Only the QUIC driver increments it; the + /// stream backends leave it at zero. + pub(crate) datagrams_sent: Rc>, /// The write half of the user-coalescer shed counter, injected into the /// endpoint by `T::run` so its increments land in the cell the handle reads. pub(crate) user_drop: CompioDropCounter, @@ -56,6 +64,11 @@ where pub(crate) member_drop: CompioDropCounter, pub(crate) snapshot: SnapshotCell, pub(crate) shutdown_flag: Rc>, + /// The driver half of the teardown-completion latch, dropped by the pump once + /// its bind sockets are released. A `shutdown()` the pump could no longer + /// accept parks on the matching waiter, so it returns only after the ports are + /// free. + pub(crate) shutdown_complete: ShutdownComplete, pub(crate) driver_options: RuntimeOptions, pub(crate) serf_options: SerfOptions, /// Optional per-member reconnect-timeout override (Go serf `ReconnectDelegate`), @@ -91,10 +104,12 @@ where events_tx: Sender>, events_dropped: Rc>, observation_dropped: Rc>, + datagrams_sent: Rc>, user_drop: CompioDropCounter, member_drop: CompioDropCounter, snapshot: SnapshotCell, shutdown_flag: Rc>, + shutdown_complete: ShutdownComplete, driver_options: RuntimeOptions, serf_options: SerfOptions, reconnect_delegate: Option>>, @@ -108,10 +123,12 @@ where events_tx, events_dropped, observation_dropped, + datagrams_sent, user_drop, member_drop, snapshot, shutdown_flag, + shutdown_complete, driver_options, serf_options, reconnect_delegate, diff --git a/serf-compio/src/transport/tests.rs b/serf-compio/src/transport/tests.rs index 50a39d9e..bca53afc 100644 --- a/serf-compio/src/transport/tests.rs +++ b/serf-compio/src/transport/tests.rs @@ -140,3 +140,39 @@ fn a_scoped_or_flow_labelled_ipv6_is_refused() { )); validate_advertise_addr(&plain).expect("an unscoped IPv6 unicast contact is accepted"); } + +/// A seed keyring whose keys collide across ciphers — the same raw bytes under +/// two cipher variants — is refused at construction: the coordinator's rotation +/// ops match on bytes alone, so such a ring would let a later `use`/`remove` +/// promote or drop the wrong cipher's key. A same-cipher multi-key ring, and a +/// cross-cipher ring with DISTINCT bytes, both stay admissible. +#[cfg(all(feature = "aes-gcm", feature = "chacha20-poly1305"))] +#[test] +fn cross_cipher_twin_keyring_is_rejected_at_construction() { + use memberlist_proto::{EncryptionOptions, Keyring, SecretKey}; + + use super::reject_cross_cipher_keyring; + + let aes = |b: u8| SecretKey::Aes256([b; 32]); + let chacha = |b: u8| SecretKey::ChaCha20Poly1305([b; 32]); + + let mut twinned = Keyring::new(aes(1)); + twinned.insert_secondary(chacha(1)); + let err = reject_cross_cipher_keyring(&EncryptionOptions::new().with_keyring(twinned)) + .expect_err("a cross-cipher byte twin makes every later key op ambiguous"); + assert!( + matches!(err, crate::SerfError::Io(ref e) if e.kind() == std::io::ErrorKind::InvalidInput), + "the twin refusal is an InvalidInput, got {err:?}" + ); + + let clean = Keyring::with_secondaries(aes(1), [aes(2), chacha(3)]); + assert!( + reject_cross_cipher_keyring(&EncryptionOptions::new().with_keyring(clean)).is_ok(), + "distinct key bytes across ciphers are unambiguous and stay admissible" + ); + + assert!( + reject_cross_cipher_keyring(&EncryptionOptions::new()).is_ok(), + "a node with no keyring configured has nothing to refuse" + ); +} diff --git a/serf-compio/tests/cluster/mod.rs b/serf-compio/tests/cluster/mod.rs index 6bb1287d..c5132ca4 100644 --- a/serf-compio/tests/cluster/mod.rs +++ b/serf-compio/tests/cluster/mod.rs @@ -11,11 +11,10 @@ //! compio is thread-per-core and `!Send`, so the fixture is `Rc`/`RefCell`-based //! and every node, collector, and assertion runs on the one runtime thread. //! -//! Unlike the reactor's transport options, serf-compio's `TcpTransportOptions` -//! exposes no memberlist SWIM knobs, so the nodes run the coordinator's default -//! probe / gossip / suspicion timing. [`ClusterTiming`] therefore tunes only the -//! serf-level reaper, and the poll ceiling is sized for a default-timing failure -//! detection. +//! The nodes run fast SWIM failure-detection timing (the transport probe / gossip +//! / suspicion overrides), so an abruptly-killed peer is detected as Failed in +//! well under a second. [`ClusterTiming`] carries both those memberlist knobs and +//! the serf-level reaper windows. use core::time::Duration; use std::{cell::RefCell, net::SocketAddr, rc::Rc}; @@ -23,8 +22,7 @@ use std::{cell::RefCell, net::SocketAddr, rc::Rc}; use futures_util::StreamExt; use memberlist_proto::MaybeResolved; use serf_compio::{ - FirstAddrResolver, RuntimeOptions, Serf, SocketAddrResolver, TcpTransport, TcpTransportOptions, - VoidDelegate, gossip_rng, + FirstAddrResolver, RuntimeOptions, Serf, SocketAddrResolver, TcpTransportOptions, VoidDelegate, }; use serf_proto::{ event::{Event, MemberEventKind}, @@ -33,15 +31,13 @@ use serf_proto::{ }; use smol_str::SmolStr; -/// A compio TCP node handle. -pub type Node = Serf; +/// A compio TCP node handle. The advertise address is configured as a resolved +/// `SocketAddr`, so that is the handle's address brand. +pub type Node = Serf; /// Wall-clock ceiling for every fixture poll loop, so a convergence or detection -/// regression surfaces as a bounded timeout rather than a hang. Sized for the -/// coordinator's DEFAULT suspicion timing (a 1 s probe interval and a 4x -/// suspicion multiplier put an abrupt kill's Failed transition several seconds -/// out), which serf-compio's transport options cannot shorten. -const POLL_TIMEOUT: Duration = Duration::from_secs(45); +/// regression surfaces as a bounded timeout rather than a hang. +const POLL_TIMEOUT: Duration = Duration::from_secs(20); /// Poll granularity for the fixture's await loops. const POLL_STEP: Duration = Duration::from_millis(20); @@ -50,17 +46,23 @@ pub fn loopback_ephemeral() -> SocketAddr { "127.0.0.1:0".parse().expect("loopback addr") } -/// Serf-level reaper timing shared by every node in a fixture cluster. +/// Failure-detection and reap timing shared by every node in a fixture cluster. /// -/// [`fast`](Self::fast) reaps a failed member almost immediately after the -/// coordinator declares it Failed (short reconnect timeout) and drops a -/// gracefully-left member almost immediately after its Leave (short tombstone -/// timeout). A test that wants to OBSERVE a member sitting in a Failed or Left -/// state raises the matching window with +/// The probe / gossip / suspicion knobs tune the memberlist SWIM layer (carried on +/// the transport options); the reap / reconnect knobs tune the serf reaper (carried +/// on the serf `Options`). [`fast`](Self::fast) yields CI-speed values that detect +/// an abrupt kill in sub-second time and reap the failed member shortly after +/// (short reconnect timeout), and drop a gracefully-left member shortly after its +/// Leave (short tombstone timeout). A test that wants to OBSERVE a member sitting +/// in a Failed or Left state raises the matching window with /// [`with_reconnect_timeout`](Self::with_reconnect_timeout) / /// [`with_tombstone_timeout`](Self::with_tombstone_timeout). #[derive(Clone)] pub struct ClusterTiming { + probe_interval: Duration, + probe_timeout: Duration, + gossip_interval: Duration, + suspicion_mult: u32, reap_interval: Duration, reconnect_interval: Duration, reconnect_timeout: Duration, @@ -69,12 +71,21 @@ pub struct ClusterTiming { } impl ClusterTiming { - /// CI-speed serf reaper timing: the reaper ticks every 100 ms and holds a - /// failed or left member for ~nothing, so an abrupt kill converges to a - /// reaped-out cluster as soon as the coordinator's default suspicion timing - /// declares the peer Failed. + /// CI-speed timing: sub-second SWIM failure detection on loopback with the + /// failed member reaped shortly after. + /// + /// The probe timeout sits BELOW the probe interval so an unanswered probe still + /// has an indirect/fallback window inside its own cycle, and the suspicion + /// multiplier keeps the small-cluster suspicion floor at ~300 ms — a live peer + /// survives a couple hundred milliseconds of executor starvation on an + /// oversubscribed CI runner without being falsely declared Failed, while + /// detection of a real kill stays comfortably sub-second. pub fn fast() -> Self { Self { + probe_interval: Duration::from_millis(100), + probe_timeout: Duration::from_millis(50), + gossip_interval: Duration::from_millis(20), + suspicion_mult: 3, reap_interval: Duration::from_millis(100), reconnect_interval: Duration::from_millis(100), reconnect_timeout: Duration::from_millis(1), @@ -112,6 +123,18 @@ impl ClusterTiming { .with_tombstone_timeout(self.tombstone_timeout) .with_leave_propagate_delay(self.leave_propagate_delay) } + + /// Apply the memberlist SWIM knobs to a fixture node's transport options. + pub fn apply( + &self, + opts: TcpTransportOptions, + ) -> TcpTransportOptions { + opts + .with_probe_interval(self.probe_interval) + .with_probe_timeout(self.probe_timeout) + .with_gossip_interval(self.gossip_interval) + .with_suspicion_mult(self.suspicion_mult) + } } /// One observed member event: its kind and the member ids it names. @@ -143,9 +166,7 @@ impl Cluster { pub async fn spawn(ids: &[&str], timing: ClusterTiming) -> Self { let mut slots = Vec::with_capacity(ids.len()); for id in ids { - let serf = build_node(id, timing.serf_opts()) - .await - .expect("spawn serf tcp node"); + let serf = build_node(id, &timing).await.expect("spawn serf tcp node"); let log: EventLog = Rc::new(RefCell::new(Vec::new())); // Attach the collector before the handle moves into the slot, so no member // event can slip past between construction and the first join. @@ -305,19 +326,21 @@ impl Cluster { } } -/// Spawn a fixture node on an ephemeral loopback port with `serf_opts`. -async fn build_node(id: &str, serf_opts: SerfOptions) -> serf_compio::Result { - let opts = TcpTransportOptions::::new() - .with_local_id(SmolStr::new(id)) - .with_advertise_addr(MaybeResolved::Resolved(loopback_ephemeral())); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( +/// Spawn a fixture node on an ephemeral loopback port with `timing`'s memberlist +/// SWIM knobs and serf reaper windows. +async fn build_node(id: &str, timing: &ClusterTiming) -> serf_compio::Result { + let opts = timing.apply( + TcpTransportOptions::::new() + .with_local_id(SmolStr::new(id)) + .with_advertise_addr(MaybeResolved::Resolved(loopback_ephemeral())), + ); + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), - serf_opts, - gossip_rng().expect("seed gossip rng"), + timing.serf_opts(), None, None, None, diff --git a/serf-compio/tests/quic.rs b/serf-compio/tests/quic.rs index 436b639d..c01c791a 100644 --- a/serf-compio/tests/quic.rs +++ b/serf-compio/tests/quic.rs @@ -28,7 +28,6 @@ use serf_compio::{ Channel, Delegate, FirstAddrResolver, Ipv4PreferringResolver, MemberDelegate, MergeDelegate, QueryDelegate, QuicOptions, QuicTransport, QuicTransportOptions, Resolver, RuntimeOptions, Serf, SerfError, SnapshotOptions, SocketAddrResolver, Transport, UserEventDelegate, VoidDelegate, - gossip_rng, }; use serf_proto::{ Tags, UserEventMessage, @@ -133,10 +132,10 @@ fn test_client() -> quinn_proto::ClientConfig { quinn_proto::ClientConfig::new(Arc::new(qcc)) } -/// A QUIC bundle with an idle timeout well past a localhost handshake and -/// datagram-mode unreliable transport. A fresh bundle is built per node so each -/// owns its own cert and quinn endpoint config. -fn test_quic_options() -> QuicOptions { +/// A QUIC bundle with an idle timeout well past a localhost handshake, carrying +/// `unreliable` as the gossip plane's wire. A fresh bundle is built per node so +/// each owns its own cert and quinn endpoint config. +fn test_quic_options_with(unreliable: UnreliableTransport) -> QuicOptions { let mut transport = quinn_proto::TransportConfig::default(); transport.max_idle_timeout(Some( quinn_proto::IdleTimeout::try_from(Duration::from_secs(20)).expect("a valid idle timeout"), @@ -147,10 +146,15 @@ fn test_quic_options() -> QuicOptions { test_client(), transport, "localhost", - UnreliableTransport::Datagram, + unreliable, ) } +/// The default bundle: datagram-mode gossip (`UnreliableTransport::Datagram`). +fn test_quic_options() -> QuicOptions { + test_quic_options_with(UnreliableTransport::Datagram) +} + // ── fixtures ────────────────────────────────────────────────────────────────── /// A resolver that answers with a dual-stack candidate set (IPv6 first, then @@ -180,6 +184,27 @@ struct Observed { user_events: RefCell>, } +impl Observed { + /// Poll `recorded` until it contains `id`, bounded by `window`; returns + /// whether it landed in time. Each observation hook is delivered on a path + /// separate from the membership snapshot, so a hook can land a moment after + /// the snapshot a test has already awaited — poll for it rather than sampling + /// the hook once. + async fn recorded_within(recorded: &RefCell>, id: &str, window: Duration) -> bool { + compio::time::timeout(window, async { + loop { + let present = recorded.borrow().iter().any(|got| got.as_str() == id); + if present { + break; + } + compio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .is_ok() + } +} + /// A [`Delegate`] that records which observation hooks the QUIC driver fired. struct RecordingDelegate(Rc); @@ -279,6 +304,9 @@ where serf: SerfOptions, merge: Option>>, snapshot: Option, + /// The wire the gossip plane rides: QUIC datagrams over the peer's pooled + /// connection, or the shared plain-UDP socket. + unreliable: UnreliableTransport, #[cfg(encryption)] encryption: EncryptionOptions, #[cfg(encryption)] @@ -293,6 +321,7 @@ impl NodeSpec> { serf: SerfOptions::new(), merge: None, snapshot: None, + unreliable: UnreliableTransport::Datagram, #[cfg(encryption)] encryption: EncryptionOptions::new(), #[cfg(encryption)] @@ -315,6 +344,7 @@ where serf: self.serf, merge: self.merge, snapshot: self.snapshot, + unreliable: self.unreliable, #[cfg(encryption)] encryption: self.encryption, #[cfg(encryption)] @@ -327,6 +357,13 @@ where self } + /// Route the gossip plane over `unreliable` instead of the default QUIC + /// datagrams. + fn with_unreliable(mut self, unreliable: UnreliableTransport) -> Self { + self.unreliable = unreliable; + self + } + fn with_merge(mut self, merge: Box>) -> Self { self.merge = Some(merge); self @@ -350,24 +387,23 @@ where } /// Build and spawn the node on an ephemeral loopback UDP port. - async fn spawn(self, id: &str) -> Serf { + async fn spawn(self, id: &str) -> Serf { #[allow(unused_mut)] let mut opts = QuicTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(loopback_ephemeral())) - .with_quic_config(test_quic_options()); + .with_quic_config(test_quic_options_with(self.unreliable)); #[cfg(encryption)] { opts = opts.with_encryption(self.encryption); } - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::quic( opts, &SocketAddrResolver, &FirstAddrResolver, self.delegate, self.runtime, self.serf, - gossip_rng().expect("seed gossip rng"), None, self.merge, self.snapshot, @@ -380,12 +416,12 @@ where } /// Spawn a plain loopback QUIC node. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { NodeSpec::new().spawn(id).await } /// Poll both nodes until each reports the full two-member cluster. -async fn converge(a: &Serf, b: &Serf) { +async fn converge(a: &Serf, b: &Serf) { compio::time::timeout(WINDOW, async { loop { if a.num_members() == 2 && b.num_members() == 2 { @@ -399,7 +435,7 @@ async fn converge(a: &Serf, b: &Serf) { } /// Join `joiner` to `seed` over QUIC and wait for both to converge. -async fn join_and_converge(joiner: &Serf, seed: &Serf) { +async fn join_and_converge(joiner: &Serf, seed: &Serf) { joiner .join( &SocketAddrResolver, @@ -455,6 +491,124 @@ fn test_secret_key(fill: u8) -> SecretKey { // ── scenarios ───────────────────────────────────────────────────────────────── +/// Datagram-mode non-vacuity: with `UnreliableTransport::Datagram` the outbound +/// gossip is routed through the QUIC datagram plane (`queue_unreliable_datagram` + +/// `flush_outbound_transmits`), NOT the plain-UDP fallback. Two nodes join and +/// converge, B broadcasts a user event that A receives over gossip, and the +/// sender's `datagrams_sent` counter proves the gossip actually rode QUIC +/// datagrams over the pooled, TLS-protected connection. +/// +/// This is the discriminator the plain-UDP fallback would otherwise mask: a driver +/// that bypassed the configured mode and always sent on the plain socket still +/// delivers the event and converges, but leaves `datagrams_sent` at `0`. +/// `datagrams_sent` advances only on a `DatagramSendStatus::Queued`, so asserting +/// it is non-zero fails on that regression while the convergence assertions alone +/// would not. Paired with `udp_mode_gossip_bypasses_the_datagram_plane`, which +/// asserts the exact opposite for the `Udp` opt-out. +#[compio::test] +async fn datagram_mode_gossip_rides_quic_datagrams() { + let a = spawn_node("dg-a").await; + let b = spawn_node("dg-b").await; + + let mut a_events = a.events(); + join_and_converge(&a, &b).await; + + b.user_event("greet", Bytes::from_static(b"hello"), false) + .await + .expect("B broadcasts a user event"); + + let got = compio::time::timeout(WINDOW, async { + loop { + match a_events.next().await { + Some(Event::User(u)) if u.name.as_str() == "greet" => break u.payload.clone(), + Some(_) => {} + None => panic!("A's event stream closed before the user event arrived"), + } + } + }) + .await + .expect("A receives B's user event within the window"); + assert_eq!( + got, + Bytes::from_static(b"hello"), + "A receives B's user-event payload over datagram-mode gossip" + ); + + // The discriminator: the gossip that crossed rode QUIC datagrams, not the + // plain-UDP fallback. Both nodes hold a warm pooled connection after the join, + // so their periodic gossip is queued as datagrams; `datagrams_sent` advances + // only on a `DatagramSendStatus::Queued`. + compio::time::timeout(WINDOW, async { + loop { + if b.datagrams_sent() > 0 { + break; + } + compio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .expect( + "B's gossip must ride the QUIC datagram plane (datagrams_sent > 0), not the plain-UDP fallback", + ); + + a.shutdown().await.expect("dg-a shuts down"); + b.shutdown().await.expect("dg-b shuts down"); +} + +/// The `Udp` unreliable-transport opt-out routes ALL gossip over the shared plain +/// UDP socket instead of the QUIC datagram plane: two nodes still join and gossip +/// a user event, and the sender's `datagrams_sent` stays at ZERO — the exact +/// discriminator `datagram_mode_gossip_rides_quic_datagrams` asserts the opposite +/// of. The two together are what make each assertion non-vacuous. +#[compio::test] +async fn udp_mode_gossip_bypasses_the_datagram_plane() { + let a = NodeSpec::new() + .with_unreliable(UnreliableTransport::Udp) + .spawn("udp-a") + .await; + let b = NodeSpec::new() + .with_unreliable(UnreliableTransport::Udp) + .spawn("udp-b") + .await; + + let mut a_events = a.events(); + join_and_converge(&a, &b).await; + + b.user_event("greet", Bytes::from_static(b"hello"), false) + .await + .expect("B broadcasts a user event"); + + let got = compio::time::timeout(WINDOW, async { + loop { + match a_events.next().await { + Some(Event::User(u)) if u.name.as_str() == "greet" => break u.payload.clone(), + Some(_) => {} + None => panic!("A's event stream closed before the user event arrived"), + } + } + }) + .await + .expect("A receives B's user event over plain-UDP gossip"); + assert_eq!( + got, + Bytes::from_static(b"hello"), + "the event crosses over the plain-UDP gossip plane" + ); + + // The discriminator: in `Udp` mode NO gossip payload may ride a QUIC datagram. + // The counter advances only on a `DatagramSendStatus::Queued`, so a driver that + // ignored the configured mode would leave it non-zero here. + assert_eq!( + b.datagrams_sent(), + 0, + "the Udp opt-out must route every gossip payload over the plain socket" + ); + assert_eq!(a.datagrams_sent(), 0); + + a.shutdown().await.expect("udp-a shuts down"); + b.shutdown().await.expect("udp-b shuts down"); +} + /// A user event broadcast by B over the QUIC datagram gossip plane reaches A's /// event stream with the original name and payload, and fires A's /// `notify_user_event` observation hook. @@ -489,7 +643,7 @@ async fn a_quic_user_event_reaches_the_peer_stream_and_delegate() { assert_eq!(got, payload, "the payload survives the QUIC broadcast"); assert!( - seen.user_events.borrow().iter().any(|n| n == "deploy"), + Observed::recorded_within(&seen.user_events, "deploy", WINDOW).await, "the QUIC driver fired A's notify_user_event hook" ); @@ -596,7 +750,7 @@ async fn quic_set_tags_propagates_as_a_member_update() { assert_eq!(got.as_str(), "worker", "A's view of B carries the new tag"); assert!( - seen.updated.borrow().iter().any(|id| id == "qt-b"), + Observed::recorded_within(&seen.updated, "qt-b", WINDOW).await, "the QUIC driver fired A's notify_update hook for the re-tagged peer" ); @@ -1343,39 +1497,31 @@ async fn quic_key_rotation_rotates_both_live_keyrings() { b.shutdown().await.expect("qrot-b shuts down"); } -/// The sending half of the driver's rotation-durability acknowledgement channel -/// ([`serf_driver::KeyringPersistRx`] is its receiver). -#[cfg(encryption)] -type PersistTx = std::sync::mpsc::Sender>; - -/// A keyring delegate whose persistence resolves OUT OF BAND: `keyring_updated` -/// hands back a pending receiver, and the test releases it after a delay. The -/// pump must park the key response until the acknowledgement lands and only then -/// route it — so the originator still collects BOTH nodes' successes. +/// A keyring delegate whose persistence resolves OUT OF BAND, the way a real +/// persistence worker does: `keyring_updated` hands the acknowledgement to a +/// detached completer that resolves it on the next runtime turn — after the pump +/// has parked the key response, but tied to the rotation itself rather than any +/// wall-clock delay. The pump must hold the key response until the +/// acknowledgement lands and only then route it, so the originator still +/// collects BOTH nodes' successes. #[cfg(encryption)] -#[derive(Default)] -struct DeferredKeyring { - /// Acknowledgement senders for every rotation this delegate parked, in order. - parked: RefCell>, -} - -#[cfg(encryption)] -impl DeferredKeyring { - /// Acknowledge every parked rotation as durable. - fn release_all(&self) { - for tx in self.parked.borrow_mut().drain(..) { - // Ignoring Err: the pump dropped the receiver (its key request already - // timed out); nothing to acknowledge. - let _ = tx.send(Ok(())); - } - } -} +struct DeferredKeyring; #[cfg(encryption)] impl KeyringDelegate for DeferredKeyring { fn keyring_updated(&self, _keyring: &Keyring) -> serf_driver::KeyringPersistence { let (tx, rx) = std::sync::mpsc::channel(); - self.parked.borrow_mut().push(tx); + // Complete the acknowledgement from a detached task, as an out-of-band + // persistence worker would: it runs on the next runtime turn — after the + // pump has parked the response on `rx` — so the parked response is released + // and routed as soon as the pump next polls it, always inside the key + // query's response window and never gated on a fixed delay. + compio::runtime::spawn(async move { + // Ignoring Err: the pump dropped the receiver (its key request already + // timed out); nothing to acknowledge. + let _ = tx.send(Ok(())); + }) + .detach(); serf_driver::KeyringPersistence::Pending(rx) } } @@ -1392,10 +1538,9 @@ async fn a_parked_key_response_is_routed_once_persistence_acknowledges() { let k2 = test_secret_key(0x44); let enc = || EncryptionOptions::new().with_keyring(Keyring::new(k1)); - let deferred = Rc::new(DeferredKeyring::default()); let b = NodeSpec::new() .with_encryption(enc()) - .with_keyring(deferred.clone()) + .with_keyring(Rc::new(DeferredKeyring)) .spawn("qdef-b") .await; // A keeps the DEFAULT keyring delegate: its own live-ring rotation still @@ -1407,15 +1552,6 @@ async fn a_parked_key_response_is_routed_once_persistence_acknowledges() { let mut a_events = a.events(); a.install_key(k2).await.expect("install_key dispatched"); - // Release B's parked acknowledgement shortly after the rotation lands, well - // inside the key query's response window. - let releaser = deferred.clone(); - compio::runtime::spawn(async move { - compio::time::sleep(Duration::from_millis(300)).await; - releaser.release_all(); - }) - .detach(); - let kr = next_key_response(&mut a_events).await; assert!( kr.num_resp >= 2, diff --git a/serf-compio/tests/tcp.rs b/serf-compio/tests/tcp.rs index f730eb19..11cd9d25 100644 --- a/serf-compio/tests/tcp.rs +++ b/serf-compio/tests/tcp.rs @@ -98,6 +98,27 @@ struct Observed { user_events: RefCell>, } +impl Observed { + /// Poll `recorded` until it contains `id`, bounded by `window`; returns + /// whether it landed in time. Each observation hook is delivered on a path + /// separate from the membership snapshot, so a hook can land a moment after + /// the snapshot a test has already awaited — poll for it rather than sampling + /// the hook once. + async fn recorded_within(recorded: &RefCell>, id: &str, window: Duration) -> bool { + compio::time::timeout(window, async { + loop { + let present = recorded.borrow().iter().any(|got| got.as_str() == id); + if present { + break; + } + compio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .is_ok() + } +} + /// A [`Delegate`] that records which observation hooks the driver fired. struct RecordingDelegate(Rc); @@ -230,18 +251,17 @@ where } /// Build and spawn the node on an ephemeral loopback port. - async fn spawn(self, id: &str) -> Serf { + async fn spawn(self, id: &str) -> Serf { let opts = TcpTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(cluster::loopback_ephemeral())); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tcp( opts, &SocketAddrResolver, &FirstAddrResolver, self.delegate, self.runtime, self.serf, - gossip_rng().expect("seed gossip rng"), None, None, self.snapshot, @@ -254,12 +274,12 @@ where } /// Spawn a plain loopback node. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { NodeSpec::new().spawn(id).await } /// Poll both nodes until each reports the full two-member cluster. -async fn converge(a: &Serf, b: &Serf) { +async fn converge(a: &Serf, b: &Serf) { compio::time::timeout(WINDOW, async { loop { if a.num_members() == 2 && b.num_members() == 2 { @@ -273,7 +293,7 @@ async fn converge(a: &Serf, b: &Serf) { } /// Join `joiner` to `seed` and wait for both to converge. -async fn join_and_converge(joiner: &Serf, seed: &Serf) { +async fn join_and_converge(joiner: &Serf, seed: &Serf) { joiner .join( &SocketAddrResolver, @@ -410,7 +430,7 @@ async fn user_event_reaches_the_peer_stream_and_delegate() { assert_eq!(got, payload, "the payload survives the broadcast"); assert!( - seen.user_events.borrow().iter().any(|n| n == "deploy"), + Observed::recorded_within(&seen.user_events, "deploy", WINDOW).await, "the driver fired A's notify_user_event hook for the broadcast" ); @@ -454,7 +474,7 @@ async fn set_tags_propagates_as_a_member_update() { assert_eq!(got.as_str(), "worker", "A's view of B carries the new tag"); assert!( - seen.updated.borrow().iter().any(|id| id == "tag-b"), + Observed::recorded_within(&seen.updated, "tag-b", WINDOW).await, "the driver fired A's notify_update hook for the re-tagged peer" ); @@ -911,7 +931,7 @@ async fn an_abrupt_kill_surfaces_failed_then_reap() { .expect("A detects the killed peer Failed and reaps it out of the membership"); assert!( - seen.failed.borrow().iter().any(|id| id == "kill-b"), + Observed::recorded_within(&seen.failed, "kill-b", WINDOW).await, "the driver fired notify_failed for the abruptly-killed peer (saw {:?})", seen.failed.borrow() ); @@ -1569,3 +1589,141 @@ fn test_secret_key(fill: u8) -> serf_compio::SecretKey { let key = serf_compio::SecretKey::ChaCha20Poly1305([fill; 32]); key } + +/// The transport's SWIM knobs actually reach the coordinator's `EndpointOptions`, +/// so the memberlist failure detector is tunable from serf-compio. +/// +/// Tuned to a 100 ms probe interval with both suspicion multipliers at 1, an +/// abruptly-killed peer is declared Failed within a few hundred milliseconds. A +/// coordinator left on its OWN defaults needs well over ten seconds for the same +/// kill in a two-node cluster: the probe interval is 1 s, the minimum suspicion +/// timeout is `suspicion_mult(4) * log10(N+1) * probe_interval`, and — with no +/// third node to confirm the Suspect — the timer runs its full +/// `suspicion_max_timeout_mult(6)` multiple of that minimum rather than decaying to +/// it. Asserting detection inside the window below therefore FAILS if +/// `Transport::run` accepted the knobs and dropped them on the floor. +#[compio::test] +async fn swim_knobs_reach_the_coordinator_and_speed_failure_detection() { + /// Detection must land far inside this bound; a default-timing coordinator + /// could not. + const DETECT_WINDOW: Duration = Duration::from_secs(3); + + /// A node whose failure detector is tuned for sub-second detection. + async fn spawn_tuned(id: &str) -> Serf { + let opts = TcpTransportOptions::::new() + .with_local_id(SmolStr::new(id)) + .with_advertise_addr(MaybeResolved::Resolved(cluster::loopback_ephemeral())) + .with_probe_interval(Duration::from_millis(100)) + .with_probe_timeout(Duration::from_millis(50)) + .with_gossip_interval(Duration::from_millis(20)) + // A two-node cluster has no third node to confirm a Suspect, so the + // max-timeout multiple is what the suspicion timer actually runs. Pinning + // both multipliers to 1 keeps it at the probe-scaled minimum. + .with_suspicion_mult(1) + .with_suspicion_max_timeout_mult(1); + Serf::tcp( + opts, + &SocketAddrResolver, + &FirstAddrResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + // Hold the Failed member rather than reaping it, so the status is + // observable instead of racing the reaper. + SerfOptions::new().with_reconnect_timeout(Duration::from_secs(3600)), + None, + None, + None, + #[cfg(encryption)] + Rc::new(serf_compio::VoidKeyringDelegate), + ) + .await + .expect("spawn tuned serf tcp node") + } + + let a = spawn_tuned("swim-a").await; + let b = spawn_tuned("swim-b").await; + + b.join( + &SocketAddrResolver, + MaybeResolved::Resolved(a.advertise_address()), + false, + ) + .await + .expect("B joins A"); + converge(&a, &b).await; + + let subject = SmolStr::new("swim-b"); + b.shutdown().await.expect("swim-b is killed abruptly"); + + compio::time::timeout(DETECT_WINDOW, async { + loop { + let failed = a + .members() + .iter() + .any(|m| m.node().id_ref() == &subject && m.status() == MemberStatus::Failed); + if failed { + break; + } + compio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .expect( + "the tuned probe/suspicion knobs must reach the coordinator: a default-timing \ + coordinator could not declare the killed peer Failed this fast", + ); + + a.shutdown().await.expect("swim-a shuts down"); +} + +/// The transport's user-facing address type `A` is usable with the crate's OWN +/// host-address resolvers, so the declared default `A = HostAddr` is a +/// real, constructible configuration rather than a dead type parameter. +/// +/// `OsResolver` and `DnsResolver` both resolve `hostaddr::HostAddr`, and +/// `HostAddr` carries no wire-codec impl (nor could a downstream crate add one — +/// it is a foreign type). A transport that constrained `A` to the wire-codec trait +/// would therefore reject every one of the crate's host-address resolvers and +/// admit only an already-resolved `SocketAddr`; `A` is only ever resolved at the +/// boundary and never encoded, so no such bound is warranted. This spawns a real +/// node through the default `A`, resolving `localhost:0` with `OsResolver`. +#[compio::test] +async fn an_unresolved_host_advertise_addr_resolves_through_the_os_resolver() { + let host: hostaddr::HostAddr = "localhost:0".parse().expect("a host:port address"); + + let node = Serf::tcp_with_rng( + TcpTransportOptions::new() + .with_local_id(SmolStr::new("hostaddr-node")) + .with_advertise_addr(MaybeResolved::Unresolved(host)), + &OsResolver, + &Ipv4PreferringResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + SerfOptions::new(), + gossip_rng().expect("seed gossip rng"), + None, + None, + None, + #[cfg(encryption)] + Rc::new(serf_compio::VoidKeyringDelegate), + ) + .await + .expect("a HostAddr advertise address resolves and binds through OsResolver"); + + // The resolver produced a concrete, dialable contact: the OS-assigned port is + // read back from the bound socket, and the loopback name resolved to a + // loopback IP. + let advertise = node.advertise_address(); + assert!( + advertise.ip().is_loopback(), + "localhost resolved to a loopback contact, got {advertise}" + ); + assert_ne!( + advertise.port(), + 0, + "the ephemeral :0 must be read back as a concrete bound port" + ); + assert_eq!(node.local_id(), &SmolStr::new("hostaddr-node")); + + node.shutdown().await.expect("hostaddr-node shuts down"); +} diff --git a/serf-compio/tests/tls.rs b/serf-compio/tests/tls.rs index 3c0b7201..41cd6371 100644 --- a/serf-compio/tests/tls.rs +++ b/serf-compio/tests/tls.rs @@ -32,7 +32,7 @@ use rustls::{ use serf_compio::{ FirstAddrResolver, Ipv4PreferringResolver, MergeDelegate, Resolver, RuntimeOptions, Serf, SerfError, SnapshotOptions, SocketAddrResolver, TlsOptions, TlsTransport, TlsTransportOptions, - Transport, VoidDelegate, gossip_rng, + Transport, VoidDelegate, }; use serf_proto::{ event::{Event, MemberEventKind, QueryEvent}, @@ -168,7 +168,7 @@ impl MergeDelegate for RecordingMerge { } /// Build a TLS node on an ephemeral loopback port with the fixture's cert bundle. -async fn spawn_node(id: &str) -> Serf { +async fn spawn_node(id: &str) -> Serf { spawn_node_with(id, None, None) .await .expect("spawn serf tls node") @@ -179,19 +179,18 @@ async fn spawn_node_with( id: &str, merge: Option>>, snapshot: Option, -) -> Result, SerfError> { +) -> Result, SerfError> { let opts = TlsTransportOptions::::new() .with_local_id(SmolStr::new(id)) .with_advertise_addr(MaybeResolved::Resolved(loopback_ephemeral())) .with_tls_options(test_tls_options()); - Serf::new::, SocketAddrResolver, FirstAddrResolver, _, _>( + Serf::tls( opts, &SocketAddrResolver, &FirstAddrResolver, VoidDelegate::::new(), RuntimeOptions::new(), SerfOptions::new(), - gossip_rng().expect("seed gossip rng"), None, merge, snapshot, @@ -202,7 +201,7 @@ async fn spawn_node_with( } /// Poll both nodes until each reports the full two-member cluster. -async fn converge(a: &Serf, b: &Serf) { +async fn converge(a: &Serf, b: &Serf) { compio::time::timeout(WINDOW, async { loop { if a.num_members() == 2 && b.num_members() == 2 { diff --git a/serf-reactor/src/lib.rs b/serf-reactor/src/lib.rs index 4d72db8e..28b71fc8 100644 --- a/serf-reactor/src/lib.rs +++ b/serf-reactor/src/lib.rs @@ -63,7 +63,8 @@ pub(crate) fn os_seeded_std_rng() -> crate::Result { } pub use error::{ - GossipMtuTooSmall, InvalidAdvertiseAddr, InvalidGossipMtu, InvalidOption, Result, SerfError, + GossipMtuTooSmall, InvalidAdvertiseAddr, InvalidGossipMtu, InvalidOption, JoinFailed, Result, + SerfError, }; /// The seed/advertise address form re-exported from `memberlist-proto`: either an @@ -149,9 +150,9 @@ pub use events::EventStream; pub use driver::options::{ Channel, DEFAULT_BRIDGE_INBOUND_CAP, DEFAULT_BRIDGE_RECV_BUF_LEN, DEFAULT_CLOSE_TIMEOUT, DEFAULT_DIAL_TIMEOUT, DEFAULT_EVENT_QUEUE_CAP, DEFAULT_IDLE_WAKE_INTERVAL, - DEFAULT_ITER_DRAIN_CAP, DEFAULT_LEAVE_TIMEOUT, DEFAULT_OBSERVATION_CHANNEL, - DEFAULT_SNAPSHOT_COMPACT_THRESHOLD, ParseChannelError, RuntimeOptions, SnapshotOptions, - StreamTransportOptions, + DEFAULT_ITER_DRAIN_CAP, DEFAULT_JOIN_DEADLINE, DEFAULT_LEAVE_TIMEOUT, + DEFAULT_OBSERVATION_CHANNEL, DEFAULT_SNAPSHOT_COMPACT_THRESHOLD, ParseChannelError, + RuntimeOptions, SnapshotOptions, StreamTransportOptions, }; #[cfg(any(feature = "tcp", feature = "quic"))] #[cfg_attr(docsrs, doc(cfg(any(feature = "tcp", feature = "quic"))))] diff --git a/serf-reactor/src/quic/mod.rs b/serf-reactor/src/quic/mod.rs index 8513f2fb..aadf1b40 100644 --- a/serf-reactor/src/quic/mod.rs +++ b/serf-reactor/src/quic/mod.rs @@ -27,7 +27,7 @@ use agnostic::{ }; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, EndpointOptions, Id, MaybeResolved, QuicEndpoint as Coordinator, + CheapClone, EndpointOptions, Id, MaybeResolved, QuicEndpoint as Coordinator, }; use rand::rngs::StdRng; use smol_str::SmolStr; @@ -384,12 +384,21 @@ where encryption: EncryptionOptions, } +// `A` is the caller's UNRESOLVED address domain, and it takes no codec bound: `new` +// resolves it to a `SocketAddr` once, and nothing downstream encodes it — the quinn +// endpoint, the coordinator, and every membership address are `SocketAddr`. A +// `Data` bound here would instead exclude the very address type the built-in +// resolvers produce, since `OsResolver` / `DnsResolver` yield +// `hostaddr::HostAddr`, which this crate cannot implement `Data` for (both +// the trait and the type are foreign to it). `Send + 'static` are structural: the +// `MaybeResolved` field must keep the transport `Send + 'static` for +// the detached driver pump. impl Transport for QuicTransport where R: Runtime, I: Id + CheapClone + Clone + core::fmt::Debug + core::fmt::Display + Send + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, { type Error = SerfError; type Id = I; diff --git a/serf-reactor/src/serf/mod.rs b/serf-reactor/src/serf/mod.rs index 14c18836..bac04654 100644 --- a/serf-reactor/src/serf/mod.rs +++ b/serf-reactor/src/serf/mod.rs @@ -56,10 +56,10 @@ use crate::{ snapshot::SerfSnapshot, transport::{Transport, TransportRuntime}, }; +#[cfg(any(feature = "tcp", feature = "quic"))] +use memberlist_proto::CheapClone; #[cfg(encryption)] use memberlist_proto::SecretKey; -#[cfg(any(feature = "tcp", feature = "quic"))] -use memberlist_proto::{CheapClone, Data}; /// The initial published snapshot: the local node, `Alive`, with empty tags and /// zeroed Lamport clocks. Superseded by the driver's first real republish. @@ -88,11 +88,23 @@ where /// last handle is dropped (or [`shutdown`](Serf::shutdown) is called). Membership /// reads are lock-free via the published [`SerfSnapshot`]. /// -/// `Serf` carries the wire id type `I`, the resolver's unresolved address -/// type `A`, and the agnostic runtime `R` its driver was spawned on. `I` flows -/// into the snapshot and events channel (both ``); `A` ties `join`'s -/// seeds to the address domain the node was built with; `R` brands the handle so a -/// tokio-backed node is a distinct type from a smol-backed one. +/// `Serf` carries the wire id type `I`, the unresolved address type `A` +/// the node's ADVERTISE address was configured in, and the agnostic runtime `R` its +/// driver was spawned on. `I` flows into the snapshot and events channel (both +/// ``); `R` brands the handle so a tokio-backed node is a distinct +/// type from a smol-backed one. +/// +/// `A` is a brand, not a wire type: the advertise address is resolved to a +/// [`SocketAddr`] once at construction, and every membership address from then on is +/// a `SocketAddr`. It is therefore free to be a hostname type — e.g. the +/// [`hostaddr::HostAddr`](hostaddr::HostAddr) that +/// [`OsResolver`](crate::OsResolver) consumes. +/// +/// `A` does NOT constrain the seeds accepted by [`join`](Serf::join) / +/// [`join_many`](Serf::join_many) / [`dispatch_join`](Serf::dispatch_join): each is +/// generic over the [`Resolver`](crate::Resolver) it is handed and takes its seeds +/// in THAT resolver's address domain. Joining by hostname while advertising a +/// resolved `SocketAddr` (or the reverse) is deliberately allowed. #[cfg(any(feature = "tcp", feature = "quic"))] #[cfg_attr(docsrs, doc(cfg(any(feature = "tcp", feature = "quic"))))] pub struct Serf { @@ -105,8 +117,10 @@ pub struct Serf { /// [`default_query_timeout`](Serf::default_query_timeout) derives the query /// timeout from the live snapshot member count without a driver round-trip. query_timeout_mult: usize, - /// Ties the handle to the resolver's unresolved address type. Not held in any - /// field — `join` enforces seeds resolve in this address domain. + /// Brands the handle with the unresolved address type the node's advertise + /// address was configured in. No `A` value survives construction (it is resolved + /// to a `SocketAddr` before the driver starts), and the `join` family is generic + /// over the resolver it is handed, so this constrains no seed. _a: PhantomData, /// Brands the handle with the agnostic runtime its driver was spawned on. Not /// held in any field — the driver task is spawned detached. @@ -264,7 +278,7 @@ where + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, R: Runtime, { /// Build a TCP-backed serf node and spawn its driver on the runtime `R`. @@ -370,7 +384,7 @@ where + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, R: Runtime, { /// Build a TLS-backed serf node and spawn its driver on the runtime `R`. @@ -479,7 +493,7 @@ where + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, R: Runtime, { /// Build a QUIC-backed serf node and spawn its driver on the runtime `R`. diff --git a/serf-reactor/src/serf/tests.rs b/serf-reactor/src/serf/tests.rs index 445f1565..80867952 100644 --- a/serf-reactor/src/serf/tests.rs +++ b/serf-reactor/src/serf/tests.rs @@ -1,16 +1,19 @@ //! Handle-level unit tests for the reactor `Serf` on tokio: the ergonomic -//! `Serf::tcp` constructor's option validation, single-node shutdown/rebind, and -//! the fast join-failure paths. The multi-node convergence / event / query / -//! leave behavior is covered by the real-node suite in `tests/tcp.rs`. +//! `Serf::tcp` constructor's option validation, single-node shutdown/rebind, the +//! fast join-failure paths, and the hostname-addressed advertise path (a node whose +//! `A` is the `HostAddr` domain the built-in name resolvers consume). The multi-node +//! convergence / event / query / leave behavior is covered by the real-node suite in +//! `tests/tcp.rs`. use core::{future::Future, time::Duration}; use std::net::SocketAddr; use agnostic::tokio::TokioRuntime; +use hostaddr::HostAddr; use crate::{ - Channel, FirstAddrResolver, MaybeResolved, Resolver, RuntimeOptions, Serf, SerfError, - SocketAddrResolver, TcpTransportOptions, VoidDelegate, + Channel, FirstAddrResolver, Ipv4PreferringResolver, MaybeResolved, OsResolver, Resolver, + RuntimeOptions, Serf, SerfError, SocketAddrResolver, TcpTransportOptions, VoidDelegate, }; use serf_proto::options::Options as SerfOptions; use smol_str::SmolStr; @@ -18,6 +21,29 @@ use smol_str::SmolStr; /// A tokio-backed reactor TCP node handle. type Node = Serf; +/// A tokio-backed reactor TCP node whose advertise address is supplied UNRESOLVED, +/// in the `HostAddr` domain that [`OsResolver`] and [`DnsResolver`](crate::DnsResolver) +/// consume — the address type a node can be built with only because the transport +/// puts no wire-codec bound on `A`. +type HostNode = Serf, TokioRuntime>; + +/// Wall-clock ceiling for the poll loops below, so a convergence regression fails +/// with a diagnosis instead of hanging. +const POLL_TIMEOUT: Duration = Duration::from_secs(20); +/// Poll granularity for the await loops below. +const POLL_STEP: Duration = Duration::from_millis(20); + +/// Poll `cond` until it holds, bounded by [`POLL_TIMEOUT`]. +async fn await_condition(what: &str, cond: impl Fn() -> bool) { + tokio::time::timeout(POLL_TIMEOUT, async { + while !cond() { + tokio::time::sleep(POLL_STEP).await; + } + }) + .await + .unwrap_or_else(|_| panic!("timed out waiting for {what}")); +} + /// A loopback address with a port nothing listens on — `connect()` returns /// `ECONNREFUSED` immediately, so its push/pull exchange fails fast. The port is /// below the OS ephemeral range, so a `:0` test bind never collides with it. @@ -413,3 +439,211 @@ async fn tcp_default_query_param_defaults() { a.shutdown().await.expect("node shuts down"); } + +/// Build a node whose advertise address is the UNRESOLVED host `host`, resolved +/// through `resolver` at construction — so the node's address domain `A` is +/// `HostAddr`, what the built-in name resolvers produce, rather than a +/// wire `SocketAddr`. The advertise candidate set is narrowed by +/// [`Ipv4PreferringResolver`], which still falls back to the head of the set on an +/// IPv6-only host. +async fn try_spawn_host_node( + id: &str, + host: &str, + resolver: &RES, +) -> Result +where + RES: Resolver
>, +{ + let advertise: HostAddr = host.parse().expect("host addr"); + let opts = TcpTransportOptions::>::new() + .with_local_id(SmolStr::new(id)) + .with_advertise_addr(MaybeResolved::Unresolved(advertise)); + Serf::, TokioRuntime>::tcp( + opts, + resolver, + &Ipv4PreferringResolver, + VoidDelegate::::new(), + RuntimeOptions::new(), + SerfOptions::new(), + None, + None, + None, + #[cfg(encryption)] + std::sync::Arc::new(crate::VoidKeyringDelegate), + ) + .await +} + +/// A node can advertise a HOSTNAME: built over the `HostAddr` address domain and an +/// [`OsResolver`], it resolves `localhost:0` at construction, binds the resolved +/// loopback address, and publishes the concrete `SocketAddr` it bound as its +/// contact. A second such node then joins the first BY HOSTNAME through the same +/// resolver and both converge — so the resolved address is a real, reachable +/// contact, not merely a value that type-checked. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn tcp_advertise_and_join_through_os_resolver() { + let resolver = OsResolver::::default(); + + let a = try_spawn_host_node("os-host-a", "localhost:0", &resolver) + .await + .expect("a hostname advertise address resolves and binds"); + let a_addr = a.advertise_address(); + assert!( + a_addr.ip().is_loopback(), + "the node advertises the RESOLVED loopback IP, not the hostname it was given: {a_addr}" + ); + assert_ne!( + a_addr.port(), + 0, + "the ephemeral `:0` resolved to the concrete port the listener bound" + ); + + let b = try_spawn_host_node("os-host-b", "localhost:0", &resolver) + .await + .expect("a second hostname-addressed node binds"); + assert_ne!( + b.advertise_address(), + a_addr, + "the two nodes bind distinct ephemeral ports" + ); + + // The seed is a HOSTNAME (`localhost:`), never a `SocketAddr`: `join` + // resolves it through the same resolver and reports the seed it actually reached. + let seed: HostAddr = format!("localhost:{}", a_addr.port()) + .parse() + .expect("hostname seed"); + let reached = b + .join(&resolver, MaybeResolved::Unresolved(seed), false) + .await + .expect("the hostname seed resolves and its node is contacted"); + assert_eq!( + reached, a_addr, + "the contacted seed is the first node's bound advertise address" + ); + + await_condition("both hostname-addressed nodes to see 2 members", || { + a.num_members() == 2 && b.num_members() == 2 + }) + .await; + + a.shutdown().await.expect("node a shuts down"); + b.shutdown().await.expect("node b shuts down"); +} + +/// A loopback TCP nameserver answering exactly ONE query with a single `A` record +/// for `127.0.0.1`. Speaks just enough of TCP-DNS (RFC 1035 §4.2.2: a 2-byte +/// big-endian length prefix, then the message) for the resolver's TCP-first path; +/// the resolver harvests A/AAAA answers without validating the question, so a fixed +/// answer needs no query parsing. Returns the bound address to point a resolver at. +/// Nothing leaves loopback. +#[cfg(feature = "dns")] +async fn spawn_loopback_nameserver() -> (SocketAddr, tokio::task::JoinHandle<()>) { + use std::net::Ipv4Addr; + + use agnostic::{ + Runtime, + net::{Net, TcpListener}, + }; + use futures_util::{AsyncReadExt, AsyncWriteExt}; + use hickory_proto::{ + op::{Message, OpCode}, + rr::{Name, RData, Record, rdata::A}, + }; + + let listener = <::Net as Net>::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind loopback nameserver"); + let addr = listener.local_addr().expect("nameserver local_addr"); + + let handle = tokio::spawn(async move { + let Ok((mut stream, _)) = listener.accept().await else { + return; + }; + // The 2-byte big-endian length prefix, then the query body it announces. + let mut len_buf = [0u8; 2]; + if stream.read_exact(&mut len_buf).await.is_err() { + return; + } + let mut query = vec![0u8; u16::from_be_bytes(len_buf) as usize]; + if stream.read_exact(&mut query).await.is_err() { + return; + } + + let mut resp = Message::response(0, OpCode::Query); + resp.add_answer(Record::from_rdata( + Name::from_ascii(ADVERTISE_NAME_FQDN).expect("answer name"), + 60, + RData::A(A(Ipv4Addr::LOCALHOST)), + )); + let body = resp.to_vec().expect("encode DNS response"); + let mut framed = Vec::with_capacity(2 + body.len()); + framed.extend_from_slice(&(body.len() as u16).to_be_bytes()); + framed.extend_from_slice(&body); + // Ignoring Err: a best-effort single write from a one-shot fixture; a client + // that hung up surfaces as the resolving node's construction failure instead. + let _ = stream.write_all(&framed).await; + }); + + (addr, handle) +} + +/// The advertise name the fixture nameserver answers for. Its `.invalid` TLD is +/// reserved never to resolve (RFC 6761 §6.4), so the OS fallback inside +/// `DnsResolver` CANNOT produce an address for it — a node that comes up on +/// loopback therefore did so on the nameserver's `A` record, not on a fallback. +#[cfg(feature = "dns")] +const ADVERTISE_NAME_FQDN: &str = "seed.cluster.invalid."; + +/// A node can advertise a DNS name: built over the `HostAddr` address domain and a +/// [`DnsResolver`](crate::DnsResolver) pointed at the loopback fixture nameserver, +/// it resolves `seed.cluster.invalid:0` at construction and binds the `127.0.0.1` +/// its `A` record carried. Because that name is unresolvable by the OS fallback, +/// binding loopback proves the DNS answer drove the bind. A plain peer then joins +/// the DNS-derived contact and both converge, proving it is genuinely reachable. +#[cfg(feature = "dns")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn tcp_advertise_resolved_through_dns_resolver() { + use std::net::{IpAddr, Ipv4Addr}; + + use crate::DnsResolver; + + let (nameserver, server) = spawn_loopback_nameserver().await; + let resolver = DnsResolver::::from_servers(vec![nameserver]) + .with_timeout(Duration::from_secs(5)); + + let a = try_spawn_host_node("dns-host-a", "seed.cluster.invalid:0", &resolver) + .await + .expect("the fixture nameserver's answer resolves the advertise address"); + server.await.expect("the nameserver answered one query"); + + let a_addr = a.advertise_address(); + assert_eq!( + a_addr.ip(), + IpAddr::V4(Ipv4Addr::LOCALHOST), + "the node bound the IP its `A` record carried" + ); + assert_ne!( + a_addr.port(), + 0, + "the ephemeral `:0` resolved to the concrete port the listener bound" + ); + + // The DNS-derived contact is real: a peer dials it and the two converge. + let b = spawn_node("dns-peer-b").await; + let reached = b + .join(&SocketAddrResolver, MaybeResolved::Resolved(a_addr), false) + .await + .expect("the DNS-advertised node is reachable at the address it published"); + assert_eq!(reached, a_addr, "the peer contacted the advertised address"); + + await_condition( + "the DNS-advertised node and its peer to see 2 members", + || a.num_members() == 2 && b.num_members() == 2, + ) + .await; + + a.shutdown() + .await + .expect("the DNS-advertised node shuts down"); + b.shutdown().await.expect("the peer shuts down"); +} diff --git a/serf-reactor/src/tcp/mod.rs b/serf-reactor/src/tcp/mod.rs index 844abbe4..3eb93a75 100644 --- a/serf-reactor/src/tcp/mod.rs +++ b/serf-reactor/src/tcp/mod.rs @@ -18,7 +18,7 @@ use agnostic::{ }; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, Endpoint, EndpointOptions, Id, MaybeResolved, RawRecords, + CheapClone, Endpoint, EndpointOptions, Id, MaybeResolved, RawRecords, streams::{LabelOptions, StreamEndpoint as Coordinator}, }; use rand::rngs::StdRng; @@ -348,12 +348,21 @@ where encryption: EncryptionOptions, } +// `A` is the caller's UNRESOLVED address domain, and it takes no codec bound: `new` +// resolves it to a `SocketAddr` once, and nothing downstream encodes it — the +// coordinator, the snapshot, and every membership address are `SocketAddr`. A +// `Data` bound here would instead exclude the very address type the built-in +// resolvers produce, since `OsResolver` / `DnsResolver` yield +// `hostaddr::HostAddr`, which this crate cannot implement `Data` for (both +// the trait and the type are foreign to it). `Send + 'static` are structural: the +// `MaybeResolved` field must keep the transport `Send + 'static` for +// the detached driver pump. impl Transport for TcpTransport where R: Runtime, I: Id + CheapClone + Clone + core::fmt::Debug + core::fmt::Display + Send + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, { type Error = SerfError; type Id = I; diff --git a/serf-reactor/src/tls/mod.rs b/serf-reactor/src/tls/mod.rs index 38276428..2c08cd0f 100644 --- a/serf-reactor/src/tls/mod.rs +++ b/serf-reactor/src/tls/mod.rs @@ -41,7 +41,7 @@ use agnostic::{ }; use hostaddr::HostAddr; use memberlist_proto::{ - CheapClone, Data, Endpoint, EndpointOptions, Id, MaybeResolved, TlsRecords, + CheapClone, Endpoint, EndpointOptions, Id, MaybeResolved, TlsRecords, streams::{LabelOptions, Labeled, StreamEndpoint as Coordinator}, }; use rand::rngs::StdRng; @@ -454,12 +454,21 @@ where encryption: EncryptionOptions, } +// `A` is the caller's UNRESOLVED address domain, and it takes no codec bound: `new` +// resolves it to a `SocketAddr` once, and nothing downstream encodes it — the TLS +// record layer, the coordinator, and every membership address are `SocketAddr`. A +// `Data` bound here would instead exclude the very address type the built-in +// resolvers produce, since `OsResolver` / `DnsResolver` yield +// `hostaddr::HostAddr`, which this crate cannot implement `Data` for (both +// the trait and the type are foreign to it). `Send + 'static` are structural: the +// `MaybeResolved` field must keep the transport `Send + 'static` for +// the detached driver pump. impl Transport for TlsTransport where R: Runtime, I: Id + CheapClone + Clone + core::fmt::Debug + core::fmt::Display + Send + Sync + Unpin + 'static, - A: Data + Clone + Send + Sync + 'static, + A: Clone + Send + Sync + 'static, { type Error = SerfError; type Id = I; diff --git a/serf/Cargo.toml b/serf/Cargo.toml index 65c64051..b4a4bc79 100644 --- a/serf/Cargo.toml +++ b/serf/Cargo.toml @@ -61,8 +61,16 @@ embedded = ["dep:serf-embedded", "alloc", "serf-embedded/alloc"] # the `-aws-lc-rs` variant) — which turn on `tls` / `quic` transitively. tcp = ["serf-proto/tcp", "serf-reactor?/tcp", "serf-compio?/tcp"] tls = ["serf-proto/tls", "serf-reactor?/tls", "serf-compio?/tls"] -tls-rustls-ring = ["tls", "serf-reactor?/tls-rustls-ring"] -tls-rustls-aws-lc-rs = ["tls", "serf-reactor?/tls-rustls-aws-lc-rs"] +tls-rustls-ring = [ + "tls", + "serf-reactor?/tls-rustls-ring", + "serf-compio?/tls-rustls-ring", +] +tls-rustls-aws-lc-rs = [ + "tls", + "serf-reactor?/tls-rustls-aws-lc-rs", + "serf-compio?/tls-rustls-aws-lc-rs", +] quic = ["serf-proto/quic", "serf-reactor?/quic", "serf-compio?/quic"] quic-rustls-ring = [ "quic",