From ef0c73ccffb2132ae08a0b7973e7f063e2c0d74a Mon Sep 17 00:00:00 2001 From: Krishna Vishal Date: Fri, 4 Sep 2026 11:22:36 +0530 Subject: [PATCH 1/6] fix(partitions): reserve offsets before confirming them A solo node ACKs a send from its in-memory journal, so the client holds a concrete base offset before the threshold-gated flush puts it in a segment. A SIGKILL in that window loses the only record that the offset was issued: boot restores the counter from the highest segment offset, and the next send is confirmed at an offset another message already carried. Consumers positioned above the reissued range also miss the new messages. `offset_frontier` was in the superblock already, but its write gate only ran on view changes, so stable-view traffic never persisted it. Add `offset_reserved`, a monotonic ceiling on the offsets that may have been issued. The append fence claims a block in the superblock before an offset can escape, covering primary mints and backup re-stamps alike, and minting resumes from that ceiling on boot. One write per block rather than per batch: at the default 64 Ki lease and 100k messages per second that is roughly three fsyncs per second, and a crash wastes at most one block of the u64 offset space. `[partition] offset_reservation_lease` sizes it. The ceiling stays a second field because folding it into `offset_frontier` would make a state-transfer offer inside the reserved block read as a rewind. The rewind guard compares against stored data instead, sized segments plus the resident journal, since the append counter may sit a lease block ahead after recovery. A refused claim answers the client with `TransientNotAccepted` at admission, before the request enters the pipeline, where nothing has been admitted and a retry anywhere is safe. The same preflight repeats per promotion out of the request queue, because queued batches can cumulatively cross the lease while they wait. Reaching the fence at the mint instead would leave no answer but fencing the partition and taking the node down. Boot re-anchors the segment tail so the gap a reservation leaves lands on a segment boundary: an empty tail claiming a range is removed, a sized one is sealed, and the next segment is planted at the frontier. `ensure_contiguous_chain` admits a gap ending inside `offset_reserved` for the same reason, and still refuses one reaching past the claimed ceiling. Graceful shutdown collapses the reservation to the frontier, so only crashes spend offsets. --- core/configs/src/server_config/cluster.rs | 9 +- core/configs/src/server_config/defaults.rs | 3 + core/configs/src/server_config/displays.rs | 7 +- core/configs/src/server_config/partition.rs | 109 + core/consensus/src/impls.rs | 4 +- core/consensus/src/vsr_state.rs | 160 +- .../tests/cluster/crash_offset_reuse.rs | 446 +++- core/journal/src/superblock.rs | 4 +- core/metadata/src/impls/recovery.rs | 1 + core/partitions/src/iggy_partition.rs | 2372 +++++++++++++++-- core/partitions/src/lib.rs | 15 + core/partitions/src/log.rs | 27 +- core/partitions/src/segment_anchor.rs | 329 +++ core/partitions/src/state_transfer.rs | 37 +- core/server/config.toml | 39 +- core/server/src/boot/recovery.rs | 104 +- core/server/src/partition_helpers.rs | 226 +- core/server/src/segment_recovery.rs | 390 ++- core/shard/src/lib.rs | 136 +- core/simulator/src/lib.rs | 41 + 20 files changed, 4086 insertions(+), 373 deletions(-) create mode 100644 core/partitions/src/segment_anchor.rs diff --git a/core/configs/src/server_config/cluster.rs b/core/configs/src/server_config/cluster.rs index 88d9b195fa..09b9be3d44 100644 --- a/core/configs/src/server_config/cluster.rs +++ b/core/configs/src/server_config/cluster.rs @@ -276,10 +276,11 @@ pub struct ClusterConfig { /// be > 0 and <= `MAX_REPAIR_CHUNK_MAX`. #[serde(default = "default_repair_chunk_max")] pub repair_chunk_max: usize, - /// How long the metadata superblock may stay unwritable before the replica - /// fail-stops. While wedged the replica is already fenced quorum-invisible - /// and peers elect around it; this converts the log-only limp into a - /// distinct exit status a supervisor can act on. Zero (and the `0` / + /// How long a superblock may stay unwritable before the replica fail-stops. + /// Applies per plane: the metadata superblock, and each PARTITION's own. While + /// wedged the group is already fenced quorum-invisible and peers elect around + /// it; this converts the log-only limp into a distinct exit status a + /// supervisor can act on. Zero (and the `0` / /// `disabled` / `unlimited` sentinels, which all parse to zero) disables /// the fail-stop; nonzero values below /// `MIN_SUPERBLOCK_WEDGED_FATAL_TIMEOUT` are rejected at boot. diff --git a/core/configs/src/server_config/defaults.rs b/core/configs/src/server_config/defaults.rs index 743f25d91a..303ccd0a36 100644 --- a/core/configs/src/server_config/defaults.rs +++ b/core/configs/src/server_config/defaults.rs @@ -41,6 +41,7 @@ use crate::common::server::{ ConsumerGroupConfig, DataMaintenanceConfig, HeartbeatConfig, PersonalAccessTokenConfig, TelemetryConfig, }; +use std::num::NonZeroU32; use std::sync::Arc; // Same embedded TOML the shared sections read; re-exported so sibling @@ -177,6 +178,8 @@ impl Default for PartitionConfig { PartitionConfig { prepare_queue_depth: partition.prepare_queue_depth as usize, dedup_clients_max: partition.dedup_clients_max as usize, + offset_reservation_lease: NonZeroU32::new(partition.offset_reservation_lease as u32) + .expect("the embedded config.toml carries a nonzero offset_reservation_lease"), evicted_ring_capacity: partition.evicted_ring_capacity as usize, evicted_ring_bytes_max: partition.evicted_ring_bytes_max.parse().unwrap(), transfer_served_cache_bytes_max: partition diff --git a/core/configs/src/server_config/displays.rs b/core/configs/src/server_config/displays.rs index aa4f79f980..5c46c7b82c 100644 --- a/core/configs/src/server_config/displays.rs +++ b/core/configs/src/server_config/displays.rs @@ -56,10 +56,11 @@ impl Display for PartitionConfig { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!( f, - "{{ prepare_queue_depth: {}, evicted_ring_capacity: {}, \ - evicted_ring_bytes_max: {}, transfer_served_cache_bytes_max: {}, \ - transfer_artifact_bytes_max: {} }}", + "{{ prepare_queue_depth: {}, offset_reservation_lease: {}, \ + evicted_ring_capacity: {}, evicted_ring_bytes_max: {}, \ + transfer_served_cache_bytes_max: {}, transfer_artifact_bytes_max: {} }}", self.prepare_queue_depth, + self.offset_reservation_lease, self.evicted_ring_capacity, self.evicted_ring_bytes_max, self.transfer_served_cache_bytes_max, diff --git a/core/configs/src/server_config/partition.rs b/core/configs/src/server_config/partition.rs index 6d32c22320..86ee3a7c4c 100644 --- a/core/configs/src/server_config/partition.rs +++ b/core/configs/src/server_config/partition.rs @@ -44,6 +44,7 @@ use crate::common::validators::SEGMENT_MAX_SIZE_BYTES; use configs::ConfigEnv; use iggy_common::{IggyByteSize, Validatable}; use serde::{Deserialize, Serialize}; +use std::num::NonZeroU32; /// Mirrors `consensus::PIPELINE_PREPARE_QUEUE_MAX`. pub const DEFAULT_PARTITION_PREPARE_QUEUE_DEPTH: usize = 32; @@ -97,6 +98,22 @@ pub const DEFAULT_TRANSFER_SERVED_CACHE_BYTES_MAX: u64 = /// count. pub const MAX_TRANSFER_BYTES: u64 = 64 * 1024 * 1024 * 1024; +/// Upper bound on `offset_reservation_lease`: a typo guard, so a slipped digit +/// cannot reach the arithmetic in the append fence. +pub const MAX_OFFSET_RESERVATION_LEASE: u32 = 16 * 1024 * 1024; + +/// Mirrors `partitions::DEFAULT_OFFSET_RESERVATION_LEASE`; pinned against drift +/// by `default_offset_reservation_lease_matches_partitions_constant` in the +/// server crate, which can see both. +pub const DEFAULT_OFFSET_RESERVATION_LEASE: u32 = 64 * 1024; + +/// Serde fallback for a `[partition]` table that omits +/// `offset_reservation_lease`. +fn default_offset_reservation_lease() -> NonZeroU32 { + NonZeroU32::new(DEFAULT_OFFSET_RESERVATION_LEASE) + .expect("DEFAULT_OFFSET_RESERVATION_LEASE is a nonzero literal") +} + /// Mirrors `partitions::EVICTED_RING_CAPACITY`. pub const DEFAULT_EVICTED_RING_CAPACITY: usize = 4096; @@ -144,6 +161,27 @@ pub struct PartitionConfig { /// single partition actually sees, not the node's client total. pub dedup_clients_max: usize, + /// Offsets claimed in the superblock ahead of the mint counter before an + /// append, so a crash-restarted replica resumes above what it confirmed. + /// One superblock write per block: lowering it raises the fsync rate, + /// raising it wastes at most one block per crash. Must be <= + /// [`MAX_OFFSET_RESERVATION_LEASE`]. + /// + /// SINGLE-REPLICA groups only: a replicated group acks once a quorum has + /// journaled the batch, claims nothing, and ignores this. + /// + /// `NonZeroU32` rather than a `u32` with a floor check: a zero lease claims + /// nothing and would write the superblock before every append, and the type + /// is what stops the partition-side setter from having to silently coerce it + /// to one, a coercion that hid wiring errors the validator could not see. + /// + /// The serde fallback is for the providers that do NOT merge the embedded + /// defaults: the file provider does, so a partial `[partition]` table only + /// fails through direct deserialization or an alternate provider. + #[serde(default = "default_offset_reservation_lease")] + #[config_env(leaf)] + pub offset_reservation_lease: NonZeroU32, + /// Entries the evicted ring retains per multi-replica partition for /// journal repair after a peer rejoins. Larger widens the window a /// restarting peer can be served from the ring before falling back to @@ -206,6 +244,14 @@ impl Validatable for PartitionConfig { ); return Err(ConfigurationError::InvalidConfigurationValue); } + if self.offset_reservation_lease.get() > MAX_OFFSET_RESERVATION_LEASE { + eprintln!( + "{COMPONENT} partition.offset_reservation_lease ({}) exceeds the maximum \ + ({MAX_OFFSET_RESERVATION_LEASE})", + self.offset_reservation_lease + ); + return Err(ConfigurationError::InvalidConfigurationValue); + } if self.evicted_ring_capacity == 0 { eprintln!("{COMPONENT} partition.evicted_ring_capacity must be > 0"); return Err(ConfigurationError::InvalidConfigurationValue); @@ -332,6 +378,69 @@ mod tests { assert!(config.validate().is_ok()); } + fn lease(value: u32) -> NonZeroU32 { + NonZeroU32::new(value).expect("a nonzero test lease") + } + + /// A `[partition]` table as an alternate provider hands it over: every other + /// field present, the lease optional. + fn partial_table(lease: Option) -> String { + let entry = lease.map_or_else(String::new, |lease| { + format!(r#""offset_reservation_lease": {lease},"#) + }); + format!( + r#"{{"prepare_queue_depth": 32, {entry} + "dedup_clients_max": 4096, + "evicted_ring_capacity": 4096, + "evicted_ring_bytes_max": "16 MiB", + "transfer_served_cache_bytes_max": "64 MiB", + "transfer_artifact_bytes_max": "64 MiB"}}"# + ) + } + + /// The floor is the type's, so a zero cannot be constructed to validate -- + /// it is refused at deserialization instead. + #[test] + fn rejects_zero_offset_reservation_lease_at_deserialization() { + let error = serde_json::from_str::(&partial_table(Some(0))) + .expect_err("a zero lease reserves nothing and must not deserialize"); + assert!( + error.to_string().contains("nonzero"), + "the refusal must name the constraint, got {error}" + ); + } + + /// The providers that do not merge the embedded defaults hand over a partial + /// table, which must still deserialize. + #[test] + fn given_a_table_without_the_lease_when_deserialized_should_fall_back_to_the_default() { + let config = serde_json::from_str::(&partial_table(None)) + .expect("a table omitting the lease must deserialize"); + assert_eq!( + config.offset_reservation_lease.get(), + DEFAULT_OFFSET_RESERVATION_LEASE + ); + assert!(config.validate().is_ok()); + } + + #[test] + fn rejects_offset_reservation_lease_above_ceiling() { + let config = PartitionConfig { + offset_reservation_lease: lease(MAX_OFFSET_RESERVATION_LEASE + 1), + ..PartitionConfig::default() + }; + assert!(config.validate().is_err()); + } + + #[test] + fn accepts_offset_reservation_lease_at_ceiling() { + let config = PartitionConfig { + offset_reservation_lease: lease(MAX_OFFSET_RESERVATION_LEASE), + ..PartitionConfig::default() + }; + assert!(config.validate().is_ok()); + } + #[test] fn rejects_zero_evicted_ring_capacity() { let config = PartitionConfig { diff --git a/core/consensus/src/impls.rs b/core/consensus/src/impls.rs index 5ad273c35f..159e0ca5e6 100644 --- a/core/consensus/src/impls.rs +++ b/core/consensus/src/impls.rs @@ -2139,8 +2139,10 @@ impl> VsrConsensus { checkpoint_op, checkpoint_checksum, // Consensus mints no message offsets: the PARTITION plane stamps - // this in before it writes (`IggyPartition::write_superblock`). + // both of these in before it writes + // (`IggyPartition::write_superblock`). offset_frontier: 0, + offset_reserved: 0, } } diff --git a/core/consensus/src/vsr_state.rs b/core/consensus/src/vsr_state.rs index 9040906adc..3023d3df13 100644 --- a/core/consensus/src/vsr_state.rs +++ b/core/consensus/src/vsr_state.rs @@ -31,20 +31,38 @@ use std::fmt; /// Number of bytes [`VsrState::to_bytes`] produces: `cluster`(16) + /// `replica_id`(1) + `replica_count`(1) + `view`(4) + `log_view`(4) + /// `commit_max`(8) + `checkpoint_op`(8) + `checkpoint_checksum`(16) + -/// `offset_frontier`(8). -pub const ENCODED_LEN: usize = 66; +/// `offset_frontier`(8) + `offset_reserved`(8). +/// +/// Growing this is one-way: a record of this length is [`VsrStateError::WrongLength`] +/// to every build that predates the field, so a ROLLBACK needs the data directory +/// wiped even though the upgrade does not. Stated for operators beside +/// `partition.offset_reservation_lease` in `config.toml`. +pub const ENCODED_LEN: usize = 74; -/// The layout before `offset_frontier` was appended. +/// The layout before `offset_reserved` was appended. +/// +/// [`VsrState::try_from`] accepts records of this length. Without it every +/// superblock already on disk decodes as [`VsrStateError::WrongLength`], which +/// the metadata plane treats as a durability violation and refuses the whole +/// node's boot on. +/// +/// "Already on disk" is not a clustering concern. `PingPongSuperblock::open` +/// runs unconditionally in metadata recovery, so EVERY server writes one of +/// these into `metadata/superblock.a`, clustered or not, on every view change +/// and checkpoint. Every release from `server-0.9.0-edge.2` (the first that +/// carries this module at all) through `edge.6` wrote exactly 66 bytes, +/// single-node deployments that never enabled clustering included. +/// +/// The 58-byte layout that preceded it is deliberately NOT accepted: it left +/// trunk before any release carried it -- `server-0.8.2-edge.1` has no +/// `vsr_state.rs`, and `edge.2` already wrote 66 -- so a record that short is +/// corruption, not history. /// -/// [`VsrState::try_from`] still accepts records of this length and zero-fills -/// the new field. Without it every superblock already on disk -- the metadata -/// plane writes one on every view change and checkpoint, single-node included -- -/// would decode as [`VsrStateError::WrongLength`] and refuse boot as a -/// durability violation. A version bump instead of this would not help on its -/// own: `classify` compares the version for exact equality, so a v2 build turns -/// every v1 record into `Unreadable`, which is the same refusal wearing a -/// different name. -pub const ENCODED_LEN_WITHOUT_FRONTIER: usize = 58; +/// A version bump instead of this would not help on its own: `classify` compares +/// the version for exact equality, so a v2 build turns every v1 record into +/// `Unreadable`, which is the same refusal wearing a different name -- and it +/// would make this tolerance unreachable, refusing even records that decode. +pub const ENCODED_LEN_WITHOUT_RESERVATION: usize = 66; /// The durable consensus state of one replica for one consensus group. /// @@ -97,6 +115,20 @@ pub struct VsrState { /// /// Always `0` on the metadata plane, which mints no message offsets. pub offset_frontier: u64, + /// PARTITION plane: a monotone CEILING on the offsets this replica may + /// already have minted, claimed ahead of the counter in blocks so an append + /// pays one superblock write per block instead of one per batch. + /// + /// Never folded into [`Self::offset_frontier`]. The frontier is a claim + /// about DATA, which state transfer's rewind guard refuses to destroy; a + /// reservation names no bytes, only "an offset up to here may have reached + /// a client". Comparing an offer against it refuses every legitimate offer + /// below the lease headroom, and the replica cycles transfer -> refusal -> + /// backoff forever. Boot seeds the mint counter from both; the rewind guard + /// reads the frontier alone. + /// + /// Always `0` on the metadata plane, which mints no message offsets. + pub offset_reserved: u64, } impl VsrState { @@ -113,6 +145,7 @@ impl VsrState { out[34..42].copy_from_slice(&self.checkpoint_op.to_le_bytes()); out[42..58].copy_from_slice(&self.checkpoint_checksum.to_le_bytes()); out[58..66].copy_from_slice(&self.offset_frontier.to_le_bytes()); + out[66..74].copy_from_slice(&self.offset_reserved.to_le_bytes()); out } } @@ -121,16 +154,27 @@ impl TryFrom<&[u8]> for VsrState { type Error = VsrStateError; fn try_from(bytes: &[u8]) -> Result { - // Length-tolerant: a pre-`offset_frontier` record is padded out and the - // new field reads as 0, which is exactly "no recorded frontier" (the - // read sites filter it). One length check up front then puts every - // field slice below in bounds by construction, so the `try_into`s - // cannot fail. + // Length-tolerant for ONE legacy layout, the pre-`offset_reserved` record + // every tagged release wrote. The one length check then puts every field + // slice below in bounds by construction, so the `try_into`s cannot fail. + // + // `offset_reserved` is filled from `offset_frontier`, not zeroed. The two + // agree on a record written before the reservation existed: the frontier + // is what that build's data proved, and the write side clamps the + // reservation up to it anyway, so this is the same value a first write + // under this build would record. A 0 would instead claim "nothing + // reserved" for offsets the frontier says exist. + // + // It cannot recover what the old build never wrote down -- offsets acked + // out of RAM above the frontier are gone with the process either way -- + // but refusing the record recovers nothing and costs the node its boot. let mut padded = [0u8; ENCODED_LEN]; match bytes.len() { ENCODED_LEN => padded.copy_from_slice(bytes), - ENCODED_LEN_WITHOUT_FRONTIER => { - padded[..ENCODED_LEN_WITHOUT_FRONTIER].copy_from_slice(bytes); + ENCODED_LEN_WITHOUT_RESERVATION => { + padded[..ENCODED_LEN_WITHOUT_RESERVATION].copy_from_slice(bytes); + padded[ENCODED_LEN_WITHOUT_RESERVATION..ENCODED_LEN] + .copy_from_slice(&bytes[58..66]); } actual => { return Err(VsrStateError::WrongLength { @@ -150,6 +194,7 @@ impl TryFrom<&[u8]> for VsrState { checkpoint_op: u64::from_le_bytes(field(bytes, 34)), checkpoint_checksum: u128::from_le_bytes(field(bytes, 42)), offset_frontier: u64::from_le_bytes(field(bytes, 58)), + offset_reserved: u64::from_le_bytes(field(bytes, 66)), }; // A record violating `log_view <= view` decodes into a replica that looks // healthy locally while `DoViewChangeHeader::validate` makes every peer drop @@ -190,8 +235,8 @@ impl fmt::Display for VsrStateError { Self::WrongLength { expected, actual } => { write!( f, - "VsrState needs {expected} bytes (or {ENCODED_LEN_WITHOUT_FRONTIER}, \ - the layout before the offset frontier), got {actual}" + "VsrState needs {expected} bytes (or {ENCODED_LEN_WITHOUT_RESERVATION}, \ + the layout before the offset reservation), got {actual}" ) } Self::LogViewAheadOfView { view, log_view } => write!( @@ -224,7 +269,8 @@ mod tests { commit_max: 6, checkpoint_op: 7, checkpoint_checksum: 8, - offset_frontier: 0, + offset_frontier: 9, + offset_reserved: 10, }; let bytes = state.to_bytes(); assert_eq!(bytes.len(), ENCODED_LEN); @@ -236,17 +282,20 @@ mod tests { assert_eq!(bytes[26], 6, "commit_max low byte"); assert_eq!(bytes[34], 7, "checkpoint_op low byte"); assert_eq!(bytes[42], 8, "checkpoint_checksum low byte"); + assert_eq!(bytes[58], 9, "offset_frontier low byte"); + assert_eq!(bytes[66], 10, "offset_reserved low byte"); assert_eq!(VsrState::try_from(&bytes[..]).unwrap(), state); assert!(VsrState::try_from(&bytes[..ENCODED_LEN - 1]).is_err()); } - /// A superblock written before `offset_frontier` existed must still decode: - /// the metadata plane writes one on every view change, so an exact-length - /// decode turns an in-place upgrade into a boot refusal on every deployment - /// that ever ran. + /// A superblock written before `offset_reserved` existed must still decode: + /// every release from `server-0.9.0-edge.2` to `edge.6` wrote that layout on + /// both planes -- the metadata one unconditionally, so single-node + /// deployments that never enabled clustering have one -- and the metadata + /// plane refuses the whole node's boot on a record it cannot decode. #[test] - fn given_pre_frontier_record_when_decoded_should_accept_and_zero_fill() { + fn given_pre_reservation_record_when_decoded_should_fill_from_the_frontier() { let full = VsrState { cluster: 3, replica_id: 1, @@ -257,23 +306,62 @@ mod tests { checkpoint_op: 7, checkpoint_checksum: 5, offset_frontier: 77, + offset_reserved: 88, } .to_bytes(); - let legacy = &full[..ENCODED_LEN_WITHOUT_FRONTIER]; - let decoded = VsrState::try_from(legacy).expect("a pre-frontier record must decode"); - assert_eq!(decoded.offset_frontier, 0, "the new field zero-fills"); + let legacy = &full[..ENCODED_LEN_WITHOUT_RESERVATION]; + let decoded = VsrState::try_from(legacy).expect("a pre-reservation record must decode"); + assert_eq!(decoded.offset_frontier, 77); + assert_eq!( + decoded.offset_reserved, 77, + "the reservation fills from the frontier, not from zero: a 0 would claim \ + nothing was reserved for offsets the frontier says exist" + ); assert_eq!(decoded.view, 9); assert_eq!(decoded.log_view, 8); assert_eq!(decoded.commit_max, 41); assert_eq!(decoded.checkpoint_op, 7); assert_eq!(decoded.checkpoint_checksum, 5); - // Anything that is neither layout is still refused. - assert!(matches!( - VsrState::try_from(&full[..40]), - Err(VsrStateError::WrongLength { .. }) - )); + // Anything that is neither layout is still refused, the 58-byte + // pre-frontier layout included: `server-0.8.2-edge.1` has no + // `vsr_state.rs` and `edge.2` already wrote 66, so no release ever put a + // 58-byte record on a disk and one that short is corruption, not + // history. + for short in [40, 58] { + assert!( + matches!( + VsrState::try_from(&full[..short]), + Err(VsrStateError::WrongLength { .. }) + ), + "a {short}-byte record must not decode" + ); + } + } + + /// A zero frontier is the shape a record written before either offset field + /// existed decodes into, and the fill must not turn that into a claim. + #[test] + fn given_pre_reservation_record_with_no_frontier_when_decoded_should_reserve_nothing() { + let full = VsrState { + cluster: 3, + replica_id: 1, + replica_count: 3, + view: 2, + log_view: 2, + commit_max: 0, + checkpoint_op: 0, + checkpoint_checksum: 0, + offset_frontier: 0, + offset_reserved: 0, + } + .to_bytes(); + + let decoded = VsrState::try_from(&full[..ENCODED_LEN_WITHOUT_RESERVATION]) + .expect("a pre-reservation record must decode"); + assert_eq!(decoded.offset_frontier, 0); + assert_eq!(decoded.offset_reserved, 0); } #[test] @@ -294,9 +382,11 @@ mod tests { // Distinct and nonzero: with 0 here a transposed write over the // trailing field would still satisfy every assertion below. offset_frontier: 9, + offset_reserved: 11, } .to_bytes(); assert_eq!(bytes[58], 9, "offset_frontier must occupy bytes 58..66"); + assert_eq!(bytes[66], 11, "offset_reserved must occupy bytes 66..74"); bytes[22] = 5; // log_view = 5, view stays 4 assert_eq!( diff --git a/core/integration/tests/cluster/crash_offset_reuse.rs b/core/integration/tests/cluster/crash_offset_reuse.rs index 447a6cc892..92e4ae89f9 100644 --- a/core/integration/tests/cluster/crash_offset_reuse.rs +++ b/core/integration/tests/cluster/crash_offset_reuse.rs @@ -15,17 +15,17 @@ // specific language governing permissions and limitations // under the License. -//! RED SPEC, expected to FAIL: offset identity across a crash. +//! Offset identity across a crash. //! //! `SendMessagesResponse::confirmations` hands clients concrete base offsets, -//! which makes offset reuse client-visible: a client that recorded offset N -//! for its message must never see the server confirm a DIFFERENT message at -//! N later. A solo node acks below the flush thresholds from RAM only and -//! persists no offset watermark, so after a SIGKILL it restarts the partition -//! log at the last flushed position and re-mints offsets it already -//! confirmed. Passes only once a durable watermark (or durable journal tail) -//! keeps post-restart offsets above everything ever acked. +//! which makes offset reuse client-visible: a client that recorded offset N for +//! its message must never see the server confirm a DIFFERENT message at N +//! later. A solo node acks below the flush thresholds from RAM only, so nothing +//! in the segments says those offsets were ever handed out. What keeps them +//! from being re-minted is the offset RESERVATION in the partition superblock, +//! claimed by the append fence before any of them exist and read back by boot. +use std::path::Path; use std::time::Duration; use iggy::prelude::*; @@ -97,13 +97,8 @@ async fn wait_until_serving(harness: &TestHarness, budget: Duration) -> IggyClie } } -// TODO(hubcio): fix this test -#[ignore = "confirmed offsets re-minted after a crash; no durable offset watermark"] -#[iggy_harness(cluster_nodes = 1)] -async fn given_confirmed_sends_below_flush_threshold_when_a_solo_node_is_killed_should_not_remint_offsets( - harness: &mut TestHarness, -) { - let client = harness.tcp_root_client().await.unwrap(); +/// Create the stream and its single-partition topic. +async fn create_topic(client: &IggyClient, messages_required_to_save: Option) { client .create_stream(STREAM_NAME) .await @@ -115,20 +110,83 @@ async fn given_confirmed_sends_below_flush_threshold_when_a_solo_node_is_killed_ &TopicCreateOptions { partitions_count: Some(1), message_expiry: Some(IggyExpiry::NeverExpire), + messages_required_to_save, ..TopicCreateOptions::default() }, ) .await .expect("create topic"); +} - let acked = produce_acked(&client, "pre-crash", PRE_CRASH_SENDS).await; - let highest_confirmed = *acked.last().expect("confirmed sends"); - drop(client); +/// Base offsets of every segment file under `root`, from the file names, which +/// are the on-disk claim about where each range begins. +/// +/// Reading them is the only way to assert the shape the re-anchor produces. A +/// black-box offset assertion passes either way on the boot that WRITES the +/// wrong shape; the cost only lands on the boot that reads it back, where the +/// walk refuses and the solo arm tombstones the partition. +fn segment_base_offsets(root: &Path) -> Vec { + let mut offsets = Vec::new(); + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if path.extension().is_some_and(|extension| extension == "log") + && let Some(stem) = path.file_stem().and_then(|stem| stem.to_str()) + && let Ok(offset) = stem.parse::() + { + offsets.push(offset); + } + } + } + offsets.sort_unstable(); + offsets +} + +/// Poll until the only node has the replicated consumer offset on disk. Panics +/// at the deadline. +async fn wait_for_stored_offset_on_disk(harness: &TestHarness, expected: u64, budget: Duration) { + let data_path = harness.node(0).data_path(); + let deadline = tokio::time::Instant::now() + budget; + loop { + if integration::harness::disk::read_replicated_consumer_offset(&data_path) == Some(expected) + { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "the node did not persist consumer offset {expected} within {budget:?} \ + (found {:?})", + integration::harness::disk::read_replicated_consumer_offset(&data_path), + ); + sleep(POLL_INTERVAL).await; + } +} +/// Kill the node, bring it back, and return a client onto the restarted one. +async fn crash_and_recover(harness: &mut TestHarness) -> IggyClient { harness.kill_node(0).expect("SIGKILL the only node"); harness.restart_node(0).expect("restart it"); + wait_until_serving(harness, SERVE_TIMEOUT).await +} - let client = wait_until_serving(harness, SERVE_TIMEOUT).await; +#[iggy_harness(cluster_nodes = 1)] +async fn given_confirmed_sends_below_flush_threshold_when_a_solo_node_is_killed_should_not_remint_offsets( + harness: &mut TestHarness, +) { + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, None).await; + + let acked = produce_acked(&client, "pre-crash", PRE_CRASH_SENDS).await; + let highest_confirmed = *acked.last().expect("confirmed sends"); + drop(client); + + let client = crash_and_recover(harness).await; let post_crash_offset = produce_acked(&client, "post-crash", 1).await[0]; assert!( @@ -140,4 +198,354 @@ async fn given_confirmed_sends_below_flush_threshold_when_a_solo_node_is_killed_ so two different messages now share an offset and consumers reading by offset get \ silently different data" ); + + // The shape this boot WROTE is only paid for by the boot that reads it back: + // nothing reached a segment before the crash, so the re-anchor emptied the + // chain and had to plant at the append point rather than leave the segment + // named 0 with the first mint a lease block inside it. + let bases = segment_base_offsets(harness.test_dir()); + assert!( + bases.iter().any(|&base| base > highest_confirmed), + "the re-anchor left no segment above the pre-crash offsets, so the post-crash \ + mint landed inside a segment named below it: segment bases {bases:?}, last \ + offset confirmed before the crash {highest_confirmed}" + ); + drop(client); + + // And the boot that reads it: a hole inside a segment refuses the walk and + // tombstones the partition, which shows up here as a node that never serves + // the stream again. + let client = crash_and_recover(harness).await; + let third_life = produce_acked(&client, "third-life", 1).await[0]; + assert!( + third_life > post_crash_offset, + "the second restart re-minted: {post_crash_offset} was confirmed after the \ + first crash, yet the node came back and handed out {third_life}" + ); +} + +/// The graceful stop is the runbook answer to an incident, so it must not be the +/// action that undoes the fix. Between the two boots here the node takes no +/// traffic at all: nothing reaches a segment, the committed frontier stays at +/// what the crash left, and the reservation is the only record that offsets were +/// handed out. +/// +/// End to end, not a probe of one mechanism: three separate things carry the +/// append point across this sequence (the boot seed, the segment the re-anchor +/// plants, and the collapse writing the append point rather than the committed +/// frontier), so any one of them alone keeps this green. The collapse is pinned +/// on its own by `given_an_unspent_reservation_when_collapsing_should_leave_it_standing` +/// in `core/partitions`. +#[iggy_harness(cluster_nodes = 1)] +async fn given_a_crash_restarted_node_when_stopped_cleanly_should_still_not_remint_offsets( + harness: &mut TestHarness, +) { + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, None).await; + + let acked = produce_acked(&client, "pre-crash", PRE_CRASH_SENDS).await; + let highest_confirmed = *acked.last().expect("confirmed sends"); + drop(client); + + // Boot one: reads the reservation back and never spends it. + let client = crash_and_recover(harness).await; + drop(client); + + // The clean stop, then boot two. `restart_node` stops the running node with + // SIGTERM and waits for it, so the shutdown flush and its collapse both run. + harness + .restart_node(0) + .expect("cleanly restart the only node"); + let client = wait_until_serving(harness, SERVE_TIMEOUT).await; + let post_stop_offset = produce_acked(&client, "post-clean-stop", 1).await[0]; + + assert!( + post_stop_offset > highest_confirmed, + "a clean stop between the crash and the next send re-minted: offset \ + {highest_confirmed} was handed to a client before the SIGKILL, and after a \ + graceful restart the node confirmed {post_stop_offset}. The shutdown collapse \ + dropped a reservation the boot had not spent yet" + ); +} + +/// The combination neither clean-stop nor flushed case covers on its own: +/// `given_a_crash_restarted_node_when_stopped_cleanly_should_still_not_remint_offsets` +/// stops cleanly but takes no traffic between boots, so its collapse only ever +/// sees an UNSPENT reservation, while every flushed case re-enters through +/// SIGKILL and never runs the collapse at all. +/// +/// Here the second life spends the reservation, appends past the flush threshold, +/// and then stops gracefully, so the collapse writes an append point over a chain +/// the re-anchor already planted a gap into. The boot that follows has to accept +/// that chain and resume above it; a refusal tombstones the partition and shows +/// up as a node that never serves the stream again. +#[iggy_harness(cluster_nodes = 1)] +async fn given_a_flushed_crash_restarted_node_when_stopped_cleanly_should_still_not_remint_offsets( + harness: &mut TestHarness, +) { + const FLUSH_THRESHOLD: u32 = 4; + /// Past the threshold, so the life leaves a chain and a flushed tail. + const SENDS_PER_LIFE: u32 = 6; + + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, Some(FLUSH_THRESHOLD)).await; + + let acked = produce_acked(&client, "first-life", SENDS_PER_LIFE).await; + let first_life_max = *acked.last().expect("confirmed sends"); + drop(client); + + let client = crash_and_recover(harness).await; + let second_life = produce_acked(&client, "second-life", SENDS_PER_LIFE).await; + let second_life_max = *second_life.last().expect("confirmed sends"); + assert!( + second_life[0] > first_life_max, + "the crash restart re-minted: confirmed {first_life_max} before the SIGKILL, \ + then {} after it", + second_life[0] + ); + drop(client); + + // SIGTERM and wait, so the shutdown flush and its collapse both run over the + // re-anchored chain. + harness + .restart_node(0) + .expect("cleanly restart the only node"); + let client = wait_until_serving(harness, SERVE_TIMEOUT).await; + let post_stop = produce_acked(&client, "post-clean-stop", 1).await[0]; + + assert!( + post_stop > second_life_max, + "a clean stop after a spent reservation re-minted: {second_life_max} was \ + confirmed to a client, and the graceful restart handed out {post_stop}. The \ + collapse recorded the committed frontier rather than the append point, so the \ + next boot resumed inside offsets the previous life had already confirmed" + ); + + let bases = segment_base_offsets(harness.test_dir()); + assert!( + bases.iter().any(|&base| base > first_life_max), + "the chain lost the re-anchor's plant across the clean stop: bases {bases:?}, \ + last offset confirmed before the crash {first_life_max}" + ); +} + +/// The behaviour the reservation is SOLD on, which every other case here leaves +/// implicit: a consumer positioned inside the pre-crash range must keep reading +/// forward across the hole the reservation creates, and must land on the real +/// post-crash message rather than on a phantom offset inside the gap. +/// +/// The other cases assert only that newly confirmed offsets rise. That is the +/// producer's half. A consumer that stored a position, survived the crash and +/// then polled `Next` is what actually walks the boundary the re-anchor planted: +/// `disk_poll_start` has to carry the walk on into the segment above the gap, +/// and the stored offset has to still mean the same place. +#[iggy_harness(cluster_nodes = 1)] +async fn given_a_stored_consumer_position_when_polling_across_a_reservation_hole_should_not_miss( + harness: &mut TestHarness, +) { + const CONSUMER_ID: u32 = 7; + const POST_CRASH_PAYLOAD: &str = "across-the-hole-000"; + + let stream = Identifier::named(STREAM_NAME).unwrap(); + let topic = Identifier::named(TOPIC_NAME).unwrap(); + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, None).await; + + let acked = produce_acked(&client, "pre-crash", PRE_CRASH_SENDS).await; + let highest_confirmed = *acked.last().expect("confirmed sends"); + // Positioned one BELOW the last confirmed offset, so the pre-crash tail is + // still ahead of the consumer when the node dies. A position at the tail + // would make the first post-crash poll indistinguishable from a fresh read. + let stored = highest_confirmed - 1; + let consumer = Consumer::new(Identifier::numeric(CONSUMER_ID).unwrap()); + client + .store_consumer_offset(&consumer, &stream, &topic, Some(PARTITION_ID), stored) + .await + .expect("store the pre-crash consumer position"); + // Gated on the position reaching DISK before the SIGKILL. The ack is granted + // at commit and the consumer-offset write is threshold-gated like any other, + // so without this the crash can legitimately take the position with it and + // the assertion below races. + wait_for_stored_offset_on_disk(harness, stored, SERVE_TIMEOUT).await; + drop(client); + + let client = crash_and_recover(harness).await; + assert_eq!( + client + .get_consumer_offset(&consumer, &stream, &topic, Some(PARTITION_ID)) + .await + .expect("read the consumer offset back") + .expect("the stored position survived the crash") + .stored_offset, + stored, + "the position a consumer committed before the crash must mean the same \ + offset after it, or every consumer silently re-reads or skips" + ); + + // One message above the hole, with a payload no earlier send used, so the + // poll cannot pass by matching something the pre-crash range already held. + let post_crash = produce_acked(&client, "across-the-hole", 1).await[0]; + assert!( + post_crash > highest_confirmed, + "the premise: the restart must mint above the confirmed range" + ); + + let polled = client + .poll_messages( + &stream, + &topic, + Some(PARTITION_ID), + &consumer, + &PollingStrategy::next(), + 1, + false, + ) + .await + .expect("poll forward from the stored position"); + + let message = polled + .messages + .first() + .unwrap_or_else(|| panic!("`Next` from offset {stored} served nothing at all")); + assert_eq!( + String::from_utf8_lossy(&message.payload), + POST_CRASH_PAYLOAD, + "the consumer must land on the post-crash message, not on stale bytes or a \ + phantom offset inside the reservation's hole" + ); + assert_eq!( + message.header.offset, post_crash, + "and it must be served at the offset the producer was confirmed at: the hole \ + between {highest_confirmed} and {post_crash} is unwritten offset space, not \ + messages a consumer may be handed" + ); +} + +/// The replicated fence path, which no other case here reaches. A three-node +/// group acks a send once a quorum has journaled it, so it claims no reservation +/// and re-anchors nothing -- and one node crashing must still not disturb the +/// offsets the group hands out. +#[iggy_harness(cluster_nodes = 3)] +async fn given_a_replicated_group_when_a_node_is_killed_should_not_remint_offsets( + harness: &mut TestHarness, +) { + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, None).await; + + let acked = produce_acked(&client, "pre-crash", PRE_CRASH_SENDS).await; + let highest_confirmed = *acked.last().expect("confirmed sends"); + assert_eq!( + acked, + (0..u64::from(PRE_CRASH_SENDS)).collect::>(), + "the pre-crash run mints a contiguous range from zero" + ); + drop(client); + + let client = crash_and_recover(harness).await; + let post_crash_offset = produce_acked(&client, "post-crash", 1).await[0]; + + assert!( + post_crash_offset > highest_confirmed, + "a replicated group re-minted after one node restarted: {highest_confirmed} was \ + confirmed before the SIGKILL, then {post_crash_offset} after it" + ); + + // No reservation was claimed, so no gap was planted: a replicated group's + // segment boundaries have to stay a function of its batches alone, or the + // reconciler's offset-keyed segment GC never converges. + let bases = segment_base_offsets(harness.test_dir()); + assert!( + bases.iter().all(|&base| base <= post_crash_offset), + "a replicated group planted a segment above every offset it minted, so it \ + re-anchored around a reservation it should never have claimed: bases {bases:?}" + ); +} + +/// The fix has to survive its own side effect: the hole the reservation leaves +/// between the recovered segments and the new append point makes the next boot +/// REFUSE the chain if it lands INSIDE a segment, which tombstones the partition +/// on a solo node. +/// +/// So the SECOND crash is the one that matters, and only if the run between the +/// two reaches disk, which is what the flush threshold is for. +#[iggy_harness(cluster_nodes = 1)] +async fn given_a_crash_restarted_node_when_it_flushes_and_crashes_again_should_still_not_remint_offsets( + harness: &mut TestHarness, +) { + const FLUSH_THRESHOLD: u32 = 4; + /// Past the threshold, so every life leaves a chain for the next boot. + const SENDS_PER_LIFE: u32 = 6; + + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, Some(FLUSH_THRESHOLD)).await; + + let acked = produce_acked(&client, "first-life", SENDS_PER_LIFE).await; + let first_life_max = *acked.last().expect("confirmed sends"); + drop(client); + + // The boot that consumes a reservation and re-anchors, then flushes the + // hole's far side to disk. + let client = crash_and_recover(harness).await; + let second_life = produce_acked(&client, "second-life", SENDS_PER_LIFE).await; + let second_life_min = second_life[0]; + let second_life_max = *second_life.last().expect("confirmed sends"); + assert!( + second_life_min > first_life_max, + "the first restart re-minted: confirmed {first_life_max} before the crash, \ + then {second_life_min} after it" + ); + drop(client); + + // ON a segment boundary, not inside one: a segment still named 0 while + // holding the second life's offsets claims a range it does not have. + let bases = segment_base_offsets(harness.test_dir()); + assert!( + bases.iter().any(|&base| base > first_life_max), + "no segment is anchored above the pre-crash offsets, so the second life \ + appended into a segment named for the first: segment bases {bases:?}, last \ + offset confirmed before the crash {first_life_max}" + ); + + // The first boot that has to read a chain the re-anchor wrote. + let client = crash_and_recover(harness).await; + let third_life = produce_acked(&client, "third-life", 1).await[0]; + assert!( + third_life > second_life_max, + "the SECOND restart re-minted: {second_life_max} was confirmed between the \ + two crashes, yet the node came back and handed out {third_life}. A hole \ + left INSIDE a segment costs the tail that proved the frontier" + ); +} + +/// The partially-flushed shape a real workload crashes in: the run below the +/// threshold is confirmed out of the journal while everything before it is on +/// disk. The recovered chain then ends BELOW the reservation with bytes in it, +/// so the re-anchor has to seal it rather than append into the gap. +#[iggy_harness(cluster_nodes = 1)] +async fn given_sends_straddling_the_flush_threshold_when_the_node_is_killed_should_not_remint_offsets( + harness: &mut TestHarness, +) { + const FLUSH_THRESHOLD: u32 = 4; + const STRADDLING_SENDS: u32 = 6; + + let client = harness.tcp_root_client().await.unwrap(); + create_topic(&client, Some(FLUSH_THRESHOLD)).await; + + let acked = produce_acked(&client, "straddle", STRADDLING_SENDS).await; + let highest_confirmed = *acked.last().expect("confirmed sends"); + assert_eq!( + acked, + (0..u64::from(STRADDLING_SENDS)).collect::>(), + "the pre-crash run mints a contiguous range from zero" + ); + drop(client); + + let client = crash_and_recover(harness).await; + let post_crash = produce_acked(&client, "post-straddle", 1).await[0]; + assert!( + post_crash > highest_confirmed, + "offsets confirmed out of the journal above the last flushed one were \ + re-minted: {highest_confirmed} went to a client before the SIGKILL, and the \ + first send after it was confirmed at {post_crash}" + ); } diff --git a/core/journal/src/superblock.rs b/core/journal/src/superblock.rs index fd8b761244..77f49ffe09 100644 --- a/core/journal/src/superblock.rs +++ b/core/journal/src/superblock.rs @@ -70,8 +70,8 @@ const MIN_RECORD_LEN: usize = HEADER_LEN + CHECKSUM_LEN; /// Ceiling on a record's payload, bounding every allocation this module makes from /// a length it read off disk (`PrepareJournal::MAX_ENTRY_SIZE` bounds the WAL for the -/// same reason). The only payload today is a [`consensus::VsrState`], 66 bytes now -/// that it carries the offset frontier (58 before it, a length its decode still +/// same reason). The only payload today is a [`consensus::VsrState`], 74 bytes now +/// that it carries the offset reservation (66 before it, a length its decode still /// accepts); the headroom is for a payload that grows fields, not for bulk data. /// `read_slot` treats a longer file as corrupt WITHOUT reading it, and /// `build_record` refuses to write one, so a length this store could have diff --git a/core/metadata/src/impls/recovery.rs b/core/metadata/src/impls/recovery.rs index 39d5b9847d..60ffcf58f1 100644 --- a/core/metadata/src/impls/recovery.rs +++ b/core/metadata/src/impls/recovery.rs @@ -843,6 +843,7 @@ mod tests { checkpoint_op, checkpoint_checksum, offset_frontier: 0, + offset_reserved: 0, } } diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index 3cbdf3e177..ab16b256fa 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -79,12 +79,26 @@ use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; use std::fmt; use std::hash::Hash; +use std::num::NonZeroU32; use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use tokio::sync::Mutex as TokioMutex; use tracing::{debug, error, warn}; +/// Which of a partition's offset counters are live. +/// +/// Two bits, not one: a reservation-seeded boot makes the append counter live +/// with nothing committed behind it, and folding them together reports +/// `offset_frontier() == 1` for a partition holding nothing. +#[derive(Debug, Default, Clone, Copy)] +pub struct OffsetSpace { + /// The APPEND counter is live, so the next mint continues it. + pub append_live: bool, + /// The COMMITTED counter names data. + pub committed_seeded: bool, +} + // This struct aliases in terms of the code contained the `LocalPartition from `core/server/src/streaming/partitions/local_partition.rs`. pub struct IggyPartition where @@ -120,7 +134,7 @@ where pub stats: Arc, pub created_at: IggyTimestamp, pub revision_id: u64, - pub should_increment_offset: bool, + pub(crate) offset_space: OffsetSpace, pub write_lock: Arc>, pub(crate) consumer_offsets_path: Option, pub(crate) consumer_group_offsets_path: Option, @@ -239,6 +253,27 @@ where /// segments that were the only other witness, after which the rebuild /// re-mints offsets the group already handed out. durable_offset_frontier: Cell, + /// The `offset_reserved` ceiling the last successful superblock write + /// recorded, seeded at boot from the record that write left behind. + /// + /// A `Cell` rather than a re-read of the record because every append reads + /// it, and the steady state ("the block still covers this batch") has to + /// cost nothing. Kept apart from [`Self::durable_offset_frontier`]: see + /// `consensus::VsrState::offset_reserved`. + durable_offset_reserved: Cell, + /// A send arrived for a partition with no reservation on disk yet, and was + /// bounced so the shard tick could claim the first block off the request + /// path. Cleared by the write it asks for. + /// + /// The trigger the tick consults is otherwise gated on a partition that has + /// already minted, deliberately: arming every idle partition at boot would + /// write a superblock per partition for nothing. This bit is what separates + /// "idle" from "wanted", so the cost falls only on partitions someone + /// actually produced to. + offset_reservation_wanted: Cell, + /// Offsets the append fence claims per superblock write; installed by boot + /// from `PartitionsConfig`. + offset_reservation_lease: u64, /// In-flight state transfer for this group (rejoin whose repair floor was /// refused); tail repair takes over at install. See /// [`PartitionTransferSession`]. @@ -300,7 +335,7 @@ where .field("namespace", &self.consensus.group()) .field("offset", &self.offset) .field("dirty_offset", &self.dirty_offset) - .field("should_increment_offset", &self.should_increment_offset) + .field("offset_space", &self.offset_space) .field("partition_dir", &self.partition_dir) .field("repair", &self.repair) .field("recovered_durable_offset", &self.recovered_durable_offset) @@ -481,7 +516,7 @@ where stats, created_at: IggyTimestamp::now(), revision_id: 0, - should_increment_offset: false, + offset_space: OffsetSpace::default(), write_lock: Arc::new(TokioMutex::new(())), consumer_offsets_path: None, consumer_group_offsets_path: None, @@ -504,6 +539,9 @@ where superblock_retry_after_micros: Cell::new(0), purge_deferred: false, durable_offset_frontier: Cell::new(0), + durable_offset_reserved: Cell::new(0), + offset_reservation_wanted: Cell::new(false), + offset_reservation_lease: u64::from(crate::DEFAULT_OFFSET_RESERVATION_LEASE), transfer: None, transfer_attempts: 0, transfer_failures: 0, @@ -606,7 +644,7 @@ where partition .dirty_offset .store(start_offset, Ordering::Relaxed); - partition.should_increment_offset = false; + partition.set_offset_space_used(false); partition.stats.increment_segments_count(1); partition } @@ -631,6 +669,8 @@ where self.superblock = Some(superblock); self.durable_offset_frontier .set(recovered.map_or(0, |state| state.offset_frontier)); + self.durable_offset_reserved + .set(recovered.map_or(0, |state| state.offset_reserved)); } /// Persist this group's VSR state to its superblock when the view changed @@ -707,24 +747,87 @@ where /// view it is in must not act in it, so it withholds every view-scoped /// send for this group, goes quiet, and its peers elect around it. Only /// THIS partition's group is fenced; the rest of the node keeps serving. + /// A RUN of failures is terminal for the process, not the group: the shard + /// tick fail-stops on `superblock_wedged`. #[allow(clippy::future_not_send)] async fn write_superblock(&self, superblock: &SB, offset_frontier: u64) -> bool { - // ADVANCE direction: never below what this replica has already minted, - // and never below what the record ALREADY holds. Both bounds are - // needed and neither implies the other -- a failed install leaves the - // counter behind the record it wrote before the swap, so maxing against - // the counter alone lets the fence that follows lower the durable - // frontier. The reset direction goes through `write_superblock_inner`. - let advanced = offset_frontier - .max(self.offset_frontier()) - .max(self.durable_offset_frontier.get()); - self.write_superblock_inner(superblock, advanced).await + self.write_superblock_advancing(superblock, offset_frontier, 0) + .await + } + + /// [`Self::write_superblock`] for a caller that also has a reservation to + /// claim. Both fields advance, neither can regress. + #[allow(clippy::future_not_send)] + async fn write_superblock_advancing( + &self, + superblock: &SB, + offset_frontier: u64, + offset_reserved: u64, + ) -> bool { + // ADVANCE direction; the reset direction goes through + // `write_superblock_inner`. Both bounds inside `advanced_frontier` are + // needed: a failed install leaves the chain behind the record it wrote + // before the swap, so maxing against the data alone would let the fence + // that follows lower the durable frontier. + let advanced = self.advanced_frontier(offset_frontier); + // Nothing but the record witnesses a reservation, so a caller with no + // claim of its own (every view-change write) passes 0 and carries the + // recorded one forward; dropping it would let the next boot seed the + // counter below what an earlier append already fenced. + let reserved = offset_reserved.max(self.durable_offset_reserved.get()); + self.write_superblock_inner(superblock, advanced, reserved) + .await + } + + /// The advance rule for the frontier, shared by every writer that claims + /// one: never below what this replica holds, never below what the record + /// already says. Held messages, NOT [`Self::mint_frontier`], which stands a + /// lease block above them after a reservation-seeded boot and names none of + /// them. + fn advanced_frontier(&self, claim: u64) -> u64 { + claim + .max(self.held_offset_frontier()) + .max(self.durable_offset_frontier.get()) + } + + /// Record an incoming state-transfer frontier, advancing the frontier and + /// SETTING the reservation to the frontier this write records. + /// + /// Not to the offer: `write_superblock_inner` clamps the reservation up to + /// the frontier it writes, which is `advanced_frontier(frontier)` and sits + /// above the offer whenever an earlier over-claiming write left the durable + /// frontier higher. + /// + /// The one place the otherwise-monotone reservation may come down, and the + /// one place it must: the offer describes the group's committed log, so a + /// local reservation above it covers offsets this replica never confirmed. + /// Carried forward, it re-seeds the counter a lease block above the group + /// after the next restart, where every replicated prepare fails the + /// `base_offset == dirty_offset + 1` check. + #[allow(clippy::future_not_send)] + #[must_use = "the bool is the durability verdict; dropping it silently ignores a failed write"] + pub async fn install_offset_frontier_at(&self, frontier: u64) -> bool { + let Some(superblock) = self.superblock.as_ref().map(Rc::clone) else { + return true; + }; + if self.superblock_write_is_backed_off() { + return false; + } + let _superblock_guard = self.superblock_lock.acquire().await; + let advanced = self.advanced_frontier(frontier); + self.write_superblock_inner(superblock.as_ref(), advanced, frontier) + .await } /// The write itself; the advance and reset directions differ only in the - /// frontier they hand in. + /// values they hand in. #[allow(clippy::future_not_send)] - async fn write_superblock_inner(&self, superblock: &SB, offset_frontier: u64) -> bool { + async fn write_superblock_inner( + &self, + superblock: &SB, + offset_frontier: u64, + offset_reserved: u64, + ) -> bool { // The pairing fields stay `(0, 0)` and `commit_max` is a dead write // on this plane: nothing reads either back (`restore_partition_view` // restores view/log_view only), because recovery re-derives the @@ -737,18 +840,36 @@ where // write stamps the current counter, so whichever write lands last (a // view change, or the explicit persist an install issues) leaves a // lower bound boot can re-seed from. + // Sampled BEFORE the write: the writers that bypass the gate (the append + // fence, the quarantine record, the shutdown collapse) attempt inside an + // open window, and counting those makes the wedge threshold a function of + // producer retry rate rather than of elapsed time. + let inside_backoff_window = self.superblock_write_is_backed_off(); let mut state = self.consensus.vsr_state(0, 0); state.offset_frontier = offset_frontier; + // A frontier of N says offsets below N exist, so a reservation under it + // is not a reservation. Clamped here rather than per caller so the + // reset direction gets it too, where a stale higher reservation would + // seed the next boot into the offset space the reset just erased. + state.offset_reserved = offset_reserved.max(offset_frontier); match superblock.write(&state.to_bytes()).await { Ok(()) => { self.consensus .mark_superblock_durable(state.view, state.log_view); self.durable_offset_frontier.set(state.offset_frontier); + self.durable_offset_reserved.set(state.offset_reserved); self.superblock_write_failures.set(0); self.superblock_retry_after_micros.set(0); true } Err(error) => { + // One failure per backoff step. `superblock_wedged` reads the + // count as elapsed time (the window is capped at 1 s, the + // default threshold is 2 m), which a per-attempt count would + // turn into seconds under a handful of retrying producers. + if inside_backoff_window { + return false; + } let failures = self.superblock_write_failures.get() + 1; self.superblock_write_failures.set(failures); let backoff = SUPERBLOCK_RETRY_BACKOFF_BASE_MICROS @@ -784,37 +905,131 @@ where /// proved. /// /// The record is a lower bound, never a completeness claim: it exists - /// because three paths leave a replica whose counter would otherwise - /// restart at 0 while the group is at N (a transfer install of an all-GC'd - /// origin, a crash inside the install's swap window, and the - /// fence-and-rebuild path, which needs no crash at all). Restarting the + /// because four paths leave a replica whose counter would otherwise + /// restart below where the group already is (a transfer install of an + /// all-GC'd origin, a crash inside the install's swap window, the + /// fence-and-rebuild path, which needs no crash at all, and a crash while + /// acked messages were still resident in the journal). Restarting the /// counter is not a lag -- replicas re-stamp `base_offset` from it and /// recompute `batch_checksum` over the result, so the next replicated /// prepare would persist different bytes here than on every peer, silently. /// + /// The two counters are seeded separately, because the record carries two + /// bounds. `offset_frontier` is what messages reached, so it seeds the + /// COMMITTED counter; `offset_reserved` is what may have been handed to a + /// client, so it seeds the APPEND counter and nothing else. Folding the + /// reservation into the committed one would publish a `current_offset` over + /// a lease-block hole, and `store_consumer_offset` would then admit offsets + /// inside it. + /// + /// The reservation is SOLO ONLY. A backup mints nothing: it re-stamps what + /// the primary sends and rejects anything that does not continue its own + /// counter, so an append point a lease block above its group would have every + /// peer refuse the batch. A replicated group is also less exposed, since an + /// ack there means a quorum journaled the batch and the hole needs a + /// FULL-cluster crash. + /// /// Lives HERE rather than in the server crate so the boot paths and the /// simulator share one implementation. A copy in the harness was a copy of /// the max rule that had lost the max, in the one place built to catch /// violations of it. pub fn restore_offset_frontier(&mut self, recovered: Option<&consensus::VsrState>) { - let Some(frontier) = recovered - .map(|state| state.offset_frontier) - .filter(|&f| f > 0) - else { + let Some(state) = recovered else { return; }; - let recovered_end = frontier - 1; - if self.should_increment_offset && self.offset.load(Ordering::Acquire) >= recovered_end { + let frontier = state.offset_frontier; + let reserved = if self.consensus.replica_count() == 1 { + state.offset_reserved + } else { + 0 + }; + // NOT `frontier > 0`: the shape a crash before the first flush leaves is + // a zero frontier and a nonzero reservation, because the append fence + // runs before the journal append, so the first record a partition ever + // writes names no data at all. + let append_point = frontier.max(reserved); + if append_point == 0 { + return; + } + let seeded = self.offset_space.append_live; + // Each counter takes its own max, since the record's two bounds move + // independently: a graceful stop collapses the reservation onto the + // append point while the frontier stays where the data ended. + let committed_restored = frontier.checked_sub(1).is_some_and(|committed_end| { + let raise = !seeded || self.offset.load(Ordering::Acquire) < committed_end; + if raise { + self.offset.store(committed_end, Ordering::Release); + } + raise + }); + let append_end = append_point - 1; + let append_restored = !seeded || self.dirty_offset.load(Ordering::Relaxed) < append_end; + if append_restored { + self.dirty_offset.store(append_end, Ordering::Relaxed); + } + if !committed_restored && !append_restored { return; } tracing::info!( namespace_raw = self.consensus().group(), offset_frontier = frontier, - "restored partition offset frontier from its superblock" + offset_reserved = reserved, + append_point, + "restored partition offset counters from its superblock" ); - self.offset.store(recovered_end, Ordering::Release); - self.dirty_offset.store(recovered_end, Ordering::Relaxed); - self.should_increment_offset = true; + self.offset_space.append_live = true; + // Only the frontier names data. A record carrying a reservation alone is + // the pre-first-flush shape, where the committed counter still seeds + // nothing. + self.offset_space.committed_seeded |= frontier > 0; + } + + /// Path of the anchor whose lifecycle is this segment's. + /// + /// Anchors are unlinked with the segment they sit beside. Left behind, a + /// purge resets the offset space to 0 and the stale record still `covers` + /// the bounds a later plant reuses. + pub(crate) fn anchor_cleanup_path(&self, start_offset: u64) -> Option { + self.partition_dir + .as_deref() + .map(|dir| crate::segment_anchor::anchor_path(dir, start_offset)) + } + + /// Seed or clear BOTH offset-space bits. + /// + /// For the callers that genuinely move both: a fresh or purged partition + /// (neither counter names anything) and a boot off segments (both do, + /// because a segment on disk holds committed messages only). Everything on + /// the live path moves ONE bit -- see [`Self::note_append_live`] and + /// [`Self::note_committed_seeded`]. + pub const fn set_offset_space_used(&mut self, used: bool) { + self.offset_space = OffsetSpace { + append_live: used, + committed_seeded: used, + }; + } + + /// The append counter is live: an offset has been journaled, so the next + /// mint continues from it. + /// + /// APPEND only. A journaled offset is not a committed one: `build_poll_plan` + /// gates on `OffsetSpace::committed_seeded`, and seeding that here would + /// serve a resident offset 0 to a consumer before the first commit and let + /// the frontier persist name data no quorum agreed on. A view change may + /// still truncate this offset away. + pub const fn note_append_live(&mut self) { + self.offset_space.append_live = true; + } + + /// The committed counter names data: an offset has passed commit, so it is + /// pollable and the frontier may record it. + /// + /// Implies the append counter is live too -- nothing commits that was not + /// journaled first -- but the reverse does not hold, which is the whole + /// reason the two bits are separate. + pub const fn note_committed_seeded(&mut self) { + self.offset_space.append_live = true; + self.offset_space.committed_seeded = true; } /// Whether this partition ever stamped an offset, i.e. whether its offset @@ -823,7 +1038,7 @@ where /// that never took a write: both report `(0, 0)`. #[cfg(any(test, feature = "simulator"))] pub const fn offset_space_used(&self) -> bool { - self.should_increment_offset + self.offset_space.append_live } /// Adopt a log carried over from a previous incarnation of this partition, @@ -854,7 +1069,7 @@ where // prepare mint from a base no peer agrees on. // // Keyed on the RETIRED incarnation's flag, never on `(0, 0)` or on this - // partition's own `should_increment_offset`. One message at offset 0 reports + // partition's own `append_live`. One message at offset 0 reports // the same two zeroes as an empty partition, and this instance is freshly // built so its own flag is always false. The arithmetic test would therefore // adopt the log, skip the counters, and let the next write stamp @@ -869,13 +1084,74 @@ where .max(self.dirty_offset.load(Ordering::Relaxed)); self.offset.store(durable, Ordering::Release); self.dirty_offset.store(dirty, Ordering::Relaxed); - self.should_increment_offset = true; + self.set_offset_space_used(true); // Everything carried over is already persisted as far as this replica is // concerned, so the flush and commit paths must not re-persist or re-count // it, the same contract boot gives a partition recovered from segments. self.recovered_durable_offset = Some(durable); } + /// The in-memory half of [`Self::reanchor_to_offset_frontier`], for a + /// simulator partition rebuilt over a restored offset counter. + /// + /// The production re-anchor cannot be reused here: it creates and unlinks + /// real segment files, and an in-memory partition carries no + /// `partition_dir`, so it would write a chain to whatever path the config + /// resolves. What it does to the chain's SHAPE is the part the simulator + /// needs, and this makes exactly the same two decisions on the same two + /// conditions. + /// + /// Without it a restored replica keeps a single segment named at 0 while the + /// counter resumes a lease block above it, and the next mint lands INSIDE + /// that segment. Production can never reach that shape -- boot plants at the + /// append point -- so the harness both diverges from what it is modelling and + /// cannot expose the chain refusal a real node would hit on the boot after. + #[cfg(any(test, feature = "simulator"))] + pub fn reanchor_in_memory_to_mint_frontier(&mut self, segment_size: IggyByteSize) { + let frontier = self.mint_frontier(); + if frontier == 0 { + return; + } + // Empty tails named below the append point claim a range they do not + // hold. No files to unlink, so the retire is the whole job. + while let Some(segment) = self.log.segments().last() { + if segment.size.as_bytes_u64() > 0 || segment.start_offset >= frontier { + break; + } + if self.log.retire_back().is_none() { + break; + } + self.stats.decrement_segments_count(1); + } + let tail = self + .log + .segments() + .last() + .map(|segment| (segment.end_offset, segment.size.as_bytes_u64())); + let plant = match tail { + // An emptied chain: plant at the append point, the same as boot's + // `None` arm. Nothing precedes it, so there is no gap to record. + None => true, + // A SIZED tail below the append point gets sealed and planted past. + // An empty one either just went or is already named at the frontier + // and can take the appends as it stands. + Some((sealed_end, size)) if size > 0 && sealed_end.saturating_add(1) < frontier => { + self.log.active_segment_mut().sealed = true; + true + } + Some(_) => false, + }; + if plant { + self.log.add_persisted_segment( + crate::Segment::new(frontier, segment_size), + server_common::SegmentStorage::default(), + None, + None, + ); + self.stats.increment_segments_count(1); + } + } + /// Copy this incarnation's offset counter into the shared /// [`PartitionStats`], making it the value readers (offset validation, /// `get_topic`, `get_stats`) see. @@ -894,28 +1170,83 @@ where .set_current_offset(self.offset.load(Ordering::Acquire)); } - /// The next message offset this replica will mint, `0` while the offset - /// space is still empty. The value stamped into the durable record. + /// One past the highest COMMITTED offset, `0` while the offset space is + /// still empty. The value stamped into the durable record, and what a + /// transfer offer advertises. + /// + /// Not [`Self::mint_frontier`]: after a reservation-seeded boot the append + /// point stands a lease block above this, and neither the record nor an + /// offer may claim offsets no message reached. #[must_use] pub fn offset_frontier(&self) -> u64 { - if self.should_increment_offset { + if self.offset_space.committed_seeded { self.offset.load(Ordering::Acquire).saturating_add(1) } else { 0 } } + /// The offset the next mint will take, which is where the segment chain has + /// to be anchored for the appends that follow to land contiguously. + /// + /// Above [`Self::offset_frontier`] by exactly the offsets this replica has + /// journaled but not committed, plus -- on the first boot after a crash that + /// took acked-but-unflushed messages with it -- the lease block the durable + /// reservation claimed. That gap is the whole point: the reservation is the + /// only surviving witness that those offsets were handed to a client, so the + /// counter resumes above them instead of re-minting them. + #[must_use] + pub fn mint_frontier(&self) -> u64 { + if self.offset_space.append_live { + self.dirty_offset.load(Ordering::Relaxed).saturating_add(1) + } else { + 0 + } + } + + /// One past the highest offset this replica holds and may not lose: named by + /// a sized segment, or committed and still resident in the journal. `0` when + /// it holds nothing. + /// + /// The committed arm is not redundant with the disk arm, since the + /// threshold-gated flush routinely leaves committed messages unnamed by any + /// segment. It reads the COMMITTED counter and not `journal.info`, whose + /// `current_offset` is the DIRTY tail: a view change truncates that tail + /// (`truncate_uncommitted_from`) while the durable frontier only advances, so + /// recording it would leave every later boot seeding the counter above the + /// group, where each replicated prepare fails the + /// `base_offset == dirty_offset + 1` check until a state transfer -- again on + /// the boot after that one. + #[must_use] + pub fn held_offset_frontier(&self) -> u64 { + // Reverse search, not a scan-and-max: the chain is ordered and the + // contiguity guard keeps end offsets ascending, so the LAST sized segment + // is the highest one. Only the trailing empties are walked. + let on_disk = self + .log + .segments() + .iter() + .rev() + .find(|segment| segment.size.as_bytes_u64() > 0) + .map_or(0, |segment| segment.end_offset.saturating_add(1)); + on_disk.max(self.offset_frontier()) + } + /// Force the durable record to catch up with the current offset frontier, /// outside the view-change gate. /// /// [`Self::persist_superblock_if_needed`] fires on `(view, log_view)` /// changes only, which is the right trigger for the split-brain fence and /// the wrong one for the frontier: an install can move the counter by - /// millions without touching the view. Called where the frontier changes - /// with nothing else durable naming it -- after a state-transfer install - /// and after the convergence that follows a failed one. Returns whether the - /// record now holds it; a failure is logged by the writer and left to the - /// ordinary retry, since the install itself already succeeded. + /// millions without touching the view. + /// + /// One production caller, at the END of a state-transfer install, which is + /// also the converge path that follows a failed one. It pairs with the + /// [`Self::persist_offset_frontier_at`] the install writes BEFORE its + /// destructive swap: that one is a lower bound across the swap window, this + /// one records what the installed chain actually holds. Returns whether the + /// record now holds it; a failure is logged there and left to the ordinary + /// retry, since the install itself already succeeded. #[allow(clippy::future_not_send)] #[must_use = "the bool is the durability verdict; dropping it silently ignores a failed write"] pub async fn persist_offset_frontier(&self) -> bool { @@ -959,7 +1290,9 @@ where return false; } let _superblock_guard = self.superblock_lock.acquire().await; - self.write_superblock_inner(superblock.as_ref(), frontier) + // Reservation reset with it: left above, it would seed the next boot + // back into the offset space this reset just left behind. + self.write_superblock_inner(superblock.as_ref(), frontier, frontier) .await } @@ -977,6 +1310,11 @@ where /// `intended` is the frontier the caller knows the group is at, written /// verbatim; `None` means the live counter is authoritative and the /// advancing form applies. + /// + /// The reservation keeps its max either way: `vsr_state` calls it a monotone + /// ceiling, and only a purge or an install may bring it down. Inert while + /// the one `Some` caller is the replicated `ConvergeFailed` arm, where the + /// fence never ran and it already equals the frontier. #[allow(clippy::future_not_send)] #[must_use = "the bool is the durability verdict; dropping it silently ignores a failed write"] pub async fn record_frontier_before_quarantine(&self, intended: Option) -> bool { @@ -986,7 +1324,8 @@ where let _superblock_guard = self.superblock_lock.acquire().await; match intended { Some(frontier) => { - self.write_superblock_inner(superblock.as_ref(), frontier) + let reserved = frontier.max(self.durable_offset_reserved.get()); + self.write_superblock_inner(superblock.as_ref(), frontier, reserved) .await } None => { @@ -1007,6 +1346,48 @@ where self.consensus.clock_realtime_micros() < self.superblock_retry_after_micros.get() } + /// Drop the reservation back onto the frontier, once a graceful flush has + /// made the segments account for every offset this replica confirmed. + /// + /// The reservation is there for the crash case, where they do not. Left + /// standing it would make every ordinary restart resume a lease block + /// higher and hole the offset space for nothing. + /// + /// Collapses onto [`Self::mint_frontier`], not [`Self::offset_frontier`]: the + /// append point is what the next boot has to resume at, and on a boot that + /// consumed a reservation without appending it is the reservation itself, so + /// reading the committed frontier here would write a record BELOW what an + /// earlier life already confirmed to a client. A clean stop is the runbook + /// answer to an incident, which would make it the one action that undoes the + /// protection. + /// + /// The frontier field still records only what is held: a graceful stop + /// flushes the committed prefix, but the journal can hold an uncommitted tail + /// that the next view legitimately truncates. + /// + /// Callers must have flushed FIRST, and must not call this when the flush + /// failed: the claim it makes is precisely that the flush succeeded. + /// + /// BYPASSES the retry backoff, like `record_frontier_before_quarantine`: the + /// stop is the last chance, not deferred work. Skipped, the reservation + /// stands and the next boot seeds the append counter a lease block above the + /// data, holing the offset space for nothing. + #[allow(clippy::future_not_send)] + #[must_use = "the bool is the durability verdict; a failed collapse leaves a gap"] + pub async fn collapse_offset_reservation(&self) -> bool { + let append_point = self.mint_frontier(); + if self.durable_offset_reserved.get() <= append_point { + return true; + } + let Some(superblock) = self.superblock.as_ref().map(Rc::clone) else { + return true; + }; + let _superblock_guard = self.superblock_lock.acquire().await; + let held = self.advanced_frontier(0); + self.write_superblock_inner(superblock.as_ref(), held, append_point) + .await + } + /// [`Self::persist_offset_frontier`] for a frontier this replica has not /// reached yet. /// @@ -1030,6 +1411,370 @@ where self.write_superblock(superblock.as_ref(), frontier).await } + /// Whether the offset reservation is close enough to being consumed that it + /// should be extended NOW, off the append path. + /// + /// The append fence is correct but badly placed: it writes the superblock + /// inline in the shard's frame pump, where the consensus tick is a sibling + /// arm, so its two fsyncs delay heartbeats for every group on the core. The + /// fix is to make the fence's fast path + /// (`durable_offset_reserved > end_offset`) the only path it ever takes under + /// load, by extending from the tick instead. + /// + /// HALF a block of headroom, which is a wide margin on purpose: over-claiming + /// costs nothing but offset space, while arriving late puts the write back on + /// the append path. Floored at 1, since validation admits a lease of 1 and + /// `1 / 2` would never trigger, leaving every append to pay the inline claim. + /// A partition that has never minted is skipped unless a send has already + /// been bounced for it (`should_defer_first_reservation`): extending + /// every idle partition at boot would write a superblock per partition for + /// nothing, while a partition someone is producing to needs its first block + /// claimed off the request path like every later one. + #[must_use] + pub fn needs_offset_reservation_extension(&self) -> bool { + if self.consensus.replica_count() > 1 || self.superblock.is_none() { + return false; + } + if self.offset_reservation_wanted.get() { + return true; + } + if !self.offset_space.append_live { + return false; + } + let headroom = self + .durable_offset_reserved + .get() + .saturating_sub(self.mint_frontier()); + headroom < (self.offset_reservation_lease / 2).max(1) + } + + /// Whether this send should be BOUNCED so the shard tick claims the + /// partition's first block, rather than paying for it inline. + /// + /// Without this the first append to every untouched solo partition awaits a + /// create, write, file fsync, rename and directory fsync inside the shard's + /// request pump, where the consensus tick is a sibling arm. A + /// high-cardinality first-write burst serializes those fences and delays + /// unrelated group work and heartbeats on the same core. + /// + /// One retry, once in a partition's life: the bounce is + /// `TransientNotAccepted`, which admitted nothing, and by the time the client + /// re-sends, the tick has claimed the block and the fence takes its fast + /// path. + /// + /// `false` once a claim covers the batch, and `false` for a first batch wider + /// than the whole lease -- the tick's claim would not cover that one either, + /// so bouncing it would bounce the same request forever. + /// + /// `false` with no store attached, for the same reason. A storeless partition + /// (in-memory, simulated) reserves nothing at all, and + /// [`Self::needs_offset_reservation_extension`] skips it, so nothing would + /// ever clear the bounce: every first send would be denied for the life of + /// the partition. The gates here and there must agree on which partitions the + /// tick can serve. + #[must_use] + const fn should_defer_first_reservation(&self, end_offset: u64) -> bool { + self.superblock.is_some() + && !self.offset_space.append_live + && self.durable_offset_reserved.get() <= end_offset + && end_offset < self.offset_reservation_lease + } + + /// Extend the reservation a full block past the CEILING already on disk. + /// + /// Pairs with [`Self::needs_offset_reservation_extension`]; the caller is the + /// shard tick, so this write is off the append path. A failure needs no + /// handling beyond the writer's own logging and backoff: the fence at the + /// mint is still there, and it is what refuses the append if the ceiling + /// never caught up. + /// + /// From the ceiling, NOT [`Self::mint_frontier`]. The trigger fires while the + /// append point still sits under the ceiling -- that being the point of + /// extending early -- so claiming a lease past the append point buys back only + /// the headroom the trigger had left, about half a lease, and doubles the + /// write rate the default lease is sized for. Maxed against the append point + /// so a ceiling that somehow fell behind still comes forward. + #[allow(clippy::future_not_send)] + pub async fn extend_offset_reservation(&self) -> bool { + let Some(superblock) = self.superblock.as_ref().map(Rc::clone) else { + return true; + }; + if self.superblock_write_is_backed_off() { + return false; + } + let _superblock_guard = self.superblock_lock.acquire().await; + let ceiling = self.durable_offset_reserved.get().max(self.mint_frontier()); + let written = self.write_claim_from(superblock.as_ref(), ceiling).await; + if written { + // Only on success: a failed write leaves the bounce standing so the + // next tick retries it, rather than dropping the partition back to + // paying inline. + self.offset_reservation_wanted.set(false); + } + written + } + + /// Upper bound on the offsets a pending `SendMessages` request will mint, for + /// fencing it BEFORE it enters the pipeline. + /// + /// `project` assigns an op, not a base offset, so the exact range is unknown + /// until the mint runs under `write_lock`. This is deliberately loose: a + /// request pipelined behind others can land above it, and the fence at the + /// mint stays as the exact check. It does not need to be tight -- the claim + /// runs a whole lease block past whatever it is handed, so one of these + /// covers every batch in flight unless a run of them crosses a block + /// boundary. + /// + /// `None` above one replica, where nothing is reserved, and when the body is + /// not one canonical batch, which `convert_request_message` has already + /// rejected by the time this runs. + /// + /// Header decode, NOT `decode_batch_slice`: the verifying decode fails on + /// every ordinary send, because `convert_request_message` runs at + /// [`ChecksumMode::Skip`] and leaves `batch_checksum` zeroed, which would + /// silently drop the fence back to the mint. It also re-hashes every body + /// `admit_wire_request` already hashed. + fn request_mint_ceiling(&self, message: &Message) -> Option { + if self.consensus.replica_count() > 1 { + return None; + } + let body = message + .as_slice() + .get(std::mem::size_of::()..message.header().size as usize)?; + let count = BatchHeader::decode(body).ok()?.message_count; + // The batch's LAST offset, not one past it: the claim adds the exclusive + // successor and the lease itself, so a ceiling one too high wastes an + // offset on every claim and overstates what a crash can lose. + // + // Saturating rather than `None` on overflow. `None` means "no fence + // applies here" and would send an exhausted offset space on to the mint, + // where the refusal fences the partition and takes the node down; a + // saturated ceiling reaches the fence instead and comes back as a + // retryable transient. + let last = u64::from(count).saturating_sub(1); + Some(self.mint_frontier().saturating_add(last)) + } + + /// The append fence: make sure the durable record already permits every + /// offset up to and including `end_offset` before the caller lets them + /// exist. + /// + /// `SendMessagesResponse` hands clients concrete base offsets and the poll + /// path serves committed messages out of the resident journal, so an offset + /// is client-visible long before the threshold-gated flush names it in a + /// segment, and a crash in between hands a second message an offset a client + /// already holds. Fencing here rather than at commit puts it upstream of + /// every way a NEWLY minted offset escapes -- the reply, the poll tier, the + /// peers a prepare reaches -- on the one path both a primary's mint and a + /// backup's re-stamp take. Journal repair + /// (`append_repaired_send_messages`) is the exception and needs none: it + /// re-journals offsets a peer already minted and fenced, so there is nothing + /// new to claim. Claiming through `end_offset + 1 + lease` rather than from + /// the live counter needs no special case for an oversized batch. + /// + /// SOLO ONLY, like everything the reservation feeds: `restore_offset_frontier` + /// seeds no counter from it above one replica, and the boot re-anchor that + /// shapes the chain around it never runs there either. A replicated group + /// paying a superblock write per block would buy nothing -- and an ack there + /// already means a quorum journaled the batch, so re-minting needs a + /// FULL-cluster crash. + /// + /// `false` when the write was attempted and failed. Fail-closed: the send is + /// rejected with nothing externalised, exactly as a failed view persist + /// withholds its sends. + /// + /// BYPASSES the retry backoff, like `record_frontier_before_quarantine` and + /// for the same reason: this is the fence at the MINT, where a refusal is + /// terminal. The failure cell is shared with every other superblock writer on + /// the partition, so a purge's frontier reset or the tick's own extension + /// failing once would otherwise open a 20 ms window (up to 1 s after repeats) + /// in which this takes the node down over a fault on another path entirely. + /// The refusal is also not deferred work: `superblock_wedged` is the gate that + /// decides a run of failures is terminal. + /// + /// The ADMITTED path does honour the backoff, through + /// `reserve_offsets_through_retryable`, because a refusal there costs + /// one client retry and re-running a full atomic replace per retry starves + /// the shard pump on a failing disk. + /// + /// WHERE it is called decides how much a refusal costs. Ahead of the pipeline + /// (`on_request`) the client gets a retryable transient and the group keeps + /// serving. At the mint the op already has its number and its ack is already + /// skipped, so `commit_max` can never pass it and nothing later can commit + /// either: `on_replicate` fences the partition there and takes the node down. + #[allow(clippy::future_not_send)] + #[must_use = "the bool is the fence verdict; dropping it lets the append escape unreserved"] + pub async fn reserve_offsets_through(&self, end_offset: u64) -> bool { + if self.consensus.replica_count() > 1 { + return true; + } + // A frontier: offsets strictly below it are permitted, so covering + // `end_offset` needs a record strictly above it. + if self.durable_offset_reserved.get() > end_offset { + return true; + } + let Some(superblock) = self.superblock.as_ref().map(Rc::clone) else { + return true; + }; + let _superblock_guard = self.superblock_lock.acquire().await; + // A batch queued behind another append's write finds the block already + // extended. + if self.durable_offset_reserved.get() > end_offset { + return true; + } + self.write_offset_claim(superblock.as_ref(), end_offset) + .await + } + + /// [`Self::reserve_offsets_through`] for the ADMITTED path, where a refusal + /// costs the client one retry rather than the process its life. + /// + /// Identical except that it honours the superblock retry backoff, AFTER the + /// coverage fast path: a batch the record already permits owes the disk + /// nothing and must not be refused by a window some other writer opened. + /// + /// The fence at the mint deliberately does not honour it. By the time that + /// one runs the op has its number and its ack is already skipped, so a + /// refusal fences the partition and takes the node down; attempting the write + /// against a disk that just refused one is strictly better than declining to + /// try. Here the client simply retries, so re-running a full create, write, + /// file fsync, rename and directory fsync per retry inside an open window + /// buys nothing and starves the shard pump for as long as the fault lasts. + #[allow(clippy::future_not_send)] + #[must_use = "the bool is the fence verdict; dropping it lets the append escape unreserved"] + async fn reserve_offsets_through_retryable(&self, end_offset: u64) -> bool { + if self.consensus.replica_count() > 1 || self.durable_offset_reserved.get() > end_offset { + return true; + } + if self.superblock_write_is_backed_off() { + return false; + } + self.reserve_offsets_through(end_offset).await + } + + /// The reservation preflight every admitted send passes, whether it arrives + /// at [`Self::on_request`] or is promoted out of the request queue. + /// + /// `true` when the send may be projected. `false` when it was ANSWERED here + /// and must go no further: the client holds a `TransientNotAccepted`, which + /// admitted nothing, so it may re-issue anywhere without double-apply risk. + /// + /// Three ways to come back `false`, none of them reaching the mint: a bounced + /// first send, an open superblock backoff window, and a claim that was + /// attempted and failed. + /// + /// `waiter` is the submit's in-process reply channel, taken only on a + /// refusal: the deny goes there because `header.client` is then the VSR + /// consensus id, which the bus cannot route. + #[allow(clippy::future_not_send)] + async fn admit_reserved_send( + &self, + message: &Message, + waiter: &mut Option>>, + ) -> bool { + if message.header().operation != Operation::SendMessages { + return true; + } + let Some(ceiling) = self.request_mint_ceiling(message) else { + return true; + }; + if self.should_defer_first_reservation(ceiling) { + self.offset_reservation_wanted.set(true); + self.deny_unreserved_send( + message.header(), + "bouncing a partition's first send so the tick claims its offset block", + waiter.take(), + ) + .await; + return false; + } + if !self.reserve_offsets_through_retryable(ceiling).await { + self.deny_unreserved_send( + message.header(), + "refusing a send: the offset reservation could not be extended", + waiter.take(), + ) + .await; + return false; + } + true + } + + /// Answer a send the reservation would not cover with a retryable transient, + /// and say why. + /// + /// `TransientNotAccepted`, per its contract: nothing was admitted, so the + /// client may re-issue anywhere without double-apply risk. It does make the + /// SDK recheck the leader and walk the roster, which finds no better node + /// when the fault is this one's disk -- wasteful, but the weaker code would + /// claim an unknown outcome for a request that provably has none. + #[allow(clippy::future_not_send)] + async fn deny_unreserved_send( + &self, + header: &RoutedRequestHeader, + reason: &'static str, + waiter: Option>>, + ) { + let consensus = self.consensus(); + emit_partition_diag( + tracing::Level::WARN, + &PartitionDiagEvent::new( + ReplicaLogContext::from_consensus(consensus, PlaneKind::Partitions), + reason, + ) + .with_operation(Operation::SendMessages), + ); + Self::send_partition_deny_or_log( + consensus, + header, + IggyError::TransientNotAccepted.as_code(), + "unreserved send transient reply send failed", + waiter, + ) + .await; + } + + /// Claim a block past `end_offset` unconditionally. + /// + /// Split from [`Self::reserve_offsets_through`] because the tick's extension + /// has to write while the ceiling still covers the append point -- that is the + /// point of extending early -- and the fence's coverage fast path would + /// short-circuit exactly that call. The advancing write is what keeps this + /// monotone: a claim below the record cannot lower it. + /// + /// `false` for an `end_offset` of `u64::MAX`, which EXHAUSTS the offset space: + /// the record is an exclusive frontier, so covering an offset needs a value + /// strictly above it and `u64::MAX + 1` does not exist. Saturating instead + /// would record `u64::MAX`, report success for an offset it does not cover, + /// and let the next boot seed the append counter at `u64::MAX - 1` and re-mint + /// an offset a client already holds -- the one defect this whole path exists + /// to prevent, at the one offset where it would be silent. + #[allow(clippy::future_not_send)] + async fn write_offset_claim(&self, superblock: &SB, end_offset: u64) -> bool { + let Some(exclusive) = end_offset.checked_add(1) else { + tracing::error!( + namespace_raw = self.consensus.group(), + end_offset, + "refusing an append that would exhaust the partition's offset space" + ); + return false; + }; + self.write_claim_from(superblock, exclusive).await + } + + /// Claim a lease of offsets past `exclusive`, the lowest offset the record + /// does not yet permit. + /// + /// Saturating on the lease is safe where [`Self::write_offset_claim`]'s + /// successor is not: a claim clamped to `u64::MAX` still sits strictly above + /// every `end_offset` below it, so the coverage test still holds. Only the + /// successor itself can push the frontier off the end of the space. + #[allow(clippy::future_not_send)] + async fn write_claim_from(&self, superblock: &SB, exclusive: u64) -> bool { + let claim = exclusive.saturating_add(self.offset_reservation_lease); + self.write_superblock_advancing(superblock, 0, claim).await + } + /// Burn one transfer stall round; `true` once the budget is exhausted. /// Lives on the partition, not the session, so a re-minted session /// cannot reset it (see [`Self::transfer_attempts`]). @@ -1135,6 +1880,18 @@ where .unwrap_or(config.messages_required_to_save) } + /// Install the offset-reservation block size resolved from this node's + /// `PartitionsConfig`. + /// + /// Carried on the partition because the fence runs inside `on_request` / + /// `on_replicate`, which take no config. `NonZeroU32` because a zero block + /// reserves nothing and would write the superblock before every append: + /// coercing it here instead would contradict the configuration validator and + /// hide a wiring error that handed this a zero. + pub const fn set_offset_reservation_lease(&mut self, lease: NonZeroU32) { + self.offset_reservation_lease = lease.get() as u64; + } + /// Whether this partition's segments reserve their bytes on open. #[must_use] pub fn effective_preallocate_segments(&self, config: &PartitionsConfig) -> bool { @@ -1604,7 +2361,7 @@ where // commit). Also used below as the poll's high-water bound: this function // is fully synchronous, so the single load cannot drift mid-plan. let commit_offset = self.offsets().commit_offset; - if !self.should_increment_offset || args.count == 0 { + if !self.offset_space.committed_seeded || args.count == 0 { return PollPlan { commit_offset, auto_commit: None, @@ -1852,7 +2609,10 @@ where return Err(IggyError::CannotAppendMessage); } - let dirty_offset = if self.should_increment_offset { + // Only here: this is the only path that mints. A backup re-stamps what + // the primary sends (`append_received_send_messages_to_journal`) and + // must follow it exactly, so raising ITS counter would fork the group. + let dirty_offset = if self.offset_space.append_live { self.dirty_offset .load(Ordering::Relaxed) .checked_add(1) @@ -1885,6 +2645,15 @@ where self.fatal.as_ref() } + /// Consecutive superblock write failures for this group, for the shard's + /// wedge fail-stop. A partition that cannot record its state withholds every + /// view-scoped send and refuses every append, so past some window it is + /// serving nothing and a supervisor should be handling it instead. + #[must_use] + pub const fn superblock_write_failures(&self) -> u64 { + self.superblock_write_failures.get() + } + /// Fence this partition after the shutdown flush failed to persist its /// committed journal prefix: that data is cluster-committed and now lives /// only in this process's memory, so the shard must not report a clean @@ -2244,6 +3013,18 @@ where offset, } } else { + // Fence AHEAD of the pipeline for a mint, not only at the mint. + // A refusal at the mint arrives after the sequencer took the op, + // where the only honest answer left is to fence the partition and + // take the node down (`on_replicate`). Here the request has + // entered nothing, so a transient disk fault costs the client one + // retry instead of costing the process its life. + // + // The preflight answers the client itself on a refusal; see + // [`Self::admit_reserved_send`]. + if !self.admit_reserved_send(&message, &mut reply).await { + return; + } // Two-queue: prepare slot -> project+replicate; prepare full + // request room -> buffer; both full -> drop+warn (client retries // via read-timeout). @@ -2311,9 +3092,18 @@ where /// Promote up to `slots_freed` buffered requests into prepares post-commit. /// - /// Promotion runs no preflight: the request was classified at admission - /// and the slice cannot have gained a higher watermark for it since (only - /// a commit moves it, and this entry has not committed). + /// Promotion runs no DEDUP preflight: the request was classified at + /// admission and the slice cannot have gained a higher watermark for it + /// since (only a commit moves it, and this entry has not committed). + /// + /// The RESERVATION preflight is repeated per promotion, and re-derives the + /// ceiling from the live mint frontier rather than trusting the one the + /// request was admitted under. Several queued batches can cumulatively cross + /// the lease while they wait, and without this the first one past it reaches + /// the exact fence at the mint, where a refusal fences the partition and + /// takes the node down instead of returning `TransientNotAccepted`. A refused + /// promotion ends the drain: the ones behind it want the same claim and would + /// each be answered with the same transient. /// /// Per-iteration `is_primary && is_normal && !is_transferring` asserts inlined /// (closure form's `&consensus` borrow conflicts with `&mut self`). Guards @@ -2332,6 +3122,16 @@ where let req = self.consensus().pop_queued_request(); let Some(mut req) = req else { break }; + // Taken before the preflight so a refusal answers the parked waiter + // instead of waking it with `Canceled`. + let mut reply_sender = req.take_reply_sender(); + if !self + .admit_reserved_send(&req.message, &mut reply_sender) + .await + { + break; + } + let prepare = { let consensus = self.consensus(); assert!( @@ -2348,7 +3148,6 @@ where ); // The waiter parked with the request; it must travel into the // prepare slot or the commit has nobody to answer. - let reply_sender = req.take_reply_sender(); let prepare = req.message.project(consensus); consensus.verify_pipeline(); match reply_sender { @@ -2593,16 +3392,57 @@ where let frozen_for_forward = match replicated_result { Ok(frozen) => frozen, Err(error) => { + // A BACKUP refusing here is the design, not a fault: it rejects + // any prepare whose `base_offset` does not continue its own + // counter, which is exactly what a dropped or out-of-order + // prepare leaves, and withholding `PrepareOk` is the fail-closed + // answer. The primary retransmits, journal repair fills the gap, + // and the group elects around the replica if it cannot catch up. + // Nothing here is owed an ack this replica already skipped. + // + // On the PRIMARY the same return is unrecoverable. The op is + // already in the pipeline with the sequencer advanced past it + // (`push_prepare_entry`), and this sits ahead of + // `send_prepare_ok`, so it never gets its ack and `commit_max` + // can never pass it: every later op journals fine and queues + // behind it forever. Nothing lifts that -- the prepare timeout + // only backs off, a solo group's retransmit target is itself, + // this plane has no `repair_primary_self_acks`, and a solo group + // never starts a view change. Clients get no reply at all, since + // replies are generated on commit, so they wait out their read + // timeout, and once the queues fill so does every send after. + // + // So fence there, the way a failed local commit of a + // cluster-committed op does: the shard picks `fatal` up on its + // next tick and takes the node down. A one-second superblock + // backoff must not cost a partition the rest of the process's + // life in the dark. emit_partition_diag( - tracing::Level::WARN, + if is_backup { + tracing::Level::WARN + } else { + tracing::Level::ERROR + }, &PartitionDiagEvent::new( self.diag_ctx(), - "failed to apply replicated partition operation", + if is_backup { + "failed to apply replicated partition operation" + } else { + "failed to apply an operation this replica sequenced; \ + fencing the partition and shutting down" + }, ) .with_operation(header.operation) .with_op(header.op) .with_error(error.to_string()), ); + if !is_backup && self.fatal.is_none() { + self.fatal = Some(FatalCommit { + namespace_raw: self.namespace().inner(), + op: header.op, + operation: header.operation, + }); + } return; } }; @@ -2882,7 +3722,7 @@ where if validated.message_count == 0 { return Err(IggyError::InvalidCommand); } - let expected_offset = if self.should_increment_offset { + let expected_offset = if self.offset_space.append_live { self.dirty_offset .load(Ordering::Relaxed) .checked_add(1) @@ -2916,7 +3756,20 @@ where .checked_add(u64::from(batch_messages_count) - 1) .ok_or(IggyError::CannotAppendMessage)?; - let segment_index = self.log.segments().len() - 1; + // Past this line the offsets are in the journal, hence committable, + // pollable, confirmable and forwardable, and no later gate can take + // them back. See [`Self::reserve_offsets_through`]. + // + // LOCK ORDER: the caller holds `write_lock` and this takes + // `superblock_lock` under it. The install path takes them in the + // reverse order, safely only because `reset_offset_frontier_at` drops + // `superblock_lock` before `try_install` takes `write_lock`. Never hold + // `superblock_lock` across a `write_lock` acquire. + if !self.reserve_offsets_through(last_dirty_offset).await { + return Err(IggyError::CannotAppendMessage); + } + + let segment_index = self.log.segments().len() - 1; let current_position = self.log.segments()[segment_index].current_position; let next_position = current_position .checked_add(batch_messages_size) @@ -2949,7 +3802,7 @@ where .await .map_err(|_| IggyError::CannotAppendMessage)?; - self.should_increment_offset = true; + self.note_append_live(); self.dirty_offset .store(last_dirty_offset, Ordering::Relaxed); self.log.segments_mut()[segment_index].current_position = next_position; @@ -3040,7 +3893,15 @@ where let next_offset = next_offset.max(minimum_next_offset); self.dirty_offset .store(next_offset.saturating_sub(1), Ordering::Relaxed); - self.should_increment_offset = next_offset > 0; + // The APPEND bit follows the rewound counter. The committed bit does + // not: this drops an uncommitted suffix, which by definition names + // nothing that ever committed, so raising it here would make a + // truncation the event that publishes offsets no quorum agreed on. + // A rewind to zero is the exception -- nothing is left at all. + self.offset_space.append_live = next_offset > 0; + if next_offset == 0 { + self.offset_space.committed_seeded = false; + } } self.consensus.invalidate_local_dvc_suffix(); Ok(removed) @@ -3340,6 +4201,7 @@ where // gated, so counting here would leave the stats lagging the visible // offset until a flush and would double-count once it fires. if let Some(durable_offset) = durable_offset { + self.note_committed_seeded(); self.offset.store(durable_offset, Ordering::Release); self.stats.set_current_offset(durable_offset); } @@ -3644,6 +4506,12 @@ where if let Some(batch_stats) = batch_stats { let end_offset = batch_stats.end_offset(); + // The committed counter now names data, which is what makes + // it pollable and persistable. Outside the recovered-offset + // guard below: that guard only skips re-counting stats a + // previous life already persisted, and those offsets are + // committed either way. + self.note_committed_seeded(); // A repaired batch at or below the boot-time recovered // durable offset was already counted (and persisted) // before the restart; skip it. Live traffic always sits @@ -4043,98 +4911,72 @@ where } async fn rotate_segment(&mut self, config: &PartitionsConfig) -> Result<(), IggyError> { - let namespace = self.namespace(); - let old_segment_index = self.log.segments().len() - 1; - let active_segment = self.log.active_segment_mut(); - active_segment.sealed = true; - let start_offset = active_segment.end_offset + 1; + let start_offset = self.log.active_segment().end_offset + 1; + self.rotate_segment_at(config, start_offset).await + } - let segment_size = self.effective_segment_size(config); - let enforce_fsync = self.effective_enforce_fsync(config); - let preallocate_segments = self.effective_preallocate_segments(config); - let segment = Segment::new(start_offset, segment_size); - // Prefer the active writer's location: a per-topic path override or a - // config change after the initial segment was created must not scatter - // one partition's segments across two directories. The config layout - // only decides for a partition with no writer yet. - let (messages_path, index_path) = self.partition_dir().map_or_else( - || { - ( - config.get_messages_path( - namespace.stream_id(), - namespace.topic_id(), - namespace.partition_id(), - start_offset, - ), - config.get_index_path( - namespace.stream_id(), - namespace.topic_id(), - namespace.partition_id(), + /// Seal the active segment and plant a fresh empty one at `start_offset`. + /// + /// Shared by the size-driven roll, which plants at `end_offset + 1`, and the + /// boot re-anchor, which plants at the append point the reservation moved the + /// counter to. One seal path, and one order: the new segment's files are + /// created BEFORE the sealed segment's writers are torn down, so a failed + /// create leaves the chain serviceable. + async fn rotate_segment_at( + &mut self, + config: &PartitionsConfig, + start_offset: u64, + ) -> Result<(), IggyError> { + let namespace = self.namespace(); + let sealed_index = self.log.segments().len() - 1; + let sealed_end = self.log.active_segment().end_offset; + debug_assert!( + start_offset > sealed_end, + "a plant at {start_offset} overlaps the sealed tail ending at {sealed_end}" + ); + // A wider gap than the roll's own is legitimate only with the anchor + // already durable, which is the caller's obligation + // (`record_reanchor_gap`) and is otherwise readable nowhere in here. + #[cfg(debug_assertions)] + if start_offset > sealed_end.saturating_add(1) + && let Some(partition_dir) = self.partition_dir() + { + debug_assert!( + matches!( + crate::segment_anchor::read_anchor(&partition_dir, start_offset).await, + Ok(Some(anchor)) if anchor.covers( start_offset, - ), - ) - }, - |dir| { - ( - format!("{dir}/{start_offset:0>20}.log"), - format!("{dir}/{start_offset:0>20}.index"), - ) - }, - ); - - let storage = SegmentStorage::new(&messages_path, &index_path, 0, 0, false) - .await - .map_err(|_| IggyError::CannotCreateSegmentLogFile(messages_path.clone()))?; - let messages_size_bytes = storage - .messages_writer - .as_ref() - .ok_or_else(|| IggyError::CannotCreateSegmentLogFile(messages_path.clone()))? - .size_counter(); - let messages_writer = Rc::new( - MessagesWriter::new( - &messages_path, - messages_size_bytes, - enforce_fsync, - false, - preallocate_segments.then_some(segment_size), - ) - .await - .map_err(|_| IggyError::CannotCreateSegmentLogFile(messages_path.clone()))?, - ); - let index_size_bytes = storage - .index_writer - .as_ref() - .ok_or_else(|| IggyError::CannotCreateSegmentIndexFile(index_path.clone()))? - .size_counter(); - let index_writer = Rc::new( - IggyIndexWriter::new(&index_path, index_size_bytes, enforce_fsync, false) - .await - .map_err(|_| IggyError::CannotCreateSegmentIndexFile(index_path.clone()))?, - ); + self.log.segments()[sealed_index].start_offset, + sealed_end, + ) + ), + "a plant at {start_offset} leaves a gap past {sealed_end} with no anchor" + ); + } + self.log.active_segment_mut().sealed = true; + self.install_empty_segment(config, start_offset).await?; + self.stats.increment_segments_count(1); - let old_storage = &mut self.log.storages_mut()[old_segment_index]; - let _ = old_storage.shutdown(); - self.log.messages_writers_mut()[old_segment_index] = None; - self.log.index_writers_mut()[old_segment_index] = None; + let sealed_storage = &mut self.log.storages_mut()[sealed_index]; + let _ = sealed_storage.shutdown(); + self.log.messages_writers_mut()[sealed_index] = None; + self.log.index_writers_mut()[sealed_index] = None; // Drop the sealed segment's in-memory index cache: only the ACTIVE // segment's cache is ever read (the `commit_messages` flush staging), // so a sealed cache is dead weight. - self.log.indexes_mut()[old_segment_index] = None; + self.log.indexes_mut()[sealed_index] = None; // The read fd cached while this segment was active is not counted by // the sealed LRU budget, so it must not survive the seal; the next // sealed poll re-fills the fresh slot under the LRU's rules. - self.log.reset_read_state(old_segment_index); - - self.log - .add_persisted_segment(segment, storage, Some(messages_writer), Some(index_writer)); - self.stats.increment_segments_count(1); + self.log.reset_read_state(sealed_index); debug!( target: "iggy.partitions.diag", plane = "partitions", namespace_raw = namespace.inner(), + sealed_end, start_offset, - "rotated to new segment" + "sealed the active segment and planted a fresh one" ); Ok(()) } @@ -4258,7 +5100,11 @@ where let _ = storage.shutdown(); drop(storage); - for path in messages_path.into_iter().chain(index_path) { + for path in messages_path + .into_iter() + .chain(index_path) + .chain(self.anchor_cleanup_path(segment.start_offset)) + { match compio::fs::remove_file(&path).await { Ok(()) => {} Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} @@ -4386,6 +5232,218 @@ where Ok(()) } + /// Re-anchor the append point after boot re-seeded the offset counter above + /// what the recovered segment chain holds. + /// + /// A hole INSIDE a segment is not survivable: `recover_segment_bounds` walks + /// a segment from its FILENAME with a running `expected_offset` and REFUSES + /// at the first offset that does not continue it (`OffsetDiscontinuity`), + /// which on a solo group tombstones the partition. A surviving index does not + /// help: a first entry that is not the file-name offset makes recovery + /// discard the index and walk from byte 0, reaching the same refusal. On a + /// segment BOUNDARY every reader copes -- absolute offsets in the index, + /// `disk_poll_start` walking on into later segments, and a chain guard that + /// admits a forward gap the reservation covers. + /// + /// So an empty tail is unlinked (its name claims a range it does not hold), a + /// sized tail (the only copy of its messages) is sealed with a fresh segment + /// planted at the append point, and a chain the unlinks emptied is planted + /// directly -- `ensure_initial_segment` names its segment for the COMMITTED + /// frontier and would put the first mint inside it. + /// + /// # Errors + /// [`IggyError`] when the fresh segment cannot be created, leaving the + /// partition without a serviceable chain. + #[allow(clippy::future_not_send, clippy::too_many_lines)] + pub async fn reanchor_to_offset_frontier( + &mut self, + config: &PartitionsConfig, + ) -> Result<(), IggyError> { + // Where the next append will land: the counter, or an armed mint floor + // above it. The floor is the whole reason a hole can appear, so + // anchoring to the counter alone would leave the chain as unprepared. + let frontier = self.mint_frontier(); + if frontier == 0 { + return Ok(()); + } + let namespace = self.namespace(); + let mut retired = 0usize; + while let Some(segment) = self.log.segments().last() { + if segment.size.as_bytes_u64() > 0 || segment.start_offset >= frontier { + break; + } + let Some((segment, mut storage)) = self.log.retire_back() else { + break; + }; + let (messages_path, index_path) = storage.segment_and_index_paths(); + let _ = storage.shutdown(); + drop(storage); + for path in messages_path + .into_iter() + .chain(index_path) + .chain(self.anchor_cleanup_path(segment.start_offset)) + { + match compio::fs::remove_file(&path).await { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + // Refused, not logged. The segment is already out of the + // in-memory chain, so a file left behind becomes a + // non-tail empty segment as soon as the plant lands -- + // `[sized][stale empty][planted]` -- which the next boot + // refuses outright as `EmptyNonTailSegment`. Failing boot + // here says so while the directory is still readable. + error!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = namespace.inner(), + path = %path, + %error, + "failed to unlink a stale empty segment during the boot \ + re-anchor; refusing to plant beside it" + ); + return Err(IggyError::CannotDeleteFile); + } + } + } + tracing::info!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = namespace.inner(), + start_offset = segment.start_offset, + offset_frontier = frontier, + "unlinked an empty segment named below the restored offset frontier" + ); + // Boot DOES count the recovered chain -- `load_persisted_segments` + // increments per segment before it looks at the size, so empty tails + // are in the total -- and retention pairs its own retire with a + // decrement. Without this the count stays one high on the wire for + // the life of the process. + self.stats.decrement_segments_count(1); + retired += 1; + } + // Durable before anything is planted beside them: a crash in between + // would boot the stale name back into the chain. Refused, not logged: + // the emptied-chain arm plants through `install_empty_segment`, which + // fsyncs no directory of its own, so a swallowed error here is the whole + // promise gone. + if retired > 0 + && let Some(partition_dir) = self.partition_dir.clone() + && let Err(error) = crate::state_transfer::fsync_dir(&partition_dir).await + { + error!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = namespace.inner(), + partition_dir, + %error, + "boot re-anchor could not fsync the partition dir after unlinking; \ + refusing to plant beside a name that may come back" + ); + return Err(IggyError::CannotSyncFile); + } + // Bounds copied out: the plant below takes `&mut self`, so the borrow on + // the chain cannot still be live. + let tail = self + .log + .segments() + .last() + .map(|segment| (segment.start_offset, segment.end_offset, segment.size)); + match tail { + // An EMPTIED chain still needs the plant, and it cannot be left to + // the caller's `ensure_initial_segment`, which names the segment for + // the COMMITTED frontier and knows nothing of the append point. On + // the shape a crash before the first flush leaves -- committed + // frontier 0, append point a lease block up -- that plants + // `0.log` and then mints inside it, the hole this function exists to + // prevent. The index does not save it either: a first entry that is + // not the file-name offset makes recovery discard the index and walk + // from byte 0, where the discontinuity tombstones the partition. + // + // No anchor: with nothing before it the plant leaves no gap, so the + // chain guard has no pair to judge. + None => { + self.install_empty_segment(config, frontier).await?; + self.stats.increment_segments_count(1); + tracing::info!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = namespace.inner(), + offset_frontier = frontier, + "planted a fresh segment at the restored offset frontier over an \ + empty recovered chain" + ); + } + // Only a SIZED tail: an empty one either just went, or is already + // named at the frontier and can take the appends as it is. + Some((sealed_start, sealed_end, size)) + if size.as_bytes_u64() > 0 && sealed_end.saturating_add(1) < frontier => + { + self.record_reanchor_gap(frontier, sealed_start, sealed_end) + .await?; + self.rotate_segment_at(config, frontier).await?; + tracing::info!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = namespace.inner(), + sealed_end, + offset_frontier = frontier, + "sealed the recovered tail and planted a fresh segment at the \ + restored offset frontier" + ); + } + Some(_) => {} + } + Ok(()) + } + + /// Write the anchor that makes the gap a plant at `frontier` leaves + /// legitimate, and make it durable before the segment exists. + /// + /// The chain guard admits a forward gap only when the far side carries an + /// anchor naming exactly the near side, so this record is what separates the + /// re-anchor's own gap from a lost segment. Ordering is load-bearing in one + /// direction only: an anchor with no segment is swept at the next boot, + /// while a segment with no anchor reads as damage. + /// + /// # Errors + /// [`IggyError::CannotCreateSegmentLogFile`] naming the anchor path. The + /// caller must not plant. + #[allow(clippy::future_not_send)] + async fn record_reanchor_gap( + &self, + frontier: u64, + sealed_start: u64, + sealed_end: u64, + ) -> Result<(), IggyError> { + // No directory means an in-memory partition, whose chain no boot reads. + let Some(partition_dir) = self.partition_dir() else { + return Ok(()); + }; + let anchor = crate::segment_anchor::SegmentAnchor { + planted_start: frontier, + sealed_start, + sealed_end, + }; + if let Err(error) = crate::segment_anchor::write_anchor(&partition_dir, anchor).await { + error!( + target: "iggy.partitions.diag", + plane = "partitions", + namespace_raw = self.namespace().inner(), + offset_frontier = frontier, + sealed_start, + sealed_end, + %error, + "could not record the boot re-anchor's gap; refusing to plant a segment \ + the next boot would read as a lost one" + ); + return Err(IggyError::CannotCreateSegmentLogFile( + crate::segment_anchor::anchor_path(&partition_dir, frontier), + )); + } + Ok(()) + } + /// Record the purge's frontier reset BEFORE the purge touches anything. /// /// The unlinks are made durable by their own directory fsync, so a crash @@ -4477,7 +5535,7 @@ where // Drain every segment (including the active one) and unlink its files. let segment_count = self.log.segments().len(); for _ in 0..segment_count { - let Some((_, mut storage)) = self.log.retire_front() else { + let Some((segment, mut storage)) = self.log.retire_front() else { break; }; @@ -4485,7 +5543,11 @@ where let _ = storage.shutdown(); drop(storage); - for path in messages_path.into_iter().chain(index_path) { + for path in messages_path + .into_iter() + .chain(index_path) + .chain(self.anchor_cleanup_path(segment.start_offset)) + { match compio::fs::remove_file(&path).await { Ok(()) => {} Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} @@ -4532,7 +5594,7 @@ where // whole body. self.offset.store(start_offset, Ordering::Release); self.dirty_offset.store(start_offset, Ordering::Relaxed); - self.should_increment_offset = false; + self.set_offset_space_used(false); // Recreate a fresh empty segment at offset 0 with real writers. Every // segment is drained by now, so a failure here is the fence case. @@ -5060,7 +6122,7 @@ where .await .map_err(|_| IggyError::CannotAppendMessage)?; - self.should_increment_offset = true; + self.note_append_live(); self.dirty_offset .store(dirty.max(last_offset), Ordering::Relaxed); self.log.segments_mut()[segment_index].current_position = next_position; @@ -5384,6 +6446,7 @@ mod tests { use server_common::MESSAGE_ALIGN; use server_common::send_messages::{ COMMAND_HEADER_SIZE, IggyMessage, IggyMessageHeader, IggyMessages, SendMessagesOwned, + decode_batch_slice, }; use std::cell::RefCell; use std::rc::Rc; @@ -5409,6 +6472,26 @@ mod tests { ) } + /// A SOLO partition, the shape the offset reservation is scoped to. + fn solo_recording_partition() -> IggyPartition { + let namespace = IggyNamespace::new(1, 1, 0); + let consensus = VsrConsensus::new( + TEST_CLUSTER, + 0, + 1, + namespace.inner(), + IggyMessageBus::new(0), + LocalPipeline::new(), + ); + consensus.init(); + IggyPartition::with_in_memory_storage( + Arc::new(PartitionStats::default()), + consensus, + IggyByteSize::from(1024 * 1024), + false, + ) + } + /// Partition whose consensus already advanced to `(view, log_view)` with /// nothing marked durable, as after a view change and before the persist /// gate runs. @@ -5517,80 +6600,872 @@ mod tests { .offset_frontier } - /// The fence path persists the frontier while the live counter still sits - /// at its pre-install value, so an advance that maxes against the counter - /// alone erases the record and then quarantines the segments that were its - /// only other witness. Boot re-mints from 0 against a group at N after that. - #[compio::test] - async fn given_record_above_live_counter_when_advancing_should_keep_the_record() { - let mut partition = partition_at_view(1, 1); - let store = Rc::new(RecordingSuperblock::default()); - partition.set_superblock(store.clone(), None); - - assert!(partition.persist_offset_frontier_at(9_000).await); - assert_eq!(last_recorded_frontier(&store), 9_000); - assert_eq!( - partition.offset_frontier(), - 0, - "a partition that never minted reports a zero frontier, which is the \ - value the fence would otherwise persist" - ); - - assert!(partition.persist_offset_frontier().await); - - assert_eq!( - last_recorded_frontier(&store), - 9_000, - "the advance direction must not lower the durable frontier" - ); + fn last_recorded_reservation(store: &RecordingSuperblock) -> u64 { + let writes = store.writes.borrow(); + let bytes = writes.last().expect("a superblock write landed"); + consensus::VsrState::try_from(bytes.as_slice()) + .expect("recorded payload decodes as a VsrState") + .offset_reserved } - /// Attaching a store seeds the last-written frontier from the record - /// itself, so an advance maxes against what boot read off disk even before - /// this replica has written anything. The sibling test reaches that state by - /// WRITING first, which cannot catch an attach site that skips the seed. - #[compio::test] - async fn given_attached_record_when_advancing_should_keep_the_recorded_frontier() { - let mut partition = partition_at_view(1, 1); - let store = Rc::new(RecordingSuperblock::default()); - let recovered = consensus::VsrState { + fn recorded_state(offset_frontier: u64, offset_reserved: u64) -> consensus::VsrState { + consensus::VsrState { cluster: TEST_CLUSTER, replica_id: 0, - replica_count: 3, + replica_count: 1, view: 1, log_view: 1, commit_max: 0, checkpoint_op: 0, checkpoint_checksum: 0, - offset_frontier: 4_200, - }; - partition.set_superblock(store.clone(), Some(&recovered)); - assert_eq!(partition.offset_frontier(), 0, "nothing minted locally"); - - assert!(partition.persist_offset_frontier().await); + offset_frontier, + offset_reserved, + } + } - assert_eq!( - last_recorded_frontier(&store), - 4_200, - "the first write after an attach must not lower the record it was attached to" - ); + fn test_lease(value: u32) -> NonZeroU32 { + NonZeroU32::new(value).expect("a nonzero test lease") } - /// The reset direction is the only way down, and it must actually go there: - /// an install under an advancing purge generation records a frontier below - /// the live counter on purpose. + /// One superblock write per block, not per batch: a fence writing per append + /// would put two fsyncs in front of every produce. #[compio::test] - async fn given_reset_below_live_counter_when_written_should_lower_the_record() { - let mut partition = partition_at_view(1, 1); + async fn given_appends_inside_the_block_when_fencing_should_write_the_superblock_once() { let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); partition.set_superblock(store.clone(), None); - partition.offset.store(9_000, Ordering::Release); - partition.should_increment_offset = true; - - assert!(partition.persist_offset_frontier().await); - assert_eq!(last_recorded_frontier(&store), 9_001); + partition.set_offset_reservation_lease(test_lease(16)); - assert!(partition.reset_offset_frontier_at(12).await); + assert!(partition.reserve_offsets_through(0).await); + assert_eq!(store.attempts.get(), 1, "the first offset claims a block"); + assert_eq!( + last_recorded_reservation(&store), + 17, + "the claim runs one past the offset plus the lease" + ); + + for offset in 1..=16 { + assert!(partition.reserve_offsets_through(offset).await); + } + assert_eq!( + store.attempts.get(), + 1, + "every offset inside the block is covered by the claim already on disk" + ); + + assert!(partition.reserve_offsets_through(17).await); + assert_eq!( + store.attempts.get(), + 2, + "the first offset past the block extends it" + ); + assert_eq!(last_recorded_reservation(&store), 34); + } + + /// A restored counter above the chain must move the chain, not just the + /// counter. Left as one segment named at 0, the next mint lands INSIDE it -- + /// a shape production's boot never produces, so the harness would model + /// something the server cannot reach and could not expose the chain refusal + /// the boot after would hit. + #[test] + fn given_a_restored_counter_above_an_empty_chain_when_reanchoring_should_plant_at_the_mint() { + let mut partition = solo_recording_partition(); + let recovered = recorded_state(0, 65_537); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!(partition.mint_frontier(), 65_537); + assert_eq!( + partition.log.segments().len(), + 1, + "the premise: one empty segment named at 0, as the rebuild leaves it" + ); + + partition.reanchor_in_memory_to_mint_frontier(IggyByteSize::from(1024 * 1024)); + + let starts: Vec = partition + .log + .segments() + .iter() + .map(|segment| segment.start_offset) + .collect(); + assert_eq!( + starts, + vec![65_537], + "the empty segment claiming 0.. is retired and one planted at the \ + append point, exactly as boot's emptied-chain arm does" + ); + } + + /// A SIZED tail is the only copy of its messages, so it is sealed and the + /// plant goes past it, leaving the gap the chain guard admits by anchor. + #[test] + fn given_a_restored_counter_above_a_sized_tail_when_reanchoring_should_seal_and_plant_past_it() + { + let mut partition = solo_recording_partition(); + { + let tail = partition.log.active_segment_mut(); + tail.size = IggyByteSize::from(4_096); + tail.end_offset = 9; + } + partition.note_committed_seeded(); + partition.offset.store(9, Ordering::Release); + partition.dirty_offset.store(9, Ordering::Relaxed); + let recovered = recorded_state(10, 65_547); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!(partition.mint_frontier(), 65_547); + + partition.reanchor_in_memory_to_mint_frontier(IggyByteSize::from(1024 * 1024)); + + let segments = partition.log.segments(); + assert_eq!(segments.len(), 2, "the sized tail is kept, not retired"); + assert!(segments[0].sealed, "and sealed before the plant lands"); + assert_eq!(segments[0].end_offset, 9); + assert_eq!( + segments[1].start_offset, 65_547, + "the plant names the append point, so the next mint starts a segment \ + rather than landing inside one" + ); + } + + /// A tail already named AT the append point takes the appends as it stands. + /// Planting beside it would leave two segments claiming the same start + /// offset, which no chain guard admits. + #[test] + fn given_a_chain_already_anchored_at_the_mint_when_reanchoring_should_leave_it_alone() { + let segment_size = IggyByteSize::from(1024 * 1024); + let mut partition = solo_recording_partition(); + { + let tail = partition.log.active_segment_mut(); + tail.size = IggyByteSize::from(4_096); + tail.end_offset = 9; + } + // The shape a clean boot leaves: the flushed tail, then an empty segment + // already named for the append point. + partition.log.add_persisted_segment( + crate::Segment::new(10, segment_size), + server_common::SegmentStorage::default(), + None, + None, + ); + partition.note_committed_seeded(); + partition.offset.store(9, Ordering::Release); + partition.dirty_offset.store(9, Ordering::Relaxed); + assert_eq!(partition.mint_frontier(), 10); + + partition.reanchor_in_memory_to_mint_frontier(segment_size); + + let starts: Vec = partition + .log + .segments() + .iter() + .map(|segment| segment.start_offset) + .collect(); + assert_eq!( + starts, + vec![0, 10], + "the empty tail is named at the append point, so it is neither retired \ + nor planted beside" + ); + assert!( + !partition.log.segments()[0].sealed, + "and nothing was sealed, since no plant needed a gap" + ); + } + + /// The record is an EXCLUSIVE frontier, so `u64::MAX` can never be covered: + /// covering it would need `u64::MAX + 1`. Saturating and reporting success + /// there confirms an offset to a client that the next boot re-mints, which is + /// the exact defect this path exists to prevent, at the one offset where it + /// would be silent. + #[compio::test] + async fn given_an_exhausted_offset_space_when_fencing_should_refuse_rather_than_confirm() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + + assert!( + partition.reserve_offsets_through(u64::MAX - 1).await, + "the last representable offset is still reservable" + ); + assert_eq!( + last_recorded_reservation(&store), + u64::MAX, + "a claim clamped to the top of the space still sits strictly above \ + the offset it covers" + ); + + let attempts = store.attempts.get(); + assert!( + !partition.reserve_offsets_through(u64::MAX).await, + "the terminal offset must be refused, not confirmed" + ); + assert_eq!( + store.attempts.get(), + attempts, + "and refused without attempting a write it could not make correct" + ); + } + + /// The boot after that refusal: the counter resumes AT the terminal offset + /// and the fence keeps refusing it, so nothing a client holds is reissued. + #[compio::test] + async fn given_a_saturated_reservation_when_restored_should_keep_refusing_the_terminal_offset() + { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + let recovered = recorded_state(0, u64::MAX); + partition.set_superblock(store.clone(), Some(&recovered)); + partition.set_offset_reservation_lease(test_lease(16)); + partition.restore_offset_frontier(Some(&recovered)); + + assert_eq!( + partition.mint_frontier(), + u64::MAX, + "the append point resumes above every offset the reservation covered" + ); + assert!( + !partition.reserve_offsets_through(u64::MAX).await, + "the one offset the record never covered must stay unmintable" + ); + } + + /// The tick claims a lease past the CEILING. Extending past the append point + /// instead buys back only the headroom the trigger had left -- about half a + /// lease -- and doubles the write rate the default lease is sized for. + #[compio::test] + async fn given_a_tick_extension_when_it_writes_should_advance_the_ceiling_a_full_lease() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + + assert!(partition.reserve_offsets_through(0).await); + assert_eq!(last_recorded_reservation(&store), 17); + partition.note_append_live(); + + // Half the block consumed, which is where the trigger fires. + partition.dirty_offset.store(11, Ordering::Relaxed); + assert!(partition.needs_offset_reservation_extension()); + assert!(partition.extend_offset_reservation().await); + assert_eq!( + last_recorded_reservation(&store), + 33, + "a full lease past the ceiling of 17, not past the append point of 12" + ); + + // And the trigger is genuinely satisfied for a full block of appends, + // rather than re-firing after another half. + for offset in 12..=24 { + partition.dirty_offset.store(offset, Ordering::Relaxed); + assert!( + !partition.needs_offset_reservation_extension(), + "offset {offset} still sits a full half-lease under the new ceiling" + ); + } + } + + /// The first send to an untouched partition is BOUNCED so the tick claims the + /// block, rather than awaiting a create, write, fsync, rename and directory + /// fsync inside the shard's request pump. + #[compio::test] + async fn given_an_untouched_partition_when_a_send_arrives_should_bounce_it_to_the_tick() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + + assert!( + partition.should_defer_first_reservation(0), + "a first send with no claim on disk must not pay for one inline" + ); + assert!( + !partition.should_defer_first_reservation(16), + "a first batch wider than the whole lease must not be bounced: the \ + tick's claim would not cover it either, so it would bounce forever" + ); + + // Arming is what the bounce does; the tick then writes, off this path. + partition.offset_reservation_wanted.set(true); + assert!( + partition.needs_offset_reservation_extension(), + "an armed partition needs the write even though it has never minted" + ); + assert!(partition.extend_offset_reservation().await); + assert_eq!(store.attempts.get(), 1); + assert!( + !partition.needs_offset_reservation_extension(), + "the write it asked for disarms it" + ); + + // The retry finds the block already claimed and writes nothing. + assert!(!partition.should_defer_first_reservation(0)); + assert!(partition.reserve_offsets_through(0).await); + assert_eq!( + store.attempts.get(), + 1, + "the bounced send's retry takes the fence's fast path" + ); + } + + /// A storeless partition reserves nothing, and the tick skips it, so a bounce + /// there is a send denied for the life of the partition with nothing able to + /// clear it. The two gates have to agree on which partitions the tick serves. + #[test] + fn given_a_storeless_partition_when_a_send_arrives_should_not_bounce_it() { + let mut partition = solo_recording_partition(); + partition.set_offset_reservation_lease(test_lease(16)); + assert!(partition.superblock.is_none(), "the premise: no store"); + assert!(!partition.offset_space.append_live); + + assert!( + !partition.should_defer_first_reservation(0), + "nothing would ever claim the block this bounce waits for" + ); + assert!( + !partition.needs_offset_reservation_extension(), + "and the tick agrees it has nothing to do here" + ); + } + + /// Idle partitions stay idle: arming is what separates a partition someone + /// produced to from one boot merely materialized, and without that a node + /// with many partitions writes a superblock per partition for nothing. + #[test] + fn given_an_unarmed_untouched_partition_when_ticking_should_still_not_extend() { + let mut partition = solo_recording_partition(); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), None); + assert!(!partition.offset_space.append_live); + assert!(!partition.offset_reservation_wanted.get()); + assert!(!partition.needs_offset_reservation_extension()); + } + + /// Inside an open backoff window the ADMITTED path refuses without touching + /// the disk the last writer just found broken. Every producer retry otherwise + /// re-runs a full atomic replace, which starves the shard pump for as long as + /// the fault lasts. + #[compio::test] + async fn given_an_open_backoff_window_when_preflighting_a_send_should_refuse_without_writing() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + partition + .superblock_retry_after_micros + .set(partition.consensus().clock_realtime_micros() + 1_000_000); + + assert!( + !partition.reserve_offsets_through_retryable(0).await, + "a claim that would need a write is refused inside the window" + ); + assert_eq!( + store.attempts.get(), + 0, + "and refused without any I/O at all" + ); + + // The fence at the MINT keeps its bypass: a refusal there fences the + // partition and takes the node down, so trying the write is strictly + // better than declining to. + assert!(partition.reserve_offsets_through(0).await); + assert_eq!(store.attempts.get(), 1); + + // A batch the record already covers owes the disk nothing, so the open + // window must not refuse it either. + assert!( + partition.reserve_offsets_through_retryable(0).await, + "the coverage fast path wins over the backoff window" + ); + assert_eq!(store.attempts.get(), 1); + } + + /// A journaled offset is not a committed one. Seeding the committed bit at + /// the append would serve a resident offset 0 to a consumer before the first + /// commit and let the frontier persist name data no quorum agreed on. + #[test] + fn given_a_journaled_offset_when_uncommitted_should_not_seed_the_committed_counter() { + let mut partition = solo_recording_partition(); + + partition.note_append_live(); + partition.dirty_offset.store(0, Ordering::Relaxed); + assert!(partition.offset_space.append_live); + assert!( + !partition.offset_space.committed_seeded, + "the append moves the append counter alone" + ); + assert_eq!(partition.mint_frontier(), 1, "the next mint continues it"); + assert_eq!( + partition.offset_frontier(), + 0, + "and the frontier a persist would record still names no data" + ); + + partition.note_committed_seeded(); + assert_eq!( + partition.offset_frontier(), + 1, + "commit is what publishes the offset" + ); + } + + /// Fail-closed: offsets the record does not cover would be confirmed to a + /// client with nothing durable saying they were handed out. + /// The fence ahead of the pipeline must bound a mint the ordinary send path + /// actually takes. `convert_request_message` runs at `ChecksumMode::Skip`, so + /// a verifying decode of its output fails and the ceiling would come back + /// `None`, dropping every solo send back to the fence at the mint, where a + /// refusal fences the partition and exits the node. + #[test] + fn given_a_checksumless_send_when_bounding_the_mint_should_read_the_batch_header() { + let partition = solo_recording_partition(); + let namespace = IggyNamespace::from_raw(partition.consensus().group()); + let message = checksumless_send_request(namespace, 3); + let body = &message.as_slice() + [std::mem::size_of::()..message.header().size as usize]; + + assert!( + decode_batch_slice(body).is_err(), + "the premise: a Skip-converted body does not survive a verifying decode" + ); + assert_eq!( + partition.request_mint_ceiling(&message), + Some(2), + "the ceiling is the batch's LAST offset: three messages from a frontier \ + of 0 mint 0, 1 and 2, and the claim adds the exclusive successor itself" + ); + } + + /// Nothing is reserved above one replica, so the ceiling is not computed + /// there either. + #[test] + fn given_a_replicated_group_when_bounding_the_mint_should_not_compute_a_ceiling() { + let partition = partition_at_view(1, 1); + assert!(partition.consensus().replica_count() > 1); + let namespace = IggyNamespace::from_raw(partition.consensus().group()); + assert_eq!( + partition.request_mint_ceiling(&checksumless_send_request(namespace, 3)), + None + ); + } + + #[compio::test] + async fn given_failing_superblock_when_fencing_should_refuse() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(4)); + store.fail_writes.set(true); + + assert!( + !partition.reserve_offsets_through(0).await, + "an unrecordable claim must refuse the append" + ); + } + + /// The point of extending from the tick: after it runs, the append path finds + /// the ceiling already covering it and writes nothing. Without this the two + /// fsyncs land in front of a produce, inside the frame pump the consensus + /// tick shares. + #[compio::test] + async fn given_a_consumed_block_when_the_tick_extends_should_leave_the_append_path_writeless() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + + // First append pays for the first claim, as it must: nothing durable yet. + assert!(partition.reserve_offsets_through(0).await); + assert_eq!(store.attempts.get(), 1); + partition.set_offset_space_used(true); + + // Inside the block there is nothing to do, from either caller. + partition.dirty_offset.store(4, Ordering::Relaxed); + assert!( + !partition.needs_offset_reservation_extension(), + "a full block of headroom needs no extension" + ); + + // Past half the block the tick takes the write. + partition.dirty_offset.store(11, Ordering::Relaxed); + assert!( + partition.needs_offset_reservation_extension(), + "under half a block of headroom the tick must extend" + ); + assert!(partition.extend_offset_reservation().await); + assert_eq!( + store.attempts.get(), + 2, + "the tick wrote, not the append path" + ); + + // And now the append path is writeless across the rest of the old block. + let before = store.attempts.get(); + for offset in 12..=16 { + assert!(partition.reserve_offsets_through(offset).await); + } + assert_eq!( + store.attempts.get(), + before, + "every append after the extension must take the fence's fast path" + ); + } + + /// The extension must not fire for a partition that has never minted, or boot + /// would write a superblock per idle partition for nothing. + #[test] + fn given_an_untouched_partition_when_ticking_should_not_extend_the_reservation() { + let mut partition = solo_recording_partition(); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), None); + assert!(!partition.offset_space.append_live); + assert!(!partition.needs_offset_reservation_extension()); + } + + /// A boot that consumed a reservation has ZERO headroom -- the append point + /// sits exactly at the ceiling -- so the extension has to fire before the + /// first produce rather than after it. + #[test] + fn given_a_reservation_seeded_boot_when_ticking_should_extend_before_the_first_produce() { + let mut partition = solo_recording_partition(); + let recovered = recorded_state(0, 65_537); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!(partition.mint_frontier(), 65_537); + assert!( + partition.needs_offset_reservation_extension(), + "the seeded append point is at the ceiling, so the next append would \ + otherwise pay for the write" + ); + } + + /// A replicated group pays nothing for a protection it cannot use: nothing + /// seeds its counter from the reservation, its chain never gets re-anchored, + /// and an ack there means a quorum journaled the batch. + #[compio::test] + async fn given_a_replicated_group_when_fencing_should_not_write_at_all() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = partition_at_view(1, 1); + assert!(partition.consensus().replica_count() > 1); + partition.set_superblock(store.clone(), None); + partition.set_offset_reservation_lease(test_lease(16)); + store.fail_writes.set(true); + + assert!( + partition.reserve_offsets_through(1_000).await, + "the fence is inert above one replica, so not even a failing store can \ + refuse the append" + ); + assert_eq!(store.attempts.get(), 0, "and it attempts no write"); + } + + /// The reservation seeds the APPEND counter and only on a solo group. The + /// whole fix: after a crash below the flush thresholds it is the only witness + /// that those offsets were confirmed. + #[test] + fn given_a_recorded_reservation_when_solo_should_seed_the_append_counter() { + let mut partition = solo_recording_partition(); + assert_eq!( + partition.consensus().replica_count(), + 1, + "the reservation seed is scoped to solo groups" + ); + let store = Rc::new(RecordingSuperblock::default()); + + partition.set_superblock(store.clone(), None); + partition.restore_offset_frontier(None); + assert_eq!( + partition.mint_frontier(), + 0, + "nothing recorded, nothing seeded" + ); + + // The shape a crash before the first flush leaves: the fence recorded a + // block, and no message ever reached a segment, so the frontier is 0. + let recovered = recorded_state(0, 65_537); + partition.set_superblock(store, Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!( + partition.mint_frontier(), + 65_537, + "the first mint must land above every offset the reservation covered" + ); + assert_eq!( + partition.offset_frontier(), + 0, + "the committed frontier must not inherit the reservation, nor the \ + append counter's own liveness: nothing was flushed, so the partition \ + holds no offset at all" + ); + } + + /// A replicated group must not take the jump: a backup rejects any prepare + /// whose `base_offset` does not continue its own counter, so an append point a + /// lease block above the group has every peer refuse the batch. + #[test] + fn given_a_recorded_reservation_when_replicated_should_not_seed_the_append_counter() { + let mut partition = partition_at_view(1, 1); + assert!(partition.consensus().replica_count() > 1); + let recovered = recorded_state(0, 70_000); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!( + partition.mint_frontier(), + 0, + "a replicated group's offsets are the group's to decide" + ); + } + + /// The committed frontier still seeds both counters: it is a claim about data + /// every replica shares, so it is not gated on the replica count. + #[test] + fn given_a_recorded_frontier_when_replicated_should_seed_both_counters() { + let mut partition = partition_at_view(1, 1); + assert!(partition.consensus().replica_count() > 1); + let recovered = recorded_state(40, 70_000); + partition.set_superblock(Rc::new(RecordingSuperblock::default()), Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + assert_eq!(partition.offset_frontier(), 40); + assert_eq!( + partition.mint_frontier(), + 40, + "the reservation is ignored here, so the append point is the frontier" + ); + } + + /// The rewind fence asks whether an offer would destroy something this replica + /// COMMITTED. An append point standing a lease block above that -- what a + /// reservation-seeded boot leaves until the first append -- claims no + /// messages, so it must not turn a legitimate offer inside the block into a + /// refusal the replica would then cycle on forever. + #[compio::test] + async fn given_an_append_point_above_every_message_when_an_offer_arrives_should_not_call_it_a_rewind() + { + let partition_dir = transfer_fence_dir("append-point-is-not-data").await; + let mut partition = test_partition(); + partition.set_partition_dir(partition_dir.clone()); + partition.set_offset_space_used(true); + partition.dirty_offset.store(69_999, Ordering::Relaxed); + assert_eq!(partition.mint_frontier(), 70_000); + assert_eq!( + partition.offset_frontier(), + 1, + "nothing committed: the append point speaks for no messages" + ); + + let offer = crate::state_transfer::ConsumerOffsetsWire { + purge_generation: 0, + next_offset: 1_030, + consumers: Vec::new(), + groups: Vec::new(), + dedup: Vec::new(), + }; + let outcome = partition + .install_state_transfer(&repair_config(), 12, Vec::new(), &offer.encode(), 0) + .await; + assert!( + !matches!( + outcome, + Err(crate::state_transfer::PartitionInstallError::OfferRewindsDurableData { .. }) + ), + "the offer destroys nothing this replica holds, so the fence must let it \ + through: got {outcome:?}" + ); + + let _ = std::fs::remove_dir_all(&partition_dir); + } + + /// After a clean shutdown the segments account for every confirmed offset, + /// so collapsing the reservation keeps the offset space dense across an + /// ordinary restart instead of jumping a lease block every time. + #[compio::test] + async fn given_a_flushed_partition_when_collapsing_should_drop_the_reservation_to_the_frontier() + { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), Some(&recorded_state(0, 65_537))); + // A flushed partition: the append point and the committed head agree. + partition.set_offset_space_used(true); + partition.offset.store(24, Ordering::Release); + partition.dirty_offset.store(24, Ordering::Relaxed); + + assert!(partition.collapse_offset_reservation().await); + assert_eq!(last_recorded_frontier(&store), 25); + assert_eq!( + last_recorded_reservation(&store), + 25, + "a clean stop leaves no claim above what the segments prove" + ); + + // And the restart that follows mints where it left off, not a block up. + let recovered = recorded_state(25, 25); + let mut restarted = solo_recording_partition(); + restarted.set_superblock(store.clone(), Some(&recovered)); + restarted.restore_offset_frontier(Some(&recovered)); + assert_eq!( + restarted.mint_frontier(), + 25, + "the collapsed record puts the append point back at the frontier" + ); + } + + /// The graceful stop must not undo the crash protection. A boot that read a + /// reservation back and then took no traffic has a committed frontier of 0 + /// while the reservation is the only record that offsets were confirmed, so a + /// collapse reading the committed frontier would write it away -- and a clean + /// stop is the runbook answer to an incident, which would make it the one + /// action that re-opens the defect. + #[compio::test] + async fn given_an_unspent_reservation_when_collapsing_should_leave_it_standing() { + let store = Rc::new(RecordingSuperblock::default()); + let recovered = recorded_state(0, 65_537); + let mut partition = solo_recording_partition(); + partition.set_superblock(store.clone(), Some(&recovered)); + partition.restore_offset_frontier(Some(&recovered)); + + assert!(partition.collapse_offset_reservation().await); + assert_eq!( + store.attempts.get(), + 0, + "the reservation already covers the append point, so there is nothing to \ + collapse and nothing to write" + ); + + // The restart after the clean stop still resumes above every offset the + // crashed incarnation confirmed. + let mut restarted = solo_recording_partition(); + restarted.set_superblock(store, Some(&recovered)); + restarted.restore_offset_frontier(Some(&recovered)); + assert_eq!(restarted.mint_frontier(), 65_537); + } + + /// An install is the one place the reservation may come down: left high, it + /// re-seeds the counter above the group and every replicated prepare fails + /// the `base_offset == dirty_offset + 1` check. + #[compio::test] + async fn given_install_frontier_when_recorded_should_set_the_reservation_down_to_it() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = partition_at_view(1, 1); + partition.set_superblock(store.clone(), Some(&recorded_state(0, 70_000))); + + assert!(partition.install_offset_frontier_at(1_030).await); + assert_eq!( + last_recorded_reservation(&store), + 1_030, + "the install's frontier replaces the stale reservation" + ); + assert_eq!(last_recorded_frontier(&store), 1_030); + } + + /// A purge resets the offset space to zero and the reservation goes with it: + /// a survivor would re-seed the counter into the space just erased. + #[compio::test] + async fn given_purge_reset_when_recorded_should_clear_the_reservation() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = partition_at_view(1, 1); + partition.set_superblock(store.clone(), Some(&recorded_state(500, 70_000))); + + assert!(partition.reset_offset_frontier_at(0).await); + assert_eq!(last_recorded_frontier(&store), 0); + assert_eq!( + last_recorded_reservation(&store), + 0, + "a reset that left the reservation behind would resurrect the old space" + ); + } + + /// The reservation can never sit below the frontier: "offsets under N exist" + /// is stronger than "offsets under N may have been handed out". + #[compio::test] + async fn given_reservation_below_the_frontier_when_written_should_clamp_it_up() { + let store = Rc::new(RecordingSuperblock::default()); + let mut partition = partition_at_view(1, 1); + partition.set_superblock(store.clone(), None); + partition.set_offset_space_used(true); + partition.offset.store(99, Ordering::Release); + + assert!(partition.persist_offset_frontier_at(100).await); + assert_eq!(last_recorded_frontier(&store), 100); + assert_eq!( + last_recorded_reservation(&store), + 100, + "the reservation is clamped up to the frontier it accompanies" + ); + } + + /// The fence path persists the frontier while the live counter still sits + /// at its pre-install value, so an advance that maxes against the counter + /// alone erases the record and then quarantines the segments that were its + /// only other witness. Boot re-mints from 0 against a group at N after that. + #[compio::test] + async fn given_record_above_live_counter_when_advancing_should_keep_the_record() { + let mut partition = partition_at_view(1, 1); + let store = Rc::new(RecordingSuperblock::default()); + partition.set_superblock(store.clone(), None); + + assert!(partition.persist_offset_frontier_at(9_000).await); + assert_eq!(last_recorded_frontier(&store), 9_000); + assert_eq!( + partition.offset_frontier(), + 0, + "a partition that never minted reports a zero frontier, which is the \ + value the fence would otherwise persist" + ); + + assert!(partition.persist_offset_frontier().await); + + assert_eq!( + last_recorded_frontier(&store), + 9_000, + "the advance direction must not lower the durable frontier" + ); + } + + /// Attaching a store seeds the last-written frontier from the record + /// itself, so an advance maxes against what boot read off disk even before + /// this replica has written anything. The sibling test reaches that state by + /// WRITING first, which cannot catch an attach site that skips the seed. + #[compio::test] + async fn given_attached_record_when_advancing_should_keep_the_recorded_frontier() { + let mut partition = partition_at_view(1, 1); + let store = Rc::new(RecordingSuperblock::default()); + let recovered = consensus::VsrState { + cluster: TEST_CLUSTER, + replica_id: 0, + replica_count: 3, + view: 1, + log_view: 1, + commit_max: 0, + checkpoint_op: 0, + checkpoint_checksum: 0, + offset_frontier: 4_200, + offset_reserved: 0, + }; + partition.set_superblock(store.clone(), Some(&recovered)); + assert_eq!(partition.offset_frontier(), 0, "nothing minted locally"); + + assert!(partition.persist_offset_frontier().await); + + assert_eq!( + last_recorded_frontier(&store), + 4_200, + "the first write after an attach must not lower the record it was attached to" + ); + } + + /// The reset direction is the only way down, and it must actually go there: + /// an install under an advancing purge generation records a frontier below + /// the live counter on purpose. + #[compio::test] + async fn given_reset_below_live_counter_when_written_should_lower_the_record() { + let mut partition = partition_at_view(1, 1); + let store = Rc::new(RecordingSuperblock::default()); + partition.set_superblock(store.clone(), None); + partition.offset.store(9_000, Ordering::Release); + partition.set_offset_space_used(true); + + assert!(partition.persist_offset_frontier().await); + assert_eq!(last_recorded_frontier(&store), 9_001); + + assert!(partition.reset_offset_frontier_at(12).await); assert_eq!( last_recorded_frontier(&store), @@ -5609,7 +7484,7 @@ mod tests { let store = Rc::new(RecordingSuperblock::default()); partition.set_superblock(store.clone(), None); partition.offset.store(9_000, Ordering::Release); - partition.should_increment_offset = true; + partition.set_offset_space_used(true); store.fail_writes.set(true); assert!( @@ -5820,6 +7695,44 @@ mod tests { (partition, sent_to_clients) } + /// A `SendMessages` request in the shape `convert_request_message` leaves at + /// [`ChecksumMode::Skip`]: canonical batch, `batch_checksum` zeroed. + fn checksumless_send_request( + namespace: IggyNamespace, + message_count: u32, + ) -> Message { + let mut batch = IggyMessages::with_capacity(message_count as usize); + for _ in 0..message_count { + batch.push(IggyMessage { + header: IggyMessageHeader { + payload_length: 8, + ..Default::default() + }, + payload: Bytes::from_static(b"abcdefgh"), + user_headers: None, + }); + } + let mut owned = + SendMessagesOwned::from_messages(namespace, &batch).expect("build send_messages batch"); + owned.header.batch_checksum = 0; + + let header_size = std::mem::size_of::(); + let total = header_size + COMMAND_HEADER_SIZE + owned.blob.len(); + let mut message = Message::::new(total); + let body = &mut message.as_mut_slice()[header_size..]; + owned.header.encode_into(&mut body[..COMMAND_HEADER_SIZE]); + body[COMMAND_HEADER_SIZE..].copy_from_slice(&owned.blob); + message.transmute_header(|_, header: &mut RoutedRequestHeader| { + header.command = Command::Request; + header.operation = Operation::SendMessages; + header.client = 1; + header.session = 1; + header.request = 1; + header.group = namespace.inner(); + header.size = u32::try_from(total).expect("request size fits u32"); + }) + } + fn delete_offset_request( client_id: u128, request_id: u64, @@ -7422,8 +9335,15 @@ mod tests { let partition_dir = transfer_fence_dir("rewind-refused").await; let mut partition = test_partition(); partition.set_partition_dir(partition_dir.clone()); - partition.should_increment_offset = true; + partition.set_offset_space_used(true); partition.offset.store(99, Ordering::Release); + // Committed and resident: the threshold-gated flush leaves exactly this + // shape, and the fence has to count it as data. + { + let info = &mut partition.log.journal_mut().info; + info.messages_count = 100; + info.current_offset = 99; + } let behind = crate::state_transfer::ConsumerOffsetsWire { purge_generation: 0, @@ -7474,6 +9394,53 @@ mod tests { let _ = std::fs::remove_dir_all(&partition_dir); } + /// A chain installed EMPTY at frontier N holds no sized segment and no + /// journal entry, so a fence reading only held bytes reads 0 and skips + /// itself, letting a stale offer rewind the counter under offsets this + /// replica already claimed. The committed frontier is what carries N. + #[compio::test] + async fn given_an_empty_chain_installed_at_a_frontier_when_a_stale_offer_arrives_should_refuse() + { + let partition_dir = transfer_fence_dir("empty-install-rewind").await; + let mut partition = test_partition(); + partition.set_partition_dir(partition_dir.clone()); + // What an install of an all-GC'd origin leaves: the counter at the group + // frontier, nothing on disk, nothing resident. + partition.set_offset_space_used(true); + partition.offset.store(4_095, Ordering::Release); + partition.dirty_offset.store(4_095, Ordering::Relaxed); + assert_eq!( + partition.held_offset_frontier(), + 4_096, + "the committed arm has to carry a frontier no byte on disk names" + ); + + let stale = crate::state_transfer::ConsumerOffsetsWire { + purge_generation: 0, + next_offset: 1_000, + consumers: Vec::new(), + groups: Vec::new(), + dedup: Vec::new(), + }; + let refused = partition + .install_state_transfer(&repair_config(), 12, Vec::new(), &stale.encode(), 0) + .await; + assert!( + matches!( + refused, + Err( + crate::state_transfer::PartitionInstallError::OfferRewindsDurableData { + offer_next_offset: 1_000, + local_next_offset: 4_096, + } + ) + ), + "expected a rewind refusal, got {refused:?}" + ); + + let _ = std::fs::remove_dir_all(&partition_dir); + } + /// The canonical post-restart rejoin: this replica applied a purge before /// the restart but was killed before the purge's `purge.gen` record step, /// so the metadata plane's COMMITTED generation is 1 while its own @@ -7485,8 +9452,15 @@ mod tests { let partition_dir = transfer_fence_dir("restart-purge-rewind").await; let mut partition = test_partition(); partition.set_partition_dir(partition_dir.clone()); - partition.should_increment_offset = true; + partition.set_offset_space_used(true); partition.offset.store(99, Ordering::Release); + // Committed and resident: the threshold-gated flush leaves exactly this + // shape, and the fence has to count it as data. + { + let info = &mut partition.log.journal_mut().info; + info.messages_count = 100; + info.current_offset = 99; + } assert_eq!( partition.applied_purge_generation(), 0, @@ -7535,7 +9509,7 @@ mod tests { let partition_dir = transfer_fence_dir("missed-purge-reset").await; let mut partition = test_partition(); partition.set_partition_dir(partition_dir.clone()); - partition.should_increment_offset = true; + partition.set_offset_space_used(true); partition.offset.store(99, Ordering::Release); assert_eq!( partition.applied_purge_generation(), diff --git a/core/partitions/src/lib.rs b/core/partitions/src/lib.rs index 818872029d..013bc6de6a 100644 --- a/core/partitions/src/lib.rs +++ b/core/partitions/src/lib.rs @@ -28,6 +28,7 @@ mod messages_writer; pub mod offset_storage; mod poll_plan; mod segment; +pub mod segment_anchor; pub mod state_transfer; mod types; @@ -39,6 +40,20 @@ pub use iggy_index_writer::IggyIndexWriter; pub use iggy_partition::{IggyPartition, PurgeError, SegmentRemoval}; pub use iggy_partitions::IggyPartitions; pub use journal::{EVICTED_RING_BYTES_MAX, EVICTED_RING_CAPACITY}; + +/// Offsets a partition claims in its superblock ahead of the mint counter +/// before it will append, so a crash-restarted replica resumes above every +/// offset it confirmed instead of re-minting it. +/// +/// One superblock write (two fsyncs) per block: at 100k messages/s a 1Ki block +/// costs ~200 fsyncs/s, 64Ki costs ~3/s. The waste is at most one block of a +/// `u64` space per crash, visible only as a segment boundary at boot. +/// +/// Lives HERE and not in `iggy_common`: it is a server-side write-path default +/// that no client ever reads, and the shared crate is the client-facing API. +/// Both consumers -- the fallback in [`IggyPartition`] and the `[partition]` +/// config default boot installs -- already depend on this crate. +pub const DEFAULT_OFFSET_RESERVATION_LEASE: u32 = 64 * 1024; pub use messages_writer::MessagesWriter; pub use offset_storage::delete_persisted_offset; pub use poll_plan::{AutoCommitApplied, PollPlan}; diff --git a/core/partitions/src/log.rs b/core/partitions/src/log.rs index 92d9c1768d..64ae2f5209 100644 --- a/core/partitions/src/log.rs +++ b/core/partitions/src/log.rs @@ -192,8 +192,9 @@ where } /// Mutable segment views. Length mutation lives in - /// [`Self::add_persisted_segment`] / [`Self::retire_front`] only, so the - /// parallel vecs cannot desync from the outside. + /// [`Self::add_persisted_segment`] / [`Self::retire_front`] / + /// [`Self::retire_back`] only, so the parallel vecs cannot desync from the + /// outside. pub fn segments_mut(&mut self) -> &mut [Segment] { &mut self.segments } @@ -314,6 +315,28 @@ where Some((segment, storage)) } + /// Retire the NEWEST segment, the mirror of [`Self::retire_front`]. + /// + /// Boot re-anchor only, and only for an EMPTY tail named below the re-seeded + /// counter, which would otherwise claim a range it does not hold. Nothing + /// else may take from the back: a sized tail is the only copy of its + /// messages. + pub fn retire_back(&mut self) -> Option<(Segment, SegmentStorage)> { + if self.segments.is_empty() { + return None; + } + self.debug_assert_lockstep(); + let segment = self.segments.pop()?; + let storage = self.storage.pop()?; + self.indexes.pop(); + self.messages_writers.pop(); + self.index_writers.pop(); + self.sealed_read_state.pop(); + self.sealed_lru + .retain(|&offset| offset != segment.start_offset); + Some((segment, storage)) + } + fn debug_assert_lockstep(&self) { debug_assert!( self.segments.len() == self.storage.len() diff --git a/core/partitions/src/segment_anchor.rs b/core/partitions/src/segment_anchor.rs new file mode 100644 index 0000000000..1891ce5fe7 --- /dev/null +++ b/core/partitions/src/segment_anchor.rs @@ -0,0 +1,329 @@ +// 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 record that makes a gap in the segment chain legitimate. +//! +//! Recovery derives each segment's bounds from its own bytes, so a gap has no +//! author: the boot re-anchor's planted gap and a lost segment look identical. +//! The re-anchor writes its intent down instead, and the chain guard admits a +//! forward gap only when the far side carries an anchor naming exactly the near +//! side. +//! +//! Written and directory-fsynced BEFORE that segment is created. A crash in the +//! window then leaves an anchor with no segment, which the boot sweep collects; +//! the other order leaves a planted segment with no anchor, which the guard +//! reads as damage on an intact chain. + +use crate::state_transfer::STAGING_SUFFIX; +use compio::io::AsyncWriteAtExt; +use consensus::state_artifact_checksum; +use std::io; + +/// File extension for an anchor record, `{start_offset:020}.anchor` beside the +/// `{start_offset:020}.log` it belongs to. +pub const ANCHOR_EXTENSION: &str = "anchor"; + +/// [`ANCHOR_EXTENSION`] as a filename suffix, for the directory sweeps that +/// match on one. +pub const ANCHOR_SUFFIX: &str = ".anchor"; + +/// Leading bytes of an anchor record, so a file that is not one (a truncated +/// write, an operator's copy) is refused rather than decoded. +const ANCHOR_MAGIC: u64 = u64::from_le_bytes(*b"IGGYANCH"); + +/// `magic`(8) + `planted_start`(8) + `sealed_start`(8) + `sealed_end`(8) + +/// `checksum`(8). +pub const ANCHOR_ENCODED_LEN: usize = 40; + +/// The gap one planted segment is allowed to leave behind it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SegmentAnchor { + /// Start offset of the segment this record sits beside, i.e. the FAR side of + /// the gap. + /// + /// Redundant with the file name and deliberately so: the name is not + /// checksummed, so without this field the payload authenticates the + /// predecessor bounds while saying nothing about which plant it authorises. + /// Valid anchor bytes copied beside a later segment would then cover a wider + /// gap after the same predecessor -- exactly the operator's copy this module + /// claims to refuse. + pub planted_start: u64, + /// Start offset of the segment that was sealed, i.e. the near side of the + /// gap. Names WHICH segment, so an anchor cannot be satisfied by a + /// different file that happens to end where this one expects. + pub sealed_start: u64, + /// End offset the sealed segment held when it was sealed. The gap runs from + /// here to the planted segment's own start offset. + pub sealed_end: u64, +} + +impl SegmentAnchor { + /// Encode to the fixed little-endian on-disk layout. + #[must_use] + pub fn to_bytes(&self) -> [u8; ANCHOR_ENCODED_LEN] { + let mut out = [0u8; ANCHOR_ENCODED_LEN]; + out[0..8].copy_from_slice(&ANCHOR_MAGIC.to_le_bytes()); + out[8..16].copy_from_slice(&self.planted_start.to_le_bytes()); + out[16..24].copy_from_slice(&self.sealed_start.to_le_bytes()); + out[24..32].copy_from_slice(&self.sealed_end.to_le_bytes()); + let checksum = state_artifact_checksum(&out[0..32]); + out[32..40].copy_from_slice(&checksum.to_le_bytes()); + out + } + + /// Decode a record, returning `None` for anything this build did not write: + /// a wrong length, a wrong magic, or a checksum that does not match. + /// + /// A `None` is never treated as "no gap was intended". It means the record + /// proves nothing, so the gap it would have covered stays damage. + #[must_use] + pub fn from_bytes(bytes: &[u8]) -> Option { + if bytes.len() != ANCHOR_ENCODED_LEN { + return None; + } + let field = |at: usize| -> u64 { + let mut raw = [0u8; 8]; + raw.copy_from_slice(&bytes[at..at + 8]); + u64::from_le_bytes(raw) + }; + if field(0) != ANCHOR_MAGIC || field(32) != state_artifact_checksum(&bytes[0..32]) { + return None; + } + Some(Self { + planted_start: field(8), + sealed_start: field(16), + sealed_end: field(24), + }) + } + + /// Whether this anchor legitimises the gap between the segment starting at + /// `sealed_start` / ending at `sealed_end` and the segment planted at + /// `planted_start`. + /// + /// All THREE bounds must match. The predecessor pair alone would let an + /// anchor left by an earlier incarnation of the chain cover a gap it never + /// saw; the plant alone would let any predecessor satisfy it. Binding the + /// plant is what stops valid bytes from being copied beside a later segment + /// to authorise a wider gap after the same predecessor. + #[must_use] + pub const fn covers(&self, planted_start: u64, sealed_start: u64, sealed_end: u64) -> bool { + self.planted_start == planted_start + && self.sealed_start == sealed_start + && self.sealed_end == sealed_end + } +} + +/// Path of the anchor record beside the segment starting at `start_offset`. +#[must_use] +pub fn anchor_path(partition_dir: &str, start_offset: u64) -> String { + format!("{partition_dir}/{start_offset:0>20}.{ANCHOR_EXTENSION}") +} + +/// Write the anchor for a segment about to be planted at `start_offset`, then +/// fsync the directory so the record cannot arrive after the segment it +/// describes. +/// +/// # Errors +/// +/// Any I/O failure. The caller must NOT plant the segment: a planted segment +/// whose anchor is missing reads as damage on the next boot. +pub async fn write_anchor(partition_dir: &str, anchor: SegmentAnchor) -> io::Result<()> { + // Temp, fsync, rename, fsync the dir, like the superblock in this same + // directory. There is no second slot to fall back on, so a truncating + // in-place write would leave a torn record where the guard needs either the + // old one or the new one. + // + // The path comes from the record's own `planted_start` rather than a second + // parameter: the guard matches the two for equality, so a caller that could + // pass them separately could write a record that never satisfies anything. + let path = anchor_path(partition_dir, anchor.planted_start); + // `STAGING_SUFFIX` rather than a suffix of its own: every sweep already + // unlinks it unconditionally, boot included, so a torn write leaves nothing + // a later guard can read. + let tmp_path = format!("{path}{STAGING_SUFFIX}"); + let mut file = compio::fs::File::create(&tmp_path).await?; + let (result, _buf) = file + .write_all_at(anchor.to_bytes().to_vec(), 0) + .await + .into(); + result?; + file.sync_all().await?; + compio::fs::rename(&tmp_path, &path).await?; + crate::state_transfer::fsync_dir(partition_dir).await +} + +/// Read the anchor beside the segment starting at `start_offset`. +/// +/// `Ok(None)` when the file is absent, is not exactly [`ANCHOR_ENCODED_LEN`] +/// bytes, or does not decode; all of them mean the same thing to the guard, so +/// the caller needs no distinction. +/// +/// The length is checked from the metadata BEFORE any read, so a corrupt or +/// foreign file left at this path cannot size an allocation on the boot path. +/// `from_bytes` would refuse it either way, but only after reading all of it. +/// +/// # Errors +/// +/// Any other stat or read failure. NOT folded into `Ok(None)`: an `EACCES` or +/// `EIO` over a healthy planted chain would read as no gap intended, refusing +/// it as damage for as long as the fault lasts. +pub async fn read_anchor( + partition_dir: &str, + start_offset: u64, +) -> io::Result> { + let path = anchor_path(partition_dir, start_offset); + let length = match compio::fs::metadata(&path).await { + Ok(metadata) => metadata.len(), + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + }; + if length != ANCHOR_ENCODED_LEN as u64 { + return Ok(None); + } + match compio::fs::read(&path).await { + Ok(bytes) => Ok(SegmentAnchor::from_bytes(&bytes)), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(error), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn anchor() -> SegmentAnchor { + SegmentAnchor { + planted_start: 8_192, + sealed_start: 7, + sealed_end: 4_095, + } + } + + #[test] + fn given_an_anchor_when_round_tripped_should_decode_identically() { + let bytes = anchor().to_bytes(); + assert_eq!(bytes.len(), ANCHOR_ENCODED_LEN); + assert_eq!(SegmentAnchor::from_bytes(&bytes), Some(anchor())); + } + + #[test] + fn given_a_flipped_bit_when_decoded_should_refuse() { + // Every byte the checksum covers, so a corrupted record can never read as + // a legitimate gap. + for index in 0..32 { + let mut bytes = anchor().to_bytes(); + bytes[index] ^= 1; + assert_eq!( + SegmentAnchor::from_bytes(&bytes), + None, + "a record corrupted at byte {index} must not decode" + ); + } + } + + #[test] + fn given_a_wrong_length_when_decoded_should_refuse() { + let bytes = anchor().to_bytes(); + assert_eq!(SegmentAnchor::from_bytes(&bytes[..39]), None); + assert_eq!(SegmentAnchor::from_bytes(&[]), None); + } + + #[test] + fn given_an_anchor_when_matching_a_different_predecessor_should_not_cover_it() { + let anchor = SegmentAnchor { + planted_start: 30, + sealed_start: 10, + sealed_end: 20, + }; + assert!(anchor.covers(30, 10, 20)); + assert!(!anchor.covers(30, 10, 21), "a different end must not match"); + assert!( + !anchor.covers(30, 0, 20), + "the same end under a different segment must not match: an anchor left \ + by an earlier chain would otherwise cover a gap it never saw" + ); + } + + /// The copy this module claims to refuse: valid, checksum-clean bytes moved + /// beside a LATER segment. Without the plant in the payload the record still + /// authenticates, still names the same predecessor, and authorises a gap that + /// is now arbitrarily wide. + #[test] + fn given_valid_anchor_bytes_copied_beside_a_later_segment_should_not_cover_the_wider_gap() { + let planted = SegmentAnchor { + planted_start: 100, + sealed_start: 0, + sealed_end: 99, + }; + let decoded = SegmentAnchor::from_bytes(&planted.to_bytes()) + .expect("the copied bytes are checksum-clean, which is the premise"); + + assert!( + decoded.covers(100, 0, 99), + "beside its own segment it holds" + ); + assert!( + !decoded.covers(5_000, 0, 99), + "the same bytes beside a segment planted at 5000 must not authorise \ + the gap 100..5000 after the same predecessor" + ); + } + + /// A foreign or corrupt file at the anchor path must be refused from its + /// metadata, not by reading however many bytes it happens to hold. + #[compio::test] + async fn given_an_oversized_file_at_the_anchor_path_when_read_should_refuse_it() { + let dir = tempfile::tempdir().expect("tempdir"); + let partition_dir = dir.path().to_str().expect("utf-8 tempdir"); + let path = anchor_path(partition_dir, 42); + + std::fs::write(&path, vec![0u8; 1 << 20]).expect("plant an oversized file"); + assert_eq!( + read_anchor(partition_dir, 42) + .await + .expect("an oversized file is refused, not an error"), + None + ); + + std::fs::write(&path, anchor().to_bytes()).expect("plant a real record"); + assert_eq!( + read_anchor(partition_dir, 42) + .await + .expect("a well-formed record reads back"), + Some(anchor()) + ); + } + + /// The path is derived from the record, so a write always lands where the + /// guard will look for it. + #[compio::test] + async fn given_an_anchor_when_written_should_land_at_its_own_planted_start() { + let dir = tempfile::tempdir().expect("tempdir"); + let partition_dir = dir.path().to_str().expect("utf-8 tempdir"); + + write_anchor(partition_dir, anchor()) + .await + .expect("write the anchor"); + + assert_eq!( + read_anchor(partition_dir, anchor().planted_start) + .await + .expect("read it back"), + Some(anchor()) + ); + } +} diff --git a/core/partitions/src/state_transfer.rs b/core/partitions/src/state_transfer.rs index 7444b9d76b..1872d52165 100644 --- a/core/partitions/src/state_transfer.rs +++ b/core/partitions/src/state_transfer.rs @@ -32,6 +32,7 @@ use crate::offset_storage::{ PURGE_GENERATION_FILE, delete_persisted_offset, persist_offset, persist_purge_generation, }; use crate::segment::Segment; +use crate::segment_anchor::ANCHOR_SUFFIX; use crate::types::PartitionsConfig; use crate::{IggyIndexWriter, IggyPartition}; use compio::io::{AsyncReadAtExt, AsyncWriteAtExt}; @@ -1487,7 +1488,7 @@ pub async fn quarantine_segment_files(partition_dir: &str) -> std::io::Result std::io::Result committed_purge_generation || (self.applied_purge_generation < committed_purge_generation && offsets_wire.next_offset == 0); + // The COMMITTED frontier, which is what an offer is comparable against: + // `held_offset_frontier` reads 0 for a chain installed empty at frontier + // N (its disk arm filters empty segments and the install clears the + // journal), and a 0 skips the guard below entirely, letting a stale offer + // rewind the counter under data this replica already claimed. The append + // point is not usable either -- it can stand a lease block high -- but + // only on a solo group, which never receives an offer. let local_next_offset = self.offset_frontier(); if !purge_advances && local_next_offset > 0 && offsets_wire.next_offset < local_next_offset { @@ -2295,11 +2303,16 @@ where // zero `.log` files and re-seed the counter from the pre-purge frontier, // above a group that restarted at the offer's, and the next prepare // would stamp a `base_offset` and `batch_checksum` no peer shares. + // + // Identical to the advancing form on every group that can receive an + // offer today, since the reservation is solo-only and there equals the + // frontier. Spelled out because the shape is what makes it a reset, not + // the arithmetic that currently coincides. let frontier_durable = if purge_advances { self.reset_offset_frontier_at(offsets_wire.next_offset) .await } else { - self.persist_offset_frontier_at(offsets_wire.next_offset) + self.install_offset_frontier_at(offsets_wire.next_offset) .await }; if !frontier_durable { @@ -2411,11 +2424,19 @@ where // the NEWEST suffix, which is contiguous) and drop the in-memory // vectors in lockstep, exactly as `purge` does. let namespace_raw = self.consensus().group(); - while let Some((_, mut storage)) = self.log.retire_front() { + while let Some((segment, mut storage)) = self.log.retire_front() { let (messages_path, index_path) = storage.segment_and_index_paths(); let _ = storage.shutdown(); drop(storage); - for path in messages_path.into_iter().chain(index_path) { + // Anchors go with the chain they describe, as everywhere else. + // Unreachable for an install today, since anchors are planted only + // by the solo boot re-anchor, but a record outliving its segment is + // the one way a later gap gets admitted for free. + for path in messages_path + .into_iter() + .chain(index_path) + .chain(self.anchor_cleanup_path(segment.start_offset)) + { match compio::fs::remove_file(&path).await { Ok(()) => {} Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} @@ -2805,7 +2826,7 @@ where let end = next_offset.saturating_sub(1); self.offset.store(end, Ordering::Release); self.dirty_offset.store(end, Ordering::Relaxed); - self.should_increment_offset = next_offset > 0; + self.set_offset_space_used(next_offset > 0); self.recovered_durable_offset = installed_end; // Where the group's offset space starts on this replica: everything // below is represented by this install, so the repair floor check @@ -2959,7 +2980,7 @@ where .into_iter() .filter(|path| { path.to_str().is_some_and(|path| { - [".log", ".index", STAGING_SUFFIX] + [".log", ".index", STAGING_SUFFIX, ANCHOR_SUFFIX] .iter() .any(|extension| path.ends_with(extension)) }) @@ -3005,7 +3026,7 @@ where let end = minted_next_offset.saturating_sub(1); self.offset.store(end, Ordering::Release); self.dirty_offset.store(end, Ordering::Relaxed); - self.should_increment_offset = minted_next_offset > 0; + self.set_offset_space_used(minted_next_offset > 0); self.recovered_durable_offset = None; // The frontier claims "everything below me is represented here", and the // repair floor check accepts any floor at or below it. Nothing was diff --git a/core/server/config.toml b/core/server/config.toml index 5584b03788..3362651867 100644 --- a/core/server/config.toml +++ b/core/server/config.toml @@ -664,15 +664,25 @@ repair_retry_interval = "1s" # the queue and drops frames. Must be > 0 and <= 1024. repair_chunk_max = 128 -# How long the metadata superblock may stay unwritable before the replica -# fail-stops (duration). A replica that cannot persist its view is already -# fenced quorum-invisible and retries with capped backoff; past this window the -# process exits with a distinct status so a supervisor restarts or replaces it -# instead of an operator finding the wedge in logs. "0" disables the fail-stop -# and leaves the replica fenced indefinitely. Nonzero values must be at least -# 30s so a transient disk hiccup cannot kill the process. +# How long a superblock may stay unwritable before the replica fail-stops +# (duration). Applies to the metadata superblock and to each partition's own. A +# group that cannot persist its view is already fenced quorum-invisible and +# retries with capped backoff; past this window the process exits with a distinct +# status so a supervisor restarts or replaces it instead of an operator finding +# the wedge in logs. "0" disables the fail-stop and leaves the group fenced +# indefinitely. Nonzero values must be at least 30s so a transient disk hiccup +# cannot kill the process. superblock_wedged_fatal_timeout = "2m" +# DOWNGRADE, every deployment: this release grew the superblock record from 66 +# to 74 bytes to carry partition.offset_reservation_lease's claim. The metadata +# superblock is written by EVERY server, clustered or not, so upgrading rewrites +# it at the new length on a plain single node too. A build that predates the +# field refuses a record of that length and treats the refusal as a durability +# violation, which fails the whole node's boot -- so rolling BACK to +# server-0.9.0-edge.6 or earlier needs the data directory wiped on every node. +# Upgrading needs nothing: the 66-byte record still decodes. + # Replica-to-replica authentication (PSK + BLAKE3 keyed-MAC handshake). [cluster.auth] # When true, every replica peer must complete the authenticated handshake or be @@ -996,6 +1006,21 @@ prepare_queue_depth = 32 # actually sees, not the node's client total. dedup_clients_max = 4096 +# How many offsets a partition claims in its superblock ahead of the mint +# counter before it will append, so a crash-restarted replica resumes above +# every offset it confirmed to a client instead of re-minting it for a different +# message. One superblock write (two fsyncs) per block: lowering it raises the +# fsync rate on the write path, raising it wastes at most one block of the u64 +# offset space per crash. Must be > 0 and <= 16777216. +# +# SINGLE-REPLICA groups only. A replicated group acks a send once a quorum has +# journaled it, so re-minting there needs a full-cluster crash; it claims +# nothing, pays no write, and ignores this value. +# +# DOWNGRADE: see the note on superblock_wedged_fatal_timeout in [cluster]. +# Upgrading needs nothing. +offset_reservation_lease = 65536 + # Entries the evicted ring retains per multi-replica partition for journal # repair after a peer rejoins. Larger widens the window a restarting peer can be # served from the ring before falling back to bulk sync, at the cost of pinned diff --git a/core/server/src/boot/recovery.rs b/core/server/src/boot/recovery.rs index e614e688f5..808eefe4aa 100644 --- a/core/server/src/boot/recovery.rs +++ b/core/server/src/boot/recovery.rs @@ -333,11 +333,9 @@ const _: () = assert!(consensus::DVC_HEADERS_MAX == iggy_binary_protocol::consensus::DVC_HEADERS_MAX); const _: () = assert!(consensus::DVC_HEADERS_MAX == u128::BITS as usize); -/// `[cluster] superblock_wedged_fatal_timeout` as a consecutive-failure count. -/// Retries pin at the backoff cap after warmup, so the window divided by -/// [`journal::superblock::SUPERBLOCK_RETRY_BACKOFF_MAX_MICROS`] bounds how -/// long a wedged replica may limp before it fail-stops. Zero stays zero -/// (fail-stop disabled). +/// `[cluster] superblock_wedged_fatal_timeout` as a consecutive-failure count, +/// which is the only shape the shard's `superblock_wedged` can compare. Zero +/// stays zero (fail-stop disabled). fn superblock_wedged_fatal_failures(config: &ServerConfig) -> u64 { superblock_window_to_failures( config @@ -347,12 +345,44 @@ fn superblock_wedged_fatal_failures(config: &ServerConfig) -> u64 { ) } +/// The failure count whose arrival time is the first at or past `window`. +/// +/// Walks the real retry schedule rather than dividing by the backoff cap. Only +/// the retries past warmup pin at the cap: the first six wait 20, 40, 80, 160, +/// 320 and 640 ms, so they spend 1.26 s of the window where a flat division +/// charges them six. The default 2 m window came out as 120 failures, which +/// arrive after about 114.26 s -- the fail-stop firing almost six seconds before +/// the window the operator configured. fn superblock_window_to_failures(window: Duration) -> u64 { if window.is_zero() { return 0; } - let cap_micros = u128::from(journal::superblock::SUPERBLOCK_RETRY_BACKOFF_MAX_MICROS); - u64::try_from((window.as_micros() / cap_micros).max(1)).unwrap_or(u64::MAX) + // `write_superblock_inner` records failure N and only then arms the wait + // that follows it, so failure N ARRIVES at the sum of the N-1 waits before + // it -- the first arrives at zero. The loop sums forward until the window is + // covered, and the count that satisfies it is one past the last wait summed. + let window_micros = window.as_micros(); + let mut elapsed = 0u128; + let mut waits = 0u64; + while elapsed < window_micros { + waits += 1; + elapsed += u128::from(superblock_retry_backoff_micros(waits)); + } + // A window shorter than the very first retry lands here with one wait + // summed, giving two: a threshold of one would fail-stop on the first + // failure, before any of the window had elapsed at all. + waits.saturating_add(1) +} + +/// The wait `IggyPartition`'s superblock writer arms after its `failures`-th +/// consecutive failure. +/// +/// Mirrors that arithmetic exactly. A divergence here does not fail a test, it +/// moves the fail-stop to a time no operator asked for. +fn superblock_retry_backoff_micros(failures: u64) -> u64 { + journal::superblock::SUPERBLOCK_RETRY_BACKOFF_BASE_MICROS + .saturating_mul(1 << failures.min(journal::superblock::SUPERBLOCK_RETRY_BACKOFF_MAX_SHIFT)) + .min(journal::superblock::SUPERBLOCK_RETRY_BACKOFF_MAX_MICROS) } /// Floor for the post-restart read-recovery deadline (see @@ -592,16 +622,53 @@ mod tests { ); assert_eq!( superblock_window_to_failures(Duration::from_mins(2)), - 120, - "past warmup one retry rides each 1s backoff cap" + 126, + "the six warmup retries spend 1.26s, not 6s: 120 would fire at ~114.26s" + ); + assert_eq!( + superblock_window_to_failures(Duration::from_secs(30)), + 36, + "the configured floor for a nonzero window" ); assert_eq!( superblock_window_to_failures(Duration::from_micros(500)), - 1, - "a sub-cap window still needs one failure to fire" + 2, + "a window shorter than the first retry must not fail-stop on the \ + very first failure, before any of it elapsed" ); } + /// The threshold is a floor on elapsed time, never a ceiling: the failure it + /// names must arrive at or after the configured window, and its predecessor + /// must arrive before it. Walked against the writer's own schedule. + #[test] + fn given_a_fatal_window_when_converted_should_never_fire_before_it_elapses() { + for window in [ + Duration::from_secs(30), + Duration::from_secs(45), + Duration::from_mins(2), + Duration::from_mins(10), + ] { + let threshold = superblock_window_to_failures(window); + // Failure N arrives at the sum of the N-1 waits before it. + let arrival = |count: u64| -> u128 { + (1..count) + .map(|wait| u128::from(superblock_retry_backoff_micros(wait))) + .sum() + }; + assert!( + arrival(threshold) >= window.as_micros(), + "{window:?}: failure {threshold} arrives at {}us, inside the window", + arrival(threshold) + ); + assert!( + arrival(threshold - 1) < window.as_micros(), + "{window:?}: failure {} already covers the window, so {threshold} is late", + threshold - 1 + ); + } + } + #[test] fn default_cluster_heartbeat_timeout_matches_consensus_constant() { // The config default lives in core/server/config.toml (a string, @@ -849,6 +916,21 @@ mod tests { ); } + #[test] + fn default_offset_reservation_lease_matches_partitions_constant() { + // `IggyPartition::new` falls back to the partitions constant (simulator, + // unit tests) while boot installs this one, so drift would have the + // fence write at a different rate in the simulator than in production. + let config_default = + configs::partition::PartitionConfig::default().offset_reservation_lease; + assert_eq!( + config_default.get(), + partitions::DEFAULT_OFFSET_RESERVATION_LEASE, + "[partition] offset_reservation_lease default drifted from \ + partitions::DEFAULT_OFFSET_RESERVATION_LEASE" + ); + } + #[test] fn default_evicted_ring_capacity_matches_partitions_constant() { // Belt and suspenders with the static assert above; this pins the diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index ec39868e5c..f04d0d5376 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -46,7 +46,9 @@ use journal::superblock::{PingPongSuperblock, SuperblockContents}; use message_bus::IggyMessageBus; use metadata::stm::stream::Partition; use metadata::{IdentityField, ReplicaIdentity}; -use partitions::{IggyIndexWriter, IggyPartition, IggyPartitions, MessagesWriter, Segment}; +use partitions::{ + IggyIndexWriter, IggyPartition, IggyPartitions, MessagesWriter, PartitionsConfig, Segment, +}; use server_common::SegmentStorage; use server_common::fs_utils::remove_dir_all; use server_common::sharding::IggyNamespace; @@ -143,15 +145,16 @@ pub async fn create_partition_file_hierarchy( /// Populate `partition` with consumer-offset / consumer-group-offset storage. /// /// Hydrates from on-disk state if files exist (recovery path) or -/// configures empty maps (fresh partition path). `current_offset` bounds -/// recovered offsets so a partition that lost its tail does not surface -/// consumer offsets ahead of its current log head. +/// configures empty maps (fresh partition path). Recovered offsets are bounded +/// so a partition that lost its tail does not surface consumer offsets ahead of +/// an offset it never handed out, and `current_offset` is where a bounded one +/// lands. /// /// # Errors /// /// Returns [`ServerError::ConsumerOffsetsLoad`] when the on-disk files -/// exist but fail to decode. A stored offset ahead of `current_offset` is -/// clamped (with a warning), not an error. +/// exist but fail to decode. A stored offset past the offset space is clamped +/// to `current_offset` (with a warning), not an error. pub fn configure_consumer_offsets( partition: &mut IggyPartition>, config: &ServerConfig, @@ -169,6 +172,17 @@ pub fn configure_consumer_offsets( config .system .get_consumer_group_offsets_path(stream_id, topic_id, partition_id); + // The bound is the offset space this replica could have MINTED, not the data + // it can still serve. A boot re-anchor leaves the append point a lease block + // above the recovered chain, so on the restart after a crash that took + // acked-but-unflushed messages, a position stored before that crash names a + // real offset sitting under an empty chain -- confirmed to a client, and not + // "past the log" the way a torn offset file is. Bounding it by the data head + // instead walks a committed consumer position BACKWARD across the restart, + // which is the silent re-read the reservation exists to prevent. + // `mint_frontier` is one past the next mint, and reads 0 on the fresh-build + // path, where the max leaves `current_offset` in charge as before. + let offset_space_ceiling = current_offset.max(partition.mint_frontier().saturating_sub(1)); let loaded_consumer_offsets = load_partition_consumer_offsets( &consumer_offsets_path, @@ -182,7 +196,7 @@ pub fn configure_consumer_offsets( let guard = consumer_offsets.pin(); for offset in loaded_consumer_offsets { let recovered_offset = offset.offset.load(Ordering::Relaxed); - if recovered_offset > current_offset { + if recovered_offset > offset_space_ceiling { // A crash can persist an offset ahead of the flushed data // (offsets are stored eagerly, messages flush later). Clamp to // the recovered head so the consumer resumes instead of being @@ -191,6 +205,7 @@ pub fn configure_consumer_offsets( consumer_id = offset.consumer_id, recovered_offset, current_offset, + offset_space_ceiling, stream_id, topic_id, partition_id, @@ -213,11 +228,12 @@ pub fn configure_consumer_offsets( let guard = consumer_group_offsets.pin(); for (group_id, offset) in loaded_group_offsets { let recovered_offset = offset.offset.load(Ordering::Relaxed); - if recovered_offset > current_offset { + if recovered_offset > offset_space_ceiling { warn!( consumer_group_id = group_id.0, recovered_offset, current_offset, + offset_space_ceiling, stream_id, topic_id, partition_id, @@ -328,7 +344,7 @@ pub async fn ensure_initial_segment( // `rposition(|s| s.start_offset <= offset)` routes every poll for `0..N-1` // into it, the next boot makes that shape durable, and this replica starts // offering peers a segment that claims `[0..N]`. - let start_offset = partition.offset_frontier(); + let start_offset = partition.mint_frontier(); let messages_path = config .system @@ -535,6 +551,7 @@ pub async fn load_partition_or_fence( // outgrow clippy's `large_futures` cap, and this runs once per partition. match Box::pin(load_partition( config, + partitions.config(), namespace, Arc::clone(&partition_stats), partition_metadata, @@ -708,6 +725,7 @@ pub async fn load_partition_or_fence( #[allow(clippy::too_many_arguments)] async fn load_partition( config: &ServerConfig, + partitions_config: &PartitionsConfig, namespace: IggyNamespace, stats: Arc, partition_metadata: &Partition, @@ -798,6 +816,7 @@ async fn load_partition( config.partition.evicted_ring_bytes_max.as_bytes_u64(), ); partition.set_dedup_clients_max(config.partition.dedup_clients_max); + partition.set_offset_reservation_lease(config.partition.offset_reservation_lease); partition.set_partition_dir(partition_dir.clone()); // Before the hydrate: the durable record is keyed by incarnation, so a // `purge.gen` left behind by a previous life of this namespace reads 0. @@ -813,6 +832,28 @@ async fn load_partition( ) .await?; + partition.created_at = partition_metadata.created_at; + restore_partition_offsets(&mut partition, partitions_config, recovered_state.as_ref()).await?; + let current_offset = partition.offset.load(Ordering::Acquire); + + configure_consumer_offsets(&mut partition, config, namespace, current_offset)?; + ensure_initial_segment(&mut partition, config, stream_id, topic_id, partition_id).await?; + + Ok(partition) +} + +/// Restore the offset counter of a recovered partition from what boot could +/// prove about its offset space, then put the next append point where the +/// recovery walk can read it back. +/// +/// Three carriers, weakest last: the sized segments' end offset, an empty +/// chain's file name (a state-transfer install at the group frontier), and the +/// superblock's durable frontier as a lower bound over both. +async fn restore_partition_offsets( + partition: &mut IggyPartition>, + partitions_config: &PartitionsConfig, + recovered_state: Option<&VsrState>, +) -> Result<(), ServerError> { let sized_end = partition .log .segments() @@ -825,15 +866,23 @@ async fn load_partition( // frontier after the origin GC'd everything: the file name carries the // frontier, and re-minting offsets from 0 here would fork this // replica's batch stamps from the rest of the group after a restart. + // + // Bounded by the durable frontier: an install writes it at the group + // frontier, so the name is corroborated, while the boot re-anchor and + // `ensure_initial_segment` plant at `mint_frontier()`, a RESERVATION that + // names no data and leaves the frontier far below. Without the bound two + // crashes under the flush threshold promote 65537 to committed on a + // partition holding nothing, and `store_consumer_offset` admits the hole. + let durable_frontier = recovered_state.map_or(0, |state| state.offset_frontier); let empty_frontier = partition .log .segments() .iter() .map(|segment| segment.start_offset) .max() + .map(|start| start.min(durable_frontier)) .filter(|&start| sized_end.is_none() && start > 0); let current_offset = sized_end.or_else(|| empty_frontier.map(|start| start - 1)); - partition.created_at = partition_metadata.created_at; partition.recovered_durable_offset = sized_end; // The OFFSET COUNTER is restored from that file name (above), but the // `installed_frontier` CLAIM deliberately is not: the claim says "everything @@ -851,18 +900,27 @@ async fn load_partition( let counter = current_offset.unwrap_or(0); partition.offset.store(counter, Ordering::Release); partition.dirty_offset.store(counter, Ordering::Relaxed); - partition.should_increment_offset = current_offset.is_some(); + partition.set_offset_space_used(current_offset.is_some()); // The durable frontier is a LOWER BOUND on top of what the segments proved: // it is the only carrier left when the segments that named the frontier are // gone (an all-GC'd origin's install, a crash inside the swap window), and // taking the max means real recovered data always wins. - partition.restore_offset_frontier(recovered_state.as_ref()); - let current_offset = partition.offset.load(Ordering::Acquire); - - configure_consumer_offsets(&mut partition, config, namespace, current_offset)?; - ensure_initial_segment(&mut partition, config, stream_id, topic_id, partition_id).await?; - - Ok(partition) + partition.restore_offset_frontier(recovered_state); + // Minting from the reservation leaves a hole between the recovered chain + // and the new append point, and the recovery walk REFUSES a hole inside a + // segment (tombstoning the partition on the solo arm), so put it on a + // segment boundary instead. + // + // Solo only, in step with the reservation itself: a replicated group's + // segment boundaries must be a function of the batches alone or the + // reconciler's offset-keyed segment GC never converges. + if partition.consensus().replica_count() == 1 { + partition + .reanchor_to_offset_frontier(partitions_config) + .await + .map_err(|error| ServerError::Iggy(Box::new(error)))?; + } + Ok(()) } /// Recover this partition's persisted segment chain, stamping each segment @@ -887,26 +945,18 @@ async fn recover_partition_segments( let enforce_fsync = runtime_options .enforce_fsync .unwrap_or(iggy_common::DEFAULT_ENFORCE_FSYNC); - load_persisted_segments( - config, - stream_id, - topic_id, - partition_id, - segment_size, - enforce_fsync, - stats, - ) - .await - .map_err(|source| { - error!( - stream_id, - topic_id, - partition_id, - error = %source, - "failed to load partition log during server bootstrap" - ); - source - }) + load_persisted_segments(config, namespace, segment_size, enforce_fsync, stats) + .await + .map_err(|source| { + error!( + stream_id, + topic_id, + partition_id, + error = %source, + "failed to load partition log during server bootstrap" + ); + source + }) } /// Reopen writers over a recovered segment chain. @@ -1078,9 +1128,9 @@ fn hydrate_reopen_error( /// `seed_view` comment below for why a group left at view 0 is unreachable. A /// restart materialization ignores it and probes for the live view instead. /// -/// The returned partition's `offset` / `dirty_offset` are `0` and -/// `should_increment_offset` is `false`, mirroring a clean append starting -/// at the empty segment. +/// The returned partition's `offset` / `dirty_offset` are `0` and its +/// `OffsetSpace` is unused, mirroring a clean append starting at the empty +/// segment. /// /// # Errors /// @@ -1212,6 +1262,7 @@ pub async fn build_partition_fresh( config.partition.evicted_ring_bytes_max.as_bytes_u64(), ); partition.set_dedup_clients_max(config.partition.dedup_clients_max); + partition.set_offset_reservation_lease(config.partition.offset_reservation_lease); partition.set_partition_dir(partition_dir); // Fresh dirs read generation 0; a dir surviving from a crashed process // (this "fresh" build races repair re-materialization) reads the last @@ -1224,7 +1275,7 @@ pub async fn build_partition_fresh( partition.created_at = IggyTimestamp::now(); partition.offset.store(0, Ordering::Release); partition.dirty_offset.store(0, Ordering::Relaxed); - partition.should_increment_offset = false; + partition.set_offset_space_used(false); debug_assert!( !partition.log.has_segments(), "fresh partition must not carry recovered segments" @@ -1312,6 +1363,7 @@ pub async fn delete_partitions_from_disk( #[cfg(test)] mod tests { use super::*; + use configs::server::ServerSystemConfig; use journal::superblock::SuperblockStore; const CLUSTER: u128 = 7; @@ -1329,6 +1381,19 @@ mod tests { checkpoint_op: 0, checkpoint_checksum: 0, offset_frontier: 0, + offset_reserved: 0, + } + } + + /// The solo shape the reservation is scoped to, with a claim already + /// recorded and nothing flushed behind it. + fn reserved_solo_state(reserved: u64) -> VsrState { + VsrState { + replica_id: 0, + replica_count: 1, + commit_max: 0, + offset_reserved: reserved, + ..recorded_state(0, 0) } } @@ -1344,6 +1409,83 @@ mod tests { } } + /// A rebuild that reads a reservation back must NAME its planted segment for + /// the append point. Named 0, the segment takes the first append's + /// `base_offset` of N instead, `rposition(|s| s.start_offset <= offset)` + /// routes every poll for `0..N-1` into it, and the next boot makes that + /// durable. + #[compio::test] + async fn given_a_recorded_reservation_when_building_fresh_should_plant_at_the_append_point() { + const RESERVED: u64 = 65_537; + let root = tempfile::tempdir().expect("tempdir"); + let config = ServerConfig { + system: Arc::new(ServerSystemConfig { + path: root.path().to_string_lossy().into_owned(), + ..ServerSystemConfig::default() + }), + ..ServerConfig::default() + }; + let namespace = IggyNamespace::new(1, 1, 0); + let dir = config.system.get_partition_path(1, 1, 0); + + let identity = ReplicaIdentity { + cluster: CLUSTER, + replica_id: 0, + replica_count: 1, + }; + let (store, recovered) = open_partition_superblock(&dir, identity) + .await + .expect("open a fresh partition superblock"); + assert!(recovered.is_none()); + store + .write(&reserved_solo_state(RESERVED).to_bytes()) + .await + .expect("record the reservation"); + drop(store); + + let partition = build_partition_fresh( + &config, + namespace, + Arc::new(PartitionStats::default()), + 0, + TopicRuntimeOptions::default(), + CLUSTER, + 0, + 1, + 0, + Rc::new(IggyMessageBus::new(0)), + ) + .await + .expect("rebuild the partition over its recorded reservation"); + + assert_eq!( + partition.mint_frontier(), + RESERVED, + "the append point must resume above every offset the reservation covered" + ); + assert_eq!( + partition.offset_frontier(), + 0, + "nothing was flushed, so the committed frontier names no data" + ); + + let planted: Vec = std::fs::read_dir(&dir) + .expect("list the partition dir") + .flatten() + .filter_map(|entry| { + let path = entry.path(); + (path.extension()? == "log") + .then(|| path.file_name()?.to_str().map(str::to_owned)) + .flatten() + }) + .collect(); + assert_eq!( + planted, + vec![format!("{RESERVED:0>20}.log")], + "the initial segment must be named for the append point, not offset 0" + ); + } + #[compio::test] async fn given_fresh_partition_dir_when_superblock_opened_should_yield_no_state() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/core/server/src/segment_recovery.rs b/core/server/src/segment_recovery.rs index 2c889fa92b..0e192c0794 100644 --- a/core/server/src/segment_recovery.rs +++ b/core/server/src/segment_recovery.rs @@ -30,15 +30,17 @@ use crate::server_error::{PartitionRecoveryRefusal, ServerError}; use configs::server::ServerConfig; use iggy_common::{IggyByteSize, IggyError, MAX_MESSAGE_SIZE_UPPER_BYTES, PartitionStats}; +use partitions::segment_anchor::ANCHOR_EXTENSION; use partitions::state_transfer::STAGING_SUFFIX; use partitions::{IggyIndex, IggyIndexReader, Segment}; use server_common::send_messages::{BatchHeader, COMMAND_HEADER_SIZE, decode_batch_slice}; +use server_common::sharding::IggyNamespace; use server_common::{SegmentStorage, yield_to_reactor}; use std::fs; use std::io; use std::os::unix::fs::FileExt; use std::path::{Path, PathBuf}; -use tracing::{error, warn}; +use tracing::{error, info, warn}; const LOG_EXTENSION: &str = "log"; const INDEX_EXTENSION: &str = "index"; @@ -173,16 +175,21 @@ pub struct RecoveredSegment { /// makes a durable index entry evidence about the log (see /// [`PartitionRecoveryRefusal::FsyncedLogLoss`]), so passing it wrong either /// refuses healthy chains or hides previously durable data loss. +/// +/// Takes no offset ceiling. A legitimate gap is proved by the anchor the boot +/// re-anchor writes beside the segment it plants, not inferred from how far the +/// superblock's reservation happens to reach. #[allow(clippy::too_many_lines)] pub async fn load_persisted_segments( config: &ServerConfig, - stream_id: usize, - topic_id: usize, - partition_id: usize, + namespace: IggyNamespace, segment_size: IggyByteSize, enforce_fsync: bool, stats: &PartitionStats, ) -> Result, ServerError> { + let stream_id = namespace.stream_id(); + let topic_id = namespace.topic_id(); + let partition_id = namespace.partition_id(); let partition_path = config .system .get_partition_path(stream_id, topic_id, partition_id); @@ -298,9 +305,9 @@ pub async fn load_persisted_segments( last.segment.sealed = false; } - // Pass B: the chain guard reads only the planned bounds, so it can refuse - // BEFORE anything is truncated. - ensure_contiguous_chain(identity, &planned)?; + // Pass B: the chain guard reads the planned bounds and, for a gap, the + // anchor beside it, so it can refuse BEFORE anything is truncated. + ensure_contiguous_chain(identity, &planned).await?; // Pass C: the chain is accepted; make disk match the bounds and open // storage over them. @@ -529,13 +536,35 @@ struct ScanScratch { /// chain and push `current_offset` past data this replica does not hold. /// Refuse loudly instead of serving a holed log. /// +/// A FORWARD gap is admitted only when the far side carries a +/// [`SegmentAnchor`] naming exactly the near side, which is the record the boot +/// re-anchor writes before it plants. Nothing else legitimises a gap: an +/// overlap, a backwards pair, or a gap with no anchor is damage. +/// +/// The anchors are what a monotone offset ceiling could not be. A ceiling says +/// only "some boot claimed up to N", and every plant base sits below the current +/// N -- but so does the successor of a segment that was deleted, so a lost middle +/// segment read as a plant. The anchor is written by the one component that +/// creates legitimate gaps, names which segment it sealed, and is swept as soon +/// as its segment is gone. +/// +/// Retention needs no allowance: it removes a contiguous FRONT prefix, so the +/// remaining chain stays contiguous and no interior gap appears. +/// +/// # Known residual +/// +/// An anchor whose planted segment survived while the sealed segment it names was +/// itself lost still reads as legitimate, because the pair the anchor describes +/// is then simply absent from the chain and the guard never examines it. Catching +/// that needs a durable count of the chain, which this record does not carry. +/// /// Runs on the planned bounds alone, BEFORE any truncation, so the segment /// files a refusal quarantines are exactly the bytes boot found. The refusal /// names the partition and its directory so the caller can fence THAT group /// rather than abort the node's boot: the shapes it rejects are exactly what /// a failed quarantine leaves behind, and one damaged local chain must not /// take the whole node down. -fn ensure_contiguous_chain( +async fn ensure_contiguous_chain( identity: PartitionIdentity<'_>, planned: &[PlannedSegment], ) -> Result<(), ServerError> { @@ -567,7 +596,41 @@ fn ensure_contiguous_chain( } // `checked_add`, not `+`: an end offset at u64::MAX must read as a // hole (no start offset can follow it), not overflow. - if previous.end_offset.checked_add(1) != Some(next.start_offset) { + if previous.end_offset.checked_add(1) == Some(next.start_offset) { + continue; + } + // FORWARD only, and only with the plant's own record beside it. Start + // offsets come off the file names so they ascend, but each end offset is + // walked from that file's own bytes with nothing clamping it against the + // next start, so a half-installed transfer or an operator copy can leave + // a pair that overlaps -- and no re-anchor ever plants a segment whose + // range a predecessor already covers. + // Read HERE rather than collected up front: only a gap needs an anchor, + // so a contiguous chain -- every chain that never crashed mid-block -- + // opens no file at all, and the ones that do are already walking this + // pair. An unreadable anchor is unknown, not absent, and treating it as + // absent would refuse a healthy chain for as long as the fault lasts. + let read = + partitions::segment_anchor::read_anchor(identity.partition_path, next.start_offset) + .await + .map_err(|error| { + error!( + partition_path = identity.partition_path, + start_offset = next.start_offset, + %error, + "failed to read a segment anchor during recovery" + ); + ServerError::from(IggyError::CannotReadFile) + })?; + let anchored = previous.end_offset < next.start_offset + && read.is_some_and(|anchor| { + anchor.covers( + next.start_offset, + previous.start_offset, + previous.end_offset, + ) + }); + if !anchored { return Err(identity.refusal(PartitionRecoveryRefusal::Hole { previous_start: previous.start_offset, previous_end: previous.end_offset, @@ -575,6 +638,14 @@ fn ensure_contiguous_chain( recoverable_bytes, })); } + info!( + partition_path = identity.partition_path, + previous_start = previous.start_offset, + previous_end = previous.end_offset, + next_start = next.start_offset, + "admitted a gap in the recovered segment chain: the planted segment \ + carries the boot re-anchor's own record of it" + ); } Ok(()) } @@ -636,7 +707,11 @@ fn sweep_scratch_files_and_collect_offsets(partition_path: &str) -> Result orphan_candidates.push(path), + // An anchor outlives nothing: it describes the gap in front of ONE + // segment, so once that segment is gone (retention, a failed plant + // that never landed) the record can only mislead a later guard into + // admitting a gap it never saw. + Some(INDEX_EXTENSION | ANCHOR_EXTENSION) => orphan_candidates.push(path), _ => {} } } @@ -2407,6 +2482,7 @@ mod tests { use super::*; use bytes::Bytes; use configs::server::ServerSystemConfig; + use partitions::segment_anchor::SegmentAnchor; use server_common::send_messages::{ IggyMessage, IggyMessageHeader, IggyMessages, SendMessagesOwned, calculate_batch_checksum, }; @@ -2565,6 +2641,44 @@ mod tests { (messages_path, index_path) } + /// Path of the anchor beside the segment planted at `start_offset`. + fn anchor_fixture_path(config: &ServerConfig, start_offset: u64) -> String { + partitions::segment_anchor::anchor_path( + &config + .system + .get_partition_path(STREAM_ID, TOPIC_ID, PARTITION_ID), + start_offset, + ) + } + + /// Write the anchor a plant at `planted_start` leaves behind, naming the tail + /// it sealed. + fn write_anchor_fixture( + config: &ServerConfig, + planted_start: u64, + sealed_start: u64, + sealed_end: u64, + ) { + let anchor = SegmentAnchor { + planted_start, + sealed_start, + sealed_end, + }; + fs::write( + anchor_fixture_path(config, planted_start), + anchor.to_bytes(), + ) + .expect("write anchor fixture"); + } + + /// Recover expecting a refusal, with `context` naming what should have failed. + async fn refusal(config: &ServerConfig, context: &str) -> ServerError { + match recover(config).await { + Ok(recovered) => panic!("{context}, got {} segments", recovered.len()), + Err(error) => error, + } + } + fn len_of(path: &str) -> u64 { fs::metadata(path).expect("stat fixture file").len() } @@ -2584,18 +2698,23 @@ mod tests { } async fn recover(config: &ServerConfig) -> Result, ServerError> { - recover_under_fsync(config, false).await + recover_with(config, false).await } async fn recover_under_fsync( config: &ServerConfig, enforce_fsync: bool, + ) -> Result, ServerError> { + recover_with(config, enforce_fsync).await + } + + async fn recover_with( + config: &ServerConfig, + enforce_fsync: bool, ) -> Result, ServerError> { load_persisted_segments( config, - STREAM_ID, - TOPIC_ID, - PARTITION_ID, + IggyNamespace::new(STREAM_ID, TOPIC_ID, PARTITION_ID), IggyByteSize::from(SEGMENT_MAX_SIZE), enforce_fsync, &PartitionStats::default(), @@ -3018,6 +3137,249 @@ mod tests { assert_eq!(bytes_of(&next_index_path), next_index); } + /// Valid, checksum-clean anchor bytes COPIED beside a later segment must not + /// authorise the wider gap they now sit in front of. The record names its own + /// plant, and the guard matches that against the file it was found beside, so + /// an operator's `cp` buys nothing. + #[compio::test] + async fn given_an_anchor_copied_beside_a_later_segment_when_recovering_should_refuse() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 500, &encoded_batch(500, 1), &index_entry(500, 0)); + // The anchor a legitimate plant at 10 would have left, moved beside the + // segment at 500 without touching a byte of it. + let stolen = fs::read({ + write_anchor_fixture(&config, 10, 0, 2); + anchor_fixture_path(&config, 10) + }) + .expect("read the legitimate anchor"); + fs::remove_file(anchor_fixture_path(&config, 10)).expect("unlink the original"); + fs::write(anchor_fixture_path(&config, 500), &stolen).expect("copy it beside 500"); + + let error = refusal(&config, "a copied anchor must not cover a wider gap").await; + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { .. }, + .. + } + ), + "expected a hole refusal, got {error:?}" + ); + } + + /// The shape the boot re-anchor leaves: a sealed tail, then the next segment + /// planted above it, with the anchor beside the plant naming the tail. + #[compio::test] + async fn given_an_anchored_gap_when_recovering_should_accept_the_chain() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 1), &index_entry(10, 0)); + write_anchor_fixture(&config, 10, 0, 2); + + let recovered = recover(&config) + .await + .expect("a gap the plant recorded is the re-anchor's, not damage"); + + assert_eq!(recovered.len(), 2); + assert_eq!(recovered[0].segment.start_offset, 0); + assert_eq!(recovered[1].segment.start_offset, 10); + } + + /// The regression this record exists to close: with no anchor the gap is a + /// segment that went missing, and admitting it serves a holed log silently. + #[compio::test] + async fn given_an_unanchored_gap_when_recovering_should_refuse() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 1), &index_entry(10, 0)); + + let error = refusal(&config, "a gap no plant recorded must refuse recovery").await; + + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { .. }, + .. + } + ), + "expected a hole refusal, got {error:?}" + ); + } + + /// An anchor names WHICH segment it sealed, so one left behind by an earlier + /// chain cannot legitimise a gap it never saw. + #[compio::test] + async fn given_an_anchor_naming_another_segment_when_recovering_should_refuse() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 1), &index_entry(10, 0)); + // Right shape, wrong predecessor: this anchor describes a tail ending at + // 5, and the chain's tail ends at 2. + write_anchor_fixture(&config, 10, 0, 5); + + let error = refusal(&config, "an anchor for a different tail must not cover it").await; + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { .. }, + .. + } + ), + "expected a hole refusal, got {error:?}" + ); + } + + /// A corrupt anchor proves nothing, so the gap it would have covered stays + /// damage rather than becoming legitimate by default. + #[compio::test] + async fn given_a_corrupt_anchor_when_recovering_should_refuse() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 1), &index_entry(10, 0)); + let path = anchor_fixture_path(&config, 10); + let mut bytes = SegmentAnchor { + planted_start: 10, + sealed_start: 0, + sealed_end: 2, + } + .to_bytes(); + bytes[8] ^= 1; + fs::write(&path, bytes).expect("write corrupt anchor"); + + let error = refusal(&config, "a corrupt anchor must not cover a gap").await; + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { .. }, + .. + } + ), + "expected a hole refusal, got {error:?}" + ); + } + + /// Every crash cycle leaves one more re-anchor gap, and the guard runs on the + /// NEXT boot -- before the re-anchor -- so by the second cycle the earlier gap + /// is no longer the last pair. A rule keyed on the last pair alone would + /// refuse this, and the solo arm tombstones a chain with bytes in it, taking a + /// healthy partition dark from the second crash onward. + #[compio::test] + async fn given_gaps_from_several_crash_cycles_when_recovering_should_accept_the_chain() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 2), &index_entry(10, 0)); + write_segment(&config, 20, &encoded_batch(20, 1), &index_entry(20, 0)); + write_anchor_fixture(&config, 10, 0, 2); + write_anchor_fixture(&config, 20, 10, 11); + + let recovered = recover(&config) + .await + .expect("anchored gaps accumulate one per crash, not one total"); + + assert_eq!(recovered.len(), 3); + assert_eq!(recovered[0].segment.start_offset, 0); + assert_eq!(recovered[1].segment.start_offset, 10); + assert_eq!(recovered[2].segment.start_offset, 20); + } + + /// One lost segment in the middle of a chain whose OTHER gaps are all + /// anchored. The anchors say nothing about this pair, so it must still refuse + /// -- the shape a single monotone ceiling could not separate from a plant. + #[compio::test] + async fn given_a_lost_segment_among_anchored_gaps_when_recovering_should_refuse() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 2), &index_entry(10, 0)); + // 20 was an ordinary rotation off 12, then went missing; 30 is a plant. + write_segment(&config, 30, &encoded_batch(30, 1), &index_entry(30, 0)); + write_anchor_fixture(&config, 10, 0, 2); + + let error = refusal(&config, "a lost middle segment must refuse recovery").await; + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { + previous_start: 10, + previous_end: 11, + next_start: 30, + .. + }, + .. + } + ), + "expected a hole refusal naming the lost pair, got {error:?}" + ); + } + + /// No re-anchor plants a segment whose range a predecessor already covers, so + /// an anchor must not launder an overlap either. Reachable because each end + /// offset is walked from its own file with nothing clamping it against the + /// next start. + #[compio::test] + async fn given_overlapping_segments_when_recovering_should_refuse_even_when_anchored() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + // `0.log` walks to 0..=15 while `10.log` claims 10 onward: the pair runs + // backwards, which no legitimate chain does. + write_segment(&config, 0, &encoded_batch(0, 16), &index_entry(0, 0)); + write_segment(&config, 10, &encoded_batch(10, 3), &index_entry(10, 0)); + write_anchor_fixture(&config, 10, 0, 15); + + let error = refusal(&config, "an overlap must refuse however it is recorded").await; + assert!( + matches!( + &error, + ServerError::PartitionRecoveryRefused { + reason: PartitionRecoveryRefusal::Hole { .. }, + .. + } + ), + "expected a hole refusal, got {error:?}" + ); + } + + /// An anchor whose segment is gone can only mislead a later guard, so the boot + /// sweep collects it the way it collects an orphaned index. + #[compio::test] + async fn given_an_anchor_with_no_segment_when_recovering_should_sweep_it() { + let tmp = tempdir().expect("tempdir"); + let config = test_config(&tmp); + prepare_partition_dir(&config); + write_segment(&config, 0, &encoded_batch(0, 3), &index_entry(0, 0)); + // The window a crash between the anchor write and the plant leaves. + write_anchor_fixture(&config, 10, 0, 2); + let orphan = anchor_fixture_path(&config, 10); + + let recovered = recover(&config).await.expect("recover the intact chain"); + + assert_eq!(recovered.len(), 1); + assert!( + !Path::new(&orphan).exists(), + "an anchor naming a segment that does not exist must be swept" + ); + } + #[compio::test] async fn given_non_monotone_index_entries_when_recovering_should_rebuild_the_index_from_the_log() { diff --git a/core/shard/src/lib.rs b/core/shard/src/lib.rs index fe4deaea5a..67458f598b 100644 --- a/core/shard/src/lib.rs +++ b/core/shard/src/lib.rs @@ -936,6 +936,16 @@ impl RestorableMetadataStm for M where /// so the bounded per-peer bus queue can never drop a burst tail. Clamped /// against the live bus ceiling by /// [`IggyShard::state_chunk_len_max`] rather than assumed to fit. +/// Superblock writes issued at once when a whole shard's groups need one in the +/// same pass: a node-wide view change, or a graceful stop collapsing every +/// partition's offset reservation. +/// +/// Each write is a create + write + 2 fsyncs. Serial, a few hundred groups on +/// ordinary storage overrun the view-change escalation window (and, on the stop +/// path, a supervisor's kill timeout); unbounded, they dump the whole burst of +/// fds and fsyncs onto the reactor in one pass. +const SUPERBLOCK_FAN_OUT: usize = 16; + const STATE_CHUNK_LEN: u32 = 256 * 1024; /// Bus frame ceiling assumed before bootstrap overrides it. Matches the @@ -3990,11 +4000,10 @@ where } // Retained log before the frontier restore, so the restore maxes against // the offsets the log proved rather than the zeroes of an empty one. - // `restore_offset_frontier` STORES `recovered_end` once past its guard, so - // it can lower `dirty_offset`; harmless only because `write_superblock` - // maxes the recorded frontier against `offset_frontier()`. The order also - // keeps that restore's precondition (`should_increment_offset` already set - // by a recovered offset space) meaningful. + // `restore_offset_frontier` takes each counter's own max against what is + // already loaded, so neither can be lowered here -- but only if the log is + // adopted first, or those maxes are taken against the zeroes of a + // partition that has not got its offsets back yet. if let Some(state) = retained { partition.adopt_retained_log(state); // OPT-IN, off by default: it models durability Iggy does not have. @@ -4027,6 +4036,12 @@ where // the restore at all, a simulator replica rebuilt against a retained // store resumes minting at 0 while its group is at N. partition.restore_offset_frontier(recovered_state.as_ref()); + // And the chain transition that restore obliges, which production's boot + // does through `reanchor_to_offset_frontier`. A restored counter can sit + // a lease block above the chain, and leaving the tail named below it puts + // the next mint inside a segment -- a shape boot never produces, so the + // harness would be modelling something the server cannot reach. + partition.reanchor_in_memory_to_mint_frontier(partitions.config().segment_size); partitions.insert(namespace, partition); if self.redispatch_parked_frames(namespace, epoch) { // This mutation occurs outside the pump, unlike production's @@ -6817,40 +6832,49 @@ where // partitions-plane borrow is held across the tick `.await`. namespace_scratch.extend(partitions.namespaces().copied()); - // Pre-pass: issue every group's pending superblock persist - // CONCURRENTLY. A cluster-wide view change makes every group on - // this shard need one in the same tick, and each `atomic_replace` - // is a create + write + 2 fsyncs; run serially, a few hundred - // groups on ordinary storage exceed the 5s view-change escalation - // and loop elections. The persists are independent (each group owns - // its store, lock, and failure bookkeeping, all behind `&self`), - // and the per-group loop below re-checks the gate on its lock-free - // fast path, so gating semantics are unchanged. + // Pre-pass: issue every group's pending superblock write CONCURRENTLY. + // A cluster-wide view change makes every group on this shard need one in + // the same tick, and each `atomic_replace` is a create + write + 2 + // fsyncs; run serially, a few hundred groups on ordinary storage exceed + // the 5s view-change escalation and loop elections. The writes are + // independent (each group owns its store, lock, and failure bookkeeping, + // all behind `&self`), and the per-group loop below re-checks the persist + // gate on its lock-free fast path, so gating semantics are unchanged. + // + // The offset-reservation extension rides the same pre-pass, which is the + // whole point of it being here: the append fence writes the superblock + // INLINE in this pump, where those two fsyncs delay the tick above for + // every group on the core. Extending at half a block of headroom keeps + // the fence on its lock-free fast path under load, so the write happens + // here instead of in front of a produce. Ordered BEFORE the persist + // because any write marks the view durable, so one write can satisfy + // both and the persist gate below then finds nothing to do. let pending_persists: Vec<_> = namespace_scratch .iter() .copied() .filter(|namespace| { - partitions - .get_by_ns(namespace) - .is_some_and(|partition| partition.consensus().needs_superblock_persist()) + partitions.get_by_ns(namespace).is_some_and(|partition| { + partition.consensus().needs_superblock_persist() + || partition.needs_offset_reservation_extension() + }) }) .map(|namespace| async move { if let Some(partition) = partitions.get_by_ns(&namespace) { - // The only dropped durability verdict in the tree: this pre-pass - // exists to coalesce the writes, and the per-group loop below re-runs - // the same gate on its lock-free fast path and withholds every - // view-scoped send when it fails, so the verdict here is redundant - // rather than ignored. + // Verdicts dropped on purpose. The reservation is backstopped + // by the fence at the mint, which refuses the append if the + // ceiling never caught up; and the persist gate is re-run by + // the per-group loop below on its lock-free fast path, which + // withholds every view-scoped send when it fails. + if partition.needs_offset_reservation_extension() { + let _ = partition.extend_offset_reservation().await; + } let _ = partition.persist_superblock_if_needed().await; } }) .collect(); - // Capped fan-out: each persist is a create + write + 2 fsyncs, and a - // node-wide view change over many partitions must not dump an - // unbounded fd/fsync burst onto the reactor in one tick. let mut pending_persists = pending_persists.into_iter(); loop { - let chunk: Vec<_> = pending_persists.by_ref().take(16).collect(); + let chunk: Vec<_> = pending_persists.by_ref().take(SUPERBLOCK_FAN_OUT).collect(); if chunk.is_empty() { break; } @@ -6880,6 +6904,27 @@ where } continue; } + // Same bound the metadata plane fail-stops on, applied per group, + // and it exits the NODE rather than fencing the group: a partition + // whose superblock keeps refusing withholds every view-scoped send, + // and on a solo group refuses every append too, so it serves nothing + // while the process still reports healthy. + let superblock_failures = partition.superblock_write_failures(); + if superblock_wedged( + superblock_failures, + self.superblock_wedged_fatal_failures.get(), + ) { + consensus::fatal( + FatalReason::SuperblockWedged, + &format!( + "partition superblock persist failed {superblock_failures} consecutive \ + times for namespace {}, past the [cluster] \ + superblock_wedged_fatal_timeout window; exiting so a supervisor handles \ + the wedge instead of the replica limping fenced", + namespace.inner() + ), + ); + } let consensus = partition.consensus(); // Only while a view change is live. A `Normal` tick has no consumer: @@ -7103,6 +7148,7 @@ where partitions = namespaces.len(), "shutdown flush: draining committed journals to segment storage" ); + let mut collapse_pending = Vec::new(); for namespace in namespaces { let Some(partition) = partitions.get_mut_by_ns(&namespace) else { continue; @@ -7121,7 +7167,45 @@ where // after this flush). A partition already fenced by the commit // path keeps its original fault. partition.fence_flush_failure(); + // The collapse claims the segments account for every confirmed + // offset, which a failed flush is exactly the case against, so + // leave the reservation standing. + continue; + } + collapse_pending.push(namespace); + } + + // Collapsed CONCURRENTLY, for the same reason the tick coalesces its view + // persists: see [`SUPERBLOCK_FAN_OUT`]. Each group owns its store, lock + // and failure bookkeeping, all behind `&self`. + // + // The flushes above stay serial: they take `&mut`, and the writers they + // drive are the shard's, not the partition's. + let mut pending = collapse_pending + .into_iter() + .map(|namespace| async move { + // The segments now prove where the offset space ends, so the + // reservation has nothing left to witness. Without the collapse + // every clean stop would leave a lease-block-wide hole. + let Some(partition) = partitions.get_by_ns(&namespace) else { + return; + }; + if !partition.collapse_offset_reservation().await { + tracing::warn!( + namespace_raw = namespace.inner(), + "could not collapse the offset reservation on shutdown; the restart \ + will resume above it and leave a gap in the offset space" + ); + } + }) + .collect::>() + .into_iter(); + loop { + let chunk: Vec<_> = pending.by_ref().take(SUPERBLOCK_FAN_OUT).collect(); + if chunk.is_empty() { + break; } + futures::future::join_all(chunk).await; } } diff --git a/core/simulator/src/lib.rs b/core/simulator/src/lib.rs index 3d2fd3feaa..36473b8dc7 100644 --- a/core/simulator/src/lib.rs +++ b/core/simulator/src/lib.rs @@ -1324,6 +1324,43 @@ impl Simulator { Some(u64::from(partition.consensus().view())) } + /// Claim the first offset block for every live replica's copy of a + /// materialised solo partition, standing in for the shard tick that does it + /// in production. + /// + /// The append fence BOUNCES the first send to a partition with no claim on + /// disk, so the superblock write lands on the tick rather than inside the + /// request pump. A real client retries that `TransientNotAccepted`; the + /// simulator has no retry loop, so a scenario that produces to a freshly + /// materialised partition and expects the send to commit has to claim the + /// block first. + /// + /// Not folded into [`Self::init_partition`]: materialising a partition is not + /// the same event as producing to one, and arming every materialised + /// partition would model a superblock write per idle partition that + /// production deliberately does not make. + /// + /// # Panics + /// If the simulated superblock refuses the claim, which no scenario injects: + /// a silent skip would leave the caller's next send bounced with nothing to + /// say why. + #[allow(clippy::cast_possible_truncation)] + pub fn claim_partition_offset_block(&self, namespace: IggyNamespace) { + for (index, replica) in self.replicas.iter().enumerate() { + if self.crashed.contains(&(index as u8)) { + continue; + } + let shard = replica.partition_shard(namespace); + let Some(partition) = shard.plane.partitions().get_by_ns(&namespace) else { + continue; + }; + assert!( + futures::executor::block_on(partition.extend_offset_reservation()), + "the simulated superblock must accept the first offset claim" + ); + } + } + /// One replica's view of a partition group's consensus, or `None` when it does /// not host the namespace. Read by the quiesce oracle to decide whether a group /// has settled into one view, which its leader-relative checks depend on once @@ -2638,6 +2675,10 @@ mod tests { ); sim.init_partition(namespace); + // The tick's job in production. Without it the append fence bounces this + // first send so the superblock write stays off the request pump, and the + // simulator has no client to retry the bounce. + sim.claim_partition_offset_block(namespace); assert_eq!( shard.redispatched_frame_count(), 1, From 4be4ac52e04f6ea2270085cf873234bacaad6109 Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Fri, 4 Sep 2026 10:49:54 +0200 Subject: [PATCH 2/6] fix(partitions): claim the first offset block at partition create The first send to a solo partition with no reservation on disk was answered TransientNotAccepted so the shard tick would claim the block off the request pump. That assumed every producer replays the transient. The binary SDKs do; the HTTP plane does not. The acked route has no replay loop, so it surfaced the bounce as HTTP 503, and `?ack=none` never reads a reply at all, so it answered 202 and dropped the message. Deterministic, once per partition: a topic with N partitions silently lost the first message to each. Claim the block where the partition is created instead. That path already pays for a superblock write, so the claim costs a new partition one extra atomic replace and takes the transient off every plane at once. The objection the old design raised against eager claiming is about boot, where many idle partitions would each pay a write for nothing; claiming at create does not touch boot, and the tick's trigger stays gated on a partition that has minted. The cost is offset space after an unclean stop: a partition created and never produced to now resumes a lease block above zero, the same hole the reservation already leaves after a crash mid-produce. A graceful stop collapses it. The simulator's `claim_partition_offset_block` stood in for the tick that the bounce required, and goes with it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/integration/tests/server/http_vsr.rs | 89 +++++++++++++ core/partitions/src/iggy_partition.rs | 152 +++------------------- core/server/src/partition_helpers.rs | 30 ++++- core/simulator/src/lib.rs | 41 ------ 4 files changed, 135 insertions(+), 177 deletions(-) diff --git a/core/integration/tests/server/http_vsr.rs b/core/integration/tests/server/http_vsr.rs index 2ae84bac2f..c97be5a65c 100644 --- a/core/integration/tests/server/http_vsr.rs +++ b/core/integration/tests/server/http_vsr.rs @@ -735,6 +735,95 @@ async fn given_ack_none_when_producing_should_return_202_and_commit(harness: &Te } } +/// The FIRST send to a partition that has never minted an offset, on both +/// produce routes, against the only cluster shape where the offset reservation +/// runs at all. +/// +/// `cluster_nodes = 1` is load-bearing, not a speed-up: `request_mint_ceiling` +/// returns `None` above one replica, so this suite's three-node default leaves +/// the whole reservation path as dead code and proves nothing here. +/// +/// The reservation writes the partition's superblock before it hands out an +/// offset, and a first send is where that claim is missing. Neither HTTP route +/// can carry a retryable refusal back to the caller: the acked route has no +/// transient replay loop, and `?ack=none` never reads a reply at all, so a +/// refusal there would answer 202 and drop the message. Both partitions are +/// produced to exactly once, so a per-partition regression cannot hide behind a +/// second send. +#[iggy_harness(cluster_nodes = 1)] +async fn given_a_solo_topic_when_producing_its_first_http_messages_should_commit_them( + harness: &TestHarness, +) { + const ACKED_PARTITION: u32 = 0; + const UNACKED_PARTITION: u32 = 1; + + let http = HttpClient::login_root(harness).await; + http.create_stream_and_topic("http-first-send", "first", 2) + .await; + + let response = http + .produce( + "http-first-send", + "first", + ACKED_PARTITION, + vec![text_message(1, "first-acked".to_string())], + ) + .await; + assert_eq!( + response.status(), + StatusCode::CREATED, + "the first acked send to a never-minted partition must commit, not be refused" + ); + let polled = http + .poll("http-first-send", "first", ACKED_PARTITION, 0, 10) + .await; + assert_eq!(polled.messages.len(), 1, "the first acked send is durable"); + assert_eq!( + polled.messages[0].payload, + bytes::Bytes::from("first-acked") + ); + + let response = http + .produce_with_query( + "http-first-send", + "first", + UNACKED_PARTITION, + vec![text_message(2, "first-unacked".to_string())], + "?ack=none", + ) + .await; + assert_eq!( + response.status(), + StatusCode::ACCEPTED, + "ack=none must answer before the commit" + ); + + // 202 says nothing about the commit, which is the whole hazard: a refusal + // on this route is answered the same way and leaves no trace. Only the poll + // proves the message survived. + let deadline = Instant::now() + ASYNC_COMMIT_TIMEOUT; + loop { + if let Some(polled) = http + .try_poll("http-first-send", "first", UNACKED_PARTITION, 0, 10) + .await + && !polled.messages.is_empty() + { + assert_eq!(polled.messages.len(), 1, "exactly one message was produced"); + assert_eq!( + polled.messages[0].payload, + bytes::Bytes::from("first-unacked"), + "the first ack=none send to a never-minted partition must not be dropped" + ); + break; + } + assert!( + Instant::now() < deadline, + "the first ack=none send never became pollable within {ASYNC_COMMIT_TIMEOUT:?}" + ); + sleep(ASYNC_COMMIT_RETRY_INTERVAL).await; + } +} + /// End-to-end RBAC proof: an ungranted user is 403 on a metadata read and on a /// data-plane produce, root stays 200/201, and the auth-only cluster-metadata /// route is never gated. Exercises the HTTP per-op gates (read + partition diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index ab16b256fa..a8976bb1e2 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -261,16 +261,6 @@ where /// cost nothing. Kept apart from [`Self::durable_offset_frontier`]: see /// `consensus::VsrState::offset_reserved`. durable_offset_reserved: Cell, - /// A send arrived for a partition with no reservation on disk yet, and was - /// bounced so the shard tick could claim the first block off the request - /// path. Cleared by the write it asks for. - /// - /// The trigger the tick consults is otherwise gated on a partition that has - /// already minted, deliberately: arming every idle partition at boot would - /// write a superblock per partition for nothing. This bit is what separates - /// "idle" from "wanted", so the cost falls only on partitions someone - /// actually produced to. - offset_reservation_wanted: Cell, /// Offsets the append fence claims per superblock write; installed by boot /// from `PartitionsConfig`. offset_reservation_lease: u64, @@ -540,7 +530,6 @@ where purge_deferred: false, durable_offset_frontier: Cell::new(0), durable_offset_reserved: Cell::new(0), - offset_reservation_wanted: Cell::new(false), offset_reservation_lease: u64::from(crate::DEFAULT_OFFSET_RESERVATION_LEASE), transfer: None, transfer_attempts: 0, @@ -1425,19 +1414,15 @@ where /// costs nothing but offset space, while arriving late puts the write back on /// the append path. Floored at 1, since validation admits a lease of 1 and /// `1 / 2` would never trigger, leaving every append to pay the inline claim. - /// A partition that has never minted is skipped unless a send has already - /// been bounced for it (`should_defer_first_reservation`): extending - /// every idle partition at boot would write a superblock per partition for - /// nothing, while a partition someone is producing to needs its first block - /// claimed off the request path like every later one. + /// A partition that has never minted is skipped: extending every idle + /// partition at boot would write a superblock per partition for nothing, and + /// the first block is already claimed where the partition is created, on a + /// path that pays for a superblock write anyway. #[must_use] pub fn needs_offset_reservation_extension(&self) -> bool { if self.consensus.replica_count() > 1 || self.superblock.is_none() { return false; } - if self.offset_reservation_wanted.get() { - return true; - } if !self.offset_space.append_live { return false; } @@ -1448,38 +1433,6 @@ where headroom < (self.offset_reservation_lease / 2).max(1) } - /// Whether this send should be BOUNCED so the shard tick claims the - /// partition's first block, rather than paying for it inline. - /// - /// Without this the first append to every untouched solo partition awaits a - /// create, write, file fsync, rename and directory fsync inside the shard's - /// request pump, where the consensus tick is a sibling arm. A - /// high-cardinality first-write burst serializes those fences and delays - /// unrelated group work and heartbeats on the same core. - /// - /// One retry, once in a partition's life: the bounce is - /// `TransientNotAccepted`, which admitted nothing, and by the time the client - /// re-sends, the tick has claimed the block and the fence takes its fast - /// path. - /// - /// `false` once a claim covers the batch, and `false` for a first batch wider - /// than the whole lease -- the tick's claim would not cover that one either, - /// so bouncing it would bounce the same request forever. - /// - /// `false` with no store attached, for the same reason. A storeless partition - /// (in-memory, simulated) reserves nothing at all, and - /// [`Self::needs_offset_reservation_extension`] skips it, so nothing would - /// ever clear the bounce: every first send would be denied for the life of - /// the partition. The gates here and there must agree on which partitions the - /// tick can serve. - #[must_use] - const fn should_defer_first_reservation(&self, end_offset: u64) -> bool { - self.superblock.is_some() - && !self.offset_space.append_live - && self.durable_offset_reserved.get() <= end_offset - && end_offset < self.offset_reservation_lease - } - /// Extend the reservation a full block past the CEILING already on disk. /// /// Pairs with [`Self::needs_offset_reservation_extension`]; the caller is the @@ -1504,14 +1457,7 @@ where } let _superblock_guard = self.superblock_lock.acquire().await; let ceiling = self.durable_offset_reserved.get().max(self.mint_frontier()); - let written = self.write_claim_from(superblock.as_ref(), ceiling).await; - if written { - // Only on success: a failed write leaves the bounce standing so the - // next tick retries it, rather than dropping the partition back to - // paying inline. - self.offset_reservation_wanted.set(false); - } - written + self.write_claim_from(superblock.as_ref(), ceiling).await } /// Upper bound on the offsets a pending `SendMessages` request will mint, for @@ -1602,6 +1548,9 @@ where /// serving. At the mint the op already has its number and its ack is already /// skipped, so `commit_max` can never pass it and nothing later can commit /// either: `on_replicate` fences the partition there and takes the node down. + /// At CREATE (`build_partition_fresh`) nothing has been externalised at all, + /// so a refusal only drops the partition back to claiming its first block + /// inline on the append path. #[allow(clippy::future_not_send)] #[must_use = "the bool is the fence verdict; dropping it lets the append escape unreserved"] pub async fn reserve_offsets_through(&self, end_offset: u64) -> bool { @@ -1659,9 +1608,8 @@ where /// and must go no further: the client holds a `TransientNotAccepted`, which /// admitted nothing, so it may re-issue anywhere without double-apply risk. /// - /// Three ways to come back `false`, none of them reaching the mint: a bounced - /// first send, an open superblock backoff window, and a claim that was - /// attempted and failed. + /// Two ways to come back `false`, neither reaching the mint: an open + /// superblock backoff window, and a claim that was attempted and failed. /// /// `waiter` is the submit's in-process reply channel, taken only on a /// refusal: the deny goes there because `header.client` is then the VSR @@ -1678,16 +1626,6 @@ where let Some(ceiling) = self.request_mint_ceiling(message) else { return true; }; - if self.should_defer_first_reservation(ceiling) { - self.offset_reservation_wanted.set(true); - self.deny_unreserved_send( - message.header(), - "bouncing a partition's first send so the tick claims its offset block", - waiter.take(), - ) - .await; - return false; - } if !self.reserve_offsets_through_retryable(ceiling).await { self.deny_unreserved_send( message.header(), @@ -6865,78 +6803,24 @@ mod tests { } } - /// The first send to an untouched partition is BOUNCED so the tick claims the - /// block, rather than awaiting a create, write, fsync, rename and directory - /// fsync inside the shard's request pump. - #[compio::test] - async fn given_an_untouched_partition_when_a_send_arrives_should_bounce_it_to_the_tick() { - let store = Rc::new(RecordingSuperblock::default()); - let mut partition = solo_recording_partition(); - partition.set_superblock(store.clone(), None); - partition.set_offset_reservation_lease(test_lease(16)); - - assert!( - partition.should_defer_first_reservation(0), - "a first send with no claim on disk must not pay for one inline" - ); - assert!( - !partition.should_defer_first_reservation(16), - "a first batch wider than the whole lease must not be bounced: the \ - tick's claim would not cover it either, so it would bounce forever" - ); - - // Arming is what the bounce does; the tick then writes, off this path. - partition.offset_reservation_wanted.set(true); - assert!( - partition.needs_offset_reservation_extension(), - "an armed partition needs the write even though it has never minted" - ); - assert!(partition.extend_offset_reservation().await); - assert_eq!(store.attempts.get(), 1); - assert!( - !partition.needs_offset_reservation_extension(), - "the write it asked for disarms it" - ); - - // The retry finds the block already claimed and writes nothing. - assert!(!partition.should_defer_first_reservation(0)); - assert!(partition.reserve_offsets_through(0).await); - assert_eq!( - store.attempts.get(), - 1, - "the bounced send's retry takes the fence's fast path" - ); - } - - /// A storeless partition reserves nothing, and the tick skips it, so a bounce - /// there is a send denied for the life of the partition with nothing able to - /// clear it. The two gates have to agree on which partitions the tick serves. + /// A storeless partition (in-memory, simulated) reserves nothing at all, so + /// the tick must never reach a write for one. #[test] - fn given_a_storeless_partition_when_a_send_arrives_should_not_bounce_it() { + fn given_a_storeless_partition_when_ticking_should_not_extend() { let mut partition = solo_recording_partition(); partition.set_offset_reservation_lease(test_lease(16)); assert!(partition.superblock.is_none(), "the premise: no store"); - assert!(!partition.offset_space.append_live); - - assert!( - !partition.should_defer_first_reservation(0), - "nothing would ever claim the block this bounce waits for" - ); - assert!( - !partition.needs_offset_reservation_extension(), - "and the tick agrees it has nothing to do here" - ); + assert!(!partition.needs_offset_reservation_extension()); } - /// Idle partitions stay idle: arming is what separates a partition someone - /// produced to from one boot merely materialized, and without that a node - /// with many partitions writes a superblock per partition for nothing. + /// Idle partitions stay idle: the first block is claimed where the partition + /// is created, and a node with many partitions must not write a superblock + /// per partition at boot for nothing. #[test] - fn given_an_unarmed_untouched_partition_when_ticking_should_still_not_extend() { + fn given_an_untouched_partition_when_ticking_should_not_extend() { let mut partition = solo_recording_partition(); partition.set_superblock(Rc::new(RecordingSuperblock::default()), None); assert!(!partition.offset_space.append_live); - assert!(!partition.offset_reservation_wanted.get()); assert!(!partition.needs_offset_reservation_extension()); } diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index f04d0d5376..2c88403dd3 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -1116,8 +1116,9 @@ fn hydrate_reopen_error( /// Steps performed (all idempotent on retry after a partial failure): /// 1. Create directory hierarchy on disk. /// 2. Build per-partition VSR consensus group, resuming any superblock-recorded view. -/// 3. Configure empty consumer-offset storage with the on-disk paths set. -/// 4. Provision the initial segment + writers (offset 0). +/// 3. Claim the group's first offset-reservation block (solo groups with a store). +/// 4. Configure empty consumer-offset storage with the on-disk paths set. +/// 5. Provision the initial segment + writers (offset 0). /// /// The namespace arrives packed, so its components are in range by /// construction. Metadata admission is what bounds them. @@ -1296,6 +1297,31 @@ pub async fn build_partition_fresh( // frontier before quarantining, and the boot-path chain refusal to carry // the refused chain's max `end_offset` on its error. partition.restore_offset_frontier(recovered_state.as_ref()); + + // Claim the first offset-reservation block HERE, where this path is already + // paying for a superblock write, so no send ever pays the create, write, + // file fsync, rename and directory fsync of a first claim inline in the + // shard's request pump, where the consensus tick is a sibling arm. No-op + // above one replica and with no store attached, where nothing is reserved, + // and no-op on a rebuild whose record already covers the next mint. + // + // The shard tick takes over from the first mint onward + // (`needs_offset_reservation_extension`), which stays gated on a partition + // that has minted so boot cannot write a superblock per idle partition. + if !partition + .reserve_offsets_through(partition.mint_frontier()) + .await + { + // Degraded, not fatal: the fence on the append path still claims + // inline, so the first send pays for the block instead of the create. + warn!( + stream_id, + topic_id, + partition_id, + "could not claim the partition's first offset reservation; its first send \ + will claim one inline" + ); + } let current_offset = partition.offset.load(Ordering::Acquire); configure_consumer_offsets(&mut partition, config, namespace, current_offset)?; diff --git a/core/simulator/src/lib.rs b/core/simulator/src/lib.rs index 36473b8dc7..3d2fd3feaa 100644 --- a/core/simulator/src/lib.rs +++ b/core/simulator/src/lib.rs @@ -1324,43 +1324,6 @@ impl Simulator { Some(u64::from(partition.consensus().view())) } - /// Claim the first offset block for every live replica's copy of a - /// materialised solo partition, standing in for the shard tick that does it - /// in production. - /// - /// The append fence BOUNCES the first send to a partition with no claim on - /// disk, so the superblock write lands on the tick rather than inside the - /// request pump. A real client retries that `TransientNotAccepted`; the - /// simulator has no retry loop, so a scenario that produces to a freshly - /// materialised partition and expects the send to commit has to claim the - /// block first. - /// - /// Not folded into [`Self::init_partition`]: materialising a partition is not - /// the same event as producing to one, and arming every materialised - /// partition would model a superblock write per idle partition that - /// production deliberately does not make. - /// - /// # Panics - /// If the simulated superblock refuses the claim, which no scenario injects: - /// a silent skip would leave the caller's next send bounced with nothing to - /// say why. - #[allow(clippy::cast_possible_truncation)] - pub fn claim_partition_offset_block(&self, namespace: IggyNamespace) { - for (index, replica) in self.replicas.iter().enumerate() { - if self.crashed.contains(&(index as u8)) { - continue; - } - let shard = replica.partition_shard(namespace); - let Some(partition) = shard.plane.partitions().get_by_ns(&namespace) else { - continue; - }; - assert!( - futures::executor::block_on(partition.extend_offset_reservation()), - "the simulated superblock must accept the first offset claim" - ); - } - } - /// One replica's view of a partition group's consensus, or `None` when it does /// not host the namespace. Read by the quiesce oracle to decide whether a group /// has settled into one view, which its leader-relative checks depend on once @@ -2675,10 +2638,6 @@ mod tests { ); sim.init_partition(namespace); - // The tick's job in production. Without it the append fence bounces this - // first send so the superblock write stays off the request pump, and the - // simulator has no client to retry the bounce. - sim.claim_partition_offset_block(namespace); assert_eq!( shard.redispatched_frame_count(), 1, From c22e79817176a4e779553a450c674f32aa9798c2 Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Fri, 4 Sep 2026 11:21:30 +0200 Subject: [PATCH 3/6] fix(partitions): refuse a create whose first offset claim fails A failed claim only logged, and the partition went live anyway. The failed write arms a 20 ms superblock retry backoff, so the first send inside that window is refused at the admitted path with a transient the HTTP plane does not replay: an acked produce answers 503 and an ack=none produce answers 202 with the message silently dropped. That is the defect the create-time claim was added to remove, reopened in a narrow window. Failing the build instead leaves the namespace unmaterialised, so the reconciler backs off and retries with a partition whose backoff cell starts clear, and produces park until a later pass succeeds. The claim also moves behind consumer-offset configuration and the initial segment, which restores the step list's idempotence: a build that failed after it used to leave the claim on disk and burn another lease block per retry. It is now skipped once the offset space is live, because a rebuild resumes its append point exactly on the reservation it recovered and would otherwise write and burn a block every time. Such a rebuild pays one inline fence on its first send, the same cost a graceful stop and boot already carries. The unit coverage asserted only on the returned partition, which the inline fence at the mint satisfies on its own; it now reads the durable record back. Two comments claimed the fresh-create path already paid for a superblock write, and it did not: before the claim, that path only read the superblock. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/partitions/src/iggy_partition.rs | 13 +- core/server/src/partition_helpers.rs | 177 ++++++++++++++++++-------- core/server/src/server_error.rs | 7 + 3 files changed, 137 insertions(+), 60 deletions(-) diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index a8976bb1e2..69d625579e 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -1416,8 +1416,8 @@ where /// `1 / 2` would never trigger, leaving every append to pay the inline claim. /// A partition that has never minted is skipped: extending every idle /// partition at boot would write a superblock per partition for nothing, and - /// the first block is already claimed where the partition is created, on a - /// path that pays for a superblock write anyway. + /// the first block is already claimed where the partition is created, off + /// the append path. #[must_use] pub fn needs_offset_reservation_extension(&self) -> bool { if self.consensus.replica_count() > 1 || self.superblock.is_none() { @@ -1549,8 +1549,10 @@ where /// skipped, so `commit_max` can never pass it and nothing later can commit /// either: `on_replicate` fences the partition there and takes the node down. /// At CREATE (`build_partition_fresh`) nothing has been externalised at all, - /// so a refusal only drops the partition back to claiming its first block - /// inline on the append path. + /// so a refusal fails the build and leaves the namespace unmaterialised for + /// the reconciler to retry. Going live without the block instead would let + /// the first send land inside the backoff the failed write just armed, where + /// the admitted path refuses it with a transient. #[allow(clippy::future_not_send)] #[must_use = "the bool is the fence verdict; dropping it lets the append escape unreserved"] pub async fn reserve_offsets_through(&self, end_offset: u64) -> bool { @@ -6809,6 +6811,9 @@ mod tests { fn given_a_storeless_partition_when_ticking_should_not_extend() { let mut partition = solo_recording_partition(); partition.set_offset_reservation_lease(test_lease(16)); + // Without this the untouched offset space would satisfy the gate on its + // own and the store check would go untested. + partition.note_append_live(); assert!(partition.superblock.is_none(), "the premise: no store"); assert!(!partition.needs_offset_reservation_extension()); } diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index 2c88403dd3..b55aacf426 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -1116,9 +1116,9 @@ fn hydrate_reopen_error( /// Steps performed (all idempotent on retry after a partial failure): /// 1. Create directory hierarchy on disk. /// 2. Build per-partition VSR consensus group, resuming any superblock-recorded view. -/// 3. Claim the group's first offset-reservation block (solo groups with a store). -/// 4. Configure empty consumer-offset storage with the on-disk paths set. -/// 5. Provision the initial segment + writers (offset 0). +/// 3. Configure empty consumer-offset storage with the on-disk paths set. +/// 4. Provision the initial segment + writers (offset 0). +/// 5. Claim the group's first offset-reservation block (solo groups with a store). /// /// The namespace arrives packed, so its components are in range by /// construction. Metadata admission is what bounds them. @@ -1135,8 +1135,8 @@ fn hydrate_reopen_error( /// /// # Errors /// -/// Returns [`ServerError`] when directory creation, superblock recovery, or -/// segment provisioning fails. +/// Returns [`ServerError`] when directory creation, superblock recovery, +/// segment provisioning, or the first offset-reservation claim fails. #[allow(clippy::too_many_arguments)] pub async fn build_partition_fresh( config: &ServerConfig, @@ -1298,34 +1298,42 @@ pub async fn build_partition_fresh( // the refused chain's max `end_offset` on its error. partition.restore_offset_frontier(recovered_state.as_ref()); - // Claim the first offset-reservation block HERE, where this path is already - // paying for a superblock write, so no send ever pays the create, write, - // file fsync, rename and directory fsync of a first claim inline in the - // shard's request pump, where the consensus tick is a sibling arm. No-op - // above one replica and with no store attached, where nothing is reserved, - // and no-op on a rebuild whose record already covers the next mint. + let current_offset = partition.offset.load(Ordering::Acquire); + + configure_consumer_offsets(&mut partition, config, namespace, current_offset)?; + ensure_initial_segment(&mut partition, config, stream_id, topic_id, partition_id).await?; + + // Claim the first offset-reservation block HERE so no send ever pays the + // create, write, file fsync, rename and directory fsync of a first claim + // inline in the shard's request pump, where the consensus tick is a sibling + // arm. It is a NEW write on a path that otherwise only READS the superblock: + // one atomic replace per created partition, serialised with its siblings in + // the reconciler's addition loop, so it lengthens the window a produce + // arriving with the create spends parked. + // + // LAST of the steps, so the rest stay idempotent on retry: a failure between + // the claim and the return would burn a lease block per reconciler pass. + // + // No-op above one replica and with no store attached, where nothing is + // reserved. Skipped once the offset space is live (`mint_frontier` reads 0 + // only while it is not): a rebuild resumes its append point exactly ON the + // reservation it recovered, never above it, so an unconditional claim would + // write and burn a block every time. It pays one inline fence on its first + // send instead, which is what a graceful stop and boot already costs. // // The shard tick takes over from the first mint onward // (`needs_offset_reservation_extension`), which stays gated on a partition // that has minted so boot cannot write a superblock per idle partition. - if !partition - .reserve_offsets_through(partition.mint_frontier()) - .await - { - // Degraded, not fatal: the fence on the append path still claims - // inline, so the first send pays for the block instead of the create. - warn!( - stream_id, - topic_id, - partition_id, - "could not claim the partition's first offset reservation; its first send \ - will claim one inline" - ); + if partition.mint_frontier() == 0 && !partition.reserve_offsets_through(0).await { + // Not degraded-but-live: the failed write armed the group's superblock + // retry backoff, and `reserve_offsets_through_retryable` refuses every + // send arriving inside it with a transient the HTTP plane does not + // replay. The reconciler backs the namespace off and retries with a + // fresh partition, whose backoff cell starts clear. + return Err(ServerError::PartitionOffsetReservationClaim { + namespace_raw: namespace.inner(), + }); } - let current_offset = partition.offset.load(Ordering::Acquire); - - configure_consumer_offsets(&mut partition, config, namespace, current_offset)?; - ensure_initial_segment(&mut partition, config, stream_id, topic_id, partition_id).await?; Ok(partition) } @@ -1435,6 +1443,78 @@ mod tests { } } + /// The offset reservation is solo-only, so every test that touches it builds + /// under this identity. + const fn solo_identity() -> ReplicaIdentity { + ReplicaIdentity { + cluster: CLUSTER, + replica_id: 0, + replica_count: 1, + } + } + + fn solo_config(root: &tempfile::TempDir) -> ServerConfig { + ServerConfig { + system: Arc::new(ServerSystemConfig { + path: root.path().to_string_lossy().into_owned(), + ..ServerSystemConfig::default() + }), + ..ServerConfig::default() + } + } + + async fn build_solo_partition( + config: &ServerConfig, + ) -> Result>, ServerError> { + build_partition_fresh( + config, + IggyNamespace::new(1, 1, 0), + Arc::new(PartitionStats::default()), + 0, + TopicRuntimeOptions::default(), + CLUSTER, + 0, + 1, + 0, + Rc::new(IggyMessageBus::new(0)), + ) + .await + } + + /// The reservation the partition left on disk, which is the only copy a + /// restart or a first send can read. + async fn recorded_reservation(dir: &str) -> u64 { + let (_store, recorded) = open_partition_superblock(dir, solo_identity()) + .await + .expect("reopen the partition superblock"); + recorded + .expect("a partition that recorded a reservation") + .offset_reserved + } + + /// The create claims the first lease block, so the DURABLE record covers the + /// first send before it arrives. Asserting on the returned partition alone + /// would pass with no claim at all: the inline fence at the mint writes the + /// same block on the first send, which is exactly what this moves off the + /// append path. + #[compio::test] + async fn given_a_fresh_solo_partition_when_building_should_record_its_first_claim() { + let root = tempfile::tempdir().expect("tempdir"); + let config = solo_config(&root); + let dir = config.system.get_partition_path(1, 1, 0); + + let partition = build_solo_partition(&config) + .await + .expect("build a fresh partition"); + drop(partition); + + assert_eq!( + recorded_reservation(&dir).await, + 1 + u64::from(config.partition.offset_reservation_lease.get()), + "the create must leave a full lease block covering offset 0 on disk" + ); + } + /// A rebuild that reads a reservation back must NAME its planted segment for /// the append point. Named 0, the segment takes the first append's /// `base_offset` of N instead, `rposition(|s| s.start_offset <= offset)` @@ -1444,22 +1524,10 @@ mod tests { async fn given_a_recorded_reservation_when_building_fresh_should_plant_at_the_append_point() { const RESERVED: u64 = 65_537; let root = tempfile::tempdir().expect("tempdir"); - let config = ServerConfig { - system: Arc::new(ServerSystemConfig { - path: root.path().to_string_lossy().into_owned(), - ..ServerSystemConfig::default() - }), - ..ServerConfig::default() - }; - let namespace = IggyNamespace::new(1, 1, 0); + let config = solo_config(&root); let dir = config.system.get_partition_path(1, 1, 0); - let identity = ReplicaIdentity { - cluster: CLUSTER, - replica_id: 0, - replica_count: 1, - }; - let (store, recovered) = open_partition_superblock(&dir, identity) + let (store, recovered) = open_partition_superblock(&dir, solo_identity()) .await .expect("open a fresh partition superblock"); assert!(recovered.is_none()); @@ -1469,20 +1537,9 @@ mod tests { .expect("record the reservation"); drop(store); - let partition = build_partition_fresh( - &config, - namespace, - Arc::new(PartitionStats::default()), - 0, - TopicRuntimeOptions::default(), - CLUSTER, - 0, - 1, - 0, - Rc::new(IggyMessageBus::new(0)), - ) - .await - .expect("rebuild the partition over its recorded reservation"); + let partition = build_solo_partition(&config) + .await + .expect("rebuild the partition over its recorded reservation"); assert_eq!( partition.mint_frontier(), @@ -1510,6 +1567,14 @@ mod tests { vec![format!("{RESERVED:0>20}.log")], "the initial segment must be named for the append point, not offset 0" ); + + drop(partition); + assert_eq!( + recorded_reservation(&dir).await, + RESERVED, + "a rebuild resumes ON its recorded reservation, so re-claiming here would \ + burn a lease block and two fsyncs per rebuild" + ); } #[compio::test] diff --git a/core/server/src/server_error.rs b/core/server/src/server_error.rs index 7f0f86c5e3..dee1fa0ace 100644 --- a/core/server/src/server_error.rs +++ b/core/server/src/server_error.rs @@ -201,6 +201,13 @@ pub enum ServerError { partition_id: usize, reason: PartitionRecoveryRefusal, }, + /// Fails the create rather than letting the partition go live without its + /// first reservation: the failed write arms the group's superblock retry + /// backoff, and a send arriving inside that window is refused with a + /// transient the HTTP plane does not replay. `namespace_raw` joins this to + /// the write's own `iggy.partitions.diag` line, which carries the cause. + #[error("partition namespace {namespace_raw} could not claim its first offset reservation")] + PartitionOffsetReservationClaim { namespace_raw: u64 }, #[error( "shard {shard_id} aborted while waiting for shard-0 to broadcast the metadata \ factory bundle; shard 0 dropped its sender (most likely it failed to recover)" From 71a5672b19cc9a498dc95b0f1d2878a51a210605 Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Fri, 4 Sep 2026 11:35:41 +0200 Subject: [PATCH 4/6] refactor(partitions): drop the redundant first-claim gate The create-time offset claim was guarded by `mint_frontier() == 0` so it would skip a fenced rebuild. That guard never decided anything: `reserve_offsets_through(0)` returns on its own coverage check, and a record with a live offset space always carries a reservation above zero, so the skip came from the argument and not the gate. Deleting it leaves one untested condition fewer and the same behaviour. Three comments beside it described a retry that cannot happen. A create that fails leaves its directory behind, and the reconciler routes any namespace with a directory to the loader, so nothing re-enters this builder. The claim still belongs last, but for the reason those comments missed: a reservation written before a failing step outlives the create, and the loader then resumes the append point at it, holing every offset below on a partition that never took a write. The claim failure also rendered as a packed namespace integer where its neighbours spell the stream, topic and partition. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/partitions/src/iggy_partition.rs | 4 ++-- core/server/src/partition_helpers.rs | 33 +++++++++++++++++---------- core/server/src/server_error.rs | 12 ++++++++-- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index 69d625579e..94c577ad6b 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -6811,8 +6811,8 @@ mod tests { fn given_a_storeless_partition_when_ticking_should_not_extend() { let mut partition = solo_recording_partition(); partition.set_offset_reservation_lease(test_lease(16)); - // Without this the untouched offset space would satisfy the gate on its - // own and the store check would go untested. + // Without this the assert would pass on `!append_live` alone, leaving + // the store check it is named for untested. partition.note_append_live(); assert!(partition.superblock.is_none(), "the premise: no store"); assert!(!partition.needs_offset_reservation_extension()); diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index b55aacf426..0ee71dadc2 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -1113,7 +1113,8 @@ fn hydrate_reopen_error( /// already on disk is routed through the loader instead, so a prior /// life's segments are hydrated rather than built over. /// -/// Steps performed (all idempotent on retry after a partial failure): +/// Steps performed. 1 to 4 are idempotent on retry after a partial failure; the +/// claim is last precisely because it is not (see its own comment): /// 1. Create directory hierarchy on disk. /// 2. Build per-partition VSR consensus group, resuming any superblock-recorded view. /// 3. Configure empty consumer-offset storage with the on-disk paths set. @@ -1311,26 +1312,34 @@ pub async fn build_partition_fresh( // the reconciler's addition loop, so it lengthens the window a produce // arriving with the create spends parked. // - // LAST of the steps, so the rest stay idempotent on retry: a failure between - // the claim and the return would burn a lease block per reconciler pass. + // LAST of the steps, because a claim written before a step that then fails + // outlives the create. The reconciler routes any namespace whose directory + // exists to `load_partition_or_fence`, and step 1 made that directory, so + // the retry comes back through the loader: `restore_offset_frontier` there + // resumes the append point at the recorded reservation and holes every + // offset below it on a partition that never took a write. // - // No-op above one replica and with no store attached, where nothing is - // reserved. Skipped once the offset space is live (`mint_frontier` reads 0 - // only while it is not): a rebuild resumes its append point exactly ON the - // reservation it recovered, never above it, so an unconditional claim would - // write and burn a block every time. It pays one inline fence on its first - // send instead, which is what a graceful stop and boot already costs. + // `0`, not `mint_frontier()`: a rebuild recovers its append point exactly ON + // the reservation it recorded, so asking to cover the frontier would fail + // the callee's strict `>` and rewrite the record on every rebuild. Asking + // only for offset 0 leaves that same check to skip every partition already + // carrying a reservation, which pays one inline fence on its first send + // instead, the cost a graceful stop and boot already carries. No-op above + // one replica and with no store attached, where nothing is reserved. // // The shard tick takes over from the first mint onward // (`needs_offset_reservation_extension`), which stays gated on a partition // that has minted so boot cannot write a superblock per idle partition. - if partition.mint_frontier() == 0 && !partition.reserve_offsets_through(0).await { + if !partition.reserve_offsets_through(0).await { // Not degraded-but-live: the failed write armed the group's superblock // retry backoff, and `reserve_offsets_through_retryable` refuses every // send arriving inside it with a transient the HTTP plane does not - // replay. The reconciler backs the namespace off and retries with a - // fresh partition, whose backoff cell starts clear. + // replay. The reconciler backs the namespace off; the retry materialises + // through the loader, whose partition carries a clear backoff cell. return Err(ServerError::PartitionOffsetReservationClaim { + stream_id, + topic_id, + partition_id, namespace_raw: namespace.inner(), }); } diff --git a/core/server/src/server_error.rs b/core/server/src/server_error.rs index dee1fa0ace..b9e744755c 100644 --- a/core/server/src/server_error.rs +++ b/core/server/src/server_error.rs @@ -206,8 +206,16 @@ pub enum ServerError { /// backoff, and a send arriving inside that window is refused with a /// transient the HTTP plane does not replay. `namespace_raw` joins this to /// the write's own `iggy.partitions.diag` line, which carries the cause. - #[error("partition namespace {namespace_raw} could not claim its first offset reservation")] - PartitionOffsetReservationClaim { namespace_raw: u64 }, + #[error( + "partition {stream_id}/{topic_id}/{partition_id} (namespace {namespace_raw}) could not \ + claim its first offset reservation" + )] + PartitionOffsetReservationClaim { + stream_id: usize, + topic_id: usize, + partition_id: usize, + namespace_raw: u64, + }, #[error( "shard {shard_id} aborted while waiting for shard-0 to broadcast the metadata \ factory bundle; shard 0 dropped its sender (most likely it failed to recover)" From f20c4d316e8ab993ac701636d559d3cff9b38c82 Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Fri, 4 Sep 2026 12:03:35 +0200 Subject: [PATCH 5/6] fix(partitions): fence one partition on a refused first claim The create-time offset claim was written for the reconciler, which records the failure and backs the namespace off. It has a second caller: the loader's fence-and-rebuild arm, reached for a solo group whose segment chain is refused with nothing recoverable in it. Boot calls that loader with `?`, so a superblock write failure there aborted the whole shard's start over one partition. Tombstone it there instead, like the sibling failure arms. That is also the only safe answer: the failed build has already quarantined the chain and planted an empty segment 0, which the next load would accept and serve as a healthy empty partition, hiding exactly the loss the fence surfaces. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/partitions/src/iggy_partition.rs | 30 ++----- core/server/src/partition_helpers.rs | 116 ++++++++++++++++++++++++-- 2 files changed, 118 insertions(+), 28 deletions(-) diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index 94c577ad6b..2c31d4c6c6 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -1549,10 +1549,10 @@ where /// skipped, so `commit_max` can never pass it and nothing later can commit /// either: `on_replicate` fences the partition there and takes the node down. /// At CREATE (`build_partition_fresh`) nothing has been externalised at all, - /// so a refusal fails the build and leaves the namespace unmaterialised for - /// the reconciler to retry. Going live without the block instead would let - /// the first send land inside the backoff the failed write just armed, where - /// the admitted path refuses it with a transient. + /// so a refusal fails the build and leaves the namespace unmaterialised: the + /// reconciler retries it, the loader tombstones it. Going live without the + /// block instead would let the first send land inside the backoff the failed + /// write just armed, where the admitted path refuses it with a transient. #[allow(clippy::future_not_send)] #[must_use = "the bool is the fence verdict; dropping it lets the append escape unreserved"] pub async fn reserve_offsets_through(&self, end_offset: u64) -> bool { @@ -1629,12 +1629,8 @@ where return true; }; if !self.reserve_offsets_through_retryable(ceiling).await { - self.deny_unreserved_send( - message.header(), - "refusing a send: the offset reservation could not be extended", - waiter.take(), - ) - .await; + self.deny_unreserved_send(message.header(), waiter.take()) + .await; return false; } true @@ -1652,7 +1648,6 @@ where async fn deny_unreserved_send( &self, header: &RoutedRequestHeader, - reason: &'static str, waiter: Option>>, ) { let consensus = self.consensus(); @@ -1660,7 +1655,7 @@ where tracing::Level::WARN, &PartitionDiagEvent::new( ReplicaLogContext::from_consensus(consensus, PlaneKind::Partitions), - reason, + "refusing a send: the offset reservation could not be extended", ) .with_operation(Operation::SendMessages), ); @@ -6818,17 +6813,6 @@ mod tests { assert!(!partition.needs_offset_reservation_extension()); } - /// Idle partitions stay idle: the first block is claimed where the partition - /// is created, and a node with many partitions must not write a superblock - /// per partition at boot for nothing. - #[test] - fn given_an_untouched_partition_when_ticking_should_not_extend() { - let mut partition = solo_recording_partition(); - partition.set_superblock(Rc::new(RecordingSuperblock::default()), None); - assert!(!partition.offset_space.append_live); - assert!(!partition.needs_offset_reservation_extension()); - } - /// Inside an open backoff window the ADMITTED path refuses without touching /// the disk the last writer just found broken. Every producer retry otherwise /// re-runs a full atomic replace, which starves the shard pump for as long as diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index 0ee71dadc2..90180df293 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -676,10 +676,10 @@ pub async fn load_partition_or_fence( return Ok(None); } } - Box::pin(build_partition_fresh( + match Box::pin(build_partition_fresh( config, namespace, - partition_stats, + Arc::clone(&partition_stats), partition_metadata.created_revision, topic_runtime, cluster_id, @@ -689,7 +689,31 @@ pub async fn load_partition_or_fence( Rc::clone(&bus), )) .await - .map(Some) + { + Ok(partition) => Ok(Some(partition)), + // Boot propagates whatever this returns, so an `Err` here costs a + // whole shard its start over ONE partition's failed write. + // Tombstoning is also the only safe answer: the failed build + // already quarantined the chain and planted an empty segment 0, + // which the next load would accept and serve as a healthy empty + // partition. The stats lose the segment that build counted before + // the claim refused. + Err(error @ ServerError::PartitionOffsetReservationClaim { .. }) => { + error!( + stream_id, + topic_id, + partition_id = partition_metadata.id, + partition_dir, + %error, + "failed to claim the rebuilt partition's first offset reservation; \ + leaving it tombstoned rather than serving it unreserved" + ); + partition_stats.zero_out_all(); + partitions.tombstone(namespace); + Ok(None) + } + Err(error) => Err(error), + } } // An untrustworthy superblock fences ONE group, not the node. The // segment files stay exactly where they are -- unlike a refused @@ -1334,8 +1358,11 @@ pub async fn build_partition_fresh( // Not degraded-but-live: the failed write armed the group's superblock // retry backoff, and `reserve_offsets_through_retryable` refuses every // send arriving inside it with a transient the HTTP plane does not - // replay. The reconciler backs the namespace off; the retry materialises - // through the loader, whose partition carries a clear backoff cell. + // replay. Both callers absorb this without escalating: the reconciler + // backs the namespace off and its retry materialises through the loader, + // whose partition carries a clear backoff cell, while the loader's own + // fence-and-rebuild arm tombstones the partition, since boot propagates + // anything it returns. return Err(ServerError::PartitionOffsetReservationClaim { stream_id, topic_id, @@ -1408,6 +1435,8 @@ mod tests { use super::*; use configs::server::ServerSystemConfig; use journal::superblock::SuperblockStore; + use partitions::PartitionPathLayout; + use server_common::sharding::ShardId; const CLUSTER: u128 = 7; const REPLICA: u8 = 1; @@ -1490,6 +1519,24 @@ mod tests { .await } + /// The container the loader tombstones into. Its config is never read on the + /// paths under test, which stop before `restore_partition_offsets`. + fn solo_partitions() -> IggyPartitions> { + IggyPartitions::new( + ShardId::new(0), + PartitionsConfig { + messages_required_to_save: 1, + size_of_messages_required_to_save: IggyByteSize::from(1024_u64), + enforce_fsync: false, + validate_checksum: true, + segment_size: IggyByteSize::from(1_048_576_u64), + preallocate_segments: false, + encryptor: None, + path_layout: PartitionPathLayout::default(), + }, + ) + } + /// The reservation the partition left on disk, which is the only copy a /// restart or a first send can read. async fn recorded_reservation(dir: &str) -> u64 { @@ -1586,6 +1633,65 @@ mod tests { ); } + /// The loader's fence-and-rebuild arm is the claim's SECOND caller, and boot + /// propagates whatever the loader returns: an `Err` here costs the shard its + /// whole start over one partition's failed write. + #[compio::test] + async fn given_a_failing_claim_when_rebuilding_a_fenced_chain_should_tombstone_the_partition() { + let root = tempfile::tempdir().expect("tempdir"); + let config = solo_config(&root); + let namespace = IggyNamespace::new(1, 1, 0); + let dir = config.system.get_partition_path(1, 1, 0); + std::fs::create_dir_all(&dir).expect("partition dir"); + // Two empty segments make the first a NON-tail empty, the refusal a solo + // group rebuilds through (zero recoverable bytes) instead of tombstoning + // where it stands. + for start_offset in [0, 1] { + std::fs::File::create(config.system.get_messages_file_path(1, 1, 0, start_offset)) + .expect("empty segment log"); + } + // The rebuild's claim is this group's first superblock write, so it + // targets slot A. A directory where its temp file goes fails the atomic + // replace and nothing else: the slot reads still find the store empty, + // and the quarantine moves segment files only. + std::fs::create_dir(Path::new(&dir).join("superblock.a.tmp")).expect("block slot A"); + + let stats = Arc::new(PartitionStats::default()); + let partitions = solo_partitions(); + let loaded = load_partition_or_fence( + &config, + namespace, + Arc::clone(&stats), + &Partition::new(0, namespace.inner(), IggyTimestamp::now(), 0, 0), + TopicRuntimeOptions::default(), + CLUSTER, + 0, + 1, + Rc::new(IggyMessageBus::new(0)), + &partitions, + ) + .await; + + match loaded { + Ok(None) => {} + Ok(Some(_)) => panic!("the planted directory must fail the rebuild's claim"), + Err(error) => panic!("a refused claim must fence one partition, not boot: {error}"), + } + assert!( + std::fs::metadata(format!("{dir}.fenced.0")).is_ok(), + "the quarantine must have run, or this asserts on the wrong arm" + ); + assert!( + partitions.is_tombstoned(&namespace), + "an unreserved partition must stay unrouted" + ); + assert_eq!( + stats.segments_count_inconsistent(), + 0, + "the rebuild counted its initial segment before the claim refused" + ); + } + #[compio::test] async fn given_fresh_partition_dir_when_superblock_opened_should_yield_no_state() { let root = tempfile::tempdir().expect("tempdir"); From 512c9dc35387ab0738617b2411e361bbf02236fe Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Fri, 4 Sep 2026 12:33:54 +0200 Subject: [PATCH 6/6] fix(partitions): retry a refused first claim instead of tombstoning The loader absorbed a refused create-time offset claim by tombstoning the namespace, and a tombstone is permanent: it keeps the reconciler away for the life of the process, and only an operator delete lifts it. A superblock write that failed once is not that kind of verdict. The identical fault on the fresh-create path merely backs the namespace off and retries. Let the refusal propagate out of the loader and absorb it at the only caller that cannot survive an error. Boot now skips that one partition and leaves it to the reconciler's addition pass, which does reach it: the pass targets every committed partition, and a namespace boot left unmaterialised and untombstoned falls through to the loader again on the first tick. The build also counted its initial segment into the parent topic before the claim ran, so a refused create leaked a segment the retry counts a second time while recovering it. Zeroing the partition's stats before the refusal covers both callers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/integration/tests/server/http_vsr.rs | 23 +++++--- core/partitions/src/iggy_partition.rs | 4 +- core/server/src/boot/recovery.rs | 27 ++++++++-- core/server/src/partition_helpers.rs | 64 +++++++++-------------- 4 files changed, 64 insertions(+), 54 deletions(-) diff --git a/core/integration/tests/server/http_vsr.rs b/core/integration/tests/server/http_vsr.rs index c97be5a65c..0438937154 100644 --- a/core/integration/tests/server/http_vsr.rs +++ b/core/integration/tests/server/http_vsr.rs @@ -743,13 +743,22 @@ async fn given_ack_none_when_producing_should_return_202_and_commit(harness: &Te /// returns `None` above one replica, so this suite's three-node default leaves /// the whole reservation path as dead code and proves nothing here. /// -/// The reservation writes the partition's superblock before it hands out an -/// offset, and a first send is where that claim is missing. Neither HTTP route -/// can carry a retryable refusal back to the caller: the acked route has no -/// transient replay loop, and `?ack=none` never reads a reply at all, so a -/// refusal there would answer 202 and drop the message. Both partitions are -/// produced to exactly once, so a per-partition regression cannot hide behind a -/// second send. +/// What it guards is the BOUNCE REMOVAL. A first send used to be answered +/// `TransientNotAccepted` so the shard tick would claim the block off the +/// request pump, and neither HTTP route can carry a retryable refusal back to +/// the caller: the acked route has no transient replay loop, and `?ack=none` +/// never reads a reply at all, so that bounce answered 202 and dropped the +/// message. +/// +/// It is NOT sensitive to where the claim is taken. With the bounce gone the +/// inline fence at the mint writes the same block on the first send, so both +/// routes still commit with the create-time claim deleted; +/// `given_a_fresh_solo_partition_when_building_should_record_its_first_claim` +/// in `core/server/src/partition_helpers.rs` reads the durable record and is +/// the test that fails without it. +/// +/// Both partitions are produced to exactly once, so a per-partition regression +/// cannot hide behind a second send. #[iggy_harness(cluster_nodes = 1)] async fn given_a_solo_topic_when_producing_its_first_http_messages_should_commit_them( harness: &TestHarness, diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index 2c31d4c6c6..19db69e4c0 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -1549,8 +1549,8 @@ where /// skipped, so `commit_max` can never pass it and nothing later can commit /// either: `on_replicate` fences the partition there and takes the node down. /// At CREATE (`build_partition_fresh`) nothing has been externalised at all, - /// so a refusal fails the build and leaves the namespace unmaterialised: the - /// reconciler retries it, the loader tombstones it. Going live without the + /// so a refusal fails the build and leaves the namespace unmaterialised for + /// the reconciler to retry, boot included. Going live without the /// block instead would let the first send land inside the backoff the failed /// write just armed, where the admitted path refuses it with a transient. #[allow(clippy::future_not_send)] diff --git a/core/server/src/boot/recovery.rs b/core/server/src/boot/recovery.rs index 808eefe4aa..eb99734d69 100644 --- a/core/server/src/boot/recovery.rs +++ b/core/server/src/boot/recovery.rs @@ -50,7 +50,7 @@ use std::cell::RefCell; use std::rc::Rc; use std::sync::Arc; use std::time::Duration; -use tracing::{info, warn}; +use tracing::{error, info, warn}; #[allow(clippy::too_many_arguments, clippy::too_many_lines)] pub(in crate::boot) async fn build_shard_for_thread( @@ -172,7 +172,7 @@ pub(in crate::boot) async fn build_shard_for_thread( // `Arc` atomics race only against other atomic adds. for (stream_id, topic_id, partition_stats, partition_metadata, topic_runtime) in owned { let namespace = IggyNamespace::new(stream_id, topic_id, partition_metadata.id); - let Some(partition) = load_partition_or_fence( + let loaded = load_partition_or_fence( config, namespace, partition_stats, @@ -184,9 +184,26 @@ pub(in crate::boot) async fn build_shard_for_thread( Rc::clone(&bus), &partitions, ) - .await? - else { - continue; + .await; + let partition = match loaded { + Ok(Some(partition)) => partition, + Ok(None) => continue, + // A refused claim is a failed superblock write, not damage: the + // namespace stays materialisable, so skipping it here costs one + // partition its start instead of the whole shard, and the + // reconciler's addition pass retries it within a tick. + Err(error @ ServerError::PartitionOffsetReservationClaim { .. }) => { + error!( + stream_id, + topic_id, + partition_id = partition_metadata.id, + %error, + "skipping this partition at boot; the reconciler retries its first \ + offset reservation claim" + ); + continue; + } + Err(error) => return Err(error), }; partitions.insert(namespace, partition); shards_table.insert( diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index 90180df293..15a519a223 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -676,7 +676,7 @@ pub async fn load_partition_or_fence( return Ok(None); } } - match Box::pin(build_partition_fresh( + Box::pin(build_partition_fresh( config, namespace, Arc::clone(&partition_stats), @@ -689,31 +689,7 @@ pub async fn load_partition_or_fence( Rc::clone(&bus), )) .await - { - Ok(partition) => Ok(Some(partition)), - // Boot propagates whatever this returns, so an `Err` here costs a - // whole shard its start over ONE partition's failed write. - // Tombstoning is also the only safe answer: the failed build - // already quarantined the chain and planted an empty segment 0, - // which the next load would accept and serve as a healthy empty - // partition. The stats lose the segment that build counted before - // the claim refused. - Err(error @ ServerError::PartitionOffsetReservationClaim { .. }) => { - error!( - stream_id, - topic_id, - partition_id = partition_metadata.id, - partition_dir, - %error, - "failed to claim the rebuilt partition's first offset reservation; \ - leaving it tombstoned rather than serving it unreserved" - ); - partition_stats.zero_out_all(); - partitions.tombstone(namespace); - Ok(None) - } - Err(error) => Err(error), - } + .map(Some) } // An untrustworthy superblock fences ONE group, not the node. The // segment files stay exactly where they are -- unlike a refused @@ -1358,11 +1334,15 @@ pub async fn build_partition_fresh( // Not degraded-but-live: the failed write armed the group's superblock // retry backoff, and `reserve_offsets_through_retryable` refuses every // send arriving inside it with a transient the HTTP plane does not - // replay. Both callers absorb this without escalating: the reconciler - // backs the namespace off and its retry materialises through the loader, - // whose partition carries a clear backoff cell, while the loader's own - // fence-and-rebuild arm tombstones the partition, since boot propagates - // anything it returns. + // replay. Neither caller escalates: the reconciler backs the namespace + // off and its retry materialises through the loader, whose partition + // carries a clear backoff cell, and boot skips the partition, leaving it + // to that same retry. + // + // `ensure_initial_segment` folded its segment into the parent topic + // before the claim ran, and the retry counts the same file again while + // recovering it. + partition.stats.zero_out_all(); return Err(ServerError::PartitionOffsetReservationClaim { stream_id, topic_id, @@ -1633,11 +1613,13 @@ mod tests { ); } - /// The loader's fence-and-rebuild arm is the claim's SECOND caller, and boot - /// propagates whatever the loader returns: an `Err` here costs the shard its - /// whole start over one partition's failed write. + /// The loader's fence-and-rebuild arm is the claim's SECOND caller. A + /// refused claim is a failed superblock write, not damage, so it has to come + /// back as an error every caller can retry: absorbing it into a tombstone + /// here would darken the namespace for the life of the process over a fault + /// the next attempt may not even hit. #[compio::test] - async fn given_a_failing_claim_when_rebuilding_a_fenced_chain_should_tombstone_the_partition() { + async fn given_a_failing_claim_when_rebuilding_a_fenced_chain_should_refuse_not_tombstone() { let root = tempfile::tempdir().expect("tempdir"); let config = solo_config(&root); let namespace = IggyNamespace::new(1, 1, 0); @@ -1673,22 +1655,24 @@ mod tests { .await; match loaded { - Ok(None) => {} + Err(ServerError::PartitionOffsetReservationClaim { .. }) => {} + Err(other) => panic!("expected the claim's own refusal, got {other}"), Ok(Some(_)) => panic!("the planted directory must fail the rebuild's claim"), - Err(error) => panic!("a refused claim must fence one partition, not boot: {error}"), + Ok(None) => panic!("a refused claim must reach the caller, not be absorbed here"), } assert!( std::fs::metadata(format!("{dir}.fenced.0")).is_ok(), "the quarantine must have run, or this asserts on the wrong arm" ); assert!( - partitions.is_tombstoned(&namespace), - "an unreserved partition must stay unrouted" + !partitions.is_tombstoned(&namespace), + "a transient write failure must leave the namespace materialisable" ); assert_eq!( stats.segments_count_inconsistent(), 0, - "the rebuild counted its initial segment before the claim refused" + "the rebuild counted its initial segment before the claim refused, and the \ + retry counts the same file again" ); }