Do not prune after a backup that found a configured path empty (closes #388)
check / check (push) Waiting to run
check / check (push) Waiting to run
snapshot create --prune of a snapshot whose path was an empty directory, as the mount point of a volume that is not mounted is, recorded an empty snapshot and then purged the older snapshots that held the data, with their blobs. The scan now counts the entries it finds in each configured path. Under --prune, a run in which a path of a snapshot it made held nothing skips the prune, keeps every older snapshot, warns naming the path and snapshot, and exits non-zero, as #382 does for unreadable files. Only the current run is looked at. Without --prune nothing changes. The README's --prune text and the flag's help say so. Model: opus-5-5
This commit is contained in:
@@ -325,7 +325,13 @@ value where a list belongs or a word where a number belongs), is refused.
|
||||
`--skip-errors` skipped a file or directory it could not read or list, the
|
||||
prune does not run and every older snapshot is kept, since one of them may
|
||||
be the only one that holds what was skipped; the command says so and exits
|
||||
non-zero. A file removed during the backup does not stop the prune.
|
||||
non-zero. A file removed during the backup does not stop the prune. The
|
||||
same holds when the backup found nothing in a configured path, as in the
|
||||
mount point of a volume that is not mounted: the prune does not run, every
|
||||
older snapshot is kept, and the command names the path and exits non-zero,
|
||||
on every run until the path holds something again. Take a path that is
|
||||
empty on purpose out of the config, or remove the older snapshots by hand
|
||||
with `snapshot purge`.
|
||||
* `--keep-newer-than <duration>`: With `--prune`, keep snapshots newer than
|
||||
this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`).
|
||||
Without `--prune`, or with a value that is not a valid duration, the
|
||||
|
||||
@@ -24,6 +24,12 @@ owner's; a `v*` tag runs the release build in
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-14: Stopped `snapshot create --prune` pruning after a backup
|
||||
that found nothing in a configured path
|
||||
([issue #388](https://git.eeqj.de/sneak/vaultik/issues/388)), as in
|
||||
the mount point of a volume that is not mounted. Such a prune could
|
||||
delete every snapshot still holding that path's files.
|
||||
|
||||
- 2026-10-14: Stopped `snapshot create --prune` pruning after a backup
|
||||
that skipped files it could not read
|
||||
([issue #382](https://git.eeqj.de/sneak/vaultik/issues/382)). Such a
|
||||
|
||||
@@ -115,8 +115,9 @@ on Linux), then /etc/vaultik/config.yml.`,
|
||||
return
|
||||
}
|
||||
|
||||
if errors.Is(err, vaultik.ErrFilesUnreadable) {
|
||||
log.Error("Backup completed with unreadable files", "error", err)
|
||||
if errors.Is(err, vaultik.ErrFilesUnreadable) ||
|
||||
errors.Is(err, vaultik.ErrSourcePathEmpty) {
|
||||
log.Error("Backup completed with an error", "error", err)
|
||||
ReportErrorf("Backup completed, but %v", err)
|
||||
|
||||
return
|
||||
@@ -133,7 +134,8 @@ on Linux), then /etc/vaultik/config.yml.`,
|
||||
cmd.Flags().BoolVar(&opts.Prune, "prune", false,
|
||||
"After backup, drop older snapshots of the same name and remove "+
|
||||
"orphaned blobs older than prune_grace_period (not after a "+
|
||||
"backup that skipped unreadable files)")
|
||||
"backup that skipped unreadable files or found a configured "+
|
||||
"path empty)")
|
||||
cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "",
|
||||
"With --prune: keep snapshots newer than this duration "+
|
||||
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+
|
||||
|
||||
@@ -168,6 +168,12 @@ type ScanResult struct {
|
||||
// skipped because it could not be read or listed; the rest are files
|
||||
// removed after the walk listed them.
|
||||
FilesUnreadable int
|
||||
|
||||
// EntriesFound counts the entries the walk found that no exclude
|
||||
// pattern matches, leaving out the scanned path itself when it is a
|
||||
// directory. It is zero for an empty directory, such as the mount
|
||||
// point of a volume that is not mounted.
|
||||
EntriesFound int
|
||||
}
|
||||
|
||||
// NewScanner creates a new scanner instance
|
||||
@@ -928,6 +934,11 @@ func (s *Scanner) scanPhase(
|
||||
default:
|
||||
}
|
||||
|
||||
// The scanned directory itself is not an entry found in it.
|
||||
if filePath != path || !info.IsDir() {
|
||||
result.EntriesFound++
|
||||
}
|
||||
|
||||
// Handle symlinks and directories
|
||||
handled, err := s.recordSpecialEntry(
|
||||
filePath, info, existingFiles, knownFiles, collector, result)
|
||||
@@ -940,18 +951,8 @@ func (s *Scanner) scanPhase(
|
||||
return nil
|
||||
}
|
||||
|
||||
// Track this file as existing (for deletion detection)
|
||||
existingFiles[filePath] = struct{}{}
|
||||
|
||||
// Check file against in-memory map (no DB query!)
|
||||
file := s.buildRegularFileEntry(filePath, info)
|
||||
needsProcessing := s.checkFileInMemory(file, knownFiles)
|
||||
|
||||
filesScanned, changedCount := collector.recordFile(
|
||||
filePath, info, file, needsProcessing)
|
||||
|
||||
// Update result stats
|
||||
s.updateScanEntryStats(result, needsProcessing, info)
|
||||
filesScanned, changedCount := s.recordRegularFile(
|
||||
filePath, info, existingFiles, knownFiles, collector, result)
|
||||
|
||||
// Output periodic status
|
||||
if time.Since(lastStatusTime) >= scanStatusInterval {
|
||||
@@ -1027,6 +1028,30 @@ func (s *Scanner) recordSpecialEntry(
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// recordRegularFile records a regular file as recordSpecialEntry does a
|
||||
// symlink and returns the running scanned and changed counts.
|
||||
func (s *Scanner) recordRegularFile(
|
||||
filePath string, info os.FileInfo,
|
||||
existingFiles map[string]struct{},
|
||||
knownFiles map[string]*database.File,
|
||||
collector *scanCollector, result *ScanResult,
|
||||
) (int64, int) {
|
||||
// Track this file as existing (for deletion detection)
|
||||
existingFiles[filePath] = struct{}{}
|
||||
|
||||
// Check file against in-memory map (no DB query!)
|
||||
file := s.buildRegularFileEntry(filePath, info)
|
||||
needsProcessing := s.checkFileInMemory(file, knownFiles)
|
||||
|
||||
filesScanned, changedCount := collector.recordFile(
|
||||
filePath, info, file, needsProcessing)
|
||||
|
||||
// Update result stats
|
||||
s.updateScanEntryStats(result, needsProcessing, info)
|
||||
|
||||
return filesScanned, changedCount
|
||||
}
|
||||
|
||||
// countRemovedDuringWalk counts an entry that was in its directory's
|
||||
// listing but was gone when the walk looked it up, like a file removed
|
||||
// before phase 2 reads it: as examined and failed, and not as an error,
|
||||
|
||||
@@ -44,6 +44,13 @@ var ErrPostBackupPruneFailed = errors.New("post-backup prune failed")
|
||||
// kept, so the caller can report it apart from a failed backup.
|
||||
var ErrFilesUnreadable = errors.New("file(s) could not be read")
|
||||
|
||||
// ErrSourcePathEmpty is returned when --prune skipped the post-backup
|
||||
// prune because the scan found nothing in a configured path of a
|
||||
// snapshot, as in the mount point of a volume that is not mounted. The
|
||||
// snapshots are complete and kept, so the caller can report it apart
|
||||
// from a failed backup.
|
||||
var ErrSourcePathEmpty = errors.New("source path(s) were empty")
|
||||
|
||||
// listRecentLimit caps how many snapshot rows are fetched from the
|
||||
// local index for listing and purge operations.
|
||||
const listRecentLimit = 10000
|
||||
@@ -134,17 +141,10 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// Process each named snapshot
|
||||
filesUnreadable := 0
|
||||
|
||||
for snapIdx, snapName := range snapshotNames {
|
||||
unreadable, err := v.createNamedSnapshot(
|
||||
opts, hostname, snapName, snapIdx+1, len(snapshotNames))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
filesUnreadable += unreadable
|
||||
filesUnreadable, emptyPaths, err := v.createNamedSnapshots(
|
||||
opts, hostname, snapshotNames)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Print overall summary if multiple snapshots
|
||||
@@ -161,6 +161,16 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
|
||||
"files_unreadable", filesUnreadable)
|
||||
v.UI.Warningf("Skipped the post-backup prune because %d file(s) "+
|
||||
"could not be read; older snapshots are kept.", filesUnreadable)
|
||||
case opts.Prune && len(emptyPaths) > 0:
|
||||
// An older snapshot may be the only one that still holds what an
|
||||
// empty path held. The previous snapshot is no guide: after one
|
||||
// run like this it is empty too.
|
||||
log.Warn("Skipping post-backup prune", "empty_paths", emptyPaths)
|
||||
v.UI.Warningf("Skipped the post-backup prune because nothing was "+
|
||||
"found in %s; older snapshots are kept.",
|
||||
strings.Join(emptyPaths, ", "))
|
||||
|
||||
return fmt.Errorf("%d %w", len(emptyPaths), ErrSourcePathEmpty)
|
||||
case opts.Prune:
|
||||
err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan)
|
||||
if err != nil {
|
||||
@@ -171,6 +181,34 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
|
||||
return v.finishCreateSnapshot(filesUnreadable)
|
||||
}
|
||||
|
||||
// createNamedSnapshots creates each named snapshot in turn. It returns
|
||||
// the number of entries --skip-errors skipped because it could not read
|
||||
// or list them, and each configured path in which the scan found
|
||||
// nothing, named with its snapshot.
|
||||
func (v *Vaultik) createNamedSnapshots(
|
||||
opts *SnapshotCreateOptions, hostname string, snapshotNames []string,
|
||||
) (int, []string, error) {
|
||||
filesUnreadable := 0
|
||||
|
||||
var emptyPaths []string
|
||||
|
||||
for snapIdx, snapName := range snapshotNames {
|
||||
stats, err := v.createNamedSnapshot(
|
||||
opts, hostname, snapName, snapIdx+1, len(snapshotNames))
|
||||
if err != nil {
|
||||
return 0, nil, err
|
||||
}
|
||||
|
||||
filesUnreadable += stats.totalFilesUnreadable
|
||||
|
||||
for _, dir := range stats.emptyPaths {
|
||||
emptyPaths = append(emptyPaths, dir+" of snapshot "+snapName)
|
||||
}
|
||||
}
|
||||
|
||||
return filesUnreadable, emptyPaths, nil
|
||||
}
|
||||
|
||||
// finishCreateSnapshot ends a run whose snapshots are all complete and
|
||||
// kept. Like finishRestore, it returns an error when --skip-errors
|
||||
// skipped anything it could not read or list; otherwise it prints the
|
||||
@@ -274,6 +312,9 @@ type snapshotStats struct {
|
||||
// could not be read or listed.
|
||||
totalFilesUnreadable int
|
||||
|
||||
// The configured paths in which the scan found nothing.
|
||||
emptyPaths []string
|
||||
|
||||
// The sizes of all blobs the snapshot references, set by
|
||||
// finalizeSnapshotMetadata once snapshot_blobs is populated.
|
||||
blobSize int64
|
||||
@@ -281,11 +322,10 @@ type snapshotStats struct {
|
||||
}
|
||||
|
||||
// createNamedSnapshot creates a single named snapshot and returns the
|
||||
// number of entries --skip-errors skipped because it could not read or
|
||||
// list them.
|
||||
// statistics of its scan.
|
||||
func (v *Vaultik) createNamedSnapshot(
|
||||
opts *SnapshotCreateOptions, hostname, snapName string, idx, total int,
|
||||
) (int, error) {
|
||||
) (*snapshotStats, error) {
|
||||
snapshotStartTime := time.Now()
|
||||
|
||||
if total > 1 {
|
||||
@@ -294,12 +334,12 @@ func (v *Vaultik) createNamedSnapshot(
|
||||
|
||||
resolvedDirs, err := v.resolveSnapshotPaths(snapName)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
return nil, err
|
||||
}
|
||||
|
||||
previousSnapshotID, err := v.previousSnapshotID(snapName)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
return nil, err
|
||||
}
|
||||
|
||||
scanner := v.ScannerFactory(snapshot.ScannerParams{
|
||||
@@ -314,7 +354,7 @@ func (v *Vaultik) createNamedSnapshot(
|
||||
snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
|
||||
v.ctx, hostname, snapName, v.Globals.Version, v.Globals.Commit)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("creating snapshot: %w", err)
|
||||
return nil, fmt.Errorf("creating snapshot: %w", err)
|
||||
}
|
||||
|
||||
log.Info("Beginning snapshot", "snapshot_id", snapshotID, "name", snapName)
|
||||
@@ -322,12 +362,12 @@ func (v *Vaultik) createNamedSnapshot(
|
||||
|
||||
stats, err := v.scanAllDirectories(scanner, resolvedDirs, snapshotID)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
return nil, err
|
||||
}
|
||||
|
||||
err = v.finalizeSnapshotMetadata(snapshotID, stats)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
return nil, err
|
||||
}
|
||||
|
||||
log.Info("Snapshot complete",
|
||||
@@ -340,7 +380,7 @@ func (v *Vaultik) createNamedSnapshot(
|
||||
|
||||
v.printSnapshotSummary(snapshotID, snapshotStartTime, stats)
|
||||
|
||||
return stats.totalFilesUnreadable, nil
|
||||
return stats, nil
|
||||
}
|
||||
|
||||
// previousSnapshotID returns the ID of the most recent completed snapshot
|
||||
@@ -434,6 +474,10 @@ func (v *Vaultik) scanAllDirectories(
|
||||
stats.totalBytesUploaded += result.BytesUploaded
|
||||
stats.uploadDuration += result.UploadDuration
|
||||
|
||||
if result.EntriesFound == 0 {
|
||||
stats.emptyPaths = append(stats.emptyPaths, dir)
|
||||
}
|
||||
|
||||
log.Info("Directory scan complete",
|
||||
"path", dir,
|
||||
"files", result.FilesScanned,
|
||||
|
||||
@@ -1,8 +1,15 @@
|
||||
package vaultik_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/vaultik/internal/config"
|
||||
"sneak.berlin/go/vaultik/internal/log"
|
||||
"sneak.berlin/go/vaultik/internal/vaultik"
|
||||
)
|
||||
|
||||
@@ -25,3 +32,137 @@ func TestSnapshotCreateOptions_PruneFlag(t *testing.T) {
|
||||
t.Error("Expected Prune to be false")
|
||||
}
|
||||
}
|
||||
|
||||
// A backup under --prune that finds nothing in a configured path, as in
|
||||
// the mount point of a volume that is not mounted, does not prune: run
|
||||
// after run, the snapshot that holds the files is kept with everything
|
||||
// it stored, and the run fails. That holds whether the empty path is the
|
||||
// snapshot's only path or one of two. See
|
||||
// https://git.eeqj.de/sneak/vaultik/issues/388.
|
||||
func TestBackupWithEmptySourcePathDoesNotPrune(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
t.Parallel()
|
||||
|
||||
// emptied is the path, relative to the source directory, that is
|
||||
// emptied after the first backup.
|
||||
const (
|
||||
snapshotName = "media"
|
||||
emptied = "music"
|
||||
)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
paths []string // relative to the source directory
|
||||
}{
|
||||
{name: "only path", paths: []string{emptied}},
|
||||
{name: "one of two paths", paths: []string{emptied, "docs"}},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := context.Background()
|
||||
env := newDeletedCountEnv(t, map[string][]byte{
|
||||
emptied + "/a.mp3": []byte("song a"),
|
||||
emptied + "/b.mp3": []byte("song b"),
|
||||
"docs/notes.txt": []byte("notes"),
|
||||
})
|
||||
|
||||
paths := make([]string, 0, len(tt.paths))
|
||||
|
||||
for _, rel := range tt.paths {
|
||||
paths = append(paths, filepath.Join(env.src, rel))
|
||||
}
|
||||
|
||||
env.v.Config.Snapshots[snapshotName] = config.SnapshotConfig{
|
||||
Paths: paths,
|
||||
}
|
||||
env.backUp(t, snapshotName)
|
||||
|
||||
firstID := localSnapshotID(ctx, t, env.v.Repositories, snapshotName)
|
||||
stored, err := env.v.Storage.List(ctx, "")
|
||||
require.NoError(t, err)
|
||||
|
||||
// A volume that is not mounted leaves its mount point behind
|
||||
// as an empty directory.
|
||||
emptiedDir := filepath.Join(env.src, emptied)
|
||||
require.NoError(t, os.RemoveAll(emptiedDir))
|
||||
require.NoError(t, os.Mkdir(emptiedDir, 0o750))
|
||||
|
||||
for range 2 {
|
||||
env.out.Reset()
|
||||
|
||||
err := env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
|
||||
Prune: true,
|
||||
Snapshots: []string{snapshotName},
|
||||
})
|
||||
require.ErrorIs(t, err, vaultik.ErrSourcePathEmpty)
|
||||
assert.Contains(t, env.out.String(),
|
||||
"Skipped the post-backup prune because nothing was found in ")
|
||||
assert.Contains(t, env.out.String(),
|
||||
emptied+" of snapshot "+snapshotName+";")
|
||||
assert.NotContains(t, env.out.String(), "Running post-backup prune.")
|
||||
}
|
||||
|
||||
snapshots, err := env.v.Repositories.Snapshots.ListRecent(
|
||||
ctx, listRecentTestLimit)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, snapshots, 3)
|
||||
assert.Equal(t, firstID, snapshots[2].ID.String(),
|
||||
"the snapshot that holds the files is kept")
|
||||
|
||||
for _, key := range stored {
|
||||
_, err := env.v.Storage.Stat(ctx, key)
|
||||
require.NoError(t, err, "%s is kept", key)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A backup under --prune that finds something in each configured path
|
||||
// prunes the previous snapshot, also when a path is a single file. See
|
||||
// https://git.eeqj.de/sneak/vaultik/issues/388.
|
||||
func TestBackupWithNoEmptySourcePathPrunes(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
snapshotName = "album"
|
||||
file = "pictures/one.jpg" // relative to the source directory
|
||||
)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
path string // relative to the source directory
|
||||
}{
|
||||
{name: "directory", path: filepath.Dir(file)},
|
||||
{name: "single file", path: file},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := context.Background()
|
||||
env := newDeletedCountEnv(t, map[string][]byte{
|
||||
file: []byte("picture"),
|
||||
})
|
||||
env.setSnapshot(snapshotName, tt.path)
|
||||
env.backUp(t, snapshotName)
|
||||
|
||||
firstID := localSnapshotID(ctx, t, env.v.Repositories, snapshotName)
|
||||
|
||||
require.NoError(t, env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
|
||||
Prune: true,
|
||||
Snapshots: []string{snapshotName},
|
||||
}))
|
||||
|
||||
snapshots, err := env.v.Repositories.Snapshots.ListRecent(
|
||||
ctx, listRecentTestLimit)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, snapshots, 1, "the previous snapshot is pruned")
|
||||
assert.NotEqual(t, firstID, snapshots[0].ID.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user