Keep unreferenced blobs newer than prune_grace_period (closes #387)

A backup uploads its blobs before the manifest that lists them, and the
process lock covers one user on one machine, so a prune from another host
or user sharing the destination deleted a running backup's blobs.

Prune now deletes an unreferenced blob only once its listed time is older
than the new config key prune_grace_period (default 24h; a negative value
is refused). A newer blob, or one listed with no time, is kept and counted
(blobs_too_new). A backup uploads a blob already on the destination again
instead of skipping it, so it gets a fresh time. The rclone backend takes
each object's time from the listing (use_server_modtime).

Cost: after database delete, the next backup uploads all its data again.
Side effect: a rebuilt blob replaces the stored copy; if compression_level
or the recipients changed since, older snapshots listing it fail verify.

Model: opus-5-5

internal/cli/config.go

@@ -239,6 +239,13 @@ storage_url: ""
 # Default: 3
 # compression_level: 3
 
+# How old a blob that no snapshot lists must be before prune deletes it.
+# A backup uploads its blobs before the manifest that lists them, so on a
+# destination shared between hosts or users this must be longer than the
+# longest backup run. Accepts: 36h, 90m, etc. 0s deletes them at once.
+# Default: 24h
+# prune_grace_period: 24h
+
 # Hostname used in snapshot IDs. Default: system hostname.
 # hostname: myserver
 

internal/cli/database.go

@@ -48,9 +48,9 @@ This will erase all local tracking of:
 - The storage-binding record
 
 The remote storage is NOT affected. After deletion, the next backup
-will perform a full scan and re-deduplicate against existing remote
-blobs, and the local index will re-bind to the currently configured
-storage destination on that run.
+will perform a full scan and upload all the data again, and the local
+index will re-bind to the currently configured storage destination on
+that run.
 
 Use --force to skip the confirmation prompt.`,
 		Args: cobra.NoArgs,

internal/cli/entry_prune_quiet_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -14,8 +15,8 @@ import (
 const flagCron = "--cron"
 
 // plantUnreferencedBlob writes a blob that no snapshot references to the
-// destination store at storeDir, so that a prune deletes it. Returns its
-// path.
+// destination store at storeDir, dated long before any prune grace period,
+// so that a prune deletes it. Returns its path.
 func plantUnreferencedBlob(t *testing.T, storeDir string) string {
 	t.Helper()
 
@@ -25,6 +26,9 @@ func plantUnreferencedBlob(t *testing.T, storeDir string) string {
 	require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o750))
 	require.NoError(t, os.WriteFile(path, []byte("unreferenced"), 0o600))
 
+	longAgo := time.Unix(0, 0)
+	require.NoError(t, os.Chtimes(path, longAgo, longAgo))
+
 	return path
 }
 

internal/cli/prune.go

@@ -20,7 +20,10 @@ Local cleanup drops incomplete snapshots and any files, chunks, or
 blobs no longer referenced by a completed snapshot. Remote cleanup
 scans every snapshot manifest in the destination store, builds the
 set of still-referenced blob hashes, and deletes any blob not in that
-set.
+set that is older than prune_grace_period (default 24h). A newer one
+is kept: it may belong to a backup still running, on this host or
+another sharing the destination store, that has not yet uploaded its
+manifest.
 
 Snapshot create --prune runs the same cleanup automatically; this
 command is the manual entry point for the same work (e.g. after a

internal/cli/snapshot.go

@@ -132,8 +132,8 @@ on Linux), then /etc/vaultik/config.yml.`,
 		"Run in cron mode (silent unless warning or error)")
 	cmd.Flags().BoolVar(&opts.Prune, "prune", false,
 		"After backup, drop older snapshots of the same name and remove "+
-			"orphaned blobs (not after a backup that skipped unreadable "+
-			"files)")
+			"orphaned blobs older than prune_grace_period (not after a "+
+			"backup that skipped unreadable files)")
 	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) "+

internal/config/config.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"time"
 
 	"filippo.io/age"
 	"git.eeqj.de/sneak/smartconfig"
