From ee26845d87185238ef41c01c0f84823456255edd Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Tue, 23 Jun 2026 20:20:26 -0700 Subject: [PATCH 1/2] feat(fence): single-attach generation fence (closes split-brain data-loss gap) Under a network partition, instd's node-death recovery can let two nodes attach and write the same GlideFS volume at once -> split-brain -> data loss. A node-lease TTL provably cannot prevent it (a partitioned-but-alive predecessor's lease lapses and a replacement re-forks and re-attaches); only the storage layer can authoritatively stop two writers. This is the GlideFS half of the recovery fix (formal spec 12-recovery-formal-spec.md S6.1, Inv1). The fence is a monotonic generation (the orchestrator's per-instance placement generation) checked at the resource with the rule `my_gen >= stored_gen => Grant` (mirrors instd recovery_kernel::attach_fence verbatim; `>=` admits an idempotent self re-attach, only a strictly newer generation fences an older holder out). gen 0 = back-compat bypass, so existing callers that send no generation are never fenced. Design (manifest-anchored, single source of truth, reuses the existing If-Match manifest CAS -- no new primitive): - block/fence.rs: the pure rule + gen-0 bypass wrapper, unit-tested. - volume_manifest.rs: generation carried as a zero-churn trailer (v7 = generation only, v8 = prefetch+generation), emitted only when > 0 so un-fenced manifests stay byte-identical to v5/v6. - router::enforce_attach_fence + api: ATTACH-time fence. On grant the winner SEIZES the volume by stamping its generation into the manifest via the If-Match CAS, before serving any I/O (a rejected attach -> HTTP 409 with zero packs uploaded, so nothing orphans). The load-bearing half. - write_cache flush/inner/write/error: SYNC-time fence. The generation is stamped on every manifest sync; on a precondition failure the writer re-reads and, if a strictly-newer generation owns the volume, returns a terminal CacheError::Fenced -- the flush scheduler stops and the write path latches closed so the guest stops getting ACKs for writes that can never durably commit. Tested (511 lib tests pass with --features ublk): the pure >= rule incl. equal-gen re-attach and the gen-0 bypass; v5/v7/v8 manifest round-trips (zero churn proven byte-for-byte); a two-router shared-object-store split-brain integration test (stale gen -> 409, newer gen wins, superseded owner's re-attach is fenced); and a write_cache test proving a superseded incumbent is rejected at sync and its write path latches closed. Co-Authored-By: Claude Opus 4.8 (1M context) --- glidefs/src/block/api.rs | 184 +++++++++++++++++++++++++ glidefs/src/block/fence.rs | 106 ++++++++++++++ glidefs/src/block/mod.rs | 1 + glidefs/src/block/router.rs | 105 ++++++++++++++ glidefs/src/block/volume_manifest.rs | 154 ++++++++++++++++++--- glidefs/src/block/write_cache/error.rs | 16 ++- glidefs/src/block/write_cache/flush.rs | 51 ++++++- glidefs/src/block/write_cache/init.rs | 4 + glidefs/src/block/write_cache/inner.rs | 16 +++ glidefs/src/block/write_cache/tests.rs | 62 +++++++++ glidefs/src/block/write_cache/write.rs | 15 ++ 11 files changed, 692 insertions(+), 22 deletions(-) create mode 100644 glidefs/src/block/fence.rs diff --git a/glidefs/src/block/api.rs b/glidefs/src/block/api.rs index afd6a21..755eb57 100644 --- a/glidefs/src/block/api.rs +++ b/glidefs/src/block/api.rs @@ -57,6 +57,13 @@ pub struct CreateVolumeRequest { /// cuts S3 PUT write-amp on overwrite-heavy DB volumes. Typical value: 8. #[serde(default)] pub compaction_cooldown: Option, + /// Single-attach fencing token (the orchestrator's monotonic per-instance + /// placement generation). When `> 0`, the attach is admitted only if this + /// generation is `>=` the volume's stored generation; a strictly-newer + /// generation fences an older holder out (see [`crate::block::fence`]). + /// Omitted / `0` = the caller does not participate in fencing (back-compat). + #[serde(default)] + pub generation: Option, } /// Optional request body for POST /api/exports/{name}/snapshot. @@ -93,6 +100,10 @@ pub struct ExportInfoResponse { /// Filesystem used bytes from ext4 superblock (None if not ext4 or read failed). #[serde(skip_serializing_if = "Option::is_none")] pub fs_used_bytes: Option, + /// Current single-attach fencing generation owning this export (0 / omitted + /// = un-fenced). Lets a caller confirm which generation won the attach. + #[serde(skip_serializing_if = "Option::is_none")] + pub generation: Option, } impl From for ExportInfoResponse { @@ -107,6 +118,7 @@ impl From for ExportInfoResponse { dirty_bytes: e.dirty_bytes, s3_bytes: e.s3_bytes, fs_used_bytes: e.fs_used_bytes, + generation: (e.generation > 0).then_some(e.generation), } } } @@ -331,8 +343,33 @@ async fn create_or_attach_volume( req: &CreateVolumeRequest, from: &FromRef, ) -> Response { + // Single-attach fencing token (orchestrator placement generation). 0 = the + // caller does not participate in fencing (back-compat). + let my_gen = req.generation.unwrap_or(0); + // Already attached on this node → resize-or-noop (idempotent create). if let Some(export) = router.get_export_info(name).await { + // Re-PUT of an already-attached export: re-run the fence. A strictly + // higher generation (the same instance re-forked onto this node) seizes + // and is honored; a stale generation is rejected. + match router.enforce_attach_fence(name, my_gen).await { + Ok(crate::block::fence::Fence::Grant) => {} + Ok(crate::block::fence::Fence::Reject) => { + return error_response( + StatusCode::CONFLICT, + &format!( + "attach rejected: volume '{name}' is owned by a newer generation \ + than {my_gen} (fenced)" + ), + ); + } + Err(e) => { + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("attach fence failed for '{name}': {e}"), + ); + } + } let current_size_gb = export.size as f64 / 1_073_741_824.0; if req.size_gb > current_size_gb { return match router.resize_export(name, req.size_gb).await { @@ -431,6 +468,31 @@ async fn create_or_attach_volume( match create_result { Ok(()) => { + // Attach-time fence + seize, BEFORE persisting the index, registering + // the device, or serving any I/O. A rejected attach has uploaded zero + // data packs, so there is nothing to orphan — this is what prevents + // the at-sync orphaned-packs data loss. Skipped for the gen-0 bypass. + match router.enforce_attach_fence(name, my_gen).await { + Ok(crate::block::fence::Fence::Grant) => {} + Ok(crate::block::fence::Fence::Reject) => { + router.cleanup_failed_create(name, &transport).await; + return error_response( + StatusCode::CONFLICT, + &format!( + "attach rejected: volume '{name}' is owned by a newer generation \ + than {my_gen} (fenced)" + ), + ); + } + Err(e) => { + router.cleanup_failed_create(name, &transport).await; + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("attach fence failed for '{name}': {e}"), + ); + } + } + // Persist the index entry and register the device concurrently. // save_export MUST succeed; register_device is best-effort. let t_io = Instant::now(); @@ -1728,6 +1790,128 @@ mod tests { assert_eq!(body_json(resp).await["s3_prefix"], "custompool"); } + /// Single-attach generation fence: two nodes sharing one object store cannot + /// both own a volume. A stale generation is rejected (409); a newer one wins + /// and fences the old owner's stale re-attach. + #[tokio::test] + async fn test_attach_generation_fence_prevents_split_brain() { + let shared: Arc = + Arc::new(object_store::memory::InMemory::new()); + + // Node A attaches vol1 at generation 5 → wins and seizes the manifest. + let temp_a = TempDir::new().unwrap(); + let node_a = create_test_router_with_store(&temp_a, Arc::clone(&shared)).await; + let resp = request( + &node_a, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 5}"#), + ) + .await; + assert_eq!(resp.status(), StatusCode::CREATED); + let info: ExportInfoResponse = serde_json::from_slice( + &resp.into_body().collect().await.unwrap().to_bytes(), + ) + .unwrap(); + assert_eq!(info.generation, Some(5), "node A owns generation 5"); + + // Node B attaches with a STALE generation 4 → fenced (409), no clobber. + let temp_b = TempDir::new().unwrap(); + let node_b = create_test_router_with_store(&temp_b, Arc::clone(&shared)).await; + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 4}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::CONFLICT, + "a stale generation must be fenced out" + ); + + // Node B attaches with a NEWER generation 6 → wins, bumps the manifest. + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 6}"#), + ) + .await; + assert_eq!(resp.status(), StatusCode::CREATED); + let info: ExportInfoResponse = serde_json::from_slice( + &resp.into_body().collect().await.unwrap().to_bytes(), + ) + .unwrap(); + assert_eq!(info.generation, Some(6), "node B took ownership at generation 6"); + + // The old owner (gen 5) tries to re-attach on a fresh node → now fenced, + // because the volume is owned by the strictly-newer generation 6. + let temp_a2 = TempDir::new().unwrap(); + let node_a2 = create_test_router_with_store(&temp_a2, Arc::clone(&shared)).await; + let resp = request( + &node_a2, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 5}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::CONFLICT, + "the superseded owner must not be able to re-attach" + ); + + // Re-attach at the SAME winning generation 6 is idempotent (>= rule). + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 6}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::OK, + "re-attaching the owning generation must be admitted" + ); + } + + /// Back-compat: a caller that does not send a generation (the gen-0 bypass) + /// is never fenced, even against a volume already owned at a high generation. + #[tokio::test] + async fn test_attach_without_generation_bypasses_fence() { + let shared: Arc = + Arc::new(object_store::memory::InMemory::new()); + + let temp_a = TempDir::new().unwrap(); + let node_a = create_test_router_with_store(&temp_a, Arc::clone(&shared)).await; + request( + &node_a, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 9}"#), + ) + .await; + + // A legacy caller (no generation field) attaches on a fresh node → granted. + let temp_b = TempDir::new().unwrap(); + let node_b = create_test_router_with_store(&temp_b, Arc::clone(&shared)).await; + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::CREATED, + "the gen-0 bypass must not be fenced" + ); + } + /// A blank volume (no `from`) gets its own pool (= its name). #[tokio::test] async fn test_create_blank_volume_owns_pool() { diff --git a/glidefs/src/block/fence.rs b/glidefs/src/block/fence.rs new file mode 100644 index 0000000..7717ce3 --- /dev/null +++ b/glidefs/src/block/fence.rs @@ -0,0 +1,106 @@ +//! Single-attach generation fence — the storage-layer half of the recovery +//! split-brain fix. +//! +//! Under a network partition, instd's node-death recovery can let two nodes +//! attach and write the same volume at once (a partitioned-but-alive +//! predecessor's lease lapses, a replacement re-forks and re-attaches). A TTL +//! lease provably cannot prevent this; only the storage layer can authoritatively +//! stop two writers. GlideFS does it with a monotonic *generation* (fencing +//! token) issued by the orchestrator's per-instance placement generation: the +//! token is checked at the resource, and any operation carrying a stale token is +//! rejected. +//! +//! This is the GlideFS copy of the rule model-checked and unit-tested on the +//! instd side — `recovery_kernel.rs::attach_fence` (K4) and +//! `placement_kernel.rs::fence_check` (P3). There is no shared crate; the formal +//! spec (`12-recovery-formal-spec.md` §6.1) pins the copies together. Keep this +//! function byte-for-byte equivalent to those. + +/// Outcome of [`attach_fence`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Fence { + /// `my_gen >= stored_gen` — admit; this generation becomes the accepted + /// writer, fencing any strictly-older holder out. + Grant, + /// `my_gen < stored_gen` — a newer generation owns the volume; reject. A + /// partitioned / superseded predecessor's writes must not land. + Reject, +} + +/// Decide whether an attach/commit carrying generation `my_gen` is admitted, +/// given `stored_gen` (the highest generation that has owned the volume). +/// +/// `>=` (NOT `>`) is deliberate, exactly as instd's K4/P3: a node +/// re-reading/re-attaching its OWN generation must still be admitted; only a +/// strictly NEWER generation fences an older one out. +/// +/// This is the pure rule. The gen-0 back-compat bypass (a caller that does not +/// participate in fencing) lives at the call sites, NOT here — see +/// [`fence_attach`] — so this function stays a faithful mirror of instd. +#[must_use] +pub fn attach_fence(stored_gen: u64, my_gen: u64) -> Fence { + if my_gen >= stored_gen { + Fence::Grant + } else { + Fence::Reject + } +} + +/// Call-site wrapper applying the back-compat bypass around [`attach_fence`]. +/// +/// Fencing engages only when the caller opts in with `my_gen > 0`. A +/// `my_gen == 0` caller (a legacy instd that does not send a generation, or any +/// non-participating client) is always granted and never bumps the stored +/// generation. Roll-out is therefore one-way per volume: once a volume has been +/// stamped with a generation `>= 1`, only callers presenting a high-enough +/// generation are admitted. +#[must_use] +pub fn fence_attach(stored_gen: u64, my_gen: u64) -> Fence { + if my_gen == 0 { + return Fence::Grant; + } + attach_fence(stored_gen, my_gen) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn newer_generation_grants() { + assert_eq!(attach_fence(5, 6), Fence::Grant); + } + + #[test] + fn equal_generation_grants_reattach() { + // A node re-attaching its OWN generation must be admitted (idempotent + // re-attach / retry). This is why the rule is `>=`, not `>`. + assert_eq!(attach_fence(5, 5), Fence::Grant); + } + + #[test] + fn older_generation_rejects() { + assert_eq!(attach_fence(5, 4), Fence::Reject); + } + + #[test] + fn first_owner_grants_against_zero() { + assert_eq!(attach_fence(0, 1), Fence::Grant); + } + + #[test] + fn gen_zero_bypass_always_grants() { + // A non-participating caller (my_gen == 0) is never fenced, even against + // an already-fenced volume — this is the back-compat bypass. + assert_eq!(fence_attach(5, 0), Fence::Grant); + assert_eq!(fence_attach(0, 0), Fence::Grant); + } + + #[test] + fn gen_zero_bypass_does_not_change_pure_rule() { + // With my_gen > 0 the wrapper is identical to the pure rule. + assert_eq!(fence_attach(5, 6), attach_fence(5, 6)); + assert_eq!(fence_attach(5, 4), attach_fence(5, 4)); + assert_eq!(fence_attach(5, 5), attach_fence(5, 5)); + } +} diff --git a/glidefs/src/block/mod.rs b/glidefs/src/block/mod.rs index a1ad4ec..7987045 100644 --- a/glidefs/src/block/mod.rs +++ b/glidefs/src/block/mod.rs @@ -5,6 +5,7 @@ pub mod cache; #[allow(unsafe_code)] // libc::statvfs FFI pub mod capacity_monitor; pub mod error; +pub mod fence; pub mod flush_scheduler; pub mod foyer_engine; pub mod handler; diff --git a/glidefs/src/block/router.rs b/glidefs/src/block/router.rs index 53739fa..4126617 100644 --- a/glidefs/src/block/router.rs +++ b/glidefs/src/block/router.rs @@ -198,6 +198,8 @@ pub struct ExportInfo { pub s3_bytes: u64, /// Filesystem used bytes from ext4 superblock (None if not ext4 or read failed). pub fs_used_bytes: Option, + /// Single-attach fencing generation owning this export (0 = un-fenced). + pub generation: u64, } /// Readiness check result for health endpoint. @@ -1036,6 +1038,106 @@ impl ExportRouter { self.load_export(name).await } + /// Single-attach fence (attach side). Decide whether the node attaching this + /// already-created export with placement generation `my_gen` is admitted, and + /// on admission **seize** the volume by stamping `my_gen` into the manifest so + /// any strictly-older incumbent is fenced at its next sync. + /// + /// Must be called on an export already present in the map (after + /// `create_export`), before serving I/O — a rejected attach has uploaded zero + /// data packs, so there is nothing to orphan. + /// + /// Rule mirrors [`crate::block::fence`]: `my_gen >= stored ⇒ Grant`. `my_gen + /// == 0` is the back-compat bypass (caller does not participate in fencing) — + /// always granted, never seizes. The seize is a manifest If-Match CAS; a + /// concurrent attach that moves the manifest triggers a bounded re-read and + /// re-evaluation. + pub async fn enforce_attach_fence( + &self, + name: &str, + my_gen: u64, + ) -> Result { + use crate::block::fence::{fence_attach, Fence}; + + let (content_store, volume_manifest, cache) = { + let entry = self + .exports + .get(name) + .ok_or_else(|| RouterError::ExportNotFound(name.to_string()))?; + let s = entry.value(); + ( + Arc::clone(&s.content_store), + Arc::clone(&s.volume_manifest), + Arc::clone(&s.cache), + ) + }; + + // Bounded retries: a concurrent attach can move the manifest out from + // under our seize CAS; re-read and re-evaluate the fence each time. + for _ in 0..3u32 { + let stored_gen = volume_manifest.read().generation; + if fence_attach(stored_gen, my_gen) == Fence::Reject { + return Ok(Fence::Reject); + } + + // Granted. The accepted generation is the higher of the two (== my_gen + // once the fence passes, unless my_gen is the 0 bypass). + let effective = stored_gen.max(my_gen); + *cache.inner.current_generation.lock() = effective; + + // No seize needed: a non-participating caller (my_gen == 0), or an + // equal-generation re-attach that doesn't change the stored value. + if my_gen == 0 || effective == stored_gen { + return Ok(Fence::Grant); + } + + // Seize: stamp the new generation into the manifest via the existing + // If-Match CAS so an older incumbent's next sync is rejected. + let expected_etag = cache.inner.manifest_etag.lock().clone(); + let bytes = { + let mut m = volume_manifest.write(); + m.generation = effective; + m.serialize() + .map_err(|e| RouterError::Manifest(format!("serialize manifest: {e}")))? + }; + match content_store + .put_manifest(name, bytes, expected_etag.as_deref()) + .await + { + Ok(new_etag) => { + *cache.inner.manifest_etag.lock() = new_etag; + info!(export = %name, generation = effective, "attach fence: seized volume"); + return Ok(Fence::Grant); + } + Err(crate::block::content_store::ContentStoreError::PreconditionFailed(_)) => { + // A concurrent attach moved the manifest. Reload it (and its + // ETag) and re-run the fence against the fresh generation. + match content_store.get_manifest(name).await { + Ok(Some((data, etag))) => { + let vm = VolumeManifest::deserialize(&data).map_err(|e| { + RouterError::Manifest(format!("reload manifest: {e}")) + })?; + *volume_manifest.write() = vm; + *cache.inner.manifest_etag.lock() = etag; + } + Ok(None) => { + // Manifest vanished mid-seize; drop the ETag so the + // next attempt creates it. + *cache.inner.manifest_etag.lock() = None; + } + Err(e) => return Err(e.into()), + } + continue; + } + Err(e) => return Err(e.into()), + } + } + + Err(RouterError::Manifest(format!( + "attach fence for '{name}': manifest contended, could not seize after retries" + ))) + } + // ========================================================================= // Logical registry: image + snapshot indexes (sibling of export.json). // @@ -2520,6 +2622,7 @@ impl ExportRouter { let block_size = state.cache.block_size() as u64; let manifest = state.volume_manifest.read(); let s3_bytes = manifest.chunks.len() as u64 * manifest.chunk_size; + let generation = manifest.generation; drop(manifest); let info = ExportInfo { name: name.to_string(), @@ -2531,6 +2634,7 @@ impl ExportRouter { dirty_bytes: state.cache.dirty_block_count() * block_size, s3_bytes, fs_used_bytes: None, + generation, }; (info, Arc::clone(&state.handler)) }; @@ -2571,6 +2675,7 @@ impl ExportRouter { dirty_bytes: state.cache.dirty_block_count() * block_size, s3_bytes: manifest.chunks.len() as u64 * manifest.chunk_size, fs_used_bytes: None, + generation: manifest.generation, }; (info, Arc::clone(&state.handler)) }) diff --git a/glidefs/src/block/volume_manifest.rs b/glidefs/src/block/volume_manifest.rs index 7be4882..eedc433 100644 --- a/glidefs/src/block/volume_manifest.rs +++ b/glidefs/src/block/volume_manifest.rs @@ -29,6 +29,25 @@ pub const VOLUME_MANIFEST_VERSION: u16 = 5; /// serve an export BEFORE enabling prefetch hints for it. pub const VOLUME_MANIFEST_VERSION_PREFETCH: u16 = 6; +/// Version 7 adds an 8-byte `generation` trailer (the single-attach fencing +/// token, see [`super::fence`]) after the chunk entries, before the CRC. +/// Written ONLY when `generation > 0`, so an un-fenced manifest is byte-identical +/// to v5/v6. `prefetch_len` and `generation` are independent options, so the +/// four combinations get distinct versions: +/// - v5 — neither trailer +/// - v6 — prefetch_len only +/// - v7 — generation only +/// - v8 — both (prefetch_len first, then generation) +/// +/// DEPLOY-ORDER CAVEAT (same as v6): once any export persists a v7/v8 manifest, +/// an older daemon that predates these constants rejects it with +/// [`VolumeManifestError::UnsupportedVersion`]. Roll out the v7-aware binary to +/// every node that may serve an export BEFORE enabling fencing for it. +pub const VOLUME_MANIFEST_VERSION_GENERATION: u16 = 7; + +/// Version 8 — both the `prefetch_len` (v6) and `generation` (v7) trailers. +pub const VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION: u16 = 8; + /// Default chunk size for v4: 128 MiB (= 1 ext4 block group). pub const DEFAULT_CHUNK_SIZE: u64 = 128 * 1024 * 1024; @@ -63,6 +82,12 @@ pub struct VolumeManifest { /// the server warms `[0, prefetch_len)` into the clean cache so the guest's /// first reads are cache hits. `None` = no hint (legacy / non-prioritized). pub prefetch_len: Option, + /// Single-attach fencing token (owner generation). The highest generation + /// that has owned this volume; checked at attach and at every manifest sync + /// to fence a partitioned/superseded writer (see [`super::fence`]). `0` means + /// un-fenced (legacy export, or a caller that does not participate in + /// fencing) — the fence is a no-op for generation 0. + pub generation: u64, } #[derive(Debug, thiserror::Error)] @@ -90,6 +115,7 @@ impl VolumeManifest { block_size, chunks: BTreeMap::new(), prefetch_len: None, + generation: 0, } } @@ -240,15 +266,21 @@ impl VolumeManifest { .values() .map(|e| 4 + 2 + e.packs.len() * 8) // chunk_idx + pack_count(u16) + packs .sum(); - // v6 appends an 8-byte prefetch_len trailer; only emitted when set, so - // manifests without a hint are byte-identical to v5 (no churn). - let prefetch_trailer = if self.prefetch_len.is_some() { 8 } else { 0 }; - let version = if self.prefetch_len.is_some() { - VOLUME_MANIFEST_VERSION_PREFETCH - } else { - VOLUME_MANIFEST_VERSION + // v6 appends an 8-byte prefetch_len trailer; v7 an 8-byte generation + // trailer; v8 both. Each is emitted only when present, so an un-hinted, + // un-fenced manifest stays byte-identical to v5 (no churn). + let has_prefetch = self.prefetch_len.is_some(); + let has_generation = self.generation > 0; + let prefetch_trailer = if has_prefetch { 8 } else { 0 }; + let generation_trailer = if has_generation { 8 } else { 0 }; + let version = match (has_prefetch, has_generation) { + (false, false) => VOLUME_MANIFEST_VERSION, + (true, false) => VOLUME_MANIFEST_VERSION_PREFETCH, + (false, true) => VOLUME_MANIFEST_VERSION_GENERATION, + (true, true) => VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION, }; - let total = GLVM_HEADER_SIZE + entries_size + prefetch_trailer + 4; // +4 for CRC32 + let total = + GLVM_HEADER_SIZE + entries_size + prefetch_trailer + generation_trailer + 4; // +4 for CRC32 let mut buf = Vec::with_capacity(total); // Header (32 bytes). @@ -276,10 +308,14 @@ impl VolumeManifest { } } - // v6 prefetch_len trailer (before CRC, covered by it). + // Trailers (before CRC, covered by it). Order is fixed: prefetch_len + // first, then generation — deserialize reads them in the same order. if let Some(len) = self.prefetch_len { buf.extend_from_slice(&len.to_le_bytes()); } + if has_generation { + buf.extend_from_slice(&self.generation.to_le_bytes()); + } // CRC32 trailer. let crc = crc_fast::crc32_iscsi(&buf); @@ -311,10 +347,23 @@ impl VolumeManifest { return Err(VolumeManifestError::BadMagic); } let version = u16::from_le_bytes(data[4..6].try_into().unwrap()); - if version != VOLUME_MANIFEST_VERSION && version != VOLUME_MANIFEST_VERSION_PREFETCH { + if !matches!( + version, + VOLUME_MANIFEST_VERSION + | VOLUME_MANIFEST_VERSION_PREFETCH + | VOLUME_MANIFEST_VERSION_GENERATION + | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + ) { return Err(VolumeManifestError::UnsupportedVersion(u32::from(version))); } - let has_prefetch = version == VOLUME_MANIFEST_VERSION_PREFETCH; + let has_prefetch = matches!( + version, + VOLUME_MANIFEST_VERSION_PREFETCH | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + ); + let has_generation = matches!( + version, + VOLUME_MANIFEST_VERSION_GENERATION | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + ); let chunk_count = u32::from_le_bytes(data[6..10].try_into().unwrap()) as usize; let chunk_size = u64::from(u32::from_le_bytes(data[10..14].try_into().unwrap())); let block_size = u32::from_le_bytes(data[14..18].try_into().unwrap()); @@ -350,15 +399,26 @@ impl VolumeManifest { chunks.insert(chunk_idx, ChunkEntry { packs }); } - // v6 prefetch_len trailer sits between the last chunk entry and the CRC. + // Trailers sit between the last chunk entry and the CRC, in fixed order: + // prefetch_len (v6/v8) first, then generation (v7/v8). let prefetch_len = if has_prefetch { if pos + 8 > crc_offset { return Err(VolumeManifestError::TooShort); } - Some(u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap())) + let v = u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap()); + pos += 8; + Some(v) } else { None }; + let generation = if has_generation { + if pos + 8 > crc_offset { + return Err(VolumeManifestError::TooShort); + } + u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap()) + } else { + 0 + }; Ok(VolumeManifest { size: device_size, @@ -366,6 +426,7 @@ impl VolumeManifest { block_size, chunks, prefetch_len, + generation, }) } } @@ -412,6 +473,67 @@ mod tests { assert_eq!(back, m); } + /// Generation 0 (un-fenced) stays byte-identical v5 — zero churn for + /// existing exports — and round-trips to 0. + #[test] + fn generation_zero_stays_v5_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 0xdead_beef); + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION, + "generation 0 must still be written as v5" + ); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.generation, 0); + assert_eq!(back, m); + } + + /// A fenced manifest (generation > 0, no prefetch) is written as v7 and the + /// token round-trips. + #[test] + fn generation_only_is_v7_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.append_pack(3, 9); + m.generation = 42; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_GENERATION, + "a fenced manifest must be written as v7" + ); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.generation, 42); + assert_eq!(back, m); + } + + /// Both options present → v8, with prefetch_len first then generation, both + /// round-tripping. + #[test] + fn prefetch_and_generation_is_v8_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.prefetch_len = Some(7 * 1024 * 1024); + m.generation = 99; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION, + "prefetch + generation must be written as v8" + ); + // Trailer order: prefetch_len (8) then generation (8), before the CRC (4). + let plen = &bytes[bytes.len() - 20..bytes.len() - 12]; + let gen_bytes = &bytes[bytes.len() - 12..bytes.len() - 4]; + assert_eq!(u64::from_le_bytes(plen.try_into().unwrap()), 7 * 1024 * 1024); + assert_eq!(u64::from_le_bytes(gen_bytes.try_into().unwrap()), 99); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.prefetch_len, Some(7 * 1024 * 1024)); + assert_eq!(back.generation, 99); + assert_eq!(back, m); + } + /// Format stability: the v6 trailer is purely additive — a v6 manifest is /// exactly 8 bytes longer than the identical v5 one, and those 8 bytes are /// the LE `prefetch_len` sitting right before the CRC32. @@ -450,14 +572,14 @@ mod tests { let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); m.append_pack(0, 1); let mut bytes = m.serialize().unwrap(); - bytes[4..6].copy_from_slice(&7u16.to_le_bytes()); // forge version 7 + bytes[4..6].copy_from_slice(&99u16.to_le_bytes()); // forge an unknown version // recompute CRC so we exercise the version check, not the CRC check let crc_off = bytes.len() - 4; let crc = crc_fast::crc32_iscsi(&bytes[..crc_off]); bytes[crc_off..].copy_from_slice(&crc.to_le_bytes()); match VolumeManifest::deserialize(&bytes) { - Err(VolumeManifestError::UnsupportedVersion(7)) => {} - other => panic!("expected UnsupportedVersion(7), got {other:?}"), + Err(VolumeManifestError::UnsupportedVersion(99)) => {} + other => panic!("expected UnsupportedVersion(99), got {other:?}"), } } diff --git a/glidefs/src/block/write_cache/error.rs b/glidefs/src/block/write_cache/error.rs index d52a897..fad4351 100644 --- a/glidefs/src/block/write_cache/error.rs +++ b/glidefs/src/block/write_cache/error.rs @@ -50,6 +50,15 @@ pub enum CacheError { /// abort and let the predecessor revive. #[error("WAL file is locked by another process (pid in flock owner): {0}")] Locked(std::path::PathBuf), + + /// Single-attach fence tripped: a strictly newer generation has taken this + /// volume, so this writer has been superseded and must not commit. Terminal + /// — unlike a transient manifest race, retrying cannot succeed. The caller + /// must stop the flush loop and tear the export down (the guest's writes + /// since the last successful sync are dropped, which is correct: this node + /// was partitioned and those writes were never durably committed). + #[error("fenced: volume taken by generation {superseded_by} (this writer holds {held})")] + Fenced { held: u64, superseded_by: u64 }, } impl CacheError { @@ -67,12 +76,15 @@ impl CacheError { CacheError::InvalidMetadata } - /// True when the error is an S3 precondition failure (ETag mismatch), - /// indicating another host has taken ownership of this export's manifest. + /// True when the error means another writer owns this export and the flush + /// scheduler must stop: either an S3 precondition failure (ETag mismatch) or + /// a single-attach [`CacheError::Fenced`] (a strictly-newer generation took + /// the volume). pub fn is_manifest_conflict(&self) -> bool { matches!( self, CacheError::ContentStore(ContentStoreError::PreconditionFailed(_)) + | CacheError::Fenced { .. } ) } } diff --git a/glidefs/src/block/write_cache/flush.rs b/glidefs/src/block/write_cache/flush.rs index cdbb18a..e7cb028 100644 --- a/glidefs/src/block/write_cache/flush.rs +++ b/glidefs/src/block/write_cache/flush.rs @@ -1490,6 +1490,11 @@ impl WriteCache { total_stats.touched_chunks.insert(chunk_idx); vm.append_pack(chunk_idx, pack_id); } + // Stamp our fencing generation into every synced manifest so a + // strictly-older incumbent's sync is rejected (and so a fresh node + // learns the owner generation). 0 = un-fenced (zero churn: the + // manifest stays v5/v6). + vm.generation = *self.inner.current_generation.lock(); } #[cfg(feature = "test-utils")] @@ -1523,10 +1528,48 @@ impl WriteCache { break; } Err(e @ crate::block::content_store::ContentStoreError::PreconditionFailed(_)) => { - // Another host owns this manifest. Don't retry — - // every attempt fails with the same stale ETag. - // Return Err: outer flush_dirty_inner re-dirties - // the SYNCING blocks via flushing-file recovery. + // Another writer moved the manifest. Distinguish a single- + // attach FENCE (a strictly-newer generation took the volume + // — terminal) from a transient same-generation race. + let held = *self.inner.current_generation.lock(); + if held > 0 { + match content_store.get_manifest(&self.inner.export_name).await { + Ok(Some((data, _etag))) => { + if let Ok(s3_vm) = + crate::block::volume_manifest::VolumeManifest::deserialize( + &data, + ) + { + if s3_vm.generation > held { + // Superseded — we are fenced. Latch the + // flag so the write path rejects further + // guest writes, then return terminal so + // the scheduler stops. + self.inner.fenced.store( + true, + std::sync::atomic::Ordering::Release, + ); + warn!( + export = %self.inner.export_name, + held, + superseded_by = s3_vm.generation, + "manifest sync FENCED: volume taken by a newer generation" + ); + return Err(CacheError::Fenced { + held, + superseded_by: s3_vm.generation, + }); + } + } + } + // Couldn't read/parse S3 manifest — fall through to + // the transient path (re-dirty + retry next cycle). + Ok(None) | Err(_) => {} + } + } + // Transient same-generation race (e.g. a concurrent flush). + // Return Err: outer flush_dirty_inner re-dirties the SYNCING + // blocks via flushing-file recovery and the next cycle retries. return Err(e.into()); } Err(e) => { diff --git a/glidefs/src/block/write_cache/init.rs b/glidefs/src/block/write_cache/init.rs index d2781b0..7084404 100644 --- a/glidefs/src/block/write_cache/init.rs +++ b/glidefs/src/block/write_cache/init.rs @@ -371,6 +371,8 @@ impl WriteCache { flush_lock: tokio::sync::Mutex::new(()), manifest_etag: parking_lot::Mutex::new(None), + current_generation: parking_lot::Mutex::new(0), + fenced: std::sync::atomic::AtomicBool::new(false), page_crcs: dashmap::DashMap::new(), pages_per_block: block_size / 4096, flushing_active: AtomicBool::new(false), @@ -461,6 +463,8 @@ impl WriteCache { flush_lock: tokio::sync::Mutex::new(()), manifest_etag: parking_lot::Mutex::new(None), + current_generation: parking_lot::Mutex::new(0), + fenced: std::sync::atomic::AtomicBool::new(false), page_crcs: dashmap::DashMap::new(), pages_per_block: block_size / 4096, flushing_active: AtomicBool::new(false), diff --git a/glidefs/src/block/write_cache/inner.rs b/glidefs/src/block/write_cache/inner.rs index 3597a38..e736d0b 100644 --- a/glidefs/src/block/write_cache/inner.rs +++ b/glidefs/src/block/write_cache/inner.rs @@ -504,6 +504,22 @@ pub(crate) struct CacheInner { /// `None` means first upload (unconditional PUT). pub(crate) manifest_etag: Mutex>, + /// Single-attach fencing generation this writer was granted at attach (the + /// orchestrator's monotonic per-instance placement generation; see + /// [`crate::block::fence`]). Seeded at attach. Stamped into the manifest on + /// every sync and compared against S3 on a precondition failure: if a strictly + /// newer generation has taken the volume, this writer is FENCED. `0` = + /// un-fenced (legacy / non-participating caller); the fence is a no-op. + pub(crate) current_generation: Mutex, + + /// Set once this writer is FENCED — a strictly-newer generation took the + /// volume (detected at manifest sync). Latches forever: once fenced, the + /// write path rejects further guest writes so the guest stops receiving + /// ACKs for writes that can never durably commit, and the flush scheduler + /// stops. Self-contained so the cache fences itself without reaching for the + /// handler/router. + pub(crate) fenced: AtomicBool, + /// Per-page CRC32C checksums captured at pwrite time from the guest's write /// buffer. One entry per block, value is per-page CRC array. Upsert via /// `insert()` — allocation happens outside the shard lock, lock hold time diff --git a/glidefs/src/block/write_cache/tests.rs b/glidefs/src/block/write_cache/tests.rs index 454b114..7763963 100644 --- a/glidefs/src/block/write_cache/tests.rs +++ b/glidefs/src/block/write_cache/tests.rs @@ -244,6 +244,68 @@ impl V2Harness { fn manifest(&self) -> VolumeManifest { self.volume_manifest.read().clone() } + + /// Raw flush returning the Result (so a test can assert on the error). + async fn try_flush(&self) -> Result { + self.cache + .flush_to_s3( + &self.content_store, + &self.pack_index_cache, + &self.volume_manifest, + ) + .await + } +} + +/// Sync-time single-attach fence: an incumbent writer at generation 5 that has +/// been superseded by a newer generation 6 (which seized the S3 manifest) is +/// FENCED at its next manifest sync — the flush returns `Fenced` and the write +/// path latches closed so the guest stops getting ACKs. +#[tokio::test] +async fn test_sync_fence_supersedes_incumbent() { + let h = V2Harness::new().await; + + // Incumbent owns generation 5. + *h.cache.inner.current_generation.lock() = 5; + + // First flush commits at generation 5; the S3 manifest now carries gen 5. + h.cache.write(0, b"first").unwrap(); + h.try_flush().await.expect("first flush at gen 5 succeeds"); + assert_eq!(h.manifest().generation, 5, "in-memory manifest stamped gen 5"); + + // A newer generation (6) seizes the S3 manifest out from under us — this is + // what node B's attach does. It bumps the generation and the ETag. + let (data, etag) = h + .content_store + .get_manifest("test") + .await + .unwrap() + .expect("manifest exists in S3"); + let mut s3_vm = VolumeManifest::deserialize(&data).unwrap(); + assert_eq!(s3_vm.generation, 5); + s3_vm.generation = 6; + h.content_store + .put_manifest("test", s3_vm.serialize().unwrap(), etag.as_deref()) + .await + .expect("newer generation seizes the manifest"); + + // The incumbent writes more and flushes → its cached ETag is stale, it + // re-reads, sees gen 6 > 5, and is FENCED (terminal, not a transient retry). + h.cache.write(8192, b"second").unwrap(); + match h.try_flush().await { + Err(CacheError::Fenced { held, superseded_by }) => { + assert_eq!(held, 5); + assert_eq!(superseded_by, 6); + } + other => panic!("expected Fenced, got {other:?}"), + } + + // The write path is now latched closed: further guest writes are rejected so + // the guest stops receiving ACKs for writes that can never commit. + match h.cache.write(16384, b"after-fence") { + Err(CacheError::Fenced { .. }) => {} + other => panic!("expected writes to be fenced, got {other:?}"), + } } #[tokio::test] diff --git a/glidefs/src/block/write_cache/write.rs b/glidefs/src/block/write_cache/write.rs index 12e358f..d55ab7f 100644 --- a/glidefs/src/block/write_cache/write.rs +++ b/glidefs/src/block/write_cache/write.rs @@ -158,6 +158,21 @@ impl WriteCache { data: &[u8], require_promotion: bool, ) -> Result<(), CacheError> { + // Single-attach fence: once a strictly-newer generation has taken this + // volume, reject all further guest writes. They could never durably + // commit (the manifest sync is fenced), so ACKing them would be a lie. + if self + .inner + .fenced + .load(std::sync::atomic::Ordering::Acquire) + { + let held = *self.inner.current_generation.lock(); + return Err(CacheError::Fenced { + held, + superseded_by: 0, + }); + } + let end = offset.checked_add(data.len() as u64).ok_or_else(|| { CacheError::offset_out_of_bounds(u64::MAX, self.inner.config.device_size) })?; From 156f82d3439f1e5e0cf632978c3321110aeff1a5 Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Tue, 23 Jun 2026 20:49:31 -0700 Subject: [PATCH 2/2] feat(fence): widen fencing token to the composite (generation, lease_revision) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgrade the single-attach fence from the orchestrator placement generation alone to the full COMPOSITE token the formal model proves sound (`token_composite_is_sound`; generation-only is unsound on a same-node re-fork — `token_orch_only_double_writes`). This makes GlideFS's copy a byte-for-byte mirror of instd `recovery_kernel::{attach_fence, compose_token}`, so the spec's audit rule pins the two copies identically instead of GlideFS enforcing only a subset. The token is `(generation, lease_revision)` packed lexicographically into a u128 by `compose_token`: the orchestrator generation dominates (high 64 bits, fences cross-node re-placement); the node-lease claim revision orders same-node incarnations (low 64 bits) — a node-death re-fork onto the SAME node does not advance the generation, so without the lease revision two incarnations carry equal tokens and neither fences the other. - fence.rs: `compose_token` + composite `attach_fence`/`fence_attach`. The back-compat bypass now requires BOTH halves zero; a caller sending only a lease_revision (generation 0, lease > 0) participates. - volume_manifest.rs: composite trailer as manifest v9 (composite only) / v10 (prefetch + composite), emitted ONLY when lease_revision > 0 — a generation-only or un-fenced export stays v5/v7/v8 (zero churn). - api/router/flush/inner/init: thread lease_revision alongside generation through the attach fence, the seize, the sync stamp, and runtime state. Behaviorally identical to the generation-only version until a caller supplies a non-zero lease_revision (instd plumbing en route). 520 lib tests pass with --features ublk, incl. new composite cases (generation dominates lease, same-gen-higher-lease seizes, lease-only caller participates). Co-Authored-By: Claude Opus 4.8 (1M context) --- glidefs/src/block/api.rs | 133 +++++++++++++-- glidefs/src/block/fence.rs | 189 ++++++++++++++++------ glidefs/src/block/router.rs | 66 +++++--- glidefs/src/block/volume_manifest.rs | 216 ++++++++++++++++++++++--- glidefs/src/block/write_cache/flush.rs | 35 ++-- glidefs/src/block/write_cache/init.rs | 2 + glidefs/src/block/write_cache/inner.rs | 9 ++ 7 files changed, 539 insertions(+), 111 deletions(-) diff --git a/glidefs/src/block/api.rs b/glidefs/src/block/api.rs index 755eb57..881ecc4 100644 --- a/glidefs/src/block/api.rs +++ b/glidefs/src/block/api.rs @@ -61,9 +61,18 @@ pub struct CreateVolumeRequest { /// placement generation). When `> 0`, the attach is admitted only if this /// generation is `>=` the volume's stored generation; a strictly-newer /// generation fences an older holder out (see [`crate::block::fence`]). - /// Omitted / `0` = the caller does not participate in fencing (back-compat). + /// Omitted / `0` = the caller does not participate in the high half. #[serde(default)] pub generation: Option, + /// Single-attach fencing token, low half: the node-lease claim revision. + /// Orders same-`node_id` incarnations (a node-death re-fork does NOT advance + /// the orchestrator [`Self::generation`], so this is what fences it). The + /// fence compares the composite `(generation, lease_revision)`. Omitted / + /// `0` = the caller does not participate in the low half. The back-compat + /// bypass requires BOTH halves to be 0; a caller sending only a + /// `lease_revision` DOES participate. (See [`crate::block::fence`].) + #[serde(default)] + pub lease_revision: Option, } /// Optional request body for POST /api/exports/{name}/snapshot. @@ -100,10 +109,16 @@ pub struct ExportInfoResponse { /// Filesystem used bytes from ext4 superblock (None if not ext4 or read failed). #[serde(skip_serializing_if = "Option::is_none")] pub fs_used_bytes: Option, - /// Current single-attach fencing generation owning this export (0 / omitted - /// = un-fenced). Lets a caller confirm which generation won the attach. + /// Current single-attach fencing generation (high half) owning this export + /// (0 / omitted = un-fenced on this half). Lets a caller confirm which + /// generation won the attach. #[serde(skip_serializing_if = "Option::is_none")] pub generation: Option, + /// Current single-attach fencing lease revision (low half) owning this + /// export (0 / omitted = un-fenced on this half). Lets a caller confirm + /// which same-node incarnation won the attach. + #[serde(skip_serializing_if = "Option::is_none")] + pub lease_revision: Option, } impl From for ExportInfoResponse { @@ -119,6 +134,7 @@ impl From for ExportInfoResponse { s3_bytes: e.s3_bytes, fs_used_bytes: e.fs_used_bytes, generation: (e.generation > 0).then_some(e.generation), + lease_revision: (e.lease_revision > 0).then_some(e.lease_revision), } } } @@ -343,23 +359,26 @@ async fn create_or_attach_volume( req: &CreateVolumeRequest, from: &FromRef, ) -> Response { - // Single-attach fencing token (orchestrator placement generation). 0 = the - // caller does not participate in fencing (back-compat). + // Single-attach fencing token: high half = orchestrator placement + // generation, low half = node-lease claim revision. 0/0 = the caller does + // not participate in fencing (back-compat bypass); a lease-only token still + // participates. let my_gen = req.generation.unwrap_or(0); + let my_lease = req.lease_revision.unwrap_or(0); // Already attached on this node → resize-or-noop (idempotent create). if let Some(export) = router.get_export_info(name).await { // Re-PUT of an already-attached export: re-run the fence. A strictly - // higher generation (the same instance re-forked onto this node) seizes - // and is honored; a stale generation is rejected. - match router.enforce_attach_fence(name, my_gen).await { + // higher composite token (the same instance re-placed cross-node OR + // re-forked same-node) seizes and is honored; a stale token is rejected. + match router.enforce_attach_fence(name, my_gen, my_lease).await { Ok(crate::block::fence::Fence::Grant) => {} Ok(crate::block::fence::Fence::Reject) => { return error_response( StatusCode::CONFLICT, &format!( - "attach rejected: volume '{name}' is owned by a newer generation \ - than {my_gen} (fenced)" + "attach rejected: volume '{name}' is owned by a newer token \ + than (generation={my_gen}, lease_revision={my_lease}) (fenced)" ), ); } @@ -471,16 +490,16 @@ async fn create_or_attach_volume( // Attach-time fence + seize, BEFORE persisting the index, registering // the device, or serving any I/O. A rejected attach has uploaded zero // data packs, so there is nothing to orphan — this is what prevents - // the at-sync orphaned-packs data loss. Skipped for the gen-0 bypass. - match router.enforce_attach_fence(name, my_gen).await { + // the at-sync orphaned-packs data loss. Skipped for the 0/0 bypass. + match router.enforce_attach_fence(name, my_gen, my_lease).await { Ok(crate::block::fence::Fence::Grant) => {} Ok(crate::block::fence::Fence::Reject) => { router.cleanup_failed_create(name, &transport).await; return error_response( StatusCode::CONFLICT, &format!( - "attach rejected: volume '{name}' is owned by a newer generation \ - than {my_gen} (fenced)" + "attach rejected: volume '{name}' is owned by a newer token \ + than (generation={my_gen}, lease_revision={my_lease}) (fenced)" ), ); } @@ -1878,6 +1897,92 @@ mod tests { ); } + /// Same-node re-fork fence: a node-death re-fork onto the SAME node does NOT + /// advance the orchestrator placement generation — only the node-lease claim + /// revision advances. The composite token `(generation, lease_revision)` is + /// what fences here: at a fixed generation, the higher lease_revision seizes + /// and the stale incarnation is rejected. This is the case the widening + /// exists for. + #[tokio::test] + async fn test_attach_lease_revision_fence_same_node() { + let shared: Arc = + Arc::new(object_store::memory::InMemory::new()); + + // Incarnation A attaches vol1 at generation 7, lease_revision 1 → wins + // and seizes the manifest. + let temp_a = TempDir::new().unwrap(); + let node_a = create_test_router_with_store(&temp_a, Arc::clone(&shared)).await; + let resp = request( + &node_a, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 7, "lease_revision": 1}"#), + ) + .await; + assert_eq!(resp.status(), StatusCode::CREATED); + let info: ExportInfoResponse = serde_json::from_slice( + &resp.into_body().collect().await.unwrap().to_bytes(), + ) + .unwrap(); + assert_eq!(info.generation, Some(7)); + assert_eq!(info.lease_revision, Some(1), "incarnation A owns lease rev 1"); + + // The SAME-node re-fork: generation is unchanged (7), but the node-lease + // revision advances to 2 → the fresher incarnation seizes the volume. + let temp_b = TempDir::new().unwrap(); + let node_b = create_test_router_with_store(&temp_b, Arc::clone(&shared)).await; + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 7, "lease_revision": 2}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::CREATED, + "the same-generation, higher-lease-revision incarnation must seize" + ); + let info: ExportInfoResponse = serde_json::from_slice( + &resp.into_body().collect().await.unwrap().to_bytes(), + ) + .unwrap(); + assert_eq!(info.generation, Some(7)); + assert_eq!(info.lease_revision, Some(2), "incarnation B took ownership at lease rev 2"); + + // The stale predecessor (gen 7, lease rev 1) re-attaches on a fresh node → + // now fenced, because the volume is owned by the strictly-newer composite + // token (7, 2) — the orchestrator generation never moved. + let temp_a2 = TempDir::new().unwrap(); + let node_a2 = create_test_router_with_store(&temp_a2, Arc::clone(&shared)).await; + let resp = request( + &node_a2, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 7, "lease_revision": 1}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::CONFLICT, + "the stale same-node predecessor must not be able to re-attach" + ); + + // Re-attach at the SAME winning token (7, 2) is idempotent (>= rule). + let resp = request( + &node_b, + Method::PUT, + "/api/exports/vol1", + Some(r#"{"size_gb": 0.01, "generation": 7, "lease_revision": 2}"#), + ) + .await; + assert_eq!( + resp.status(), + StatusCode::OK, + "re-attaching the owning token must be admitted" + ); + } + /// Back-compat: a caller that does not send a generation (the gen-0 bypass) /// is never fenced, even against a volume already owned at a high generation. #[tokio::test] diff --git a/glidefs/src/block/fence.rs b/glidefs/src/block/fence.rs index 7717ce3..c8629ba 100644 --- a/glidefs/src/block/fence.rs +++ b/glidefs/src/block/fence.rs @@ -5,61 +5,93 @@ //! attach and write the same volume at once (a partitioned-but-alive //! predecessor's lease lapses, a replacement re-forks and re-attaches). A TTL //! lease provably cannot prevent this; only the storage layer can authoritatively -//! stop two writers. GlideFS does it with a monotonic *generation* (fencing -//! token) issued by the orchestrator's per-instance placement generation: the -//! token is checked at the resource, and any operation carrying a stale token is -//! rejected. +//! stop two writers. GlideFS does it with a monotonic *fencing token* checked at +//! the resource: any operation carrying a stale token is rejected. //! -//! This is the GlideFS copy of the rule model-checked and unit-tested on the -//! instd side — `recovery_kernel.rs::attach_fence` (K4) and -//! `placement_kernel.rs::fence_check` (P3). There is no shared crate; the formal -//! spec (`12-recovery-formal-spec.md` §6.1) pins the copies together. Keep this -//! function byte-for-byte equivalent to those. +//! The token is **composite** and `u128`-wide, packed lexicographically by +//! [`compose_token`]: +//! +//! - High 64 bits = `generation` — the orchestrator's per-instance placement +//! generation. It dominates and strictly increases on a cross-node +//! re-placement (evacuation / deploy), so it fences the cross-node split-brain. +//! - Low 64 bits = `lease_revision` — the node-lease claim revision. It orders +//! same-`node_id` incarnations: a node-death re-fork onto the *same* node does +//! NOT advance the orchestrator generation, so without the lease revision two +//! incarnations would carry equal tokens and neither would fence the other. +//! +//! The composite is the only value monotonic across BOTH transitions. This is +//! the GlideFS copy of the rule model-checked and unit-tested on the instd side — +//! `recovery_kernel.rs::attach_fence` (K4) and its `compose_token`. There is no +//! shared crate; the formal spec (`12-recovery-formal-spec.md` §2.5/§6.1) pins +//! the copies together. Keep this byte-for-byte equivalent to those. /// Outcome of [`attach_fence`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Fence { - /// `my_gen >= stored_gen` — admit; this generation becomes the accepted + /// `my_token >= stored_token` — admit; this token becomes the accepted /// writer, fencing any strictly-older holder out. Grant, - /// `my_gen < stored_gen` — a newer generation owns the volume; reject. A + /// `my_token < stored_token` — a newer token owns the volume; reject. A /// partitioned / superseded predecessor's writes must not land. Reject, } -/// Decide whether an attach/commit carrying generation `my_gen` is admitted, -/// given `stored_gen` (the highest generation that has owned the volume). +/// Pack the composite fencing token: `(generation, lease_revision)` +/// lexicographically into a `u128`, so a plain `>=` realises the lexicographic +/// order. The orchestrator generation dominates (high bits); within a fixed +/// placement the lease revision orders same-node incarnations (low bits). /// -/// `>=` (NOT `>`) is deliberate, exactly as instd's K4/P3: a node -/// re-reading/re-attaching its OWN generation must still be admitted; only a -/// strictly NEWER generation fences an older one out. +/// Mirror of `recovery_kernel.rs::compose_token`. Keep identical. +#[must_use] +pub fn compose_token(generation: u64, lease_revision: u64) -> u128 { + ((generation as u128) << 64) | (lease_revision as u128) +} + +/// Decide whether an attach/commit carrying composite token `my_token` is +/// admitted, given `stored_token` (the highest token that has owned the volume). /// -/// This is the pure rule. The gen-0 back-compat bypass (a caller that does not +/// `>=` (NOT `>`) is deliberate, exactly as instd's K4: a node +/// re-reading/re-attaching its OWN token must still be admitted; only a strictly +/// NEWER token fences an older one out. +/// +/// This is the pure rule. The back-compat bypass (a caller that does not /// participate in fencing) lives at the call sites, NOT here — see /// [`fence_attach`] — so this function stays a faithful mirror of instd. #[must_use] -pub fn attach_fence(stored_gen: u64, my_gen: u64) -> Fence { - if my_gen >= stored_gen { +pub fn attach_fence(stored_token: u128, my_token: u128) -> Fence { + if my_token >= stored_token { Fence::Grant } else { Fence::Reject } } -/// Call-site wrapper applying the back-compat bypass around [`attach_fence`]. +/// Call-site wrapper applying the back-compat bypass around [`attach_fence`], +/// composing the caller's `(generation, lease_revision)` against the stored +/// `(stored_generation, stored_lease_revision)`. /// -/// Fencing engages only when the caller opts in with `my_gen > 0`. A -/// `my_gen == 0` caller (a legacy instd that does not send a generation, or any -/// non-participating client) is always granted and never bumps the stored -/// generation. Roll-out is therefore one-way per volume: once a volume has been -/// stamped with a generation `>= 1`, only callers presenting a high-enough -/// generation are admitted. +/// Fencing engages only when the caller opts in with a non-zero token. A caller +/// presenting `generation == 0 && lease_revision == 0` (a legacy instd that +/// sends neither, or any non-participating client) is always granted and never +/// bumps the stored token. Note the bypass requires BOTH halves to be zero: a +/// caller that sends only a `lease_revision` (generation 0, lease > 0) DOES +/// participate and is fenced on the composite. Roll-out is therefore one-way per +/// volume: once a volume has been stamped with any non-zero token, only callers +/// presenting a high-enough composite are admitted. #[must_use] -pub fn fence_attach(stored_gen: u64, my_gen: u64) -> Fence { - if my_gen == 0 { +pub fn fence_attach( + stored_generation: u64, + stored_lease_revision: u64, + my_generation: u64, + my_lease_revision: u64, +) -> Fence { + if my_generation == 0 && my_lease_revision == 0 { return Fence::Grant; } - attach_fence(stored_gen, my_gen) + attach_fence( + compose_token(stored_generation, stored_lease_revision), + compose_token(my_generation, my_lease_revision), + ) } #[cfg(test)] @@ -68,39 +100,106 @@ mod tests { #[test] fn newer_generation_grants() { - assert_eq!(attach_fence(5, 6), Fence::Grant); + assert_eq!( + attach_fence(compose_token(5, 0), compose_token(6, 0)), + Fence::Grant + ); } #[test] - fn equal_generation_grants_reattach() { - // A node re-attaching its OWN generation must be admitted (idempotent + fn equal_token_grants_reattach() { + // A node re-attaching its OWN token must be admitted (idempotent // re-attach / retry). This is why the rule is `>=`, not `>`. - assert_eq!(attach_fence(5, 5), Fence::Grant); + assert_eq!( + attach_fence(compose_token(5, 0), compose_token(5, 0)), + Fence::Grant + ); } #[test] fn older_generation_rejects() { - assert_eq!(attach_fence(5, 4), Fence::Reject); + assert_eq!( + attach_fence(compose_token(5, 0), compose_token(4, 0)), + Fence::Reject + ); } #[test] fn first_owner_grants_against_zero() { - assert_eq!(attach_fence(0, 1), Fence::Grant); + assert_eq!( + attach_fence(compose_token(0, 0), compose_token(1, 0)), + Fence::Grant + ); + } + + #[test] + fn generation_dominates_lease_revision() { + // The orchestrator generation is the high half: it dominates the lease + // revision regardless of how large the latter is. + assert!(compose_token(2, 0) > compose_token(1, u64::MAX)); + // A cross-node move (higher generation, fresh/low lease) still wins. + assert_eq!( + attach_fence(compose_token(1, 9), compose_token(2, 1)), + Fence::Grant + ); + } + + #[test] + fn same_generation_higher_lease_revision_grants_seizes() { + // The same-node re-fork case the widening exists for: the orchestrator + // generation does NOT advance, but the node-lease revision does — the + // fresher incarnation seizes. + assert_eq!( + attach_fence(compose_token(7, 3), compose_token(7, 4)), + Fence::Grant + ); + } + + #[test] + fn same_generation_lower_lease_revision_rejects() { + // A stale same-node predecessor (lower lease revision under the same + // generation) is fenced out. + assert_eq!( + attach_fence(compose_token(7, 4), compose_token(7, 3)), + Fence::Reject + ); + } + + #[test] + fn zero_token_bypass_always_grants() { + // A fully non-participating caller (generation == 0 AND + // lease_revision == 0) is never fenced, even against an already-fenced + // volume — this is the back-compat bypass. + assert_eq!(fence_attach(5, 0, 0, 0), Fence::Grant); + assert_eq!(fence_attach(0, 0, 0, 0), Fence::Grant); + assert_eq!(fence_attach(9, 4, 0, 0), Fence::Grant); } #[test] - fn gen_zero_bypass_always_grants() { - // A non-participating caller (my_gen == 0) is never fenced, even against - // an already-fenced volume — this is the back-compat bypass. - assert_eq!(fence_attach(5, 0), Fence::Grant); - assert_eq!(fence_attach(0, 0), Fence::Grant); + fn lease_only_caller_participates() { + // A caller sending only a lease_revision (generation 0, lease > 0) is NOT + // bypassed — it participates and is ordered against the stored token. + // (gen=0, lease=5) vs stored (gen=0, lease=3) → Grant (seizes). + assert_eq!(fence_attach(0, 3, 0, 5), Fence::Grant); + // (gen=0, lease=3) vs stored (gen=0, lease=5) → Reject (stale). + assert_eq!(fence_attach(0, 5, 0, 3), Fence::Reject); } #[test] - fn gen_zero_bypass_does_not_change_pure_rule() { - // With my_gen > 0 the wrapper is identical to the pure rule. - assert_eq!(fence_attach(5, 6), attach_fence(5, 6)); - assert_eq!(fence_attach(5, 4), attach_fence(5, 4)); - assert_eq!(fence_attach(5, 5), attach_fence(5, 5)); + fn bypass_does_not_change_pure_rule() { + // With a participating token the wrapper is identical to the pure rule + // over the composites. + assert_eq!( + fence_attach(5, 0, 6, 0), + attach_fence(compose_token(5, 0), compose_token(6, 0)) + ); + assert_eq!( + fence_attach(5, 0, 4, 0), + attach_fence(compose_token(5, 0), compose_token(4, 0)) + ); + assert_eq!( + fence_attach(7, 3, 7, 4), + attach_fence(compose_token(7, 3), compose_token(7, 4)) + ); } } diff --git a/glidefs/src/block/router.rs b/glidefs/src/block/router.rs index 4126617..47c87b7 100644 --- a/glidefs/src/block/router.rs +++ b/glidefs/src/block/router.rs @@ -198,8 +198,13 @@ pub struct ExportInfo { pub s3_bytes: u64, /// Filesystem used bytes from ext4 superblock (None if not ext4 or read failed). pub fs_used_bytes: Option, - /// Single-attach fencing generation owning this export (0 = un-fenced). + /// Single-attach fencing token, high half: the orchestrator placement + /// generation owning this export (0 = un-fenced on this half). pub generation: u64, + /// Single-attach fencing token, low half: the node-lease claim revision + /// owning this export (0 = un-fenced on this half). Composed with + /// `generation` into the fence token; see [`crate::block::fence`]. + pub lease_revision: u64, } /// Readiness check result for health endpoint. @@ -1039,25 +1044,29 @@ impl ExportRouter { } /// Single-attach fence (attach side). Decide whether the node attaching this - /// already-created export with placement generation `my_gen` is admitted, and - /// on admission **seize** the volume by stamping `my_gen` into the manifest so - /// any strictly-older incumbent is fenced at its next sync. + /// already-created export with composite token `(my_gen, my_lease)` is + /// admitted, and on admission **seize** the volume by stamping the winning + /// token into the manifest so any strictly-older incumbent is fenced at its + /// next sync. /// /// Must be called on an export already present in the map (after /// `create_export`), before serving I/O — a rejected attach has uploaded zero /// data packs, so there is nothing to orphan. /// - /// Rule mirrors [`crate::block::fence`]: `my_gen >= stored ⇒ Grant`. `my_gen - /// == 0` is the back-compat bypass (caller does not participate in fencing) — - /// always granted, never seizes. The seize is a manifest If-Match CAS; a + /// Rule mirrors [`crate::block::fence`]: the fence compares the composite + /// `(generation, lease_revision)` tokens, `my >= stored ⇒ Grant`. `my_gen == + /// 0 && my_lease == 0` is the back-compat bypass (caller does not participate + /// in fencing) — always granted, never seizes. A caller sending only a + /// `lease_revision` DOES participate. The seize is a manifest If-Match CAS; a /// concurrent attach that moves the manifest triggers a bounded re-read and /// re-evaluation. pub async fn enforce_attach_fence( &self, name: &str, my_gen: u64, + my_lease: u64, ) -> Result { - use crate::block::fence::{fence_attach, Fence}; + use crate::block::fence::{compose_token, fence_attach, Fence}; let (content_store, volume_manifest, cache) = { let entry = self @@ -1075,28 +1084,38 @@ impl ExportRouter { // Bounded retries: a concurrent attach can move the manifest out from // under our seize CAS; re-read and re-evaluate the fence each time. for _ in 0..3u32 { - let stored_gen = volume_manifest.read().generation; - if fence_attach(stored_gen, my_gen) == Fence::Reject { + let (stored_gen, stored_lease) = { + let m = volume_manifest.read(); + (m.generation, m.lease_revision) + }; + if fence_attach(stored_gen, stored_lease, my_gen, my_lease) == Fence::Reject { return Ok(Fence::Reject); } - // Granted. The accepted generation is the higher of the two (== my_gen - // once the fence passes, unless my_gen is the 0 bypass). - let effective = stored_gen.max(my_gen); - *cache.inner.current_generation.lock() = effective; - - // No seize needed: a non-participating caller (my_gen == 0), or an - // equal-generation re-attach that doesn't change the stored value. - if my_gen == 0 || effective == stored_gen { + // Granted. The accepted token is the higher composite of the two + // (== my token once the fence passes, unless this is the 0/0 bypass). + // Decompose it back into (generation, lease_revision) to stamp. + let stored_token = compose_token(stored_gen, stored_lease); + let my_token = compose_token(my_gen, my_lease); + let effective_token = stored_token.max(my_token); + let effective_gen = (effective_token >> 64) as u64; + let effective_lease = (effective_token & u128::from(u64::MAX)) as u64; + *cache.inner.current_generation.lock() = effective_gen; + *cache.inner.current_lease_revision.lock() = effective_lease; + + // No seize needed: a fully non-participating caller (0/0 bypass), or + // an equal-token re-attach that doesn't change the stored value. + if (my_gen == 0 && my_lease == 0) || effective_token == stored_token { return Ok(Fence::Grant); } - // Seize: stamp the new generation into the manifest via the existing + // Seize: stamp the new token into the manifest via the existing // If-Match CAS so an older incumbent's next sync is rejected. let expected_etag = cache.inner.manifest_etag.lock().clone(); let bytes = { let mut m = volume_manifest.write(); - m.generation = effective; + m.generation = effective_gen; + m.lease_revision = effective_lease; m.serialize() .map_err(|e| RouterError::Manifest(format!("serialize manifest: {e}")))? }; @@ -1106,12 +1125,12 @@ impl ExportRouter { { Ok(new_etag) => { *cache.inner.manifest_etag.lock() = new_etag; - info!(export = %name, generation = effective, "attach fence: seized volume"); + info!(export = %name, generation = effective_gen, lease_revision = effective_lease, "attach fence: seized volume"); return Ok(Fence::Grant); } Err(crate::block::content_store::ContentStoreError::PreconditionFailed(_)) => { // A concurrent attach moved the manifest. Reload it (and its - // ETag) and re-run the fence against the fresh generation. + // ETag) and re-run the fence against the fresh token. match content_store.get_manifest(name).await { Ok(Some((data, etag))) => { let vm = VolumeManifest::deserialize(&data).map_err(|e| { @@ -2623,6 +2642,7 @@ impl ExportRouter { let manifest = state.volume_manifest.read(); let s3_bytes = manifest.chunks.len() as u64 * manifest.chunk_size; let generation = manifest.generation; + let lease_revision = manifest.lease_revision; drop(manifest); let info = ExportInfo { name: name.to_string(), @@ -2635,6 +2655,7 @@ impl ExportRouter { s3_bytes, fs_used_bytes: None, generation, + lease_revision, }; (info, Arc::clone(&state.handler)) }; @@ -2676,6 +2697,7 @@ impl ExportRouter { s3_bytes: manifest.chunks.len() as u64 * manifest.chunk_size, fs_used_bytes: None, generation: manifest.generation, + lease_revision: manifest.lease_revision, }; (info, Arc::clone(&state.handler)) }) diff --git a/glidefs/src/block/volume_manifest.rs b/glidefs/src/block/volume_manifest.rs index eedc433..ea94518 100644 --- a/glidefs/src/block/volume_manifest.rs +++ b/glidefs/src/block/volume_manifest.rs @@ -48,6 +48,27 @@ pub const VOLUME_MANIFEST_VERSION_GENERATION: u16 = 7; /// Version 8 — both the `prefetch_len` (v6) and `generation` (v7) trailers. pub const VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION: u16 = 8; +/// Version 9 widens the fencing token to the **composite** `(generation, +/// lease_revision)` (see [`super::fence`]). When `lease_revision > 0` the +/// manifest must persist both halves, so v9 carries a fixed 16-byte fence +/// trailer (`generation` u64 LE, then `lease_revision` u64 LE) after the chunk +/// entries (no prefetch trailer), before the CRC. +/// +/// v9/v10 are emitted ONLY when `lease_revision > 0` — a participating same-node +/// token. A volume fenced on the orchestrator generation alone (lease_revision +/// == 0) stays byte-identical to v5/v6/v7/v8, so existing exports and +/// generation-only callers see zero churn. A v9/v10 manifest read by a daemon +/// that predates these constants is rejected as +/// [`VolumeManifestError::UnsupportedVersion`]; same deploy-order caveat as v7 — +/// roll out the v9-aware binary before enabling lease-revision fencing. +/// - v9 — composite fence trailer only (no prefetch_len) +/// - v10 — prefetch_len (8B) then composite fence trailer (16B) +pub const VOLUME_MANIFEST_VERSION_COMPOSITE: u16 = 9; + +/// Version 10 — both the `prefetch_len` (v6) trailer and the v9 composite fence +/// trailer (`generation` then `lease_revision`). See [`VOLUME_MANIFEST_VERSION_COMPOSITE`]. +pub const VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE: u16 = 10; + /// Default chunk size for v4: 128 MiB (= 1 ext4 block group). pub const DEFAULT_CHUNK_SIZE: u64 = 128 * 1024 * 1024; @@ -82,12 +103,21 @@ pub struct VolumeManifest { /// the server warms `[0, prefetch_len)` into the clean cache so the guest's /// first reads are cache hits. `None` = no hint (legacy / non-prioritized). pub prefetch_len: Option, - /// Single-attach fencing token (owner generation). The highest generation - /// that has owned this volume; checked at attach and at every manifest sync - /// to fence a partitioned/superseded writer (see [`super::fence`]). `0` means - /// un-fenced (legacy export, or a caller that does not participate in - /// fencing) — the fence is a no-op for generation 0. + /// Single-attach fencing token, high half: the orchestrator's per-instance + /// placement generation. The highest generation that has owned this volume; + /// checked (composed with [`Self::lease_revision`]) at attach and at every + /// manifest sync to fence a partitioned/superseded writer (see + /// [`super::fence`]). `0` = un-fenced on this half. pub generation: u64, + /// Single-attach fencing token, low half: the node-lease claim revision. + /// Orders same-`node_id` incarnations (a node-death re-fork onto the same + /// node does NOT advance `generation`, so this is what fences it). The fence + /// compares the composite `(generation, lease_revision)`; see + /// [`super::fence::compose_token`]. `0` = un-fenced on this half (legacy + /// export, generation-only caller, or non-participating caller). Persisted + /// only when `> 0` (manifest v9/v10), so generation-only exports stay + /// byte-identical to v5/v6/v7/v8. + pub lease_revision: u64, } #[derive(Debug, thiserror::Error)] @@ -116,6 +146,7 @@ impl VolumeManifest { chunks: BTreeMap::new(), prefetch_len: None, generation: 0, + lease_revision: 0, } } @@ -267,20 +298,40 @@ impl VolumeManifest { .map(|e| 4 + 2 + e.packs.len() * 8) // chunk_idx + pack_count(u16) + packs .sum(); // v6 appends an 8-byte prefetch_len trailer; v7 an 8-byte generation - // trailer; v8 both. Each is emitted only when present, so an un-hinted, - // un-fenced manifest stays byte-identical to v5 (no churn). + // trailer; v8 both. v9/v10 widen the fence to a 16-byte composite + // trailer (generation + lease_revision). Each is emitted only when + // present, so an un-hinted, un-fenced manifest stays byte-identical to + // v5 (no churn), and a generation-only fence stays byte-identical to + // v7/v8 (the lease half only forces v9/v10 once it is non-zero). let has_prefetch = self.prefetch_len.is_some(); + let has_lease_revision = self.lease_revision > 0; let has_generation = self.generation > 0; let prefetch_trailer = if has_prefetch { 8 } else { 0 }; - let generation_trailer = if has_generation { 8 } else { 0 }; - let version = match (has_prefetch, has_generation) { - (false, false) => VOLUME_MANIFEST_VERSION, - (true, false) => VOLUME_MANIFEST_VERSION_PREFETCH, - (false, true) => VOLUME_MANIFEST_VERSION_GENERATION, - (true, true) => VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION, + // Fence trailer: 16 bytes (composite) when lease_revision participates, + // else 8 bytes (generation only) when generation participates, else 0. + let fence_trailer = if has_lease_revision { + 16 + } else if has_generation { + 8 + } else { + 0 + }; + let version = if has_lease_revision { + if has_prefetch { + VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE + } else { + VOLUME_MANIFEST_VERSION_COMPOSITE + } + } else { + match (has_prefetch, has_generation) { + (false, false) => VOLUME_MANIFEST_VERSION, + (true, false) => VOLUME_MANIFEST_VERSION_PREFETCH, + (false, true) => VOLUME_MANIFEST_VERSION_GENERATION, + (true, true) => VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION, + } }; let total = - GLVM_HEADER_SIZE + entries_size + prefetch_trailer + generation_trailer + 4; // +4 for CRC32 + GLVM_HEADER_SIZE + entries_size + prefetch_trailer + fence_trailer + 4; // +4 for CRC32 let mut buf = Vec::with_capacity(total); // Header (32 bytes). @@ -309,11 +360,17 @@ impl VolumeManifest { } // Trailers (before CRC, covered by it). Order is fixed: prefetch_len - // first, then generation — deserialize reads them in the same order. + // first, then the fence trailer — deserialize reads them in the same + // order. The fence trailer is the composite (generation + lease_revision, + // 16 bytes) when lease_revision participates (v9/v10), else generation + // only (8 bytes, v7/v8). if let Some(len) = self.prefetch_len { buf.extend_from_slice(&len.to_le_bytes()); } - if has_generation { + if has_lease_revision { + buf.extend_from_slice(&self.generation.to_le_bytes()); + buf.extend_from_slice(&self.lease_revision.to_le_bytes()); + } else if has_generation { buf.extend_from_slice(&self.generation.to_le_bytes()); } @@ -353,17 +410,27 @@ impl VolumeManifest { | VOLUME_MANIFEST_VERSION_PREFETCH | VOLUME_MANIFEST_VERSION_GENERATION | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + | VOLUME_MANIFEST_VERSION_COMPOSITE + | VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE ) { return Err(VolumeManifestError::UnsupportedVersion(u32::from(version))); } let has_prefetch = matches!( version, - VOLUME_MANIFEST_VERSION_PREFETCH | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + VOLUME_MANIFEST_VERSION_PREFETCH + | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION + | VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE ); + // v7/v8 carry a generation-only (8-byte) fence trailer. let has_generation = matches!( version, VOLUME_MANIFEST_VERSION_GENERATION | VOLUME_MANIFEST_VERSION_PREFETCH_GENERATION ); + // v9/v10 carry the composite (16-byte) fence trailer: generation + lease_revision. + let has_composite = matches!( + version, + VOLUME_MANIFEST_VERSION_COMPOSITE | VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE + ); let chunk_count = u32::from_le_bytes(data[6..10].try_into().unwrap()) as usize; let chunk_size = u64::from(u32::from_le_bytes(data[10..14].try_into().unwrap())); let block_size = u32::from_le_bytes(data[14..18].try_into().unwrap()); @@ -411,13 +478,23 @@ impl VolumeManifest { } else { None }; - let generation = if has_generation { + let (generation, lease_revision) = if has_composite { + // v9/v10: composite (generation u64 LE, then lease_revision u64 LE). + if pos + 16 > crc_offset { + return Err(VolumeManifestError::TooShort); + } + let g = u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap()); + let l = u64::from_le_bytes(data[pos + 8..pos + 16].try_into().unwrap()); + (g, l) + } else if has_generation { + // v7/v8: generation only; lease_revision defaults to 0 (back-compat). if pos + 8 > crc_offset { return Err(VolumeManifestError::TooShort); } - u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap()) + let g = u64::from_le_bytes(data[pos..pos + 8].try_into().unwrap()); + (g, 0) } else { - 0 + (0, 0) }; Ok(VolumeManifest { @@ -427,6 +504,7 @@ impl VolumeManifest { chunks, prefetch_len, generation, + lease_revision, }) } } @@ -531,6 +609,104 @@ mod tests { let back = VolumeManifest::deserialize(&bytes).unwrap(); assert_eq!(back.prefetch_len, Some(7 * 1024 * 1024)); assert_eq!(back.generation, 99); + assert_eq!(back.lease_revision, 0); + assert_eq!(back, m); + } + + /// lease_revision 0 (un-participating low half) never forces v9/v10: a + /// generation-only fence stays byte-identical to v7, so existing fenced + /// exports see zero churn. + #[test] + fn lease_revision_zero_stays_v7() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.generation = 42; + m.lease_revision = 0; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_GENERATION, + "lease_revision 0 must stay v7 (generation-only)" + ); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.generation, 42); + assert_eq!(back.lease_revision, 0); + assert_eq!(back, m); + } + + /// A composite fence (lease_revision > 0, no prefetch) is written as v9 and + /// both halves round-trip. This is the same-node re-fork case: generation may + /// be unchanged while the lease revision advances. + #[test] + fn composite_is_v9_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.append_pack(3, 9); + m.generation = 7; + m.lease_revision = 4; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_COMPOSITE, + "a composite fence must be written as v9" + ); + // Trailer: generation (8) then lease_revision (8), before the CRC (4). + let gen_bytes = &bytes[bytes.len() - 20..bytes.len() - 12]; + let lease_bytes = &bytes[bytes.len() - 12..bytes.len() - 4]; + assert_eq!(u64::from_le_bytes(gen_bytes.try_into().unwrap()), 7); + assert_eq!(u64::from_le_bytes(lease_bytes.try_into().unwrap()), 4); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.generation, 7); + assert_eq!(back.lease_revision, 4); + assert_eq!(back, m); + } + + /// A lease-only composite (generation 0, lease_revision > 0) still persists + /// both halves as v9 and round-trips — a participating same-node token need + /// not carry an orchestrator generation. + #[test] + fn composite_lease_only_is_v9_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.generation = 0; + m.lease_revision = 5; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_COMPOSITE, + ); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.generation, 0); + assert_eq!(back.lease_revision, 5); + assert_eq!(back, m); + } + + /// prefetch + composite fence → v10, with prefetch_len first then the + /// composite (generation, lease_revision), all round-tripping. + #[test] + fn prefetch_and_composite_is_v10_and_roundtrips() { + let mut m = VolumeManifest::new(8 * 1024 * 1024, 4096); + m.append_pack(0, 1); + m.prefetch_len = Some(3 * 1024 * 1024); + m.generation = 12; + m.lease_revision = 8; + let bytes = m.serialize().unwrap(); + assert_eq!( + u16::from_le_bytes(bytes[4..6].try_into().unwrap()), + VOLUME_MANIFEST_VERSION_PREFETCH_COMPOSITE, + "prefetch + composite must be written as v10" + ); + // Trailer order: prefetch_len (8), generation (8), lease_revision (8), CRC (4). + let plen = &bytes[bytes.len() - 28..bytes.len() - 20]; + let gen_bytes = &bytes[bytes.len() - 20..bytes.len() - 12]; + let lease_bytes = &bytes[bytes.len() - 12..bytes.len() - 4]; + assert_eq!(u64::from_le_bytes(plen.try_into().unwrap()), 3 * 1024 * 1024); + assert_eq!(u64::from_le_bytes(gen_bytes.try_into().unwrap()), 12); + assert_eq!(u64::from_le_bytes(lease_bytes.try_into().unwrap()), 8); + let back = VolumeManifest::deserialize(&bytes).unwrap(); + assert_eq!(back.prefetch_len, Some(3 * 1024 * 1024)); + assert_eq!(back.generation, 12); + assert_eq!(back.lease_revision, 8); assert_eq!(back, m); } diff --git a/glidefs/src/block/write_cache/flush.rs b/glidefs/src/block/write_cache/flush.rs index e7cb028..7a306cd 100644 --- a/glidefs/src/block/write_cache/flush.rs +++ b/glidefs/src/block/write_cache/flush.rs @@ -1490,11 +1490,13 @@ impl WriteCache { total_stats.touched_chunks.insert(chunk_idx); vm.append_pack(chunk_idx, pack_id); } - // Stamp our fencing generation into every synced manifest so a - // strictly-older incumbent's sync is rejected (and so a fresh node - // learns the owner generation). 0 = un-fenced (zero churn: the - // manifest stays v5/v6). + // Stamp our composite fencing token (generation + lease_revision) + // into every synced manifest so a strictly-older incumbent's sync is + // rejected (and so a fresh node learns the owner token). 0/0 = + // un-fenced (zero churn: the manifest stays v5/v6); a generation-only + // token (lease_revision 0) stays v7/v8. vm.generation = *self.inner.current_generation.lock(); + vm.lease_revision = *self.inner.current_lease_revision.lock(); } #[cfg(feature = "test-utils")] @@ -1529,10 +1531,13 @@ impl WriteCache { } Err(e @ crate::block::content_store::ContentStoreError::PreconditionFailed(_)) => { // Another writer moved the manifest. Distinguish a single- - // attach FENCE (a strictly-newer generation took the volume - // — terminal) from a transient same-generation race. + // attach FENCE (a strictly-newer composite token took the + // volume — terminal) from a transient same-token race. let held = *self.inner.current_generation.lock(); - if held > 0 { + let held_lease = *self.inner.current_lease_revision.lock(); + let held_token = + crate::block::fence::compose_token(held, held_lease); + if held_token > 0 { match content_store.get_manifest(&self.inner.export_name).await { Ok(Some((data, _etag))) => { if let Ok(s3_vm) = @@ -1540,11 +1545,19 @@ impl WriteCache { &data, ) { - if s3_vm.generation > held { + let s3_token = crate::block::fence::compose_token( + s3_vm.generation, + s3_vm.lease_revision, + ); + if s3_token > held_token { // Superseded — we are fenced. Latch the // flag so the write path rejects further // guest writes, then return terminal so - // the scheduler stops. + // the scheduler stops. A strictly-newer + // composite covers BOTH a cross-node + // re-placement (higher generation) and a + // same-node re-fork (same generation, + // higher lease_revision). self.inner.fenced.store( true, std::sync::atomic::Ordering::Release, @@ -1552,8 +1565,10 @@ impl WriteCache { warn!( export = %self.inner.export_name, held, + held_lease, superseded_by = s3_vm.generation, - "manifest sync FENCED: volume taken by a newer generation" + superseded_by_lease = s3_vm.lease_revision, + "manifest sync FENCED: volume taken by a newer token" ); return Err(CacheError::Fenced { held, diff --git a/glidefs/src/block/write_cache/init.rs b/glidefs/src/block/write_cache/init.rs index 7084404..7fd3260 100644 --- a/glidefs/src/block/write_cache/init.rs +++ b/glidefs/src/block/write_cache/init.rs @@ -372,6 +372,7 @@ impl WriteCache { flush_lock: tokio::sync::Mutex::new(()), manifest_etag: parking_lot::Mutex::new(None), current_generation: parking_lot::Mutex::new(0), + current_lease_revision: parking_lot::Mutex::new(0), fenced: std::sync::atomic::AtomicBool::new(false), page_crcs: dashmap::DashMap::new(), pages_per_block: block_size / 4096, @@ -464,6 +465,7 @@ impl WriteCache { flush_lock: tokio::sync::Mutex::new(()), manifest_etag: parking_lot::Mutex::new(None), current_generation: parking_lot::Mutex::new(0), + current_lease_revision: parking_lot::Mutex::new(0), fenced: std::sync::atomic::AtomicBool::new(false), page_crcs: dashmap::DashMap::new(), pages_per_block: block_size / 4096, diff --git a/glidefs/src/block/write_cache/inner.rs b/glidefs/src/block/write_cache/inner.rs index e736d0b..32e2b33 100644 --- a/glidefs/src/block/write_cache/inner.rs +++ b/glidefs/src/block/write_cache/inner.rs @@ -512,6 +512,15 @@ pub(crate) struct CacheInner { /// un-fenced (legacy / non-participating caller); the fence is a no-op. pub(crate) current_generation: Mutex, + /// Single-attach fencing token, low half: the node-lease claim revision this + /// writer was granted at attach. Composed with [`Self::current_generation`] + /// into the `u128` fence token (see [`crate::block::fence::compose_token`]), + /// stamped into the manifest on every sync, and compared against S3 on a + /// precondition failure. Orders same-`node_id` incarnations (a node-death + /// re-fork does NOT advance the orchestrator generation). `0` = un-fenced on + /// this half. + pub(crate) current_lease_revision: Mutex, + /// Set once this writer is FENCED — a strictly-newer generation took the /// volume (detected at manifest sync). Latches forever: once fenced, the /// write path rejects further guest writes so the guest stops receiving