-
Notifications
You must be signed in to change notification settings - Fork 16
Description
Problem
After creating new files, directories, or renaming files, the parent directory is never fsync'd. On Linux/Unix, sync_all() on a file only guarantees the file data and metadata are durable — it does not guarantee the directory entry (the name-to-inode mapping) is durable. A power loss after file creation or rename can leave the filesystem in a state where the file does not exist, even though sync_all() was called on the file itself.
No directory fsync exists anywhere in the archive module today.
Affected Locations
1. DataFile::rename() — data_file.rs:127-140 (Critical)
fs::rename(&self.data_file_path, path)at line 135- No fsync on the parent directory of either old or new path
- Risk: A committed and renamed pack file could revert to its old name (or disappear entirely) after a crash
2. DataFile::open() — data_file.rs:34-76 (Important)
File::create_new(path)at line 40 creates a new file when opening for write- No fsync on the parent directory after creation
- Risk: A newly created data file may not exist after a crash
3. HdxIndex::open_hdx_file() — digest_index/index.rs:264-332 (Important)
fs::create_dir(dir)at line 270 creates the index directoryOpenOptions::new().create(true).open(dir.join("index.hdx"))at lines 271-276 creates the index file- No fsync on parent directory after
create_dir, no fsync ondirafter file creation - Risk: Index directory or file may not exist after a crash
4. OdxHeader::open_odx_file() — digest_index/odx_header.rs:32-61 (Important)
OpenOptions::new().create(true).open(path)at line 39 creates the overflow index file- No fsync on the parent directory
- Risk: Overflow index file may not exist after a crash
5. PositionIndex::open_pdx_file() — position_index/index.rs:121-152 (Important)
fs::create_dir(dir)at line 127 creates the index directoryDataFile::open(dir.join("index.pdx"), read_only)at line 128 creates the index file (viaFile::create_newinternally)- No fsync on parent directory after
create_dir, no fsync ondirafter file creation - Risk: Position index directory or file may not exist after a crash
Solution
Step 1: Add a fsync_directory helper function
Add a utility function in data_file.rs (or a new small utility area in the archive module) that opens a directory as a read-only file and calls sync_all() on it:
/// Fsync a directory to ensure durable directory entries (file creates, renames).
pub(crate) fn fsync_directory(path: &Path) -> Result<(), io::Error> {
let dir = File::open(path)?;
dir.sync_all()
}On Unix, File::open on a directory path returns a valid file descriptor, and sync_all() issues an fsync() syscall on it. This is the POSIX-standard way to make directory entries durable.
Step 2: Fsync after DataFile::rename() — data_file.rs
After the successful fs::rename() at line 135, fsync the parent directory of the new path (and the old path's parent too, if it differs). Since rename within the same directory is the common case, one fsync usually suffices.
pub fn rename<P: AsRef<Path>>(&mut self, path: P) -> Result<(), RenameError> {
let path = path.as_ref();
if self.data_file_path == path {
return Ok(());
}
if path.exists() {
return Err(RenameError::FilesExist);
}
let old_parent = self.data_file_path.parent().map(Path::to_owned);
let res = fs::rename(&self.data_file_path, path);
if res.is_ok() {
self.data_file_path = path.to_owned();
// Fsync the new path's parent directory to make the rename durable.
if let Some(new_parent) = path.parent() {
fsync_directory(new_parent).map_err(RenameError::RenameIO)?;
}
// If the old parent differs, fsync it too (removal of old entry).
if let Some(old_parent) = &old_parent {
if path.parent() != Some(old_parent.as_path()) {
fsync_directory(old_parent).map_err(RenameError::RenameIO)?;
}
}
}
res.map_err(RenameError::RenameIO)
}Step 3: Fsync after DataFile::open() file creation — data_file.rs
When File::create_new(path) succeeds (file was actually created), fsync the parent directory. Change line 40 from ignoring the result to checking it:
if !ro {
if File::create_new(path).is_ok() {
// New file was created — fsync parent dir so the entry is durable.
if let Some(parent) = path.parent() {
let _ = fsync_directory(parent);
}
}
}Note: Errors are still tolerated here (the let _ =) since this is a best-effort creation path — the file may already exist.
Step 4: Fsync after open_hdx_file() — digest_index/index.rs
After fs::create_dir(dir) at line 270 and after the file open/create at lines 271-276, add directory fsyncs:
let dir = dir.as_ref();
let dir_created = fs::create_dir(dir).is_ok();
// Fsync parent so the new directory entry is durable.
if dir_created {
if let Some(parent) = dir.parent() {
let _ = fsync_directory(parent);
}
}
let mut hdx_file = OpenOptions::new()
.read(true)
.write(true)
.create(true)
.truncate(false)
.open(dir.join("index.hdx"))?;Then after the file has been initialized (header written + initial buckets written for the new-file branch), fsync the directory:
// After the if/else block that writes the header for new files:
// Fsync the index directory so the file entry is durable.
let _ = fsync_directory(dir);Step 5: Fsync after open_odx_file() — digest_index/odx_header.rs
After creating and writing the header for a new odx file, fsync the parent directory:
// After the if/else block for header:
if let Some(parent) = path.parent() {
let _ = fsync_directory(parent);
}Step 6: Fsync after open_pdx_file() — position_index/index.rs
Same pattern — after fs::create_dir(dir) at line 127, fsync the parent. After DataFile::open() at line 128, fsync dir:
let dir = dir.as_ref();
let dir_created = fs::create_dir(dir).is_ok();
if dir_created {
if let Some(parent) = dir.parent() {
let _ = fsync_directory(parent);
}
}
let mut pdx_file = DataFile::open(dir.join("index.pdx"), read_only)?;
// ... after header init ...
// Fsync the index directory so the pdx file entry is durable.
let _ = fsync_directory(dir);Files to Modify
| File | Change |
|---|---|
crates/storage/src/archive/data_file.rs |
Add fsync_directory() helper, call after File::create_new and fs::rename |
crates/storage/src/archive/digest_index/index.rs |
Call fsync_directory after create_dir and after file creation in open_hdx_file |
crates/storage/src/archive/digest_index/odx_header.rs |
Call fsync_directory after file creation in open_odx_file |
crates/storage/src/archive/position_index/index.rs |
Call fsync_directory after create_dir and after file creation in open_pdx_file |
Error Handling Strategy
DataFile::rename(): Directory fsync errors are propagated viaRenameError::RenameIO— rename durability is critical.DataFile::open()and index file opens: Directory fsync errors are best-effort (let _ =) since these paths already tolerate the file pre-existing (the create calls are conditional). The file will exist in memory and be fsync'd on the nextcommit(). If the process crashes before commit, the data wasn't durable anyway.
Testing
- Existing tests in
pack.rs,digest_index/index.rs, andposition_index/index.rsshould continue to pass — the fsync calls are additive. - No new tests needed: directory fsync correctness is a kernel guarantee; we only need to verify we don't introduce regressions (e.g., trying to fsync a nonexistent directory). The existing tests exercise the create/open/rename paths and will surface any
io::Errorpanics.
Related #510