Collect orphaned snapshots by name: the sidecar lives in tmpfs
A reboot mid-backup orphaned the entire tree, permanently. The sidecar is the record of which snapshots a run pinned -- and /run is tmpfs. A reboot or crash between the recursive snapshot and its cleanup destroyed that record, leaving one snapshot per descendant dataset (250+ on a real pool) with nothing pointing at them. Nothing would ever have found them. gc_stale_snapshots() identifies leftovers by NAME, so it works when the record is gone. It runs after the sidecar reclaim -- the recorded path stays authoritative and the collector only mops up what the record lost. It deletes data on a name match, which is a weaker claim than a recorded fact, so the selection is a pure function with the harshest tests here. A snapshot is collected only if the name is exactly <dataset>@<task>-<YYYYMMDDHHMMSS>, it is not the current run's, NOTHING IS MOUNTED FROM IT (this, not the age guard, is what protects a concurrent backup), and it is over an hour old. Checked against the real pool: of 4728 snapshots including 2341 periodic ones, it selects exactly the orphans of the task being run and nothing else.
This commit is contained in:
+29
-1
@@ -207,7 +207,35 @@ worse than no alert, because one day it carries a security fix.
|
||||
warning, not a refusal: declining to install over a string we failed to read would
|
||||
be a worse failure than the one being prevented.
|
||||
|
||||
- **A stable release may not leave work stranded under `## Unreleased`.** Either it
|
||||
- **A stable release may not leave work stranded under `## Unreleased
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A reboot mid-backup orphaned the entire snapshot tree, permanently.** The sidecar
|
||||
is the record of which snapshots a run pinned — and it lives in `/run`, which is
|
||||
**tmpfs**. A reboot (or a crash) between taking the recursive snapshot and cleaning
|
||||
it up destroyed that record, leaving one snapshot per descendant dataset — **250+ on
|
||||
a real pool** — with nothing left pointing at them. Nothing would ever have found
|
||||
them again.
|
||||
|
||||
`gc_stale_snapshots()` is the backstop: it identifies leftovers **by name**, so it
|
||||
works when the record is gone. It runs at the start of every backup, after the
|
||||
sidecar reclaim — the recorded path stays authoritative, and the collector only ever
|
||||
mops up what the record lost.
|
||||
|
||||
Because it deletes data on a *name match* — a weaker claim than a recorded fact — the
|
||||
selection is a **pure function** with the harshest tests in the suite. A snapshot is
|
||||
collected only if **all** of these hold:
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| name is exactly `<dataset>@<task>-<YYYYMMDDHHMMSS>` | so `cloud_backup-5` never matches `cloud_backup-50`, an `auto-*` periodic snapshot, or anything a human made |
|
||||
| it is not the current run's | parent *and* children are excluded |
|
||||
| **nothing is mounted from it** | an in-flight run pins its own snapshots — this, not the age guard, is what protects a concurrent backup |
|
||||
| it is **over an hour old** | covers the seconds-long window where a live run has snapshotted but not yet mounted |
|
||||
|
||||
Verified against the real pool: of **4,728** snapshots — including **2,341** periodic
|
||||
ones — it selects exactly the orphans of the task being run, and nothing else.`.** Either it
|
||||
is finished and belongs in the release, or the release is premature. Candidates
|
||||
are exempt: an rc may legitimately have work queued behind it.
|
||||
|
||||
|
||||
@@ -95,6 +95,21 @@ re-scan each time.
|
||||
|
||||
### Snapshot lifecycle
|
||||
|
||||
> **Two mechanisms clean up, and the second exists because the first can be destroyed.**
|
||||
>
|
||||
> 1. **The sidecar** records exactly which snapshots a run pinned, and is removed only
|
||||
> on a confirmed-clean sweep. Precise, and it survives a middlewared restart.
|
||||
> 2. **The garbage collector** finds leftovers by *name*, so it still works when the
|
||||
> sidecar is gone — and it can be: **the sidecar lives in `/run`, which is tmpfs.** A
|
||||
> reboot mid-backup takes it, and with it the only record of a 250-snapshot tree.
|
||||
>
|
||||
> The collector runs at the start of every backup, after the sidecar reclaim. It will
|
||||
> only touch a snapshot named `<dataset>@<task>-<timestamp>` that is not the current
|
||||
> run's, has **nothing mounted from it** (which is what protects a concurrently-running
|
||||
> backup), and is **over an hour old**. Periodic `auto-*` snapshots, other tasks'
|
||||
> snapshots, and anything you made by hand are structurally out of reach.
|
||||
|
||||
|
||||
> **A snapshot may survive a run, and that is expected.** ZFS **automounts**
|
||||
> `<dataset>/.zfs/snapshot/<snap>` the moment it is read, and holds it for
|
||||
> `zfs_expire_snapshot` seconds (**300** by default) after the last access. So
|
||||
|
||||
@@ -55,6 +55,7 @@ Therefore this module owns the whole lifecycle:
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import datetime
|
||||
import os
|
||||
import stat
|
||||
import subprocess
|
||||
@@ -68,10 +69,13 @@ __all__ = [
|
||||
"cleanup_task",
|
||||
"current_mounts_under",
|
||||
"delete_snapshot_tree",
|
||||
"gc_stale_snapshots",
|
||||
"mounted_snapshots",
|
||||
"plan_staging",
|
||||
"sidecar_for",
|
||||
"snapshot_tree_names",
|
||||
"stage_nested",
|
||||
"stale_snapshot_names",
|
||||
"staging_root_for",
|
||||
"teardown",
|
||||
"verify_staged",
|
||||
@@ -176,6 +180,76 @@ def snapshot_tree_names(snapshot: str, all_names) -> list[str]:
|
||||
]
|
||||
|
||||
|
||||
#: A snapshot must be at least this old before the garbage collector will touch it.
|
||||
#:
|
||||
#: The GC identifies our leftovers by NAME, so its only real risk is deleting a
|
||||
#: snapshot belonging to a run that is still starting up -- the window between
|
||||
#: `zfs snapshot -r` and the bind mounts appearing, which is seconds. An hour is three
|
||||
#: orders of magnitude more slack than that window needs, and still reclaims a lost
|
||||
#: tree on the very next daily run.
|
||||
GC_MIN_AGE_SECONDS = 3600
|
||||
|
||||
|
||||
def stale_snapshot_names(task_name, current_snapshot, all_names, now,
|
||||
in_use=(), min_age=GC_MIN_AGE_SECONDS):
|
||||
"""Snapshots THIS task created in an earlier run and never cleaned up.
|
||||
|
||||
Pure, because this is the one function here that DELETES DATA on a name match, and
|
||||
a name match is a weaker claim than a recorded fact. Everything it relies on is an
|
||||
argument, so every way it could be wrong is a test.
|
||||
|
||||
Why a garbage collector exists at all, when there is already a sidecar: **the
|
||||
sidecar lives in /run, which is tmpfs.** A reboot mid-backup destroys it, and with
|
||||
it the only record of a 250-snapshot tree. The sidecar handles the normal case
|
||||
precisely; this handles the case where the record itself is gone.
|
||||
|
||||
A snapshot is ours to collect only if ALL of these hold:
|
||||
|
||||
* its name is exactly ``<dataset>@<task_name>-<YYYYMMDDHHMMSS>`` -- so
|
||||
``cloud_backup-5`` never matches ``cloud_backup-50``'s snapshots, and never
|
||||
matches a periodic ``auto-2026-…`` or anything a human made;
|
||||
* it is not the snapshot the current run is using;
|
||||
* nothing is mounted from it (`in_use`) -- an in-flight run pins its own
|
||||
snapshots, so this alone protects a concurrent one-time backup;
|
||||
* it is older than `min_age` -- which covers the seconds-long window in which a
|
||||
run has taken its snapshot but not yet mounted it.
|
||||
|
||||
`now` is a timezone-aware datetime; timestamps in the name are UTC (stock builds
|
||||
them with `utc_now()`).
|
||||
"""
|
||||
prefix = task_name + "-"
|
||||
stale = []
|
||||
|
||||
for name in all_names:
|
||||
_dataset, _, snapname = name.partition("@")
|
||||
if not snapname or not snapname.startswith(prefix):
|
||||
continue
|
||||
if name == current_snapshot or snapname == _snapname_of(current_snapshot):
|
||||
continue
|
||||
if name in in_use:
|
||||
continue
|
||||
|
||||
stamp = snapname[len(prefix):]
|
||||
try:
|
||||
when = datetime.datetime.strptime(stamp, "%Y%m%d%H%M%S").replace(
|
||||
tzinfo=datetime.UTC
|
||||
)
|
||||
except ValueError:
|
||||
# Not our timestamp format. Something else owns this name; leave it alone.
|
||||
continue
|
||||
|
||||
if (now - when).total_seconds() < min_age:
|
||||
continue
|
||||
|
||||
stale.append(name)
|
||||
|
||||
return stale
|
||||
|
||||
|
||||
def _snapname_of(snapshot):
|
||||
return snapshot.partition("@")[2] if snapshot else ""
|
||||
|
||||
|
||||
def _probe_snapdir(path):
|
||||
"""Classify a snapshot directory: ``ok``, ``missing``, or why it is unusable.
|
||||
|
||||
@@ -598,6 +672,80 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
return remaining
|
||||
|
||||
|
||||
def mounted_snapshots(mounts_file="/proc/self/mounts"):
|
||||
"""Every ZFS snapshot something is currently mounted from.
|
||||
|
||||
The device field of a snapshot mount IS the snapshot name (`Tap/apps/x@snap`), for
|
||||
both our staging bind mounts and ZFS's own .zfs automounts. So this is a direct,
|
||||
factual answer to "is anything using this snapshot right now" -- which is what
|
||||
protects a concurrently-running backup from the garbage collector, rather than
|
||||
trusting an age heuristic to be generous enough.
|
||||
"""
|
||||
live = set()
|
||||
try:
|
||||
with open(mounts_file, encoding="utf-8") as fh:
|
||||
for line in fh:
|
||||
dev = line.split(" ", 1)[0]
|
||||
if "@" in dev:
|
||||
live.add(dev.replace("\\040", " "))
|
||||
except OSError:
|
||||
return set()
|
||||
return live
|
||||
|
||||
|
||||
def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
|
||||
now=None, mounts_file="/proc/self/mounts"):
|
||||
"""Delete snapshots this task left behind in an earlier run. Returns what remains.
|
||||
|
||||
The backstop for when the RECORD is gone, not just the snapshots: the sidecar lives
|
||||
in /run (tmpfs), so a reboot mid-backup takes it with them. Without this, that tree
|
||||
-- one snapshot per descendant dataset, 250+ on a real pool -- is orphaned with
|
||||
nothing left pointing at it.
|
||||
|
||||
Selection is `stale_snapshot_names()`, which is pure and heavily tested, because a
|
||||
name match is a weaker claim than a recorded fact and this deletes data on one.
|
||||
"""
|
||||
dataset = current_snapshot.partition("@")[0]
|
||||
now = now or datetime.datetime.now(datetime.UTC)
|
||||
|
||||
try:
|
||||
snaps = middleware.call_sync(
|
||||
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
|
||||
)
|
||||
except Exception as e: # noqa: BLE001 - cannot enumerate; collect nothing
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: could not enumerate snapshots for GC: %r", e
|
||||
)
|
||||
return []
|
||||
|
||||
stale = stale_snapshot_names(
|
||||
task_name, current_snapshot, [s["name"] for s in snaps], now,
|
||||
in_use=mounted_snapshots(mounts_file),
|
||||
)
|
||||
if not stale:
|
||||
return []
|
||||
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: %d snapshot(s) from an earlier run of %s were never "
|
||||
"cleaned up (a lost record, e.g. a reboot mid-backup); collecting them",
|
||||
len(stale), task_name,
|
||||
)
|
||||
|
||||
remaining = []
|
||||
for name in stale:
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", name)
|
||||
except Exception as e: # noqa: BLE001 - busy, or gone; either way, next run
|
||||
remaining.append(name)
|
||||
if logger:
|
||||
logger.debug(
|
||||
"truecloud-patch: could not collect %s: %r", name, e
|
||||
)
|
||||
return remaining
|
||||
|
||||
|
||||
def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
|
||||
task_name, datasets, logger=None):
|
||||
"""Build a complete staging tree for `path` from the already-taken `snapshot`.
|
||||
@@ -650,6 +798,18 @@ def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
|
||||
"carrying them forward to the next run", len(pending),
|
||||
)
|
||||
|
||||
# ...and collect anything from an earlier run that has NO record at all.
|
||||
#
|
||||
# The sidecar above is precise but lives in /run, which is tmpfs -- a reboot
|
||||
# mid-backup destroys it and orphans the whole tree with nothing pointing at it.
|
||||
# This finds those by name and is the only thing that ever will.
|
||||
#
|
||||
# It runs AFTER the sidecar reclaim on purpose: the recorded path is authoritative
|
||||
# and cheap, and the GC should only ever be mopping up what the record lost.
|
||||
pending.extend(
|
||||
gc_stale_snapshots(middleware, task_name, snapshot, logger=logger)
|
||||
)
|
||||
|
||||
# Record the snapshot BEFORE mounting anything, not after. middlewared can
|
||||
# die at any point (this patch even schedules a restart at boot), and the
|
||||
# sidecar is the only thing that survives it -- an in-process dict would take
|
||||
|
||||
@@ -893,3 +893,159 @@ class TestTheSidecarCarriesEveryPendingTree:
|
||||
assert len(notes) == 2
|
||||
assert "'Tap@a'" in notes[0] and "'Tap@b'" in notes[1]
|
||||
assert "[" not in "".join(notes)
|
||||
|
||||
|
||||
class TestGarbageCollectorSelection:
|
||||
"""`stale_snapshot_names` DELETES DATA on a name match.
|
||||
|
||||
A name match is a weaker claim than a recorded fact, so every way it could be wrong
|
||||
is a test. It exists because the sidecar — which IS a recorded fact — lives in /run,
|
||||
which is tmpfs: a reboot mid-backup destroys it and orphans a 250-snapshot tree with
|
||||
nothing left pointing at it. This is the only thing that would ever find those.
|
||||
"""
|
||||
|
||||
import datetime as _dt
|
||||
NOW = _dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=_dt.UTC)
|
||||
CURRENT = "Tap@cloud_backup-5-20260714115900" # 1 minute ago
|
||||
OLD = "Tap/apps/x@cloud_backup-5-20260713030000" # ~33 hours ago
|
||||
|
||||
def collect(self, names, **kw):
|
||||
import truecloud_nested as tn
|
||||
return tn.stale_snapshot_names(
|
||||
"cloud_backup-5", self.CURRENT, names, self.NOW, **kw
|
||||
)
|
||||
|
||||
def test_it_collects_our_own_leftovers(self):
|
||||
assert self.collect([self.OLD]) == [self.OLD]
|
||||
|
||||
def test_it_NEVER_touches_the_current_run(self):
|
||||
# Both the parent and its children share the current snapname.
|
||||
names = [self.CURRENT, "Tap/apps/x@cloud_backup-5-20260714115900"]
|
||||
assert self.collect(names) == []
|
||||
|
||||
def test_it_NEVER_touches_a_periodic_snapshot(self):
|
||||
assert self.collect(["Tap/apps/x@auto-2026-07-13_03-00"]) == []
|
||||
|
||||
def test_it_NEVER_touches_a_human_made_snapshot(self):
|
||||
assert self.collect(["Tap@before-i-broke-everything"]) == []
|
||||
|
||||
def test_it_NEVER_touches_another_TASK(self):
|
||||
# cloud_backup-5 must not match cloud_backup-50. This is why the prefix
|
||||
# carries the trailing dash.
|
||||
assert self.collect(["Tap/apps/x@cloud_backup-50-20260713030000"]) == []
|
||||
assert self.collect(["Tap/apps/x@cloud_backup-7-20260713030000"]) == []
|
||||
|
||||
def test_it_NEVER_touches_a_one_time_backup(self):
|
||||
assert self.collect(["Tap@cloud_backup-onetime-20260713030000"]) == []
|
||||
|
||||
def test_it_NEVER_touches_a_snapshot_that_is_MOUNTED(self):
|
||||
# An in-flight run pins its own snapshots. This — not the age heuristic — is
|
||||
# what actually protects a concurrent backup.
|
||||
assert self.collect([self.OLD], in_use={self.OLD}) == []
|
||||
|
||||
def test_it_NEVER_touches_a_snapshot_younger_than_the_minimum_age(self):
|
||||
# Covers the seconds-long window between `zfs snapshot -r` and the mounts
|
||||
# appearing, when a live run's snapshots look exactly like garbage.
|
||||
young = "Tap/apps/x@cloud_backup-5-20260714113000" # 30 minutes ago
|
||||
assert self.collect([young]) == []
|
||||
assert self.collect([young], min_age=60) == [young]
|
||||
|
||||
def test_a_name_it_cannot_parse_is_left_alone(self):
|
||||
assert self.collect(["Tap@cloud_backup-5-not-a-timestamp"]) == []
|
||||
assert self.collect(["Tap@cloud_backup-5-"]) == []
|
||||
|
||||
def test_a_realistic_mixed_pool(self):
|
||||
names = [
|
||||
self.CURRENT, # ours, running
|
||||
"Tap/apps/x@cloud_backup-5-20260714115900", # ours, running (child)
|
||||
self.OLD, # ours, orphaned <-
|
||||
"Tap/apps/y@cloud_backup-5-20260712030000", # ours, orphaned <-
|
||||
"Tap/apps/x@auto-2026-07-13_03-00", # periodic
|
||||
"Tap/apps/x@cloud_backup-7-20260713030000", # another task
|
||||
"Tap@manual-keepme", # human
|
||||
]
|
||||
assert sorted(self.collect(names)) == sorted(
|
||||
[self.OLD, "Tap/apps/y@cloud_backup-5-20260712030000"]
|
||||
)
|
||||
|
||||
|
||||
class TestMountedSnapshots:
|
||||
def test_it_reads_snapshot_names_out_of_the_mount_table(self, tmp_path):
|
||||
import truecloud_nested as tn
|
||||
mounts = tmp_path / "mounts"
|
||||
mounts.write_text(
|
||||
"tmpfs /run tmpfs rw 0 0\n"
|
||||
"Tap/apps/x@snap1 /run/truecloud-nested/t/apps/x zfs ro 0 0\n"
|
||||
"Tap/apps/y@snap1 /mnt/Tap/apps/y/.zfs/snapshot/snap1 zfs ro 0 0\n"
|
||||
"Tap/live /mnt/Tap/live zfs rw 0 0\n"
|
||||
)
|
||||
live = tn.mounted_snapshots(str(mounts))
|
||||
assert live == {"Tap/apps/x@snap1", "Tap/apps/y@snap1"}
|
||||
assert "Tap/live" not in live # a live dataset is not a snapshot
|
||||
|
||||
|
||||
class TestGarbageCollectorExecution:
|
||||
def test_it_deletes_the_stale_ones_and_nothing_else(self, monkeypatch, tmp_path):
|
||||
import datetime as dt
|
||||
import truecloud_nested as tn
|
||||
|
||||
mounts = tmp_path / "mounts"
|
||||
mounts.write_text("")
|
||||
now = dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC)
|
||||
|
||||
mw = FakeMiddleware([
|
||||
"Tap@cloud_backup-5-20260714115900", # current run
|
||||
"Tap/apps/x@cloud_backup-5-20260713030000", # orphan <-
|
||||
"Tap/apps/x@auto-2026-07-13_03-00", # periodic
|
||||
"Tap/apps/x@cloud_backup-7-20260713030000", # other task
|
||||
])
|
||||
remaining = tn.gc_stale_snapshots(
|
||||
mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
|
||||
now=now, mounts_file=str(mounts),
|
||||
)
|
||||
assert remaining == []
|
||||
assert mw.snapshots == [
|
||||
"Tap@cloud_backup-5-20260714115900",
|
||||
"Tap/apps/x@auto-2026-07-13_03-00",
|
||||
"Tap/apps/x@cloud_backup-7-20260713030000",
|
||||
]
|
||||
|
||||
def test_a_busy_orphan_is_reported_not_swallowed(self, monkeypatch, tmp_path):
|
||||
import datetime as dt
|
||||
import truecloud_nested as tn
|
||||
|
||||
mounts = tmp_path / "mounts"
|
||||
mounts.write_text("")
|
||||
now = dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC)
|
||||
orphan = "Tap/apps/x@cloud_backup-5-20260713030000"
|
||||
|
||||
mw = BusyMiddleware(
|
||||
["Tap@cloud_backup-5-20260714115900", orphan],
|
||||
busy=[orphan], busy_for=99,
|
||||
)
|
||||
remaining = tn.gc_stale_snapshots(
|
||||
mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
|
||||
now=now, mounts_file=str(mounts),
|
||||
)
|
||||
assert remaining == [orphan]
|
||||
|
||||
def test_it_collects_NOTHING_when_the_query_fails(self, tmp_path):
|
||||
# Cannot enumerate => cannot know what is ours => delete nothing.
|
||||
import datetime as dt
|
||||
import truecloud_nested as tn
|
||||
|
||||
mounts = tmp_path / "mounts"
|
||||
mounts.write_text("")
|
||||
|
||||
class Broken(FakeMiddleware):
|
||||
def call_sync(self, method, *args):
|
||||
if method == "zfs.snapshot.query":
|
||||
raise RuntimeError("middleware is having a day")
|
||||
return super().call_sync(method, *args)
|
||||
|
||||
assert tn.gc_stale_snapshots(
|
||||
Broken(["Tap/apps/x@cloud_backup-5-20260713030000"]),
|
||||
"cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
|
||||
now=dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC),
|
||||
mounts_file=str(mounts),
|
||||
) == []
|
||||
|
||||
Reference in New Issue
Block a user