diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8079402..045e39c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -359,7 +359,7 @@ CreateSnapshot(opts) ## 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. diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 10a5541..61ac3d0 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -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 s.summarizeScanPhase(result, filesToProcess) @@ -855,14 +860,15 @@ func (s *Scanner) batchInsertFileData( // ScanPhaseResult contains the results of the scan phase type ScanPhaseResult struct { 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 // It uses the pre-loaded knownFiles map for fast change detection without DB // queries. It also populates existingFiles map for deletion detection. -// Returns files needing processing and IDs of unchanged files for snapshot -// association. +// Returns files needing processing, IDs of unchanged files for snapshot +// association, and the directories found. func (s *Scanner) scanPhase( ctx context.Context, path string, @@ -870,8 +876,10 @@ func (s *Scanner) scanPhase( existingFiles map[string]struct{}, knownFiles map[string]*database.File, ) (*ScanPhaseResult, error) { - // Use known file count as estimate for progress (accurate for subsequent backups) - estimatedTotal := int64(len(knownFiles)) + // Use known file count as estimate for progress (accurate for + // subsequent backups). The walk does not count directories, so the + // estimate leaves them out. + estimatedTotal := countNonDirectories(knownFiles) collector := &scanCollector{} @@ -904,7 +912,7 @@ func (s *Scanner) scanPhase( // Handle symlinks and directories handled, err := s.recordSpecialEntry( - filePath, info, existingFiles, collector, result) + filePath, info, existingFiles, knownFiles, collector, result) if handled { return err } @@ -918,9 +926,10 @@ func (s *Scanner) scanPhase( existingFiles[filePath] = struct{}{} // 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) // Update result stats @@ -944,15 +953,32 @@ func (s *Scanner) scanPhase( return &ScanPhaseResult{ FilesToProcess: collector.filesToProcess, UnchangedFileIDs: collector.unchangedFileIDs, + Directories: collector.directories, }, 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 -// data to chunk) and reports whether it handled the entry. For a symlink -// whose target cannot be read it returns handleWalkError's result. +// data to chunk) and reports whether it handled the entry. A symlink is +// 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( filePath string, info os.FileInfo, existingFiles map[string]struct{}, + knownFiles map[string]*database.File, collector *scanCollector, result *ScanResult, ) (bool, error) { // Handle symlinks @@ -963,8 +989,9 @@ func (s *Scanner) recordSpecialEntry( } existingFiles[filePath] = struct{}{} - collector.addToProcess(filePath, info, file) - s.updateScanEntryStats(result, true, info) + needsProcessing := s.checkFileInMemory(file, knownFiles) + collector.recordFile(filePath, info, file, needsProcessing) + s.updateScanEntryStats(result, needsProcessing, info) return true, nil } @@ -974,7 +1001,7 @@ func (s *Scanner) recordSpecialEntry( if info.IsDir() { file := s.buildDirectoryEntry(filePath, info) existingFiles[filePath] = struct{}{} - collector.addToProcess(filePath, info, file) + collector.addDirectory(filePath, info, file) return true, nil } @@ -1004,28 +1031,29 @@ type scanCollector struct { mu sync.Mutex filesToProcess []*FileToProcess unchangedFileIDs []types.FileID // Just IDs - no new records needed + directories []*FileToProcess filesScanned int64 } -// addToProcess queues a file entry for processing. -func (c *scanCollector) addToProcess( +// addDirectory sets a directory aside to be recorded. It is not counted +// as a file. +func (c *scanCollector) addDirectory( path string, info os.FileInfo, file *database.File, ) { c.mu.Lock() defer c.mu.Unlock() - c.filesToProcess = append(c.filesToProcess, &FileToProcess{ + c.directories = append(c.directories, &FileToProcess{ Path: path, FileInfo: info, File: file, }) - c.filesScanned++ } -// recordRegularFile records a regular file as changed (to process) or -// unchanged (snapshot association only) and returns the running scanned -// and changed counts. -func (c *scanCollector) recordRegularFile( +// recordFile records a regular file or symlink as changed (to process) +// or unchanged (snapshot association only) and returns the running +// scanned and changed counts. +func (c *scanCollector) recordFile( path string, info os.FileInfo, file *database.File, needsProcessing bool, ) (int64, int) { 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( path string, info os.FileInfo, ) (*database.File, error) { @@ -1144,7 +1193,6 @@ func (s *Scanner) buildSymlinkEntry( } return &database.File{ - ID: types.NewFileID(), Path: types.FilePath(path), SourcePath: types.SourcePath(s.currentSourcePath), 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 // and associates it with the current snapshot. No chunking is performed. 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 func (s *Scanner) checkFileInMemory( - path string, info os.FileInfo, knownFiles map[string]*database.File, -) (*database.File, bool) { - var uid, gid uint32 - 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, - } + file *database.File, knownFiles map[string]*database.File, +) bool { + existingFile, exists := knownFiles[file.Path.String()] // New file - needs processing 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) - if _, rechunk := s.filesToRechunk[fileID]; rechunk { - return file, true + if _, rechunk := s.filesToRechunk[file.ID]; rechunk { + return true } // Check if file has changed - if existingFile.Size != file.Size || + return existingFile.Size != file.Size || !existingFile.MTime.Equal(file.MTime) || existingFile.Mode != file.Mode || existingFile.UID != file.UID || - existingFile.GID != file.GID { - return file, true - } - - // File unchanged - return file, false + existingFile.GID != file.GID || + existingFile.LinkTarget != file.LinkTarget } // batchAddFilesToSnapshot adds existing file IDs to the snapshot association table @@ -1371,17 +1409,16 @@ func (s *Scanner) processPhase( 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 // files and skip-errors mode. Returns (skipped, error). func (s *Scanner) processFileWithErrorHandling( ctx context.Context, fileToProcess *FileToProcess, result *ScanResult, ) (bool, error) { - // A directory or symlink has no data to open or read; it is only - // recorded in the local index. An error recording it stops the run - // even under --skip-errors, or the snapshot would complete without it. - mode := os.FileMode(fileToProcess.File.Mode) - if mode&os.ModeSymlink != 0 || mode.IsDir() { + // A symlink has no data to open or read; it is only recorded in the + // local index. An error recording it stops the run even under + // --skip-errors, or the snapshot would complete without it. + if os.FileMode(fileToProcess.File.Mode)&os.ModeSymlink != 0 { err := s.recordNonRegularFile(ctx, fileToProcess) if err != nil { return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err) diff --git a/internal/snapshot/scanner_estimate_test.go b/internal/snapshot/scanner_estimate_test.go new file mode 100644 index 0000000..501c1a2 --- /dev/null +++ b/internal/snapshot/scanner_estimate_test.go @@ -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) + } +} diff --git a/internal/vaultik/snapshot_summary_test.go b/internal/vaultik/snapshot_summary_test.go index a901562..1385637 100644 --- a/internal/vaultik/snapshot_summary_test.go +++ b/internal/vaultik/snapshot_summary_test.go @@ -13,6 +13,7 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/database" "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)) } +// 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 // copy.bin's chunks are deduplicated within the run. Each file and byte // is still counted once. @@ -248,6 +285,85 @@ func TestSnapshotSummaryIncrementalRunWithDeduplicatedChunks(t *testing.T) { 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 // still reach the summary and the snapshots row. The snapshot has two // paths, each backed up by its own scan.