mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 23:18:24 +00:00
fix(storage): persist backup retirement before journal removal
Motivation: writing Retired before recursive deletion makes retries safe, but removing the journal durably without fsyncing the backup directory changes could leave a resurrected backup and no journal after power loss. Approach: fsync the immediate backup parent after remove_dir_all, then fsync each parent after pruning an empty child directory. The journal is removed only after those directory-entry changes are durable. Non-empty or already-absent ancestors stop optional pruning; unexpected pruning errors are logged without undoing the already-durable backup removal. Correctness assumptions: the migration root and journals reside on the same durable Git-data filesystem, and directory fsync provides the persistence boundary for entry removal. A failure before journal removal retains Retired state for an idempotent restart. Deliberately excluded: this does not change which backups qualify for retirement or the content-addressed quarantine layout. Validation: all migration unit tests pass, including complete and Retired restart states, successful parent pruning, retained backups, and content quarantine. Formatting and diff checks pass.
This commit is contained in:
+2
-1
@@ -81,7 +81,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
family migrates, bounding peak migration disk overhead to the family in
|
||||
flight. Unindexed legacy packs are quarantined by content under
|
||||
`.grasp/migration/unindexed-packs/`, unverifiable backups are retained with
|
||||
a warning, and completed installations whose backups were already removed
|
||||
a warning, backup-directory removals are fsynced before their journals are
|
||||
removed, and completed installations whose backups were already removed
|
||||
manually start unchanged.
|
||||
- Stop treating server-side shallow repositories as valid family storage.
|
||||
Migration preserves a legacy `shallow` marker on the installed thin view
|
||||
|
||||
@@ -291,6 +291,11 @@ before the next family is migrated. Peak migration overhead is therefore
|
||||
bounded by the family currently in flight except for the small set of families
|
||||
still awaiting repair.
|
||||
|
||||
Backup deletion is itself durable: every directory entry removed while
|
||||
deleting and pruning a backup is fsynced before its journal is removed. A
|
||||
power loss therefore leaves either a discoverable `retired` journal that
|
||||
finishes deletion or no backup entry to rediscover.
|
||||
|
||||
Packs without an index are Git-invisible, so the superset proof cannot vouch
|
||||
for them; they are moved into content-addressed directories beneath
|
||||
`.grasp/migration/unindexed-packs/` before their backup is deleted. Repeated
|
||||
|
||||
+31
-4
@@ -979,6 +979,7 @@ fn remove_retired_backup(storage: &LocalGitStorage, backup: &Path) -> Result<boo
|
||||
backup.display()
|
||||
));
|
||||
}
|
||||
let backup_parent = backup.parent().context("migration backup has no parent")?;
|
||||
match std::fs::remove_dir_all(backup) {
|
||||
Ok(()) => {}
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false),
|
||||
@@ -987,12 +988,38 @@ fn remove_retired_backup(storage: &LocalGitStorage, backup: &Path) -> Result<boo
|
||||
.with_context(|| format!("remove retired backup {}", backup.display()));
|
||||
}
|
||||
}
|
||||
let mut parent = backup.parent();
|
||||
while let Some(directory) = parent {
|
||||
if !directory.starts_with(&root) || std::fs::remove_dir(directory).is_err() {
|
||||
// Commit removal of the backup entry before the durable journal is
|
||||
// removed. Otherwise a power loss could restore the backup without its
|
||||
// journal, leaving an orphan that no later launch discovers.
|
||||
fsync_directory(backup_parent)?;
|
||||
|
||||
let mut directory = backup_parent;
|
||||
while directory.starts_with(&root) {
|
||||
let Some(parent) = directory.parent() else {
|
||||
break;
|
||||
};
|
||||
match std::fs::remove_dir(directory) {
|
||||
Ok(()) => {
|
||||
fsync_directory(parent)?;
|
||||
directory = parent;
|
||||
}
|
||||
Err(error)
|
||||
if matches!(
|
||||
error.kind(),
|
||||
std::io::ErrorKind::DirectoryNotEmpty | std::io::ErrorKind::NotFound
|
||||
) =>
|
||||
{
|
||||
break;
|
||||
}
|
||||
Err(error) => {
|
||||
warn!(
|
||||
directory = %directory.display(),
|
||||
%error,
|
||||
"Could not prune empty migration backup directory"
|
||||
);
|
||||
break;
|
||||
}
|
||||
}
|
||||
parent = directory.parent();
|
||||
}
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user