From 19f81ae397653a14febafdd44cd219611a899840 Mon Sep 17 00:00:00 2001 From: Ron Kuris Date: Tue, 28 Nov 2023 07:53:21 -0800 Subject: [PATCH] Growth ring file handling improvements (#381) --- growth-ring/Cargo.toml | 1 + growth-ring/src/lib.rs | 6 ++-- growth-ring/src/wal.rs | 59 ++++++++++++++++++++++++++------- growth-ring/tests/common/mod.rs | 5 +-- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/growth-ring/Cargo.toml b/growth-ring/Cargo.toml index 80d58437f..7a3ae56fe 100644 --- a/growth-ring/Cargo.toml +++ b/growth-ring/Cargo.toml @@ -26,6 +26,7 @@ hex = "0.4.3" rand = "0.8.5" indexmap = "2.0.0" tokio = { version = "1.28.1", features = ["tokio-macros", "rt", "macros"] } +test-case = "3.1.0" [lib] name = "growthring" diff --git a/growth-ring/src/lib.rs b/growth-ring/src/lib.rs index 3a6e58d4c..94debb137 100644 --- a/growth-ring/src/lib.rs +++ b/growth-ring/src/lib.rs @@ -48,8 +48,6 @@ //! // After each recovery, the /tmp/walfiles is empty. //! ``` -#[macro_use] -extern crate scan_fmt; pub mod wal; pub mod walerror; @@ -175,7 +173,7 @@ pub fn oflags() -> OFlag { #[async_trait(?Send)] impl WalStore for WalStoreImpl { - type FileNameIter = std::vec::IntoIter; + type FileNameIter = std::vec::IntoIter; async fn open_file(&self, filename: &str, _touch: bool) -> Result { let path = self.root_dir.join(filename); @@ -193,7 +191,7 @@ impl WalStore for WalStoreImpl { fn enumerate_files(&self) -> Result { let mut filenames = Vec::new(); for path in fs::read_dir(&self.root_dir)?.filter_map(|entry| entry.ok()) { - filenames.push(path.file_name().into_string().unwrap()); + filenames.push(path.path()); } Ok(filenames.into_iter()) } diff --git a/growth-ring/src/wal.rs b/growth-ring/src/wal.rs index 71ee1ed30..738a79a82 100644 --- a/growth-ring/src/wal.rs +++ b/growth-ring/src/wal.rs @@ -9,11 +9,15 @@ use futures::{ Future, }; -use std::cell::{RefCell, UnsafeCell}; use std::convert::{TryFrom, TryInto}; use std::mem::MaybeUninit; use std::num::NonZeroUsize; use std::pin::Pin; +use std::{ + cell::{RefCell, UnsafeCell}, + ffi::OsStr, + path::{Path, PathBuf}, +}; use std::{ collections::{hash_map, BinaryHeap, HashMap, VecDeque}, marker::PhantomData, @@ -21,8 +25,6 @@ use std::{ pub use crate::walerror::WalError; -const FILENAME_FMT: &str = r"[0-9a-f]+\.log"; - enum WalRingType { #[allow(dead_code)] Null = 0x0, @@ -59,8 +61,23 @@ type WalFileId = u64; pub type WalBytes = Box<[u8]>; pub type WalPos = u64; -fn get_fid(fname: &str) -> WalFileId { - scan_fmt!(fname, "{x}.log", [hex WalFileId]).unwrap() +// convert XXXXXX.log into number from the XXXXXX (in hex) +fn get_fid(fname: &Path) -> Result { + let wal_err: WalError = WalError::Other("not a log file".to_string()); + + if fname.extension() != Some(OsStr::new("log")) { + return Err(wal_err); + } + + u64::from_str_radix( + fname + .file_stem() + .unwrap_or(OsStr::new("")) + .to_str() + .unwrap_or(""), + 16, + ) + .map_err(|_| wal_err) } fn get_fname(fid: WalFileId) -> String { @@ -199,7 +216,7 @@ pub trait WalFile { #[async_trait(?Send)] pub trait WalStore { - type FileNameIter: Iterator; + type FileNameIter: Iterator; /// Open a file given the filename, create the file if not exists when `touch` is `true`. async fn open_file(&self, filename: &str, touch: bool) -> Result; @@ -729,7 +746,6 @@ impl> WalWriter { nrecords: usize, recover_policy: &RecoverPolicy, ) -> Result, WalError> { - let filename_fmt = regex::Regex::new(FILENAME_FMT).unwrap(); let file_pool = &self.file_pool; let file_nbit = file_pool.file_nbit; let block_size = 1 << file_pool.block_nbit; @@ -740,8 +756,7 @@ impl> WalWriter { file_pool .store .enumerate_files()? - .filter(|f| filename_fmt.is_match(f)) - .map(|s| get_fid(&s)) + .flat_map(|s| get_fid(&s)) .collect(), ); @@ -1179,7 +1194,6 @@ impl WalLoader { let msize = std::mem::size_of::(); assert!(self.file_nbit > self.block_nbit); assert!(msize < 1 << self.block_nbit); - let filename_fmt = regex::Regex::new(FILENAME_FMT).unwrap(); let mut file_pool = WalFilePool::new(store, self.file_nbit, self.block_nbit, self.cache_size).await?; let logfiles = sort_fids( @@ -1187,8 +1201,7 @@ impl WalLoader { file_pool .store .enumerate_files()? - .filter(|f| filename_fmt.is_match(f)) - .map(|s| get_fid(&s)) + .flat_map(|s| get_fid(&s)) .collect(), ); @@ -1301,3 +1314,25 @@ impl WalLoader { } pub const CRC32: crc::Crc = crc::Crc::::new(&crc::CRC_32_CKSUM); + +#[cfg(test)] +mod test { + use super::*; + use test_case::test_case; + + #[test_case("foo", Err("not a log file"); "no log extension")] + #[test_case("foo.log", Err("not a log file"); "invalid digit found in string")] + #[test_case("0000001.log", Ok(1); "happy path")] + #[test_case("1.log", Ok(1); "no leading zeroes")] + + fn test_get_fid(input: &str, expected: Result) { + let got = get_fid(Path::new(input)); + match expected { + Err(has) => { + let err = got.err().unwrap().to_string(); + assert!(err.contains(has), "{:?}", err) + } + Ok(val) => assert_eq!(got.unwrap(), val), + } + } +} diff --git a/growth-ring/tests/common/mod.rs b/growth-ring/tests/common/mod.rs index 3c5955a4f..d461c8216 100644 --- a/growth-ring/tests/common/mod.rs +++ b/growth-ring/tests/common/mod.rs @@ -11,6 +11,7 @@ use std::cell::RefCell; use std::collections::VecDeque; use std::collections::{hash_map, HashMap}; use std::convert::TryInto; +use std::path::PathBuf; use std::rc::Rc; pub trait FailGen { @@ -126,7 +127,7 @@ impl<'a, G> WalStore> for WalStoreEmul<'a, G> where G: 'static + FailGen, { - type FileNameIter = std::vec::IntoIter; + type FileNameIter = std::vec::IntoIter; async fn open_file(&self, filename: &str, touch: bool) -> Result, WalError> { if self.fgen.next_fail() { @@ -175,7 +176,7 @@ where } let mut logfiles = Vec::new(); for (fname, _) in self.state.borrow().files.iter() { - logfiles.push(fname.clone()) + logfiles.push(fname.into()) } Ok(logfiles.into_iter()) }