Count a file as deleted only if the previous snapshot of its name had it #342

Open
clawbot wants to merge 1 commits from issue-304-deleted-count into next
8 changed files with 455 additions and 50 deletions
+4 -4
View File
@@ -279,8 +279,8 @@ Key methods:
- `Scan(ctx, path, snapshotID)` → Main scan operation - `Scan(ctx, path, snapshotID)` → Main scan operation
Scan phases: Scan phases:
1. **Phase 0**: Detect deleted files from previous snapshots 1. **Phase 1**: Walk directory, identify files needing processing
2. **Phase 1**: Walk directory, identify files needing processing 2. **Phase 1b**: Count as deleted the files that the previous snapshot of the same name lists, the walk did not find, and are gone from disk; a file that is excluded, or inside an excluded directory, is not counted
3. **Phase 2**: Process files (chunk → pack → upload) 3. **Phase 2**: Process files (chunk → pack → upload)
#### SnapshotManager #### SnapshotManager
@@ -320,13 +320,13 @@ CreateSnapshot(opts)
│ │ │ │
│ ├─► scanner.Scan(ctx, path, snapshotID) │ ├─► scanner.Scan(ctx, path, snapshotID)
│ │ │ │ │ │
│ │ ├─► Phase 0: detectDeletedFiles()
│ │ │
│ │ ├─► Phase 1: scanPhase() │ │ ├─► Phase 1: scanPhase()
│ │ │ Walk directory │ │ │ Walk directory
│ │ │ Check file metadata changes │ │ │ Check file metadata changes
│ │ │ Build list of files to process │ │ │ Build list of files to process
│ │ │ │ │ │
│ │ ├─► Phase 1b: detectDeletedFiles()
│ │ │
│ │ └─► Phase 2: processPhase() │ │ └─► Phase 2: processPhase()
│ │ For each file: │ │ For each file:
│ │ chunker.ChunkReaderStreaming() │ │ chunker.ChunkReaderStreaming()
+39
View File
@@ -267,6 +267,45 @@ func (r *FileRepository) ListUnderPath(
return files, rows.Err() return files, rows.Err()
} }
// ListUnderPathInSnapshot returns the files that ListUnderPath returns
// for path and that the snapshot lists, in no particular order.
func (r *FileRepository) ListUnderPathInSnapshot(
ctx context.Context, path, snapshotID string,
) ([]*File, error) {
path = strings.TrimRight(path, "/")
dirPrefix := path + "/"
query := `
SELECT f.id, f.path, f.source_path, f.mtime, f.mtime_nsec, f.size,
f.mode, f.uid, f.gid, f.link_target
FROM files f
JOIN snapshot_files sf ON sf.file_id = f.id
WHERE sf.snapshot_id = ?
AND (f.path = ? OR substr(f.path, 1, length(?)) = ?)
`
rows, err := r.db.conn.QueryContext(ctx, query,
snapshotID, path, dirPrefix, dirPrefix)
if err != nil {
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() { _ = rows.Close() }()
var files []*File
for rows.Next() {
file, err := r.scanFileRows(rows)
if err != nil {
return nil, fmt.Errorf("scanning file: %w", err)
}
files = append(files, file)
}
return files, rows.Err()
}
// ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose // ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose
// path starts with prefix and that list at least one chunk held by no // path starts with prefix and that list at least one chunk held by no
// blob whose upload has completed (uploaded_ts set). A new snapshot // blob whose upload has completed (uploaded_ts set). A new snapshot
+59
View File
@@ -212,6 +212,65 @@ func TestFileRepositoryListUnderPath(t *testing.T) {
} }
} }
// ListUnderPathInSnapshot leaves out files outside path and files the
// snapshot does not list, though another snapshot lists them.
func TestFileRepositoryListUnderPathInSnapshot(t *testing.T) {
t.Parallel()
db, cleanup := setupTestDB(t)
defer cleanup()
ctx := context.Background()
files := database.NewFileRepository(db)
snapshots := database.NewSnapshotRepository(db)
listed := map[string][]string{
"one": {"/src/a.txt", "/src2/c.txt"},
"two": {"/src/b.txt"},
}
for snapshotID, paths := range listed {
err := snapshots.Create(ctx, nil, &database.Snapshot{
ID: types.SnapshotID(snapshotID),
Hostname: "host",
StartedAt: time.Now(),
})
if err != nil {
t.Fatalf("failed to create snapshot %s: %v", snapshotID, err)
}
for _, path := range paths {
err := files.Create(ctx, nil, &database.File{
Path: types.FilePath(path),
MTime: time.Now().Truncate(time.Second),
Mode: 0644,
})
if err != nil {
t.Fatalf("failed to create %s: %v", path, err)
}
err = snapshots.AddFile(ctx, nil, snapshotID, path)
if err != nil {
t.Fatalf("failed to add %s to %s: %v", path, snapshotID, err)
}
}
}
found, err := files.ListUnderPathInSnapshot(ctx, "/src", "one")
if err != nil {
t.Fatalf("failed to list files: %v", err)
}
got := make([]string, 0, len(found))
for _, f := range found {
got = append(got, f.Path.String())
}
if !slices.Equal(got, []string{"/src/a.txt"}) {
t.Errorf("got %q, want only /src/a.txt", got)
}
}
func TestFileRepositorySymlink(t *testing.T) { func TestFileRepositorySymlink(t *testing.T) {
t.Parallel() t.Parallel()
+15 -11
View File
@@ -16,6 +16,9 @@ type ScannerParams struct {
Fs afero.Fs Fs afero.Fs
Exclude []string // Exclude patterns (combined global + snapshot-specific) Exclude []string // Exclude patterns (combined global + snapshot-specific)
SkipErrors bool // Skip file read errors (log loudly but continue) SkipErrors bool // Skip file read errors (log loudly but continue)
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
} }
// Module exports backup functionality as an fx module. // Module exports backup functionality as an fx module.
@@ -44,17 +47,18 @@ func provideScannerFactory(
} }
return NewScanner(ScannerConfig{ return NewScanner(ScannerConfig{
FS: params.Fs, FS: params.Fs,
ChunkSize: cfg.ChunkSize.Int64(), ChunkSize: cfg.ChunkSize.Int64(),
Repositories: repos, Repositories: repos,
Storage: storer, Storage: storer,
MaxBlobSize: cfg.BlobSizeLimit.Int64(), MaxBlobSize: cfg.BlobSizeLimit.Int64(),
CompressionLevel: cfg.CompressionLevel, CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients, AgeRecipients: cfg.AgeRecipients,
EnableProgress: params.EnableProgress, EnableProgress: params.EnableProgress,
UI: params.UI, UI: params.UI,
Exclude: excludes, Exclude: excludes,
SkipErrors: params.SkipErrors, SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
}) })
} }
} }
+72 -19
View File
@@ -58,6 +58,10 @@ type Scanner struct {
compressionLevel int compressionLevel int
ageRecipient string ageRecipient string
snapshotID string // Current snapshot being processed snapshotID string // Current snapshot being processed
// previousSnapshotID is the most recent completed snapshot of the
// same name, "" for none. Only a file it lists can be counted as
// deleted.
previousSnapshotID string
// currentSourcePath is the source directory being scanned, stored with // currentSourcePath is the source directory being scanned, stored with
// each file record. // each file record.
currentSourcePath string currentSourcePath string
@@ -129,6 +133,9 @@ type ScannerConfig struct {
// SkipErrors skips files that cannot be opened or read (log loudly but // SkipErrors skips files that cannot be opened or read (log loudly but
// continue); packer, database, encryption, and upload errors still abort. // continue); packer, database, encryption, and upload errors still abort.
SkipErrors bool SkipErrors bool
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
} }
// ScanResult contains the results of a scan operation. Files and bytes // ScanResult contains the results of a scan operation. Files and bytes
@@ -207,6 +214,7 @@ func NewScanner(cfg ScannerConfig) *Scanner {
compiledExclude: compiledExclude, compiledExclude: compiledExclude,
progress: progress, progress: progress,
skipErrors: cfg.SkipErrors, skipErrors: cfg.SkipErrors,
previousSnapshotID: cfg.PreviousSnapshotID,
ui: uiw, ui: uiw,
pendingChunkHashes: make(map[string]struct{}), pendingChunkHashes: make(map[string]struct{}),
} }
@@ -258,8 +266,9 @@ func (s *Scanner) Scan(
filesToProcess := scanResult.FilesToProcess filesToProcess := scanResult.FilesToProcess
// Phase 1b: Detect deleted files by comparing DB against scanned files // Phase 1b: Detect deleted files by comparing the previous snapshot
err = s.detectDeletedFilesFromMap(ctx, knownFiles, existingFiles, result) // of this name against scanned files
err = s.detectDeletedFiles(ctx, path, existingFiles, result)
if err != nil { if err != nil {
return nil, fmt.Errorf("detecting deleted files: %w", err) return nil, fmt.Errorf("detecting deleted files: %w", err)
} }
@@ -439,9 +448,8 @@ func (s *Scanner) summarizeScanPhase(
} }
// loadKnownFiles loads the known files at and beneath path from the // loadKnownFiles loads the known files at and beneath path from the
// database into a map for fast lookup. Every loaded file the scan does // database into a map for fast lookup. This avoids per-file database
// not find is counted as deleted. This avoids per-file database queries // queries during the scan phase.
// during the scan phase.
func (s *Scanner) loadKnownFiles( func (s *Scanner) loadKnownFiles(
ctx context.Context, path string, ctx context.Context, path string,
) (map[string]*database.File, error) { ) (map[string]*database.File, error) {
@@ -1954,21 +1962,29 @@ func (s *Scanner) queueFileForBatchInsert(
}) })
} }
// detectDeletedFilesFromMap finds files that existed in previous snapshots // detectDeletedFiles counts as deleted each file at or beneath path that
// but no longer exist. Uses pre-loaded maps to avoid any filesystem or // the previous snapshot of this name lists, the walk did not find, and
// database access. // is gone from disk. A file that is excluded, or inside an excluded
func (s *Scanner) detectDeletedFilesFromMap( // directory, is not counted: the walk skips it whether or not it is
// still on disk.
func (s *Scanner) detectDeletedFiles(
ctx context.Context, ctx context.Context,
knownFiles map[string]*database.File, path string,
existingFiles map[string]struct{}, existingFiles map[string]struct{},
result *ScanResult, result *ScanResult,
) error { ) error {
if len(knownFiles) == 0 { if s.previousSnapshotID == "" {
return nil return nil
} }
// Check each known file against the enumerated set (no filesystem access needed) previousFiles, err := s.repos.Files.ListUnderPathInSnapshot(
for path, file := range knownFiles { ctx, path, s.previousSnapshotID)
if err != nil {
return fmt.Errorf("listing files of snapshot %s: %w",
s.previousSnapshotID, err)
}
for _, file := range previousFiles {
// Check context cancellation periodically // Check context cancellation periodically
select { select {
case <-ctx.Done(): case <-ctx.Done():
@@ -1976,13 +1992,22 @@ func (s *Scanner) detectDeletedFilesFromMap(
default: default:
} }
// Check if the file exists in our enumerated set filePath := file.Path.String()
if _, exists := existingFiles[path]; !exists {
// File has been deleted _, exists := existingFiles[filePath]
result.FilesDeleted++ if exists || s.excludedOrInExcludedDir(filePath, path) {
result.BytesDeleted += file.Size continue
log.Debug("Detected deleted file", "path", path, "size", file.Size)
} }
// Under --skip-errors the walk also misses what it cannot reach,
// such as the files in a directory it cannot open.
if !s.goneFromDisk(filePath) {
continue
}
result.FilesDeleted++
result.BytesDeleted += file.Size
log.Debug("Detected deleted file", "path", filePath, "size", file.Size)
} }
if result.FilesDeleted > 0 { if result.FilesDeleted > 0 {
@@ -1993,6 +2018,34 @@ func (s *Scanner) detectDeletedFilesFromMap(
return nil return nil
} }
// excludedOrInExcludedDir reports whether filePath, which is at or
// beneath rootPath, or any directory between rootPath and it, matches an
// exclude pattern. A pattern such as */.local/share/Trash matches the
// directory but not the files inside it, which the walk skips too.
func (s *Scanner) excludedOrInExcludedDir(filePath, rootPath string) bool {
for p := filePath; len(p) > len(rootPath); p = filepath.Dir(p) {
if s.shouldExclude(p, rootPath) {
return true
}
}
return false
}
// goneFromDisk reports whether looking filePath up on disk finds that
// it does not exist. Like the walk, it does not follow a symlink, so a
// symlink whose target is gone is still on disk.
func (s *Scanner) goneFromDisk(filePath string) bool {
var err error
if lstater, ok := s.fs.(afero.Lstater); ok {
_, _, err = lstater.LstatIfPossible(filePath)
} else {
_, err = s.fs.Stat(filePath)
}
return errors.Is(err, os.ErrNotExist)
}
// wrapPermissionError augments permission errors with platform-specific // wrapPermissionError augments permission errors with platform-specific
// remediation instructions. On macOS, TCC-protected directories (Calendars, // remediation instructions. On macOS, TCC-protected directories (Calendars,
// Reminders, Photos, etc.) return EPERM unless the running application has // Reminders, Photos, etc.) return EPERM unless the running application has
+12 -11
View File
@@ -661,17 +661,18 @@ func faultScannerFactory(
) snapshot.ScannerFactory { ) snapshot.ScannerFactory {
return func(params snapshot.ScannerParams) *snapshot.Scanner { return func(params snapshot.ScannerParams) *snapshot.Scanner {
return snapshot.NewScanner(snapshot.ScannerConfig{ return snapshot.NewScanner(snapshot.ScannerConfig{
FS: params.Fs, FS: params.Fs,
Storage: storer, Storage: storer,
ChunkSize: faultChunkSize, ChunkSize: faultChunkSize,
MaxBlobSize: faultMaxBlobSize, MaxBlobSize: faultMaxBlobSize,
CompressionLevel: cfg.CompressionLevel, CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients, AgeRecipients: cfg.AgeRecipients,
Repositories: repos, Repositories: repos,
EnableProgress: params.EnableProgress, EnableProgress: params.EnableProgress,
UI: params.UI, UI: params.UI,
Exclude: params.Exclude, Exclude: params.Exclude,
SkipErrors: params.SkipErrors, SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
}) })
} }
} }
+29 -5
View File
@@ -243,12 +243,18 @@ func (v *Vaultik) createNamedSnapshot(
return err return err
} }
previousSnapshotID, err := v.previousSnapshotID(snapName)
if err != nil {
return err
}
scanner := v.ScannerFactory(snapshot.ScannerParams{ scanner := v.ScannerFactory(snapshot.ScannerParams{
EnableProgress: !opts.Cron, EnableProgress: !opts.Cron,
UI: v.UI, UI: v.UI,
Fs: v.Fs, Fs: v.Fs,
Exclude: v.Config.GetExcludes(snapName), Exclude: v.Config.GetExcludes(snapName),
SkipErrors: opts.SkipErrors, SkipErrors: opts.SkipErrors,
PreviousSnapshotID: previousSnapshotID,
}) })
snapshotID, err := v.SnapshotManager.CreateSnapshotWithName( snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
@@ -283,6 +289,24 @@ func (v *Vaultik) createNamedSnapshot(
return nil return nil
} }
// previousSnapshotID returns the ID of the most recent completed snapshot
// named snapName in the local index, or "" when there is none.
func (v *Vaultik) previousSnapshotID(snapName string) (string, error) {
snapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
if err != nil {
return "", fmt.Errorf("listing snapshots: %w", err)
}
for _, s := range snapshots {
if s.CompletedAt != nil &&
parseSnapshotName(s.ID.String(), s.Hostname.String()) == snapName {
return s.ID.String(), nil
}
}
return "", nil
}
// resolveSnapshotPaths resolves source directories to absolute paths // resolveSnapshotPaths resolves source directories to absolute paths
// with symlink resolution. // with symlink resolution.
func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) { func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) {
@@ -0,0 +1,225 @@
package vaultik_test
import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"testing"
"time"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// These tests cover https://git.eeqj.de/sneak/vaultik/issues/304: a
// backup counts a file as deleted only when the previous snapshot of
// the same name had it and it is gone, and never counts an excluded
// file.
// deletedCountEnv is a backup setup over the source directory src whose
// user-facing output is kept in out. Each test configures its snapshots
// with setSnapshot.
type deletedCountEnv struct {
v *vaultik.Vaultik
out *bytes.Buffer
src string
}
// newDeletedCountEnv writes files, keyed by path relative to src.
func newDeletedCountEnv(t *testing.T, files map[string][]byte) *deletedCountEnv {
t.Helper()
ctx := context.Background()
fs := afero.NewOsFs()
tempDir := t.TempDir()
srcDir := filepath.Join(tempDir, "src")
for rel, content := range files {
path := filepath.Join(srcDir, rel)
require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755))
require.NoError(t, afero.WriteFile(fs, path, content, 0o644))
}
cfg := faultTestConfig()
cfg.IndexPath = filepath.Join(tempDir, "index.sqlite")
cfg.Snapshots = map[string]config.SnapshotConfig{}
store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
require.NoError(t, err)
db, err := database.New(ctx, cfg.IndexPath)
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
out := &bytes.Buffer{}
v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
v.UI = ui.NewWithColor(out, false)
return &deletedCountEnv{v: v, out: out, src: srcDir}
}
// setSnapshot configures the snapshot name to back up the path rel
// beneath src, with the given excludes.
func (e *deletedCountEnv) setSnapshot(name, rel string, exclude ...string) {
e.v.Config.Snapshots[name] = config.SnapshotConfig{
Paths: []string{filepath.Join(e.src, rel)},
Exclude: exclude,
}
}
// backUp runs a backup of the named snapshot and returns its output.
func (e *deletedCountEnv) backUp(t *testing.T, name string) string {
t.Helper()
e.out.Reset()
require.NoError(t, e.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
Snapshots: []string{name},
}))
return e.out.String()
}
// A file this snapshot excludes is not counted as deleted: neither one
// that another snapshot backs up, nor one that the previous snapshot of
// this name had before the exclude was added.
func TestBackupDoesNotCountExcludedFileAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"top.txt": []byte("top"),
"cache/tmp.bin": []byte("cache"),
"photos/a.jpg": []byte("photo a"),
"photos/b.jpg": []byte("photo b"),
"photos/c.jpg": []byte("photo c"),
})
env.setSnapshot("photos", "photos")
env.setSnapshot("home", "", "/photos")
env.backUp(t, "photos")
assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
env.setSnapshot("home", "", "/photos", "/cache")
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A file inside a directory that an exclude added after a backup
// matches is not counted as deleted, though the pattern matches only the
// directory.
func TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"top.txt": []byte("top"),
"user/.local/share/Trash/old.txt": []byte("old"),
})
env.setSnapshot("home", "")
assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
env.setSnapshot("home", "", "*/.local/share/Trash")
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// Under --skip-errors a file inside a directory that the walk cannot
// open is not counted as deleted: it is still on disk.
func TestBackupDoesNotCountFileInUnopenableDirectoryAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"open.txt": []byte("open"),
"locked/inside.txt": []byte("inside"),
})
env.setSnapshot("work", "")
assert.Contains(t, env.backUp(t, "work"), filesLine(2, 2, 0))
// A directory's mode does not lock out root, who runs the tests, so
// opening the directory is made to fail instead.
env.v.Fs = &openFailFs{
path: filepath.Join(env.src, "locked"),
err: os.ErrPermission,
}
env.out.Reset()
require.NoError(t, env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
SkipErrors: true,
Snapshots: []string{"work"},
}))
assert.Contains(t, env.out.String(), filesLine(1, 0, 1))
}
// A backup counts only files that the previous snapshot of its own name
// had, not one that a later snapshot of another name over the same path
// had and that is since removed.
func TestBackupIgnoresSnapshotOfAnotherName(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"kept.txt": []byte("kept"),
})
env.setSnapshot("home", "")
env.setSnapshot("other", "")
env.backUp(t, "home")
// Snapshot start times are kept in whole seconds: wait so that other's
// snapshot is strictly the most recent.
time.Sleep(time.Second)
extra := filepath.Join(env.src, "extra.txt")
require.NoError(t, os.WriteFile(extra, []byte("extra"), 0o600))
env.backUp(t, "other")
require.NoError(t, os.Remove(extra))
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A file removed from disk is counted as deleted by the next backup of
// the snapshot that had it, and not by any later backup of that
// snapshot, nor by the first backup of another name over the same path.
func TestBackupCountsDeletedFileOnce(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
aContent := bytes.Repeat([]byte("a"), 1000)
bContent := bytes.Repeat([]byte("b"), 2000)
keptContent := bytes.Repeat([]byte("k"), 500)
env := newDeletedCountEnv(t, map[string][]byte{
"a.txt": aContent,
"b.txt": bContent,
"kept.txt": keptContent,
})
env.setSnapshot("home", "")
env.setSnapshot("other", "")
assert.Contains(t, env.backUp(t, "home"), filesLine(3, 3, 0))
require.NoError(t, os.Remove(filepath.Join(env.src, "a.txt")))
require.NoError(t, os.Remove(filepath.Join(env.src, "b.txt")))
summary := env.backUp(t, "home")
assert.Contains(t, summary,
"Files: 1 examined, 0 backed up, 1 unchanged, 2 deleted.")
assert.Contains(t, summary,
fmt.Sprintf("Data: %s total (%s backed up), %s deleted.",
env.v.UI.Size(int64(len(keptContent))), env.v.UI.Size(0),
env.v.UI.Size(int64(len(aContent)+len(bContent)))))
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
assert.Contains(t, env.backUp(t, "other"), filesLine(1, 0, 1))
}