Backups: an archive is written under its final name, so one cut short looks like a backup and costs a good one at retention #1123

New issue

Closed

opened 2026-10-31 03:04:11 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-31 03:04:11 +00:00

Owner

What exists today

src/postulo/core/backup.py:501 resolves the target to its final name (resolve_target, lines 450-457: postulo-backup-YYYYMMDD-HHMMSS.tar.gz) and line 548 opens it directly with tarfile.open(path, "w:gz"). Verification runs afterwards on the same path (line 571, verify_backup(path)). If anything raises in between, or the process is killed, or verification fails, the file stays under that name; nothing unlinks it (no try/except around the write, and core/backups.py:247-255 take() only releases the lock).

That file then matches core/backups.py:50 NAME, so archives() (lines 160-173) lists it. _describe (lines 129-157) reads only the manifest, which is the archive's first member (backup.py:549-552), so a truncated archive whose manifest was written is shown on the Backups page as an ordinary backup with its counts; it is marked only once somebody runs a check. apply_retention (lines 370-398) keeps the newest keep archives that are not uploads, so a broken archive counts as one of them and, on the next good run, one more good archive than intended is deleted. With keep at 1 or 2 that can leave the instance with a single good backup or none beside a broken one.

manage.py backup <path> uses the same write_backup and has the same behaviour for a path the operator names.

What would change

In write_backup, write to a temporary name in the same directory that does not match NAME (for example tempfile.mkstemp(dir=path.parent, prefix=".postulo-backup-", suffix=".partial")), run verify_backup on it, and only then os.replace it to the final name; on any exception, unlink the temporary file and re-raise. os.replace within one directory is atomic, so a listed archive is always a complete, verified one. The same pattern is already used for the heartbeat (core/scheduler.py:60-66).

Leftovers from a hard kill (SIGKILL, power loss) are dot-files that NAME ignores; the scheduler's tidy can remove .postulo-backup-*.partial older than LOCK_SECONDS (6 hours, backups.py:54), since no backup runs that long.

Tests: a write_backup that raises half way (patch _add_tree to raise) leaves no file matching NAME and no temporary file; a successful one leaves exactly the final file; retention with a stale .partial beside it counts only real archives.

What it saves or protects

medium: protects the kept backups: a run cut short (disk full, container killed, OOM) no longer leaves a file that the Backups page lists as normal and that retention counts as one of the newest N

Accessibility

none

Decisions

Remove stale partial files automatically? Recommended: yes, past six hours, logged as a count.

Found by the efficiency and reliability audit of 2026-10-30 (finding D3; index postulo/postulo#1108). Checked against main at daee30b15 (2026-10-30).

## What exists today `src/postulo/core/backup.py:501` resolves the target to its final name (`resolve_target`, lines 450-457: `postulo-backup-YYYYMMDD-HHMMSS.tar.gz`) and line 548 opens it directly with `tarfile.open(path, "w:gz")`. Verification runs afterwards on the same path (line 571, `verify_backup(path)`). If anything raises in between, or the process is killed, or verification fails, the file stays under that name; nothing unlinks it (no `try`/`except` around the write, and `core/backups.py:247-255` `take()` only releases the lock). That file then matches `core/backups.py:50` `NAME`, so `archives()` (lines 160-173) lists it. `_describe` (lines 129-157) reads only the manifest, which is the archive's first member (`backup.py:549-552`), so a truncated archive whose manifest was written is shown on the Backups page as an ordinary backup with its counts; it is marked only once somebody runs a check. `apply_retention` (lines 370-398) keeps the newest `keep` archives that are not uploads, so a broken archive counts as one of them and, on the next good run, one more good archive than intended is deleted. With `keep` at 1 or 2 that can leave the instance with a single good backup or none beside a broken one. `manage.py backup <path>` uses the same `write_backup` and has the same behaviour for a path the operator names. ## What would change In `write_backup`, write to a temporary name in the same directory that does not match `NAME` (for example `tempfile.mkstemp(dir=path.parent, prefix=".postulo-backup-", suffix=".partial")`), run `verify_backup` on it, and only then `os.replace` it to the final name; on any exception, unlink the temporary file and re-raise. `os.replace` within one directory is atomic, so a listed archive is always a complete, verified one. The same pattern is already used for the heartbeat (`core/scheduler.py:60-66`). Leftovers from a hard kill (SIGKILL, power loss) are dot-files that `NAME` ignores; the scheduler's tidy can remove `.postulo-backup-*.partial` older than `LOCK_SECONDS` (6 hours, `backups.py:54`), since no backup runs that long. Tests: a `write_backup` that raises half way (patch `_add_tree` to raise) leaves no file matching `NAME` and no temporary file; a successful one leaves exactly the final file; retention with a stale `.partial` beside it counts only real archives. ## What it saves or protects medium: protects the kept backups: a run cut short (disk full, container killed, OOM) no longer leaves a file that the Backups page lists as normal and that retention counts as one of the newest N ## Accessibility none ## Decisions - **Remove stale partial files automatically?** Recommended: yes, past six hours, logged as a count. --- Found by the efficiency and reliability audit of 2026-10-30 (finding D3; index postulo/postulo#1108). Checked against `main` at `daee30b15` (2026-10-30).

tiagoagueda added this to the 0.6.0 milestone

2026-10-31 03:04:11 +00:00

tiagoagueda added the

performance

bug

labels

2026-10-31 03:04:11 +00:00

tiagoagueda referenced this issue

2026-10-31 03:04:40 +00:00

Efficiency and reliability audit of 2026-10-30: index #1108

tiagoagueda referenced this issue from a commit

2026-11-10 13:37:55 +00:00