@@ -38,6 +39,7 @@ const (
 	defaultChunkSize        = Size(10 * 1000 * 1000)        // 10MB
 	defaultS3PartSize       = Size(5 * 1024 * 1024)         // 5MiB
 	defaultCompressionLevel = 3
+	defaultPruneGracePeriod = 24 * time.Hour
 	minChunkSize            = 1000 * 1000 // 1MB
 	minCompressionLevel     = 1
 	maxCompressionLevel     = 19
@@ -67,6 +69,8 @@ var (
 	errFileURLNotAbsolute = errors.New(
 		"a file:// storage_url needs an absolute path, " +
 			"as in file:///path/to/backup")
+	errNegativePruneGracePeriod = errors.New(
+		"prune_grace_period must not be negative")
 	errStorageNotConfigured = errors.New(
 		"storage not configured; set storage_url or provide s3.endpoint + " +
 			"s3.bucket + credentials")
@@ -187,6 +191,13 @@ type Config struct {
 	Snapshots        map[string]SnapshotConfig `yaml:"snapshots"`
 	CompressionLevel int                       `yaml:"compression_level"`
 
+	// PruneGracePeriod is how old a blob no manifest lists must be before
+	// a prune deletes it. A backup lists its blobs in a manifest only
+	// once it has uploaded them all, so a younger one may belong to a
+	// backup still running, on this host or another sharing the
+	// destination store.
+	PruneGracePeriod time.Duration `yaml:"prune_grace_period"`
+
 	// StorageURL specifies the storage backend using a URL format.
 	// Takes precedence over S3Config if set.
 	// Supported formats:
@@ -288,6 +299,7 @@ func Parse(data []byte) (*Config, error) {
 		ChunkSize:        defaultChunkSize,
 		IndexPath:        filepath.Join(xdg.DataHome, appName, "index.sqlite"),
 		CompressionLevel: defaultCompressionLevel,
+		PruneGracePeriod: defaultPruneGracePeriod,
 		S3:               S3Config{PartSize: defaultS3PartSize},
 	}
 
@@ -356,6 +368,7 @@ func Parse(data []byte) (*Config, error) {
 //     exceeds the configured limit
 //   - Compression level must be between 1 and 19
 //   - S3 part size must be between 5MiB and 5GiB, the part sizes S3 accepts
+//   - Prune grace period must not be negative
 //
 // Returns an error describing the first validation failure encountered.
 func (c *Config) Validate() error {
@@ -405,6 +418,10 @@ func (c *Config) Validate() error {
 		return errBadS3PartSize
 	}
 
+	if c.PruneGracePeriod < 0 {
+		return errNegativePruneGracePeriod
+	}
+
 	return nil
 }
 

internal/config/config_test.go

@@ -412,6 +412,44 @@ func TestDocumentedSizeExamples(t *testing.T) {
 	}
 }
 
+// TestValidatePruneGracePeriod checks that a negative prune_grace_period
+// is refused rather than acting like 0s, which deletes unreferenced blobs
+// at once, and that 0s and the documented examples are accepted.
+func TestValidatePruneGracePeriod(t *testing.T) {
+	t.Parallel()
+
+	const withoutGracePeriod = "snapshots:\n" +
+		"  test:\n" +
+		"    paths: [/tmp/vaultik-test-source]\n" +
+		"storage_url: file:///tmp/vaultik-test-storage\n"
+
+	tests := []struct {
+		setting string
+		wantErr error
+	}{
+		{"prune_grace_period: 0s", nil},
+		{"prune_grace_period: 90m", nil},
+		{"prune_grace_period: 36h", nil},
+		{"prune_grace_period: -1h", errNegativePruneGracePeriod},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.setting, func(t *testing.T) {
+			t.Parallel()
+
+			cfg, err := Parse([]byte(withoutGracePeriod + tt.setting + "\n"))
+			if err != nil {
+				t.Fatalf("Parse() unexpected error: %v", err)
+			}
+
+			err = cfg.Validate()
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatalf("Validate() error = %v, want %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
 // TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
 // relative path fails at load. Accepted, it would resolve against the
 // working directory, so a backup run from another directory would write

internal/database/blobs.go

@@ -155,9 +155,7 @@ func (r *BlobRepository) UpdateFinished(
 }
 
 // UpdateUploaded marks a blob as uploaded and records the size of the
-// stored object as its compressed size. That size can differ from the
-// local copy's when an identical blob was already stored with another
-// compression level or other recipients.
+// stored object as its compressed size.
 func (r *BlobRepository) UpdateUploaded(
 	ctx context.Context, tx *sql.Tx, id string, storedSize int64,
 ) error {

internal/s3/client.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"strings"
 	"sync/atomic"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
@@ -302,7 +303,10 @@ func IsNotFound(err error) bool {
 type ObjectInfo struct {
 	Key  string
 	Size int64
-	Err  error
+	// ModTime is the object's LastModified time. Only ListObjectsStream
+	// sets it.
+	ModTime time.Time
+	Err     error
 }
 
 // ListObjectsStream lists objects with the given prefix and returns a channel.
@@ -343,8 +347,9 @@ func (c *Client) ListObjectsStream(
 					}
 
 					ch <- ObjectInfo{
-						Key:  key,
-						Size: *obj.Size,
+						Key:     key,
+						Size:    *obj.Size,
+						ModTime: aws.ToTime(obj.LastModified),
 					}
 				}
 			}

internal/snapshot/scanner.go

@@ -1641,8 +1641,7 @@ func (s *Scanner) handleBlobReady(
 	blobPath := fmt.Sprintf("blobs/%s/%s/%s",
 		finishedBlob.Hash[:2], finishedBlob.Hash[2:4], finishedBlob.Hash)
 
-	storedSize, blobExists, err := s.uploadBlobIfNeeded(
-		ctx, blobPath, blobWithReader, startTime, result)
+	err := s.uploadBlob(ctx, blobPath, blobWithReader, startTime, result)
 	if err != nil {
 		cleanupErr := s.cleanupBlobTempFile(blobWithReader)
 
@@ -1650,8 +1649,7 @@ func (s *Scanner) handleBlobReady(
 			fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err), cleanupErr)
 	}
 
-	err = s.recordBlobMetadata(
-		ctx, finishedBlob, storedSize, blobExists, startTime)
+	err = s.recordBlobMetadata(ctx, finishedBlob, startTime)
 	if err != nil {
 		cleanupErr := s.cleanupBlobTempFile(blobWithReader)
 
@@ -1675,46 +1673,33 @@ func (s *Scanner) handleBlobReady(
 	return nil
 }
 
-// uploadBlobIfNeeded uploads the blob to storage if it doesn't already
-// exist. It returns the size of the object in storage, which for an
-// existing object can differ from the local copy's, and whether it
-// already existed.
-func (s *Scanner) uploadBlobIfNeeded(
+// uploadBlob uploads the blob to storage. A blob already there, left by
+// an earlier run stopped before its manifest, is uploaded again rather
+// than skipped: keeping its old time would let a prune elsewhere delete
+// it as an unreferenced blob older than prune_grace_period before this
+// backup's manifest lists it.
+func (s *Scanner) uploadBlob(
 	ctx context.Context,
 	blobPath string,
 	blobWithReader *blob.WithReader,
 	startTime time.Time,
 	result *ScanResult,
-) (int64, bool, error) {
+) error {
 	finishedBlob := blobWithReader.FinishedBlob
-
-	// Check if blob already exists (deduplication after restart)
 	destination := s.storage.Info().Location
 
-	stored, err := s.storage.Stat(ctx, blobPath)
-	if err == nil {
-		log.Info("Blob already exists in storage, skipping upload",
-			"hash", finishedBlob.Hash,
-			"size", humanize.Bytes(safeUint64(finishedBlob.Compressed)))
-		s.ui.Infof("Blob %s (%s) already exists at %s. Skipping upload.",
-			s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
-			s.ui.Path(destination))
-
-		return stored.Size, true, nil
-	}
-
 	s.ui.Beginf("Uploading blob %s (%s) to %s.",
 		s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
 		s.ui.Path(destination))
 
 	progressCallback := s.makeUploadProgressCallback(ctx, finishedBlob, startTime)
 
-	err = s.storage.PutWithProgress(ctx, blobPath, blobWithReader.Reader,
+	err := s.storage.PutWithProgress(ctx, blobPath, blobWithReader.Reader,
 		finishedBlob.Compressed, progressCallback)
 	if err != nil {
 		log.Error("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)
 
-		return 0, false, fmt.Errorf("uploading blob to storage: %w", err)
+		return fmt.Errorf("uploading blob to storage: %w", err)
 	}
 
 	uploadDuration := time.Since(startTime)
@@ -1745,7 +1730,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 		stats.BytesUploaded.Add(finishedBlob.Compressed)
 	}
 
-	return finishedBlob.Compressed, false, nil
+	return nil
 }
 
 // makeUploadProgressCallback creates a progress callback for blob uploads.
@@ -1815,8 +1800,6 @@ func (s *Scanner) makeUploadProgressCallback(
 func (s *Scanner) recordBlobMetadata(
 	ctx context.Context,
 	finishedBlob *blob.FinishedBlob,
-	storedSize int64,
-	blobExists bool,
 	startTime time.Time,
 ) error {
 	finishedBlobID, err := types.ParseBlobID(finishedBlob.ID)
@@ -1828,7 +1811,7 @@ func (s *Scanner) recordBlobMetadata(
 
 	return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error {
 		err := s.repos.Blobs.UpdateUploaded(
-			txCtx, tx, finishedBlob.ID, storedSize)
+			txCtx, tx, finishedBlob.ID, finishedBlob.Compressed)
 		if err != nil {
 			return fmt.Errorf("updating blob upload timestamp: %w", err)
 		}
@@ -1839,19 +1822,17 @@ func (s *Scanner) recordBlobMetadata(
 			return fmt.Errorf("adding blob to snapshot: %w", err)
 		}
 
-		if !blobExists {
-			upload := &database.Upload{
-				BlobHash:   finishedBlob.Hash,
-				SnapshotID: s.snapshotID,
-				UploadedAt: startTime,
-				Size:       finishedBlob.Compressed,
-				DurationMs: uploadDuration.Milliseconds(),
-			}
+		upload := &database.Upload{
+			BlobHash:   finishedBlob.Hash,
+			SnapshotID: s.snapshotID,
+			UploadedAt: startTime,
+			Size:       finishedBlob.Compressed,
+			DurationMs: uploadDuration.Milliseconds(),
+		}
 
-			err := s.repos.Uploads.Create(txCtx, tx, upload)
-			if err != nil {
-				return fmt.Errorf("recording upload metrics: %w", err)
-			}
+		err = s.repos.Uploads.Create(txCtx, tx, upload)
+		if err != nil {
+			return fmt.Errorf("recording upload metrics: %w", err)
 		}
 
 		return nil

internal/storage/conformance_test.go

@@ -8,6 +8,7 @@ import (
 	"reflect"
 	"sort"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/vaultik/internal/storage"
 )
@@ -19,15 +20,17 @@ import (
 // constructor, so the contract is defined once.
 //
 // It exercises the public Storer interface: round-trip, stat, list with
-// prefix filtering, overwrite, delete, delete-of-missing, and not-found on
-// Get and Stat. Each section takes its own fresh backend instance, so the
-// order of sections never matters and no section sees another's objects.
+// prefix filtering, the modification time ListStream reports, overwrite,
+// delete, delete-of-missing, and not-found on Get and Stat. Each section
+// takes its own fresh backend instance, so the order of sections never
+// matters and no section sees another's objects.
 func runStorerConformance(t *testing.T, newStorer func(*testing.T) storage.Storer) {
 	t.Helper()
 
 	conformanceRoundTrip(t, newStorer(t))
 	conformanceOverwrite(t, newStorer(t))
 	conformanceList(t, newStorer(t))
+	conformanceListStreamModTime(t, newStorer(t))
 	conformanceDelete(t, newStorer(t))
 	conformanceNotFound(t, newStorer(t))
 }
@@ -118,6 +121,35 @@ func conformanceList(t *testing.T, s storage.Storer) {
 	}
 }
 
+// conformanceListStreamModTime checks that ListStream gives an object
+// just written a modification time of about now, which prune compares
+// with its grace period.
+func conformanceListStreamModTime(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+
+	err := s.Put(ctx, "blobs/dd/written-now", bytes.NewReader([]byte("data")))
+	if err != nil {
+		t.Fatalf("Put: %v", err)
+	}
+
+	var listed []storage.ObjectInfo
+
+	for info := range s.ListStream(ctx, "blobs/") {
+		listed = append(listed, info)
+	}
+
+	if len(listed) != 1 || listed[0].Err != nil {
+		t.Fatalf("ListStream(\"blobs/\") = %v, want the one object", listed)
+	}
+
+	if time.Since(listed[0].ModTime).Abs() > time.Minute {
+		t.Errorf("ListStream ModTime = %v, want within a minute of now",
+			listed[0].ModTime)
+	}
+}
+
 // conformanceDelete checks that Delete removes an object and that deleting
 // a missing key is not an error.
 func conformanceDelete(t *testing.T, s storage.Storer) {

internal/storage/file.go

@@ -235,8 +235,9 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
 				// Normalize path separators
 				relPath = strings.ReplaceAll(relPath, string(filepath.Separator), "/")
 				ch <- ObjectInfo{
-					Key:  relPath,
-					Size: info.Size(),
+					Key:     relPath,
+					Size:    info.Size(),
+					ModTime: info.ModTime(),
 				}
 			}
 

internal/storage/rclone.go

@@ -53,6 +53,14 @@ func NewRcloneStorer(ctx context.Context, remote, path string) (*RcloneStorer, e
 		rclonePath += path
 	}
 
+	// On S3, Swift and some other remotes rclone keeps a time of its own in
+	// each object's metadata, which costs one request per object to read.
+	// rclone's use_server_modtime setting makes ModTime the time the
+	// listing gives instead: when the object was stored, which is what
+	// prune compares with prune_grace_period.
+	ctx, ci := fs.AddConfig(ctx)
+	ci.UseServerModTime = true
+
 	// Create the rclone filesystem
 	fsys, err := fs.NewFs(ctx, rclonePath)
 	if err != nil {
@@ -211,8 +219,9 @@ func (r *RcloneStorer) ListStream(
 
 			if prefix == "" || strings.HasPrefix(key, prefix) {
 				ch <- ObjectInfo{
-					Key:  key,
-					Size: obj.Size(),
+					Key:     key,
+					Size:    obj.Size(),
+					ModTime: obj.ModTime(ctx),
 				}
 			}
 		})

internal/storage/rclone_test.go

@@ -5,11 +5,17 @@ import (
 	"context"
 	"errors"
 	"io"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync/atomic"
 	"testing"
+	"time"
 
+	"github.com/johannesboyne/gofakes3"
+	"github.com/johannesboyne/gofakes3/backend/s3mem"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/config"
 	"github.com/rclone/rclone/fs/config/configmap"
@@ -429,3 +435,72 @@ func TestNewRcloneStorerReadsRcloneConfigEnv(t *testing.T) {
 		t.Errorf("object is not in the remote's directory: %v", err)
 	}
 }
+
+// TestRcloneListStreamOnS3MakesNoRequestPerObject checks that listing an
+// rclone S3 remote takes each object's time from the listing itself. By
+// default rclone reads the time it keeps in each object's metadata, one
+// request per object, and every command that lists the destination store
+// would slow down as it grows.
+//
+//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
+func TestRcloneListStreamOnS3MakesNoRequestPerObject(t *testing.T) {
+	backend := s3mem.New()
+
+	err := backend.CreateBucket(s3TestBucket)
+	if err != nil {
+		t.Fatalf("create bucket: %v", err)
+	}
+
+	const body = "blob-bytes"
+
+	_, err = backend.PutObject(s3TestBucket, testBlobKey, nil,
+		strings.NewReader(body), int64(len(body)))
+	if err != nil {
+		t.Fatalf("seed object: %v", err)
+	}
+
+	var headRequests atomic.Int64
+
+	fakeS3 := gofakes3.New(backend).Server()
+	srv := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			if r.Method == http.MethodHead {
+				headRequests.Add(1)
+			}
+
+			fakeS3.ServeHTTP(w, r)
+		}))
+	t.Cleanup(srv.Close)
+
+	remote := ":s3,provider=Other" +
+		",access_key_id=" + s3TestAccessKeyID +
+		",secret_access_key=" + s3TestSecretAccessKey +
+		",endpoint='" + srv.URL + "'"
+	ctx := context.Background()
+
+	s, err := storage.NewRcloneStorer(ctx, remote, s3TestBucket)
+	if err != nil {
+		t.Fatalf("NewRcloneStorer: %v", err)
+	}
+
+	headRequests.Store(0)
+
+	var listed []storage.ObjectInfo
+
+	for info := range s.ListStream(ctx, "blobs/") {
+		listed = append(listed, info)
+	}
+
+	if len(listed) != 1 || listed[0].Err != nil {
+		t.Fatalf("ListStream(\"blobs/\") = %v, want the one object", listed)
+	}
+
+	if time.Since(listed[0].ModTime).Abs() > time.Minute {
+		t.Errorf("ListStream ModTime = %v, want within a minute of now",
+			listed[0].ModTime)
+	}
+
+	if n := headRequests.Load(); n != 0 {
+		t.Errorf("ListStream made %d HEAD request(s), want none", n)
+	}
+}

internal/storage/s3.go

@@ -89,9 +89,10 @@ func (s *S3Storer) ListStream(ctx context.Context, prefix string) <-chan ObjectI
 
 		for info := range s.client.ListObjectsStream(ctx, prefix, false) {
 			ch <- ObjectInfo{
-				Key:  info.Key,
-				Size: info.Size,
-				Err:  info.Err,
+				Key:     info.Key,
+				Size:    info.Size,
+				ModTime: info.ModTime,
+				Err:     info.Err,
 			}
 		}
 	}()

internal/storage/storer.go

@@ -14,6 +14,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"time"
 )
 
 // ErrNotFound is returned when an object does not exist.
@@ -27,7 +28,10 @@ type ProgressCallback func(bytesTransferred int64) error
 type ObjectInfo struct {
 	Key  string // Object key/path
 	Size int64  // Size in bytes
-	Err  error  // Error for streaming results (nil on success)
+	// ModTime is when the object was written, as the store reports it.
+	// Only ListStream sets it; Stat leaves it zero.
+	ModTime time.Time
+	Err     error // Error for streaming results (nil on success)
 }
 
 // Info provides human-readable storage configuration.
@@ -67,7 +71,8 @@ type Storer interface {
 	// For large result sets, prefer ListStream.
 	List(ctx context.Context, prefix string) ([]string, error)
 
-	// ListStream returns a channel of ObjectInfo for large result sets.
+	// ListStream returns a channel of ObjectInfo, with each object's key,
+	// size and modification time, for large result sets.
 	// The channel is closed when listing completes.
 	// If an error occurs during listing, the final item will have Err set.
 	ListStream(ctx context.Context, prefix string) <-chan ObjectInfo

internal/vaultik/info.go

@@ -457,9 +457,9 @@ func (v *Vaultik) scanRemoteBlobStorage(
 		return err
 	}
 
-	for _, size := range allBlobs {
+	for _, blob := range allBlobs {
 		result.TotalBlobCount++
-		result.TotalBlobSize += size
+		result.TotalBlobSize += blob.Size
 	}
 
 	// A blob named only by a manifest that could not be read, or by one
@@ -474,10 +474,10 @@ func (v *Vaultik) scanRemoteBlobStorage(
 		orphanedSize  int64
 	)
 
-	for hash, size := range allBlobs {
+	for hash, blob := range allBlobs {
 		if _, referenced := referencedBlobs[hash]; !referenced {
 			orphanedCount++
-			orphanedSize += size
+			orphanedSize += blob.Size
 		}
 	}
 

internal/vaultik/prune.go

@@ -5,8 +5,10 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 
 	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/storage"
 )
 
 // PruneOptions contains options for the prune command
@@ -163,6 +165,10 @@ type PruneBlobsResult struct {
 	BlobsDeleted int   `json:"blobs_deleted"`
 	BlobsFailed  int   `json:"blobs_failed,omitempty"`
 	BytesFreed   int64 `json:"bytes_freed"`
+	// BlobsTooNew counts the unreferenced blobs kept because they are
+	// newer than the prune grace period, or of unknown age. BlobsFound
+	// does not include them.
+	BlobsTooNew int `json:"blobs_too_new"`
 }
 
 // Prune removes orphaned data from the local index database AND
@@ -208,9 +214,15 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 		return err
 	}
 
-	unreferencedBlobs, totalSize := v.findUnreferencedBlobs(allBlobs, allBlobsReferenced)
+	unreferencedBlobs, totalSize, tooNew := v.findUnreferencedBlobs(
+		allBlobs, allBlobsReferenced)
 
-	result := &PruneBlobsResult{BlobsFound: len(unreferencedBlobs)}
+	result := &PruneBlobsResult{
+		BlobsFound:  len(unreferencedBlobs),
+		BlobsTooNew: tooNew,
+	}
+
+	v.reportBlobsTooNew(tooNew, opts)
 
 	if len(unreferencedBlobs) == 0 {
 		log.Info("No unreferenced blobs found")
@@ -344,11 +356,12 @@ func (v *Vaultik) listUniqueSnapshotIDs() ([]string, error) {
 	return snapshotIDs, nil
 }
 
-// listAllRemoteBlobs returns a map of all blob hashes to their sizes in remote storage
-func (v *Vaultik) listAllRemoteBlobs() (map[string]int64, error) {
+// listAllRemoteBlobs returns a map of all blob hashes in remote storage
+// to their listing entries.
+func (v *Vaultik) listAllRemoteBlobs() (map[string]storage.ObjectInfo, error) {
 	log.Info("Listing all blobs in storage")
 
-	allBlobs := make(map[string]int64)
+	allBlobs := make(map[string]storage.ObjectInfo)
 	blobObjectCh := v.Storage.ListStream(v.ctx, "blobs/")
 
 	for object := range blobObjectCh {
@@ -372,7 +385,7 @@ func (v *Vaultik) listAllRemoteBlobs() (map[string]int64, error) {
 			continue
 		}
 
-		allBlobs[parts[3]] = object.Size
+		allBlobs[parts[3]] = object
 	}
 
 	log.Info("Found blobs in storage", "count", len(allBlobs))
@@ -380,30 +393,59 @@ func (v *Vaultik) listAllRemoteBlobs() (map[string]int64, error) {
 	return allBlobs, nil
 }
 
-// findUnreferencedBlobs returns blob hashes not referenced by any
-// manifest and their total size.
+// findUnreferencedBlobs returns the hashes of the blobs no manifest
+// references that are older than the prune grace period, and their total
+// size. It also returns how many unreferenced blobs it left out for
+// being newer than that, or of unknown age because the listing gave no
+// time for them.
 func (v *Vaultik) findUnreferencedBlobs(
-	allBlobs map[string]int64, referenced map[string]bool,
-) ([]string, int64) {
+	allBlobs map[string]storage.ObjectInfo, referenced map[string]bool,
+) ([]string, int64, int) {
 	var (
 		unreferenced []string
 		totalSize    int64
+		tooNew       int
 	)
 
-	for hash, size := range allBlobs {
-		if !referenced[hash] {
-			unreferenced = append(unreferenced, hash)
-			totalSize += size
+	cutoff := time.Now().UTC().Add(-v.Config.PruneGracePeriod)
+
+	for hash, blob := range allBlobs {
+		if referenced[hash] {
+			continue
 		}
+
+		if blob.ModTime.IsZero() || blob.ModTime.After(cutoff) {
+			tooNew++
+
+			continue
+		}
+
+		unreferenced = append(unreferenced, hash)
+		totalSize += blob.Size
 	}
 
-	return unreferenced, totalSize
+	return unreferenced, totalSize, tooNew
+}
+
+// reportBlobsTooNew says how many unreferenced blobs prune kept for being
+// newer than the grace period or of unknown age. Under --json the
+// document carries the count instead.
+func (v *Vaultik) reportBlobsTooNew(tooNew int, opts *PruneOptions) {
+	if tooNew == 0 || opts.JSON {
+		return
+	}
+
+	v.UI.Infof("Kept %d unreferenced blob(s) newer than "+
+		"prune_grace_period (%s), or of unknown age: a backup still "+
+		"running may need them.",
+		tooNew, v.Config.PruneGracePeriod)
 }
 
 // deleteUnreferencedBlobs deletes the given blobs from storage and
 // populates the result.
 func (v *Vaultik) deleteUnreferencedBlobs(
-	unreferencedBlobs []string, allBlobs map[string]int64, result *PruneBlobsResult,
+	unreferencedBlobs []string, allBlobs map[string]storage.ObjectInfo,
+	result *PruneBlobsResult,
 ) {
 	log.Info("Deleting unreferenced blobs")
 
@@ -418,7 +460,7 @@ func (v *Vaultik) deleteUnreferencedBlobs(
 		}
 
 		result.BlobsDeleted++
-		result.BytesFreed += allBlobs[hash]
+		result.BytesFreed += allBlobs[hash].Size
 
 		if (i+1)%progressLogEvery == 0 || i == len(unreferencedBlobs)-1 {
 			log.Info("Deletion progress",

internal/vaultik/prune_grace_period_test.go

@@ -0,0 +1,260 @@
+package vaultik_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"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/storage/faultstore"
+	"sneak.berlin/go/vaultik/internal/ui"
+	"sneak.berlin/go/vaultik/internal/vaultik"
+)
+
+// TestPruneDuringBackupKeepsItsBlobs runs a prune from another host
+// sharing the destination store at the moment a backup has uploaded its
+// blobs but not yet its manifest, so no manifest lists those blobs. The
+// prune must leave them, and the backup must then restore.
+func TestPruneDuringBackupKeepsItsBlobs(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	storeDir := t.TempDir()
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	ctx := context.Background()
+	cfg := faultTestConfig()
+	testFiles := writeFaultSourceTree(t, fs, dataDir)
+
+	inner, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	otherHostCfg := faultTestConfig()
+	otherHostCfg.PruneGracePeriod = time.Hour
+	otherHost := newReaderVaultik(ctx, otherHostCfg, inner, nil, fs)
+
+	var (
+		pruned   bool
+		pruneErr error
+	)
+
+	store := faultstore.New(inner)
+	store.OnPut = func(key string) faultstore.PutAction {
+		if strings.HasSuffix(key, "/manifest.json.zst") {
+			pruned = true
+			pruneErr = otherHost.PruneBlobs(&vaultik.PruneOptions{Force: true})
+		}
+
+		return faultstore.PutNormal
+	}
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+
+	repos := database.NewRepositories(db)
+
+	id := fullFaultBackup(ctx, t, fs, store, cfg, repos, dataDir, dbPath, "shared")
+	require.NoError(t, db.Close())
+
+	require.True(t, pruned, "the prune must run before the manifest upload")
+	require.NoError(t, pruneErr)
+
+	v := newReaderVaultik(ctx, cfg, inner, nil, fs)
+	require.NoError(t, v.Restore(&vaultik.RestoreOptions{
+		SnapshotID: id,
+		TargetDir:  restoreDir,
+		Verify:     true,
+	}), "a prune during the backup must not delete the backup's blobs")
+
+	assertRestoredTree(t, fs, restoreDir, testFiles)
+}
+
+// TestPruneDuringBackupKeepsBlobsOfKilledRun covers a backup that builds
+// the blobs an earlier run, stopped before its manifest upload, left on
+// the destination store, older than the grace period by the time of the
+// backup. A prune from another host runs before the backup's manifest
+// upload, and the backup must then restore.
+func TestPruneDuringBackupKeepsBlobsOfKilledRun(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	storeDir := t.TempDir()
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	ctx := context.Background()
+	testFiles := writeFaultSourceTree(t, fs, dataDir)
+
+	cfg := faultTestConfig()
+	cfg.IndexPath = dbPath
+	cfg.ChunkSize = config.Size(faultChunkSize)
+	cfg.Snapshots = map[string]config.SnapshotConfig{
+		"data": {Paths: []string{dataDir}},
+	}
+
+	inner, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	otherHostCfg := faultTestConfig()
+	otherHostCfg.PruneGracePeriod = time.Hour
+	otherHost := newReaderVaultik(ctx, otherHostCfg, inner, nil, fs)
+
+	var (
+		killedRun = true
+		pruned    bool
+		pruneErr  error
+	)
+
+	store := faultstore.New(inner)
+	store.OnPut = func(key string) faultstore.PutAction {
+		if !strings.HasSuffix(key, "/manifest.json.zst") {
+			return faultstore.PutNormal
+		}
+
+		if killedRun {
+			return faultstore.PutFail
+		}
+
+		pruned = true
+		pruneErr = otherHost.PruneBlobs(&vaultik.PruneOptions{Force: true})
+
+		return faultstore.PutNormal
+	}
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+
+	repos := database.NewRepositories(db)
+	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
+	opts := &vaultik.SnapshotCreateOptions{Cron: true}
+
+	require.Error(t, v.CreateSnapshot(opts),
+		"the first run must stop at its manifest upload")
+
+	beforeGracePeriod := time.Now().Add(-2 * otherHostCfg.PruneGracePeriod)
+
+	for object := range inner.ListStream(ctx, "blobs/") {
+		require.NoError(t, object.Err)
+		require.NoError(t, os.Chtimes(filepath.Join(storeDir, object.Key),
+			beforeGracePeriod, beforeGracePeriod))
+	}
+
+	killedRun = false
+
+	require.NoError(t, v.CreateSnapshot(opts))
+	require.True(t, pruned, "the prune must run before the manifest upload")
+	require.NoError(t, pruneErr)
+
+	local, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
+	require.NoError(t, err)
+	require.Len(t, local, 1)
+	require.NoError(t, db.Close())
+
+	reader := newReaderVaultik(ctx, cfg, inner, nil, fs)
+	require.NoError(t, reader.Restore(&vaultik.RestoreOptions{
+		SnapshotID: local[0].ID.String(),
+		TargetDir:  restoreDir,
+		Verify:     true,
+	}), "a prune during the backup must not delete the blobs it rebuilt")
+
+	assertRestoredTree(t, fs, restoreDir, testFiles)
+}
+
+// TestPruneKeepsUnreferencedBlobWithNoTime checks that a blob no
+// manifest lists, whose listing gives no modification time (as an S3
+// listing entry without LastModified does), is kept and counted with
+// the blobs too new to delete: its age is unknown.
+func TestPruneKeepsUnreferencedBlobWithNoTime(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newListEnv(t)
+
+	// Written into the mock store's map directly rather than through
+	// Put, so its listing gives no time.
+	blobKey := "blobs/" + testBlobHashA[:2] + "/" + testBlobHashA[2:4] +
+		"/" + testBlobHashA
+	env.store.data[blobKey] = []byte("blob")
+
+	require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{JSON: true}))
+
+	assert.True(t, env.store.hasKey(blobKey),
+		"a blob of unknown age must be kept")
+
+	var result vaultik.PruneBlobsResult
+
+	require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &result))
+	assert.Equal(t, 0, result.BlobsDeleted)
+	assert.Equal(t, 1, result.BlobsTooNew)
+}
+
+// TestPruneDeletesUnreferencedBlobOlderThanGracePeriod checks that the
+// grace period only delays deletion: a blob no manifest lists is deleted
+// once it is older than the grace period, and kept and counted until
+// then.
+func TestPruneDeletesUnreferencedBlobOlderThanGracePeriod(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := t.TempDir()
+
+	store, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	oldKey := "blobs/" + testBlobHashA[:2] + "/" + testBlobHashA[2:4] +
+		"/" + testBlobHashA
+	newKey := "blobs/" + testBlobHashB[:2] + "/" + testBlobHashB[2:4] +
+		"/" + testBlobHashB
+
+	for _, key := range []string{oldKey, newKey} {
+		require.NoError(t, store.Put(ctx, key, strings.NewReader("blob")))
+	}
+
+	gracePeriod := time.Hour
+	beforeGracePeriod := time.Now().Add(-gracePeriod - time.Minute)
+	require.NoError(t, os.Chtimes(filepath.Join(storeDir, oldKey),
+		beforeGracePeriod, beforeGracePeriod))
+
+	stdout := &bytes.Buffer{}
+	v := &vaultik.Vaultik{
+		Config:  &config.Config{PruneGracePeriod: gracePeriod},
+		Storage: store,
+		Stdout:  stdout,
+		UI:      ui.NewWithColor(stdout, false),
+	}
+	v.SetContext(ctx)
+
+	require.NoError(t, v.PruneBlobs(&vaultik.PruneOptions{JSON: true}))
+
+	_, err = store.Stat(ctx, oldKey)
+	require.ErrorIs(t, err, storage.ErrNotFound,
+		"a blob older than the grace period must be deleted")
+
+	_, err = store.Stat(ctx, newKey)
+	require.NoError(t, err, "a blob newer than the grace period must be kept")
+
+	var result vaultik.PruneBlobsResult
+
+	require.NoError(t, json.Unmarshal(stdout.Bytes(), &result))
+	assert.Equal(t, 1, result.BlobsDeleted)
+	assert.Equal(t, 1, result.BlobsTooNew)
+}

internal/vaultik/remove_snapshot_test.go

@@ -26,11 +26,14 @@ import (
 type testStorer struct {
 	mu   sync.Mutex
 	data map[string][]byte
+	// putTimes holds when Put stored each object; ListStream reports it.
+	putTimes map[string]time.Time
 }
 
 func newTestStorer() *testStorer {
 	return &testStorer{
-		data: make(map[string][]byte),
+		data:     make(map[string][]byte),
+		putTimes: make(map[string]time.Time),
 	}
 }
 
@@ -44,6 +47,7 @@ func (s *testStorer) Put(_ context.Context, key string, reader io.Reader) error
 	}
 
 	s.data[key] = data
+	s.putTimes[key] = time.Now().UTC()
 
 	return nil
 }
@@ -87,6 +91,7 @@ func (s *testStorer) Delete(_ context.Context, key string) error {
 	defer s.mu.Unlock()
 
 	delete(s.data, key)
+	delete(s.putTimes, key)
 
 	return nil
 }
@@ -120,8 +125,9 @@ func (s *testStorer) ListStream(
 		for key, data := range s.data {
 			if prefix == "" || strings.HasPrefix(key, prefix) {
 				ch <- storage.ObjectInfo{
-					Key:  key,
-					Size: int64(len(data)),
+					Key:     key,
+					Size:    int64(len(data)),
+					ModTime: s.putTimes[key],
 				}
 			}
 		}

internal/vaultik/reused_blob_verify_test.go

@@ -14,13 +14,13 @@ import (
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
-// TestVerifyPassesWhenBackupReusesStoredBlob backs up a file, then backs it
-// up again from an empty local index, as after `database delete`, with an
-// extra age recipient. The second backup builds the same blob, finds it
-// already stored and skips the upload, but its own copy is larger, because
-// age writes a header entry for each recipient. The second snapshot must
-// record the size of the stored object, so shallow and deep verify pass.
-func TestVerifyPassesWhenBackupReusesStoredBlob(t *testing.T) {
+// TestVerifyPassesWhenBackupRebuildsStoredBlob backs up a file, then backs
+// it up again from an empty local index, as after `database delete`, with
+// an extra age recipient. The second backup builds the same blob, already
+// stored, and uploads it again; its copy is larger, because age writes a
+// header entry for each recipient. The second snapshot must record the
+// size of the object now stored, so shallow and deep verify pass.
+func TestVerifyPassesWhenBackupRebuildsStoredBlob(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
 
@@ -58,8 +58,8 @@ func TestVerifyPassesWhenBackupReusesStoredBlob(t *testing.T) {
 	snapshotID, result := runDedupSnapshot(ctx, t, env.sm, env.makeScanner(),
 		env.cfg.Hostname, "second", dataDir, secondDBPath)
 	require.Positive(t, result.BlobsCreated)
-	require.Zero(t, result.BlobsUploaded,
-		"the second backup should find every blob already stored")
+	require.Equal(t, result.BlobsCreated, result.BlobsUploaded,
+		"the second backup should upload every blob again")
 
 	require.NoError(t, env.db.Close())