diff --git a/core/integration/tests/server/legacy_login_vsr.rs b/core/integration/tests/server/legacy_login_vsr.rs index 98b685e1b3..f0a88c1342 100644 --- a/core/integration/tests/server/legacy_login_vsr.rs +++ b/core/integration/tests/server/legacy_login_vsr.rs @@ -27,19 +27,15 @@ //! TCP socket: a header-only non-replicated frame carrying the code in the //! reserved command slot. -use iggy_binary_protocol::HEADER_SIZE; +use iggy_binary_protocol::EvictionReason; use iggy_binary_protocol::codes::{LOGIN_USER_CODE, LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE}; -use iggy_binary_protocol::consensus::{Command, Operation, RequestHeader}; +use iggy_binary_protocol::consensus::Command; use integration::harness::TestHarness; use integration::iggy_harness; -use std::mem::offset_of; -use std::time::Duration; -use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use tokio::net::TcpStream; -use tokio::time::timeout; -// Wire byte pinned to `EvictionReason::MalformedLogin` in consensus::header. -const EVICTION_REASON_MALFORMED_LOGIN: u8 = 15; +use crate::server::raw_tcp::{ + connect, eviction_reason, frame_command, non_replicated_header, read_frame_header, write_frame, +}; #[iggy_harness] async fn given_legacy_login_user_code_when_sent_raw_should_evict_malformed_login( @@ -60,45 +56,26 @@ async fn given_legacy_pat_login_code_when_sent_raw_should_evict_malformed_login( /// eviction. The reject runs before the session gate, so this unbound socket /// exercises the same path a bound connection would. async fn assert_legacy_login_code_evicted(harness: &TestHarness, code: u32) { - let mut header = RequestHeader { - command: Command::Request, - operation: Operation::NonReplicated, - size: u32::try_from(HEADER_SIZE).unwrap(), - // NonReplicated leaves session / request unchecked, but the header - // validator still requires a nonzero client id. - client: 0xC0FFEE, - session: 0, - request: 0, - ..Default::default() - }; - // A non-replicated command code travels in the first 4 reserved bytes. - header.reserved[..4].copy_from_slice(&code.to_le_bytes()); + // NonReplicated leaves session / request unchecked, but the header + // validator still requires a nonzero client id. + let header = non_replicated_header(0xC0FFEE, 0, 0, code); - let addr = harness - .server() - .tcp_addr() - .expect("server must expose a TCP address"); - let mut stream = TcpStream::connect(addr).await.unwrap(); - stream.write_all(bytemuck::bytes_of(&header)).await.unwrap(); + let mut stream = connect(harness).await; + write_frame(&mut stream, &header, &[]).await; - // Eviction is header-only: exactly 256 bytes. The timeout makes the + // Eviction is header-only: exactly 256 bytes. The bounded read makes the // fail-fast contract explicit -- a regression that silently drops the // frame trips this instead of hanging until the test wall clock. - let mut reply = [0u8; HEADER_SIZE]; - timeout(Duration::from_secs(5), stream.read_exact(&mut reply)) - .await - .expect("server must answer a legacy login code within 5s, not stall") - .expect("reading the eviction frame must succeed"); + let reply = read_frame_header(&mut stream).await; - let command_offset = offset_of!(RequestHeader, command); assert_eq!( - reply[command_offset], + frame_command(&reply), Command::Eviction as u8, "expected an Eviction frame for legacy login code {code}, not a Reply" ); assert_eq!( - reply[HEADER_SIZE - 1], - EVICTION_REASON_MALFORMED_LOGIN, + eviction_reason(&reply), + EvictionReason::MalformedLogin as u8, "legacy login code {code} must evict with MalformedLogin" ); } diff --git a/core/integration/tests/server/mod.rs b/core/integration/tests/server/mod.rs index a1da26b1e3..97e538491e 100644 --- a/core/integration/tests/server/mod.rs +++ b/core/integration/tests/server/mod.rs @@ -21,9 +21,16 @@ mod a2a_jwt; mod cg; // Flush (FLUSH_UNSAVED_BUFFER) has no the server primitive; it must deny typed. mod flush_vsr; +// Raw TCP framing (connect, hand-crafted frames, root register) for the +// server suites that send what the SDK cannot. +pub(crate) mod raw_tcp; // Legacy login codes (LOGIN_USER / LOGIN_WITH_PAT) have no the server handler; // they must evict typed (MalformedLogin), not stall or reply empty-ok. mod legacy_login_vsr; +// A non-replicated code no read serves (unknown, or table-listed without an +// arm) must deny typed (InvalidCommand) at the read gate, not stall or reply +// empty-ok. +mod unknown_code_vsr; // A failed credential login must report the credential failure, not the // payload shape it fell through to. mod login_credentials_vsr; diff --git a/core/integration/tests/server/raw_tcp.rs b/core/integration/tests/server/raw_tcp.rs new file mode 100644 index 0000000000..cfe5d8c698 --- /dev/null +++ b/core/integration/tests/server/raw_tcp.rs @@ -0,0 +1,207 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Raw TCP framing for the server suites that hand-craft client frames the +//! SDK cannot emit: connect to the harness server, write one request frame, +//! read the header the server answers with, and register root so a frame +//! can ride a bound session. + +use std::mem::offset_of; +use std::time::Duration; + +use iggy::prelude::*; +use iggy_binary_protocol::codec::{WireDecode, WireEncode}; +use iggy_binary_protocol::consensus::{ + Command, Operation, ReplyHeader, RequestHeader, read_size_field, result_code, + result_section_len, +}; +use iggy_binary_protocol::requests::users::LoginRegisterRequest; +use iggy_binary_protocol::responses::users::LoginRegisterResponse; +use iggy_binary_protocol::{ + ClientVersionInfo, EvictionHeader, HEADER_SIZE, IGGY_PROTOCOL_VERSION, WireName, +}; +use integration::harness::TestHarness; +use secrecy::SecretString; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpStream; +use tokio::time::{Instant, sleep, timeout}; + +/// Per-frame reply wait. A server that drops the frame answers nothing at +/// all, so an unanswered read is a verdict, not a reason to wait longer. +const REPLY_WAIT: Duration = Duration::from_secs(5); + +/// Budget for the register to commit: right after boot the single node may +/// still be electing itself and answers transient rejections meanwhile. +const COMMIT_BUDGET: Duration = Duration::from_secs(15); + +const RETRY_PAUSE: Duration = Duration::from_millis(100); + +pub(crate) async fn connect(harness: &TestHarness) -> TcpStream { + let addr = harness + .server() + .tcp_addr() + .expect("server must expose a TCP address"); + TcpStream::connect(addr).await.unwrap() +} + +pub(crate) fn request_header( + operation: Operation, + client: u128, + session: u64, + request: u64, + body_len: usize, +) -> RequestHeader { + RequestHeader { + command: Command::Request, + operation, + size: u32::try_from(HEADER_SIZE + body_len).unwrap(), + client, + session, + request, + ..Default::default() + } +} + +/// A header-only `NonReplicated` frame; the command code travels in the +/// first 4 reserved bytes. +pub(crate) fn non_replicated_header( + client: u128, + session: u64, + request: u64, + code: u32, +) -> RequestHeader { + let mut header = request_header(Operation::NonReplicated, client, session, request, 0); + header.reserved[..4].copy_from_slice(&code.to_le_bytes()); + header +} + +pub(crate) async fn write_frame(stream: &mut TcpStream, header: &RequestHeader, body: &[u8]) { + stream.write_all(bytemuck::bytes_of(header)).await.unwrap(); + if !body.is_empty() { + stream.write_all(body).await.unwrap(); + } +} + +/// Read the header of the next server frame, within [`REPLY_WAIT`]. +pub(crate) async fn read_frame_header(stream: &mut TcpStream) -> [u8; HEADER_SIZE] { + let mut header = [0u8; HEADER_SIZE]; + timeout(REPLY_WAIT, stream.read_exact(&mut header)) + .await + .expect("server must answer within the reply wait, not stall") + .expect("reply header read failed"); + header +} + +/// Write one frame and read one Reply off the lockstep connection, the body +/// sized by the reply's size field. +pub(crate) async fn exchange( + stream: &mut TcpStream, + header: &RequestHeader, + body: &[u8], +) -> ([u8; HEADER_SIZE], Vec) { + write_frame(stream, header, body).await; + let reply_header = read_frame_header(stream).await; + let command = frame_command(&reply_header); + assert_eq!( + command, + Command::Reply as u8, + "expected a Reply frame, got command byte {command} (an Eviction carries reason {})", + eviction_reason(&reply_header) + ); + + let total_size = read_size_field(&reply_header).expect("reply size field") as usize; + // `read_size_field` is a bare 4-byte LE read with no floor (the SDK adds + // its own guard, which this helper cannot inherit), so an under-sized + // field would either panic on subtract overflow or wrap into a ~1.8e19 + // allocation and abort - either way hiding the regression under test. + assert!( + total_size >= HEADER_SIZE, + "reply size field {total_size} is below the {HEADER_SIZE}-byte header" + ); + let mut reply_body = vec![0u8; total_size - HEADER_SIZE]; + timeout(REPLY_WAIT, stream.read_exact(&mut reply_body)) + .await + .expect("reply body timed out") + .expect("reply body read failed"); + (reply_header, reply_body) +} + +pub(crate) fn frame_command(header: &[u8; HEADER_SIZE]) -> u8 { + header[offset_of!(RequestHeader, command)] +} + +/// The typed reason byte of an `Eviction` frame, read by field offset rather +/// than by a hardcoded index: `EvictionReason` is renumberable, so a literal +/// would keep passing while checking a different reason. +pub(crate) fn eviction_reason(header: &[u8; HEADER_SIZE]) -> u8 { + header[offset_of!(EvictionHeader, reason)] +} + +pub(crate) fn reply_status(reply_header: &[u8; HEADER_SIZE]) -> u32 { + let offset = offset_of!(ReplyHeader, status); + u32::from_le_bytes(reply_header[offset..offset + 4].try_into().unwrap()) +} + +/// Root login/register on the socket as `client`, replayed on transient +/// rejections until it commits; returns the bound session id. The session +/// binds to THIS transport connection server-side, so a frame that must ride +/// it has to reuse the stream. +pub(crate) async fn register_root(stream: &mut TcpStream, client: u128) -> u64 { + let body = LoginRegisterRequest { + version_info: ClientVersionInfo { + protocol_version: IGGY_PROTOCOL_VERSION, + sdk_name: WireName::new("raw-tcp").unwrap(), + sdk_version: WireName::new("0.0.1").unwrap(), + }, + username: WireName::new(DEFAULT_ROOT_USERNAME).unwrap(), + password: SecretString::from(DEFAULT_ROOT_PASSWORD), + client_context: None, + } + .to_bytes(); + let header = request_header(Operation::Register, client, 0, 0, body.len()); + let deadline = Instant::now() + COMMIT_BUDGET; + loop { + let (reply_header, reply_body) = exchange(stream, &header, &body).await; + // A pre-commit deny rides the status word; a committed verdict rides + // the result section that leads a Register reply body. + let code = match reply_status(&reply_header) { + 0 => result_code(&reply_body).expect("register reply must carry a result section"), + status => status, + }; + if code == 0 { + let payload_start = result_section_len(&reply_body).unwrap(); + let response = LoginRegisterResponse::decode_from(&reply_body[payload_start..]) + .expect("register payload must decode"); + assert_ne!(response.session, 0, "server must bind a nonzero session"); + return response.session; + } + assert!( + is_transient(code), + "register rejected with a terminal code {code}" + ); + assert!( + Instant::now() < deadline, + "register did not commit within {COMMIT_BUDGET:?}" + ); + sleep(RETRY_PAUSE).await; + } +} + +fn is_transient(code: u32) -> bool { + code == IggyError::TransientNotCommitted.as_code() + || code == IggyError::TransientNotAccepted.as_code() +} diff --git a/core/integration/tests/server/unknown_code_vsr.rs b/core/integration/tests/server/unknown_code_vsr.rs new file mode 100644 index 0000000000..622e5bac10 --- /dev/null +++ b/core/integration/tests/server/unknown_code_vsr.rs @@ -0,0 +1,92 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Armless non-replicated command codes against the server (vsr). The read +//! gate is total over the protocol command table, so a bound session sending +//! a `NonReplicated` header whose reserved command slot carries a code the +//! reads have no arm for must get a typed `InvalidCommand` deny Reply. Two +//! codes pin the two halves. One no table entry claims (the SDK forwards +//! unknown codes untouched, `COMMAND_TABLE` being a registry rather than a +//! capability list): the shared response builder's catch-all already denied +//! it `InvalidCommand`, so that test pins the pre-existing deny now that the +//! gate owns it. One the table lists but no read serves (`LOGOUT_USER`, which +//! the SDK only ever sends as `Operation::Logout`): the old gate fell open +//! and let the builder acknowledge it empty-ok, as if a logout had happened, +//! so that test pins the closed fail-open. The frames are hand-crafted on a +//! raw TCP socket to pin the status word the SDK maps through +//! `IggyError::from_code`. + +use iggy::prelude::*; +use iggy_binary_protocol::codes::LOGOUT_USER_CODE; +use iggy_binary_protocol::lookup_command; +use integration::harness::TestHarness; +use integration::iggy_harness; + +use crate::server::raw_tcp::{ + connect, exchange, non_replicated_header, register_root, reply_status, +}; + +/// A code no `COMMAND_TABLE` entry claims. +const UNKNOWN_CODE: u32 = 9999; + +/// The header validator requires a nonzero client id; the value is otherwise +/// free since nothing here reconnects. +const CLIENT_ID: u128 = 0xBAD_C0DE; + +#[iggy_harness] +async fn given_bound_session_when_unknown_non_replicated_code_sent_should_deny_invalid_command( + harness: &TestHarness, +) { + assert!( + lookup_command(UNKNOWN_CODE).is_none(), + "test needs a code absent from COMMAND_TABLE" + ); + assert_non_replicated_code_denied_invalid_command(harness, UNKNOWN_CODE).await; +} + +#[iggy_harness] +async fn given_bound_session_when_table_listed_code_without_read_arm_sent_should_deny_invalid_command( + harness: &TestHarness, +) { + assert!( + lookup_command(LOGOUT_USER_CODE).is_some_and(|meta| !meta.is_replicated()), + "test needs a non-replicated COMMAND_TABLE entry" + ); + assert_non_replicated_code_denied_invalid_command(harness, LOGOUT_USER_CODE).await; +} + +/// Register root on a raw socket, send a header-only `NonReplicated` frame +/// carrying `code` in the reserved command slot on that bound connection, and +/// assert the server answers with an `InvalidCommand` deny Reply. +async fn assert_non_replicated_code_denied_invalid_command(harness: &TestHarness, code: u32) { + let mut stream = connect(harness).await; + let session = register_root(&mut stream, CLIENT_ID).await; + + let header = non_replicated_header(CLIENT_ID, session, 1, code); + let (reply_header, reply_body) = exchange(&mut stream, &header, &[]).await; + + assert_eq!( + reply_status(&reply_header), + IggyError::InvalidCommand.as_code(), + "a bound session sending non-replicated code {code} must be denied InvalidCommand" + ); + assert!( + reply_body.is_empty(), + "a deny Reply carries an empty body, got {} bytes", + reply_body.len() + ); +} diff --git a/core/metadata/src/impls/metadata.rs b/core/metadata/src/impls/metadata.rs index 0c19eb1563..7f5b46580c 100644 --- a/core/metadata/src/impls/metadata.rs +++ b/core/metadata/src/impls/metadata.rs @@ -734,8 +734,13 @@ pub struct IggyMetadata { /// policy. superblock_write_failures: Cell, superblock_retry_after_micros: Cell, - /// State machine - lives on all shards - pub mux_stm: M, + /// State machine - lives on all shards. + /// + /// Shared so shard 0's bootstrap can keep a clone alive past every + /// fallible step that owns this struct: the peer shards read through + /// handles minted off this writer, and dropping it makes their + /// `LeftRight::read` panic. See `server/src/boot::shard_main`. + pub mux_stm: Rc, pub allocator: ConsensusGroupAllocator, /// Snapshot coordinator - present when persistent checkpointing is configured. pub coordinator: Option>, @@ -785,9 +790,10 @@ where journal: Option, snapshot: Option, superblock: Option>, - mux_stm: M, + mux_stm: impl Into>, data_dir: Option, ) -> Self { + let mux_stm = mux_stm.into(); let allocator = ConsensusGroupAllocator::new(mux_stm.streams().highest_partition_consensus_group_id()); let coordinator = data_dir.map(|dir| SnapshotCoordinator::new(dir, IggySnapshot::create)); @@ -2799,7 +2805,7 @@ where // Normal op: apply SM, commit_reply. `Err` is decode/corruption // only; a business rejection commits as a deterministic no-op // whose `code` rides the reply body, replayed on retry. - let apply = gated_apply(&self.mux_stm, prepare).unwrap_or_else(|err| { + let apply = gated_apply(&*self.mux_stm, prepare).unwrap_or_else(|err| { panic!( "on_ack: committed metadata op={} failed to apply: {err}", prepare_header.op @@ -3206,7 +3212,7 @@ where // (see the phantom-op comment at the call site). let client_table = self.client_table.borrow().to_snapshot(); let checksum = match coordinator.persist_snapshot( - &self.mux_stm, + &*self.mux_stm, snap_op, created_at, Some(client_table), @@ -3567,7 +3573,7 @@ where // table, while their state-machine effects still have to replay // (the snapshot sits at a lower op). apply_committed_prepare( - &self.mux_stm, + &*self.mux_stm, &self.client_table, self.client_table_mutation_allowed(header.op), |operation| self.fire_commit_notifier(operation), diff --git a/core/server/src/boot/mod.rs b/core/server/src/boot/mod.rs index 841b3e3ab0..62424a4d45 100644 --- a/core/server/src/boot/mod.rs +++ b/core/server/src/boot/mod.rs @@ -47,20 +47,20 @@ use crate::boot::listeners::{ make_replica_delegation_fns, make_shard_zero_client_accept_fns, start_tcp_runtime, }; use crate::boot::recovery::{ - RecoveredOwnerState, build_shard_for_thread, restore_metadata_consensus, + RecoveredOwnerState, ShardBuild, build_shard_for_thread, restore_metadata_consensus, }; use crate::boot::threads::{ - StopSignals, await_pump_drain, install_panic_hook, join_partial_shard_survivors, - resolve_shard_assignments, run_shard_thread, spawn_shutdown_watchdog, - validate_sharding_runtime_knobs, + PeerExitCountdown, PeerExitWait, ShutdownDeadline, StopSignals, await_pump_drain, + install_panic_hook, join_partial_shard_survivors, resolve_shard_assignments, run_shard_thread, + spawn_shutdown_watchdog, validate_sharding_runtime_knobs, }; use crate::boot::topology::{RosterCells, resolve_tcp_topology}; use crate::dispatch::partition::make_partition_read_handler; use crate::dispatch::session_ops::warm_dummy_password_hash; use crate::dispatch::submit::make_metadata_submit_handler; use crate::dispatch::{ - make_client_request_handler, make_deferred_client_request_handler, - make_deferred_replica_message_handler, make_list_clients_handler, + make_deferred_client_request_handler, make_deferred_replica_message_handler, + make_list_clients_handler, }; use crate::server_error::ServerError; use crate::session_manager::SessionManager; @@ -277,6 +277,12 @@ pub fn bootstrap( // thread body or in a task compio's `spawn` would swallow, escapes it. let first_panic = install_panic_hook(Arc::clone(&shutdown_flag)); let config = Arc::new(config); + // One post-shutdown budget for the whole process: the main thread's + // shard joins and shard 0's peer wait both count down this instant + // instead of each arming a full `shutdown_join_timeout`. + let shutdown_deadline = Arc::new(ShutdownDeadline::new( + config.system.sharding.shutdown_join_timeout.get_duration(), + )); // One owner table per server process, Arc-cloned into every shard's bus so // any shard's bus reads the same atomic slots that the owning // shard's installer / disconnect path writes. @@ -301,6 +307,11 @@ pub fn bootstrap( // count so a sender never blocks (each peer sends exactly once). let (ready_tx, ready_rx) = crossfire::mpmc::bounded_async::(metadata_peers); + // Shard 0 blocks on this at exit until every peer thread is done (see + // `PeerExitCountdown`). Sized to the real peer count, not the clamped + // channel capacity above: a single-shard server must not wait at all. + let peer_exit = Arc::new(PeerExitCountdown::new(shards_count.saturating_sub(1))); + let mut shard_threads: Vec<(u16, thread::JoinHandle>)> = Vec::with_capacity(shards_count); let roster_cells = RosterCells::default(); @@ -345,6 +356,8 @@ pub fn bootstrap( let roster_cells_for_shard = roster_cells.clone(); let shard_metrics_for_shard = shard_metrics_all.clone(); + let peer_exit_for_shard = Arc::clone(&peer_exit); + let shutdown_deadline_for_shard = Arc::clone(&shutdown_deadline); let handle = match thread::Builder::new() .name(format!("shard-{shard_id}")) .spawn(move || -> Result<(), ServerError> { @@ -363,6 +376,8 @@ pub fn bootstrap( owner_table_for_shard, roster_cells_for_shard, shard_metrics_for_shard, + peer_exit_for_shard, + shutdown_deadline_for_shard, ) }) { Ok(handle) => handle, @@ -379,10 +394,12 @@ pub fn bootstrap( drop(metadata_bundle_rx); drop(ready_tx); drop(ready_rx); - join_partial_shard_survivors( - shard_threads, - config.system.sharding.shutdown_join_timeout.get_duration(), - ); + // Shard 0 spawns first, so the vec holds it plus every peer + // that made it; the rest are peers shard 0's exit wait would + // otherwise sit out its whole budget for. + let spawned_peers = shard_threads.len().saturating_sub(1); + peer_exit.peers_never_spawned(shards_count.saturating_sub(1) - spawned_peers); + join_partial_shard_survivors(shard_threads, &shutdown_deadline); return Err(ServerError::ShardSpawnFailed { shard_id, source }); } }; @@ -406,7 +423,7 @@ pub fn bootstrap( Ok(ShardHandles { shutdown_flag, shard_threads, - join_timeout: config.system.sharding.shutdown_join_timeout.get_duration(), + deadline: shutdown_deadline, first_panic, }) } @@ -430,6 +447,8 @@ async fn shard_main( owner_table: Arc, roster_cells: RosterCells, shard_metrics_all: Vec, + peer_exit: Arc, + shutdown_deadline: Arc, ) -> Result<(), ServerError> { let topology = resolve_tcp_topology(config, replica_id)?; let bus = Rc::new(IggyMessageBus::with_config_and_owner_table( @@ -467,7 +486,12 @@ async fn shard_main( // metadata VSR; per-commit `publish()` (in `WriteCell::apply`) // bounds reader staleness to one op. let data_dir = Path::new(&config.system.path); - let (mux_stm, owner_state) = match metadata_handoff { + // The bundle broadcast is deliberately NOT inside the owner arm: it is + // the first moment a peer can hold a read handle over shard 0's writer, + // so the writer must first be parked in a binding that outlives the peer + // wait armed below. A `recover()` failure inside the arm is safe for the + // same reason -- no peer holds a handle yet. + let (mux_stm, pending_bundle_tx, owner_state) = match metadata_handoff { MetadataHandoff::Owner { bundle_tx } => { // Root is created locally at boot (never journaled), so replay // must start from the same baseline or every WAL-created user @@ -509,17 +533,9 @@ async fn shard_main( } } }); - broadcast_metadata_bundle( - shard_id, - &bundle_tx, - recovered.mux_stm.factory_bundle(), - total_shards.saturating_sub(1), - &shutdown_flag_for_handoff, - poll_interval, - ) - .await?; ( - recovered.mux_stm, + Rc::new(recovered.mux_stm), + Some(bundle_tx), Some(RecoveredOwnerState { journal: recovered.journal, snapshot: recovered.snapshot, @@ -540,10 +556,40 @@ async fn shard_main( poll_interval, ) .await?; - (ServerMuxStateMachine::from_factory_bundle(bundle), None) + ( + Rc::new(ServerMuxStateMachine::from_factory_bundle(bundle)), + None, + None, + ) } }; + // Shard 0 owns the metadata state machine's only write handle, and the + // peers read through it until their runtimes are gone. Declared after + // `mux_stm` so it drops first: every exit from here on, clean or `?`, + // waits for the peers before the write side goes. The `Rc` above is what + // makes that hold -- the handle no longer travels by value into the + // fallible shard build, which would drop it inside the callee. + let _peer_exit_wait = (shard_id == 0).then(|| { + PeerExitWait::new( + peer_exit, + Arc::clone(&shutdown_flag_for_handoff), + shutdown_deadline, + ) + }); + + if let Some(bundle_tx) = pending_bundle_tx { + broadcast_metadata_bundle( + shard_id, + &bundle_tx, + mux_stm.factory_bundle(), + total_shards.saturating_sub(1), + &shutdown_flag_for_handoff, + poll_interval, + ) + .await?; + } + // Metadata consensus + journal + snapshot live only on shard 0. // `IggyShard::tick_metadata` short-circuits when `consensus.is_none()`, // so peer shards have no caller that reads `journal` or `snapshot`. @@ -578,7 +624,7 @@ async fn shard_main( journal_for_metadata, snapshot_for_metadata, superblock_for_metadata, - mux_stm, + Rc::clone(&mux_stm), Some(PathBuf::from(&config.system.path)), ); // Size the VSR client table before listeners bind and any client registers. @@ -612,7 +658,12 @@ async fn shard_main( // Heap-pin like `run_shard_thread` pins `shard_main`: the builder future // carries the whole shard construction state machine and outgrew clippy's // `large_futures` cap; one allocation per shard startup. - let (shard, sessions) = Box::pin(build_shard_for_thread( + let ShardBuild { + shard, + sessions, + on_client_request, + shard_handle, + } = Box::pin(build_shard_for_thread( shard_id, total_shards, config, @@ -879,11 +930,15 @@ async fn shard_main( boot_view, shard.plane.metadata().client_table.borrow().client_ids(), ); - let on_client_request = make_client_request_handler( - &shard, - &sessions, - Arc::clone(&config.system), - config.personal_access_token.max_tokens_per_user, + // The request handler strands every frame until the weak + // self-reference is backfilled, so the build must have done that + // before the first listener binds. + debug_assert!( + shard_handle + .borrow() + .as_ref() + .is_some_and(|weak| weak.upgrade().is_some()), + "shard self-reference must be backfilled before listeners bind" ); let (accepted_replica, dialed_replica) = make_replica_delegation_fns(Rc::clone(&coord), &bus); diff --git a/core/server/src/boot/recovery.rs b/core/server/src/boot/recovery.rs index e614e688f5..81b4066222 100644 --- a/core/server/src/boot/recovery.rs +++ b/core/server/src/boot/recovery.rs @@ -23,7 +23,8 @@ use crate::partition_helpers::load_partition_or_fence; use crate::server_error::ServerError; use crate::session_manager::SessionManager; use crate::shell::{ - ServerMetadata, ServerShard, ShellHandlers, consensus_timers, repair_retry_ticks, + ServerMetadata, ServerShard, ShellHandlers, ShellShardHandle, consensus_timers, + repair_retry_ticks, }; use configs::server::ServerConfig; use consensus::{ @@ -35,6 +36,7 @@ use journal::Journal; use journal::prepare_journal::PrepareJournal; use journal::superblock::PingPongSuperblock; use message_bus::IggyMessageBus; +use message_bus::client_listener::RequestHandler; use metadata::impls::metadata::{IggySnapshot, StreamsFrontend}; use metadata::stm::snapshot::Snapshot; use partitions::{IggyPartitions, PartitionsConfig}; @@ -52,6 +54,18 @@ use std::sync::Arc; use std::time::Duration; use tracing::{info, warn}; +/// A shard built for its thread, with what `shard_main` wires after the +/// build: the session manager its request plane shares, the shard's one +/// client-request handler (shard 0 hands the same instance to its local +/// transports), and the weak self-reference the deferred handlers +/// upgrade per frame, already backfilled. +pub(in crate::boot) struct ShardBuild { + pub shard: Rc, + pub sessions: Rc>, + pub on_client_request: RequestHandler, + pub shard_handle: ShellShardHandle, PrepareJournal, IggySnapshot>, +} + #[allow(clippy::too_many_arguments, clippy::too_many_lines)] pub(in crate::boot) async fn build_shard_for_thread( shard_id: u16, @@ -65,7 +79,7 @@ pub(in crate::boot) async fn build_shard_for_thread( reply_inbox: ShardReceiver, metrics: ShardMetrics, roster_cells: &RosterCells, -) -> Result<(Rc, Rc>), ServerError> { +) -> Result { let shard_local_id = ShardId::new(shard_id); let total_partitions = metadata.mux_stm.streams().read(|inner| { inner @@ -226,7 +240,7 @@ pub(in crate::boot) async fn build_shard_for_thread( ShardIdentity::new(shard_id, shard_name), Rc::clone(&bus), on_replica_message, - on_client_request, + Rc::clone(&on_client_request), on_metadata_submit, on_list_clients, on_partition_read, @@ -272,7 +286,12 @@ pub(in crate::boot) async fn build_shard_for_thread( usize::try_from(config.message_bus.max_message_size.as_bytes_u64()).unwrap_or(usize::MAX), ); *shard_handle.borrow_mut() = Some(Rc::downgrade(&shard)); - Ok((shard, sessions)) + Ok(ShardBuild { + shard, + sessions, + on_client_request, + shard_handle, + }) } // Pin the configs-crate default literals (duplicated there to avoid a diff --git a/core/server/src/boot/threads.rs b/core/server/src/boot/threads.rs index 2dc1efd729..492cc436f1 100644 --- a/core/server/src/boot/threads.rs +++ b/core/server/src/boot/threads.rs @@ -25,7 +25,8 @@ use crate::shard_allocator::{ShardAllocator, ShardInfo}; use compio::runtime::ResumeUnwind; use configs::server::ServerConfig; use configs::sharding::{ - INBOX_CAPACITY_MAX, SHUTDOWN_DRAIN_TIMEOUT_MAX, SHUTDOWN_POLL_INTERVAL_MAX, + INBOX_CAPACITY_MAX, SHUTDOWN_DRAIN_TIMEOUT_MAX, SHUTDOWN_JOIN_TIMEOUT_MAX, + SHUTDOWN_POLL_INTERVAL_MAX, }; use message_bus::{IggyMessageBus, ReplicaOwnerTable}; use partitions::FatalCommit; @@ -35,7 +36,7 @@ use shard::{Receiver as ShardReceiver, Sender, ShardFrame, TaggedSender}; use std::backtrace::Backtrace; use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, OnceLock}; +use std::sync::{Arc, Condvar, Mutex, OnceLock, PoisonError}; use std::time::{Duration, Instant}; use std::{panic, thread}; use tracing::{error, info, warn}; @@ -45,12 +46,12 @@ use tracing::{error, info, warn}; /// Carries the cross-thread shutdown flag, one OS-thread `JoinHandle` /// per shard, and the first panic `install_panic_hook` recorded. The /// caller flips the flag via [`Self::install_ctrlc_handler`] and then -/// drains every shard via [`Self::join_all`], bounded by `join_timeout` -/// (`system.sharding.shutdown_join_timeout`). +/// drains every shard via [`Self::join_all`], bounded by the shared +/// `ShutdownDeadline` (`system.sharding.shutdown_join_timeout`). pub struct ShardHandles { pub(in crate::boot) shutdown_flag: Arc, pub(in crate::boot) shard_threads: Vec<(u16, thread::JoinHandle>)>, - pub(in crate::boot) join_timeout: Duration, + pub(in crate::boot) deadline: Arc, pub(in crate::boot) first_panic: Arc>, } @@ -106,30 +107,23 @@ impl ShardHandles { /// compio's `spawn` caught, which no thread result can carry. pub fn join_all(self) -> Result<(), ServerError> { let mut failures: Vec = Vec::new(); - // Armed on the first poll that observes the shutdown flag, shared - // across all shards: one budget covers the whole drain, not one - // budget per shard. - let mut deadline: Option = None; // Shards run thread-per-core with compio's blocking fallback pool // disabled, so an io_uring opcode the kernel lacks aborts every shard // with the same panic. Surface the actionable diagnostic once. let mut io_uring_diagnostic_shown = false; for (shard_id, handle) in self.shard_threads { - let Some(joined) = join_until_shutdown_deadline( - handle, - &self.shutdown_flag, - self.join_timeout, - &mut deadline, - ) else { + let Some(joined) = + join_until_shutdown_deadline(handle, &self.shutdown_flag, &self.deadline) + else { error!( shard_id, - waited = ?self.join_timeout, + waited = ?self.deadline.budget, "shard thread still running at the shutdown join deadline; abandoning it" ); failures.push(ShardJoinFailure { shard_id, kind: ShardJoinFailureKind::Wedged { - waited: self.join_timeout, + waited: self.deadline.budget, }, }); continue; @@ -217,28 +211,22 @@ pub(in crate::boot) fn install_panic_hook(shutdown_flag: Arc) -> Arc const JOIN_POLL_INTERVAL: Duration = Duration::from_millis(25); /// Join `handle`, waiting indefinitely while the server runs. The -/// `join_timeout` clock starts only when `shutdown_flag` is observed set -/// (arming the caller-shared `deadline` once, so all shards drain under -/// ONE budget); a running server parked here for hours must never be -/// mistaken for a wedged shard. `None` means the thread was still -/// running at the post-shutdown deadline and the handle was dropped -/// (the OS thread keeps running detached; process exit reaps it). -/// `JoinHandle` has no timed join, so this polls `is_finished` at +/// budget clock starts only when `shutdown_flag` is observed set (arming +/// the shared [`ShutdownDeadline`], so every shard join and shard 0's +/// peer wait drain under ONE budget); a running server parked here for +/// hours must never be mistaken for a wedged shard. `None` means the +/// thread was still running at the post-shutdown deadline and the handle +/// was dropped (the OS thread keeps running detached; process exit reaps +/// it). `JoinHandle` has no timed join, so this polls `is_finished` at /// [`JOIN_POLL_INTERVAL`]; the closing `join()` on a finished thread /// returns immediately. fn join_until_shutdown_deadline( handle: thread::JoinHandle>, shutdown_flag: &AtomicBool, - join_timeout: Duration, - deadline: &mut Option, + deadline: &ShutdownDeadline, ) -> Option>> { while !handle.is_finished() { - if deadline.is_none() && shutdown_flag.load(Ordering::Relaxed) { - *deadline = Some(Instant::now() + join_timeout); - } - if let Some(deadline) = deadline - && Instant::now() >= *deadline - { + if shutdown_flag.load(Ordering::Relaxed) && deadline.remaining().is_zero() { return None; } thread::sleep(JOIN_POLL_INTERVAL); @@ -273,9 +261,8 @@ fn panic_payload_to_string(payload: &(dyn std::any::Any + Send)) -> String { /// spawn error instead of hanging on a wedged shard. pub(in crate::boot) fn join_partial_shard_survivors( shard_threads: Vec<(u16, thread::JoinHandle>)>, - join_timeout: Duration, + deadline: &ShutdownDeadline, ) { - let deadline = Instant::now() + join_timeout; let mut remaining = shard_threads; loop { let mut still_running = Vec::with_capacity(remaining.len()); @@ -288,7 +275,7 @@ pub(in crate::boot) fn join_partial_shard_survivors( } } remaining = still_running; - if remaining.is_empty() || Instant::now() >= deadline { + if remaining.is_empty() || deadline.remaining().is_zero() { break; } thread::sleep(JOIN_POLL_INTERVAL); @@ -296,7 +283,7 @@ pub(in crate::boot) fn join_partial_shard_survivors( for (shard_id, _survivor) in remaining { error!( shard_id, - waited = ?join_timeout, + waited = ?deadline.budget, "survivor shard thread still running at the shutdown join deadline; abandoning it" ); } @@ -333,6 +320,173 @@ impl Drop for ShutdownOnDrop { } } +/// The single post-shutdown budget, shared by the main thread's shard +/// joins and shard 0's peer wait. +/// +/// Both waits are bounded by `system.sharding.shutdown_join_timeout` and +/// they NEST: shard 0 cannot start waiting for its peers until its own +/// drain returned, which is already inside the join budget. Arming one +/// instant on first use, whichever wait gets there first, keeps the two +/// inside one deadline instead of stacking two full budgets, so a +/// correct shutdown cannot report shard 0 as wedged. +pub(in crate::boot) struct ShutdownDeadline { + armed: OnceLock, + budget: Duration, +} + +impl ShutdownDeadline { + pub(in crate::boot) const fn new(budget: Duration) -> Self { + Self { + armed: OnceLock::new(), + budget, + } + } + + /// Time left in the shared budget, arming it on the first call. + /// Callers must only reach this once the shutdown flag is set: a + /// running server would otherwise start the clock. + fn remaining(&self) -> Duration { + self.armed + .get_or_init(|| Instant::now() + self.budget) + .saturating_duration_since(Instant::now()) + } +} + +/// Peer shards still running: each [`PeerExitGuard`] counts one out, +/// [`PeerExitWait`] blocks shard 0 until the count is zero. +/// +/// Shard 0 owns the metadata state machine's only write handle and every +/// peer reads through handles that stop working the moment it drops, so +/// the shard that owns the writer must outlive every reader on every exit +/// but one: [`PeerExitWait::drop`] skips the wait while shard 0 is +/// panicking, because parking an unwinding thread on a `Condvar` inside +/// `runtime.block_on` would stall the `io_uring` driver, and a panic in +/// shard 0's own pump can be exactly what the peers are blocked on. A +/// peer mid-read then panics too; the first panic is already recorded and +/// the process is going down either way. +pub(in crate::boot) struct PeerExitCountdown { + running: Mutex, + all_exited: Condvar, +} + +impl PeerExitCountdown { + pub(in crate::boot) const fn new(peers: usize) -> Self { + Self { + running: Mutex::new(peers), + all_exited: Condvar::new(), + } + } + + /// Block until every peer has counted itself out or `timeout` elapses. + /// `Err` carries the number of peers still running at the deadline. + fn wait(&self, timeout: Duration) -> Result<(), usize> { + let (guard, _) = self + .all_exited + .wait_timeout_while( + self.running.lock().unwrap_or_else(PoisonError::into_inner), + timeout, + |running| *running > 0, + ) + .unwrap_or_else(PoisonError::into_inner); + let running = *guard; + drop(guard); + if running == 0 { Ok(()) } else { Err(running) } + } + + fn peer_exited(&self) { + let mut running = self.running.lock().unwrap_or_else(PoisonError::into_inner); + *running = running.saturating_sub(1); + if *running == 0 { + self.all_exited.notify_all(); + } + } + + /// Count out peers that never spawned. The countdown is sized before the + /// spawn loop, so a failed `thread::Builder::spawn` leaves peers whose + /// [`PeerExitGuard`] will never exist: without this shard 0 waits out its + /// whole join budget for threads that were never there. + pub(in crate::boot) fn peers_never_spawned(&self, count: usize) { + for _ in 0..count { + self.peer_exited(); + } + } +} + +/// Counts one peer shard out of the [`PeerExitCountdown`] on drop. +/// +/// Held by `run_shard_thread` from before the runtime exists, so it drops +/// after the runtime and its tasks are gone: past that point nothing on +/// the peer's thread can still read shard 0's metadata. Drop runs on every +/// exit path, the error `?` returns and panic unwinds included. +struct PeerExitGuard { + countdown: Arc, +} + +impl PeerExitGuard { + const fn new(countdown: Arc) -> Self { + Self { countdown } + } +} + +impl Drop for PeerExitGuard { + fn drop(&mut self) { + self.countdown.peer_exited(); + } +} + +/// Shard 0's side of the [`PeerExitCountdown`]: blocks on drop until every +/// peer has exited, so whatever is declared before it outlives every +/// peer's reads. +/// +/// Flips the shutdown flag before waiting: a peer parked on its bus token +/// only starts its drain once the flag is set, and the thread-level +/// `ShutdownOnDrop` flips it only after `block_on` returns, which is after +/// this wait. Bounded by what is left of the [`ShutdownDeadline`] that +/// [`ShardHandles::join_all`] shares, with the same abandon-and-log +/// policy, so a wedged peer cannot hold shard 0 past the deadline the +/// main thread is itself counting down. +pub(in crate::boot) struct PeerExitWait { + countdown: Arc, + shutdown_flag: Arc, + deadline: Arc, +} + +impl PeerExitWait { + pub(in crate::boot) const fn new( + countdown: Arc, + shutdown_flag: Arc, + deadline: Arc, + ) -> Self { + Self { + countdown, + shutdown_flag, + deadline, + } + } +} + +impl Drop for PeerExitWait { + fn drop(&mut self) { + // The panic is the fault to report and `ShutdownOnDrop` still + // flips the flag for the peers; blocking an unwinding thread here + // would only delay it. + if thread::panicking() { + return; + } + self.shutdown_flag.store(true, Ordering::Relaxed); + let remaining = self.deadline.remaining(); + if let Err(peers_running) = self.countdown.wait(remaining) { + warn!( + peers_running, + waited = ?remaining, + budget = ?self.deadline.budget, + "peer shards still running at the shutdown join deadline; \ + releasing shard 0 anyway" + ); + } + } +} + /// Resolve the operator's `cpu_allocation` into concrete shard /// assignments plus the checked `u16` shard count. /// @@ -406,6 +560,22 @@ pub(in crate::boot) fn validate_sharding_runtime_knobs( drain: drain_timeout, }); } + let join_timeout = sharding.shutdown_join_timeout.get_duration(); + if join_timeout > SHUTDOWN_JOIN_TIMEOUT_MAX { + return Err(ServerError::InvalidShutdownJoinTimeout { + value: join_timeout, + max: SHUTDOWN_JOIN_TIMEOUT_MAX, + }); + } + // A join budget under the drain budget abandons shards mid-drain, + // interrupting the WAL fsync / replica drain, and now also cuts shard + // 0's peer wait short of the writer's last reader. + if join_timeout < drain_timeout { + return Err(ServerError::ShutdownJoinBelowDrain { + join: join_timeout, + drain: drain_timeout, + }); + } Ok(()) } @@ -427,11 +597,17 @@ pub(in crate::boot) fn run_shard_thread( owner_table: Arc, roster_cells: RosterCells, shard_metrics_all: Vec, + peer_exit: Arc, + shutdown_deadline: Arc, ) -> Result<(), ServerError> { // Armed for the whole thread body: a post-spawn error `?` or a panic // unwind here must flip `shutdown_flag` so sibling watchdogs drive // their bus shutdown instead of parking forever on `bus.token().wait()`. let mut shutdown_guard = ShutdownOnDrop::new(Arc::clone(&shutdown_flag)); + // Declared before the runtime so it drops after it: a peer counts + // itself out only once no task of its runtime can read shard 0's + // metadata any more. + let _peer_exit_guard = (shard_id != 0).then(|| PeerExitGuard::new(Arc::clone(&peer_exit))); assignment .bind_cpu() @@ -470,6 +646,8 @@ pub(in crate::boot) fn run_shard_thread( owner_table, roster_cells, shard_metrics_all, + peer_exit, + shutdown_deadline, )) .await }); @@ -649,6 +827,188 @@ mod tests { ); } + #[test] + fn peer_exit_countdown_releases_the_waiter_once_every_peer_is_out() { + let countdown = Arc::new(PeerExitCountdown::new(2)); + let guards: [PeerExitGuard; 2] = + std::array::from_fn(|_| PeerExitGuard::new(Arc::clone(&countdown))); + assert_eq!( + countdown.wait(Duration::from_millis(10)), + Err(2), + "two live peers must hold the waiter past a short deadline" + ); + let peers: Vec<_> = guards + .into_iter() + .map(|guard| { + thread::spawn(move || { + thread::sleep(Duration::from_millis(20)); + drop(guard); + }) + }) + .collect(); + assert_eq!( + countdown.wait(Duration::from_secs(30)), + Ok(()), + "the last guard drop must release the waiter" + ); + for peer in peers { + peer.join() + .expect("peer thread dropped its guard without panicking"); + } + } + + #[test] + fn peer_exit_guard_counts_out_during_unwind() { + let countdown = Arc::new(PeerExitCountdown::new(1)); + let guard = PeerExitGuard::new(Arc::clone(&countdown)); + // `resume_unwind` skips the panic hook, so the unwind is silent. + let unwound = panic::catch_unwind(panic::AssertUnwindSafe(|| { + let _guard = guard; + panic::resume_unwind(Box::new("peer shard body panicked")); + })); + assert!(unwound.is_err()); + assert_eq!( + countdown.wait(Duration::ZERO), + Ok(()), + "a guard dropped by a panic unwind must still count its peer out" + ); + } + + #[test] + fn peer_exit_wait_flips_the_flag_before_waiting() { + // Stands in for a peer parked on its bus token: it exits only once + // the shutdown flag is set, so a waiter that set the flag after + // waiting would sit out the whole budget. + let countdown = Arc::new(PeerExitCountdown::new(1)); + let shutdown_flag = Arc::new(AtomicBool::new(false)); + let peer = thread::spawn({ + let guard = PeerExitGuard::new(Arc::clone(&countdown)); + let shutdown_flag = Arc::clone(&shutdown_flag); + move || { + while !shutdown_flag.load(Ordering::Relaxed) { + thread::sleep(Duration::from_millis(1)); + } + drop(guard); + } + }); + let started = Instant::now(); + drop(PeerExitWait::new( + countdown, + Arc::clone(&shutdown_flag), + Arc::new(ShutdownDeadline::new(Duration::from_secs(30))), + )); + assert!( + started.elapsed() < Duration::from_secs(5), + "the waiter must not sit out its budget on a peer that waits for the flag" + ); + assert!(shutdown_flag.load(Ordering::Relaxed)); + peer.join() + .expect("peer thread dropped its guard without panicking"); + } + + #[test] + fn peer_exit_wait_gives_up_at_the_deadline() { + let countdown = Arc::new(PeerExitCountdown::new(1)); + let _wedged_peer = PeerExitGuard::new(Arc::clone(&countdown)); + let timeout = Duration::from_millis(50); + let started = Instant::now(); + drop(PeerExitWait::new( + countdown, + Arc::new(AtomicBool::new(false)), + Arc::new(ShutdownDeadline::new(timeout)), + )); + let waited = started.elapsed(); + assert!( + waited >= timeout && waited < Duration::from_secs(5), + "a wedged peer must be abandoned at the deadline, waited {waited:?}" + ); + } + + #[test] + fn peer_wait_and_shard_join_share_one_shutdown_budget() { + // Regression: the two waits nest (shard 0 cannot start waiting for + // its peers until its own drain returned, already inside the join + // budget). With a budget each, a clean shutdown reported shard 0 as + // wedged and exited non-zero. + let deadline = Arc::new(ShutdownDeadline::new(Duration::from_millis(200))); + let shutdown_flag = AtomicBool::new(true); + // Never finishes: stands in for the slow drain that arms and then + // spends the shared budget. The thread leaks into the test process. + let wedged_shard = thread::spawn(|| -> Result<(), ServerError> { + loop { + thread::sleep(Duration::from_secs(1)); + } + }); + assert!( + join_until_shutdown_deadline(wedged_shard, &shutdown_flag, &deadline).is_none(), + "the join must spend the budget it armed" + ); + + let countdown = Arc::new(PeerExitCountdown::new(1)); + let _wedged_peer = PeerExitGuard::new(Arc::clone(&countdown)); + let started = Instant::now(); + drop(PeerExitWait::new( + countdown, + Arc::new(AtomicBool::new(false)), + Arc::clone(&deadline), + )); + assert!( + started.elapsed() < Duration::from_millis(100), + "the peer wait must inherit what is left of the budget, not arm a second one" + ); + } + + #[test] + fn peer_exit_wait_ignores_peers_that_never_spawned() { + // A failed spawn leaves peers with no guard to count them out; the + // one that did spawn must still be the only thing the waiter waits on. + let countdown = Arc::new(PeerExitCountdown::new(3)); + countdown.peers_never_spawned(2); + let spawned = PeerExitGuard::new(Arc::clone(&countdown)); + assert_eq!( + countdown.wait(Duration::from_millis(10)), + Err(1), + "the peer that did spawn must still hold the waiter" + ); + drop(spawned); + assert_eq!(countdown.wait(Duration::ZERO), Ok(())); + } + + #[test] + fn peer_exit_wait_with_no_peers_returns_at_once() { + let started = Instant::now(); + drop(PeerExitWait::new( + Arc::new(PeerExitCountdown::new(0)), + Arc::new(AtomicBool::new(false)), + Arc::new(ShutdownDeadline::new(Duration::from_secs(30))), + )); + assert!( + started.elapsed() < Duration::from_secs(1), + "a single-shard server has nobody to wait for" + ); + } + + #[test] + fn peer_exit_wait_never_blocks_an_unwinding_thread() { + let countdown = Arc::new(PeerExitCountdown::new(1)); + let _still_running = PeerExitGuard::new(Arc::clone(&countdown)); + let wait = PeerExitWait::new( + countdown, + Arc::new(AtomicBool::new(false)), + Arc::new(ShutdownDeadline::new(Duration::from_secs(30))), + ); + let started = Instant::now(); + let unwound = panic::catch_unwind(panic::AssertUnwindSafe(|| { + let _wait = wait; + panic::resume_unwind(Box::new("shard 0 body panicked")); + })); + assert!(unwound.is_err()); + assert!( + started.elapsed() < Duration::from_secs(1), + "a waiter dropped during unwind must return without waiting" + ); + } + #[compio::test] async fn pump_drain_timeout_is_not_reported_as_clean() { let mut config = ServerConfig::default(); @@ -711,19 +1071,14 @@ mod tests { thread::sleep(Duration::from_millis(300)); Ok(()) }); - let mut deadline = None; - let joined = join_until_shutdown_deadline( - handle, - &shutdown_flag, - Duration::from_millis(20), - &mut deadline, - ); + let deadline = ShutdownDeadline::new(Duration::from_millis(20)); + let joined = join_until_shutdown_deadline(handle, &shutdown_flag, &deadline); assert!( matches!(joined, Some(Ok(Ok(())))), "a running server must be awaited indefinitely, not abandoned as wedged" ); assert!( - deadline.is_none(), + deadline.armed.get().is_none(), "the join deadline must not arm before the shutdown flag flips" ); } @@ -741,7 +1096,7 @@ mod tests { let handles = ShardHandles { shutdown_flag: Arc::new(AtomicBool::new(true)), shard_threads: vec![(0, handle)], - join_timeout: Duration::from_secs(1), + deadline: Arc::new(ShutdownDeadline::new(Duration::from_secs(1))), first_panic, }; let error = handles @@ -785,17 +1140,15 @@ mod tests { thread::sleep(Duration::from_secs(1)); } }); - let mut deadline = None; - let joined = join_until_shutdown_deadline( - handle, - &shutdown_flag, - Duration::from_millis(100), - &mut deadline, - ); + let deadline = ShutdownDeadline::new(Duration::from_millis(100)); + let joined = join_until_shutdown_deadline(handle, &shutdown_flag, &deadline); assert!( joined.is_none(), "a shard still running past the post-shutdown budget must be abandoned" ); - assert!(deadline.is_some(), "the deadline arms once the flag is set"); + assert!( + deadline.armed.get().is_some(), + "the deadline arms once the flag is set" + ); } } diff --git a/core/server/src/dispatch/authz.rs b/core/server/src/dispatch/authz.rs index 436aa2222e..5fddea7676 100644 --- a/core/server/src/dispatch/authz.rs +++ b/core/server/src/dispatch/authz.rs @@ -29,9 +29,10 @@ use std::rc::Rc; use consensus::MetadataHandle; use iggy_binary_protocol::codes::{ - DESCRIBE_OPTIONS_CODE, GET_CLUSTER_METADATA_CODE, GET_CONSUMER_GROUP_CODE, - GET_CONSUMER_GROUPS_CODE, GET_PERSONAL_ACCESS_TOKENS_CODE, GET_STATS_CODE, GET_STREAM_CODE, - GET_STREAMS_CODE, GET_TOPIC_CODE, GET_TOPICS_CODE, GET_USER_CODE, GET_USERS_CODE, + DESCRIBE_OPTIONS_CODE, FLUSH_UNSAVED_BUFFER_CODE, GET_CLUSTER_METADATA_CODE, + GET_CONSUMER_GROUP_CODE, GET_CONSUMER_GROUPS_CODE, GET_PERSONAL_ACCESS_TOKENS_CODE, + GET_STATS_CODE, GET_STREAM_CODE, GET_STREAMS_CODE, GET_TOPIC_CODE, GET_TOPICS_CODE, + GET_USER_CODE, GET_USERS_CODE, }; use iggy_binary_protocol::requests::consumer_groups::{ GetConsumerGroupRequest, GetConsumerGroupsRequest, @@ -39,20 +40,15 @@ use iggy_binary_protocol::requests::consumer_groups::{ use iggy_binary_protocol::requests::streams::GetStreamRequest; use iggy_binary_protocol::requests::topics::{GetTopicRequest, GetTopicsRequest}; use iggy_binary_protocol::requests::users::GetUserRequest; -use iggy_binary_protocol::{ - Operation, PrepareHeader, RoutedRequestHeader, WireDecode, WireIdentifier, -}; +use iggy_binary_protocol::{Operation, PrepareHeader, WireDecode, WireIdentifier, lookup_command}; use iggy_common::IggyError; use journal::superblock::SuperblockStore; use journal::{Journal, JournalHandle}; use metadata::impls::metadata::StreamsFrontend; use metadata::permissioner::Permissioner; use server_common::Message; -use tracing::warn; -use crate::responses::{ - build_deny_reply, current_metadata_commit, resolve_stream_id, resolve_topic_id, -}; +use crate::responses::{resolve_stream_id, resolve_topic_id}; use crate::shell::{ShellBus, ShellShard}; /// Authorize a partition-plane op on its resolved (stream, topic) for the @@ -132,74 +128,6 @@ where decision.err().map(|error| error.as_code()) } -/// Reply to a request rejected before it reached its plane with the request's -/// own frame: empty body + nonzero `status`. The nonzero status is the whole -/// point: the SDK peeks it and surfaces the typed error, whereas a status-0 -/// frame reads as a committed ack for work that never happened. Silence is no -/// better, the connection decodes replies in lockstep and would wedge on every -/// later request. -#[allow(clippy::future_not_send)] -pub(in crate::dispatch) async fn send_deny_reply( - shard: &Rc>, - transport_client_id: u128, - request_header: &RoutedRequestHeader, - status: u32, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let commit = current_metadata_commit(shard); - let reply = build_deny_reply(request_header, transport_client_id, 0, commit, status); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - status, - error = %error, - operation = ?request_header.operation, - "failed to surface request denial" - ); - } -} - -/// Deny a request from an unbound transport without disclosing the metadata -/// commit frontier. The status is the only field a pre-authenticated caller -/// needs, while the live commit would expose cluster write activity. -#[allow(clippy::future_not_send)] -pub(in crate::dispatch) async fn send_unbound_deny_reply( - shard: &Rc>, - transport_client_id: u128, - request_header: &RoutedRequestHeader, - status: u32, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let reply = build_deny_reply(request_header, transport_client_id, 0, 0, status); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - status, - error = %error, - operation = ?request_header.operation, - "failed to surface unbound request denial" - ); - } -} - /// Run an unscoped non-replicated-read rule for the acting user. A `None` user /// id (only the pre-auth path, which serves ungated codes) fails closed. pub(in crate::dispatch) fn authorize_uid( @@ -263,6 +191,11 @@ where /// topic]) against committed state first. The PAT list is self-scoped, so /// authentication is its whole rule, and `GET_CLUSTER_METADATA` -- which /// describes the private replica network -- is gated the same way. +/// +/// The gate is total over the protocol command table: every code the builder +/// serves has a named arm, and the tail refuses everything else (a replicated +/// code inside a `NonReplicated` header, a table-listed code with no arm, an +/// unknown code) instead of deferring it to the builder's catch-all. pub(in crate::dispatch) fn authorize_default_read( shard: &Rc>, code: u32, @@ -276,8 +209,8 @@ where S: 'static, SB: SuperblockStore + 'static, { - // A `u32` match cannot be exhaustive: every gated code is named explicitly, - // and the final arm is the ungated set the builder serves without a rule. + // A `u32` match cannot be exhaustive, so totality is by construction: + // every code with a decision is named, and the tail refuses the rest. match code { GET_STATS_CODE => authorize_uid(shard, user_id, Permissioner::get_stats), GET_USERS_CODE => authorize_uid(shard, user_id, Permissioner::get_users), @@ -328,46 +261,19 @@ where |request| (&request.stream_id, &request.topic_id), Permissioner::get_consumer_groups, ), - _ => Ok(()), - } -} - -/// Reply to a denied non-replicated read with the request's reply frame: empty -/// body + nonzero `status`. The SDK peeks the status before body decode and -/// surfaces the typed error, so a poll denial never reaches the empty-poll -/// "0 messages" body path. -#[allow(clippy::future_not_send)] -pub(in crate::dispatch) async fn send_non_replicated_deny( - shard: &Rc>, - request: &Message, - transport_client_id: u128, - status: u32, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let commit = current_metadata_commit(shard); - let reply = build_deny_reply( - request.header(), - request.header().client, - request.header().session, - commit, - status, - ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - status, - error = %error, - "failed to surface non-replicated authz denial" - ); + // No on-demand flush primitive exists. This arm is the only thing + // answering `FeatureUnavailable` for flush now: the HTTP path reaches + // the builder through `read_local`, whose call sites pass eleven fixed + // read codes, none of them flush. + FLUSH_UNSAVED_BUFFER_CODE => Err(IggyError::FeatureUnavailable), + // A replicated code smuggled inside a `NonReplicated` header keeps the + // builder's `FeatureUnavailable`; a table-listed code with no arm above + // and an unknown code are both refused as `InvalidCommand`. The builder + // refuses the same set, so a new caller cannot land on a fail-open. + _ => match lookup_command(code) { + Some(meta) if meta.is_replicated() => Err(IggyError::FeatureUnavailable), + _ => Err(IggyError::InvalidCommand), + }, } } @@ -508,3 +414,168 @@ where Some((stream_id, topic_id)) }) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::dispatch::test_support::{FIRST_BOOT, SpyBus, TestShard, test_shard}; + use iggy_binary_protocol::COMMAND_TABLE; + use iggy_binary_protocol::codes::{ + CREATE_STREAM_CODE, GET_CLIENT_CODE, GET_CLIENTS_CODE, GET_CONSUMER_OFFSET_CODE, + GET_ME_CODE, GET_SNAPSHOT_FILE_CODE, LOGIN_REGISTER_CODE, LOGIN_REGISTER_WITH_PAT_CODE, + LOGIN_USER_CODE, LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE, LOGOUT_USER_CODE, PING_CODE, + POLL_MESSAGES_CODE, SYNC_CONSUMER_GROUP_CODE, + }; + use iggy_common::defaults::DEFAULT_ROOT_USER_ID; + + /// A code no `COMMAND_TABLE` entry claims. + const UNKNOWN_CODE: u32 = 9999; + + /// The gate's answer as the wire sees it: status 0 allows, anything else + /// is the deny code stamped into `ReplyHeader.status`. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum Verdict { + Allow, + Deny(u32), + } + + /// Shard with root seeded, so the root column below exercises the + /// permissioner rules rather than a missing-user deny. + fn gate_shard() -> Rc { + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + shard + .plane + .metadata() + .mux_stm + .users() + .ensure_root_user("iggy", "hash"); + shard + } + + /// Gate a header-only frame (`body = &[]`) for `user_id`. + fn gate(shard: &Rc, code: u32, user_id: Option) -> Verdict { + match authorize_default_read(shard, code, &[], user_id) { + Ok(()) => Verdict::Allow, + Err(error) => Verdict::Deny(error.as_code()), + } + } + + /// Expected verdicts per non-replicated code for a header-only frame, as + /// `(code, unbound caller, root)`. Header-only means the identifier-scoped + /// arms never decode a body and defer to the builder's own error for both + /// callers. Codes the reads router serves on its own arms (`PING`, `GET_ME`, + /// `GET_CLIENTS`, ...) and the codes the classifier settles (the legacy + /// logins) never reach the gate, which refuses them like any other armless + /// code. + fn expected_header_only_verdicts() -> Vec<(u32, Verdict, Verdict)> { + let allow = Verdict::Allow; + let unauthenticated = Verdict::Deny(IggyError::Unauthenticated.as_code()); + let invalid_command = Verdict::Deny(IggyError::InvalidCommand.as_code()); + let feature_unavailable = Verdict::Deny(IggyError::FeatureUnavailable.as_code()); + vec![ + (PING_CODE, invalid_command, invalid_command), + (GET_STATS_CODE, unauthenticated, allow), + (GET_SNAPSHOT_FILE_CODE, invalid_command, invalid_command), + (GET_CLUSTER_METADATA_CODE, unauthenticated, allow), + (GET_ME_CODE, invalid_command, invalid_command), + (GET_CLIENT_CODE, invalid_command, invalid_command), + (GET_CLIENTS_CODE, invalid_command, invalid_command), + (GET_USER_CODE, allow, allow), + (GET_USERS_CODE, unauthenticated, allow), + (LOGIN_USER_CODE, invalid_command, invalid_command), + (LOGOUT_USER_CODE, invalid_command, invalid_command), + (LOGIN_REGISTER_CODE, invalid_command, invalid_command), + (GET_PERSONAL_ACCESS_TOKENS_CODE, unauthenticated, allow), + ( + LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE, + invalid_command, + invalid_command, + ), + (POLL_MESSAGES_CODE, invalid_command, invalid_command), + ( + FLUSH_UNSAVED_BUFFER_CODE, + feature_unavailable, + feature_unavailable, + ), + (GET_CONSUMER_OFFSET_CODE, invalid_command, invalid_command), + (GET_STREAM_CODE, allow, allow), + (GET_STREAMS_CODE, unauthenticated, allow), + (GET_TOPIC_CODE, allow, allow), + (GET_TOPICS_CODE, allow, allow), + (GET_CONSUMER_GROUP_CODE, allow, allow), + (GET_CONSUMER_GROUPS_CODE, allow, allow), + (SYNC_CONSUMER_GROUP_CODE, invalid_command, invalid_command), + ( + LOGIN_REGISTER_WITH_PAT_CODE, + invalid_command, + invalid_command, + ), + (DESCRIBE_OPTIONS_CODE, unauthenticated, allow), + ] + } + + /// Every non-replicated table entry has a named decision above. A new + /// entry without a row fails by name, so it cannot slip past the gate + /// unnoticed; a row without a table entry is stale and fails too. + #[test] + fn gate_is_total_over_the_command_table() { + let shard = gate_shard(); + let expected = expected_header_only_verdicts(); + for meta in COMMAND_TABLE.iter().filter(|meta| !meta.is_replicated()) { + let Some((_, unbound, root)) = expected.iter().find(|row| row.0 == meta.code) else { + panic!( + "non-replicated command {} ({}) has no ratchet row: decide it in \ + authorize_default_read and add the row", + meta.name, meta.code + ); + }; + assert_eq!( + gate(&shard, meta.code, None), + *unbound, + "{} ({}) from an unbound caller", + meta.name, + meta.code + ); + assert_eq!( + gate(&shard, meta.code, Some(DEFAULT_ROOT_USER_ID)), + *root, + "{} ({}) from root", + meta.name, + meta.code + ); + } + for (code, _, _) in &expected { + assert!( + lookup_command(*code).is_some_and(|meta| !meta.is_replicated()), + "ratchet row {code} names no non-replicated table entry" + ); + } + } + + #[test] + fn unknown_code_is_refused_as_invalid_command() { + assert!( + lookup_command(UNKNOWN_CODE).is_none(), + "test needs a code absent from COMMAND_TABLE" + ); + let shard = gate_shard(); + let invalid_command = Verdict::Deny(IggyError::InvalidCommand.as_code()); + assert_eq!(gate(&shard, UNKNOWN_CODE, None), invalid_command); + assert_eq!( + gate(&shard, UNKNOWN_CODE, Some(DEFAULT_ROOT_USER_ID)), + invalid_command + ); + } + + #[test] + fn replicated_code_in_a_non_replicated_frame_is_refused_as_feature_unavailable() { + let shard = gate_shard(); + let feature_unavailable = Verdict::Deny(IggyError::FeatureUnavailable.as_code()); + assert_eq!(gate(&shard, CREATE_STREAM_CODE, None), feature_unavailable); + assert_eq!( + gate(&shard, CREATE_STREAM_CODE, Some(DEFAULT_ROOT_USER_ID)), + feature_unavailable + ); + } +} diff --git a/core/server/src/dispatch/failure.rs b/core/server/src/dispatch/failure.rs new file mode 100644 index 0000000000..a6065c7c3d --- /dev/null +++ b/core/server/src/dispatch/failure.rs @@ -0,0 +1,677 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! The wire failure channels and the one send exit for host-built frames. +//! +//! Every frame the dispatch host builds, success or rejection, leaves through +//! [`send_host_frame`], so the send-failure log has one shape. Which channel a +//! failure rides is a wire contract with the SDK: +//! +//! | channel | carrier | when | +//! |---|---|---| +//! | [`FrameChannel::TypedDeny`] | Reply, nonzero status + empty body, or a result-framed rejection body | rejections that must unblock the SDK's lockstep request slot: checksum, authz, pre-consensus rewrite, unknown or unsupported non-replicated code, unbound non-PING read, transient replay hints | +//! | [`FrameChannel::Eviction`] | session-terminal Eviction frame with a typed reason | the client must register again: `NoSession`, `MalformedLogin`, heartbeat and login evictions. The reason rides the channel label, since one `context` covers four of them | +//! | [`FrameChannel::ResyncSentinel`] | status-0 poll reply, body carries `RESYNC_REQUIRED_PARTITION_SENTINEL` | a fenced consumer-group poll: the consumer must re-sync its assignment; HTTP mirrors it as `resync_required_polled_messages` in `crate::http::wire` | +//! | [`FrameChannel::EmptyFrame`] | status-0 fail-fast body, empty or the 16-byte empty poll | the partition cannot answer yet; the SDK fails fast (empty poll) and retries. A permanent client error never rides this channel: an undecodable body denies typed, because there is nothing to retry | +//! | [`FrameChannel::Reply`] | status-0 success frame | host-built success replies: login/register, ping, logout, non-replicated read bodies, committed metadata replies | +//! | silent drop | no frame | one deliberate case, a transient consensus submit failure: the SDK read-timeout replays the same request id, and a synthesized failure could contradict a write that commits moments later. A header `RequestHeader::validate` rejected also drops, but that one is a GAP, not a contract - the fields decode, so a deny could be echoed under the transport id, and the client instead waits out its read timeout | +//! | HTTP status | HTTP status code | the HTTP spine maps the same rejections in `crate::http::error`; it never rides these frames | +//! +//! The last two send nothing, so [`FrameChannel`] has no variant for them. +//! +//! Scope: the table covers `crate::dispatch` only. Two neighbours answer on +//! their own paths by design - the partitions engine builds and sends +//! produce/poll replies, and the shard crate builds client-shaped denies of +//! its own (`IggyShard::deny_partition_request_transient` and +//! `stage_transient_deny`, both `TypedDeny`-shaped, the latter shedding the +//! frame outright when its lifecycle queue is full). + +use crate::responses::{ + NonReplicatedResponse, build_deny_reply, build_empty_reply, current_metadata_commit, +}; +use crate::rewrite::RewriteStage; +use crate::shell::{ShellBus, ShellShard}; +use bytes::Bytes; +use consensus::{ + EvictionContext, MetadataHandle, build_eviction_message, + build_incompatible_protocol_eviction_message, build_result_rejection_reply, +}; +use iggy_binary_protocol::{EvictionReason, PrepareHeader, RoutedRequestHeader}; +use iggy_common::IggyError; +use journal::superblock::SuperblockStore; +use journal::{Journal, JournalHandle}; +use message_bus::BusMessage; +use server_common::Message; +use std::rc::Rc; +use tracing::warn; + +/// Labels the channel a host-built frame rides, for the send-failure log. +/// The taxonomy, including the two channels that never construct a frame, +/// is on the module doc. +#[derive(Clone, Copy, Debug)] +pub(in crate::dispatch) enum FrameChannel { + TypedDeny, + /// The reason travels with the channel: five call sites share the + /// `"login_rejection"` context across four distinct reasons, so + /// `context` alone cannot tell a `MalformedLogin` send failure from an + /// `InvalidCredentials` one. + Eviction(EvictionReason), + ResyncSentinel, + EmptyFrame, + Reply, +} + +impl std::fmt::Display for FrameChannel { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TypedDeny => formatter.write_str("typed_deny"), + Self::Eviction(reason) => write!(formatter, "eviction({reason:?})"), + Self::ResyncSentinel => formatter.write_str("resync_sentinel"), + Self::EmptyFrame => formatter.write_str("empty_frame"), + Self::Reply => formatter.write_str("reply"), + } + } +} + +/// The one send exit for host-built client frames. Best-effort: a failed +/// send means the connection is gone (or its queue is full), and there is +/// nothing left to reply on, so the error is logged and dropped. `frame` is +/// any bus message: a contiguous frozen frame or the vectored poll reply. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_host_frame( + bus: &B, + transport_client_id: u128, + frame: impl Into, + channel: FrameChannel, + context: &'static str, +) { + if let Err(send_error) = bus.send_to_client(transport_client_id, frame).await { + warn!( + transport_client_id, + error = %send_error, + channel = %channel, + context, + "failed to send host frame to client" + ); + } +} + +/// Reply to a request rejected before it reached its plane with the request's +/// own frame: empty body + nonzero `status`. The nonzero status is the whole +/// point: the SDK peeks it and surfaces the typed error, whereas a status-0 +/// frame reads as a committed ack for work that never happened. Silence is no +/// better, the connection decodes replies in lockstep and would wedge on every +/// later request. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_deny_reply( + shard: &Rc>, + transport_client_id: u128, + request_header: &RoutedRequestHeader, + status: u32, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let commit = current_metadata_commit(shard); + let reply = build_deny_reply(request_header, transport_client_id, 0, commit, status); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + "request_denial", + ) + .await; +} + +/// Deny a request from an unbound transport without disclosing the metadata +/// commit frontier. The status is the only field a pre-authenticated caller +/// needs, while the live commit would expose cluster write activity. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_unbound_deny_reply( + shard: &Rc>, + transport_client_id: u128, + request_header: &RoutedRequestHeader, + status: u32, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let reply = build_deny_reply(request_header, transport_client_id, 0, 0, status); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + "unbound_request_denial", + ) + .await; +} + +/// Reply to a denied non-replicated read with the request's reply frame: empty +/// body + nonzero `status`. The SDK peeks the status before body decode and +/// surfaces the typed error, so a poll denial never reaches the empty-poll +/// "0 messages" body path. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_non_replicated_deny( + shard: &Rc>, + request: &Message, + transport_client_id: u128, + status: u32, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let commit = current_metadata_commit(shard); + let reply = build_deny_reply( + request.header(), + request.header().client, + request.header().session, + commit, + status, + ); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + "non_replicated_denial", + ) + .await; +} + +/// Reject a request before it reaches consensus: warn, then send the typed +/// deny reply. A silent drop would wedge every later request on the +/// connection until the socket read timeout. `stage` names the chain step +/// for both log lines, so the set stays enumerable. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_pre_consensus_deny( + shard: &Rc>, + transport_client_id: u128, + request_header: &RoutedRequestHeader, + error: &IggyError, + stage: RewriteStage, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let context = stage.as_str(); + warn!( + transport_client_id, + error = %error, + operation = ?request_header.operation, + context, + "denying request pre-consensus" + ); + let commit = current_metadata_commit(shard); + let reply = build_deny_reply( + request_header, + transport_client_id, + 0, + commit, + error.as_code(), + ); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + context, + ) + .await; +} + +/// Result-framed rejection Reply: status 0, body `[count=1][index][code]`. +/// The SDK decodes the nonzero result code, so a transient code makes it +/// replay the same request at once instead of waiting out its read timeout. +/// Replying empty instead would surface as a hard `InvalidFormat` decode +/// failure and break the replay. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_result_rejection( + shard: &Rc>, + transport_client_id: u128, + request_header: &RoutedRequestHeader, + error: &IggyError, + context: &'static str, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let commit = current_metadata_commit(shard); + let reply = build_result_rejection_reply(request_header, commit, error.as_code()); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + context, + ) + .await; +} + +/// Best-effort session-terminal `Eviction` frame: the client's session is +/// gone (or was never granted), so it must register again. Every frame +/// transport decodes `Command::Eviction` and maps the typed reason +/// (`NoSession` -> `Unauthenticated`, ...), so clients fail fast with the +/// real cause instead of a body-decode failure or a timeout. Consensus +/// context (cluster/view/replica) is stamped on the metadata shard and +/// zeroed elsewhere; the SDK only reads the reason, plus the protocol +/// window on `IncompatibleProtocol`. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_eviction( + shard: &Rc>, + transport_client_id: u128, + vsr_client_id: u128, + reason: EvictionReason, + context: &'static str, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let ctx = shard.plane.metadata().consensus.as_ref().map_or( + EvictionContext { + cluster: 0, + view: 0, + replica: 0, + }, + EvictionContext::from_consensus, + ); + let eviction = match reason { + EvictionReason::IncompatibleProtocol => { + build_incompatible_protocol_eviction_message(ctx, vsr_client_id) + } + _ => build_eviction_message(ctx, vsr_client_id, reason), + }; + send_host_frame( + &shard.bus, + transport_client_id, + eviction.into_generic().into_frozen(), + FrameChannel::Eviction(reason), + context, + ) + .await; +} + +/// Send a non-replicated reply body to a client, stamping the current +/// metadata commit. Shared by the non-replicated read arms; `channel` +/// labels the body shape (a real answer, a fail-fast empty poll, or the +/// re-sync sentinel) for the send-failure log. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_non_replicated_bytes( + shard: &Rc>, + request: &Message, + transport_client_id: u128, + bytes: Bytes, + channel: FrameChannel, + context: &'static str, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let commit = current_metadata_commit(shard); + let reply = NonReplicatedResponse::Bytes(bytes).into_reply( + request.header(), + request.header().client, + request.header().session, + commit, + ); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + channel, + context, + ) + .await; +} + +/// Ack a consumer-offset op whose body could not be rewritten for the +/// partition plane with an empty Reply. The SDK connection processes replies +/// in lockstep, so a silent drop wedges every subsequent request on that +/// connection. +#[allow(clippy::future_not_send)] +pub(in crate::dispatch) async fn send_empty_partition_reply( + shard: &Rc>, + transport_client_id: u128, + request_header: &RoutedRequestHeader, +) where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let commit = current_metadata_commit(shard); + let reply = build_empty_reply(request_header, transport_client_id, 0, commit); + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::EmptyFrame, + "empty_partition_reply", + ) + .await; +} + +// Byte snapshots pinning each channel's frame to the pre-refactor inline +// construction. What they hold is that routing a rejection through this +// module changed no byte a client sees: same command, same status, same +// header echo, same body length per channel. They are NOT the wire +// contract - a deliberate protocol change updates them. +#[cfg(test)] +mod tests { + use super::*; + use crate::dispatch::handle_client_request; + use crate::dispatch::test_support::{ + FIRST_BOOT, SpyBus, TestShard, request_message, test_shard, + }; + use crate::session_manager::SessionManager; + use configs::server::ServerSystemConfig; + use iggy_binary_protocol::Operation; + use iggy_binary_protocol::codes::PING_CODE; + use iggy_common::RESYNC_REQUIRED_PARTITION_SENTINEL; + use std::cell::RefCell; + use std::sync::Arc; + + const TRANSPORT: u128 = 42; + const VSR_CLIENT: u128 = 7; + const SESSION: u64 = 3; + const REQUEST: u64 = 5; + + fn snapshot_shard() -> (SpyBus, Rc) { + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + (bus, shard) + } + + fn sole_client_frame(bus: &SpyBus) -> (u128, Vec) { + let replies = bus.client_replies.borrow(); + assert_eq!(replies.len(), 1, "expected exactly one client-bound frame"); + replies[0].clone() + } + + fn poll_request() -> Message { + request_message(Operation::NonReplicated, VSR_CLIENT, SESSION, REQUEST, &[]) + } + + /// The 16-byte empty `PolledMessages` body exactly as the pre-refactor + /// `partition::empty_polled_messages_body` built it. + fn old_empty_polled_messages_body(partition_id: u32) -> Bytes { + let mut body = Vec::with_capacity(16); + body.extend_from_slice(&partition_id.to_le_bytes()); + body.extend_from_slice(&0u64.to_le_bytes()); + body.extend_from_slice(&0u32.to_le_bytes()); + Bytes::from(body) + } + + fn old_eviction_context(shard: &Rc) -> EvictionContext { + shard.plane.metadata().consensus.as_ref().map_or( + EvictionContext { + cluster: 0, + view: 0, + replica: 0, + }, + EvictionContext::from_consensus, + ) + } + + #[compio::test] + async fn snapshot_typed_deny_commit_stamped_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let status = IggyError::Unauthenticated.as_code(); + let old = build_deny_reply( + request.header(), + TRANSPORT, + 0, + current_metadata_commit(&shard), + status, + ) + .into_generic(); + + send_deny_reply(&shard, TRANSPORT, request.header(), status).await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + #[compio::test] + async fn snapshot_typed_deny_unbound_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let status = IggyError::Unauthenticated.as_code(); + let old = build_deny_reply(request.header(), TRANSPORT, 0, 0, status).into_generic(); + + send_unbound_deny_reply(&shard, TRANSPORT, request.header(), status).await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + #[compio::test] + async fn snapshot_typed_deny_non_replicated_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let status = IggyError::Unauthenticated.as_code(); + let old = build_deny_reply( + request.header(), + request.header().client, + request.header().session, + current_metadata_commit(&shard), + status, + ) + .into_generic(); + + send_non_replicated_deny(&shard, &request, TRANSPORT, status).await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + #[compio::test] + async fn snapshot_typed_deny_result_framed_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let code = IggyError::TransientNotAccepted; + let old = build_result_rejection_reply( + request.header(), + current_metadata_commit(&shard), + code.as_code(), + ) + .into_generic(); + + send_result_rejection(&shard, TRANSPORT, request.header(), &code, "snapshot").await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + #[compio::test] + async fn snapshot_eviction_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let ctx = old_eviction_context(&shard); + + // Old `send_unauthenticated_eviction`: reason NoSession, client id = + // the transport id. + let old = build_eviction_message(ctx, TRANSPORT, EvictionReason::NoSession).into_generic(); + send_eviction( + &shard, + TRANSPORT, + TRANSPORT, + EvictionReason::NoSession, + "snapshot", + ) + .await; + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + bus.client_replies.borrow_mut().clear(); + + // Old `send_login_eviction`: reason MalformedLogin, client id = the + // request's VSR client id. + let old = + build_eviction_message(ctx, VSR_CLIENT, EvictionReason::MalformedLogin).into_generic(); + send_eviction( + &shard, + TRANSPORT, + VSR_CLIENT, + EvictionReason::MalformedLogin, + "snapshot", + ) + .await; + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + bus.client_replies.borrow_mut().clear(); + + // Old `send_login_eviction` on `IncompatibleProtocol`: the protocol + // window rides the frame, client id = the request's VSR client id. + let old = build_incompatible_protocol_eviction_message(ctx, VSR_CLIENT).into_generic(); + send_eviction( + &shard, + TRANSPORT, + VSR_CLIENT, + EvictionReason::IncompatibleProtocol, + "snapshot", + ) + .await; + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + /// The HTTP mirror (`resync_required_polled_messages` in + /// `crate::http::wire`) is a JSON DTO, not a wire frame, so the shared + /// contract asserted here is the sentinel constant itself; the DTO's own + /// test pins its `partition_id` to the same constant. + #[compio::test] + async fn snapshot_resync_sentinel_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let body = old_empty_polled_messages_body(RESYNC_REQUIRED_PARTITION_SENTINEL); + assert_eq!( + body[..4], + RESYNC_REQUIRED_PARTITION_SENTINEL.to_le_bytes(), + "sentinel poll body must lead with the re-sync sentinel partition id" + ); + let old = NonReplicatedResponse::Bytes(body.clone()) + .into_reply( + request.header(), + request.header().client, + request.header().session, + current_metadata_commit(&shard), + ) + .into_generic(); + + send_non_replicated_bytes( + &shard, + &request, + TRANSPORT, + body, + FrameChannel::ResyncSentinel, + "poll_messages", + ) + .await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + #[compio::test] + async fn snapshot_empty_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let request = poll_request(); + let old = build_empty_reply( + request.header(), + TRANSPORT, + 0, + current_metadata_commit(&shard), + ) + .into_generic(); + + send_empty_partition_reply(&shard, TRANSPORT, request.header()).await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } + + /// The PING reply is the one host-built success Reply the funnel serves + /// without a consensus round; the old side is the frame the reads + /// router's PING arm built inline before the exit existed. + #[compio::test] + async fn snapshot_reply_frame_unchanged() { + let (bus, shard) = snapshot_shard(); + let sessions = Rc::new(RefCell::new(SessionManager::new())); + let system_config = Arc::new(ServerSystemConfig::default()); + let request = request_message(Operation::NonReplicated, VSR_CLIENT, SESSION, REQUEST, &[]) + .transmute_header(|header, ping: &mut RoutedRequestHeader| { + *ping = header; + ping.reserved[..4].copy_from_slice(&PING_CODE.to_le_bytes()); + // The funnel promotes the client wire header with `group` + // unset, so the old side must build from the same bytes. + ping.group = 0; + }); + let old = build_empty_reply( + request.header(), + request.header().client, + request.header().session, + current_metadata_commit(&shard), + ) + .into_generic(); + + handle_client_request( + &shard, + &sessions, + &system_config, + 1, + TRANSPORT, + request.into_generic(), + ) + .await; + + let (target, frame) = sole_client_frame(&bus); + assert_eq!(target, TRANSPORT); + assert_eq!(frame, old.as_slice().to_vec()); + } +} diff --git a/core/server/src/dispatch/mod.rs b/core/server/src/dispatch/mod.rs index 8f69e7886e..ed8c8660b0 100644 --- a/core/server/src/dispatch/mod.rs +++ b/core/server/src/dispatch/mod.rs @@ -20,7 +20,9 @@ //! The tree: [`session_ops`] (login/register/logout and their replica //! forwards), [`partition`] (the partition data plane, both mesh ends), //! [`reads`] (the non-replicated read router), [`submit`] (the shard-0 -//! metadata-submit RPC), `authz` (the wire-path authorization gates). +//! metadata-submit RPC), `authz` (the wire-path authorization gates), +//! `failure` (the wire failure channels and the one send exit for +//! host-built frames). //! //! Deliberate asymmetry (the two authz gates): replicated metadata ops are //! authorized in-apply by the STM, in committed order on every replica; @@ -30,6 +32,7 @@ //! error contract (404-before-403) is pinned client-visible behavior. mod authz; +mod failure; pub mod login_error; pub mod partition; mod reads; @@ -39,105 +42,45 @@ pub mod submit; mod test_support; use crate::consumer_group::maybe_rewrite_consumer_group_request; -use crate::dispatch::authz::{send_deny_reply, send_unbound_deny_reply}; +use crate::dispatch::failure::{ + FrameChannel, send_deny_reply, send_eviction, send_host_frame, send_pre_consensus_deny, + send_unbound_deny_reply, +}; use crate::dispatch::partition::{dispatch_partition_request, handle_delete_segments_request}; use crate::dispatch::reads::handle_non_replicated_request; use crate::dispatch::session_ops::{ - handle_login_register_request, handle_logout_request, send_login_eviction, - send_unauthenticated_eviction, submit_disconnect_logout, + handle_login_register_request, handle_logout_request, submit_disconnect_logout, }; use crate::dispatch::submit::submit_client_request_on_owner; -use crate::pat::maybe_rewrite_pat_request; -use crate::responses::{ - NonReplicatedResponse, build_deny_reply, build_raw_pat_reply, current_metadata_commit, -}; -use crate::segment_cleaner::UNENFORCEABLE_TOPIC_SIZE_WARN; -use crate::session_manager::SessionManager; +use crate::responses::build_raw_pat_reply; +use crate::rewrite::{RewriteDeny, RewriteStage, tcp_chain}; +use crate::session_manager::{ConnectionContext, SessionManager}; use crate::shell::{ShellBus, ShellShard, ShellShardHandle}; -use crate::users::maybe_rewrite_user_password_request; -use crate::wire::{request_body, verify_request_checksum}; -use bytes::Bytes; +use crate::wire::verify_request_checksum; +use ahash::{AHashMap, AHashSet}; use configs::server::ServerSystemConfig; -use consensus::MetadataHandle; use iggy_binary_protocol::PrepareHeader; use iggy_binary_protocol::codes::{ - GET_CLUSTER_METADATA_CODE, LOGIN_USER_CODE, LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE, PING_CODE, + LOGIN_USER_CODE, LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE, PING_CODE, }; -use iggy_binary_protocol::requests::partitions::{ - CreatePartitionsRequest, DeletePartitionsRequest, -}; -use iggy_binary_protocol::requests::streams::{CreateStreamRequest, UpdateStreamRequest}; -use iggy_binary_protocol::requests::topics::{CreateTopicRequest, UpdateTopicRequest}; -use iggy_binary_protocol::requests::users::{CreateUserRequest, UpdateUserRequest}; use iggy_binary_protocol::{ - EvictionReason, GenericHeader, MAX_PARTITIONS_PER_REQUEST, Operation, RequestHeader, - RoutedRequestHeader, WireDecode, WireIdentifier, WireOptions, -}; -use iggy_common::{ - IggyByteSize, IggyError, MaxTopicSize, TopicCreateOptions, UPDATABLE_STREAM_OPTION_KEYS, - UPDATABLE_TOPIC_OPTION_KEYS, UPDATABLE_USER_OPTION_KEYS, validate_preallocated_topic_bytes, - validate_topic_segment_size, + EvictionReason, GenericHeader, Operation, RequestHeader, RoutedRequestHeader, }; +use iggy_common::IggyError; use journal::superblock::SuperblockStore; use journal::{Journal, JournalHandle}; -use message_bus::BusMessage; use message_bus::client_listener::RequestHandler; use message_bus::replica::listener::MessageHandler; -use metadata::impls::metadata::StreamsFrontend; -use metadata::stm::stream::Streams; use server_common::Message; use shard::{ConnectedClientInfo, ListClientsHandler}; use std::cell::RefCell; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::VecDeque; use std::rc::Rc; use std::sync::Arc; -use tracing::{debug, warn}; +use tracing::warn; -type ClientRequestQueues = Rc>>>>; -type ActiveClientRequests = Rc>>; - -pub fn make_client_request_handler( - shard: &Rc>, - sessions: &Rc>, - system_config: Arc, - max_tokens_per_user: u32, -) -> RequestHandler -where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let shard = Rc::clone(shard); - let sessions = Rc::clone(sessions); - let queues: ClientRequestQueues = Rc::new(RefCell::new(HashMap::new())); - let active: ActiveClientRequests = Rc::new(RefCell::new(HashSet::new())); - let sessions_for_disconnect = Rc::clone(&sessions); - let shard_for_disconnect = Rc::clone(&shard); - shard - .bus - .set_client_connection_lost_fn(Rc::new(move |client_id| { - if let Some((vsr_client_id, session)) = sessions_for_disconnect - .borrow_mut() - .remove_connection(client_id) - { - submit_disconnect_logout(Rc::clone(&shard_for_disconnect), vsr_client_id, session); - } - })); - Rc::new(move |client_id, message| { - enqueue_client_request( - Rc::clone(&shard), - Rc::clone(&sessions), - Arc::clone(&system_config), - max_tokens_per_user, - Rc::clone(&queues), - Rc::clone(&active), - client_id, - message, - ); - }) -} +type ClientRequestQueues = Rc>>>>; +type ActiveClientRequests = Rc>>; /// Build the per-shard [`ListClientsHandler`]: on a `ListClients` /// broadcast, serialize this shard's locally-homed connected clients from @@ -172,6 +115,12 @@ where }) } +/// Build the shard's one client-request handler: per-client FIFO queues +/// drained one task per client, and the bus connection-lost hook that +/// logs a dropped connection out. Every transport on the shard must +/// share the instance (shard 0 hands it to its local QUIC, TCP-TLS and +/// WSS listeners as well), or a client's ordering guarantee and the +/// disconnect hook split by transport. pub fn make_deferred_client_request_handler( bus: &B, shard_handle: &ShellShardHandle, @@ -188,50 +137,46 @@ where { let shard_handle = Rc::clone(shard_handle); let sessions = Rc::clone(sessions); - let queues: ClientRequestQueues = Rc::new(RefCell::new(HashMap::new())); - let active: ActiveClientRequests = Rc::new(RefCell::new(HashSet::new())); + let queues: ClientRequestQueues = Rc::new(RefCell::new(AHashMap::new())); + let active: ActiveClientRequests = Rc::new(RefCell::new(AHashSet::new())); + let queues_for_disconnect = Rc::clone(&queues); + let active_for_disconnect = Rc::clone(&active); let sessions_for_disconnect = Rc::clone(&sessions); let shard_handle_for_disconnect = Rc::clone(&shard_handle); let bus_for_spawn = (*bus).clone(); bus.set_client_connection_lost_fn(Rc::new(move |client_id| { - if let Some((vsr_client_id, session)) = sessions_for_disconnect - .borrow_mut() - .remove_connection(client_id) - && let Some(shard) = upgrade_shard_handle(&shard_handle_for_disconnect) + // The socket is gone: nothing will drain what is still queued and no + // later frame will release the active slot, so both entries go here. + // This is also the only recovery from a panic inside the drain task + // (compio catches it), which would otherwise leave the slot taken and + // every later frame for the client queued forever. + queues_for_disconnect.borrow_mut().remove(&client_id); + active_for_disconnect.borrow_mut().remove(&client_id); + // Upgrade FIRST: `remove_connection` strips the `SessionManager` + // entry, so running it ahead of a failed upgrade would drop the + // binding without ever submitting the replicated `Logout`, leaking + // the `ClientTable` entry and its consumer-group memberships. The + // window is pre-build / post-runtime-drop only. + if let Some(shard) = upgrade_shard_handle(&shard_handle_for_disconnect) + && let Some((vsr_client_id, session)) = sessions_for_disconnect + .borrow_mut() + .remove_connection(client_id) { submit_disconnect_logout(shard, vsr_client_id, session); } })); Rc::new(move |client_id, message| { - let shard_handle = Rc::clone(&shard_handle); - let sessions = Rc::clone(&sessions); - let system_config = Arc::clone(&system_config); - let queues = Rc::clone(&queues); - let active = Rc::clone(&active); - queues - .borrow_mut() - .entry(client_id) - .or_default() - .push_back(message); - if !active.borrow_mut().insert(client_id) { - return; - } - bus_for_spawn.spawn(async move { - let Some(shard) = upgrade_shard_handle(&shard_handle) else { - active.borrow_mut().remove(&client_id); - return; - }; - drain_client_requests( - shard, - sessions, - system_config, - max_tokens_per_user, - queues, - active, - client_id, - ) - .await; - }); + enqueue_client_request( + &bus_for_spawn, + &shard_handle, + &sessions, + &system_config, + max_tokens_per_user, + &queues, + &active, + client_id, + message, + ); }) } @@ -275,12 +220,13 @@ where #[allow(clippy::too_many_arguments)] fn enqueue_client_request( - shard: Rc>, - sessions: Rc>, - system_config: Arc, + bus: &B, + shard_handle: &ShellShardHandle, + sessions: &Rc>, + system_config: &Arc, max_tokens_per_user: u32, - queues: ClientRequestQueues, - active: ActiveClientRequests, + queues: &ClientRequestQueues, + active: &ActiveClientRequests, client_id: u128, message: Message, ) where @@ -299,8 +245,19 @@ fn enqueue_client_request( return; } - let bus = shard.bus.clone(); + let shard_handle = Rc::clone(shard_handle); + let sessions = Rc::clone(sessions); + let system_config = Arc::clone(system_config); + let queues = Rc::clone(queues); + let active = Rc::clone(active); bus.spawn(async move { + // The handle is set once the shard is built. A frame that beats + // it stays queued and the client's next frame drains both, so the + // active slot must be released here or that next frame never spawns. + let Some(shard) = upgrade_shard_handle(&shard_handle) else { + active.borrow_mut().remove(&client_id); + return; + }; drain_client_requests( shard, sessions, @@ -351,208 +308,106 @@ fn pop_next_client_request( active: &ActiveClientRequests, client_id: u128, ) -> Option> { - let mut queues = queues.borrow_mut(); - let Some(queue) = queues.get_mut(&client_id) else { - active.borrow_mut().remove(&client_id); - return None; - }; - let message = queue.pop_front(); - if queue.is_empty() { - queues.remove(&client_id); - } + // The entry survives draining to empty: removing it would cost a + // `HashMap` insert plus a `VecDeque` alloc/free per request for a + // lockstep client. The connection-lost hook is what frees it. + let message = queues + .borrow_mut() + .get_mut(&client_id) + .and_then(VecDeque::pop_front); if message.is_none() { active.borrow_mut().remove(&client_id); } message } -/// Per-request partitions-count cap, shared by create-topic, create-partitions -/// and delete-partitions admission. Runs pre-consensus like -/// [`validate_topic_bounds`]: an oversized count must not burn a replicated -/// log entry (create-partitions admission would also allocate that many -/// consensus-group ids before replicating). -/// -/// Zero passes here because a zero-partition TOPIC is legal (legacy -/// `create_topic` admits `0..=MAX`); the add/remove requests reject it in -/// [`validate_partitions_change_count`]. -const fn validate_partitions_count(partitions_count: u32) -> Result<(), IggyError> { - if partitions_count > MAX_PARTITIONS_PER_REQUEST { - return Err(IggyError::TooManyPartitions); - } - Ok(()) +/// Where the funnel routes a client request. Derived by [`classify`], which +/// IS the routing: [`handle_client_request`] matches on its result. The +/// variant ORDER mirrors the order of the checks inside [`classify`], and +/// that order is semantics (documented there). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(in crate::dispatch) enum RequestClass { + /// Legacy pre-register login code: rejected with a typed + /// `MalformedLogin` eviction before the session gate. + LegacyLogin, + /// Non-replicated code other than PING on an unbound transport: + /// denied `Unauthenticated` (plain deny reply, never an eviction). + UnauthenticatedRead, + /// Non-replicated read for the reads router. + NonReplicatedRead, + /// The register handshake (`session == 0 && request == 0`). + LoginRegister, + Logout, + /// Replicated operation on an unbound transport: `Eviction(NoSession)`. + UnboundReplicated, + /// Neither a partition nor a metadata consensus op: resolved to a + /// replicated `TruncatePartition` by the owning shard. + DeleteSegments, + /// Partition-plane operation. + Partition, + /// Everything else: replicated metadata consensus. + ReplicatedMetadata, } -/// [`validate_partitions_count`] plus the zero rejection that create-partitions -/// and delete-partitions carry: adding or removing zero partitions is a no-op -/// that would still burn a replicated log entry, bump `Streams::revision` and -/// force every shard through a rebalance pass. Legacy rejects it with -/// `TooManyPartitions` in both handlers (`1..=MAX` on create, `== 0` on -/// delete), so the code matches rather than inventing a new one. -const fn validate_partitions_change_count(partitions_count: u32) -> Result<(), IggyError> { - if partitions_count == 0 { - return Err(IggyError::TooManyPartitions); +/// Route a client request: [`handle_client_request`] matches on the result, +/// so this function IS the routing and the order of the checks below is the +/// semantics. Pure: `bound` stands for +/// `sessions.get_session(transport_client_id).is_some()`, the only session +/// fact the checks consult. +/// +/// The pins, in check order: +/// - the legacy-login rejection precedes the session gate: a legacy code +/// must get the typed `MalformedLogin` eviction, not the generic +/// unauthenticated deny; +/// - the pre-auth allowlist is PING only; `GET_CLUSTER_METADATA` is +/// deliberately NOT pre-auth (see the funnel's auth-bypass guard); +/// - poll-messages and consumer-offset reads are non-replicated CODES, not +/// partition operations: they classify +/// [`RequestClass::NonReplicatedRead`] and route inside the reads router; +/// - a `Register` with `session != 0` or `request != 0` falls through to +/// the default. Those rows are classify-only: `RequestHeader::validate` +/// rejects such a header before the funnel sees it, so no mid-session +/// register ever reaches consensus; +/// - `DeleteSegments` is neither a partition nor a metadata op, so its +/// check sits before `is_partition`; +/// - the checksum and heartbeat pre-gates run BEFORE classification in the +/// funnel. +pub(in crate::dispatch) fn classify(header: &RoutedRequestHeader, bound: bool) -> RequestClass { + if header.operation == Operation::NonReplicated { + let nr_code = non_replicated_code(header); + if matches!( + nr_code, + LOGIN_USER_CODE | LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE + ) { + return RequestClass::LegacyLogin; + } + if nr_code != PING_CODE && !bound { + return RequestClass::UnauthenticatedRead; + } + return RequestClass::NonReplicatedRead; } - validate_partitions_count(partitions_count) -} - -/// Static create-topic bounds shared by the TCP and HTTP ingresses. Runs -/// pre-consensus: a rejected request must not burn a replicated log entry, -/// and `prepare_request` errors evict the session instead of denying typed. -/// `ServerDefault` is exempt from the size floor (it resolves against server -/// config at admission, matching legacy); `Unlimited` passes numerically. -/// `segment_size_bytes` is the topic's RESOLVED segment size (explicit -/// option, else this node's default), so a per-topic segment above the -/// global default still floors the topic cap. -pub fn validate_topic_bounds( - partitions_count: u32, - max_topic_size: MaxTopicSize, - segment_size_bytes: u64, -) -> Result<(), IggyError> { - validate_partitions_count(partitions_count)?; - validate_topic_size_floor(max_topic_size, segment_size_bytes) -} - -/// A topic cap below one segment can never be enforced: the first segment -/// already exceeds it. Split out of [`validate_topic_bounds`] because update -/// admission checks the cap without a partitions count to check. -pub fn validate_topic_size_floor( - max_topic_size: MaxTopicSize, - segment_size_bytes: u64, -) -> Result<(), IggyError> { - if !matches!(max_topic_size, MaxTopicSize::ServerDefault) - && max_topic_size.as_bytes_u64() < segment_size_bytes - { - return Err(IggyError::InvalidTopicSize( - max_topic_size, - IggyByteSize::from(segment_size_bytes), - )); + if header.operation == Operation::Register && header.session == 0 && header.request == 0 { + return RequestClass::LoginRegister; } - Ok(()) -} - -/// Announce an accepted `max_topic_size` the server cannot enforce as written. -/// -/// [`validate_topic_size_floor`] admits any cap of one segment or more, but -/// retention runs PER PARTITION and floors each partition's share at one SEALED -/// segment, which reaches up to one maximum bus frame past `segment_size`. A cap -/// between the two is stored and echoed back verbatim while the server actually -/// keeps `(segment_size + max_message_size) * partitions_count`, so the only -/// moment an operator can be told is the one where they set it. -/// -/// Warns rather than rejects: which caps are accepted is client-visible wire -/// behavior, and tightening it would break topics that already exist. -pub fn warn_unenforceable_topic_size( - max_topic_size: MaxTopicSize, - segment_size_bytes: u64, - max_message_size_bytes: usize, - partitions_count: u32, -) { - let MaxTopicSize::Custom(configured) = max_topic_size else { - return; - }; - let max_message_size_bytes = u64::try_from(max_message_size_bytes).unwrap_or(u64::MAX); - let per_partition_floor = segment_size_bytes.saturating_add(max_message_size_bytes); - let topic_floor = per_partition_floor.saturating_mul(u64::from(partitions_count)); - if configured.as_bytes_u64() >= topic_floor { - return; + if header.operation == Operation::Logout { + return RequestClass::Logout; } - warn!( - max_topic_size = configured.as_bytes_u64(), - partitions_count, - segment_size = segment_size_bytes, - enforced_per_partition = per_partition_floor, - "{UNENFORCEABLE_TOPIC_SIZE_WARN}" - ); -} - -/// Announce the same unenforceable cap when partitions are ADDED to a topic. -/// -/// The cap is topic-wide but enforcement is per partition, so every added -/// partition shrinks the share: a cap that cleared the floor when the topic was -/// created can stop clearing it here. The request carries only the delta, so -/// the stored cap, segment size and current partition count come from metadata. -pub fn warn_unenforceable_topic_size_on_partition_add( - streams: &Streams, - stream_id: &WireIdentifier, - topic_id: &WireIdentifier, - max_message_size_bytes: usize, - added_partitions_count: u32, -) { - let Some(((stream_slab, topic_slab), _)) = streams.partition_count_context(stream_id, topic_id) - else { - return; - }; - let Some((_, max_topic_size, partitions_count, segment_size)) = - streams.topic_retention_config(stream_slab, topic_slab) - else { - return; - }; - warn_unenforceable_topic_size( - max_topic_size, - segment_size.map_or(iggy_common::DEFAULT_SEGMENT_SIZE, |segment_size| { - segment_size.as_bytes_u64() - }), - max_message_size_bytes, - u32::try_from(partitions_count) - .unwrap_or(u32::MAX) - .saturating_add(added_partitions_count), - ); -} - -/// Reject option keys outside the resource's catalog, pre-consensus. Unknown -/// keys are rejected rather than skipped: a silently ignored knob would hand -/// the client server defaults without it ever learning. Streams and users -/// have no catalog keys yet, so `known` is empty for both until one lands. -pub fn validate_option_keys(options: &WireOptions, known: &[&str]) -> Result<(), IggyError> { - for entry in options { - // Wire validation already enforced UTF-8 string keys. - let key = String::from_utf8_lossy(entry.key); - if !known.contains(&key.as_ref()) { - return Err(IggyError::UnsupportedOptionKey(key.into_owned())); - } + if !bound { + return RequestClass::UnboundReplicated; + } + if header.operation == Operation::DeleteSegments { + return RequestClass::DeleteSegments; } - Ok(()) + if header.operation.is_partition() { + return RequestClass::Partition; + } + RequestClass::ReplicatedMetadata } -/// Reject a request before it reaches consensus: warn, then send the typed -/// deny reply. A silent drop would wedge every later request on the -/// connection until the socket read timeout. `context` labels the rejection -/// site in both log lines. -#[allow(clippy::future_not_send)] -async fn send_pre_consensus_deny( - shard: &Rc>, - header: &RoutedRequestHeader, - transport_client_id: u128, - error: &IggyError, - context: &'static str, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - warn!( - transport_client_id, - error = %error, - operation = ?header.operation, - context, - "denying request pre-consensus" - ); - let commit = current_metadata_commit(shard); - let reply = build_deny_reply(header, transport_client_id, 0, commit, error.as_code()); - if let Err(send_error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %send_error, - context, - "failed to send pre-consensus deny reply" - ); - } +/// The command code a `NonReplicated` header carries in its first four +/// reserved bytes. +fn non_replicated_code(header: &RoutedRequestHeader) -> u32 { + u32::from_le_bytes(header.reserved[..4].try_into().unwrap()) } #[allow(clippy::future_not_send, clippy::too_many_lines)] @@ -606,69 +461,65 @@ async fn handle_client_request( return; } - ensure_transport_connection(shard, sessions, transport_client_id); - - // Any request is liveness proof, not just PING: an idle-but-active client - // (e.g. an admin issuing reads between long sleeps) must not be evicted by - // the heartbeat verifier. A genuinely dead connection sends nothing, so the - // intended stale-client eviction still fires. No-ops for an unbound client. - sessions.borrow_mut().record_heartbeat(transport_client_id); + // ONE `connections` walk for the whole prologue. Any request is liveness + // proof, not just PING: an idle-but-active client (e.g. an admin issuing + // reads between long sleeps) must not be evicted by the heartbeat + // verifier. A genuinely dead connection sends nothing, so the intended + // stale-client eviction still fires. + // Bound, not matched: a `match` on `borrow_mut()` would hold the guard + // across the arm that borrows again. + let mut touched = sessions.borrow_mut().touch_connection(transport_client_id); + if touched.is_none() { + // A transport's first frame: the peer address and transport kind live + // on the bus, so only this path pays that lookup. + ensure_transport_connection(shard, sessions, transport_client_id); + touched = sessions.borrow_mut().touch_connection(transport_client_id); + } + let ConnectionContext { + bound, + user_id, + address: client_address, + } = touched.unwrap_or_default(); - let header = *request.header(); - if header.operation == Operation::NonReplicated { - // Auth bypass guard: `PING`, the liveness probe, is the only pre-auth - // code, on every roster shape. `GET_CLUSTER_METADATA` describes the - // private replica network and is not something an unauthenticated - // caller gets to read; a client that dialed a backup no longer needs - // it to find the leader, because the backup authenticates the login - // locally and forwards only the consensus proposal - // (`submit_register_local_or_forward`). Every other non-replicated - // code MUST go through Register first, which binds the acting user - // the per-op authz gates resolve. - let nr_code = u32::from_le_bytes(request.header().reserved[..4].try_into().unwrap()); - // Legacy (pre-register) login codes. The server authenticates only via - // the Register handshake (LOGIN_REGISTER / LOGIN_REGISTER_WITH_PAT, - // Operation::Register); the vsr SDK funnels both logins there and never - // emits these. Reject them uniformly with a typed MalformedLogin (the - // SDK maps it to InvalidFormat) before the session gate, so a legacy or - // foreign client fails fast instead of getting the generic - // Unauthenticated deny the pre-auth guard would send unbound, or the - // silent empty-ok Reply the bound non-replicated path would send. - if matches!( - nr_code, - LOGIN_USER_CODE | LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE - ) { + // Borrowed, not copied: the 256-byte header is only worth a by-value + // snapshot where an arm rewrites it (`ReplicatedMetadata`) and still has + // to echo the client's original fields on a deny. + let header = request.header(); + match classify(header, bound.is_some()) { + RequestClass::LegacyLogin => { + // Legacy (pre-register) login codes. The server authenticates only via + // the Register handshake (LOGIN_REGISTER / LOGIN_REGISTER_WITH_PAT, + // Operation::Register); the vsr SDK funnels both logins there and never + // emits these. Reject them uniformly with a typed MalformedLogin (the + // SDK maps it to InvalidFormat) before the session gate, so a legacy or + // foreign client fails fast instead of getting the generic + // Unauthenticated deny the pre-auth guard would send unbound, or the + // silent empty-ok Reply the bound non-replicated path would send. + let nr_code = non_replicated_code(header); warn!( transport_client_id, code = nr_code, "rejecting legacy login code; server requires the register handshake" ); - send_login_eviction( + send_eviction( shard, transport_client_id, header.client, EvictionReason::MalformedLogin, + "legacy_login_rejection", ) .await; - return; } - let allowed_pre_auth = nr_code == PING_CODE; - if !allowed_pre_auth && sessions.borrow().get_session(transport_client_id).is_none() { - // Foreign SDKs still probe `GET_CLUSTER_METADATA` before login - // until they are fixed, so that rejection is routine traffic and - // logs at debug rather than warn. - if nr_code == GET_CLUSTER_METADATA_CODE { - debug!( - transport_client_id, - "denying pre-auth cluster-metadata read with Unauthenticated" - ); - } else { - warn!( - transport_client_id, - code = nr_code, - "denying pre-auth non-replicated read with Unauthenticated" - ); - } + RequestClass::UnauthenticatedRead => { + let nr_code = non_replicated_code(header); + // No per-code exemption: every in-tree SDK reads the roster only + // after login, so an unauthenticated roster read is a real event + // and not something to hide at debug. + warn!( + transport_client_id, + code = nr_code, + "denying pre-auth non-replicated read with Unauthenticated" + ); // A plain deny Reply, not an Eviction: there is no session to // evict, and an Eviction is session-terminal by wire contract, // so SDKs would tear down the very connection their login is @@ -681,353 +532,193 @@ async fn handle_client_request( IggyError::Unauthenticated.as_code(), ) .await; - return; } - handle_non_replicated_request(shard, sessions, system_config, transport_client_id, request) - .await; - return; - } - - if header.operation == Operation::Register && header.session == 0 && header.request == 0 { - handle_login_register_request(shard, sessions, transport_client_id, request).await; - return; - } - - if header.operation == Operation::Logout { - handle_logout_request(shard, sessions, transport_client_id, request).await; - return; - } - - let bound = sessions.borrow().get_session(transport_client_id); - if bound.is_none() { - // Replicated request on an unbound transport. Without this short- - // circuit, the rewrite below overwrites `header.client` with - // `transport_client_id` and dispatches; the request_preflight then - // rejects with `NoSession`/`Fenced` and the failure disappears - // silently, wedging the SDK until the socket timeout. A typed - // `Eviction(NoSession)` is right here, unlike the pre-auth read - // guard above: a replicated request implies the client believes it - // has a session, and that session is gone, so it must register - // again. An empty status-0 Reply is not safe here, because - // SendMessages is the one replicated operation without a result - // section, and its decoder would read the empty body as a - // successful send. - warn!( - transport_client_id, - operation = ?header.operation, - "rejecting replicated request from unbound transport with Eviction(NoSession)" - ); - send_unauthenticated_eviction(shard, transport_client_id).await; - return; - } - - // DeleteSegments is neither a partition nor a metadata consensus op: the - // owning shard resolves the requested count to a concrete offset, then a - // `TruncatePartition` is replicated through metadata (Option A). Each - // replica's reconciler trims to the committed watermark. Handle it here, - // ahead of the partition/metadata routing below. - if header.operation == Operation::DeleteSegments { - handle_delete_segments_request(shard, transport_client_id, bound, &request).await; - return; - } - - if header.operation.is_partition() { - // `bound` is Some here (unbound transports returned above). - let (vsr_client_id, bound_session) = bound.unwrap_or((0, 0)); - // `get_session` discards the acting user id the partition gate needs; - // resolve it from the same bound connection. A bound transport always - // has one, but the gate fails closed on `None` rather than trust that. - let acting_user_id = sessions.borrow().get_user_id(transport_client_id); - dispatch_partition_request( - shard, - request, - vsr_client_id, - bound_session, - transport_client_id, - acting_user_id, - ) - .await; - return; - } - - let request = request.transmute_header(|header, new_header: &mut RoutedRequestHeader| { - *new_header = header; - // Metadata-plane ops route by operation: stamp the sentinel group. - new_header.group = server_common::sharding::METADATA_GROUP; - // `bound` is always Some here (unbound transports early-return above); - // this sets the consensus client id + session for the replicated op. - if let Some((bound_client_id, bound_session)) = bound { - new_header.client = bound_client_id; - new_header.session = bound_session; - } - }); - let (request, raw_pat_token) = match maybe_rewrite_pat_request( - sessions, - transport_client_id, - max_tokens_per_user, - |user_id| { - shard - .plane - .metadata() - .mux_stm - .users() - .read(|users| users.pat_count_of(user_id)) - }, - request, - ) { - Ok(rewritten) => rewritten, - Err(error) => { - // Token cap reached, malformed body, or a lost session binding. - send_pre_consensus_deny( + RequestClass::NonReplicatedRead => { + // The auth-bypass guard is `classify`'s `UnauthenticatedRead` class: + // `PING`, the liveness probe, is the only pre-auth code, on every + // roster shape. `GET_CLUSTER_METADATA` describes the private replica + // network and is not something an unauthenticated caller gets to + // read; a client that dialed a backup no longer needs it to find the + // leader, because the backup authenticates the login locally and + // forwards only the consensus proposal + // (`submit_register_local_or_forward`). Every other non-replicated + // code MUST go through Register first, which binds the acting user + // the per-op authz gates resolve. + handle_non_replicated_request( shard, - &header, + sessions, + system_config, transport_client_id, - &error, - "personal-access-token", + request, + (user_id, client_address), ) .await; - return; } - }; - // Hash raw passwords and, for ChangePassword, verify the current password - // on the primary before replication; see `crate::users`. Replicas store the - // hash directly. A wrong current password is not denied here: it rides - // consensus and applies as a committed InvalidCredentials no-op, so the only - // Err returned is a malformed body. - let request = match maybe_rewrite_user_password_request(shard, request) { - Ok(rewritten) => rewritten, - Err(error) => { - // Malformed body: deny fast with InvalidCommand. - send_pre_consensus_deny(shard, &header, transport_client_id, &error, "user-password") - .await; - return; + RequestClass::LoginRegister => { + handle_login_register_request(shard, sessions, transport_client_id, request).await; } - }; - // Static bounds run pre-consensus so a rejected request burns no - // replicated log entry; HTTP covers the same bounds via - // `command.validate()`. A body that fails to decode denies typed too - // (`InvalidCommand`), instead of riding consensus just to fail there. - let bounds = match header.operation { - Operation::CreateTopic => CreateTopicRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|create_topic| { - // `parse` doubles as the catalog gate: an unknown key or a - // malformed value denies typed here, pre-consensus. - let options = TopicCreateOptions::parse(&create_topic.options)?; - if let Some(segment_size) = options.segment_size { - validate_topic_segment_size( - segment_size.as_bytes_u64(), - iggy_common::MAX_TOPIC_SEGMENT_SIZE, - )?; - } - let segment_size = options.segment_size.map_or_else( - || iggy_common::DEFAULT_SEGMENT_SIZE, - |segment_size| segment_size.as_bytes_u64(), - ); - if options - .preallocate_segments - .unwrap_or(iggy_common::DEFAULT_PREALLOCATE_SEGMENTS) - { - validate_preallocated_topic_bytes(segment_size, create_topic.partitions_count)?; - } - let max_topic_size = options - .max_topic_size - .unwrap_or(MaxTopicSize::ServerDefault); - validate_topic_bounds(create_topic.partitions_count, max_topic_size, segment_size)?; - warn_unenforceable_topic_size( - max_topic_size, - segment_size, - shard.bus_max_message_size(), - create_topic.partitions_count, - ); - Ok(()) - }), - Operation::CreatePartitions => CreatePartitionsRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|create_partitions| { - validate_partitions_change_count(create_partitions.partitions_count)?; - let metadata = shard.plane.metadata(); - warn_unenforceable_topic_size_on_partition_add( - metadata.mux_stm.streams(), - &create_partitions.stream_id, - &create_partitions.topic_id, - shard.bus_max_message_size(), - create_partitions.partitions_count, - ); - Ok(()) - }), - Operation::DeletePartitions => DeletePartitionsRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|delete_partitions| { - validate_partitions_change_count(delete_partitions.partitions_count) - }), - // Only the updatable subset: the create-time knobs are pushed to - // partitions when the topic is built and nothing re-pushes them, so - // accepting one here would store a value no partition ever sees. - Operation::UpdateTopic => UpdateTopicRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|update_topic| { - validate_option_keys(&update_topic.options, UPDATABLE_TOPIC_OPTION_KEYS)?; - let options = TopicCreateOptions::parse(&update_topic.options)?; - let Some(max_topic_size) = options.max_topic_size else { - return Ok(()); - }; - // An update can lower the cap below one segment just as a - // create can, and the stored map would then report a size the - // topic can never enforce. The floor is this topic's own - // segment size, since that key is create-only. - let metadata = shard.plane.metadata(); - let streams = metadata.mux_stm.streams(); - let segment_size = streams - .topic_segment_size(&update_topic.stream_id, &update_topic.topic_id) - .map_or_else( - || iggy_common::DEFAULT_SEGMENT_SIZE, - |segment_size| segment_size.as_bytes_u64(), - ); - validate_topic_size_floor(max_topic_size, segment_size)?; - let partitions_count = streams - .topic_partitions_count(&update_topic.stream_id, &update_topic.topic_id) - .unwrap_or(0); - warn_unenforceable_topic_size( - max_topic_size, - segment_size, - shard.bus_max_message_size(), - u32::try_from(partitions_count).unwrap_or(u32::MAX), - ); - Ok(()) - }), - Operation::UpdateStream => UpdateStreamRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|update_stream| { - validate_option_keys(&update_stream.options, UPDATABLE_STREAM_OPTION_KEYS) - }), - Operation::UpdateUser => UpdateUserRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|update_user| { - validate_option_keys(&update_user.options, UPDATABLE_USER_OPTION_KEYS) - }), - Operation::CreateStream => CreateStreamRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|create_stream| validate_option_keys(&create_stream.options, &[])), - Operation::CreateUser => CreateUserRequest::decode_from(request_body(&request)) - .map_err(|_| IggyError::InvalidCommand) - .and_then(|create_user| validate_option_keys(&create_user.options, &[])), - _ => Ok(()), - }; - if let Err(error) = bounds { - send_pre_consensus_deny(shard, &header, transport_client_id, &error, "static-bounds").await; - return; - } - // Enrich consumer-group Join/Leave with the client's VSR id (+ topic - // partition count for Join) before replication; see `crate::consumer_group`. - let request = match maybe_rewrite_consumer_group_request(shard, request).await { - Ok(rewritten) => rewritten, - Err(error) => { + RequestClass::Logout => { + handle_logout_request(shard, sessions, transport_client_id, request).await; + } + RequestClass::UnboundReplicated => { + // Replicated request on an unbound transport. Without this short- + // circuit, the rewrite below overwrites `header.client` with + // `transport_client_id` and dispatches; the request_preflight then + // rejects with `NoSession`/`Fenced` and the failure disappears + // silently, wedging the SDK until the socket timeout. A typed + // `Eviction(NoSession)` is right here, unlike the plain deny of + // `UnauthenticatedRead`: a replicated request implies the client + // believes it has a session, and that session is gone, so it must + // register again. An empty status-0 Reply is not safe here, because + // SendMessages is the one replicated operation without a result + // section, and its decoder would read the empty body as a + // successful send. warn!( transport_client_id, - error = %error, operation = ?header.operation, - "dropping consumer-group request with invalid payload" + "rejecting replicated request from unbound transport with Eviction(NoSession)" ); - return; + // The eviction context is best-effort off the metadata consensus + // (peer shards have none; zeroes are cosmetic -- the SDK only + // reads the reason), and the evicted id is the transport id: no + // VSR session exists to name. + send_eviction( + shard, + transport_client_id, + transport_client_id, + EvictionReason::NoSession, + "unbound replicated request", + ) + .await; } - }; - let request_header = *request.header(); - // Replicated request: run consensus on the metadata owner (shard 0) and - // bring the committed reply back here. This shard owns the connection, - // so it writes the reply to the socket via the transport client id -- - // shard 0 can't route by the consensus client id (no home-shard bits). - match submit_client_request_on_owner(shard, request).await { - Some(reply) => { - // The raw PAT token never enters consensus (it is non-deterministic - // and secret), so the committed reply body is empty. Substitute the - // raw-token response here, on the minting client's home shard, using - // the confirmed commit position from the committed reply. - let reply = match build_raw_pat_reply(&request_header, reply, raw_pat_token) { - Ok(reply) => reply, + RequestClass::DeleteSegments => { + // DeleteSegments is neither a partition nor a metadata consensus op: the + // owning shard resolves the requested count to a concrete offset, then a + // `TruncatePartition` is replicated through metadata (Option A). Each + // replica's reconciler trims to the committed watermark. `classify` + // names it ahead of the partition and metadata classes. + handle_delete_segments_request(shard, transport_client_id, bound, &request).await; + } + RequestClass::Partition => { + // `bound` is Some here: `classify` sends unbound transports to + // `UnboundReplicated`. + let (vsr_client_id, bound_session) = bound.unwrap_or((0, 0)); + // The acting user comes from the prologue's lookup. A bound + // transport always has one, but the gate below fails closed on + // `None` rather than trust that. + dispatch_partition_request( + shard, + request, + vsr_client_id, + bound_session, + transport_client_id, + user_id, + ) + .await; + } + RequestClass::ReplicatedMetadata => { + // The one arm that needs the by-value copy: the rewrite below + // stamps the consensus client / session / group over the header, + // and a pre-consensus deny still has to echo what the client sent. + let header = *header; + let request = + request.transmute_header(|header, new_header: &mut RoutedRequestHeader| { + *new_header = header; + // Metadata-plane ops route by operation: stamp the sentinel group. + new_header.group = server_common::sharding::METADATA_GROUP; + // `bound` is always Some here (`classify` sends unbound transports to + // `UnboundReplicated`); this sets the consensus client id + session + // for the replicated op. + if let Some((bound_client_id, bound_session)) = bound { + new_header.client = bound_client_id; + new_header.session = bound_session; + } + }); + let (request, raw_pat_token) = match tcp_chain( + shard, + sessions, + transport_client_id, + max_tokens_per_user, + request, + ) { + Ok(rewritten) => rewritten, + Err(RewriteDeny { stage, error }) => { + send_pre_consensus_deny(shard, transport_client_id, &header, &error, stage) + .await; + return; + } + }; + // Enrich consumer-group Join/Leave with the client's VSR id (+ topic + // partition count for Join) before replication; see `crate::consumer_group`. + let request = match maybe_rewrite_consumer_group_request(shard, request).await { + Ok(rewritten) => rewritten, Err(error) => { - warn!( + // Both of the rewrite's own failures are `InvalidCommand` + // decode errors, so a replay cannot help: deny typed + // instead of leaving the lockstep connection to its read + // timeout. (Its third error path needs a body past + // `u32::MAX` against a 64 MiB message cap, so no client + // frame reaches it; the deny is correct there too.) + send_pre_consensus_deny( + shard, transport_client_id, - error = %error, - "failed to build raw PAT reply" - ); + &header, + &error, + RewriteStage::ConsumerGroup, + ) + .await; return; } }; - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - operation = ?header.operation, - "failed to deliver committed reply to client" - ); + let request_header = *request.header(); + // Replicated request: run consensus on the metadata owner (shard 0) and + // bring the committed reply back here. This shard owns the connection, + // so it writes the reply to the socket via the transport client id -- + // shard 0 can't route by the consensus client id (no home-shard bits). + match submit_client_request_on_owner(shard, request).await { + Some(reply) => { + // The raw PAT token never enters consensus (it is non-deterministic + // and secret), so the committed reply body is empty. Substitute the + // raw-token response here, on the minting client's home shard, using + // the confirmed commit position from the committed reply. + let reply = match build_raw_pat_reply(&request_header, reply, raw_pat_token) { + Ok(reply) => reply, + Err(error) => { + warn!( + transport_client_id, + error = %error, + "failed to build raw PAT reply" + ); + // The op COMMITTED; only the reply could not be + // rendered. A typed deny is still the right frame: + // silence wedges the lockstep connection on a + // request that succeeded server-side, and the + // client can read the minted token back. + send_deny_reply(shard, transport_client_id, &header, error.as_code()) + .await; + return; + } + }; + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_frozen(), + FrameChannel::Reply, + "committed_reply", + ) + .await; + } + None => { + // Transient submit failure (not primary / not caught up / dedup + // absorbed). Stay silent; the SDK read-timeout replays. + warn!( + transport_client_id, + operation = ?header.operation, + "replicated request not committed (transient); client will replay" + ); + } } } - None => { - // Transient submit failure (not primary / not caught up / dedup - // absorbed). Stay silent; the SDK read-timeout replays. - warn!( - transport_client_id, - operation = ?header.operation, - "replicated request not committed (transient); client will replay" - ); - } - } -} - -/// Send a non-replicated reply body to a client, stamping the current -/// metadata commit. Shared by the `get_me` / `get_clients` / `get_client` -/// arms. -#[allow(clippy::future_not_send)] -async fn send_non_replicated_bytes( - shard: &Rc>, - request: &Message, - transport_client_id: u128, - bytes: Bytes, - label: &'static str, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let commit = current_metadata_commit(shard); - let reply = NonReplicatedResponse::Bytes(bytes).into_reply( - request.header(), - request.header().client, - request.header().session, - commit, - ); - send_reply_frame( - shard, - transport_client_id, - reply.into_generic().into_frozen(), - label, - ) - .await; -} - -/// Hand a built reply frame to the bus for `transport_client_id`. -#[allow(clippy::future_not_send)] -async fn send_reply_frame( - shard: &Rc>, - transport_client_id: u128, - frame: impl Into, - label: &'static str, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - if let Err(error) = shard.bus.send_to_client(transport_client_id, frame).await { - warn!(transport_client_id, label, error = %error, "failed to send non-replicated reply"); } } @@ -1072,7 +763,13 @@ mod tests { use crate::cluster_meta::ClusterRoster; use crate::dispatch::test_support::{FIRST_BOOT, SpyBus, TestMux, TestShard, test_shard}; use iggy_binary_protocol::Command; + use iggy_binary_protocol::codes::{ + GET_CLUSTER_METADATA_CODE, GET_CONSUMER_OFFSET_CODE, POLL_MESSAGES_CODE, + }; + use iggy_binary_protocol::{EvictionHeader, ReplyHeader}; + use journal::prepare_journal::PrepareJournal; use metadata::IggyMetadata; + use metadata::impls::metadata::IggySnapshot; use partitions::{IggyPartitions, PartitionPathLayout, PartitionsConfig}; use server_common::MESSAGE_ALIGN; use server_common::sharding::ShardId; @@ -1225,7 +922,6 @@ mod tests { #[compio::test] async fn pre_auth_cluster_metadata_denied_on_every_roster() { use configs::cluster::{ClusterNodeConfig, TransportPorts}; - use iggy_binary_protocol::codes::GET_CLUSTER_METADATA_CODE; use iggy_binary_protocol::{GenericHeader, ReplyHeader}; const TRANSPORT: u128 = 91; @@ -1326,89 +1022,441 @@ mod tests { } } + /// Client-wire frame for the funnel tests, shaped like the SDK sends it + /// (the funnel promotes it to the routed shape itself). + fn wire_request( + operation: Operation, + client: u128, + session: u64, + request: u64, + body: &[u8], + ) -> Message { + let header_size = size_of::(); + let total = header_size + body.len(); + let mut message = Message::::new(total); + { + let slice = message.as_mut_slice(); + slice[header_size..total].copy_from_slice(body); + let header = + bytemuck::checked::from_bytes_mut::(&mut slice[..header_size]); + *header = RequestHeader { + command: Command::Request, + operation, + size: u32::try_from(total).expect("test request fits u32"), + client, + session, + request, + ..Default::default() + }; + } + message + } + + fn non_replicated_request(client: u128, nr_code: u32) -> Message { + let mut message = wire_request(Operation::NonReplicated, client, 0, 0, &[]); + { + let header_size = size_of::(); + let header = bytemuck::checked::from_bytes_mut::( + &mut message.as_mut_slice()[..header_size], + ); + header.reserved[..4].copy_from_slice(&nr_code.to_le_bytes()); + } + message.into_generic() + } + + const fn frame_command(frame: &[u8]) -> u8 { + frame[std::mem::offset_of!(GenericHeader, command)] + } + + const fn eviction_reason_byte(frame: &[u8]) -> u8 { + frame[std::mem::offset_of!(EvictionHeader, reason)] + } + + fn reply_status(frame: &[u8]) -> u32 { + const STATUS_OFFSET: usize = std::mem::offset_of!(ReplyHeader, status); + u32::from_le_bytes(frame[STATUS_OFFSET..STATUS_OFFSET + 4].try_into().unwrap()) + } + + /// The classifier is the funnel's probe chain as data: every row pins + /// one routing decision, and the labeled rows pin the probe ORDERINGS + /// the funnel relies on. + #[allow(clippy::too_many_lines)] #[test] - fn create_topic_bounds_deny_pre_consensus() { - let segment_size = iggy_common::DEFAULT_SEGMENT_SIZE; - assert!(segment_size > 0, "default segment size must be nonzero"); + fn classify_pins_probe_order() { + fn routed(operation: Operation, session: u64, request: u64) -> RoutedRequestHeader { + RoutedRequestHeader { + command: Command::Request, + operation, + client: 7, + session, + request, + ..Default::default() + } + } + fn nr(nr_code: u32) -> RoutedRequestHeader { + let mut header = routed(Operation::NonReplicated, 0, 0); + header.reserved[..4].copy_from_slice(&nr_code.to_le_bytes()); + header + } + let table = [ + ( + "legacy login beats the session gate (unbound)", + nr(LOGIN_USER_CODE), + false, + RequestClass::LegacyLogin, + ), + ( + "legacy login beats the bound reads route", + nr(LOGIN_USER_CODE), + true, + RequestClass::LegacyLogin, + ), + ( + "legacy PAT login beats the session gate (unbound)", + nr(LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE), + false, + RequestClass::LegacyLogin, + ), + ( + "legacy PAT login beats the bound reads route", + nr(LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE), + true, + RequestClass::LegacyLogin, + ), + ( + "ping is the only pre-auth code", + nr(PING_CODE), + false, + RequestClass::NonReplicatedRead, + ), + ( + "cluster metadata is not pre-auth", + nr(GET_CLUSTER_METADATA_CODE), + false, + RequestClass::UnauthenticatedRead, + ), + ( + "poll-messages is a non-replicated code, not a partition op", + nr(POLL_MESSAGES_CODE), + true, + RequestClass::NonReplicatedRead, + ), + ( + "consumer-offset read is a non-replicated code, not a partition op", + nr(GET_CONSUMER_OFFSET_CODE), + true, + RequestClass::NonReplicatedRead, + ), + ( + "the register handshake", + routed(Operation::Register, 0, 0), + false, + RequestClass::LoginRegister, + ), + ( + "register with a session falls to the default", + routed(Operation::Register, 1, 0), + true, + RequestClass::ReplicatedMetadata, + ), + ( + "register with a request number falls to the default", + routed(Operation::Register, 0, 1), + true, + RequestClass::ReplicatedMetadata, + ), + ( + "logout, bound", + routed(Operation::Logout, 1, 1), + true, + RequestClass::Logout, + ), + ( + "logout beats the session gate", + routed(Operation::Logout, 1, 1), + false, + RequestClass::Logout, + ), + ( + "unbound metadata op", + routed(Operation::CreateStream, 1, 1), + false, + RequestClass::UnboundReplicated, + ), + ( + "the session gate beats the delete-segments probe", + routed(Operation::DeleteSegments, 1, 1), + false, + RequestClass::UnboundReplicated, + ), + ( + "the session gate beats the partition probe", + routed(Operation::SendMessages, 1, 1), + false, + RequestClass::UnboundReplicated, + ), + ( + "delete-segments is neither partition nor metadata", + routed(Operation::DeleteSegments, 1, 1), + true, + RequestClass::DeleteSegments, + ), + ( + "send-messages is partition-plane", + routed(Operation::SendMessages, 1, 1), + true, + RequestClass::Partition, + ), + ( + "store-consumer-offset is partition-plane", + routed(Operation::StoreConsumerOffset, 1, 1), + true, + RequestClass::Partition, + ), + ( + "create-topic is replicated metadata", + routed(Operation::CreateTopic, 1, 1), + true, + RequestClass::ReplicatedMetadata, + ), + ]; + for (label, header, bound, expected) in table { + assert_eq!(classify(&header, bound), expected, "{label}"); + } + } - assert!( - validate_topic_bounds( - MAX_PARTITIONS_PER_REQUEST, - MaxTopicSize::ServerDefault, - segment_size + /// A legacy login code must get the typed `MalformedLogin` eviction + /// BEFORE the session gate runs: an unbound sender must not fall into + /// the generic Unauthenticated deny the pre-auth guard sends for other + /// codes. + #[compio::test] + async fn legacy_login_codes_evicted_before_session_gate() { + const TRANSPORT: u128 = 94; + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + let sessions = Rc::new(RefCell::new(SessionManager::new())); + let system_config = Arc::new(ServerSystemConfig::default()); + + for code in [LOGIN_USER_CODE, LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE] { + handle_client_request( + &shard, + &sessions, + &system_config, + 1, + TRANSPORT, + non_replicated_request(TRANSPORT, code), ) - .is_ok(), - "the partition cap itself is admissible" + .await; + let replies = bus.client_replies.borrow(); + assert_eq!(replies.len(), 1, "code {code} must produce one frame"); + let (client, frame) = &replies[0]; + assert_eq!(*client, TRANSPORT); + assert_eq!( + frame_command(frame), + Command::Eviction as u8, + "legacy code {code} must be evicted, not denied" + ); + assert_eq!( + eviction_reason_byte(frame), + EvictionReason::MalformedLogin as u8, + "legacy code {code} must carry MalformedLogin" + ); + drop(replies); + bus.client_replies.borrow_mut().clear(); + } + } + + /// A replicated op from an unbound transport must get the typed + /// `Eviction(NoSession)`: the client believes it has a session and that + /// session is gone, so a silent drop or an empty Reply would wedge or + /// mislead it. + #[compio::test] + async fn unbound_replicated_request_gets_no_session_eviction() { + const TRANSPORT: u128 = 95; + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + let sessions = Rc::new(RefCell::new(SessionManager::new())); + let system_config = Arc::new(ServerSystemConfig::default()); + + handle_client_request( + &shard, + &sessions, + &system_config, + 1, + TRANSPORT, + wire_request(Operation::CreateStream, TRANSPORT, 1, 1, &[]).into_generic(), + ) + .await; + let replies = bus.client_replies.borrow(); + assert_eq!(replies.len(), 1, "unbound replicated op must be answered"); + let (client, frame) = &replies[0]; + assert_eq!(*client, TRANSPORT); + assert_eq!( + frame_command(frame), + Command::Eviction as u8, + "unbound replicated op must be evicted, not denied or dropped" ); - assert!( - matches!( - validate_topic_bounds( - MAX_PARTITIONS_PER_REQUEST + 1, - MaxTopicSize::ServerDefault, - segment_size - ), - Err(IggyError::TooManyPartitions) - ), - "one past the partition cap must deny" + assert_eq!( + eviction_reason_byte(frame), + EvictionReason::NoSession as u8, + "the eviction must carry NoSession" ); - // ServerDefault is numerically 0 yet exempt from the segment-size - // floor: it resolves against server config, matching legacy. - assert!(validate_topic_bounds(1, MaxTopicSize::ServerDefault, segment_size).is_ok()); - assert!(validate_topic_bounds(1, MaxTopicSize::Unlimited, segment_size).is_ok()); - let below_floor = MaxTopicSize::Custom((segment_size - 1).into()); - assert!( - matches!( - validate_topic_bounds(1, below_floor, segment_size), - Err(IggyError::InvalidTopicSize(size, floor)) - if size == below_floor && floor == IggyByteSize::from(segment_size) - ), - "custom size below the segment size must deny with the bounds" + } + + /// PING is the one pre-auth code: an unbound transport's ping must get a + /// normal status-0 Reply, not an eviction and not a deny. + #[compio::test] + async fn pre_auth_ping_allowed() { + const TRANSPORT: u128 = 96; + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + let sessions = Rc::new(RefCell::new(SessionManager::new())); + let system_config = Arc::new(ServerSystemConfig::default()); + + handle_client_request( + &shard, + &sessions, + &system_config, + 1, + TRANSPORT, + non_replicated_request(TRANSPORT, PING_CODE), + ) + .await; + let replies = bus.client_replies.borrow(); + assert_eq!(replies.len(), 1, "pre-auth ping must be answered"); + let (client, frame) = &replies[0]; + assert_eq!(*client, TRANSPORT); + assert_eq!( + frame_command(frame), + Command::Reply as u8, + "pre-auth ping must get a Reply, not an eviction" ); - let at_floor = MaxTopicSize::Custom(IggyByteSize::from(segment_size)); - assert!( - validate_topic_bounds(1, at_floor, segment_size).is_ok(), - "a topic exactly one segment large is admissible" + assert_eq!(reply_status(frame), 0, "pre-auth ping must succeed"); + } + + /// The request-checksum gate runs before every probe: a replicated op + /// from an UNBOUND transport with a bad stamp must get the checksum deny + /// Reply, not the `Eviction(NoSession)` the session gate would send. + #[compio::test] + async fn checksum_mismatch_denies_before_everything() { + const TRANSPORT: u128 = 97; + const BODY: &[u8] = b"stream-body"; + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + let sessions = Rc::new(RefCell::new(SessionManager::new())); + let system_config = Arc::new(ServerSystemConfig::default()); + + let mut message = wire_request(Operation::CreateStream, TRANSPORT, 1, 1, BODY); + { + let header_size = size_of::(); + let header = bytemuck::checked::from_bytes_mut::( + &mut message.as_mut_slice()[..header_size], + ); + // Nonzero (zero means unstamped and skips the check) and never + // the body's real checksum. + header.request_checksum = u128::from(iggy_common::calculate_checksum(BODY)) + 1; + } + handle_client_request( + &shard, + &sessions, + &system_config, + 1, + TRANSPORT, + message.into_generic(), + ) + .await; + let replies = bus.client_replies.borrow(); + assert_eq!(replies.len(), 1, "bad stamp must be answered"); + let (client, frame) = &replies[0]; + assert_eq!(*client, TRANSPORT); + assert_eq!( + frame_command(frame), + Command::Reply as u8, + "a bad stamp must get the checksum deny, not the NoSession eviction" + ); + assert_eq!( + reply_status(frame), + IggyError::InvalidFormat.as_code(), + "the deny status must carry the checksum error" ); } + /// The late-bound self-reference as the shard build creates it: unset + /// until the shard exists. + fn unset_shard_handle() -> ShellShardHandle { + Rc::new(RefCell::new(None)) + } + + /// Yield so the drain task the enqueue spawned on the bus can run. + async fn run_spawned_tasks() { + compio::time::sleep(std::time::Duration::from_millis(1)).await; + } + + /// One handler per shard means one connection-lost hook on its bus: a + /// second install would overwrite the first and orphan the sessions + /// the first one bound. #[test] - fn partitions_count_cap_denies_pre_consensus() { - assert!( - validate_partitions_count(MAX_PARTITIONS_PER_REQUEST).is_ok(), - "the cap itself is admissible" + fn deferred_handler_installs_one_connection_lost_hook() { + let bus = SpyBus::default(); + let _handler = make_deferred_client_request_handler( + &bus, + &unset_shard_handle(), + &Rc::new(RefCell::new(SessionManager::new())), + Arc::new(ServerSystemConfig::default()), + 1, ); - assert!( - matches!( - validate_partitions_count(MAX_PARTITIONS_PER_REQUEST + 1), - Err(IggyError::TooManyPartitions) - ), - "one past the cap must deny" + assert_eq!( + bus.connection_lost_hooks.get(), + 1, + "one factory call must install exactly one connection-lost hook" ); - // Zero passes the shared cap because a zero-partition TOPIC is legal - // (legacy `create_topic` admits `0..=MAX`). - assert!(validate_partitions_count(0).is_ok()); } - #[test] - fn zero_partitions_change_denies_pre_consensus() { - // Adding or removing zero partitions is a no-op that would still burn - // a replicated log entry and force a rebalance. Legacy rejects it with - // `TooManyPartitions` in both handlers, so the code matches. - assert!( - matches!( - validate_partitions_change_count(0), - Err(IggyError::TooManyPartitions) - ), - "adding or removing zero partitions must deny" + /// The handler is built before the shard it serves. A frame that + /// arrives while the self-reference is still unset stays queued, and + /// the enqueue must release the client's active slot: otherwise every + /// later frame for that client finds the slot taken and nothing is + /// ever drained, the stranded frame included. + #[compio::test] + async fn deferred_handler_drains_after_the_shard_handle_is_set() { + const TRANSPORT: u128 = 98; + let bus = SpyBus::default(); + let shard_handle = unset_shard_handle(); + let handler = make_deferred_client_request_handler( + &bus, + &shard_handle, + &Rc::new(RefCell::new(SessionManager::new())), + Arc::new(ServerSystemConfig::default()), + 1, ); - assert!(validate_partitions_change_count(1).is_ok()); - assert!(validate_partitions_change_count(MAX_PARTITIONS_PER_REQUEST).is_ok()); + + handler(TRANSPORT, non_replicated_request(TRANSPORT, PING_CODE)); + run_spawned_tasks().await; assert!( - matches!( - validate_partitions_change_count(MAX_PARTITIONS_PER_REQUEST + 1), - Err(IggyError::TooManyPartitions) - ), - "the cap still applies" + bus.client_replies.borrow().is_empty(), + "nothing can be served before the shard exists" ); + + let shard = Rc::new(test_shard(&bus, 0, 1, FIRST_BOOT)); + *shard_handle.borrow_mut() = Some(Rc::downgrade(&shard)); + handler(TRANSPORT, non_replicated_request(TRANSPORT, PING_CODE)); + for _ in 0..500 { + if bus.client_replies.borrow().len() == 2 { + break; + } + run_spawned_tasks().await; + } + + let replies = bus.client_replies.borrow(); + assert_eq!( + replies.len(), + 2, + "the stranded ping and the new one must both be served once the shard exists" + ); + for (client, frame) in replies.iter() { + assert_eq!(*client, TRANSPORT); + assert_eq!(frame_command(frame), Command::Reply as u8); + assert_eq!(reply_status(frame), 0, "a served ping succeeds"); + } } } diff --git a/core/server/src/dispatch/partition.rs b/core/server/src/dispatch/partition.rs index b2e0789bfa..4cffab1b42 100644 --- a/core/server/src/dispatch/partition.rs +++ b/core/server/src/dispatch/partition.rs @@ -28,23 +28,25 @@ //! replies to committed writes itself, straight from the owning shard, while //! everything the host builds here -- read bodies, denies, empty-poll shapes //! -- goes out on the connection's home shard. The reply path is therefore -//! split by plane, not unified, and the deny helpers in `authz` are the +//! split by plane, not unified, and the deny helpers in `failure` are the //! third leg (typed status replies for requests that never reach a plane). use crate::consumer_group::maybe_rewrite_consumer_offset_request; -use crate::dispatch::authz::{ - authorize_partition_op, authorize_partition_read, send_deny_reply, send_non_replicated_deny, +use crate::dispatch::authz::{authorize_partition_op, authorize_partition_read}; +use crate::dispatch::failure::{ + FrameChannel, send_deny_reply, send_empty_partition_reply, send_host_frame, + send_non_replicated_bytes, send_non_replicated_deny, send_result_rejection, }; use crate::dispatch::submit::submit_client_request_on_owner; -use crate::dispatch::{send_non_replicated_bytes, send_reply_frame, upgrade_shard_handle}; +use crate::dispatch::upgrade_shard_handle; use crate::responses::{ - build_consumer_offset_body, build_empty_reply, build_polled_messages_reply, - current_metadata_commit, resolve_partition_namespace, resolve_partition_request_namespace, + build_consumer_offset_body, build_polled_messages_reply, current_metadata_commit, + resolve_partition_namespace, resolve_partition_request_namespace, }; use crate::shell::{ShellBus, ShellShard, ShellShardHandle}; use crate::wire::{request_body, usize_to_u32}; use bytes::Bytes; -use consensus::{Consensus, MetadataHandle, PartitionsHandle, build_result_rejection_reply}; +use consensus::{Consensus, MetadataHandle, PartitionsHandle}; use iggy_binary_protocol::PrepareHeader; use iggy_binary_protocol::primitives::consumer::WireConsumer; use iggy_binary_protocol::primitives::polling_strategy::WirePollingStrategy; @@ -57,10 +59,10 @@ use iggy_binary_protocol::{ AckLevel, Command, KIND_CONSUMER_GROUP, Operation, RoutedRequestHeader, WireDecode, WireEncode, WireIdentifier, }; -use iggy_common::{IggyError, PollingStrategy}; +use iggy_common::{IggyError, PollingStrategy, RESYNC_REQUIRED_PARTITION_SENTINEL}; use journal::superblock::SuperblockStore; use journal::{Journal, JournalHandle}; -use message_bus::AUTO_COMMIT_CLIENT_ID; +use message_bus::{AUTO_COMMIT_CLIENT_ID, BusMessage}; use metadata::impls::metadata::{ StreamsFrontend, build_truncate_partition_client_message, build_truncate_partition_client_message_with_identifiers, @@ -581,8 +583,10 @@ async fn relay_partition_reply( /// the owning shard ([`shard::IggyShard::partition_read`]), and re-encode /// the stored batches into the legacy wire `PolledMessages` body. /// -/// Failures reply with an empty body so the SDK fails fast on decode -/// instead of hanging until its read timeout. +/// A partition that cannot answer yet replies with the 16-byte empty poll, +/// which the SDK reads as 0 messages and retries. Permanent client errors +/// (undecodable body, authz, unresolved target) deny with a nonzero status +/// instead, so they cannot be mistaken for an empty partition. #[allow(clippy::future_not_send)] pub(in crate::dispatch) async fn handle_poll_messages( shard: &Rc>, @@ -597,13 +601,15 @@ pub(in crate::dispatch) async fn handle_poll_messages( SB: SuperblockStore + 'static, { let Ok(wire) = PollMessagesRequest::decode_from(request_body(request)) else { - // Undecodable poll: keep the fail-fast empty-poll shape. - send_non_replicated_bytes( + // A permanent client error, so it must not borrow the empty-poll + // shape: that body decodes as a successful 0-message poll, and a + // consumer looping on it never learns why. The server already sends + // nonzero-status poll replies (authz, unresolved target). + send_non_replicated_deny( shard, request, transport_client_id, - empty_polled_messages_body(0), - "poll_messages", + IggyError::InvalidCommand.as_code(), ) .await; return; @@ -624,45 +630,21 @@ pub(in crate::dispatch) async fn handle_poll_messages( send_non_replicated_deny(shard, request, transport_client_id, status).await; return; } - let body = match resolve_poll_request(shard, &wire, request.header().client) { - Ok((namespace, partition_id, consumer, args)) => { - match shard - .partition_read(namespace, PartitionRead::Poll { consumer, args }) - .await - { - Some(PartitionReadReply::Poll { - fragments, - current_offset, - }) => match build_polled_messages_reply( - request.header(), - current_metadata_commit(shard), - partition_id, - current_offset, - fragments, - shard.plane.partitions().config().encryptor.as_deref(), - ) { - Ok(reply) => { - send_reply_frame(shard, transport_client_id, reply, "poll_messages").await; - return; - } - Err(error) => { - warn!( - transport_client_id, - error = %error, - "failed to re-encode polled batches; replying empty poll" - ); - empty_polled_messages_body(partition_id) - } - }, - other => { - warn!( + let (body, channel) = match resolve_poll_request(shard, &wire, request.header().client) { + Ok(resolved) => { + match read_polled_messages(shard, transport_client_id, request, resolved).await { + Ok(reply) => { + send_host_frame( + &shard.bus, transport_client_id, - namespace = namespace.inner(), - reply_was_none = other.is_none(), - "partition read failed; replying empty poll" - ); - empty_polled_messages_body(partition_id) + reply, + FrameChannel::Reply, + "poll_messages", + ) + .await; + return; } + Err(fallback) => fallback, } } Err(error) => { @@ -695,18 +677,92 @@ pub(in crate::dispatch) async fn handle_poll_messages( "poll_messages request rejected; replying empty poll" ); let partition_id = if matches!(error, IggyError::ConsumerGroupPartitionNotOwned(..)) { - iggy_common::RESYNC_REQUIRED_PARTITION_SENTINEL + RESYNC_REQUIRED_PARTITION_SENTINEL } else { 0 }; - empty_polled_messages_body(partition_id) + empty_poll_fallback(partition_id) } }; - send_non_replicated_bytes(shard, request, transport_client_id, body, "poll_messages").await; + send_non_replicated_bytes( + shard, + request, + transport_client_id, + body, + channel, + "poll_messages", + ) + .await; } -/// Serve `get_consumer_offset`. An empty body decodes as `None` on the SDK -/// side (no offset stored / partition unknown). +/// Run the resolved poll on the owning shard and re-encode the stored +/// batches into the wire `PolledMessages` reply. A failed read or re-encode +/// hands back the fail-fast empty poll for the partition instead. +#[allow(clippy::future_not_send)] +async fn read_polled_messages( + shard: &Rc>, + transport_client_id: u128, + request: &Message, + (namespace, partition_id, consumer, args): DecodedPollRequest, +) -> Result +where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + match shard + .partition_read(namespace, PartitionRead::Poll { consumer, args }) + .await + { + Some(PartitionReadReply::Poll { + fragments, + current_offset, + }) => build_polled_messages_reply( + request.header(), + current_metadata_commit(shard), + partition_id, + current_offset, + fragments, + shard.plane.partitions().config().encryptor.as_deref(), + ) + .map_err(|error| { + warn!( + transport_client_id, + error = %error, + "failed to re-encode polled batches; replying empty poll" + ); + empty_poll_fallback(partition_id) + }), + other => { + warn!( + transport_client_id, + namespace = namespace.inner(), + reply_was_none = other.is_none(), + "partition read failed; replying empty poll" + ); + Err(empty_poll_fallback(partition_id)) + } + } +} + +/// The fail-fast poll reply for a partition that could not answer: the +/// 16-byte empty poll for `partition_id`, riding the re-sync sentinel +/// channel when the id is the sentinel and the empty-frame channel +/// otherwise. +fn empty_poll_fallback(partition_id: u32) -> (Bytes, FrameChannel) { + let channel = if partition_id == RESYNC_REQUIRED_PARTITION_SENTINEL { + FrameChannel::ResyncSentinel + } else { + FrameChannel::EmptyFrame + }; + (empty_polled_messages_body(partition_id), channel) +} + +/// Serve `get_consumer_offset`. An empty status-0 body decodes as `None` on +/// the SDK side (no offset stored / partition unknown), so it is reserved for +/// that answer: a malformed request denies with a nonzero status instead. // TODO(hubcio): plain local partition_read with no primary gate, so a // follower answers from its own (possibly lagging) offset state. Needs the // same is-caught-up-primary gate the auto-commit path has, or an explicit @@ -725,13 +781,15 @@ pub(in crate::dispatch) async fn handle_get_consumer_offset( SB: SuperblockStore + 'static, { let Ok(wire) = GetConsumerOffsetRequest::decode_from(request_body(request)) else { - // Undecodable: an empty body decodes as None (no offset) on the SDK. - send_non_replicated_bytes( + // Same rule as the poll above: an empty body is the legitimate + // "no offset stored" answer, so a malformed request must not send + // one. Byte-identical frames on the same channel with the same + // context would also be indistinguishable in the send-failure log. + send_non_replicated_deny( shard, request, transport_client_id, - Bytes::new(), - "get_consumer_offset", + IggyError::InvalidCommand.as_code(), ) .await; return; @@ -789,43 +847,12 @@ pub(in crate::dispatch) async fn handle_get_consumer_offset( request, transport_client_id, body, + FrameChannel::Reply, "get_consumer_offset", ) .await; } -/// Ack a consumer-offset op whose body could not be rewritten for the -/// partition plane with an empty Reply. The SDK connection processes replies -/// in lockstep, so a silent drop wedges every subsequent request on that -/// connection. -#[allow(clippy::future_not_send)] -async fn send_empty_partition_reply( - shard: &Rc>, - transport_client_id: u128, - request_header: &RoutedRequestHeader, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let commit = current_metadata_commit(shard); - let reply = build_empty_reply(request_header, transport_client_id, 0, commit); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - operation = ?request_header.operation, - "failed to surface empty partition reply" - ); - } -} - /// Wait (bounded) until this shard holds a routing row for `namespace`. Fast /// path: row already present -> no wait. /// @@ -1087,7 +1114,7 @@ pub(in crate::dispatch) async fn handle_delete_segments_request( ) .await { - Ok(truncate) => Some(truncate), + Ok(truncate) => truncate, // The owning partition has not converged on the committed log yet, so // the delete cannot be resolved to a watermark. Reply with the // result-framed transient rejection (under the TruncatePartition @@ -1104,67 +1131,50 @@ pub(in crate::dispatch) async fn handle_delete_segments_request( 0, 0, ); - let reply = build_result_rejection_reply( + send_result_rejection( + shard, + transport_client_id, template.header(), - current_metadata_commit(shard), - IggyError::TransientNotAccepted.as_code(), - ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "delete_segments: failed to send transient rejection" - ); - } + &IggyError::TransientNotAccepted, + "delete_segments transient rejection", + ) + .await; return; } - Err(_) => None, - }; - - let reply = if let Some(truncate) = truncate { - // Forward the consensus reply verbatim, exactly like the generic - // metadata path: a committed success acks the delete, and a - // result-framed `TransientNotCommitted` rejection makes the SDK - // replay the request. Acking unconditionally here would swallow a - // not-primary rejection and drop the delete on the floor while the - // client believes it succeeded. - let Some(reply) = submit_client_request_on_owner(shard, truncate).await else { - // Transient submit failure (not primary / view change). Stay - // silent; the SDK read-timeout replays the same request id, - // which re-resolves and commits. Acking here would advance the - // client past an unrecorded request and gap the next metadata - // op. - warn!( - transport_client_id, - "delete_segments: transient submit; client will replay" - ); + // Undecodable body (never produced by the SDK): deny typed rather + // than ack empty. Both keep the lockstep stream framed, but a + // status-0 ack reads as a completed trim. Unresolvable-but-well-formed + // targets commit a typed rejection instead (see the resolve). + Err(error) => { + send_deny_reply(shard, transport_client_id, &header, error.as_code()).await; return; - }; - reply - } else { - // Undecodable body (never produced by the SDK): ack empty so the - // lockstep stream stays framed; the typed decoder surfaces the - // failure client-side. Unresolvable-but-well-formed targets commit a - // typed rejection instead (see the resolve), so only a wire-corrupt - // request can gap the sequence here. - let commit = current_metadata_commit(shard); - build_empty_reply(&header, transport_client_id, session, commit).into_generic() + } }; - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_frozen()) - .await - { + + // Forward the consensus reply verbatim, exactly like the generic metadata + // path: a committed success acks the delete, and a result-framed + // `TransientNotCommitted` rejection makes the SDK replay the request. + // Acking unconditionally here would swallow a not-primary rejection and + // drop the delete on the floor while the client believes it succeeded. + let Some(reply) = submit_client_request_on_owner(shard, truncate).await else { + // Transient submit failure (not primary / view change). Stay silent; + // the SDK read-timeout replays the same request id, which re-resolves + // and commits. Acking here would advance the client past an + // unrecorded request and gap the next metadata op. warn!( transport_client_id, - error = %error, - "delete_segments: failed to send reply" + "delete_segments: transient submit; client will replay" ); - } + return; + }; + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_frozen(), + FrameChannel::Reply, + "delete_segments_reply", + ) + .await; } /// Resolve a client `DeleteSegments` to the `TruncatePartition` that commits the @@ -1176,8 +1186,7 @@ pub(in crate::dispatch) async fn handle_delete_segments_request( /// truncate commits under. A resolvable namespace with nothing sealed to delete /// still yields a `TruncatePartition(up_to_offset = 0)` so the metadata request /// sequence stays contiguous. `Err` on a malformed body or an unresolved -/// namespace: the TCP caller drops it to a silent replay, the HTTP caller renders -/// the error. +/// namespace: the TCP caller denies typed, the HTTP caller renders the error. #[allow(clippy::future_not_send)] #[allow(clippy::cast_possible_truncation)] pub async fn resolve_delete_segments_truncate( @@ -1279,7 +1288,7 @@ where mod tests { use super::*; use crate::dispatch::test_support::{ - SpyBus, TestMux, TestShard, prepare_message, request_message, + SpyBus, TestMux, TestShard, prepare_message, request_message, test_shard, }; use iggy_binary_protocol::ReplyHeader; use iggy_binary_protocol::primitives::partition_assignment::CreatedPartitionAssignment; @@ -1302,6 +1311,74 @@ mod tests { ShardIdentity, shard_channel, }; + /// An undecodable request body is a PERMANENT client error, so every read + /// on this path must answer a nonzero status. The fail-fast shapes these + /// used to borrow all decode as success: the 16-byte empty poll reads as a + /// 0-message poll, an empty consumer-offset body as "no offset stored", + /// and a status-0 `delete_segments` ack as a completed trim. A client on a + /// skewed protocol would loop on those forever with no error to surface. + #[compio::test] + async fn undecodable_read_bodies_must_deny_typed_not_fabricate_success() { + const TRANSPORT: u128 = 91; + const VSR_CLIENT: u128 = 1; + const SESSION: u64 = 1; + const STATUS_OFFSET: usize = std::mem::offset_of!(ReplyHeader, status); + // Shorter than any of the three request encodings, so each decoder + // fails on length before it can interpret a field. + const TRUNCATED_BODY: &[u8] = &[0x01]; + + let bus = SpyBus::default(); + let shard = Rc::new(test_shard(&bus, 0, 1, 1)); + + let poll = request_message( + Operation::NonReplicated, + VSR_CLIENT, + SESSION, + 1, + TRUNCATED_BODY, + ); + handle_poll_messages(&shard, TRANSPORT, &poll, Some(DEFAULT_ROOT_USER_ID)).await; + + let offset = request_message( + Operation::NonReplicated, + VSR_CLIENT, + SESSION, + 2, + TRUNCATED_BODY, + ); + handle_get_consumer_offset(&shard, TRANSPORT, &offset, Some(DEFAULT_ROOT_USER_ID)).await; + + let delete = request_message( + Operation::DeleteSegments, + VSR_CLIENT, + SESSION, + 3, + TRUNCATED_BODY, + ); + handle_delete_segments_request(&shard, TRANSPORT, Some((VSR_CLIENT, SESSION)), &delete) + .await; + + let replies = bus.client_replies.borrow(); + assert_eq!( + replies.len(), + 3, + "one deny frame per undecodable request, none of them silent" + ); + for (label, (client, frame)) in ["poll_messages", "get_consumer_offset", "delete_segments"] + .into_iter() + .zip(replies.iter()) + { + assert_eq!(*client, TRANSPORT, "{label} deny must target the transport"); + let status = + u32::from_le_bytes(frame[STATUS_OFFSET..STATUS_OFFSET + 4].try_into().unwrap()); + assert_eq!( + status, + IggyError::InvalidCommand.as_code(), + "{label} with an undecodable body must carry a nonzero status" + ); + } + } + /// A partition write whose routable wait exhausts (namespace committed, /// but no reconciler ever seeds this shard's routing row -- the state a /// teardown/rematerialise churn leaves behind) must answer a nonzero diff --git a/core/server/src/dispatch/reads.rs b/core/server/src/dispatch/reads.rs index 2c8b43c0d8..d944152f46 100644 --- a/core/server/src/dispatch/reads.rs +++ b/core/server/src/dispatch/reads.rs @@ -26,9 +26,11 @@ //! the HTTP layer), never in the builder. use crate::cluster_meta::ClusterRoster; -use crate::dispatch::authz::{authorize_default_read, authorize_uid, send_non_replicated_deny}; +use crate::dispatch::authz::{authorize_default_read, authorize_uid}; +use crate::dispatch::failure::{ + FrameChannel, send_host_frame, send_non_replicated_bytes, send_non_replicated_deny, +}; use crate::dispatch::partition::{handle_get_consumer_offset, handle_poll_messages}; -use crate::dispatch::send_non_replicated_bytes; use crate::responses::{ build_empty_reply, build_get_me_response, build_get_personal_access_tokens_response, build_non_replicated_response, connected_client_to_response, current_metadata_commit, @@ -63,7 +65,7 @@ use metadata::impls::metadata::StreamsFrontend; use metadata::permissioner::Permissioner; use server_common::Message; use std::cell::RefCell; -use std::net::IpAddr; +use std::net::{IpAddr, SocketAddr}; use std::rc::Rc; use std::sync::Arc; use tracing::{debug, warn}; @@ -90,6 +92,7 @@ async fn handle_get_personal_access_tokens( request, transport_client_id, response.to_bytes(), + FrameChannel::Reply, "get_personal_access_tokens", ) .await; @@ -117,6 +120,7 @@ async fn handle_get_me( request, transport_client_id, response.to_bytes(), + FrameChannel::Reply, "get_me", ) .await; @@ -129,6 +133,11 @@ pub(in crate::dispatch) async fn handle_non_replicated_request( system_config: &Arc, transport_client_id: u128, request: Message, + // Acting user and peer address for the read gates below, resolved by the + // funnel in the same connection lookup as the heartbeat. `user_id` is + // `None` only on the pre-auth path (PING), which serves ungated codes; + // the gated arms fail closed on it. + (user_id, client_address): (Option, Option), ) where B: ShellBus, MJ: JournalHandle + 'static, @@ -138,15 +147,10 @@ pub(in crate::dispatch) async fn handle_non_replicated_request( { const CODE_RANGE: std::ops::Range = 0..4; let code = u32::from_le_bytes(request.header().reserved[CODE_RANGE].try_into().unwrap()); - // Acting user and peer address for the read gates below, resolved in one - // connection lookup. `user_id` is `None` only on the pre-auth path - // (PING), which serves ungated codes; the gated arms fail closed on it. - let (user_id, client_address) = sessions.borrow().read_context(transport_client_id); match code { PING_CODE => { - // A ping is the client's liveness proof; reset its staleness clock - // so the heartbeat verifier doesn't evict an active connection. - sessions.borrow_mut().record_heartbeat(transport_client_id); + // No `record_heartbeat` here: the funnel records one for EVERY + // frame before classification, so a ping is already covered. let commit = current_metadata_commit(shard); let reply = build_empty_reply( request.header(), @@ -154,17 +158,14 @@ pub(in crate::dispatch) async fn handle_non_replicated_request( request.header().session, commit, ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send non-replicated ping reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "ping_reply", + ) + .await; } GET_ME_CODE => { handle_get_me(shard, sessions, transport_client_id, &request).await; @@ -192,6 +193,7 @@ pub(in crate::dispatch) async fn handle_non_replicated_request( &request, transport_client_id, response.to_bytes(), + FrameChannel::Reply, "get_clients", ) .await; @@ -236,8 +238,15 @@ pub(in crate::dispatch) async fn handle_non_replicated_request( } .to_bytes() }); - send_non_replicated_bytes(shard, &request, transport_client_id, bytes, "get_client") - .await; + send_non_replicated_bytes( + shard, + &request, + transport_client_id, + bytes, + FrameChannel::Reply, + "get_client", + ) + .await; } GET_SNAPSHOT_FILE_CODE => { handle_get_snapshot(shard, system_config, transport_client_id, &request, user_id).await; @@ -297,6 +306,16 @@ async fn handle_default_non_replicated( // authz-free (it is byte-shared with the HTTP read path, which gates // separately); a denial replies status!=0 with an empty body. if let Err(error) = authorize_default_read(shard, code, request_body(request), user_id) { + // Same line as the builder-`Err` branch below: `send_non_replicated_deny` + // logs only on send FAILURE, so a refusal that reaches the client would + // otherwise leave nothing server-side - including the refusal this gate + // now issues for every armless or unknown non-replicated code. + warn!( + transport_client_id, + code, + error = %error, + "denying non-replicated VSR request" + ); send_non_replicated_deny(shard, request, transport_client_id, error.as_code()).await; return; } @@ -324,18 +343,14 @@ async fn handle_default_non_replicated( request.header().session, commit, ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - code, - error = %error, - "failed to send non-replicated VSR reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "non_replicated_reply", + ) + .await; } Err(error) => { // Surface the builder's typed error (unsupported op, undecodable @@ -412,6 +427,7 @@ async fn handle_get_snapshot( request, transport_client_id, GetSnapshotResponse { data: archive }.to_bytes(), + FrameChannel::Reply, "get_snapshot", ) .await; @@ -485,6 +501,7 @@ async fn handle_sync_consumer_group( request, transport_client_id, body, + FrameChannel::Reply, "sync_consumer_group", ) .await; diff --git a/core/server/src/dispatch/session_ops.rs b/core/server/src/dispatch/session_ops.rs index d43bcc17b6..3a09ce0db4 100644 --- a/core/server/src/dispatch/session_ops.rs +++ b/core/server/src/dispatch/session_ops.rs @@ -32,6 +32,9 @@ //! two together, so logout and eviction must release BOTH -- every teardown //! path below pairs `remove_connection` with a replicated `Logout`. +use crate::dispatch::failure::{ + FrameChannel, send_eviction, send_host_frame, send_result_rejection, +}; use crate::dispatch::login_error::LoginRegisterError; use crate::responses::{ build_deny_reply, build_empty_reply, build_login_register_reply, current_metadata_commit, @@ -39,11 +42,7 @@ use crate::responses::{ use crate::session_manager::{ClientSdkInfo, SessionManager}; use crate::shell::{ShellBus, ShellShard}; use crate::wire::request_body; -use consensus::{ - Consensus, DISCONNECT_LOGOUT_REQUEST_ID, EvictionContext, MetadataHandle, - build_eviction_message, build_incompatible_protocol_eviction_message, - build_result_rejection_reply, -}; +use consensus::{Consensus, DISCONNECT_LOGOUT_REQUEST_ID, MetadataHandle}; use iggy_binary_protocol::PrepareHeader; use iggy_binary_protocol::requests::users::{LoginRegisterRequest, LoginRegisterWithPatRequest}; use iggy_binary_protocol::{ @@ -241,10 +240,14 @@ where let commit = current_metadata_commit(shard).max(session); let reply = build_login_register_reply(request_header, vsr_client_id, session, commit, user_id); - let _ = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await; + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "login_replay_reply", + ) + .await; return Ok(()); } @@ -288,17 +291,14 @@ where // lower number would make one frame contradict itself. let commit = current_metadata_commit(shard).max(session); let reply = build_login_register_reply(request_header, vsr_client_id, session, commit, user_id); - let send_result = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await; - if let Err(error) = send_result { - warn!( - transport_client_id, - error = %error, - "failed to send login/register reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "login_register_reply", + ) + .await; Ok(()) } @@ -331,58 +331,28 @@ async fn surface_login_failure( SB: SuperblockStore + 'static, { if error.is_terminal() { - send_login_eviction( + send_eviction( shard, transport_client_id, request_header.client, eviction_reason_for(error), + "login_rejection", ) .await; } else { // Which code the hint carries is what tells the client whether the // replay may move to another node: see `transient_login_code`. - send_login_transient_reply( + send_result_rejection( shard, transport_client_id, request_header, - transient_login_code(error), + &transient_login_code(error), + "login transient replay hint", ) .await; } } -/// Result-framed transient Reply on a non-terminal failed Register. The SDK -/// decodes the nonzero result code and replays the same login on the same -/// connection. Only call for transient errors -- see -/// [`surface_login_failure`]. -#[allow(clippy::future_not_send)] -async fn send_login_transient_reply( - shard: &Rc>, - transport_client_id: u128, - request_header: &RoutedRequestHeader, - code: IggyError, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let commit = current_metadata_commit(shard); - let reply = build_result_rejection_reply(request_header, commit, code.as_code()); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send login transient reply" - ); - } -} - /// Wire code for a transient (non-terminal) login/register failure. /// /// `TransientNotAccepted` asserts nothing was committed: the register never @@ -420,53 +390,6 @@ const fn eviction_reason_for(error: &LoginRegisterError) -> EvictionReason { } } -/// Reject a replicated request from an unbound transport with a typed -/// `Eviction(NoSession)` frame: the session the client believes it has is -/// gone, so it must register again. Pre-auth non-replicated reads get a -/// deny Reply instead (no session exists, so nothing is evicted). -/// -/// The SDK's reply decoder maps eviction reasons to typed errors -/// (`NoSession` -> `Unauthenticated`), so clients fail fast with the same -/// error the legacy server returns instead of a body-decode failure. The -/// eviction context is best-effort off the metadata consensus (peer shards -/// have none; zeroes are cosmetic -- the SDK only reads the reason). -#[allow(clippy::future_not_send)] -pub(in crate::dispatch) async fn send_unauthenticated_eviction( - shard: &Rc>, - transport_client_id: u128, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let ctx = shard.plane.metadata().consensus.as_ref().map_or( - consensus::EvictionContext { - cluster: 0, - view: 0, - replica: 0, - }, - consensus::EvictionContext::from_consensus, - ); - let eviction = consensus::build_eviction_message( - ctx, - transport_client_id, - iggy_binary_protocol::EvictionReason::NoSession, - ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, eviction.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send unauthenticated eviction" - ); - } -} - /// Per-shard heartbeat verifier: evict connections that have not pinged within /// `1.2 x interval`. Mirrors the legacy `verify_heartbeats` periodic task. /// Eviction reuses the disconnect path (drops the client from its consumer @@ -552,35 +475,21 @@ async fn evict_stale_client( if let Some((vsr_client_id, session)) = bound { submit_disconnect_logout(Rc::clone(shard), vsr_client_id, session); } - let ctx = shard.plane.metadata().consensus.as_ref().map_or( - consensus::EvictionContext { - cluster: 0, - view: 0, - replica: 0, - }, - consensus::EvictionContext::from_consensus, - ); - let eviction = consensus::build_eviction_message( - ctx, + // The eviction itself is done and observed at this point (session + // dropped, `Logout` submitted). The client notice below is best-effort + // and logs its own send failure, so this line must not claim it. + warn!( transport_client_id, - iggy_binary_protocol::EvictionReason::StaleClient, + "evicted stale client (missed heartbeat); sending the eviction notice" ); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, eviction.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send stale-client eviction" - ); - } else { - warn!( - transport_client_id, - "evicted stale client (missed heartbeat)" - ); - } + send_eviction( + shard, + transport_client_id, + transport_client_id, + EvictionReason::StaleClient, + "stale_client_eviction", + ) + .await; } /// Answer a backup's forwarded `Register` from the node it named primary. @@ -1180,17 +1089,14 @@ pub(in crate::dispatch) async fn handle_logout_request( ); let commit = current_metadata_commit(shard); let reply = build_empty_reply(request.header(), transport_client_id, 0, commit); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send unbound logout reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "unbound_logout_reply", + ) + .await; return; }; @@ -1210,17 +1116,14 @@ pub(in crate::dispatch) async fn handle_logout_request( commit, transient_logout_code(&error).as_code(), ); - if let Err(send_error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %send_error, - "failed to send logout deny reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::TypedDeny, + "logout_transient_deny", + ) + .await; return; } }; @@ -1228,17 +1131,14 @@ pub(in crate::dispatch) async fn handle_logout_request( sessions.borrow_mut().remove_connection(transport_client_id); let reply = build_empty_reply(request.header(), vsr_client_id, session, commit); - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, reply.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - "failed to send logout reply" - ); - } + send_host_frame( + &shard.bus, + transport_client_id, + reply.into_generic().into_frozen(), + FrameChannel::Reply, + "logout_reply", + ) + .await; } /// Preserve the client identity when a Logout may already have entered the @@ -1281,11 +1181,12 @@ pub(in crate::dispatch) async fn handle_login_register_request( transport_client_id, "rejecting login: body has no decodable version prefix" ); - send_login_eviction( + send_eviction( shard, transport_client_id, vsr_client_id, EvictionReason::MalformedLogin, + "login_rejection", ) .await; return; @@ -1298,11 +1199,12 @@ pub(in crate::dispatch) async fn handle_login_register_request( sdk_version = %version_info.sdk_version, "rejecting login: incompatible protocol version" ); - send_login_eviction( + send_eviction( shard, transport_client_id, vsr_client_id, EvictionReason::IncompatibleProtocol, + "login_rejection", ) .await; return; @@ -1395,11 +1297,12 @@ pub(in crate::dispatch) async fn handle_login_register_request( transport_client_id, "rejecting register request: invalid credentials" ); - send_login_eviction( + send_eviction( shard, transport_client_id, request.header().client, EvictionReason::InvalidCredentials, + "login_rejection", ) .await; return; @@ -1409,61 +1312,16 @@ pub(in crate::dispatch) async fn handle_login_register_request( transport_client_id, "rejecting register request with unsupported payload shape" ); - send_login_eviction( + send_eviction( shard, transport_client_id, request.header().client, EvictionReason::MalformedLogin, + "login_rejection", ) .await; } -/// Best-effort login-rejection eviction. Terminal one-way frame; a gone -/// connection has nothing to recover, so the send error is logged and -/// dropped. Consensus context (cluster/view/replica) is stamped on the -/// metadata shard and zeroed elsewhere -- the SDK only reads the reason, -/// plus the protocol window on `IncompatibleProtocol`. -#[allow(clippy::future_not_send)] -pub(in crate::dispatch) async fn send_login_eviction( - shard: &Rc>, - transport_client_id: u128, - vsr_client_id: u128, - reason: EvictionReason, -) where - B: ShellBus, - MJ: JournalHandle + 'static, - MJ::Target: Journal, Header = PrepareHeader>, - S: 'static, - SB: SuperblockStore + 'static, -{ - let ctx = shard.plane.metadata().consensus.as_ref().map_or( - EvictionContext { - cluster: 0, - view: 0, - replica: 0, - }, - EvictionContext::from_consensus, - ); - let eviction = match reason { - EvictionReason::IncompatibleProtocol => { - build_incompatible_protocol_eviction_message(ctx, vsr_client_id) - } - _ => build_eviction_message(ctx, vsr_client_id, reason), - }; - if let Err(error) = shard - .bus - .send_to_client(transport_client_id, eviction.into_generic().into_frozen()) - .await - { - warn!( - transport_client_id, - error = %error, - reason = ?reason, - "failed to send login eviction" - ); - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/core/server/src/dispatch/test_support.rs b/core/server/src/dispatch/test_support.rs index e52578c64d..c0a0057345 100644 --- a/core/server/src/dispatch/test_support.rs +++ b/core/server/src/dispatch/test_support.rs @@ -54,12 +54,16 @@ pub type RecordedReplies = Rc)>>>; pub type RecordedReplicaSends = Rc)>>>; /// Records every client-bound reply and replica-bound frame (target + -/// bytes) instead of writing to a socket; everything else is a no-op. The -/// two `ShellBus` halves are stubbed. +/// bytes) instead of writing to a socket, and counts connection-lost hook +/// installs; everything else is a no-op. The two `ShellBus` halves are +/// stubbed. #[derive(Debug, Clone, Default)] pub struct SpyBus { pub client_replies: RecordedReplies, pub replica_sends: RecordedReplicaSends, + /// Installs of the client connection-lost hook, one per handler built + /// on this bus. + pub connection_lost_hooks: Rc>, /// Resolve [`MessageBus::sleep`] immediately instead of arming a real /// timer. The register forward is the only path here that races a /// timer, and its budget is five seconds -- too long to wait for in a @@ -143,7 +147,10 @@ impl ConnectionInstaller for SpyBus { fn client_meta(&self, _client_id: u128) -> Option> { None } - fn set_client_connection_lost_fn(&self, _f: ClientConnectionLostFn) {} + fn set_client_connection_lost_fn(&self, _f: ClientConnectionLostFn) { + self.connection_lost_hooks + .set(self.connection_lost_hooks.get() + 1); + } } /// Consensus incarnations standing for two successive boots of one node, as diff --git a/core/server/src/http/handlers.rs b/core/server/src/http/handlers.rs index 8483138096..5b6c2d07ef 100644 --- a/core/server/src/http/handlers.rs +++ b/core/server/src/http/handlers.rs @@ -120,10 +120,6 @@ use shard::{PartitionRead, PartitionReadReply}; use crate::dispatch::partition::{resolve_consumer_offset_request, resolve_poll_request}; use crate::dispatch::session_ops::{verify_login_credentials, verify_pat_credentials}; -use crate::dispatch::{ - validate_option_keys, validate_topic_bounds, validate_topic_size_floor, - warn_unenforceable_topic_size, warn_unenforceable_topic_size_on_partition_add, -}; use crate::http::error::{ Consistency, ConsistencyQuery, CustomError, PartitionWriteError, ProduceAck, ProduceQuery, ReadError, WriteError, @@ -149,6 +145,10 @@ use crate::http::wire::{ use crate::responses::{ build_polled_messages_body, build_raw_pat_reply, connected_client_to_response, }; +use crate::rewrite::{ + validate_option_keys, validate_topic_bounds, validate_topic_size_floor, + warn_unenforceable_topic_size, warn_unenforceable_topic_size_on_partition_add, +}; use crate::snapshot; /// `GET /ping` response body, matching the legacy HTTP server's health probe. diff --git a/core/server/src/http/submit.rs b/core/server/src/http/submit.rs index 0babb39528..a7d981880d 100644 --- a/core/server/src/http/submit.rs +++ b/core/server/src/http/submit.rs @@ -23,12 +23,10 @@ use std::rc::Rc; use std::time::{Duration, Instant}; use bytes::Bytes; -use consensus::MetadataHandle; use futures::channel::oneshot; use iggy_binary_protocol::consensus::Command; use iggy_binary_protocol::{GenericHeader, Operation, ReplyHeader, RoutedRequestHeader}; use iggy_common::IggyError; -use metadata::impls::metadata::StreamsFrontend; use server_common::{MESSAGE_ALIGN, Message, iobuf::Frozen}; use tracing::warn; @@ -43,9 +41,8 @@ use crate::http::reply::{ use crate::http::session::HttpSession; use crate::http::state::HttpInner; use crate::http::wire::build_request_message; -use crate::pat::rewrite_pat_request_for_user; +use crate::rewrite::http_chain; use crate::shell::ServerShard; -use crate::users::maybe_rewrite_user_password_request; use crate::wire::request_body; /// Bound on a partition write's (produce / consumer-offset write) wait for its @@ -198,22 +195,8 @@ async fn submit_gated( request_id, body, ); - let (message, raw_token) = rewrite_pat_request_for_user( - session.user_id, - max_tokens_per_user, - |user_id| { - shard - .plane - .metadata() - .mux_stm - .users() - .read(|users| users.pat_count_of(user_id)) - }, - message, - ) - .map_err(WriteError::Rejected)?; - let message = - maybe_rewrite_user_password_request(shard, message).map_err(WriteError::Rejected)?; + let (message, raw_token) = http_chain(shard, session.user_id, max_tokens_per_user, message) + .map_err(WriteError::Rejected)?; // `DeleteSegments` is not itself a consensus op: resolve it to the metadata // `TruncatePartition` that commits the trim before it reaches consensus, // mirroring the TCP dispatch. The truncate rides this session's burned diff --git a/core/server/src/lib.rs b/core/server/src/lib.rs index 1b1036f0ee..73bddca703 100644 --- a/core/server/src/lib.rs +++ b/core/server/src/lib.rs @@ -45,6 +45,7 @@ pub(crate) mod consumer_group; pub(crate) mod dispatch; pub(crate) mod pat; pub(crate) mod responses; +pub(crate) mod rewrite; pub mod session_manager; pub mod shell; pub(crate) mod users; diff --git a/core/server/src/responses.rs b/core/server/src/responses.rs index 4c48c0514b..41cff90961 100644 --- a/core/server/src/responses.rs +++ b/core/server/src/responses.rs @@ -594,10 +594,15 @@ where // than let the catch-all's empty-ok attest an artifact that was never // produced. GET_SNAPSHOT_FILE_CODE => Err(IggyError::InvalidCommand), + // Sequenced AFTER the named arms above, so flush keeps answering + // `FeatureUnavailable`. A table-listed non-replicated code with no arm + // is a routing bug and an unknown code is a client bug; the empty-ok + // that used to cover both attested a read that never ran. Only the + // named arms return `Empty`, and there it means "resolved to nothing" + // (the 404 the HTTP path maps). _ => match iggy_binary_protocol::dispatch::lookup_command(code) { - Some(meta) if !meta.is_replicated() => Ok(NonReplicatedResponse::Empty), - Some(_) => Err(IggyError::FeatureUnavailable), - None => Err(IggyError::InvalidCommand), + Some(meta) if meta.is_replicated() => Err(IggyError::FeatureUnavailable), + _ => Err(IggyError::InvalidCommand), }, } } diff --git a/core/server/src/rewrite.rs b/core/server/src/rewrite.rs new file mode 100644 index 0000000000..5f00c588cc --- /dev/null +++ b/core/server/src/rewrite.rs @@ -0,0 +1,570 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! The two pre-consensus request-rewrite chains, side by side. +//! +//! [`tcp_chain`] serves the TCP funnel, [`http_chain`] the HTTP submit. Each +//! step rewrites or validates a request BEFORE consensus, so a rejected +//! request burns no replicated log entry and no plaintext secret enters +//! consensus. The chains enter the PAT rewrite through different functions +//! on purpose: TCP resolves the acting user from the transport +//! `SessionManager` ([`maybe_rewrite_pat_request`]), HTTP authenticates +//! against its own session table and passes the resolved `user_id` +//! ([`rewrite_pat_request_for_user`]). +//! +//! Three more links complete the chains but live in their spines, because +//! they fire partition-read mesh RPCs or are plane-specific. Two are async: +//! the consumer-group Join/Leave enrichment ([`crate::consumer_group`], the +//! TCP funnel calls it after [`tcp_chain`]) and the `DeleteSegments` -> +//! `TruncatePartition` resolution +//! (`dispatch::partition::resolve_delete_segments_truncate`, called by both +//! spines). The third, the consumer-offset rewrite on the partition path +//! (`consumer_group::maybe_rewrite_consumer_offset_request`, called by +//! `dispatch::partition`), is synchronous. + +use crate::pat::{maybe_rewrite_pat_request, rewrite_pat_request_for_user}; +use crate::segment_cleaner::UNENFORCEABLE_TOPIC_SIZE_WARN; +use crate::session_manager::SessionManager; +use crate::shell::{ShellBus, ShellShard}; +use crate::users::maybe_rewrite_user_password_request; +use crate::wire::request_body; +use consensus::MetadataHandle; +use iggy_binary_protocol::requests::partitions::{ + CreatePartitionsRequest, DeletePartitionsRequest, +}; +use iggy_binary_protocol::requests::streams::{CreateStreamRequest, UpdateStreamRequest}; +use iggy_binary_protocol::requests::topics::{CreateTopicRequest, UpdateTopicRequest}; +use iggy_binary_protocol::requests::users::{CreateUserRequest, UpdateUserRequest}; +use iggy_binary_protocol::{ + MAX_PARTITIONS_PER_REQUEST, Operation, PrepareHeader, RoutedRequestHeader, WireDecode, + WireIdentifier, WireOptions, +}; +use iggy_common::{ + IggyByteSize, IggyError, MaxTopicSize, TopicCreateOptions, UPDATABLE_STREAM_OPTION_KEYS, + UPDATABLE_TOPIC_OPTION_KEYS, UPDATABLE_USER_OPTION_KEYS, validate_preallocated_topic_bytes, + validate_topic_segment_size, +}; +use journal::superblock::SuperblockStore; +use journal::{Journal, JournalHandle}; +use metadata::impls::metadata::StreamsFrontend; +use metadata::stm::stream::Streams; +use server_common::Message; +use std::cell::RefCell; +use std::rc::Rc; +use tracing::warn; + +/// The pre-consensus rewrite stages, in chain order. +/// +/// One owner for the set: the funnel's consumer-group rewrite runs outside +/// [`tcp_chain`] but denies through the same path, so it names a variant +/// here instead of passing a bare literal. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RewriteStage { + PersonalAccessToken, + UserPassword, + StaticBounds, + ConsumerGroup, +} + +impl RewriteStage { + /// The `context` log label, in the same `snake_case` shape as every other + /// frame context. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::PersonalAccessToken => "personal_access_token", + Self::UserPassword => "user_password", + Self::StaticBounds => "static_bounds", + Self::ConsumerGroup => "consumer_group", + } + } +} + +/// A staged pre-consensus rejection: `stage` labels the chain step for the +/// deny log line, `error` is the typed code the deny reply carries. +pub struct RewriteDeny { + pub stage: RewriteStage, + pub error: IggyError, +} + +/// The TCP funnel's pre-consensus rewrite chain, in order: the PAT rewrite +/// (resolving the acting user from the transport `sessions` binding), the +/// password rewrite, then the static bounds gate. Mirrors [`http_chain`] +/// plus that bounds step, which the binary wire needs because it has no +/// `command.validate()` layer. Returns the rewritten request and the raw +/// PAT token the funnel substitutes into the committed reply; a rejection +/// names the failing stage for the deny log line. +pub fn tcp_chain( + shard: &Rc>, + sessions: &Rc>, + transport_client_id: u128, + max_tokens_per_user: u32, + request: Message, +) -> Result<(Message, Option), RewriteDeny> +where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let (request, raw_pat_token) = maybe_rewrite_pat_request( + sessions, + transport_client_id, + max_tokens_per_user, + |user_id| { + shard + .plane + .metadata() + .mux_stm + .users() + .read(|users| users.pat_count_of(user_id)) + }, + request, + ) + // Token cap reached, malformed body, or a lost session binding. + .map_err(|error| RewriteDeny { + stage: RewriteStage::PersonalAccessToken, + error, + })?; + // Hash raw passwords and, for ChangePassword, verify the current password + // on the primary before replication; see `crate::users`. Replicas store the + // hash directly. A wrong current password is not denied here: it rides + // consensus and applies as a committed InvalidCredentials no-op, so the only + // Err returned is a malformed body. + let request = + maybe_rewrite_user_password_request(shard, request).map_err(|error| RewriteDeny { + stage: RewriteStage::UserPassword, + error, + })?; + static_bounds(shard, &request).map_err(|error| RewriteDeny { + stage: RewriteStage::StaticBounds, + error, + })?; + Ok((request, raw_pat_token)) +} + +/// The HTTP submit's pre-consensus rewrite chain: the PAT rewrite for the +/// already-authenticated `user_id`, then the password rewrite. Mirrors +/// [`tcp_chain`] minus the session lookup (the HTTP listener authenticates +/// against its own session table and resolves the acting user itself) and +/// minus the static bounds step: HTTP enforces the same bounds in its +/// handlers via `command.validate()` plus the validators below. Returns the +/// rewritten request and the raw PAT token the caller substitutes into the +/// committed reply. +pub fn http_chain( + shard: &Rc>, + user_id: u32, + max_tokens_per_user: u32, + request: Message, +) -> Result<(Message, Option), IggyError> +where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + let (request, raw_token) = rewrite_pat_request_for_user( + user_id, + max_tokens_per_user, + |user_id| { + shard + .plane + .metadata() + .mux_stm + .users() + .read(|users| users.pat_count_of(user_id)) + }, + request, + )?; + let request = maybe_rewrite_user_password_request(shard, request)?; + Ok((request, raw_token)) +} + +/// Per-request partitions-count cap, shared by create-topic, create-partitions +/// and delete-partitions admission. Runs pre-consensus like +/// [`validate_topic_bounds`]: an oversized count must not burn a replicated +/// log entry (create-partitions admission would also allocate that many +/// consensus-group ids before replicating). +/// +/// Zero passes here because a zero-partition TOPIC is legal (legacy +/// `create_topic` admits `0..=MAX`); the add/remove requests reject it in +/// [`validate_partitions_change_count`]. +const fn validate_partitions_count(partitions_count: u32) -> Result<(), IggyError> { + // The two transports carry the cap under different names: this one on the + // binary path, `MAX_PARTITIONS_COUNT` in the HTTP DTO validators. The + // parity is a documented contract ([`http_chain`]), so it fails the build + // rather than the next cross-transport test. + const _: () = assert!( + MAX_PARTITIONS_PER_REQUEST == iggy_common::MAX_PARTITIONS_COUNT, + "the binary and HTTP partitions-count caps must stay equal" + ); + + if partitions_count > MAX_PARTITIONS_PER_REQUEST { + return Err(IggyError::TooManyPartitions); + } + Ok(()) +} + +/// [`validate_partitions_count`] plus the zero rejection that create-partitions +/// and delete-partitions carry: adding or removing zero partitions is a no-op +/// that would still burn a replicated log entry, bump `Streams::revision` and +/// force every shard through a rebalance pass. Legacy rejects it with +/// `TooManyPartitions` in both handlers (`1..=MAX` on create, `== 0` on +/// delete), so the code matches rather than inventing a new one. +const fn validate_partitions_change_count(partitions_count: u32) -> Result<(), IggyError> { + if partitions_count == 0 { + return Err(IggyError::TooManyPartitions); + } + validate_partitions_count(partitions_count) +} + +/// Static create-topic bounds shared by the TCP and HTTP ingresses. Runs +/// pre-consensus: a rejected request must not burn a replicated log entry, +/// and `prepare_request` errors evict the session instead of denying typed. +/// `ServerDefault` is exempt from the size floor (it resolves against server +/// config at admission, matching legacy); `Unlimited` passes numerically. +/// `segment_size_bytes` is the topic's RESOLVED segment size (explicit +/// option, else this node's default), so a per-topic segment above the +/// global default still floors the topic cap. +pub fn validate_topic_bounds( + partitions_count: u32, + max_topic_size: MaxTopicSize, + segment_size_bytes: u64, +) -> Result<(), IggyError> { + validate_partitions_count(partitions_count)?; + validate_topic_size_floor(max_topic_size, segment_size_bytes) +} + +/// A topic cap below one segment can never be enforced: the first segment +/// already exceeds it. Split out of [`validate_topic_bounds`] because update +/// admission checks the cap without a partitions count to check. +pub fn validate_topic_size_floor( + max_topic_size: MaxTopicSize, + segment_size_bytes: u64, +) -> Result<(), IggyError> { + if !matches!(max_topic_size, MaxTopicSize::ServerDefault) + && max_topic_size.as_bytes_u64() < segment_size_bytes + { + return Err(IggyError::InvalidTopicSize( + max_topic_size, + IggyByteSize::from(segment_size_bytes), + )); + } + Ok(()) +} + +/// Announce an accepted `max_topic_size` the server cannot enforce as written. +/// +/// [`validate_topic_size_floor`] admits any cap of one segment or more, but +/// retention runs PER PARTITION and floors each partition's share at one SEALED +/// segment, which reaches up to one maximum bus frame past `segment_size`. A cap +/// between the two is stored and echoed back verbatim while the server actually +/// keeps `(segment_size + max_message_size) * partitions_count`, so the only +/// moment an operator can be told is the one where they set it. +/// +/// Warns rather than rejects: which caps are accepted is client-visible wire +/// behavior, and tightening it would break topics that already exist. +pub fn warn_unenforceable_topic_size( + max_topic_size: MaxTopicSize, + segment_size_bytes: u64, + max_message_size_bytes: usize, + partitions_count: u32, +) { + let MaxTopicSize::Custom(configured) = max_topic_size else { + return; + }; + let max_message_size_bytes = u64::try_from(max_message_size_bytes).unwrap_or(u64::MAX); + let per_partition_floor = segment_size_bytes.saturating_add(max_message_size_bytes); + let topic_floor = per_partition_floor.saturating_mul(u64::from(partitions_count)); + if configured.as_bytes_u64() >= topic_floor { + return; + } + warn!( + max_topic_size = configured.as_bytes_u64(), + partitions_count, + segment_size = segment_size_bytes, + enforced_per_partition = per_partition_floor, + "{UNENFORCEABLE_TOPIC_SIZE_WARN}" + ); +} + +/// Announce the same unenforceable cap when partitions are ADDED to a topic. +/// +/// The cap is topic-wide but enforcement is per partition, so every added +/// partition shrinks the share: a cap that cleared the floor when the topic was +/// created can stop clearing it here. The request carries only the delta, so +/// the stored cap, segment size and current partition count come from metadata. +pub fn warn_unenforceable_topic_size_on_partition_add( + streams: &Streams, + stream_id: &WireIdentifier, + topic_id: &WireIdentifier, + max_message_size_bytes: usize, + added_partitions_count: u32, +) { + let Some(((stream_slab, topic_slab), _)) = streams.partition_count_context(stream_id, topic_id) + else { + return; + }; + let Some((_, max_topic_size, partitions_count, segment_size)) = + streams.topic_retention_config(stream_slab, topic_slab) + else { + return; + }; + warn_unenforceable_topic_size( + max_topic_size, + segment_size.map_or(iggy_common::DEFAULT_SEGMENT_SIZE, |segment_size| { + segment_size.as_bytes_u64() + }), + max_message_size_bytes, + u32::try_from(partitions_count) + .unwrap_or(u32::MAX) + .saturating_add(added_partitions_count), + ); +} + +/// Reject option keys outside the resource's catalog, pre-consensus. Unknown +/// keys are rejected rather than skipped: a silently ignored knob would hand +/// the client server defaults without it ever learning. Streams and users +/// have no catalog keys yet, so `known` is empty for both until one lands. +pub fn validate_option_keys(options: &WireOptions, known: &[&str]) -> Result<(), IggyError> { + for entry in options { + // Wire validation already enforced UTF-8 string keys. + let key = String::from_utf8_lossy(entry.key); + if !known.contains(&key.as_ref()) { + return Err(IggyError::UnsupportedOptionKey(key.into_owned())); + } + } + Ok(()) +} + +/// Static bounds run pre-consensus so a rejected request burns no +/// replicated log entry; HTTP covers the same bounds via +/// `command.validate()`. A body that fails to decode denies typed too +/// (`InvalidCommand`), instead of riding consensus just to fail there. +#[allow(clippy::too_many_lines)] +fn static_bounds( + shard: &Rc>, + request: &Message, +) -> Result<(), IggyError> +where + B: ShellBus, + MJ: JournalHandle + 'static, + MJ::Target: Journal, Header = PrepareHeader>, + S: 'static, + SB: SuperblockStore + 'static, +{ + match request.header().operation { + Operation::CreateTopic => CreateTopicRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|create_topic| { + // `parse` doubles as the catalog gate: an unknown key or a + // malformed value denies typed here, pre-consensus. + let options = TopicCreateOptions::parse(&create_topic.options)?; + if let Some(segment_size) = options.segment_size { + validate_topic_segment_size( + segment_size.as_bytes_u64(), + iggy_common::MAX_TOPIC_SEGMENT_SIZE, + )?; + } + let segment_size = options.segment_size.map_or_else( + || iggy_common::DEFAULT_SEGMENT_SIZE, + |segment_size| segment_size.as_bytes_u64(), + ); + if options + .preallocate_segments + .unwrap_or(iggy_common::DEFAULT_PREALLOCATE_SEGMENTS) + { + validate_preallocated_topic_bytes(segment_size, create_topic.partitions_count)?; + } + let max_topic_size = options + .max_topic_size + .unwrap_or(MaxTopicSize::ServerDefault); + validate_topic_bounds(create_topic.partitions_count, max_topic_size, segment_size)?; + warn_unenforceable_topic_size( + max_topic_size, + segment_size, + shard.bus_max_message_size(), + create_topic.partitions_count, + ); + Ok(()) + }), + Operation::CreatePartitions => CreatePartitionsRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|create_partitions| { + validate_partitions_change_count(create_partitions.partitions_count)?; + let metadata = shard.plane.metadata(); + warn_unenforceable_topic_size_on_partition_add( + metadata.mux_stm.streams(), + &create_partitions.stream_id, + &create_partitions.topic_id, + shard.bus_max_message_size(), + create_partitions.partitions_count, + ); + Ok(()) + }), + Operation::DeletePartitions => DeletePartitionsRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|delete_partitions| { + validate_partitions_change_count(delete_partitions.partitions_count) + }), + // Only the updatable subset: the create-time knobs are pushed to + // partitions when the topic is built and nothing re-pushes them, so + // accepting one here would store a value no partition ever sees. + Operation::UpdateTopic => UpdateTopicRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|update_topic| { + validate_option_keys(&update_topic.options, UPDATABLE_TOPIC_OPTION_KEYS)?; + let options = TopicCreateOptions::parse(&update_topic.options)?; + let Some(max_topic_size) = options.max_topic_size else { + return Ok(()); + }; + // An update can lower the cap below one segment just as a + // create can, and the stored map would then report a size the + // topic can never enforce. The floor is this topic's own + // segment size, since that key is create-only. + let metadata = shard.plane.metadata(); + let streams = metadata.mux_stm.streams(); + let segment_size = streams + .topic_segment_size(&update_topic.stream_id, &update_topic.topic_id) + .map_or_else( + || iggy_common::DEFAULT_SEGMENT_SIZE, + |segment_size| segment_size.as_bytes_u64(), + ); + validate_topic_size_floor(max_topic_size, segment_size)?; + let partitions_count = streams + .topic_partitions_count(&update_topic.stream_id, &update_topic.topic_id) + .unwrap_or(0); + warn_unenforceable_topic_size( + max_topic_size, + segment_size, + shard.bus_max_message_size(), + u32::try_from(partitions_count).unwrap_or(u32::MAX), + ); + Ok(()) + }), + Operation::UpdateStream => UpdateStreamRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|update_stream| { + validate_option_keys(&update_stream.options, UPDATABLE_STREAM_OPTION_KEYS) + }), + Operation::UpdateUser => UpdateUserRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|update_user| { + validate_option_keys(&update_user.options, UPDATABLE_USER_OPTION_KEYS) + }), + Operation::CreateStream => CreateStreamRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|create_stream| validate_option_keys(&create_stream.options, &[])), + Operation::CreateUser => CreateUserRequest::decode_from(request_body(request)) + .map_err(|_| IggyError::InvalidCommand) + .and_then(|create_user| validate_option_keys(&create_user.options, &[])), + _ => Ok(()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn create_topic_bounds_deny_pre_consensus() { + let segment_size = iggy_common::DEFAULT_SEGMENT_SIZE; + assert!(segment_size > 0, "default segment size must be nonzero"); + + assert!( + validate_topic_bounds( + MAX_PARTITIONS_PER_REQUEST, + MaxTopicSize::ServerDefault, + segment_size + ) + .is_ok(), + "the partition cap itself is admissible" + ); + assert!( + matches!( + validate_topic_bounds( + MAX_PARTITIONS_PER_REQUEST + 1, + MaxTopicSize::ServerDefault, + segment_size + ), + Err(IggyError::TooManyPartitions) + ), + "one past the partition cap must deny" + ); + // ServerDefault is numerically 0 yet exempt from the segment-size + // floor: it resolves against server config, matching legacy. + assert!(validate_topic_bounds(1, MaxTopicSize::ServerDefault, segment_size).is_ok()); + assert!(validate_topic_bounds(1, MaxTopicSize::Unlimited, segment_size).is_ok()); + let below_floor = MaxTopicSize::Custom((segment_size - 1).into()); + assert!( + matches!( + validate_topic_bounds(1, below_floor, segment_size), + Err(IggyError::InvalidTopicSize(size, floor)) + if size == below_floor && floor == IggyByteSize::from(segment_size) + ), + "custom size below the segment size must deny with the bounds" + ); + let at_floor = MaxTopicSize::Custom(IggyByteSize::from(segment_size)); + assert!( + validate_topic_bounds(1, at_floor, segment_size).is_ok(), + "a topic exactly one segment large is admissible" + ); + } + + #[test] + fn partitions_count_cap_denies_pre_consensus() { + assert!( + validate_partitions_count(MAX_PARTITIONS_PER_REQUEST).is_ok(), + "the cap itself is admissible" + ); + assert!( + matches!( + validate_partitions_count(MAX_PARTITIONS_PER_REQUEST + 1), + Err(IggyError::TooManyPartitions) + ), + "one past the cap must deny" + ); + // Zero passes the shared cap because a zero-partition TOPIC is legal + // (legacy `create_topic` admits `0..=MAX`). + assert!(validate_partitions_count(0).is_ok()); + } + + #[test] + fn zero_partitions_change_denies_pre_consensus() { + // Adding or removing zero partitions is a no-op that would still burn + // a replicated log entry and force a rebalance. Legacy rejects it with + // `TooManyPartitions` in both handlers, so the code matches. + assert!( + matches!( + validate_partitions_change_count(0), + Err(IggyError::TooManyPartitions) + ), + "adding or removing zero partitions must deny" + ); + assert!(validate_partitions_change_count(1).is_ok()); + assert!(validate_partitions_change_count(MAX_PARTITIONS_PER_REQUEST).is_ok()); + assert!( + matches!( + validate_partitions_change_count(MAX_PARTITIONS_PER_REQUEST + 1), + Err(IggyError::TooManyPartitions) + ), + "the cap still applies" + ); + } +} diff --git a/core/server/src/server_error.rs b/core/server/src/server_error.rs index 7f0f86c5e3..b57eed3ead 100644 --- a/core/server/src/server_error.rs +++ b/core/server/src/server_error.rs @@ -122,6 +122,19 @@ pub enum ServerError { poll: std::time::Duration, drain: std::time::Duration, }, + #[error("system.sharding.shutdown_join_timeout must be <= {max:?}; got {value:?}")] + InvalidShutdownJoinTimeout { + value: std::time::Duration, + max: std::time::Duration, + }, + #[error( + "system.sharding.shutdown_join_timeout ({join:?}) must be >= \ + shutdown_drain_timeout ({drain:?})" + )] + ShutdownJoinBelowDrain { + join: std::time::Duration, + drain: std::time::Duration, + }, #[error("failed to serialize current server config")] CurrentConfigSerialize(#[source] toml::ser::Error), #[error("failed to write current server config at {path}")] diff --git a/core/server/src/session_manager.rs b/core/server/src/session_manager.rs index 84635a8b4f..26ee61496c 100644 --- a/core/server/src/session_manager.rs +++ b/core/server/src/session_manager.rs @@ -26,13 +26,30 @@ //! transport connection and the consensus-level `(client_id, session)` pair. use crate::cluster_meta::ClusterRoster; +use ahash::AHashMap; use message_bus::installer::conn_info::ClientTransportKind; use shard::ConnectedClientInfo; -use std::collections::HashMap; use std::net::SocketAddr; use std::rc::Rc; use std::time::{Duration, Instant}; +/// What the request funnel resolves from one `connections` lookup per frame. +/// +/// The bound consensus session, the acting user, and the transport peer +/// address. `Default` (everything absent, no address) stands for a +/// connection neither this map nor the bus knows. +#[derive(Debug, Clone, Copy, Default)] +pub struct ConnectionContext { + /// `(client_id, session)` once register committed, `None` before. + pub bound: Option<(u128, u64)>, + /// Acting user from `login`, `None` while still `Connected`. + pub user_id: Option, + /// Peer address recorded by [`SessionManager::ensure_connection`]; the + /// non-replicated reads pick the advertised address from it, and + /// `None` degrades to the catch-all address. + pub address: Option, +} + /// Connection lifecycle states. /// /// ```text @@ -95,10 +112,14 @@ pub struct Connection { /// per consensus session). If a client reconnects with the same `client_id`, /// the old connection must be evicted first. pub struct SessionManager { - connections: HashMap, + /// `ahash` over `std`: connection and client ids are server-minted, so + /// there is no `HashDoS` surface, and both maps sit on the per-frame path. + /// Neither is order-sensitive (`iter_clients` and `collect_stale` both + /// consume the whole map). + connections: AHashMap, /// Reverse index: `client_id` → `connection_id` for fast lookup when /// a consensus reply arrives and needs routing to the right connection. - client_to_connection: HashMap, + client_to_connection: AHashMap, /// This shard's copy of the configured cluster roster, served by the /// `GetClusterMetadata` read. Lives here because it is the /// per-shard context already threaded to the non-replicated read path; @@ -110,8 +131,8 @@ impl SessionManager { #[must_use] pub fn new() -> Self { Self { - connections: HashMap::new(), - client_to_connection: HashMap::new(), + connections: AHashMap::new(), + client_to_connection: AHashMap::new(), cluster_roster: Rc::new(ClusterRoster::disabled()), } } @@ -144,12 +165,30 @@ impl SessionManager { }); } - /// Record a liveness heartbeat (`ping`) for a connection, resetting its - /// staleness clock. No-op for an unknown connection. - pub fn record_heartbeat(&mut self, connection_id: u128) { - if let Some(conn) = self.connections.get_mut(&connection_id) { - conn.last_heartbeat = Instant::now(); - } + /// The request funnel's per-frame view of a connection: stamp the + /// liveness clock and read back everything the dispatch arms resolve + /// from it, in ONE map lookup. + /// + /// `None` means the connection is not registered yet, which happens + /// only on a transport's first frame; the caller installs it from the + /// bus metadata ([`Self::ensure_connection`]) and asks again. + pub fn touch_connection(&mut self, connection_id: u128) -> Option { + let conn = self.connections.get_mut(&connection_id)?; + conn.last_heartbeat = Instant::now(); + let (bound, user_id) = match conn.state { + ConnectionState::Bound { + user_id, + client_id, + session, + } => (Some((client_id, session)), Some(user_id)), + ConnectionState::Authenticated { user_id } => (None, Some(user_id)), + ConnectionState::Connected => (None, None), + }; + Some(ConnectionContext { + bound, + user_id, + address: Some(conn.address), + }) } /// Connection ids whose last heartbeat is older than `max_age` -- the @@ -279,7 +318,11 @@ impl SessionManager { /// Look up the consensus session for a connection. /// - /// Returns `(client_id, session)` if the connection is `Bound`, `None` otherwise. + /// Returns `(client_id, session)` if the connection is `Bound`, `None` + /// otherwise. The request funnel reads it through + /// [`Self::touch_connection`] instead, which resolves it in the same + /// lookup as the heartbeat; this is for the session-op paths that only + /// need the binding. #[must_use] pub fn get_session(&self, connection_id: u128) -> Option<(u128, u64)> { let conn = self.connections.get(&connection_id)?; @@ -291,34 +334,6 @@ impl SessionManager { } } - /// The transport-level peer address a connection arrived from, recorded by - /// [`Self::ensure_connection`] for every transport. The non-replicated - /// read path uses it to pick the advertised address a client is told - /// about; `None` (unknown connection) degrades to the catch-all address. - #[must_use] - pub fn connection_address(&self, connection_id: u128) -> Option { - self.connections - .get(&connection_id) - .map(|conn| conn.address) - } - - /// Acting user and transport peer address for a connection, in one map - /// lookup: the non-replicated dispatch path needs both, and the separate - /// accessors would walk the connection map twice per request. - #[must_use] - pub fn read_context(&self, connection_id: u128) -> (Option, Option) { - let Some(conn) = self.connections.get(&connection_id) else { - return (None, None); - }; - let user_id = match conn.state { - ConnectionState::Authenticated { user_id } | ConnectionState::Bound { user_id, .. } => { - Some(user_id) - } - ConnectionState::Connected => None, - }; - (user_id, Some(conn.address)) - } - /// Look up the authenticated user id for a connection. #[must_use] pub fn get_user_id(&self, connection_id: u128) -> Option { diff --git a/core/simulator/src/replica.rs b/core/simulator/src/replica.rs index 869dc61857..638315df36 100644 --- a/core/simulator/src/replica.rs +++ b/core/simulator/src/replica.rs @@ -360,7 +360,7 @@ pub fn new_shard( // every op above the floor must mutate the table. A frontier only fences a // LIVE table a state transfer just replaced. apply_committed_prepare( - &metadata.mux_stm, + &*metadata.mux_stm, &metadata.client_table, true, |_| {},