From ad88a48c0101635d2324c4726d737b13b27f0d3f Mon Sep 17 00:00:00 2001 From: Mitchell-Johnson Date: Wed, 23 Sep 2026 01:19:19 -0700 Subject: [PATCH] space: the diff's clock runs inside sudo, so zfs is killed too Co-Authored-By: Claude Fable 5.1 --- PLAN.md | 10 ++++++---- internal/space/manager.go | 17 ++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/PLAN.md b/PLAN.md index 2e51179..b959b5d 100644 --- a/PLAN.md +++ b/PLAN.md @@ -356,10 +356,12 @@ something long in one, switch away and back, refresh — it's all still there. snapshots, the scan records the newest snapshot of each dataset at start (`bases`), and the next one runs `zfs diff -H -F ` per dataset: only the directories it names are read, their ancestors kept but looked into, everything else kept wholesale without a - stat. The diffs run side by side and one is given four minutes (its process group is - killed, sudo and zfs both), after which that dataset is walked by mtime instead — on - `tank`, `immich` changes constantly and its diff ran past twenty minutes the first time, - longer than walking it. Elsewhere (ext4, or a dataset with no snapshot) a directory whose mtime — to the + stat. The diffs run side by side and one is given three minutes — `timeout -s KILL` + inside sudo, since Debian's sudo gives the command its own session and a process-group + kill from outside never reached zfs — after which that dataset is walked by mtime + instead. On `tank` a diff is metadata-bound like the walk: 8 s to 3 min per dataset for + a handful of changes, and `immich`, which changes constantly, ran past twenty minutes + the first time. A refresh of `tank` took 4 min 19 s against 7 min for the full scan. Elsewhere (ext4, or a dataset with no snapshot) a directory whose mtime — to the nanosecond — is unchanged keeps its list and only its kept files are stat'ed again, since a file can grow without its directory noticing. A **cancelled** scan keeps what it walked, marks the folders it never got to (`partial`, "not yet read" in the tree) and **Resume** diff --git a/internal/space/manager.go b/internal/space/manager.go index e60ce7e..18993a3 100644 --- a/internal/space/manager.go +++ b/internal/space/manager.go @@ -571,16 +571,19 @@ func newestSnapshot(ctx context.Context, dataset string) string { // zfsDiff lists what changed in a dataset since a snapshot: every changed, added, removed // or renamed path, with the directory whose listing that touches. func zfsDiff(ctx context.Context, snapshot, mount string) ([]change, error) { - // a diff slower than this is not worth waiting for: the walk by mtime is bounded - ctx, cancel := context.WithTimeout(ctx, 4*time.Minute) + // A diff slower than this is not worth waiting for: the walk by mtime is bounded. The + // clock runs inside sudo — `timeout` is zfs's parent and kills it as root — because sudo + // on Debian gives the command a session of its own, out of reach of our process group; + // the context is only a backstop. + const patience = 3 * time.Minute + ctx, cancel := context.WithTimeout(ctx, patience+30*time.Second) defer cancel() - args := priv.Wrap("zfs", "diff", "-H", "-F", snapshot) + args := priv.Wrap("timeout", "-s", "KILL", strconv.Itoa(int(patience.Seconds())), "zfs", "diff", "-H", "-F", snapshot) cmd := exec.CommandContext(ctx, args[0], args[1:]...) - // on timeout the whole process group goes, not just sudo: zfs itself would otherwise run - // on holding the pipe, and Wait would sit there with it cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} cmd.Cancel = func() error { return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) } cmd.WaitDelay = 5 * time.Second + t0 := time.Now() var stderr strings.Builder cmd.Stderr = &stderr out, err := cmd.StdoutPipe() @@ -609,8 +612,8 @@ func zfsDiff(ctx context.Context, snapshot, mount string) ([]change, error) { } if err := cmd.Wait(); err != nil { msg := strings.TrimSpace(stderr.String()) - if errors.Is(ctx.Err(), context.DeadlineExceeded) { - msg = "gave up after 4 min" + if errors.Is(ctx.Err(), context.DeadlineExceeded) || time.Since(t0) >= patience { + msg = "gave up after " + patience.String() } if msg == "" { msg = err.Error()