From 39a56f2b315194252a8fc5db01ef480bcfe228fc Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 15:27:46 -0400 Subject: [PATCH 01/12] stack: new module for a `Cache` stack For a read-write cache, we often want to direct new writes to a single cache directory (sharded or otherwise), but still attempt to service reads from a list of fallback read-only locations. The `stack::Cache` struct implement that pattern. TESTED=new smoke tests. --- src/lib.rs | 3 + src/stack.rs | 604 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 607 insertions(+) create mode 100644 src/stack.rs diff --git a/src/lib.rs b/src/lib.rs index db8a391..61af548 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,12 +4,15 @@ pub mod raw_cache; mod readonly; pub mod second_chance; mod sharded; +mod stack; mod trigger; pub use plain::PlainCache; pub use readonly::ReadOnlyCache; pub use readonly::ReadOnlyCacheBuilder; pub use sharded::ShardedCache; +pub use stack::Cache; +pub use stack::CacheBuilder; /// Sharded cache keys consist of a filename and two hash values. The /// two hashes should be computed by distinct functions of the key's diff --git a/src/stack.rs b/src/stack.rs new file mode 100644 index 0000000..f3779f4 --- /dev/null +++ b/src/stack.rs @@ -0,0 +1,604 @@ +//! We expect most callers to interact with Kismet via the `Cache` +//! struct defined here. A `Cache` hides the difference in behaviour +//! between plain and sharded caches via late binding, and lets +//! callers transparently handle misses by looking in a series of +//! secondary cache directories. +use std::fs::File; +use std::io::Error; +use std::io::ErrorKind; +use std::io::Result; +use std::path::Path; +use std::sync::Arc; + +use crate::Key; +use crate::PlainCache; +use crate::ReadOnlyCache; +use crate::ReadOnlyCacheBuilder; +use crate::ShardedCache; + +/// The `FullCache` trait exposes both read and write operations as +/// implemented by sharded and plain caches. +trait FullCache: + std::fmt::Debug + Sync + Send + std::panic::RefUnwindSafe + std::panic::UnwindSafe +{ + /// Returns a read-only file for `key` in the cache directory if + /// it exists, or None if there is no such file. + /// + /// Implicitly "touches" the cached file if it exists. + fn get(&self, key: Key) -> Result>; + + /// Inserts or overwrites the file at `value` as `key` in the + /// sharded cache directory. + /// + /// Always consumes the file at `value` on success; may consume it + /// on error. + fn set(&self, key: Key, value: &Path) -> Result<()>; + + /// Inserts the file at `value` as `key` in the cache directory if + /// there is no such cached entry already, or touches the cached + /// file if it already exists. + /// + /// Always consumes the file at `value` on success; may consume it + /// on error. + fn put(&self, key: Key, value: &Path) -> Result<()>; + + /// Marks the cached file `key` as newly used, if it exists. + /// + /// Returns whether a file for `key` exists in the cache. + fn touch(&self, key: Key) -> Result; +} + +impl FullCache for PlainCache { + fn get(&self, key: Key) -> Result> { + PlainCache::get(self, key.name) + } + + fn set(&self, key: Key, value: &Path) -> Result<()> { + PlainCache::set(self, key.name, value) + } + + fn put(&self, key: Key, value: &Path) -> Result<()> { + PlainCache::put(self, key.name, value) + } + + fn touch(&self, key: Key) -> Result { + PlainCache::touch(self, key.name) + } +} + +impl FullCache for ShardedCache { + fn get(&self, key: Key) -> Result> { + ShardedCache::get(self, key) + } + + fn set(&self, key: Key, value: &Path) -> Result<()> { + ShardedCache::set(self, key, value) + } + + fn put(&self, key: Key, value: &Path) -> Result<()> { + ShardedCache::put(self, key, value) + } + + fn touch(&self, key: Key) -> Result { + ShardedCache::touch(self, key) + } +} + +/// Construct a `Cache` with this builder. The resulting cache will +/// always first access its write-side cache (if defined), and, on +/// misses, will attempt to service `get` and `touch` calls by +/// iterating over the read-only caches. +#[derive(Debug, Default)] +pub struct CacheBuilder { + write_side: Option>, + read_side: ReadOnlyCacheBuilder, +} + +/// A `Cache` wraps either up to one plain or sharded read-write cache +/// in a convenient interface, and may optionally fulfill read +/// operations by deferring to a list of read-only cache when the +/// read-write cache misses. +#[derive(Clone, Debug, Default)] +pub struct Cache { + write_side: Option>, + read_side: ReadOnlyCache, +} + +impl CacheBuilder { + /// Returns a fresh empty builder. + pub fn new() -> Self { + Self::default() + } + + /// Sets the read-write cache directory to `path`. + /// + /// The read-write cache will be a plain cache directory if + /// `num_shards <= 1`, and a sharded directory otherwise. + pub fn writer(self, path: impl AsRef, num_shards: usize, total_capacity: usize) -> Self { + if num_shards <= 1 { + self.plain_writer(path, total_capacity) + } else { + self.sharded_writer(path, num_shards, total_capacity) + } + } + + /// Sets the read-write cache directory to a plain directory at + /// `path`, with a target file count of up to `capacity`. + pub fn plain_writer(mut self, path: impl AsRef, capacity: usize) -> Self { + self.write_side.insert(Arc::new(PlainCache::new( + path.as_ref().to_owned(), + capacity, + ))); + self + } + + /// Sets the read-write cache directory to a sharded directory at + /// `path`, with `num_shards` subdirectories and a target file + /// count of up to `capacity` for the entire cache. + pub fn sharded_writer( + mut self, + path: impl AsRef, + num_shards: usize, + total_capacity: usize, + ) -> Self { + self.write_side.insert(Arc::new(ShardedCache::new( + path.as_ref().to_owned(), + num_shards, + total_capacity, + ))); + self + } + + /// Adds a new read-only cache directory at `path` to the end of the + /// cache builder's search list. + /// + /// Adds a plain cache directory if `num_shards <= 1`, and a sharded + /// directory otherwise. + pub fn reader(mut self, path: impl AsRef, num_shards: usize) -> Self { + self.read_side = self.read_side.cache(path, num_shards); + self + } + + /// Adds a new plain (unsharded) read-only cache directory at + /// `path` to the end of the cache builder's search list. + pub fn plain_reader(mut self, path: impl AsRef) -> Self { + self.read_side = self.read_side.plain(path); + self + } + + /// Adds a new sharded read-only cache directory at `path` to the + /// end of the cache builder's search list. + pub fn sharded_reader(mut self, path: impl AsRef, num_shards: usize) -> Self { + self.read_side = self.read_side.sharded(path, num_shards); + self + } + + /// Returns a fresh `Cache` for the builder's write cache and + /// additional search list of read-only cache directories. + pub fn build(self) -> Cache { + Cache { + write_side: self.write_side, + read_side: self.read_side.build(), + } + } +} + +impl Cache { + /// Attempts to open a read-only file for `key`. The `Cache` will + /// query each its write cache (if any), followed by the list of + /// additional read-only cache, in definition order, and return a + /// read-only file for the first hit. + /// + /// Returns `None` if no file for `key` can be found in any of the + /// constituent caches, and bubbles up the first I/O error + /// encountered, if any. + pub fn get<'a>(&self, key: impl Into>) -> Result> { + fn doit( + write_side: Option<&dyn FullCache>, + read_side: &ReadOnlyCache, + key: Key, + ) -> Result> { + if let Some(write) = write_side { + if let Some(ret) = write.get(key)? { + return Ok(Some(ret)); + } + } + + read_side.get(key) + } + + doit( + self.write_side.as_ref().map(AsRef::as_ref), + &self.read_side, + key.into(), + ) + } + + /// Inserts or overwrites the file at `value` as `key` in the + /// write cache directory. This will always fail with + /// `Unsupported` if no write cache was defined. + /// + /// Always consumes the file at `value` on success; may consume it + /// on error. + pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { + match self.write_side.as_ref() { + Some(write) => write.set(key.into(), value.as_ref()), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } + } + + /// Inserts the file at `value` as `key` in the cache directory if + /// there is no such cached entry already, or touches the cached + /// file if it already exists. + /// + /// Always consumes the file at `value` on success; may consume it + /// on error. + pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { + match self.write_side.as_ref() { + Some(write) => write.put(key.into(), value.as_ref()), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } + } + + /// Marks a cache entry for `key` as accessed (read). The `Cache` + /// will touch the same file that would be returned by `get`. + /// + /// Returns whether a file for `key` could be found, and bubbles + /// up the first I/O error encountered, if any. + pub fn touch<'a>(&self, key: impl Into>) -> Result { + fn doit( + write_side: Option<&dyn FullCache>, + read_side: &ReadOnlyCache, + key: Key, + ) -> Result { + if let Some(write) = write_side { + if write.touch(key)? { + return Ok(true); + } + } + + read_side.touch(key) + } + + doit( + self.write_side.as_ref().map(AsRef::as_ref), + &self.read_side, + key.into(), + ) + } +} + +#[cfg(test)] +mod test { + use std::io::ErrorKind; + + use crate::Cache; + use crate::CacheBuilder; + use crate::Key; + use crate::PlainCache; + use crate::ShardedCache; + + struct TestKey { + key: String, + } + + impl TestKey { + fn new(key: &str) -> TestKey { + TestKey { + key: key.to_string(), + } + } + } + + impl<'a> From<&'a TestKey> for Key<'a> { + fn from(x: &'a TestKey) -> Key<'a> { + Key::new(&x.key, 0, 1) + } + } + + // No cache defined -> read calls should successfully do nothing, + // write calls should fail. + #[test] + fn empty() { + let cache: Cache = Default::default(); + + assert!(matches!(cache.get(&TestKey::new("foo")), Ok(None))); + assert!(matches!(cache.set(&TestKey::new("foo"), "/tmp/foo"), + Err(e) if e.kind() == ErrorKind::Unsupported)); + assert!(matches!(cache.put(&TestKey::new("foo"), "/tmp/foo"), + Err(e) if e.kind() == ErrorKind::Unsupported)); + assert!(matches!(cache.touch(&TestKey::new("foo")), Ok(false))); + } + + // Smoke test a wrapped plain cache. + #[test] + fn smoke_test_plain() { + use std::io::{Read, Write}; + use tempfile::NamedTempFile; + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp() + .create("cache", FileType::Dir) + .create("extra", FileType::Dir); + + // Populate the plain cache in `extra` with two files, "b" and "c". + { + let cache = PlainCache::new(temp.path("extra"), 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"extra") + .expect("write must succeed"); + + cache.put("b", tmp.path()).expect("put must succeed"); + + let tmp2 = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp2.as_file() + .write_all(b"extra2") + .expect("write must succeed"); + + cache.put("c", tmp2.path()).expect("put must succeed"); + } + + let cache = CacheBuilder::new() + .writer(temp.path("cache"), 1, 10) + .reader(temp.path("extra"), 1) + .build(); + + // There shouldn't be anything for "a" + assert!(matches!(cache.get(&TestKey::new("a")), Ok(None))); + assert!(matches!(cache.touch(&TestKey::new("a")), Ok(false))); + + // We should be able to touch "b" + assert!(matches!(cache.touch(&TestKey::new("b")), Ok(true))); + + // And its contents should match that of the "extra" cache dir. + { + let mut b_file = cache + .get(&TestKey::new("b")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + b_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"extra"); + } + + // Now populate "a" and "b" in the cache. + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write") + .expect("write must succeed"); + cache + .put(&TestKey::new("a"), tmp.path()) + .expect("put must succeed"); + } + + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write2") + .expect("write must succeed"); + cache + .put(&TestKey::new("b"), tmp.path()) + .expect("put must succeed"); + } + + // And overwrite "a" + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write3") + .expect("write must succeed"); + cache + .set(&TestKey::new("a"), tmp.path()) + .expect("set must succeed"); + } + + // We should find: + // a => write3 + // b => write2 + // c => extra2 + + // So we should be able to touch everything. + assert!(matches!(cache.touch(&TestKey::new("a")), Ok(true))); + assert!(matches!(cache.touch(&TestKey::new("b")), Ok(true))); + assert!(matches!(cache.touch(&TestKey::new("c")), Ok(true))); + + // And read the expected contents. + { + let mut a_file = cache + .get(&TestKey::new("a")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + a_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"write3"); + } + + { + let mut b_file = cache + .get(&TestKey::new("b")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + b_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"write2"); + } + + { + let mut c_file = cache + .get(&TestKey::new("c")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + c_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"extra2"); + } + } + + // Smoke test a wrapped sharded cache. + #[test] + fn smoke_test_sharded() { + use std::io::{Read, Write}; + use tempfile::NamedTempFile; + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp() + .create("cache", FileType::Dir) + .create("extra_plain", FileType::Dir) + .create("extra_sharded", FileType::Dir); + + // Populate the plain cache in `extra_plain` with one file, "b". + { + let cache = PlainCache::new(temp.path("extra_plain"), 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"extra_plain") + .expect("write must succeed"); + + cache.put("b", tmp.path()).expect("put must succeed"); + } + + // And now add "c" in the sharded `extra_sharded` cache. + { + let cache = ShardedCache::new(temp.path("extra_sharded"), 10, 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir(None).expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"extra_sharded") + .expect("write must succeed"); + + cache + .put((&TestKey::new("c")).into(), tmp.path()) + .expect("put must succeed"); + } + + let cache = CacheBuilder::new() + .plain_writer(temp.path("cache"), 10) + // Override the writer with a sharded cache + .writer(temp.path("cache"), 10, 10) + .plain_reader(temp.path("extra_plain")) + .sharded_reader(temp.path("extra_sharded"), 10) + .build(); + + // There shouldn't be anything for "a" + assert!(matches!(cache.get(&TestKey::new("a")), Ok(None))); + assert!(matches!(cache.touch(&TestKey::new("a")), Ok(false))); + + // We should be able to touch "b" + assert!(matches!(cache.touch(&TestKey::new("b")), Ok(true))); + + // And its contents should match that of the "extra" cache dir. + { + let mut b_file = cache + .get(&TestKey::new("b")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + b_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"extra_plain"); + } + + // Similarly for "c" + { + let mut c_file = cache + .get(&TestKey::new("c")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + c_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"extra_sharded"); + } + + // Now populate "a" and "b" in the cache. + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write") + .expect("write must succeed"); + cache + .set(&TestKey::new("a"), tmp.path()) + .expect("set must succeed"); + } + + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write2") + .expect("write must succeed"); + cache + .set(&TestKey::new("b"), tmp.path()) + .expect("set must succeed"); + } + + // And fail to update "a" with a put. + { + let tmp = NamedTempFile::new_in(temp.path(".")).expect("new temp file must succeed"); + + tmp.as_file() + .write_all(b"write3") + .expect("write must succeed"); + cache + .put(&TestKey::new("a"), tmp.path()) + .expect("put must succeed"); + } + + // We should find: + // a => write + // b => write2 + // c => extra_sharded + + // So we should be able to touch everything. + assert!(matches!(cache.touch(&TestKey::new("a")), Ok(true))); + assert!(matches!(cache.touch(&TestKey::new("b")), Ok(true))); + assert!(matches!(cache.touch(&TestKey::new("c")), Ok(true))); + + // And read the expected contents. + { + let mut a_file = cache + .get(&TestKey::new("a")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + a_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"write"); + } + + { + let mut b_file = cache + .get(&TestKey::new("b")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + b_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"write2"); + } + + { + let mut c_file = cache + .get(&TestKey::new("c")) + .expect("must succeed") + .expect("must exist"); + let mut dst = Vec::new(); + c_file.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"extra_sharded"); + } + } +} From 8bfc79d7b252e58abaf7ffbb408a2a1d1bce7dc4 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 16:43:40 -0400 Subject: [PATCH 02/12] stack: add convenience get_or_update / ensure functions The `get_or_update` method implements the common pattern of fetching a value from a cache and either filling the cache on miss, or otherwise determining whether we want to use the value verbatim or update it. When values never expire, we never want to update the value; that's what `ensure` implements. TESTED=new unit tests. --- Cargo.toml | 2 +- src/lib.rs | 2 + src/stack.rs | 508 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 511 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 684efcd..316e356 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,9 +8,9 @@ license = "MIT" [dependencies] filetime = "0.2" rand = "0.8" +tempfile = "3" [dev-dependencies] proptest = "1" proptest-derive = "0.3" -tempfile = "3" test_dir = "0.1" diff --git a/src/lib.rs b/src/lib.rs index 61af548..4293f56 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,8 @@ pub use readonly::ReadOnlyCacheBuilder; pub use sharded::ShardedCache; pub use stack::Cache; pub use stack::CacheBuilder; +pub use stack::CacheHit; +pub use stack::CacheHitAction; /// Sharded cache keys consist of a filename and two hash values. The /// two hashes should be computed by distinct functions of the key's diff --git a/src/stack.rs b/src/stack.rs index f3779f4..7deb82b 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -3,12 +3,14 @@ //! between plain and sharded caches via late binding, and lets //! callers transparently handle misses by looking in a series of //! secondary cache directories. +use std::borrow::Cow; use std::fs::File; use std::io::Error; use std::io::ErrorKind; use std::io::Result; use std::path::Path; use std::sync::Arc; +use tempfile::NamedTempFile; use crate::Key; use crate::PlainCache; @@ -27,6 +29,10 @@ trait FullCache: /// Implicitly "touches" the cached file if it exists. fn get(&self, key: Key) -> Result>; + /// Returns a temporary directory suitable for temporary files + /// that will be published as `key`. + fn temp_dir(&self, key: Key) -> Result>; + /// Inserts or overwrites the file at `value` as `key` in the /// sharded cache directory. /// @@ -53,6 +59,10 @@ impl FullCache for PlainCache { PlainCache::get(self, key.name) } + fn temp_dir(&self, _key: Key) -> Result> { + PlainCache::temp_dir(self) + } + fn set(&self, key: Key, value: &Path) -> Result<()> { PlainCache::set(self, key.name, value) } @@ -71,6 +81,10 @@ impl FullCache for ShardedCache { ShardedCache::get(self, key) } + fn temp_dir(&self, key: Key) -> Result> { + ShardedCache::temp_dir(self, Some(key)) + } + fn set(&self, key: Key, value: &Path) -> Result<()> { ShardedCache::set(self, key, value) } @@ -104,6 +118,28 @@ pub struct Cache { read_side: ReadOnlyCache, } +/// Where does a cache hit come from: the primary read-write cache, or +/// one of the secondary read-only caches? +pub enum CacheHit<'a> { + /// The file was found in the primary read-write cache; promoting + /// it is a no-op. + Primary(&'a mut File), + /// The file was found in one of the secondary read-only caches. + /// Promoting it to the primary cache will require a full copy. + Secondary(&'a mut File), +} + +/// What to do with a cache hit in a `get_or_update` call? +pub enum CacheHitAction { + /// Return the cache hit as is. + Accept, + /// Return the cache hit after promoting it to the current write + /// cache directory, if necessary. + Promote, + /// Replace with and return a new file. + Replace, +} + impl CacheBuilder { /// Returns a fresh empty builder. pub fn new() -> Self { @@ -214,6 +250,102 @@ impl Cache { ) } + /// Attempts to find a cache entry for `key`. If there is none, + /// populates the cache with a file filled by `populate`. Returns + /// a file in all cases (unless the call fails with an error). + pub fn ensure<'a>( + &self, + key: impl Into>, + populate: impl FnOnce(&mut File) -> Result<()>, + ) -> Result { + fn judge(_: CacheHit) -> CacheHitAction { + CacheHitAction::Promote + } + + self.get_or_update(key, judge, |dst, _| populate(dst)) + } + + /// Attempts to find a cache entry for `key`. If there is none, + /// populates the write cache (if possible) with a file, once + /// filled by `populate`; otherwise obeys the value returned by + /// `judge` to determine what to do with the hit. + /// + /// When we need to populate a new file, `populate` is called with + /// a mutable reference to the destination file, and the old + /// cached file (in whatever state `judge` left it), if available. + pub fn get_or_update<'a>( + &self, + key: impl Into>, + judge: impl FnOnce(CacheHit) -> CacheHitAction, + populate: impl FnOnce(&mut File, Option) -> Result<()>, + ) -> Result { + // Attempts to return the `FullCache` for this `Cache`. + fn get_write_cache(this: &Cache) -> Result<&dyn FullCache> { + match this.write_side.as_ref() { + Some(cache) => Ok(cache.as_ref()), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } + } + + // Promotes `file` to `cache`. + fn promote(cache: &dyn FullCache, key: Key, mut file: File) -> Result { + use std::io::Seek; + + let mut tmp = NamedTempFile::new_in(cache.temp_dir(key)?)?; + std::io::copy(&mut file, tmp.as_file_mut())?; + + // Force the destination file's contents to disk before + // adding it to the read-write cache: the caller can't + // tell us whether they want a `fsync`, so let's be safe + // and assume they do. + tmp.as_file().sync_all()?; + cache.put(key, tmp.path())?; + + // We got a read-only file. Rewind it before returning. + file.seek(std::io::SeekFrom::Start(0))?; + Ok(file) + } + + let cache = get_write_cache(self)?; + let key: Key = key.into(); + + // Overwritten with `Some(file)` when replacing `file`. + let mut old = None; + if let Some(mut file) = cache.get(key)? { + match judge(CacheHit::Primary(&mut file)) { + // Promote is a no-op if the file is already in the write cache. + CacheHitAction::Accept | CacheHitAction::Promote => return Ok(file), + CacheHitAction::Replace => old = Some(file), + } + } else if let Some(mut file) = self.read_side.get(key)? { + match judge(CacheHit::Secondary(&mut file)) { + CacheHitAction::Accept => return Ok(file), + CacheHitAction::Promote => return promote(get_write_cache(self)?, key, file), + CacheHitAction::Replace => old = Some(file), + } + } + + let replace = old.is_some(); + // We either have to replace or ensure there is a cache entry. + // Either way, start by populating a temporary file. + let mut tmp = NamedTempFile::new_in(cache.temp_dir(key)?)?; + populate(tmp.as_file_mut(), old)?; + + // Grab a read-only return value before publishing the file. + let path = tmp.path(); + let ret = File::open(path)?; + if replace { + cache.set(key, path)?; + } else { + cache.put(key, path)?; + } + + Ok(ret) + } + /// Inserts or overwrites the file at `value` as `key` in the /// write cache directory. This will always fail with /// `Unsupported` if no write cache was defined. @@ -280,6 +412,8 @@ mod test { use crate::Cache; use crate::CacheBuilder; + use crate::CacheHit; + use crate::CacheHitAction; use crate::Key; use crate::PlainCache; use crate::ShardedCache; @@ -309,6 +443,10 @@ mod test { let cache: Cache = Default::default(); assert!(matches!(cache.get(&TestKey::new("foo")), Ok(None))); + assert!( + matches!(cache.ensure(&TestKey::new("foo"), |_| unreachable!("should not be called when there is no write side")), + Err(e) if e.kind() == ErrorKind::Unsupported) + ); assert!(matches!(cache.set(&TestKey::new("foo"), "/tmp/foo"), Err(e) if e.kind() == ErrorKind::Unsupported)); assert!(matches!(cache.put(&TestKey::new("foo"), "/tmp/foo"), @@ -316,6 +454,376 @@ mod test { assert!(matches!(cache.touch(&TestKey::new("foo")), Ok(false))); } + // Fail to find a file, ensure it, then see that we can get it. + #[test] + fn test_ensure() { + use std::io::{Read, Write}; + use test_dir::{DirBuilder, TestDir}; + + let temp = TestDir::temp(); + let cache = CacheBuilder::new().writer(temp.path("."), 1, 10).build(); + let key = TestKey::new("foo"); + + // The file doesn't exist initially. + assert!(matches!(cache.get(&key), Ok(None))); + + { + let mut populated = cache + .ensure(&key, |file| file.write_all(b"test")) + .expect("ensure must succeed"); + + let mut dst = Vec::new(); + populated.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"test"); + } + + // And now get the file again. + { + let mut fetched = cache + .get(&key) + .expect("get must succeed") + .expect("file must be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"test"); + } + + // And make sure a later `ensure` call just grabs the file. + { + let mut populated = cache + .ensure(&key, |_| { + unreachable!("should not be called for an extant file") + }) + .expect("ensure must succeed"); + + let mut dst = Vec::new(); + populated.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"test"); + } + } + + // Use a two-level cache, and make sure `ensure` promotes copies from + // the backup to the primary location. + #[test] + fn test_ensure_promote() { + use std::io::{Read, Write}; + use tempfile::NamedTempFile; + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp() + .create("cache", FileType::Dir) + .create("extra_plain", FileType::Dir); + + // Populate the plain cache in `extra_plain` with one file. + { + let cache = PlainCache::new(temp.path("extra_plain"), 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"initial") + .expect("write must succeed"); + + cache.put("foo", tmp.path()).expect("put must succeed"); + } + + let cache = CacheBuilder::new() + .writer(temp.path("cache"), 1, 10) + .plain_reader(temp.path("extra_plain")) + .build(); + let key = TestKey::new("foo"); + + // The file is found initially. + { + let mut fetched = cache + .get(&key) + .expect("get must succeed") + .expect("file must be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"initial"); + } + + { + let mut populated = cache + .ensure(&key, |_| { + unreachable!("should not be called for an extant file") + }) + .expect("ensure must succeed"); + + let mut dst = Vec::new(); + populated.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"initial"); + } + + // And now get the file again, and make sure it doesn't come from the + // backup location. + { + let new_cache = CacheBuilder::new() + .writer(temp.path("cache"), 1, 10) + .build(); + let mut fetched = new_cache + .get(&key) + .expect("get must succeed") + .expect("file must be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"initial"); + } + } + + // Use a two-level cache, get_or_update with an `Accept` judgement. + // We should leave everything where it is. + #[test] + fn test_get_or_update_accept() { + use std::io::{Read, Write}; + use tempfile::NamedTempFile; + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp() + .create("cache", FileType::Dir) + .create("extra_plain", FileType::Dir); + + // Populate the plain cache in `extra_plain` with one file. + { + let cache = PlainCache::new(temp.path("extra_plain"), 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"initial") + .expect("write must succeed"); + + cache.put("foo", tmp.path()).expect("put must succeed"); + } + + let cache = CacheBuilder::new() + // Make it sharded, because why not? + .writer(temp.path("cache"), 2, 10) + .plain_reader(temp.path("extra_plain")) + .build(); + let key = TestKey::new("foo"); + let key2 = TestKey::new("bar"); + + // The file is found initially, in the backup cache. + { + let mut fetched = cache + .get_or_update( + &key, + |hit| { + assert!(matches!(hit, CacheHit::Secondary(_))); + CacheHitAction::Accept + }, + |_, _| unreachable!("should not have to fill an extant file"), + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"initial"); + } + + // Let's try again with a file that does not exist yet. + { + let mut fetched = cache + .get_or_update( + &key2, + |_| unreachable!("should not be called"), + |file, old| { + assert!(old.is_none()); + file.write_all(b"updated") + }, + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"updated"); + } + + // The new file is now found. + { + let mut fetched = cache + .get_or_update( + &key2, + |hit| { + assert!(matches!(hit, CacheHit::Primary(_))); + CacheHitAction::Accept + }, + |_, _| unreachable!("should not have to fill an extant file"), + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"updated"); + } + + // And now get the files again, and make sure they don't + // come from the backup location. + { + let new_cache = CacheBuilder::new() + .writer(temp.path("cache"), 2, 10) + .build(); + + // The new cache shouldn't have the old key. + assert!(matches!(new_cache.touch(&key), Ok(false))); + + // But it should have `key2`. + let mut fetched = new_cache + .get(&key2) + .expect("get must succeed") + .expect("file must be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"updated"); + } + } + + // Use a two-level cache, get_or_update with a `Replace` judgement. + // We should always overwrite everything to the write cache. + #[test] + fn test_get_or_update_replace() { + use std::io::{Read, Write}; + use tempfile::NamedTempFile; + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp() + .create("cache", FileType::Dir) + .create("extra_plain", FileType::Dir); + + // Populate the plain cache in `extra_plain` with one file. + { + let cache = PlainCache::new(temp.path("extra_plain"), 10); + + let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) + .expect("new temp file must succeed"); + tmp.as_file() + .write_all(b"initial") + .expect("write must succeed"); + + cache.put("foo", tmp.path()).expect("put must succeed"); + } + + let cache = CacheBuilder::new() + // Make it sharded, because why not? + .writer(temp.path("cache"), 2, 10) + .plain_reader(temp.path("extra_plain")) + .build(); + let key = TestKey::new("foo"); + + { + let mut fetched = cache + .get_or_update( + &key, + |hit| { + assert!(matches!(hit, CacheHit::Secondary(_))); + CacheHitAction::Replace + }, + |file, old| { + // Make sure the `old` file is the "initial" file. + let mut prev = old.expect("must have old data"); + let mut dst = Vec::new(); + prev.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"initial"); + + file.write_all(b"replace1") + }, + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace1"); + } + + // Re-read the file. + { + let mut fetched = cache + .get(&key) + .expect("get must succeed") + .expect("file should be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace1"); + } + + // Update it again. + { + let mut fetched = cache + .get_or_update( + &key, + |hit| { + assert!(matches!(hit, CacheHit::Primary(_))); + CacheHitAction::Replace + }, + |file, old| { + // Make sure the `old` file is the "initial" file. + let mut prev = old.expect("must have old data"); + let mut dst = Vec::new(); + prev.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace1"); + + file.write_all(b"replace2") + }, + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace2"); + } + + // The new file is now found. + { + let mut fetched = cache + .get_or_update( + &key, + |hit| { + assert!(matches!(hit, CacheHit::Primary(_))); + CacheHitAction::Replace + }, + |file, old| { + // Make sure the `old` file is the "replace2" file. + let mut prev = old.expect("must have old data"); + let mut dst = Vec::new(); + prev.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace2"); + + file.write_all(b"replace3") + }, + ) + .expect("get_or_update must succeed"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace3"); + } + + // And now get the same file again, and make sure it doesn't + // come from the backup location. + { + let new_cache = CacheBuilder::new() + .writer(temp.path("cache"), 2, 10) + .build(); + + // But it should have `key2`. + let mut fetched = new_cache + .get(&key) + .expect("get must succeed") + .expect("file must be found"); + + let mut dst = Vec::new(); + fetched.read_to_end(&mut dst).expect("read must succeed"); + assert_eq!(&dst, b"replace3"); + } + } + // Smoke test a wrapped plain cache. #[test] fn smoke_test_plain() { From f21e9b0b65acfb1378ec0d499abcdad9a47f4338 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 18:36:07 -0400 Subject: [PATCH 03/12] lib: expose temporary subdirectory constant And rename from ".temp" to `.kismet_temp". TESTED=existing tests. --- src/lib.rs | 4 ++++ src/plain.rs | 4 +--- src/sharded.rs | 4 +--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4293f56..377b8ef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,10 @@ pub use stack::CacheBuilder; pub use stack::CacheHit; pub use stack::CacheHitAction; +/// Kismet cache directories put temporary files under this +/// subdirectory. +pub const KISMET_TEMPORARY_SUBDIRECTORY: &str = ".kismet_temp"; + /// Sharded cache keys consist of a filename and two hash values. The /// two hashes should be computed by distinct functions of the key's /// name, and each hash function must be identical for all processes diff --git a/src/plain.rs b/src/plain.rs index 005c37c..de4a1a6 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use crate::cache_dir::CacheDir; use crate::trigger::PeriodicTrigger; +use crate::KISMET_TEMPORARY_SUBDIRECTORY as TEMP_SUBDIR; /// How many times we want to trigger maintenance per "capacity" /// inserts. For example, `MAINTENANCE_SCALE = 3` means we will @@ -13,9 +14,6 @@ use crate::trigger::PeriodicTrigger; /// ~capacity / 3 files in the cache. const MAINTENANCE_SCALE: usize = 3; -/// Put temporary file in this subdirectory of the cache directory. -const TEMP_SUBDIR: &str = ".temp"; - /// A "plain" cache is a single directory of files. Given a capacity /// of `k` files, we will trigger a second chance maintance roughly /// every `k / 3` (`k / 6` in the long run, given the way diff --git a/src/sharded.rs b/src/sharded.rs index 86cdb93..81c9e01 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -17,15 +17,13 @@ use std::sync::Arc; use crate::cache_dir::CacheDir; use crate::trigger::PeriodicTrigger; use crate::Key; +use crate::KISMET_TEMPORARY_SUBDIRECTORY as TEMP_SUBDIR; /// We will aim to trigger maintenance at least `MAINTENANCE_SCALE` /// times per total capacity inserts or updates, and at least once per /// shard capacity inserts or updates. const MAINTENANCE_SCALE: usize = 2; -/// Put temporary file in this subdirectory of the cache directory. -const TEMP_SUBDIR: &str = ".temp"; - const RANDOM_MULTIPLIER: u64 = 0xf2efdf1111adba6f; const SECONDARY_RANDOM_MULTIPLIER: u64 = 0xa55e1e02718a6a47; From 8af796d12e2bca9d3fd39e4e8d36e85ed3d0d846 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 19:09:23 -0400 Subject: [PATCH 04/12] cache_dir: validate file names We don't want to accidentally overwrite our temporary directory, nor to allow path traversal bugs. Validate all over the place to make sure tentative cache file names are non-empty, and do not start with `.`, `/`, or `\\`. TESTED=new unit tests. --- src/cache_dir.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++ src/plain.rs | 15 +++++-- src/readonly.rs | 6 +++ src/sharded.rs | 12 +++++- src/stack.rs | 18 +++++++++ 5 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/cache_dir.rs b/src/cache_dir.rs index 9f2d9ef..2f677ac 100644 --- a/src/cache_dir.rs +++ b/src/cache_dir.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::fs::File; +use std::io::Error; use std::io::ErrorKind; use std::io::Result; use std::path::Path; @@ -68,6 +69,30 @@ fn cleanup_temporary_directory(temp_dir: Cow) -> Result<()> { Ok(()) } +/// Returns `name` if it non-empty and does not start with a reserved +/// byte (dot, slash, backslash). +fn validate_file_name(name: &str) -> Result<&str> { + match name.as_bytes().get(0) { + None => Err(Error::new( + ErrorKind::InvalidInput, + "kismet cached file name must not be empty", + )), + Some(b'.') => Err(Error::new( + ErrorKind::InvalidInput, + "kismet cached file name must not start with a dot", + )), + Some(b'/') => Err(Error::new( + ErrorKind::InvalidInput, + "kismet cached file name must not starts with a forward slash", + )), + Some(b'\\') => Err(Error::new( + ErrorKind::InvalidInput, + "kismet cached file name must not starts with a backslash", + )), + Some(_) => Ok(name), + } +} + /// The `CacheDir` trait drives the actual management of a single /// cache directory. pub(crate) trait CacheDir { @@ -94,6 +119,7 @@ pub(crate) trait CacheDir { /// /// Implicitly "touches" the cached file `name` if it exists. fn get(&self, name: &str) -> Result> { + let name = validate_file_name(name)?; let mut target = self.base_dir().into_owned(); target.push(name); @@ -154,6 +180,7 @@ pub(crate) trait CacheDir { /// Always consumes the file at `value` on success; may consume it /// on error. fn set(&self, name: &str, value: &Path) -> Result> { + let name = validate_file_name(name)?; let mut dst = self.base_dir().into_owned(); let ret = self.maybe_cleanup(&dst)?; @@ -173,6 +200,7 @@ pub(crate) trait CacheDir { /// Always consumes the file at `value` on success; may consume it /// on error. fn put(&self, name: &str, value: &Path) -> Result> { + let name = validate_file_name(name)?; let mut dst = self.base_dir().into_owned(); let ret = self.maybe_cleanup(&dst)?; @@ -186,9 +214,83 @@ pub(crate) trait CacheDir { /// /// Returns whether the file `name` exists. fn touch(&self, name: &str) -> Result { + let name = validate_file_name(name)?; let mut target = self.base_dir().into_owned(); target.push(name); raw_cache::touch(&target) } } + +#[cfg(test)] +mod test { + use std::borrow::Cow; + use std::io::ErrorKind; + use std::path::Path; + use std::path::PathBuf; + + use crate::cache_dir::CacheDir; + use crate::trigger::PeriodicTrigger; + + struct DummyCacheDir {} + + impl CacheDir for DummyCacheDir { + #[cfg(not(tarpaulin_include))] + fn temp_dir(&self) -> Cow { + unreachable!("should not be called") + } + + #[cfg(not(tarpaulin_include))] + fn base_dir(&self) -> Cow { + unreachable!("should not be called") + } + + #[cfg(not(tarpaulin_include))] + fn trigger(&self) -> &PeriodicTrigger { + unreachable!("should not be called") + } + + #[cfg(not(tarpaulin_include))] + fn capacity(&self) -> usize { + unreachable!("should not be called") + } + } + + // Passing a bad file name to `get` should abort immediately. + #[test] + fn test_bad_get() { + let cache = DummyCacheDir {}; + + assert!(matches!(cache.get(""), + Err(e) if e.kind() == ErrorKind::InvalidInput)); + } + + // Passing a bad file name to `set` should abort immediately. + #[test] + fn test_bad_set() { + let cache = DummyCacheDir {}; + + let path: PathBuf = "/tmp/foo".into(); + assert!(matches!(cache.set(".foo", &path), + Err(e) if e.kind() == ErrorKind::InvalidInput)); + } + + // Passing a bad file name to `put` should abort immediately. + #[test] + fn test_bad_put() { + let cache = DummyCacheDir {}; + + let path: PathBuf = "/tmp/foo".into(); + assert!(matches!(cache.set("/asd", &path), + Err(e) if e.kind() == ErrorKind::InvalidInput)); + } + + // Passing a bad file name to `touch` should abort immediately. + #[test] + fn test_bad_touch() { + let cache = DummyCacheDir {}; + + assert!(matches!(cache.touch("\\.test"), + Err(e) if e.kind() == ErrorKind::InvalidInput)); + } +} diff --git a/src/plain.rs b/src/plain.rs index de4a1a6..141bc5d 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -71,7 +71,10 @@ impl PlainCache { } /// Returns a read-only file for `name` in the cache directory if - /// it exists, or None if there is no such file. + /// it exists, or None if there is no such file. Fails with + /// `ErrorKind::InvalidInput` if `name` is invalid (empty, or + /// starts with a dot or a forward or back slash). + /// /// /// Implicitly "touches" the cached file `name` if it exists. pub fn get(&self, name: &str) -> Result> { @@ -85,7 +88,9 @@ impl PlainCache { } /// Inserts or overwrites the file at `value` as `name` in the - /// cache directory. + /// cache directory. Fails with `ErrorKind::InvalidInput` if + /// `name` is invalid (empty, or starts with a dot or a forward + /// or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. @@ -96,7 +101,9 @@ impl PlainCache { /// Inserts the file at `value` as `name` in the cache directory /// if there is no such cached entry already, or touches the - /// cached file if it already exists. + /// cached file if it already exists. Fails with + /// `ErrorKind::InvalidInput` if `name` is invalid (empty, or + /// starts with a dot or a forward or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. @@ -106,6 +113,8 @@ impl PlainCache { } /// Marks the cached file `name` as newly used, if it exists. + /// Fails with `ErrorKind::InvalidInput` if `name` is invalid + /// (empty, or starts with a dot or a forward or back slash). /// /// Returns whether `name` exists. pub fn touch(&self, name: &str) -> Result { diff --git a/src/readonly.rs b/src/readonly.rs index 3d4f58a..c1d7494 100644 --- a/src/readonly.rs +++ b/src/readonly.rs @@ -138,6 +138,9 @@ impl ReadOnlyCache { /// `ReadOnlyCache` will query each constituent cache in order of /// registration, and return a read-only file for the first hit. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Returns `None` if no file for `key` can be found in any of the /// constituent caches, and bubbles up the first I/O error /// encountered, if any. @@ -163,6 +166,9 @@ impl ReadOnlyCache { /// `ReadOnlyCache` will touch the same file that would be returned /// by `get`. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Returns whether a file for `key` could be found, and bubbles /// up the first I/O error encountered, if any. pub fn touch<'a>(&self, key: impl Into>) -> Result { diff --git a/src/sharded.rs b/src/sharded.rs index 81c9e01..cf673a4 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -215,6 +215,8 @@ impl ShardedCache { /// Returns a read-only file for `key` in the shard cache /// directory if it exists, or None if there is no such file. + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). /// /// Implicitly "touches" the cached file if it exists. pub fn get(&self, key: Key) -> Result> { @@ -286,7 +288,9 @@ impl ShardedCache { /// Inserts or overwrites the file at `value` as `key` in the /// sharded cache directory. There may be two entries for the - /// same key with concurrent `set` or `put` calls. + /// same key with concurrent `set` or `put` calls. Fails with + /// `ErrorKind::InvalidInput` if `key.name` is invalid (empty, or + /// starts with a dot or a forward or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. @@ -321,7 +325,9 @@ impl ShardedCache { /// Inserts the file at `value` as `key` in the cache directory if /// there is no such cached entry already, or touches the cached /// file if it already exists. There may be two entries for the - /// same key with concurrent `set` or `put` calls. + /// same key with concurrent `set` or `put` calls. Fails with + /// `ErrorKind::InvalidInput` if `key.name` is invalid (empty, or + /// starts with a dot or a forward or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. @@ -350,6 +356,8 @@ impl ShardedCache { } /// Marks the cached file `key` as newly used, if it exists. + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). /// /// Returns whether a file for `key` exists in the cache. pub fn touch(&self, key: Key) -> Result { diff --git a/src/stack.rs b/src/stack.rs index 7deb82b..a9a75f6 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -225,6 +225,9 @@ impl Cache { /// additional read-only cache, in definition order, and return a /// read-only file for the first hit. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Returns `None` if no file for `key` can be found in any of the /// constituent caches, and bubbles up the first I/O error /// encountered, if any. @@ -253,6 +256,9 @@ impl Cache { /// Attempts to find a cache entry for `key`. If there is none, /// populates the cache with a file filled by `populate`. Returns /// a file in all cases (unless the call fails with an error). + /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). pub fn ensure<'a>( &self, key: impl Into>, @@ -270,6 +276,9 @@ impl Cache { /// filled by `populate`; otherwise obeys the value returned by /// `judge` to determine what to do with the hit. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// When we need to populate a new file, `populate` is called with /// a mutable reference to the destination file, and the old /// cached file (in whatever state `judge` left it), if available. @@ -350,6 +359,9 @@ impl Cache { /// write cache directory. This will always fail with /// `Unsupported` if no write cache was defined. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Always consumes the file at `value` on success; may consume it /// on error. pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { @@ -366,6 +378,9 @@ impl Cache { /// there is no such cached entry already, or touches the cached /// file if it already exists. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Always consumes the file at `value` on success; may consume it /// on error. pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { @@ -381,6 +396,9 @@ impl Cache { /// Marks a cache entry for `key` as accessed (read). The `Cache` /// will touch the same file that would be returned by `get`. /// + /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// (empty, or starts with a dot or a forward or back slash). + /// /// Returns whether a file for `key` could be found, and bubbles /// up the first I/O error encountered, if any. pub fn touch<'a>(&self, key: impl Into>) -> Result { From ec0dfc7e50b5712879edf101a4ec2e72c72567a3 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 19:17:42 -0400 Subject: [PATCH 05/12] trigger: remove useless `allow(dead_code)` TESTED=it builds without warning. --- src/trigger.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/trigger.rs b/src/trigger.rs index 6376b8d..623a159 100644 --- a/src/trigger.rs +++ b/src/trigger.rs @@ -104,7 +104,6 @@ impl PeriodicTrigger { /// Observes count "events". Returns whether the periodic trigger /// fired for any of these events (i.e., whether the caller should /// invoke the periodic behaviour). - #[allow(dead_code)] #[inline(always)] pub fn weighted_event(self, count: u64) -> bool { observe(self.scale.saturating_mul(count)) From b871478628ed35d40db9d4759ed364e6db6904e8 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 19:39:07 -0400 Subject: [PATCH 06/12] plain, sharded: hide the Cache structs behind public modules We want to encourage people to use our `Cache`/`ReadOnlyCache` wrappers. Expose the plain and sharded caches only via their modules, and not at the crate's toplevel. TESTED=existing tests build. --- src/lib.rs | 6 ++---- src/plain.rs | 31 +++++++++++++++++++++---------- src/readonly.rs | 8 ++++---- src/sharded.rs | 41 ++++++++++++++++++++++------------------- src/stack.rs | 8 ++++---- 5 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 377b8ef..c9ede62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,16 +1,14 @@ mod cache_dir; -mod plain; +pub mod plain; pub mod raw_cache; mod readonly; pub mod second_chance; -mod sharded; +pub mod sharded; mod stack; mod trigger; -pub use plain::PlainCache; pub use readonly::ReadOnlyCache; pub use readonly::ReadOnlyCacheBuilder; -pub use sharded::ShardedCache; pub use stack::Cache; pub use stack::CacheBuilder; pub use stack::CacheHit; diff --git a/src/plain.rs b/src/plain.rs index 141bc5d..1895e3c 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -1,3 +1,14 @@ +//! A `plain::Cache` stores all cached file in a single directory, and +//! periodically scans for evictions with a second chance strategy. +//! This implementation does not scale up to more than a few hundred +//! files per cache directory (a `sharded::Cache` can go higher), +//! but interoperates seamlessly with other file-based programs. +//! +//! This module is useful for lower level usage; in most cases, the +//! `Cache` is more convenient and just as efficient. +//! +//! The cache's contents will grow past its stated capacity, but +//! should rarely reach more than twice that capacity. use std::borrow::Cow; use std::fs::File; use std::io::Result; @@ -19,7 +30,7 @@ const MAINTENANCE_SCALE: usize = 3; /// every `k / 3` (`k / 6` in the long run, given the way /// `PeriodicTrigger` is implemented) insertions. #[derive(Clone, Debug)] -pub struct PlainCache { +pub struct Cache { // The cached files are siblings of this directory for temporary // files. temp_dir: PathBuf, @@ -34,7 +45,7 @@ pub struct PlainCache { capacity: usize, } -impl CacheDir for PlainCache { +impl CacheDir for Cache { #[inline] fn temp_dir(&self) -> Cow { Cow::from(&self.temp_dir) @@ -56,14 +67,14 @@ impl CacheDir for PlainCache { } } -impl PlainCache { +impl Cache { /// Returns a new cache for approximately `capacity` files in /// `base_dir`. - pub fn new(base_dir: PathBuf, capacity: usize) -> PlainCache { + pub fn new(base_dir: PathBuf, capacity: usize) -> Cache { let mut temp_dir = base_dir; temp_dir.push(TEMP_SUBDIR); - PlainCache { + Cache { temp_dir, trigger: PeriodicTrigger::new((capacity / MAINTENANCE_SCALE) as u64), capacity, @@ -142,7 +153,7 @@ fn smoke_test() { // Make sure the garbage file is old enough to be deleted. std::thread::sleep(std::time::Duration::from_secs_f64(2.5)); - let cache = PlainCache::new(temp.path("."), 10); + let cache = Cache::new(temp.path("."), 10); for i in 0..20 { let name = format!("{}", i); @@ -188,7 +199,7 @@ fn test_set() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 1); + let cache = Cache::new(temp.path("."), 1); { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) @@ -241,7 +252,7 @@ fn test_put() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 1); + let cache = Cache::new(temp.path("."), 1); { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) @@ -293,7 +304,7 @@ fn test_touch() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 5); + let cache = Cache::new(temp.path("."), 5); for i in 0..15 { let name = format!("{}", i); @@ -331,7 +342,7 @@ fn test_recent_temp_file() { // The garbage file must exist. assert!(std::fs::metadata(temp.path(&format!("{}/garbage", TEMP_SUBDIR))).is_ok()); - let cache = PlainCache::new(temp.path("."), 1); + let cache = Cache::new(temp.path("."), 1); for i in 0..2 { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) diff --git a/src/readonly.rs b/src/readonly.rs index c1d7494..de70d06 100644 --- a/src/readonly.rs +++ b/src/readonly.rs @@ -8,9 +8,9 @@ use std::io::Result; use std::path::Path; use std::sync::Arc; +use crate::plain::Cache as PlainCache; +use crate::sharded::Cache as ShardedCache; use crate::Key; -use crate::PlainCache; -use crate::ShardedCache; /// The `ReadSide` trait offers `get` and `touch`, as implemented by /// both plain and sharded caches. @@ -192,11 +192,11 @@ impl ReadOnlyCache { #[cfg(test)] mod test { + use crate::plain::Cache as PlainCache; + use crate::sharded::Cache as ShardedCache; use crate::Key; - use crate::PlainCache; use crate::ReadOnlyCache; use crate::ReadOnlyCacheBuilder; - use crate::ShardedCache; struct TestKey { key: String, diff --git a/src/sharded.rs b/src/sharded.rs index cf673a4..886cabe 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -1,10 +1,17 @@ -//! A `ShardedCache` uses the same basic file-based second chance -//! strategy as a `PlainCache`. However, while the simple plain cache -//! is well suited to small caches (down to 2-3 files, and up maybe -//! one hundred), this sharded version can scale nearly arbitrarily -//! high: each shard should have fewer than one hundred or so files, -//! but there may be arbitrarily many shards (up to filesystem limits, -//! since each shard is a subdirectory). +//! A `sharded::Cache` uses the same basic file-based second chance +//! strategy as a `plain::Cache`. However, while the simple plain +//! cache is well suited to small caches (down to 2-3 files, and up +//! maybe one hundred), this sharded version can scale nearly +//! arbitrarily high: each shard should have fewer than one hundred or +//! so files, but there may be arbitrarily many shards (up to +//! filesystem limits, since each shard is a subdirectory). +//! +//! This module is useful for lower level usage; in most cases, the +//! `Cache` is more convenient and just as efficient. +//! +//! The cache's contents will grow past its stated capacity, but +//! should rarely reach more than twice that capacity, especially +//! when the shard capacity is less than 128 files. use std::borrow::Cow; use std::fs::File; use std::io::Result; @@ -32,7 +39,7 @@ const SECONDARY_RANDOM_MULTIPLIER: u64 = 0xa55e1e02718a6a47; /// subdirectories. Each subdirectory is managed as an /// independent second chance cache directory. #[derive(Clone, Debug)] -pub struct ShardedCache { +pub struct Cache { // The current load (number of files) estimate for each shard. load_estimates: Arc<[AtomicU8]>, // The parent directory for each shard (cache subdirectory). @@ -111,14 +118,10 @@ impl CacheDir for Shard { } } -impl ShardedCache { +impl Cache { /// Returns a new cache for approximately `total_capacity` files, /// stores in `num_shards` subdirectories of `base_dir`. - pub fn new( - base_dir: PathBuf, - mut num_shards: usize, - mut total_capacity: usize, - ) -> ShardedCache { + pub fn new(base_dir: PathBuf, mut num_shards: usize, mut total_capacity: usize) -> Cache { // We assume at least two shards. if num_shards < 2 { num_shards = 2; @@ -135,7 +138,7 @@ impl ShardedCache { let trigger = PeriodicTrigger::new(shard_capacity.min(total_capacity / MAINTENANCE_SCALE) as u64); - ShardedCache { + Cache { load_estimates: load_estimates.into_boxed_slice().into(), base_dir, trigger, @@ -383,7 +386,7 @@ fn smoke_test() { const PAYLOAD_MULTIPLIER: usize = 113; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 3, 9); + let cache = Cache::new(temp.path("."), 3, 9); for i in 0..200 { let name = format!("{}", i); @@ -436,7 +439,7 @@ fn test_set() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 0, 0); + let cache = Cache::new(temp.path("."), 0, 0); { let tmp = NamedTempFile::new_in(cache.temp_dir(None).expect("temp_dir must succeed")) @@ -489,7 +492,7 @@ fn test_put() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 0, 0); + let cache = Cache::new(temp.path("."), 0, 0); { let tmp = NamedTempFile::new_in(cache.temp_dir(None).expect("temp_dir must succeed")) @@ -535,7 +538,7 @@ fn test_touch() { const PAYLOAD_MULTIPLIER: usize = 113; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 2, 600); + let cache = Cache::new(temp.path("."), 2, 600); for i in 0..2000 { // After the first write, we should find our file. diff --git a/src/stack.rs b/src/stack.rs index a9a75f6..dcef42d 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -12,11 +12,11 @@ use std::path::Path; use std::sync::Arc; use tempfile::NamedTempFile; +use crate::plain::Cache as PlainCache; +use crate::sharded::Cache as ShardedCache; use crate::Key; -use crate::PlainCache; use crate::ReadOnlyCache; use crate::ReadOnlyCacheBuilder; -use crate::ShardedCache; /// The `FullCache` trait exposes both read and write operations as /// implemented by sharded and plain caches. @@ -428,13 +428,13 @@ impl Cache { mod test { use std::io::ErrorKind; + use crate::plain::Cache as PlainCache; + use crate::sharded::Cache as ShardedCache; use crate::Cache; use crate::CacheBuilder; use crate::CacheHit; use crate::CacheHitAction; use crate::Key; - use crate::PlainCache; - use crate::ShardedCache; struct TestKey { key: String, From 466a276ec10b29f9e052a768bf2cf5adbd9e2bef Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sat, 11 Sep 2021 22:24:49 -0400 Subject: [PATCH 07/12] sharded: prefix cache subdirectory names with `.kismet_` The dot means these internal subdirectories are guaranteed not to collide with any file left from plain unsharded caching in the same directory. It's unfortunate that it also makes the files hidden by default, but hopefully this can discourage direct uninformed manipulation of the cache directory. The `.kismet_` prefix also makes it easier to map directories back to this library, and leaves the rest of the `.` namespace to the application. TESTED=existing tests. --- src/sharded.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/sharded.rs b/src/sharded.rs index 886cabe..bb38167 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -56,9 +56,16 @@ pub struct Cache { shard_capacity: usize, } +/// Converts a shard id to a subdirectory name. +/// +/// We use a dot prefix because the resulting subdirectory names are +/// guaranteed not to collide with "plain" cache filenames. This +/// means we can switch between the sharded and plain (unsharded) +/// strategy for the same directory, without any chance of +/// misinterpreted file name. #[inline] fn format_id(shard: usize) -> String { - format!("{:04x}", shard) + format!(".kismet_{:04x}", shard) } /// We create short-lived Shard objects whenever we want to work with From 30464f128b1d31c16c1b798d7b193b90971e1342 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Tue, 7 Sep 2021 21:51:43 -0400 Subject: [PATCH 08/12] more documentation More linkage, and add some sample usage to `lib.rs`. TESTED=it's all comments. --- src/lib.rs | 246 ++++++++++++++++++++++++++++++++++++++++++- src/plain.rs | 15 +-- src/raw_cache.rs | 6 +- src/readonly.rs | 51 ++++++--- src/second_chance.rs | 11 +- src/sharded.rs | 21 ++-- src/stack.rs | 91 +++++++++++----- 7 files changed, 374 insertions(+), 67 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c9ede62..786fa06 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,235 @@ +//! Kismet implements multiprocess lock-free[^lock-free-fs] +//! application-crash-safe (roughly) bounded persistent caches stored +//! in filesystem directories, with a +//! [Second Chance (Clock)](https://en.wikipedia.org/wiki/Page_replacement_algorithm#Second-chance) +//! eviction strategy. The maintenance logic is batched and invoked +//! at periodic jittered intervals to make sure accesses amortise to a +//! constant number of filesystem system calls and logarithmic (in the +//! number of cached file) time complexity. That's good for performance, +//! and enables lock-freedom,[^unlike-ccache] but does mean that +//! caches are expected to temporarily grow past their capacity +//! limits, although rarely by more than a factor of 2 or 3. +//! +//! [^lock-free-fs]: Inasmuch as anything that makes a lot of syscalls +//! can be "lock-free." The cache access algorithms implement a +//! protocol that makes a bounded number of file open, rename, or link +//! syscalls; in other words, reads and writes are as wait-free as +//! these syscalls. The batched second-chance maintenance algorithm, +//! on the other hand, is merely lock-free: it could theoretically get +//! stuck if writers kept adding new files to a cache (sub)directory. +//! Again, this guarantee is in terms of file access and directory +//! enumeration syscalls, and maintenance is only as lock-free as the +//! underlying syscalls. However, we can assume the kernel is +//! reasonably well designed, and doesn't let any sequence of syscalls +//! keep its hand on kernel locks forever. +//! +//! [^unlike-ccache]: This design choice is different from, e.g., +//! [ccache](https://ccache.dev/)'s, which attempts to maintain +//! statistics per shard with locked files. Under high load, the lock +//! to update ccache statistics becomes a bottleneck. Yet, despite +//! taking this hit, ccache batches evictions like Kismet, because +//! cleaning up a directory is slow; up-to-date access statistics +//! aren't enough to enforce tight cache capacity limits. +//! +//! In addition to constant per-cache space overhead, each Kismet +//! cache maintains a variable-length [`std::path::PathBuf`] for the +//! directory, one byte of lock-free metadata per shard, and no other +//! non-heap resource (i.e., Kismet caches do not hold on to file +//! objects). This holds for individual cache directories; when +//! stacking multiple caches in a [`Cache`], the read-write cache and +//! all constituent read-only caches will each have their own +//! `PathBuf` and per-shard metadata. +//! +//! When a Kismet cache triggers second chance evictions, it will +//! allocate temporary data. That data's size is proportional to the +//! number of files in the cache shard subdirectory undergoing +//! eviction (or the whole directory for a plain unsharded cache), and +//! includes a copy of the name (without the path prefix) for each +//! cached file in the subdirectory (or plain cache directory). This +//! eviction process is linearithmic-time in the number of files in +//! the cache subdirectory (directory), and is invoked periodically, +//! so as to amortise the maintenance time overhead to logarithmic +//! per write to a cache subdirectory. +//! +//! Kismet does not pre-allocate any long-lived file object, so it may +//! need to temporarily open file objects. However, each call into +//! Kismet will always bound the number of concurrently allocated file +//! objects; the current logic never allocates more than two +//! concurrent file objects. +//! +//! The load (number of files) in each cache may exceed the cache's +//! capacity because there is no centralised accounting, except for +//! what filesystems provide natively. This design choice forces +//! Kismet to amortise maintenance calls with randomisation, but also +//! means that any number of threads or processes may safely access +//! the same cache directories without any explicit synchronisation. +//! +//! Filesystems can't be trusted to provide much; Kismet only relies +//! on file modification times (`mtime`), and on file access times +//! (`atime`) that are either less than or equal to the `mtime`, or +//! greater than the `mtime` (i.e., `relatime` is acceptable). This +//! implies that cached files should not be linked in multiple Kismet +//! cache directories. It is however safe to hardlink cached files in +//! multiple places, as long as the files are not modified, or their +//! `mtime` otherwise updated, through these non-Kismet links. +//! +//! Kismet cache directories are plain (unsharded) or sharded. +//! +//! Plain Kismet caches are simply directories where the cache entry for +//! "key" is the file named "key." These are most effective for +//! read-only access to cache directories managed by some other +//! process, or for small caches of up to ~100 cached files. +//! +//! Sharded caches scale to higher capacities, by indexing into one of +//! a constant number of shard subdirectories with a hash, and letting +//! each shard manage fewer files (ideally 10-100 files). They are +//! also much less likely to grow to multiples of the target capacity +//! than plain (unsharded) cache directories. +//! +//! Simple usage should be covered by the [`ReadOnlyCache`] or +//! [`Cache`] structs, which wrap [`plain::Cache`] and +//! [`sharded::Cache`] in a convenient type-erased interface. The +//! caches *do not* invoke [`std::fs::File::sync_all`] or [`std::fs::File::sync_data`]: +//! the caller should sync files before letting Kismet persist them in +//! a cache if necessary. File synchronisation is not automatic +//! because it makes sense to implement persistent filesystem caches +//! that are erased after each boot, e.g., via +//! [tmpfiles.d](https://www.freedesktop.org/software/systemd/man/tmpfiles.d.html), +//! or by tagging cache directories with a +//! [boot id](https://man7.org/linux/man-pages/man3/sd_id128_get_machine.3.html). +//! +//! The cache code also does not sync the parent cache directories: we +//! assume that it's safe, if unfortunate, for caches to lose data or +//! revert to an older state after kernel or hardware crashes. In +//! general, the code attempts to be robust again direct manipulation +//! of the cache directories. It's always safe to delete cache files +//! from kismet directories (ideally not recently created files in +//! `.kismet_temp` directories), and even *adding* files should mostly +//! do what one expects: they will be picked up if they're in the +//! correct place (in a plain unsharded cache directory or in the +//! correct shard subdirectory), and eventually evicted if useless or +//! in the wrong shard. +//! +//! It is however essential to only publish files atomically to the +//! cache directories, and it probably never makes sense to modify +//! cached file objects in place. In fact, Kismet always set files +//! readonly before publishing them to the cache and always returns +//! read-only [`std::fs::File`] objects for cached data. +//! +//! # Sample usage +//! +//! One could access a list of read-only caches with a [`ReadOnlyCache`]. +//! +//! ```no_run +//! const NUM_SHARDS: usize = 10; +//! +//! let read_only = kismet_cache::ReadOnlyCacheBuilder::new() +//! .plain("/tmp/plain_cache") // Read first here +//! .sharded("/tmp/sharded_cache", NUM_SHARDS) // Then try there. +//! .build(); +//! +//! // Attempt to read the file for key "foo", with primary hash 1 +//! // and second hash 2, first from `/tmp/plain.cache`, and then +//! // from `/tmp/sharded_cache`. In practice, the hashes should +//! // probably be populated with by implementing the `From<&'a T>` +//! // trait, and passing a `&T` to the cache methods. +//! read_only.get(kismet_cache::Key::new("foo", 1, 2)); +//! ``` +//! +//! Read-write accesses should use a [`Cache`]: +//! +//! ```no_run +//! struct CacheKey { +//! // ... +//! } +//! +//! fn get_contents(key: &CacheKey) -> Vec { +//! // ... +//! # unreachable!() +//! } +//! +//! impl<'a> From<&'a CacheKey> for kismet_cache::Key<'a> { +//! fn from(key: &CacheKey) -> kismet_cache::Key { +//! // ... +//! # unreachable!() +//! } +//! } +//! +//! +//! // It's easier to increase the capacity than the number of shards, +//! // so, when in doubt, prefer a few too many shards with a lower +//! // capacity. It's not incorrect to increase the number of shards, +//! // but will result in lost cached data (eventually deleted), since +//! // Kismet does not assign shards with a consistent hash. +//! const NUM_SHARDS: usize = 100; +//! const CAPACITY: usize = 1000; +//! +//! # fn main() -> std::io::Result<()> { +//! use std::io::Read; +//! use std::io::Write; +//! +//! let cache = kismet_cache::CacheBuilder::new() +//! .sharded_writer("/tmp/root_cache", NUM_SHARDS, CAPACITY) +//! .plain_reader("/tmp/extra_cache") // Try to fill cache misses here +//! .build(); +//! +//! let key: CacheKey = // ... +//! # CacheKey {} +//! ; +//! +//! // Fetches the current cached value for `key`, or populates it with +//! // the closure argument if missing. +//! let mut cached_file = cache +//! .ensure(&key, |file| { +//! file.write_all(&get_contents(&key))?; +//! file.sync_all() +//! })?; +//! let mut contents = Vec::new(); +//! cached_file.read_to_end(&mut contents)?; +//! # Ok(()) +//! # } +//! ``` +//! +//! # Cache directory structure +//! +//! Plain (unsharded) cache directories simply store the value for +//! each `key` under a file named `key`. They also have a single +//! `.kismet_temp` subdirectory, for temporary files. +//! +//! The second chance algorithm relies on mtime / atime (`relatime` +//! updates suffice), so merely opening a file automatically updates +//! the relevant read tracking metadata. +//! +//! Sharded cache directories store the values for each `key` under +//! one of two shard subdirectories. The primary and second potential +//! shards are respectively determined by multiplying `Key::hash` and +//! `Key::secondary_hash` by different odd integers before mapping the +//! result to the shard universe with a fixed point scaling. +//! +//! Each subdirectory is named `.kismet_$HEX_SHARD_ID`, and contains +//! cached files with name equal to the cache key, and a +//! `.kismet_temp` subsubdirectory, just like plain unsharded caches. +//! In fact, each such shard is managed exactly like a plain cache. +//! +//! Sharded caches attempt to balance load between two potential +//! shards for each cache key in an attempt to make all shards grow at +//! roughly the same rate. Once all the shards have reached their +//! capacity, the sharded cache will slowly revert to storing cache +//! keys in their primary shards. +//! +//! This scheme lets plain cache directories easily interoperate with +//! other programs that are not aware of Kismet, and also lets an +//! application use the same directory to back both a plain and a +//! sharded cache (concurrently or not) without any possibility of +//! collision between cached files and Kismet's internal directories. +//! +//! Kismet will always store its internal data in files or directories +//! start start with a `.kismet` prefix, and cached data lives in +//! files with names equal to their keys. Since Kismet sanitises +//! cache keys to forbid them from starting with `.`, `/`, or `\\`, it +//! is always safe for an application to store additional data in +//! files or directories that start with a `.`, as long as they do not +//! collide with the `.kismet` prefix. mod cache_dir; pub mod plain; pub mod raw_cache; @@ -18,10 +250,16 @@ pub use stack::CacheHitAction; /// subdirectory. pub const KISMET_TEMPORARY_SUBDIRECTORY: &str = ".kismet_temp"; -/// Sharded cache keys consist of a filename and two hash values. The -/// two hashes should be computed by distinct functions of the key's -/// name, and each hash function must be identical for all processes -/// that access the same sharded cache directory. +/// Cache keys consist of a filename and two hash values. The two +/// hashes should ideally be computed by distinct functions of the +/// key's name, but Kismet will function correctly if the `hash` and +/// `secondary_hash` are the same. Each hash function **must** be +/// identical for all processes that access the same sharded cache +/// directory. +/// +/// The `name` should not be empty nor start with a dot, forward +/// slash, a backslash: caches will reject any operation on such names +/// with an `ErrorKind::InvalidInput` error. #[derive(Clone, Copy, Debug)] pub struct Key<'a> { pub name: &'a str, diff --git a/src/plain.rs b/src/plain.rs index 1895e3c..59c40d2 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -1,11 +1,14 @@ -//! A `plain::Cache` stores all cached file in a single directory, and -//! periodically scans for evictions with a second chance strategy. -//! This implementation does not scale up to more than a few hundred -//! files per cache directory (a `sharded::Cache` can go higher), -//! but interoperates seamlessly with other file-based programs. +//! A [`crate::plain::Cache`] stores all cached file in a single +//! directory (there may also be a `.kismet_temp` subdirectory for +//! temporary files), and periodically scans for evictions with a +//! second chance strategy. This implementation does not scale up to +//! more than a few hundred files per cache directory (a +//! [`crate::sharded::Cache`] can go higher), but interoperates +//! seamlessly with other file-based programs that store cache files +//! in flat directories. //! //! This module is useful for lower level usage; in most cases, the -//! `Cache` is more convenient and just as efficient. +//! [`crate::Cache`] is more convenient and just as efficient. //! //! The cache's contents will grow past its stated capacity, but //! should rarely reach more than twice that capacity. diff --git a/src/raw_cache.rs b/src/raw_cache.rs index 2466eb9..e2c5873 100644 --- a/src/raw_cache.rs +++ b/src/raw_cache.rs @@ -1,12 +1,12 @@ //! The raw cache module manages directories of read-only files //! subject to a (batched) Second Chance eviction policy. Calling -//! `prune` deletes files to make sure a cache directory does not +//! [`prune`] deletes files to make sure a cache directory does not //! exceed its capacity, in file count. The deletions will obey a //! Second Chance policy as long as insertions and updates go through -//! `insert_or_update` or `insert_or_touch`, in order to update the +//! [`insert_or_update`] or [`insert_or_touch`], in order to update the //! cached files' modification times correctly. Opening the cached //! file will automatically update its metadata to take that access -//! into account, but a path can also be `touch`ed explicitly. +//! into account, but a path can also be [`touch`]ed explicitly. //! //! This module implements mechanisms, but does not hardcode any //! policy... except the use of a second chance strategy. diff --git a/src/readonly.rs b/src/readonly.rs index de70d06..8b51443 100644 --- a/src/readonly.rs +++ b/src/readonly.rs @@ -4,6 +4,8 @@ //! and easy-to-use interface that erases the difference between plain //! and sharded caches. use std::fs::File; +#[allow(unused_imports)] // We refer to this enum in comments. +use std::io::ErrorKind; use std::io::Result; use std::path::Path; use std::sync::Arc; @@ -49,7 +51,7 @@ impl ReadSide for ShardedCache { } } -/// Construct a `ReadOnlyCache` with this builder. The resulting +/// Construct a [`ReadOnlyCache`] with this builder. The resulting /// cache will access each constituent cache directory in the order /// they were added. /// @@ -59,13 +61,20 @@ pub struct ReadOnlyCacheBuilder { stack: Vec>, } -/// A `ReadOnlyCache` wraps an arbitrary number of caches, and -/// attempts to satisfy `get` and `touch` requests by hitting each -/// constituent cache in order. This interface hides the difference -/// between plain and sharded cache directories, and should be the -/// first resort for read-only uses. +/// A [`ReadOnlyCache`] wraps an arbitrary number of +/// [`crate::plain::Cache`] and [`crate::sharded::Cache`], and attempts +/// to satisfy [`ReadOnlyCache::get`] and [`ReadOnlyCache::touch`] +/// requests by hitting each constituent cache in order. This +/// interface hides the difference between plain and sharded cache +/// directories, and should be the first resort for read-only uses. /// /// The default cache wraps an empty set of constituent caches. +/// +/// [`ReadOnlyCache`] objects are stateless and cheap to clone; don't +/// put an [`Arc`] on them. Avoid creating multiple +/// [`ReadOnlyCache`]s for the same stack of directories: there is no +/// internal state to maintain, so multiple instances simply waste +/// memory without any benefit. #[derive(Clone, Debug)] pub struct ReadOnlyCache { stack: Arc<[Box]>, @@ -114,7 +123,7 @@ impl ReadOnlyCacheBuilder { self } - /// Returns a fresh `ReadOnlyCache` for the builder's search list + /// Returns a fresh [`ReadOnlyCache`] for the builder's search list /// of constituent cache directories. pub fn build(self) -> ReadOnlyCache { ReadOnlyCache::new(self.stack) @@ -135,15 +144,19 @@ impl ReadOnlyCache { } /// Attempts to open a read-only file for `key`. The - /// `ReadOnlyCache` will query each constituent cache in order of - /// registration, and return a read-only file for the first hit. + /// [`ReadOnlyCache`] will query each constituent cache in order + /// of registration, and return a read-only file for the first + /// hit. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid - /// (empty, or starts with a dot or a forward or back slash). + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is + /// invalid (empty, or starts with a dot or a forward or back slash). /// - /// Returns `None` if no file for `key` can be found in any of the - /// constituent caches, and bubbles up the first I/O error + /// Returns [`None`] if no file for `key` can be found in any of + /// the constituent caches, and bubbles up the first I/O error /// encountered, if any. + /// + /// In the worst case, each call to `get` attempts to open two + /// files for each cache directory in the `ReadOnlyCache` stack. pub fn get<'a>(&self, key: impl Into>) -> Result> { fn doit(stack: &[Box], key: Key) -> Result> { for cache in stack.iter() { @@ -163,14 +176,18 @@ impl ReadOnlyCache { } /// Marks a cache entry for `key` as accessed (read). The - /// `ReadOnlyCache` will touch the same file that would be returned - /// by `get`. + /// [`ReadOnlyCache`] will touch the same file that would be + /// returned by `get`. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid - /// (empty, or starts with a dot or a forward or back slash). + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is + /// invalid (empty, or starts with a dot or a forward or back slash). /// /// Returns whether a file for `key` could be found, and bubbles /// up the first I/O error encountered, if any. + /// + /// In the worst case, each call to `touch` attempts to update the + /// access time on two files for each cache directory in the + /// `ReadOnlyCache` stack. pub fn touch<'a>(&self, key: impl Into>) -> Result { fn doit(stack: &[Box], key: Key) -> Result { for cache in stack.iter() { diff --git a/src/second_chance.rs b/src/second_chance.rs index 2610039..06a42b5 100644 --- a/src/second_chance.rs +++ b/src/second_chance.rs @@ -1,8 +1,9 @@ -//! The Second Chance or Clock page replacement policy is a simple -//! approximation of the Least Recently Used policy. Kismet uses the -//! second chance policy because it can be easily implemented on top -//! of the usual file modification and access times that we can trust -//! operating systems to update for us. +//! The [Second Chance or Clock](https://en.wikipedia.org/wiki/Page_replacement_algorithm#Second-chance) +//! page replacement policy is a simple approximation of the Least +//! Recently Used policy. Kismet uses the second chance policy +//! because it can be easily implemented on top of the usual file +//! modification and access times that we can trust operating systems +//! to update for us. //! //! This second chance implementation is optimised for *batch* //! maintenance: the caller is expected to perform a number of diff --git a/src/sharded.rs b/src/sharded.rs index bb38167..cd9ca42 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -1,13 +1,18 @@ -//! A `sharded::Cache` uses the same basic file-based second chance -//! strategy as a `plain::Cache`. However, while the simple plain -//! cache is well suited to small caches (down to 2-3 files, and up -//! maybe one hundred), this sharded version can scale nearly -//! arbitrarily high: each shard should have fewer than one hundred or -//! so files, but there may be arbitrarily many shards (up to -//! filesystem limits, since each shard is a subdirectory). +//! A [`crate::sharded::Cache`] uses the same basic file-based second +//! chance strategy as a [`crate::plain::Cache`]. However, while the +//! simple plain cache is well suited to small caches (down to 2-3 +//! files, and up maybe one hundred), this sharded version can scale +//! nearly arbitrarily high: each shard should have fewer than one +//! hundred or so files, but there may be arbitrarily many shards (up +//! to filesystem limits, since each shard is a subdirectory). +//! +//! A sharded cache directory consists of shard subdirectories (with +//! name equal to the shard index printed as `%04x`), each of which +//! contains the cached files for that shard, under their `key` name, +//! and an optional `.kismet_temp` subdirectory for temporary files. //! //! This module is useful for lower level usage; in most cases, the -//! `Cache` is more convenient and just as efficient. +//! [`crate::Cache`] is more convenient and just as efficient. //! //! The cache's contents will grow past its stated capacity, but //! should rarely reach more than twice that capacity, especially diff --git a/src/stack.rs b/src/stack.rs index dcef42d..fc850e6 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -1,8 +1,9 @@ -//! We expect most callers to interact with Kismet via the `Cache` -//! struct defined here. A `Cache` hides the difference in behaviour -//! between plain and sharded caches via late binding, and lets -//! callers transparently handle misses by looking in a series of -//! secondary cache directories. +//! We expect most callers to interact with Kismet via the [`Cache`] +//! struct defined here. A [`Cache`] hides the difference in +//! behaviour between [`crate::plain::Cache`] and +//! [`crate::sharded::Cache`] via late binding, and lets callers +//! transparently handle misses by looking in a series of secondary +//! cache directories. use std::borrow::Cow; use std::fs::File; use std::io::Error; @@ -98,20 +99,29 @@ impl FullCache for ShardedCache { } } -/// Construct a `Cache` with this builder. The resulting cache will +/// Construct a [`Cache`] with this builder. The resulting cache will /// always first access its write-side cache (if defined), and, on -/// misses, will attempt to service `get` and `touch` calls by -/// iterating over the read-only caches. +/// misses, will attempt to service [`Cache::get`] and +/// [`Cache::touch`] calls by iterating over the read-only caches. #[derive(Debug, Default)] pub struct CacheBuilder { write_side: Option>, read_side: ReadOnlyCacheBuilder, } -/// A `Cache` wraps either up to one plain or sharded read-write cache -/// in a convenient interface, and may optionally fulfill read +/// A [`Cache`] wraps either up to one plain or sharded read-write +/// cache in a convenient interface, and may optionally fulfill read /// operations by deferring to a list of read-only cache when the /// read-write cache misses. +/// +/// The default cache has no write-side and an empty stack of backup +/// read-only caches. +/// +/// [`Cache`] objects are cheap to clone and lock-free; don't put an +/// [`Arc`] on them. Avoid opening multiple caches for the same set +/// of directories: using the same [`Cache`] object improves the +/// accuracy of the write cache's lock-free in-memory statistics, when +/// it's a sharded cache. #[derive(Clone, Debug, Default)] pub struct Cache { write_side: Option>, @@ -129,7 +139,7 @@ pub enum CacheHit<'a> { Secondary(&'a mut File), } -/// What to do with a cache hit in a `get_or_update` call? +/// What to do with a cache hit in a [`Cache::get_or_update`] call? pub enum CacheHitAction { /// Return the cache hit as is. Accept, @@ -209,7 +219,7 @@ impl CacheBuilder { self } - /// Returns a fresh `Cache` for the builder's write cache and + /// Returns a fresh [`Cache`] for the builder's write cache and /// additional search list of read-only cache directories. pub fn build(self) -> Cache { Cache { @@ -225,12 +235,16 @@ impl Cache { /// additional read-only cache, in definition order, and return a /// read-only file for the first hit. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). /// - /// Returns `None` if no file for `key` can be found in any of the + /// Returns [`None`] if no file for `key` can be found in any of the /// constituent caches, and bubbles up the first I/O error /// encountered, if any. + /// + /// In the worst case, each call to `get` attempts to open two + /// files for the [`Cache`]'s read-write directory and for each + /// read-only backup directory. pub fn get<'a>(&self, key: impl Into>) -> Result> { fn doit( write_side: Option<&dyn FullCache>, @@ -257,8 +271,10 @@ impl Cache { /// populates the cache with a file filled by `populate`. Returns /// a file in all cases (unless the call fails with an error). /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid - /// (empty, or starts with a dot or a forward or back slash). + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is + /// invalid (empty, or starts with a dot or a forward or back slash). + /// + /// See [`Cache::get_or_update`] for more control over the operation. pub fn ensure<'a>( &self, key: impl Into>, @@ -276,12 +292,24 @@ impl Cache { /// filled by `populate`; otherwise obeys the value returned by /// `judge` to determine what to do with the hit. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid - /// (empty, or starts with a dot or a forward or back slash). + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is + /// invalid (empty, or starts with a dot or a forward or back slash). /// /// When we need to populate a new file, `populate` is called with /// a mutable reference to the destination file, and the old /// cached file (in whatever state `judge` left it), if available. + /// cached file, if available. + /// + /// See [`Cache::ensure`] for a simpler interface. + /// + /// In the worst case, each call to `get_or_update` attempts to + /// open two files for the [`Cache`]'s read-write directory and + /// for each read-only backup directory, and fails to find + /// anything. `get_or_update` then publishes a new cached file + /// (in a constant number of file operations), but not before + /// triggering a second chance maintenance (time linearithmic in + /// the number of files in the directory chosen for maintenance, + /// but amortised to logarithmic). pub fn get_or_update<'a>( &self, key: impl Into>, @@ -357,13 +385,18 @@ impl Cache { /// Inserts or overwrites the file at `value` as `key` in the /// write cache directory. This will always fail with - /// `Unsupported` if no write cache was defined. + /// [`ErrorKind::Unsupported`] if no write cache was defined. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. + /// + /// Executes in a bounded number of file operations, except for + /// the lock-free maintenance, which needs time linearithmic in + /// the number of files in the directory chosen for maintenance, + /// amortised to logarithmic. pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { match self.write_side.as_ref() { Some(write) => write.set(key.into(), value.as_ref()), @@ -376,13 +409,19 @@ impl Cache { /// Inserts the file at `value` as `key` in the cache directory if /// there is no such cached entry already, or touches the cached - /// file if it already exists. + /// file if it already exists. This will always fail with + /// [`ErrorKind::Unsupported`] if no write cache was defined. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). /// /// Always consumes the file at `value` on success; may consume it /// on error. + /// + /// Executes in a bounded number of file operations, except for + /// the lock-free maintenance, which needs time linearithmic in + /// the number of files in the directory chosen for maintenance, + /// amortised to logarithmic. pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { match self.write_side.as_ref() { Some(write) => write.put(key.into(), value.as_ref()), @@ -393,14 +432,18 @@ impl Cache { } } - /// Marks a cache entry for `key` as accessed (read). The `Cache` + /// Marks a cache entry for `key` as accessed (read). The [`Cache`] /// will touch the same file that would be returned by `get`. /// - /// Fails with `ErrorKind::InvalidInput` if `key.name` is invalid + /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). /// /// Returns whether a file for `key` could be found, and bubbles /// up the first I/O error encountered, if any. + /// + /// In the worst case, each call to `touch` attempts to update the + /// access time on two files for each cache directory in the + /// `ReadOnlyCache` stack. pub fn touch<'a>(&self, key: impl Into>) -> Result { fn doit( write_side: Option<&dyn FullCache>, From 3ca13f3438905731ebe1df115b77744c453c5a43 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sun, 12 Sep 2021 14:17:35 -0400 Subject: [PATCH 09/12] NFC stack: move most of set/put to non-generic inner functions We're about to add more logic there. TESTED=existing tests. --- src/stack.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/stack.rs b/src/stack.rs index fc850e6..57de436 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -398,13 +398,17 @@ impl Cache { /// the number of files in the directory chosen for maintenance, /// amortised to logarithmic. pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { - match self.write_side.as_ref() { - Some(write) => write.set(key.into(), value.as_ref()), - None => Err(Error::new( - ErrorKind::Unsupported, - "no kismet write cache defined", - )), + fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { + match this.write_side.as_ref() { + Some(write) => write.set(key, value), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } } + + doit(self, key.into(), value.as_ref()) } /// Inserts the file at `value` as `key` in the cache directory if @@ -423,13 +427,17 @@ impl Cache { /// the number of files in the directory chosen for maintenance, /// amortised to logarithmic. pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { - match self.write_side.as_ref() { - Some(write) => write.put(key.into(), value.as_ref()), - None => Err(Error::new( - ErrorKind::Unsupported, - "no kismet write cache defined", - )), + fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { + match this.write_side.as_ref() { + Some(write) => write.put(key, value), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } } + + doit(self, key.into(), value.as_ref()) } /// Marks a cache entry for `key` as accessed (read). The [`Cache`] From c9ded26a0e372c595abcfa42cea1b837cbe214b8 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sun, 12 Sep 2021 14:51:16 -0400 Subject: [PATCH 10/12] stack: add new auto-sync logic, enabled by default Aim for safer defaults, by letting the higher-level writable cache sync files before publishing, at least by default. Unfortunately, we sometimes have to re-open a path only to sync it, and that's an error-prone pattern, especially if a caller retries to publish the same file: the fsync call is liable to succeed silently. That's why the new code panics when we fail to sync a file by path. We can now remove the scary warning from the doc comments in `lib.rs`, and move them to the lower level layers, `plain.rs` and `sharded.rs`. TESTED=updated tests. --- src/lib.rs | 47 ++++++-------- src/plain.rs | 6 +- src/sharded.rs | 6 +- src/stack.rs | 169 ++++++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 183 insertions(+), 45 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 786fa06..2559d58 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,8 +48,9 @@ //! cached file in the subdirectory (or plain cache directory). This //! eviction process is linearithmic-time in the number of files in //! the cache subdirectory (directory), and is invoked periodically, -//! so as to amortise the maintenance time overhead to logarithmic -//! per write to a cache subdirectory. +//! so as to amortise the maintenance time overhead to logarithmic (in +//! the total number of files in the subdirectory) per write to a +//! cache subdirectory, and file operations to a constant per write. //! //! Kismet does not pre-allocate any long-lived file object, so it may //! need to temporarily open file objects. However, each call into @@ -73,6 +74,8 @@ //! multiple places, as long as the files are not modified, or their //! `mtime` otherwise updated, through these non-Kismet links. //! +//! # Plain and sharded caches +//! //! Kismet cache directories are plain (unsharded) or sharded. //! //! Plain Kismet caches are simply directories where the cache entry for @@ -88,27 +91,20 @@ //! //! Simple usage should be covered by the [`ReadOnlyCache`] or //! [`Cache`] structs, which wrap [`plain::Cache`] and -//! [`sharded::Cache`] in a convenient type-erased interface. The -//! caches *do not* invoke [`std::fs::File::sync_all`] or [`std::fs::File::sync_data`]: -//! the caller should sync files before letting Kismet persist them in -//! a cache if necessary. File synchronisation is not automatic -//! because it makes sense to implement persistent filesystem caches -//! that are erased after each boot, e.g., via -//! [tmpfiles.d](https://www.freedesktop.org/software/systemd/man/tmpfiles.d.html), -//! or by tagging cache directories with a -//! [boot id](https://man7.org/linux/man-pages/man3/sd_id128_get_machine.3.html). -//! -//! The cache code also does not sync the parent cache directories: we -//! assume that it's safe, if unfortunate, for caches to lose data or -//! revert to an older state after kernel or hardware crashes. In -//! general, the code attempts to be robust again direct manipulation -//! of the cache directories. It's always safe to delete cache files -//! from kismet directories (ideally not recently created files in -//! `.kismet_temp` directories), and even *adding* files should mostly -//! do what one expects: they will be picked up if they're in the -//! correct place (in a plain unsharded cache directory or in the -//! correct shard subdirectory), and eventually evicted if useless or -//! in the wrong shard. +//! [`sharded::Cache`] in a convenient type-erased interface. +//! +//! While the cache code syncs cached data files by default, it does +//! not sync the parent cache directories: we assume that it's safe, +//! if unfortunate, for caches to lose data or revert to an older +//! state after kernel or hardware crashes. In general, the code +//! attempts to be robust again direct manipulation of the cache +//! directories. It's always safe to delete cache files from kismet +//! directories (ideally not recently created files in `.kismet_temp` +//! subdirectories), and even *adding* files should mostly do what one +//! expects: they will be picked up if they're in the correct place +//! (in a plain unsharded cache directory or in the correct shard +//! subdirectory), and eventually evicted if useless or in the wrong +//! shard. //! //! It is however essential to only publish files atomically to the //! cache directories, and it probably never makes sense to modify @@ -180,10 +176,7 @@ //! // Fetches the current cached value for `key`, or populates it with //! // the closure argument if missing. //! let mut cached_file = cache -//! .ensure(&key, |file| { -//! file.write_all(&get_contents(&key))?; -//! file.sync_all() -//! })?; +//! .ensure(&key, |file| file.write_all(&get_contents(&key)))?; //! let mut contents = Vec::new(); //! cached_file.read_to_end(&mut contents)?; //! # Ok(()) diff --git a/src/plain.rs b/src/plain.rs index 59c40d2..d82c8cc 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -8,7 +8,11 @@ //! in flat directories. //! //! This module is useful for lower level usage; in most cases, the -//! [`crate::Cache`] is more convenient and just as efficient. +//! [`crate::Cache`] is more convenient and just as efficient. In +//! particular, a `crate::plain::Cache` *does not* invoke +//! [`std::fs::File::sync_all`] or [`std::fs::File::sync_data`]: the +//! caller should sync files before letting Kismet persist them in a +//! directory, if necessary. //! //! The cache's contents will grow past its stated capacity, but //! should rarely reach more than twice that capacity. diff --git a/src/sharded.rs b/src/sharded.rs index cd9ca42..ff5f102 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -12,7 +12,11 @@ //! and an optional `.kismet_temp` subdirectory for temporary files. //! //! This module is useful for lower level usage; in most cases, the -//! [`crate::Cache`] is more convenient and just as efficient. +//! [`crate::Cache`] is more convenient and just as efficient. In +//! particular, a `crate::sharded::Cache` *does not* invoke +//! [`std::fs::File::sync_all`] or [`std::fs::File::sync_data`]: the +//! caller should sync files before letting Kismet persist them in a +//! directory, if necessary. //! //! The cache's contents will grow past its stated capacity, but //! should rarely reach more than twice that capacity, especially diff --git a/src/stack.rs b/src/stack.rs index 57de436..a9d438c 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -103,12 +103,23 @@ impl FullCache for ShardedCache { /// always first access its write-side cache (if defined), and, on /// misses, will attempt to service [`Cache::get`] and /// [`Cache::touch`] calls by iterating over the read-only caches. -#[derive(Debug, Default)] +#[derive(Debug)] pub struct CacheBuilder { write_side: Option>, + auto_sync: bool, read_side: ReadOnlyCacheBuilder, } +impl Default for CacheBuilder { + fn default() -> CacheBuilder { + CacheBuilder { + write_side: None, + auto_sync: true, + read_side: Default::default(), + } + } +} + /// A [`Cache`] wraps either up to one plain or sharded read-write /// cache in a convenient interface, and may optionally fulfill read /// operations by deferring to a list of read-only cache when the @@ -122,12 +133,30 @@ pub struct CacheBuilder { /// of directories: using the same [`Cache`] object improves the /// accuracy of the write cache's lock-free in-memory statistics, when /// it's a sharded cache. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct Cache { + // The write-side cache services writes and is the cache of first + // resort for `get` and `touch`. write_side: Option>, + // Whether to automatically sync file contents before publishing + // them to the write-side cache. + auto_sync: bool, + // The read-side cache (a list of read-only caches) services `get` + // and `touch` calls when we fail to find something in the + // write-side cache. read_side: ReadOnlyCache, } +impl Default for Cache { + fn default() -> Cache { + Cache { + write_side: None, + auto_sync: true, + read_side: Default::default(), + } + } +} + /// Where does a cache hit come from: the primary read-write cache, or /// one of the secondary read-only caches? pub enum CacheHit<'a> { @@ -195,6 +224,26 @@ impl CacheBuilder { self } + /// Sets whether files published read-write cache will be + /// automatically flushed to disk with [`File::sync_all`] + /// before sending them to the cache directory. + /// + /// Defaults to true, for safety. Even when `auto_sync` is + /// enabled, Kismet does not `fsync` cache directories; after a + /// kernel or hardware crash, caches may partially revert to an + /// older state, but should not contain incomplete files. + /// + /// An application may want to disable `auto_sync` because it + /// already synchronises files, or because the cache directories + /// do not survive crashes: they might be erased after each boot, + /// e.g., via + /// [tmpfiles.d](https://www.freedesktop.org/software/systemd/man/tmpfiles.d.html), + /// or tagged with a [boot id](https://man7.org/linux/man-pages/man3/sd_id128_get_machine.3.html). + pub fn auto_sync(mut self, sync: bool) -> Self { + self.auto_sync = sync; + self + } + /// Adds a new read-only cache directory at `path` to the end of the /// cache builder's search list. /// @@ -224,12 +273,46 @@ impl CacheBuilder { pub fn build(self) -> Cache { Cache { write_side: self.write_side, + auto_sync: self.auto_sync, read_side: self.read_side.build(), } } } impl Cache { + /// Calls [`File::sync_all`] on `file` if `Cache::auto_sync` + /// is true. + #[inline] + fn maybe_sync(&self, file: &File) -> Result<()> { + if self.auto_sync { + file.sync_all() + } else { + Ok(()) + } + } + + /// Opens `path` and calls [`File::sync_all`] on the resulting + /// file, if `Cache::auto_sync` is true. + /// + /// Panics when [`File::sync_all`] fails. See + /// https://wiki.postgresql.org/wiki/Fsync_Errors or + /// Rebello et al's "Can Applications Recover from fsync Failures?" + /// (https://www.usenix.org/system/files/atc20-rebello.pdf) + /// for an idea of the challenges associated with handling + /// fsync failures on persistent files. + fn maybe_sync_path(&self, path: &Path) -> Result<()> { + if self.auto_sync { + // It's really not clear what happens to a file's content + // if we open it just before fsync, and fsync fails. It + // should be safe to just unlink the file + std::fs::File::open(&path)? + .sync_all() + .expect("auto_sync failed, and failure semantics are unclear for fsync"); + } + + Ok(()) + } + /// Attempts to open a read-only file for `key`. The `Cache` will /// query each its write cache (if any), followed by the list of /// additional read-only cache, in definition order, and return a @@ -328,17 +411,19 @@ impl Cache { } // Promotes `file` to `cache`. - fn promote(cache: &dyn FullCache, key: Key, mut file: File) -> Result { + fn promote(cache: &dyn FullCache, sync: bool, key: Key, mut file: File) -> Result { use std::io::Seek; let mut tmp = NamedTempFile::new_in(cache.temp_dir(key)?)?; std::io::copy(&mut file, tmp.as_file_mut())?; // Force the destination file's contents to disk before - // adding it to the read-write cache: the caller can't - // tell us whether they want a `fsync`, so let's be safe - // and assume they do. - tmp.as_file().sync_all()?; + // adding it to the read-write cache, if we're supposed to + // sync files automatically. + if sync { + tmp.as_file().sync_all()?; + } + cache.put(key, tmp.path())?; // We got a read-only file. Rewind it before returning. @@ -360,7 +445,9 @@ impl Cache { } else if let Some(mut file) = self.read_side.get(key)? { match judge(CacheHit::Secondary(&mut file)) { CacheHitAction::Accept => return Ok(file), - CacheHitAction::Promote => return promote(get_write_cache(self)?, key, file), + CacheHitAction::Promote => { + return promote(get_write_cache(self)?, self.auto_sync, key, file) + } CacheHitAction::Replace => old = Some(file), } } @@ -370,6 +457,7 @@ impl Cache { // Either way, start by populating a temporary file. let mut tmp = NamedTempFile::new_in(cache.temp_dir(key)?)?; populate(tmp.as_file_mut(), old)?; + self.maybe_sync(tmp.as_file())?; // Grab a read-only return value before publishing the file. let path = tmp.path(); @@ -393,14 +481,24 @@ impl Cache { /// Always consumes the file at `value` on success; may consume it /// on error. /// + /// When `auto_sync` is enabled (the default), the file at `value` + /// will always be [`File::sync_all`]ed before publishing to the + /// cache. Kismet will **panic** when the [`File::sync_all`] call + /// itself fails: retrying the same call to [`Cache::set`] could + /// erroneously succeed, since some filesystems clear internal I/O + /// failure flag after the first `fsync`. + /// /// Executes in a bounded number of file operations, except for /// the lock-free maintenance, which needs time linearithmic in /// the number of files in the directory chosen for maintenance, - /// amortised to logarithmic. + /// amortised to logarithmic, and constant number of file operations. pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { match this.write_side.as_ref() { - Some(write) => write.set(key, value), + Some(write) => { + this.maybe_sync_path(value)?; + write.set(key, value) + } None => Err(Error::new( ErrorKind::Unsupported, "no kismet write cache defined", @@ -422,14 +520,24 @@ impl Cache { /// Always consumes the file at `value` on success; may consume it /// on error. /// + /// When `auto_sync` is enabled (the default), the file at `value` + /// will always be [`File::sync_all`]ed before publishing to the + /// cache. Kismet will **panic** when the [`File::sync_all`] call + /// itself fails: retrying the same call to [`Cache::put`] could + /// erroneously succeed, since some filesystems clear internal I/O + /// failure flag after the first `fsync`. + /// /// Executes in a bounded number of file operations, except for /// the lock-free maintenance, which needs time linearithmic in /// the number of files in the directory chosen for maintenance, - /// amortised to logarithmic. + /// amortised to logarithmic, and constant number of file operations. pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { match this.write_side.as_ref() { - Some(write) => write.put(key, value), + Some(write) => { + this.maybe_sync_path(value)?; + write.put(key, value) + } None => Err(Error::new( ErrorKind::Unsupported, "no kismet write cache defined", @@ -509,20 +617,45 @@ mod test { // write calls should fail. #[test] fn empty() { + use test_dir::{DirBuilder, FileType, TestDir}; + + let temp = TestDir::temp().create("foo", FileType::RandomFile(10)); let cache: Cache = Default::default(); assert!(matches!(cache.get(&TestKey::new("foo")), Ok(None))); assert!( matches!(cache.ensure(&TestKey::new("foo"), |_| unreachable!("should not be called when there is no write side")), - Err(e) if e.kind() == ErrorKind::Unsupported) + Err(e) if e.kind() == ErrorKind::Unsupported) ); - assert!(matches!(cache.set(&TestKey::new("foo"), "/tmp/foo"), + assert!(matches!(cache.set(&TestKey::new("foo"), &temp.path("foo")), Err(e) if e.kind() == ErrorKind::Unsupported)); - assert!(matches!(cache.put(&TestKey::new("foo"), "/tmp/foo"), + assert!(matches!(cache.put(&TestKey::new("foo"), &temp.path("foo")), Err(e) if e.kind() == ErrorKind::Unsupported)); assert!(matches!(cache.touch(&TestKey::new("foo")), Ok(false))); } + // Disable autosync; we should get an `Unsupported` error even if the + // input file does not exist. + #[test] + fn empty_no_auto_sync() { + let cache = CacheBuilder::new().auto_sync(false).build(); + + assert!(matches!(cache.get(&TestKey::new("foo")), Ok(None))); + assert!( + matches!(cache.ensure(&TestKey::new("foo"), |_| unreachable!("should not be called when there is no write side")), + Err(e) if e.kind() == ErrorKind::Unsupported) + ); + assert!( + matches!(cache.set(&TestKey::new("foo"), "/no-such-tmp/foo"), + Err(e) if e.kind() == ErrorKind::Unsupported) + ); + assert!( + matches!(cache.put(&TestKey::new("foo"), "/no-such-tmp/foo"), + Err(e) if e.kind() == ErrorKind::Unsupported) + ); + assert!(matches!(cache.touch(&TestKey::new("foo")), Ok(false))); + } + // Fail to find a file, ensure it, then see that we can get it. #[test] fn test_ensure() { @@ -530,7 +663,11 @@ mod test { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = CacheBuilder::new().writer(temp.path("."), 1, 10).build(); + // Get some coverage for no-auto_sync config. + let cache = CacheBuilder::new() + .writer(temp.path("."), 1, 10) + .auto_sync(false) + .build(); let key = TestKey::new("foo"); // The file doesn't exist initially. From 88ba814e5ac4fd541ada50a872ad4e0f7d3002d7 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sun, 12 Sep 2021 15:01:57 -0400 Subject: [PATCH 11/12] NFC stack: move more of the `set` / `put` logic to associated functions We'll want to call these, with our own `sync` logic, for `NamedTempFile`s. TESTED=existing tests. --- src/stack.rs | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/stack.rs b/src/stack.rs index a9d438c..1c7b9c3 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -471,6 +471,16 @@ impl Cache { Ok(ret) } + fn set_impl(&self, key: Key, value: &Path) -> Result<()> { + match self.write_side.as_ref() { + Some(write) => write.set(key, value), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } + } + /// Inserts or overwrites the file at `value` as `key` in the /// write cache directory. This will always fail with /// [`ErrorKind::Unsupported`] if no write cache was defined. @@ -494,21 +504,23 @@ impl Cache { /// amortised to logarithmic, and constant number of file operations. pub fn set<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { - match this.write_side.as_ref() { - Some(write) => { - this.maybe_sync_path(value)?; - write.set(key, value) - } - None => Err(Error::new( - ErrorKind::Unsupported, - "no kismet write cache defined", - )), - } + this.maybe_sync_path(value)?; + this.set_impl(key, value) } doit(self, key.into(), value.as_ref()) } + fn put_impl(&self, key: Key, value: &Path) -> Result<()> { + match self.write_side.as_ref() { + Some(write) => write.put(key, value), + None => Err(Error::new( + ErrorKind::Unsupported, + "no kismet write cache defined", + )), + } + } + /// Inserts the file at `value` as `key` in the cache directory if /// there is no such cached entry already, or touches the cached /// file if it already exists. This will always fail with @@ -533,16 +545,8 @@ impl Cache { /// amortised to logarithmic, and constant number of file operations. pub fn put<'a>(&self, key: impl Into>, value: impl AsRef) -> Result<()> { fn doit(this: &Cache, key: Key, value: &Path) -> Result<()> { - match this.write_side.as_ref() { - Some(write) => { - this.maybe_sync_path(value)?; - write.put(key, value) - } - None => Err(Error::new( - ErrorKind::Unsupported, - "no kismet write cache defined", - )), - } + this.maybe_sync_path(value)?; + this.put_impl(key, value) } doit(self, key.into(), value.as_ref()) From 23db347bb79a1effa78f58692b0a020124c3ce8f Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sun, 12 Sep 2021 15:37:27 -0400 Subject: [PATCH 12/12] stack: add convenience functions to publish `NamedTempFile`s Getting ownership over the file also means we can safely handle fsync failures: bubbling the failure up will also delete the temporary file, which shouldn't leave any residual clean-but-never-synced page behind. TESTED=tweaked smoke test. --- src/stack.rs | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/stack.rs b/src/stack.rs index 1c7b9c3..0496db7 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -484,6 +484,8 @@ impl Cache { /// Inserts or overwrites the file at `value` as `key` in the /// write cache directory. This will always fail with /// [`ErrorKind::Unsupported`] if no write cache was defined. + /// The path at `value` must be in the same filesystem as the + /// write cache directory: we rely on atomic file renames. /// /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). @@ -511,6 +513,20 @@ impl Cache { doit(self, key.into(), value.as_ref()) } + /// Invokes [`Cache::set`] on a [`tempfile::NamedTempFile`]. + /// + /// See [`Cache::set`] for more details. The only difference is + /// that `set_temp_file` does not panic when `auto_sync` is enabled + /// and we fail to [`File::sync_all`] the [`NamedTempFile`] value. + pub fn set_temp_file<'a>(&self, key: impl Into>, value: NamedTempFile) -> Result<()> { + fn doit(this: &Cache, key: Key, value: NamedTempFile) -> Result<()> { + this.maybe_sync(value.as_file())?; + this.set_impl(key, value.path()) + } + + doit(self, key.into(), value) + } + fn put_impl(&self, key: Key, value: &Path) -> Result<()> { match self.write_side.as_ref() { Some(write) => write.put(key, value), @@ -525,6 +541,8 @@ impl Cache { /// there is no such cached entry already, or touches the cached /// file if it already exists. This will always fail with /// [`ErrorKind::Unsupported`] if no write cache was defined. + /// The path at `value` must be in the same filesystem as the + /// write cache directory: we rely on atomic file hard linkage. /// /// Fails with [`ErrorKind::InvalidInput`] if `key.name` is invalid /// (empty, or starts with a dot or a forward or back slash). @@ -552,6 +570,20 @@ impl Cache { doit(self, key.into(), value.as_ref()) } + /// Invokes [`Cache::put`] on a [`tempfile::NamedTempFile`]. + /// + /// See [`Cache::put`] for more details. The only difference is + /// that `put_temp_file` does not panic when `auto_sync` is enabled + /// and we fail to [`File::sync_all`] the [`NamedTempFile`] value. + pub fn put_temp_file<'a>(&self, key: impl Into>, value: NamedTempFile) -> Result<()> { + fn doit(this: &Cache, key: Key, value: NamedTempFile) -> Result<()> { + this.maybe_sync(value.as_file())?; + this.put_impl(key, value.path()) + } + + doit(self, key.into(), value) + } + /// Marks a cache entry for `key` as accessed (read). The [`Cache`] /// will touch the same file that would be returned by `get`. /// @@ -1107,8 +1139,9 @@ mod test { tmp.as_file() .write_all(b"write2") .expect("write must succeed"); + // Exercise put_temp_file as well. cache - .put(&TestKey::new("b"), tmp.path()) + .put_temp_file(&TestKey::new("b"), tmp) .expect("put must succeed"); } @@ -1261,8 +1294,9 @@ mod test { tmp.as_file() .write_all(b"write2") .expect("write must succeed"); + // Exercise set_temp_file. cache - .set(&TestKey::new("b"), tmp.path()) + .set_temp_file(&TestKey::new("b"), tmp) .expect("set must succeed"); }