Stop counting directories and unchanged symlinks as files to back up #383

Merged
clawbot merged 1 commits from issue-305-unchanged-dirs-symlinks into next 2026-10-14 06:46:48 +02:00
5 changed files with 254 additions and 74 deletions
+1 -1
View File
@@ -359,7 +359,7 @@ CreateSnapshot(opts)
## Deduplication Strategy ## Deduplication Strategy
1. **File-level**: Files unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked 1. **File-level**: Files and symlinks unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid, and a symlink's target), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked
2. **Chunk-level**: Chunks are content-addressed by SHA256 hash. If a chunk hash already exists in the database, the chunk data is not re-uploaded. 2. **Chunk-level**: Chunks are content-addressed by SHA256 hash. If a chunk hash already exists in the database, the chunk data is not re-uploaded.
+1 -2
View File
@@ -78,8 +78,7 @@ func TestProgressPercentWithSecondPathSmaller(t *testing.T) {
stats := progress.GetStats() stats := progress.GetStats()
// Directories count toward the total size but produce no chunks, so // With both paths counted, the percentage must not exceed 100%.
// the percentage ends just under 100%.
percent := float64(stats.BytesProcessed.Load()) / percent := float64(stats.BytesProcessed.Load()) /
float64(stats.TotalSize.Load()) * 100 float64(stats.TotalSize.Load()) * 100
if percent > 100 { if percent > 100 {
+108 -71
View File
@@ -284,6 +284,11 @@ func (s *Scanner) Scan(
} }
} }
err = s.recordDirectories(ctx, scanResult.Directories)
if err != nil {
return nil, err
}
// Summarize scan phase results and update progress // Summarize scan phase results and update progress
s.summarizeScanPhase(result, filesToProcess) s.summarizeScanPhase(result, filesToProcess)
@@ -855,14 +860,15 @@ func (s *Scanner) batchInsertFileData(
// ScanPhaseResult contains the results of the scan phase // ScanPhaseResult contains the results of the scan phase
type ScanPhaseResult struct { type ScanPhaseResult struct {
FilesToProcess []*FileToProcess FilesToProcess []*FileToProcess
UnchangedFileIDs []types.FileID // IDs of unchanged files to associate with snapshot UnchangedFileIDs []types.FileID // IDs of unchanged files to associate with snapshot
Directories []*FileToProcess // Every directory found, to record
} }
// scanPhase performs the initial directory scan to identify files to process // scanPhase performs the initial directory scan to identify files to process
// It uses the pre-loaded knownFiles map for fast change detection without DB // It uses the pre-loaded knownFiles map for fast change detection without DB
// queries. It also populates existingFiles map for deletion detection. // queries. It also populates existingFiles map for deletion detection.
// Returns files needing processing and IDs of unchanged files for snapshot // Returns files needing processing, IDs of unchanged files for snapshot
// association. // association, and the directories found.
func (s *Scanner) scanPhase( func (s *Scanner) scanPhase(
ctx context.Context, ctx context.Context,
path string, path string,
@@ -870,8 +876,10 @@ func (s *Scanner) scanPhase(
existingFiles map[string]struct{}, existingFiles map[string]struct{},
knownFiles map[string]*database.File, knownFiles map[string]*database.File,
) (*ScanPhaseResult, error) { ) (*ScanPhaseResult, error) {
// Use known file count as estimate for progress (accurate for subsequent backups) // Use known file count as estimate for progress (accurate for
estimatedTotal := int64(len(knownFiles)) // subsequent backups). The walk does not count directories, so the
// estimate leaves them out.
estimatedTotal := countNonDirectories(knownFiles)
collector := &scanCollector{} collector := &scanCollector{}
@@ -904,7 +912,7 @@ func (s *Scanner) scanPhase(
// Handle symlinks and directories // Handle symlinks and directories
handled, err := s.recordSpecialEntry( handled, err := s.recordSpecialEntry(
filePath, info, existingFiles, collector, result) filePath, info, existingFiles, knownFiles, collector, result)
if handled { if handled {
return err return err
} }
@@ -918,9 +926,10 @@ func (s *Scanner) scanPhase(
existingFiles[filePath] = struct{}{} existingFiles[filePath] = struct{}{}
// Check file against in-memory map (no DB query!) // Check file against in-memory map (no DB query!)
file, needsProcessing := s.checkFileInMemory(filePath, info, knownFiles) file := s.buildRegularFileEntry(filePath, info)
needsProcessing := s.checkFileInMemory(file, knownFiles)
filesScanned, changedCount := collector.recordRegularFile( filesScanned, changedCount := collector.recordFile(
filePath, info, file, needsProcessing) filePath, info, file, needsProcessing)
// Update result stats // Update result stats
@@ -944,15 +953,32 @@ func (s *Scanner) scanPhase(
return &ScanPhaseResult{ return &ScanPhaseResult{
FilesToProcess: collector.filesToProcess, FilesToProcess: collector.filesToProcess,
UnchangedFileIDs: collector.unchangedFileIDs, UnchangedFileIDs: collector.unchangedFileIDs,
Directories: collector.directories,
}, nil }, nil
} }
// countNonDirectories returns how many of files are not directories.
func countNonDirectories(files map[string]*database.File) int64 {
var count int64
for _, f := range files {
if f.Mode&uint32(os.ModeDir) == 0 {
count++
}
}
return count
}
// recordSpecialEntry records symlinks and directories (which have no // recordSpecialEntry records symlinks and directories (which have no
// data to chunk) and reports whether it handled the entry. For a symlink // data to chunk) and reports whether it handled the entry. A symlink is
// whose target cannot be read it returns handleWalkError's result. // counted and checked for changes as a regular file is; a directory is
// set aside to be recorded and is not counted. For a symlink whose
// target cannot be read it returns handleWalkError's result.
func (s *Scanner) recordSpecialEntry( func (s *Scanner) recordSpecialEntry(
filePath string, info os.FileInfo, filePath string, info os.FileInfo,
existingFiles map[string]struct{}, existingFiles map[string]struct{},
knownFiles map[string]*database.File,
collector *scanCollector, result *ScanResult, collector *scanCollector, result *ScanResult,
) (bool, error) { ) (bool, error) {
// Handle symlinks // Handle symlinks
@@ -963,8 +989,9 @@ func (s *Scanner) recordSpecialEntry(
} }
existingFiles[filePath] = struct{}{} existingFiles[filePath] = struct{}{}
collector.addToProcess(filePath, info, file) needsProcessing := s.checkFileInMemory(file, knownFiles)
s.updateScanEntryStats(result, true, info) collector.recordFile(filePath, info, file, needsProcessing)
s.updateScanEntryStats(result, needsProcessing, info)
return true, nil return true, nil
} }
@@ -974,7 +1001,7 @@ func (s *Scanner) recordSpecialEntry(
if info.IsDir() { if info.IsDir() {
file := s.buildDirectoryEntry(filePath, info) file := s.buildDirectoryEntry(filePath, info)
existingFiles[filePath] = struct{}{} existingFiles[filePath] = struct{}{}
collector.addToProcess(filePath, info, file) collector.addDirectory(filePath, info, file)
return true, nil return true, nil
} }
@@ -1004,28 +1031,29 @@ type scanCollector struct {
mu sync.Mutex mu sync.Mutex
filesToProcess []*FileToProcess filesToProcess []*FileToProcess
unchangedFileIDs []types.FileID // Just IDs - no new records needed unchangedFileIDs []types.FileID // Just IDs - no new records needed
directories []*FileToProcess
filesScanned int64 filesScanned int64
} }
// addToProcess queues a file entry for processing. // addDirectory sets a directory aside to be recorded. It is not counted
func (c *scanCollector) addToProcess( // as a file.
func (c *scanCollector) addDirectory(
path string, info os.FileInfo, file *database.File, path string, info os.FileInfo, file *database.File,
) { ) {
c.mu.Lock() c.mu.Lock()
defer c.mu.Unlock() defer c.mu.Unlock()
c.filesToProcess = append(c.filesToProcess, &FileToProcess{ c.directories = append(c.directories, &FileToProcess{
Path: path, Path: path,
FileInfo: info, FileInfo: info,
File: file, File: file,
}) })
c.filesScanned++
} }
// recordRegularFile records a regular file as changed (to process) or // recordFile records a regular file or symlink as changed (to process)
// unchanged (snapshot association only) and returns the running scanned // or unchanged (snapshot association only) and returns the running
// and changed counts. // scanned and changed counts.
func (c *scanCollector) recordRegularFile( func (c *scanCollector) recordFile(
path string, info os.FileInfo, file *database.File, needsProcessing bool, path string, info os.FileInfo, file *database.File, needsProcessing bool,
) (int64, int) { ) (int64, int) {
c.mu.Lock() c.mu.Lock()
@@ -1128,7 +1156,28 @@ func (s *Scanner) printScanProgressLine(
} }
} }
// buildSymlinkEntry creates a File record for a symlink. // buildRegularFileEntry creates a File record for a regular file, with
// no ID: checkFileInMemory sets it.
func (s *Scanner) buildRegularFileEntry(path string, info os.FileInfo) *database.File {
var uid, gid uint32
if stat, ok := info.Sys().(*syscall.Stat_t); ok {
uid = stat.Uid
gid = stat.Gid
}
return &database.File{
Path: types.FilePath(path),
SourcePath: types.SourcePath(s.currentSourcePath),
MTime: info.ModTime(),
Size: info.Size(),
Mode: uint32(info.Mode()),
UID: uid,
GID: gid,
}
}
// buildSymlinkEntry creates a File record for a symlink, with no ID:
// checkFileInMemory sets it.
func (s *Scanner) buildSymlinkEntry( func (s *Scanner) buildSymlinkEntry(
path string, info os.FileInfo, path string, info os.FileInfo,
) (*database.File, error) { ) (*database.File, error) {
@@ -1144,7 +1193,6 @@ func (s *Scanner) buildSymlinkEntry(
} }
return &database.File{ return &database.File{
ID: types.NewFileID(),
Path: types.FilePath(path), Path: types.FilePath(path),
SourcePath: types.SourcePath(s.currentSourcePath), SourcePath: types.SourcePath(s.currentSourcePath),
MTime: info.ModTime(), MTime: info.ModTime(),
@@ -1176,6 +1224,22 @@ func (s *Scanner) buildDirectoryEntry(path string, info os.FileInfo) *database.F
} }
} }
// recordDirectories records every directory the walk found. A directory
// has no data to back up, so it is not among the files counted and
// backed up.
func (s *Scanner) recordDirectories(
ctx context.Context, directories []*FileToProcess,
) error {
for _, dir := range directories {
err := s.recordNonRegularFile(ctx, dir)
if err != nil {
return fmt.Errorf("recording directory %s: %w", dir.Path, err)
}
}
return nil
}
// recordNonRegularFile writes a symlink or directory entry to the database // recordNonRegularFile writes a symlink or directory entry to the database
// and associates it with the current snapshot. No chunking is performed. // and associates it with the current snapshot. No chunking is performed.
func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess) error { func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess) error {
@@ -1189,62 +1253,36 @@ func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess)
}) })
} }
// checkFileInMemory checks if a file needs processing using the in-memory map // checkFileInMemory reports whether a regular file or symlink needs
// processing, and gives file the ID of its known record, or a new ID for
// a new path.
// No database access is performed - this is purely CPU/memory work // No database access is performed - this is purely CPU/memory work
func (s *Scanner) checkFileInMemory( func (s *Scanner) checkFileInMemory(
path string, info os.FileInfo, knownFiles map[string]*database.File, file *database.File, knownFiles map[string]*database.File,
) (*database.File, bool) { ) bool {
var uid, gid uint32 existingFile, exists := knownFiles[file.Path.String()]
if stat, ok := info.Sys().(*syscall.Stat_t); ok {
uid = stat.Uid
gid = stat.Gid
}
// Check against in-memory map first to get existing ID if available
existingFile, exists := knownFiles[path]
// Create file record with ID set upfront
// For new files, generate UUID immediately so it's available for chunk associations
// For existing files, reuse the existing ID
var fileID types.FileID
if exists {
fileID = existingFile.ID
} else {
fileID = types.NewFileID()
}
file := &database.File{
ID: fileID,
Path: types.FilePath(path),
SourcePath: types.SourcePath(s.currentSourcePath),
MTime: info.ModTime(),
Size: info.Size(),
Mode: uint32(info.Mode()),
UID: uid,
GID: gid,
}
// New file - needs processing // New file - needs processing
if !exists { if !exists {
return file, true file.ID = types.NewFileID()
return true
} }
file.ID = existingFile.ID
// No uploaded blob holds one of its chunks (see loadFilesToRechunk) // No uploaded blob holds one of its chunks (see loadFilesToRechunk)
if _, rechunk := s.filesToRechunk[fileID]; rechunk { if _, rechunk := s.filesToRechunk[file.ID]; rechunk {
return file, true return true
} }
// Check if file has changed // Check if file has changed
if existingFile.Size != file.Size || return existingFile.Size != file.Size ||
!existingFile.MTime.Equal(file.MTime) || !existingFile.MTime.Equal(file.MTime) ||
existingFile.Mode != file.Mode || existingFile.Mode != file.Mode ||
existingFile.UID != file.UID || existingFile.UID != file.UID ||
existingFile.GID != file.GID { existingFile.GID != file.GID ||
return file, true existingFile.LinkTarget != file.LinkTarget
}
// File unchanged
return file, false
} }
// batchAddFilesToSnapshot adds existing file IDs to the snapshot association table // batchAddFilesToSnapshot adds existing file IDs to the snapshot association table
@@ -1371,17 +1409,16 @@ func (s *Scanner) processPhase(
return s.finalizeProcessPhase(ctx, result) return s.finalizeProcessPhase(ctx, result)
} }
// processFileWithErrorHandling records a directory or symlink, or wraps // processFileWithErrorHandling records a symlink, or wraps
// processFileStreaming for a regular file with error recovery for deleted // processFileStreaming for a regular file with error recovery for deleted
// files and skip-errors mode. Returns (skipped, error). // files and skip-errors mode. Returns (skipped, error).
func (s *Scanner) processFileWithErrorHandling( func (s *Scanner) processFileWithErrorHandling(
ctx context.Context, fileToProcess *FileToProcess, result *ScanResult, ctx context.Context, fileToProcess *FileToProcess, result *ScanResult,
) (bool, error) { ) (bool, error) {
// A directory or symlink has no data to open or read; it is only // A symlink has no data to open or read; it is only recorded in the
// recorded in the local index. An error recording it stops the run // local index. An error recording it stops the run even under
// even under --skip-errors, or the snapshot would complete without it. // --skip-errors, or the snapshot would complete without it.
mode := os.FileMode(fileToProcess.File.Mode) if os.FileMode(fileToProcess.File.Mode)&os.ModeSymlink != 0 {
if mode&os.ModeSymlink != 0 || mode.IsDir() {
err := s.recordNonRegularFile(ctx, fileToProcess) err := s.recordNonRegularFile(ctx, fileToProcess)
if err != nil { if err != nil {
return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err) return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)
@@ -0,0 +1,28 @@
//nolint:testpackage // exercises the unexported countNonDirectories helper
package snapshot
import (
"os"
"testing"
"sneak.berlin/go/vaultik/internal/database"
)
// The walk counts regular files and symlinks, not directories, so the
// estimate of its count made from the local index's records leaves the
// directories out. See https://git.eeqj.de/sneak/vaultik/issues/305.
func TestCountNonDirectories(t *testing.T) {
t.Parallel()
known := map[string]*database.File{
"/src": {Mode: uint32(os.ModeDir | 0o755)},
"/src/sub": {Mode: uint32(os.ModeDir | 0o755)},
"/src/a.txt": {Mode: 0o644},
"/src/link": {Mode: uint32(os.ModeSymlink | 0o777), LinkTarget: "a.txt"},
}
got := countNonDirectories(known)
if got != 2 {
t.Errorf("countNonDirectories = %d, want 2", got)
}
}
+116
View File
@@ -13,6 +13,7 @@ import (
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
"sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
@@ -188,6 +189,42 @@ func (e *summaryEnv) dataLine(total, backedUp int64) string {
e.v.UI.Size(total), e.v.UI.Size(backedUp)) e.v.UI.Size(total), e.v.UI.Size(backedUp))
} }
// enumeratedLine returns the line printed when the walk of a source path
// ends.
func (e *summaryEnv) enumeratedLine(
files int, total int64, toBackUp int, toBackUpSize int64,
) string {
return fmt.Sprintf(
"Enumerated %d snapshot source files (%s total), %d to back up (%s).",
files, e.v.UI.Size(total), toBackUp, e.v.UI.Size(toBackUpSize))
}
// snapshotPaths returns the paths the snapshot lists, sorted.
func (e *summaryEnv) snapshotPaths(t *testing.T, snapshotID string) []string {
t.Helper()
rows, err := e.db.Conn().QueryContext(context.Background(), `
SELECT f.path FROM snapshot_files sf JOIN files f ON f.id = sf.file_id
WHERE sf.snapshot_id = ? ORDER BY f.path`, snapshotID)
require.NoError(t, err)
defer func() { _ = rows.Close() }()
var paths []string
for rows.Next() {
var path string
require.NoError(t, rows.Scan(&path))
paths = append(paths, path)
}
require.NoError(t, rows.Err())
return paths
}
// A first backup stores copy.bin's chunks while backing up a.bin, so // A first backup stores copy.bin's chunks while backing up a.bin, so
// copy.bin's chunks are deduplicated within the run. Each file and byte // copy.bin's chunks are deduplicated within the run. Each file and byte
// is still counted once. // is still counted once.
@@ -248,6 +285,85 @@ func TestSnapshotSummaryIncrementalRunWithDeduplicatedChunks(t *testing.T) {
assert.Equal(t, uncompressed, snap.BlobUncompressedSize) assert.Equal(t, uncompressed, snap.BlobUncompressedSize)
} }
// A backup of a tree with directories and a symlink, none of which
// changed since the last backup, backs up nothing, and the snapshot
// still lists every one of them. Both sides of the line that ends the
// walk count the same entries, the files and the symlink, and no
// directory's size. See https://git.eeqj.de/sneak/vaultik/issues/305.
func TestSnapshotSummaryUnchangedRunWithDirectoriesAndSymlink(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newSummaryEnv(t)
const linkTarget = "a.bin"
require.NoError(t, os.Symlink(linkTarget,
filepath.Join(filepath.Dir(env.aPath), "link")))
// A symlink's size is the length of its target.
total := env.totalSize + int64(len(linkTarget))
first := env.backUp(t, "first", false)
assert.Contains(t, first, env.enumeratedLine(4, total, 4, total))
assert.Contains(t, first, filesLine(4, 4, 0))
second := env.backUp(t, "second", false)
assert.Contains(t, second, env.enumeratedLine(4, total, 0, 0))
assert.Contains(t, second, filesLine(4, 0, 4))
assert.Contains(t, second, env.dataLine(total, 0))
assert.Equal(t,
env.snapshotPaths(t, env.snapshot(t, "first").ID.String()),
env.snapshotPaths(t, env.snapshot(t, "second").ID.String()))
}
// A directory whose mtime changed and a symlink whose target changed are
// recorded again by the next backup, which counts only the symlink as
// backed up.
func TestSnapshotSummaryChangedDirectoryAndSymlink(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newSummaryEnv(t)
// The local index keeps paths with symlinks resolved.
dir, err := filepath.EvalSymlinks(filepath.Dir(env.aPath))
require.NoError(t, err)
// The new link gets the old one's mtime, and a target, which need not
// exist, of the old target's length, a symlink's size: only the
// target differs.
linkTime := unix.NsecToTimeval(
time.Date(2029, time.March, 4, 5, 6, 7, 0, time.UTC).UnixNano())
link := filepath.Join(dir, "link")
require.NoError(t, os.Symlink("a.bin", link))
require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
env.backUp(t, "first", false)
require.NoError(t, os.Remove(link))
require.NoError(t, os.Symlink("b.bin", link))
require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
mtime := time.Date(2030, time.January, 2, 3, 4, 5, 0, time.UTC)
require.NoError(t, os.Chtimes(dir, mtime, mtime))
assert.Contains(t, env.backUp(t, "second", false), filesLine(4, 1, 3))
ctx := context.Background()
dirRow, err := env.repos.Files.GetByPath(ctx, dir)
require.NoError(t, err)
require.NotNil(t, dirRow)
assert.Equal(t, mtime.Unix(), dirRow.MTime.Unix())
linkRow, err := env.repos.Files.GetByPath(ctx, link)
require.NoError(t, err)
require.NotNil(t, linkRow)
assert.Equal(t, "b.bin", linkRow.LinkTarget.String())
}
// Under --cron the progress reporter is off; the upload figures must // Under --cron the progress reporter is off; the upload figures must
// still reach the summary and the snapshots row. The snapshot has two // still reach the summary and the snapshots row. The snapshot has two
// paths, each backed up by its own scan. // paths, each backed up by its own scan.