From b4284b2170bbbabb275613caae37120f2f8ab321 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Oct 2026 01:48:40 +0000 Subject: [PATCH] Keep the progress line of a multi-path snapshot within 100% (closes #271) One progress reporter spans every path of a snapshot. Its counts of files and bytes processed run across all the paths, while the totals they were divided by were reset to each path's own, so a second path smaller than the first showed more than 100% and an ETA of `unknown`. The totals now add up over the paths scanned so far, and the processing start time the rate is measured from is the first path's. SetTotalSize is renamed AddTotalSize because it now adds. The issue says the ETA went negative; it was computed negative and printed as `unknown`. Model: opus-5-5 --- TODO.md | 9 +++ internal/snapshot/progress.go | 21 ++++--- internal/snapshot/progress_test.go | 94 ++++++++++++++++++++++++++++++ internal/snapshot/scanner.go | 4 +- 4 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 internal/snapshot/progress_test.go diff --git a/TODO.md b/TODO.md index 9d6ca17..58ccb29 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-08: Kept the progress line of a snapshot with more than one + path within 100% + ([issue #271](https://git.eeqj.de/sneak/vaultik/issues/271)). The + bytes and files processed counted every path, but the totals they + were divided by held only the current path's, so a second path + smaller than the first showed more than 100% and an ETA of `unknown`. + The totals now add up over the paths scanned so far, and the rate is + measured from when the first path's processing started. + - 2026-10-08: Made a second `snapshot create` of one name succeed when it starts in the same second as the first ([issue #270](https://git.eeqj.de/sneak/vaultik/issues/270)). The diff --git a/internal/snapshot/progress.go b/internal/snapshot/progress.go index 9aba0f5..281e643 100644 --- a/internal/snapshot/progress.go +++ b/internal/snapshot/progress.go @@ -67,9 +67,9 @@ type ProgressStats struct { BlobsUploaded atomic.Int64 BytesUploaded atomic.Int64 CurrentFile atomic.Value // stores string - TotalSize atomic.Int64 // Total size to process (set after scan phase) - TotalFiles atomic.Int64 // Total files to process in phase 2 - ProcessStartTime atomic.Value // stores time.Time when processing starts + TotalSize atomic.Int64 // Size to process in the paths scanned so far + TotalFiles atomic.Int64 // Files to process in the paths scanned so far + ProcessStartTime atomic.Value // stores time.Time; set by the first path StartTime time.Time mu sync.RWMutex lastDetailTime time.Time @@ -148,10 +148,17 @@ func (pr *ProgressReporter) GetStats() *ProgressStats { return pr.stats } -// SetTotalSize sets the total size to process (after scan phase) -func (pr *ProgressReporter) SetTotalSize(size int64) { - pr.stats.TotalSize.Store(size) - pr.stats.ProcessStartTime.Store(time.Now().UTC()) +// AddTotalSize adds the size one path of the snapshot has to process to +// the total, once that path's scan phase is done. The processed counts +// run across every path, so the total does too, and the processing start +// time, which the rate is measured from, is the first path's. +func (pr *ProgressReporter) AddTotalSize(size int64) { + pr.stats.TotalSize.Add(size) + + _, started := pr.stats.ProcessStartTime.Load().(time.Time) + if !started { + pr.stats.ProcessStartTime.Store(time.Now().UTC()) + } } // Helper functions diff --git a/internal/snapshot/progress_test.go b/internal/snapshot/progress_test.go new file mode 100644 index 0000000..ed63c91 --- /dev/null +++ b/internal/snapshot/progress_test.go @@ -0,0 +1,94 @@ +package snapshot_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/afero" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/snapshot" +) + +// TestProgressPercentWithSecondPathSmaller backs up two paths with one +// scanner, as a snapshot with two paths does, the second path smaller +// than the first. The progress line divides the bytes processed by the +// total size, so both must count both paths to stay within 100%. +func TestProgressPercentWithSecondPathSmaller(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + files := map[string]string{ + "/large/one.txt": strings.Repeat("1", 4000), + "/large/two.txt": strings.Repeat("2", 4000), + "/small/three.txt": strings.Repeat("3", 1000), + } + + for path, content := range files { + err := fs.MkdirAll(filepath.Dir(path), 0755) + if err != nil { + t.Fatalf("mkdir: %v", err) + } + + err = afero.WriteFile(fs, path, []byte(content), 0644) + if err != nil { + t.Fatalf("write %s: %v", path, err) + } + } + + db, err := database.NewTestDB() + if err != nil { + t.Fatalf("create test db: %v", err) + } + + t.Cleanup(func() { + cerr := db.Close() + if cerr != nil { + t.Errorf("close db: %v", cerr) + } + }) + + repos := database.NewRepositories(db) + + scanner := snapshot.NewScanner(snapshot.ScannerConfig{ + FS: fs, + ChunkSize: int64(1024 * 16), + Repositories: repos, + MaxBlobSize: int64(1024 * 1024), + CompressionLevel: 3, + AgeRecipients: []string{testAgePublicKey}, + EnableProgress: true, + }) + + // Never started, but Stop releases its tickers and signal handler. + progress := scanner.GetProgress() + defer progress.Stop() + + ctx := context.Background() + snapshotID := "test-snapshot-progress" + createTestSnapshotRecord(ctx, t, repos, snapshotID) + + for _, path := range []string{"/large", "/small"} { + _, err := scanner.Scan(ctx, path, snapshotID) + if err != nil { + t.Fatalf("scanning %s: %v", path, err) + } + } + + stats := progress.GetStats() + + // Directories count toward the total size but produce no chunks, so + // the percentage ends just under 100%. + percent := float64(stats.BytesProcessed.Load()) / + float64(stats.TotalSize.Load()) * 100 + if percent > 100 { + t.Errorf("progress after both paths is %.1f%%, want at most 100%%", + percent) + } + + if stats.FilesProcessed.Load() != stats.TotalFiles.Load() { + t.Errorf("progress after both paths is %d of %d files, want all", + stats.FilesProcessed.Load(), stats.TotalFiles.Load()) + } +} diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 46765b2..79d7e33 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -407,8 +407,8 @@ func (s *Scanner) summarizeScanPhase( } if s.progress != nil { - s.progress.SetTotalSize(totalSizeToProcess) - s.progress.GetStats().TotalFiles.Store(int64(len(filesToProcess))) + s.progress.AddTotalSize(totalSizeToProcess) + s.progress.GetStats().TotalFiles.Add(int64(len(filesToProcess))) } log.Info("Phase 1 complete",