From d100f7a2c8681b2d4dd1bf03797ef8fb3157358b Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 19 Mar 2022 23:29:16 +1000 Subject: [PATCH] Don't fsync media files on import I was seeing import speeds of only 10-20 files a second before this change. --- rslib/src/import_export/package/colpkg/export.rs | 2 +- rslib/src/import_export/package/colpkg/import.rs | 4 ++-- rslib/src/io.rs | 15 ++++++++++++--- rslib/src/sync/mod.rs | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/rslib/src/import_export/package/colpkg/export.rs b/rslib/src/import_export/package/colpkg/export.rs index f73cd2671..e2247bc83 100644 --- a/rslib/src/import_export/package/colpkg/export.rs +++ b/rslib/src/import_export/package/colpkg/export.rs @@ -63,7 +63,7 @@ impl Collection { &tr, progress_fn, )?; - atomic_rename(temp_colpkg, colpkg_name) + atomic_rename(temp_colpkg, colpkg_name, true) } } diff --git a/rslib/src/import_export/package/colpkg/import.rs b/rslib/src/import_export/package/colpkg/import.rs index c013a95ed..441aa4a2d 100644 --- a/rslib/src/import_export/package/colpkg/import.rs +++ b/rslib/src/import_export/package/colpkg/import.rs @@ -75,7 +75,7 @@ pub fn import_colpkg( let media_folder = Path::new(target_media_folder); restore_media(&meta, progress_fn, &mut archive, media_folder)?; - atomic_rename(tempfile, &col_path) + atomic_rename(tempfile, &col_path, true) } fn check_collection(col_path: &Path) -> Result<()> { @@ -143,7 +143,7 @@ fn restore_media_file(meta: &Meta, zip_file: &mut ZipFile, path: &Path) -> Resul } .map_err(|err| AnkiError::file_io_error(err, path))?; - atomic_rename(tempfile, path) + atomic_rename(tempfile, path, false) } impl MediaEntry { diff --git a/rslib/src/io.rs b/rslib/src/io.rs index 367d5d67a..8a68ffdb3 100644 --- a/rslib/src/io.rs +++ b/rslib/src/io.rs @@ -14,11 +14,20 @@ pub(crate) fn tempfile_in_parent_of(file: &Path) -> Result { NamedTempFile::new_in(dir).map_err(|err| AnkiError::file_io_error(err, dir)) } -pub(crate) fn atomic_rename(file: NamedTempFile, target: &Path) -> Result<()> { - file.as_file().sync_all()?; +/// Atomically replace the target path with the provided temp file. +/// +/// If `fsync` is true, file data is synced to disk prior to renaming, and the +/// folder is synced on UNIX platforms after renaming. This minimizes the +/// chances of corruption if there is a crash or power loss directly after the +/// op, but it can be considerably slower. +pub(crate) fn atomic_rename(file: NamedTempFile, target: &Path, fsync: bool) -> Result<()> { + if fsync { + file.as_file().sync_all()?; + } file.persist(&target) .map_err(|err| AnkiError::IoError(format!("write {target:?} failed: {err}")))?; - if !cfg!(windows) { + #[cfg(not(windows))] + if fsync { if let Some(parent) = target.parent() { std::fs::File::open(parent) .and_then(|file| file.sync_all()) diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index 3e66a2226..538790a0c 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -680,7 +680,7 @@ impl Collection { let db = open_and_check_sqlite_file(out_file.path())?; db.execute_batch("update col set ls=mod")?; drop(db); - atomic_rename(out_file, &col_path)?; + atomic_rename(out_file, &col_path, true)?; Ok(()) }