diff --git a/crates/grafeo-common/src/utils/error.rs b/crates/grafeo-common/src/utils/error.rs index 656c9ca55..a76941133 100644 --- a/crates/grafeo-common/src/utils/error.rs +++ b/crates/grafeo-common/src/utils/error.rs @@ -66,6 +66,8 @@ pub enum ErrorCode { StorageCorrupted, /// Recovery from WAL failed. StorageRecoveryFailed, + /// Requested direct memory mapping is unavailable for this storage layout. + StorageDirectMmapUnavailable, // Validation errors (V) /// Request validation failed. @@ -112,6 +114,7 @@ impl ErrorCode { Self::StorageFull => "GRAFEO-S001", Self::StorageCorrupted => "GRAFEO-S002", Self::StorageRecoveryFailed => "GRAFEO-S003", + Self::StorageDirectMmapUnavailable => "GRAFEO-S004", Self::InvalidInput => "GRAFEO-V001", Self::NodeNotFound => "GRAFEO-V002", @@ -350,6 +353,9 @@ pub enum StorageError { /// Checkpoint failed. CheckpointFailed(String), + + /// The section cannot be served through the direct mapped-read path. + DirectMmapUnavailable(String), } impl StorageError { @@ -361,6 +367,7 @@ impl StorageError { Self::Full => ErrorCode::StorageFull, Self::InvalidWalEntry(_) | Self::CheckpointFailed(_) => ErrorCode::StorageCorrupted, Self::RecoveryFailed(_) => ErrorCode::StorageRecoveryFailed, + Self::DirectMmapUnavailable(_) => ErrorCode::StorageDirectMmapUnavailable, } } } @@ -373,6 +380,9 @@ impl fmt::Display for StorageError { StorageError::InvalidWalEntry(msg) => write!(f, "Invalid WAL entry: {msg}"), StorageError::RecoveryFailed(msg) => write!(f, "Recovery failed: {msg}"), StorageError::CheckpointFailed(msg) => write!(f, "Checkpoint failed: {msg}"), + StorageError::DirectMmapUnavailable(msg) => { + write!(f, "Direct mmap unavailable: {msg}") + } } } } diff --git a/crates/grafeo-core/src/graph/compact/mod.rs b/crates/grafeo-core/src/graph/compact/mod.rs index d26511bb3..e6e602d85 100644 --- a/crates/grafeo-core/src/graph/compact/mod.rs +++ b/crates/grafeo-core/src/graph/compact/mod.rs @@ -38,6 +38,7 @@ pub use builder::{CompactStoreBuilder, from_graph_store, from_graph_store_preser use std::sync::Arc; use arcstr::ArcStr; +use bytes::Bytes; use grafeo_common::types::{EdgeId, NodeId}; use grafeo_common::utils::hash::FxHashMap; @@ -82,6 +83,14 @@ pub struct CompactStore { node_offset_to_id: Option>>, /// Reverse: rel_table_id index -> vec of original `EdgeId` per CSR position. edge_offset_to_id: Option>>, + /// Full container mapping retained for a direct mapped read-only reopen. + /// + /// This is an owner handle only: it does not copy the mapped payload and + /// is intentionally excluded from [`Self::memory_bytes`]. The mapped + /// graph-view work in G-EM0.2 replaces the remaining proportional decoded + /// structures; retaining this handle guarantees no codec-free snapshot can + /// accidentally unmap while readers still hold the CompactStore. + mapped_backing: Option, } impl std::fmt::Debug for CompactStore { @@ -149,9 +158,26 @@ impl CompactStore { edge_id_map: None, node_offset_to_id: None, edge_offset_to_id: None, + mapped_backing: None, } } + /// Retains the owner for a verified direct container mapping. + /// + /// The mapping is released when this `CompactStore` and every clone of the + /// owner `Bytes` have been dropped. Callers must only pass bytes produced + /// from a successfully validated immutable container section. + pub fn retain_mapped_backing(&mut self, mapped_bytes: Bytes) { + self.mapped_backing = Some(mapped_bytes); + } + + /// Returns the direct-container mapping length when this store was opened + /// from one, excluding it from anonymous heap accounting. + #[must_use] + pub fn mapped_backing_bytes(&self) -> Option { + self.mapped_backing.as_ref().map(Bytes::len) + } + /// Resolves a table_id to its [`NodeTable`]. #[inline] fn resolve_node_table(&self, table_id: u16) -> Option<&NodeTable> { diff --git a/crates/grafeo-core/src/graph/compact/section.rs b/crates/grafeo-core/src/graph/compact/section.rs index 4242bb484..94fd6cee5 100644 --- a/crates/grafeo-core/src/graph/compact/section.rs +++ b/crates/grafeo-core/src/graph/compact/section.rs @@ -104,6 +104,33 @@ impl CompactStoreSection { Ok(()) } + /// Deserializes from a verified direct container mapping. + /// + /// Unlike [`Self::deserialize_from_bytes`], this retains one owner handle + /// on the resulting [`CompactStore`] so a mapping survives even when a + /// particular snapshot happens not to contain a column codec slice. + /// `data` must originate from the checked `GrafeoFileManager::mmap_section` + /// path; callers cannot use this to bypass container CRC validation. + /// + /// # Errors + /// + /// Returns an error when the mapped payload is truncated, CRC-invalid at + /// the CompactStore layer, or otherwise fails the standard v1–v4 codec + /// reader. Failures do not expose unchecked slices to the caller. + pub fn deserialize_from_mapped_bytes( + &mut self, + data: bytes::Bytes, + ) -> grafeo_common::utils::error::Result<()> { + let mut store = deserialize_compact_store(&data).map_err(|e| { + grafeo_common::utils::error::Error::Internal(format!( + "CompactStore deserialization failed: {e}" + )) + })?; + store.retain_mapped_backing(data); + *self.store.write() = Some(Arc::new(store)); + Ok(()) + } + /// Serializes at the requested format version. /// /// The default [`Section::serialize`] always writes [`FORMAT_VERSION`]. diff --git a/crates/grafeo-engine/src/database/mod.rs b/crates/grafeo-engine/src/database/mod.rs index 31c3b4bf0..ab0d996f5 100644 --- a/crates/grafeo-engine/src/database/mod.rs +++ b/crates/grafeo-engine/src/database/mod.rs @@ -87,6 +87,41 @@ use crate::query::cache::QueryCache; use crate::session::Session; use crate::transaction::TransactionManager; +/// Actual backing selected for a reopened CompactStore base. +/// +/// This is intentionally an observed runtime diagnostic rather than a +/// configuration echo. In particular, `ContainerMmap` is only reported after +/// `GrafeoFileManager::mmap_section` has verified the selected section CRC and +/// the CompactStore has accepted its owner-backed bytes. +#[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum CompactBacking { + /// Read-only base is owned by the immutable CompactStore section mapping. + ContainerMmap { + /// Stable identifier for this immutable container artifact. + artifact_id: String, + /// CompactStore payload version reported by the verified header. + payload_version: u8, + /// Bytes mapped from the container section; file-backed, not heap. + mapped_bytes: usize, + }, + /// Compatibility path for writable opens and legacy layouts. + LegacyEager { + /// Stable identifier for the container artifact read eagerly. + artifact_id: String, + /// CompactStore payload version reported by the verified header. + payload_version: u8, + /// Complete payload bytes allocated by the compatibility reader. + payload_bytes: usize, + }, +} + +#[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] +struct LoadedCompactBase { + store: Arc, + backing: CompactBacking, +} + /// Your handle to a Grafeo database. /// /// Start here. Create one with [`new_in_memory()`](Self::new_in_memory) for @@ -199,6 +234,9 @@ pub struct GrafeoDB { /// `layered_store` via `swap_base()`. #[cfg(all(feature = "compact-store", feature = "mmap", feature = "lpg"))] compact_tiered: Option>, + /// Observed compact-base backing after a persistent reopen. + #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] + compact_backing: Option, } impl GrafeoDB { @@ -393,9 +431,7 @@ impl GrafeoDB { // compacted database. The post-construction wiring uses this to // rebuild the LayeredStore + tier wrapper + overlay consumer. #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] - let mut loaded_compact_base: Option< - Arc, - > = None; + let mut loaded_compact_base: Option = None; // Phase 5e: snapshot of the OverlayDeletions section (if present), // applied after the LayeredStore is wired so that previously-deleted @@ -425,7 +461,7 @@ impl GrafeoDB { )?; #[cfg(feature = "compact-store")] { - loaded_compact_base = Self::extract_compact_base(&fm)?; + loaded_compact_base = Self::extract_compact_base(&fm, true)?; loaded_overlay_deletions = Self::extract_overlay_deletions(&fm)?; } } else { @@ -483,7 +519,7 @@ impl GrafeoDB { )?; #[cfg(feature = "compact-store")] { - loaded_compact_base = Self::extract_compact_base(&fm)?; + loaded_compact_base = Self::extract_compact_base(&fm, false)?; loaded_overlay_deletions = Self::extract_overlay_deletions(&fm)?; } } else { @@ -661,6 +697,8 @@ impl GrafeoDB { layered_store: None, #[cfg(all(feature = "compact-store", feature = "mmap", feature = "lpg"))] compact_tiered: None, + #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] + compact_backing: None, }; // Register storage sections as memory consumers for pressure tracking @@ -672,8 +710,9 @@ impl GrafeoDB { // engine sees the full picture and the read/write paths route // through the layered store. #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] - if let Some(compact_base) = loaded_compact_base { - db.wire_layered_after_load(compact_base, loaded_overlay_deletions)?; + if let Some(loaded) = loaded_compact_base { + db.compact_backing = Some(loaded.backing); + db.wire_layered_after_load(loaded.store, loaded_overlay_deletions)?; } // After Catalog shells + VectorStore topology + WAL + layered wiring, @@ -815,6 +854,8 @@ impl GrafeoDB { layered_store: None, #[cfg(all(feature = "compact-store", feature = "mmap", feature = "lpg"))] compact_tiered: None, + #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] + compact_backing: None, }) } @@ -906,6 +947,8 @@ impl GrafeoDB { layered_store: None, #[cfg(all(feature = "compact-store", feature = "mmap", feature = "lpg"))] compact_tiered: None, + #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] + compact_backing: None, }) } @@ -1524,10 +1567,20 @@ impl GrafeoDB { /// section file, if present. Used by the open path to reconstruct /// the LayeredStore wiring after a previously-compacted database /// reopens. + /// + /// When `direct_mmap` is true (read-only open), this path never calls + /// [`GrafeoFileManager::read_section_data`] and never copies the full + /// section into an owned payload buffer. Mapping failures — including + /// encrypted layouts — fail closed with + /// [`StorageError::DirectMmapUnavailable`](grafeo_common::utils::error::StorageError::DirectMmapUnavailable) + /// rather than silently falling back to eager materialization. + /// Writable opens keep the legacy eager path and report + /// [`CompactBacking::LegacyEager`]; that is not Milestone R evidence. #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] fn extract_compact_base( fm: &GrafeoFileManager, - ) -> Result>> { + direct_mmap: bool, + ) -> Result> { use grafeo_common::storage::{Section, SectionType}; let Some(dir) = fm.read_section_directory()? else { return Ok(None); @@ -1535,10 +1588,38 @@ impl GrafeoDB { let Some(entry) = dir.find(SectionType::CompactStore) else { return Ok(None); }; - let data = fm.read_section_data(entry)?; + let artifact_id = format!("container:{}:{:08x}", fm.path().display(), entry.checksum); let mut section = grafeo_core::graph::compact::section::CompactStoreSection::empty(); - section.deserialize(&data)?; - Ok(section.store()) + let backing = if direct_mmap { + let mapped = Arc::new(fm.mmap_section(entry)?); + let mapped_bytes = mapped.len(); + let data = mapped.into_bytes(); + let payload_version = data.get(4).copied().ok_or_else(|| { + Error::Internal("truncated CompactStore payload after mmap validation".into()) + })?; + section.deserialize_from_mapped_bytes(data)?; + CompactBacking::ContainerMmap { + artifact_id, + payload_version, + mapped_bytes, + } + } else { + let data = fm.read_section_data(entry)?; + let payload_version = data.get(4).copied().ok_or_else(|| { + Error::Internal("truncated CompactStore payload after read validation".into()) + })?; + let payload_bytes = data.len(); + section.deserialize(&data)?; + CompactBacking::LegacyEager { + artifact_id, + payload_version, + payload_bytes, + } + }; + let store = section.store().ok_or_else(|| { + Error::Internal("CompactStore section deserialized without a store".into()) + })?; + Ok(Some(LoadedCompactBase { store, backing })) } /// Reads the persisted overlay deletion log from the container, if @@ -2365,6 +2446,18 @@ impl GrafeoDB { self.compact_tiered.as_ref() } + /// Returns the actual backing selected for a reopened CompactStore base. + /// + /// `ContainerMmap` proves that this handle owns the immutable container + /// mapping directly. It is distinct from the legacy spill-sidecar tier and + /// from a directory `mmap_able` flag, neither of which proves the normal + /// reopen path avoided an owned full-section read. + #[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] + #[must_use] + pub fn compact_backing(&self) -> Option<&CompactBacking> { + self.compact_backing.as_ref() + } + /// Returns the query cache. #[must_use] pub fn query_cache(&self) -> &Arc { diff --git a/crates/grafeo-engine/src/lib.rs b/crates/grafeo-engine/src/lib.rs index c22718171..a7d340844 100644 --- a/crates/grafeo-engine/src/lib.rs +++ b/crates/grafeo-engine/src/lib.rs @@ -51,6 +51,8 @@ pub use admin::{ pub use auth::{Grant, Identity, Role, StatementKind}; pub use catalog::{Catalog, CatalogError, IndexDefinition, IndexType}; pub use config::{AccessMode, Config, ConfigError, DurabilityMode, GraphModel}; +#[cfg(all(feature = "grafeo-file", feature = "lpg", feature = "compact-store"))] +pub use database::CompactBacking; pub use database::GrafeoDB; #[cfg(all(feature = "lpg", feature = "vector-index"))] pub use database::IndexedVectorRead; diff --git a/crates/grafeo-engine/tests/compact_store_direct_mmap.rs b/crates/grafeo-engine/tests/compact_store_direct_mmap.rs new file mode 100644 index 000000000..af4a73b38 --- /dev/null +++ b/crates/grafeo-engine/tests/compact_store_direct_mmap.rs @@ -0,0 +1,158 @@ +//! Read-only CompactStore container mapping regression coverage. +//! +//! Support matrix exercised here: +//! - plaintext, mmap-able CompactStore sections → `CompactBacking::ContainerMmap` +//! - graph point lookups and CSR traversal remain byte-identical after mapped open +//! - retained mapping outlives the database handle while any base Arc is held +//! +//! Encrypted / non-mmap-able layouts return `StorageError::DirectMmapUnavailable` +//! from `GrafeoFileManager::mmap_section` (covered in storage unit tests) and must +//! never claim `ContainerMmap`. +//! +//! ```bash +//! cargo test -p grafeo-engine --features compact-store \ +//! --test compact_store_direct_mmap -- --nocapture +//! ``` + +#![cfg(all(feature = "compact-store", feature = "grafeo-file", feature = "lpg"))] + +use grafeo_common::storage::SectionType; +use grafeo_common::types::{NodeId, PropertyKey, Value}; +use grafeo_core::graph::{Direction, traits::GraphStore}; +use grafeo_engine::{CompactBacking, Config, GrafeoDB}; +use grafeo_storage::file::GrafeoFileManager; + +#[test] +fn readonly_reopen_owns_the_compact_container_mapping_and_preserves_graph_reads() { + let temp = tempfile::tempdir().expect("tempdir"); + let path = temp.path().join("direct-mapped.grafeo"); + + // Build a small multi-node graph so reopen exercises deterministic random + // property reads, not only the first two rows. + let (nodes, knows): (Vec<(NodeId, String, i64)>, _) = { + let mut db = GrafeoDB::with_config(Config::persistent(&path)).expect("create database"); + let mut nodes = Vec::new(); + for (name, rank) in [ + ("Alix", 1_i64), + ("Gus", 2), + ("Mara", 3), + ("Ned", 4), + ("Ora", 5), + ] { + let id = db + .create_node_with_props( + &["Person"], + [("name", Value::from(name)), ("rank", Value::Int64(rank))], + ) + .expect("create person"); + nodes.push((id, name.to_string(), rank)); + } + let knows = db.create_edge(nodes[0].0, nodes[1].0, "KNOWS"); + let _also = db.create_edge(nodes[2].0, nodes[3].0, "KNOWS"); + db.compact().expect("compact base"); + db.close().expect("explicit close"); + (nodes, knows) + }; + + let expected_payload_bytes = { + let manager = GrafeoFileManager::open_read_only(&path).expect("open container"); + let directory = manager + .read_section_directory() + .expect("read directory") + .expect("section directory"); + let entry = directory + .find(SectionType::CompactStore) + .expect("CompactStore section"); + assert!( + entry.flags.mmap_able, + "CompactStore directory flag is necessary but not sufficient for ContainerMmap" + ); + entry.length as usize + }; + + let db = GrafeoDB::open_read_only(&path).expect("read-only reopen"); + let CompactBacking::ContainerMmap { + artifact_id, + payload_version, + mapped_bytes, + } = db.compact_backing().expect("compact backing diagnostic") + else { + panic!("read-only CompactStore reopen must use ContainerMmap"); + }; + assert!(artifact_id.starts_with("container:")); + assert_eq!(*payload_version, 4, "current direct compatibility payload"); + assert_eq!(*mapped_bytes, expected_payload_bytes); + + let base = db + .layered_store() + .expect("layered store after compact reopen") + .base_store_arc(); + assert_eq!(base.mapped_backing_bytes(), Some(expected_payload_bytes)); + // Mapped payload is file-backed; retained heap is tracked separately. + assert!( + base.memory_bytes() > 0, + "decoded metadata remains heap-accounted for G-EM0.2 budgeting" + ); + + // Deterministic pseudo-random order (fixed LCG) over node property reads. + let mut state: u64 = 0xC0FFEE; + for _ in 0..16 { + state = state.wrapping_mul(6364136223846793005).wrapping_add(1); + let idx = (state as usize) % nodes.len(); + let (id, name, rank) = &nodes[idx]; + assert_eq!( + base.get_node_property(*id, &PropertyKey::new("name")), + Some(Value::from(name.as_str())), + "point property name must remain identical for node {idx}" + ); + assert_eq!( + base.get_node_property(*id, &PropertyKey::new("rank")), + Some(Value::Int64(*rank)), + "point property rank must remain identical for node {idx}" + ); + } + + let (alix, gus) = (nodes[0].0, nodes[1].0); + assert_eq!( + base.edges_from(alix, Direction::Outgoing), + vec![(gus, knows)] + ); + assert_eq!( + base.edges_from(gus, Direction::Incoming), + vec![(alix, knows)] + ); + assert_eq!( + base.edges_from(nodes[2].0, Direction::Outgoing).len(), + 1, + "second KNOW edge must survive mapped reopen" + ); + + // Release the shared RO lock before an exclusive writable open, but keep a + // base Arc so the mapping owner outlives the database handle. + db.close().expect("read-only close"); + assert_eq!( + base.get_node_property(gus, &PropertyKey::new("rank")), + Some(Value::Int64(2)), + "mapped owner must outlive the closed database handle" + ); + + // Writable reopen of the same artifact remains the explicit legacy path and + // must not claim Milestone R ContainerMmap evidence. + { + let writable = GrafeoDB::open(&path).expect("writable reopen"); + match writable.compact_backing() { + Some(CompactBacking::LegacyEager { payload_bytes, .. }) => { + assert_eq!(*payload_bytes, expected_payload_bytes); + } + other => panic!("writable reopen must report LegacyEager, got {other:?}"), + } + writable.close().expect("close writable handle"); + } + + // The earlier base Arc still serves reads through its retained mapping. + assert_eq!( + base.get_node_property(alix, &PropertyKey::new("name")), + Some(Value::from("Alix")) + ); + drop(base); +} diff --git a/crates/grafeo-storage/src/container/mmap.rs b/crates/grafeo-storage/src/container/mmap.rs index 7d1f84a84..8b11144ba 100644 --- a/crates/grafeo-storage/src/container/mmap.rs +++ b/crates/grafeo-storage/src/container/mmap.rs @@ -8,6 +8,9 @@ //! VectorStore, TextIndex, RdfRing, PropertyIndex). Data sections (Catalog, //! LpgStore, RdfStore) must be deserialized into RAM. +use std::sync::Arc; + +use bytes::Bytes; use grafeo_common::storage::SectionType; use super::page_fetcher::AccessHint; @@ -84,6 +87,17 @@ impl MmapSection { self.mmap.is_empty() } + /// Transfers a shared mapping owner into refcounted [`Bytes`]. + /// + /// Clones and slices of the returned `Bytes` retain the mapping until the + /// final view is dropped. This is the safe bridge used by container-backed + /// CompactStore reads: the bytes never borrow the file manager and no + /// fabricated `'static` lifetime is involved. + #[must_use] + pub fn into_bytes(self: Arc) -> Bytes { + Bytes::from_owner(MmapBytesOwner { mapping: self }) + } + /// Advise the OS about the expected access pattern for a range. /// /// On Unix this delegates to `madvise` via `memmap2`. On Windows @@ -117,6 +131,19 @@ impl MmapSection { } } +/// `Bytes::from_owner` needs an owner that directly exposes the mapped bytes. +/// Keeping the `Arc` here makes every `Bytes` clone/slice participate in the +/// mapping lifetime rather than tying it to a file-manager lock or scope. +struct MmapBytesOwner { + mapping: Arc, +} + +impl AsRef<[u8]> for MmapBytesOwner { + fn as_ref(&self) -> &[u8] { + self.mapping.as_bytes() + } +} + impl AsRef<[u8]> for MmapSection { fn as_ref(&self) -> &[u8] { &self.mmap @@ -132,3 +159,43 @@ impl std::fmt::Debug for MmapSection { .finish() } } + +#[cfg(test)] +mod tests { + use std::io::Write; + use std::sync::Arc; + + use super::MmapSection; + use grafeo_common::storage::SectionType; + + #[test] + fn bytes_views_retain_and_then_release_the_mapping_owner() { + let mut file = tempfile::NamedTempFile::new().expect("temp file"); + file.write_all(b"mapped CompactStore bytes") + .expect("write payload"); + file.flush().expect("flush payload"); + + #[allow(unsafe_code)] + let mmap = + unsafe { memmap2::MmapOptions::new().map(file.as_file()) }.expect("mmap payload"); + let mapping = Arc::new(MmapSection::new(mmap, SectionType::CompactStore, 0)); + let weak = Arc::downgrade(&mapping); + let bytes = Arc::clone(&mapping).into_bytes(); + let slice = bytes.slice(7..); + + drop(mapping); + assert!(weak.upgrade().is_some(), "Bytes must retain the mapping"); + assert_eq!(&slice[..], b"CompactStore bytes"); + + drop(bytes); + assert!( + weak.upgrade().is_some(), + "a live Bytes slice must retain the mapping" + ); + drop(slice); + assert!( + weak.upgrade().is_none(), + "the mapping must release after the final Bytes view drains" + ); + } +} diff --git a/crates/grafeo-storage/src/file/manager.rs b/crates/grafeo-storage/src/file/manager.rs index d978d8954..9efdc828d 100644 --- a/crates/grafeo-storage/src/file/manager.rs +++ b/crates/grafeo-storage/src/file/manager.rs @@ -9,7 +9,7 @@ use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use fs2::FileExt; -use grafeo_common::utils::error::{Error, Result}; +use grafeo_common::utils::error::{Error, Result, StorageError}; use parking_lot::Mutex; use super::format::{DATA_OFFSET, DbHeader, FileHeader}; @@ -729,24 +729,49 @@ impl GrafeoFileManager { /// /// Returns an error if: /// - The section is not mmap-able (data section) + /// - The section is encrypted (typed `DirectMmapUnavailable`) /// - The mmap system call fails /// - The CRC-32 checksum does not match (corrupt data) + /// + /// # Direct-mmap support matrix (G-EM0.1) + /// + /// | Layout | Direct mmap | + /// | --- | --- | + /// | Plaintext, `mmap_able`, non-zero length CompactStore/index | yes (CRC-verified) | + /// | Encrypted section (AES-GCM) | no — `DirectMmapUnavailable` | + /// | Non-`mmap_able` data section (LPG, Catalog, …) | no — `DirectMmapUnavailable` | + /// | Zero-length section | no — `DirectMmapUnavailable` | + /// | Compressed payload (none shipped today) | would need a separate design | + /// + /// CRC validation may fault every page into the OS file cache; it must not + /// copy the section into an anonymous `Vec`. Callers must not fall back to + /// `read_section_data` while still reporting a mapped backing diagnostic. #[allow(unsafe_code)] pub fn mmap_section( &self, entry: &grafeo_common::storage::SectionDirectoryEntry, ) -> Result { + // Direct mapping is valid only for the plaintext container bytes. + // AES-GCM sections require whole-section decryption today, so letting + // callers mmap ciphertext would either expose invalid bytes or tempt a + // silent eager fallback. A future page-decryption design can add a + // distinct mapped backend without weakening this fail-closed contract. + #[cfg(feature = "encryption")] + if self.section_encryptor.is_some() { + return Err(Error::Storage(StorageError::DirectMmapUnavailable( + "encrypted sections require page decryption before direct mmap".to_string(), + ))); + } + if !entry.flags.mmap_able { - return Err(Error::Internal(format!( - "section {:?} is not mmap-able (data sections must be deserialized)", - entry.section_type + return Err(Error::Storage(StorageError::DirectMmapUnavailable( + format!("section {:?} is not mmap-able", entry.section_type), ))); } if entry.length == 0 { - return Err(Error::Internal(format!( - "section {:?} has zero length, cannot mmap", - entry.section_type + return Err(Error::Storage(StorageError::DirectMmapUnavailable( + format!("section {:?} has zero length", entry.section_type), ))); } @@ -1297,7 +1322,16 @@ mod tests { let result = manager.mmap_section(lpg_entry); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("not mmap-able")); + let err = result.unwrap_err(); + assert!( + err.to_string().contains("not mmap-able"), + "unexpected error text: {err}" + ); + // Typed fail-closed result — never a silent eager materialization. + match err { + Error::Storage(StorageError::DirectMmapUnavailable(_)) => {} + other => panic!("expected DirectMmapUnavailable, got {other:?}"), + } } #[test]