mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 16:28:15 +00:00
fix(ecstore): fsync inline rename_data rollback backup (#4346)
* fix(ecstore): fsync inline rename_data rollback backup The inline branch of LocalDisk::rename_data writes the previous version's xl.meta rollback backup (<old_data_dir>/xl.meta.bkp) with a bare std::fs::write, fsyncing neither the file nor its new directory entry, even when drive_sync_enabled() is true. The same closure already syncs the new xl.meta and the commit rename directory, and the non-inline branch persists its backup durably. That backup is the sole restore source for delete_version(undo_write=true) on a set-level write-quorum failure. With sync enabled, an inline overwrite plus power loss plus a quorum failure that triggers undo_write could restore a lost or truncated backup and fail to roll back to the prior committed object. Write the inline backup via OpenOptions + write_all and, when sync is enabled, sync_data() it and fsync_dir_std its parent, mirroring the non-inline branch. Extend the inline backup test to assert the .bkp contents equal the previous metadata bytes. Refs backlog#868 (disk-durability-01). * fix(ecstore): preserve to_file_error mapping on inline backup write Address review: the rewritten inline rollback-backup write must keep the same DiskError classification as the previous std::fs::write(...).map_err( to_file_error)? call. Without it, io errors (PermissionDenied/NotFound/ StorageFull/...) would surface as DiskError::Io(_) instead of the specific DiskError variants (FileAccessDenied/FileNotFound/DiskFull/...), since From<io::Error> for DiskError only recovers variants that were wrapped via to_file_error. Map open/write_all/sync_data/fsync_dir_std through to_file_error, matching write_all_internal. --------- Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
@@ -3869,10 +3869,29 @@ impl DiskAPI for LocalDisk {
|
||||
&& let Some(dst_parent) = dst.parent()
|
||||
{
|
||||
let old_path = dst_parent.join(old_dir.to_string()).join(STORAGE_FORMAT_FILE_BACKUP);
|
||||
if let Some(old_parent) = old_path.parent() {
|
||||
let old_parent = old_path.parent().map(|p| p.to_path_buf());
|
||||
if let Some(ref old_parent) = old_parent {
|
||||
std::fs::create_dir_all(old_parent)?;
|
||||
}
|
||||
std::fs::write(&old_path, buf).map_err(to_file_error)?;
|
||||
// This rollback backup is the sole restore source for a later
|
||||
// undo_write when the set-level write quorum fails. Persist it as
|
||||
// durably as the new xl.meta written above (and as the non-inline
|
||||
// branch does): a bare std::fs::write leaves both the bytes and the
|
||||
// new directory entry in the page cache, so a crash before a
|
||||
// rollback could restore a lost or truncated backup.
|
||||
let mut backup = std::fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.write(true)
|
||||
.truncate(true)
|
||||
.open(&old_path)
|
||||
.map_err(to_file_error)?;
|
||||
std::io::Write::write_all(&mut backup, buf).map_err(to_file_error)?;
|
||||
if sync {
|
||||
backup.sync_data().map_err(to_file_error)?;
|
||||
if let Some(ref old_parent) = old_parent {
|
||||
os::fsync_dir_std(old_parent).map_err(to_file_error)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
match std::fs::rename(&src, &dst) {
|
||||
@@ -4581,11 +4600,12 @@ mod test {
|
||||
ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await;
|
||||
|
||||
let old_fi = test_file_info(object, version_id, Some(old_data_dir), None);
|
||||
let old_meta = test_meta(old_fi);
|
||||
let dst_object_dir = dir.path().join(bucket).join(object);
|
||||
fs::create_dir_all(dst_object_dir.join(old_data_dir.to_string()))
|
||||
.await
|
||||
.expect("old data dir should be created");
|
||||
fs::write(dst_object_dir.join(STORAGE_FORMAT_FILE), test_meta(old_fi))
|
||||
fs::write(dst_object_dir.join(STORAGE_FORMAT_FILE), &old_meta)
|
||||
.await
|
||||
.expect("old metadata should be written");
|
||||
|
||||
@@ -4601,11 +4621,15 @@ mod test {
|
||||
.expect("inline rename_data should commit");
|
||||
|
||||
assert_eq!(resp.old_data_dir, Some(old_data_dir));
|
||||
assert!(
|
||||
dst_object_dir
|
||||
.join(old_data_dir.to_string())
|
||||
.join(STORAGE_FORMAT_FILE_BACKUP)
|
||||
.exists()
|
||||
let backup_path = dst_object_dir.join(old_data_dir.to_string()).join(STORAGE_FORMAT_FILE_BACKUP);
|
||||
assert!(backup_path.exists());
|
||||
// The rollback backup must contain the previous metadata bytes verbatim so
|
||||
// that undo_write can restore the prior committed object; guards the inline
|
||||
// backup write against truncation/corruption regressions.
|
||||
assert_eq!(
|
||||
fs::read(&backup_path).await.expect("backup should be readable"),
|
||||
old_meta,
|
||||
"inline rollback backup must contain the previous metadata bytes verbatim"
|
||||
);
|
||||
assert!(
|
||||
!dst_object_dir
|
||||
|
||||
Reference in New Issue
Block a user