From 37b735cf7b5846e5787892047393e81c9d67fcca Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Fri, 31 Jul 2026 11:32:07 +0200 Subject: [PATCH] fix(backup): verify the post-stop volume pass by content Each volume is copied twice into the same destination: once hot with the container running, once cold after it is stopped. The cold pass is meant to correct the hot one, but it uses rsync's quick check, which compares size and whole-second mtime. A pre-allocated 16 MiB WAL segment never changes size. When its last pre-stop write and postgres' shutdown checkpoint fall in the same whole second, the cold pass skips it while still replacing global/pg_control, whose previous write was seconds earlier. The generation then holds a post-shutdown pg_control pointing at a checkpoint LSN whose WAL bytes are still zero. Restoring it gives invalid record length at 0/3CB7A20: expected at least 24, got 0 invalid checkpoint record PANIC: could not locate a valid checkpoint record at 0/3CB7A20 and postgres crash-loops until the restore test gives up after 1200s. Observed in infinito-nexus-core CI run 30586213932, debian leg; the centos leg of the same run restored cleanly, and the two generations differ by exactly one 16 MiB WAL segment. authoritative is a required keyword rather than an optional flag, so each of the four call sites states which pass it is. Set, it adds --checksum, so the cold pass compares content. The hot passes keep the quick check. Reproduced with real rsync in both directions: without --checksum the WAL stays zero while pg_control is post-shutdown; with it the pair is consistent. Hardlink dedup against --link-dest is unaffected, measured at 20/20 with generation size unchanged. Rejected alternatives, both measured rather than argued: --ignore-times destroys dedup (20/20 to 0/20 hardlinks, generation 36 to 72 MiB), and --modify-window=-1 makes correctness depend on the destination filesystem preserving sub-second mtimes, which silently degrades on NFS or ext3. COST. The quick check scales with file count; --checksum scales with bytes, read on both sides, and it runs while the container is stopped. Measured warm-cache on 215 MiB: 0.10s to 0.39s, a factor of 3.9. Derived for disk-bound runs, the extra stop time stays under a minute up to roughly 4 GB on spinning disk, 15 GB on a SATA SSD, 60 GB on NVMe. At 1 TB it is 17 minutes on NVMe and over three hours on spinning disk; at 10 TB it is hours to days. No size threshold is built in - a guessed one would drop the guarantee exactly where an unrestorable backup costs most. Volumes at that scale need filesystem snapshots or pg_basebackup rather than two rsync passes over a live tree, which is a limit of this approach and not of this change. Co-Authored-By: Claude Opus 5 (1M context) --- src/baudolo/backup/app.py | 8 ++++---- src/baudolo/backup/volume.py | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/baudolo/backup/app.py b/src/baudolo/backup/app.py index 2829a52..c613793 100644 --- a/src/baudolo/backup/app.py +++ b/src/baudolo/backup/app.py @@ -226,19 +226,19 @@ def main() -> int: if args.everything: # "everything": always do pre-rsync, then stop + rsync again stoppable = filter_stoppable(containers) - backup_volume(versions_dir, volume_name, vol_dir) + backup_volume(versions_dir, volume_name, vol_dir, authoritative=False) change_containers_status(stoppable, "stop") - backup_volume(versions_dir, volume_name, vol_dir) + backup_volume(versions_dir, volume_name, vol_dir, authoritative=True) if not args.shutdown: change_containers_status(stoppable, "start") continue # default: rsync, and if needed stop + rsync - backup_volume(versions_dir, volume_name, vol_dir) + backup_volume(versions_dir, volume_name, vol_dir, authoritative=False) if requires_stop(containers, args.images_no_stop_required): stoppable = filter_stoppable(containers) change_containers_status(stoppable, "stop") - backup_volume(versions_dir, volume_name, vol_dir) + backup_volume(versions_dir, volume_name, vol_dir, authoritative=True) if not args.shutdown: change_containers_status(stoppable, "start") diff --git a/src/baudolo/backup/volume.py b/src/baudolo/backup/volume.py index 12fa8ea..ae2fc8a 100644 --- a/src/baudolo/backup/volume.py +++ b/src/baudolo/backup/volume.py @@ -24,16 +24,26 @@ def get_last_backup_dir( return None -def backup_volume(versions_dir: str, volume_name: str, volume_dir: str) -> None: - """Perform incremental file backup of a Docker volume.""" +def backup_volume( + versions_dir: str, volume_name: str, volume_dir: str, *, authoritative: bool +) -> None: + """Perform incremental file backup of a Docker volume. + + Args: + authoritative: compare source and destination by content instead of by + size and whole-second mtime. Required on a pass whose destination was + already written from a live source, where a file can differ while + both attributes still agree. + """ dest = os.path.join(volume_dir, "files") + "/" pathlib.Path(dest).mkdir(parents=True, exist_ok=True) last = get_last_backup_dir(versions_dir, volume_name, dest) link_dest = f"--link-dest='{last}'" if last else "" source = get_storage_path(volume_name) + verify = "--checksum " if authoritative else "" - cmd = f"rsync -abP --delete --delete-excluded {link_dest} {source} {dest}" + cmd = f"rsync -abP --delete --delete-excluded {verify}{link_dest} {source} {dest}" try: execute_shell_command(cmd)