mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-27 15:37:02 +00:00
perf(ecstore): skip tmp parent dir fsync for write-then-rename paths (#4387)
Step 1 of 4 for rustfs/backlog#922 (HP-1): write_all_internal previously coupled file-content durability (fdatasync) with directory-entry durability (parent dir fsync) behind a single bool. For tmp files that are immediately renamed away, the tmp parent dir fsync contributes nothing to crash consistency: the safe-rename recipe only needs file content fdatasync -> rename -> fsync of the destination parent, because the rename removes the tmp directory entry and a crash before the rename means the PUT was never acknowledged. Replace the bool with a SyncMode enum (None / FileAndDir / FileOnly) and use FileOnly at exactly the two write-then-rename tmp write points: the tmp xl.meta write in the non-inline rename_data path and the tmp write inside write_all_meta. write_all_public (format.json etc.) and the old-metadata rollback backup keep FileAndDir since those files stay where they are written. The rest of the commit sequence (shard sync_dir_files, rename, destination parent fsync) is untouched, and behavior with drive sync disabled is unchanged. This saves one directory fsync per disk per non-inline PUT (4 on a 4-drive set). Unit tests assert, via a test-only fsync-dir recorder, that tmp write points no longer fsync their parent while the public write point, the rollback backup, and the commit-rename destination parent still do. Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -63,9 +63,33 @@ pub fn check_path_length(path_name: &str) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Test-only recorder of every directory passed to [`fsync_dir_std`].
|
||||
///
|
||||
/// Durability regressions are invisible to ordinary behavior tests (the data
|
||||
/// is on disk either way), so unit tests assert directly on which directories
|
||||
/// were fsynced. Paths are recorded globally; tests must match on paths under
|
||||
/// their own unique tempdir to stay robust against parallel test execution.
|
||||
#[cfg(test)]
|
||||
pub(crate) mod fsync_dir_recorder {
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Mutex;
|
||||
|
||||
static RECORDED: Mutex<Vec<PathBuf>> = Mutex::new(Vec::new());
|
||||
|
||||
pub(crate) fn record(dir: &Path) {
|
||||
RECORDED.lock().expect("fsync dir recorder poisoned").push(dir.to_path_buf());
|
||||
}
|
||||
|
||||
pub(crate) fn was_fsynced(dir: &Path) -> bool {
|
||||
RECORDED.lock().expect("fsync dir recorder poisoned").iter().any(|p| p == dir)
|
||||
}
|
||||
}
|
||||
|
||||
/// Fsync a directory so recently created or renamed entries survive power loss.
|
||||
/// No-op on non-Unix platforms where directories cannot be opened for syncing.
|
||||
pub fn fsync_dir_std(dir: impl AsRef<Path>) -> io::Result<()> {
|
||||
#[cfg(test)]
|
||||
fsync_dir_recorder::record(dir.as_ref());
|
||||
#[cfg(unix)]
|
||||
{
|
||||
std::fs::File::open(dir.as_ref())?.sync_all()?;
|
||||
|
||||
Reference in New Issue
Block a user