Report blob counts from snapshot verify --deep --json (closes #314)

Deep verify set verified only on success and never set missing, so a
failed --deep --json run reported zero for both. It now counts each blob
whose contents pass as it goes, and the existence check runs through
every blob, counting the ones storage reports not found as missing and
the wrong-size ones as mismatched, as shallow verify does; it fails if it
found any. Any other storage error stops the check and fails with that
error, so a blob that cannot be checked is never reported as missing.

Deep verify still stops at the first damaged blob and reads no blob
contents while any blob is missing or the wrong size, so verified is 0
there where shallow mode counts the present blobs.

Model: opus-5-5

README.md

@@ -294,7 +294,8 @@ needed.
 of the local index and the backup destination store — with timestamps
 and three sizes per snapshot (compressed remote size; total
 uncompressed chunk size; size of the chunks that snapshot stored that
-were not stored before, recorded when its backup ran).
+were not stored before). Both chunk sizes are recorded when the
+snapshot's backup runs.
 
 Listing the destination store does **not** require the age secret key,
 so it works in vaultik's intended configuration, where the backed-up

docs/DATAMODEL.md

@@ -122,6 +122,7 @@ Tracks backup snapshots.
 - `new_chunk_size` (INTEGER) - Total size of those chunks before compression; `snapshot list` shows it as NEW CHUNK SIZE
 - `blob_count` (INTEGER) - Number of blobs this snapshot created
 - `total_size` (INTEGER) - Total size of all files
+- `uncompressed_chunk_size` (INTEGER) - Total size before compression of the distinct chunks the snapshot's files reference, recorded when its backup ran; `snapshot list` shows it as UNCOMPRESSED SIZE
 - `blob_size` (INTEGER) - Total compressed size of all referenced blobs
 - `blob_uncompressed_size` (INTEGER) - Total uncompressed size of all referenced blobs
 - `compression_ratio` (REAL) - Compression ratio achieved

internal/database/schema/001.sql

@@ -89,6 +89,7 @@ CREATE TABLE IF NOT EXISTS snapshots (
     new_chunk_size INTEGER NOT NULL DEFAULT 0,
     blob_count INTEGER NOT NULL DEFAULT 0,
     total_size INTEGER NOT NULL DEFAULT 0,
+    uncompressed_chunk_size INTEGER NOT NULL DEFAULT 0,
     blob_size INTEGER NOT NULL DEFAULT 0,
     blob_uncompressed_size INTEGER NOT NULL DEFAULT 0,
     compression_ratio REAL NOT NULL DEFAULT 1.0,

internal/database/snapshots.go

@@ -120,7 +120,11 @@ func (r *SnapshotRepository) UpdateCounts(
 	return nil
 }
 
-// UpdateExtendedStats updates extended statistics for a snapshot
+// UpdateExtendedStats updates extended statistics for a snapshot. It also
+// records the total plaintext size of the distinct chunks the snapshot's
+// files reference. Only the snapshot's own backup can take that total: a
+// file's row is shared by every snapshot that lists it, and a later
+// backup of a changed file rewrites the row with the new chunks.
 func (r *SnapshotRepository) UpdateExtendedStats(
 	ctx context.Context,
 	tx *sql.Tx,
@@ -145,7 +149,17 @@ func (r *SnapshotRepository) UpdateExtendedStats(
 		    compression_level = ?,
 		    upload_bytes = ?,
 		    upload_duration_ms = ?,
-		    new_chunk_size = ?
+		    new_chunk_size = ?,
+		    uncompressed_chunk_size = (
+		        SELECT COALESCE(SUM(c.size), 0)
+		        FROM (
+		            SELECT DISTINCT fc.chunk_hash
+		            FROM snapshot_files sf
+		            JOIN file_chunks fc ON sf.file_id = fc.file_id
+		            WHERE sf.snapshot_id = snapshots.id
+		        ) sc
+		        JOIN chunks c ON sc.chunk_hash = c.chunk_hash
+		    )
 		WHERE id = ?
 	`
 
@@ -555,30 +569,22 @@ func (r *SnapshotRepository) GetSnapshotBlobSizes(
 	return compressed, uncompressed, nil
 }
 
-// GetSnapshotUncompressedChunkSize returns the sum of plaintext sizes of all unique
+// GetSnapshotUncompressedChunkSize returns the total plaintext size of the
-// chunks referenced by a snapshot (via snapshot_files → file_chunks → chunks).
+// distinct chunks the snapshot's files reference, as the snapshot's backup
+// recorded it (see UpdateExtendedStats).
 func (r *SnapshotRepository) GetSnapshotUncompressedChunkSize(
 	ctx context.Context, snapshotID string,
 ) (int64, error) {
-	query := `
+	query := `SELECT uncompressed_chunk_size FROM snapshots WHERE id = ?`
-		SELECT COALESCE(SUM(c.size), 0)
-		FROM (
-			SELECT DISTINCT fc.chunk_hash
-			FROM snapshot_files sf
-			JOIN file_chunks fc ON sf.file_id = fc.file_id
-			WHERE sf.snapshot_id = ?
-		) sc
-		JOIN chunks c ON sc.chunk_hash = c.chunk_hash
-	`
 
-	var totalSize int64
+	var uncompressedChunkSize int64
 
-	err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&totalSize)
+	err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&uncompressedChunkSize)
 	if err != nil {
 		return 0, fmt.Errorf("querying uncompressed chunk size: %w", err)
 	}
 
-	return totalSize, nil
+	return uncompressedChunkSize, nil
 }
 
 // GetSnapshotNewChunkSize returns the total plaintext size of the chunks the

internal/vaultik/deep_verify_test.go

@@ -1,7 +1,9 @@
 package vaultik_test
 
 import (
+	"bytes"
 	"context"
+	"crypto/rand"
 	"io"
 	"os"
 	"path/filepath"
@@ -9,7 +11,9 @@ import (
 
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/storage"
 	"sneak.berlin/go/vaultik/internal/ui"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -77,6 +81,174 @@ func TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob(t *testing.T) {
 		"deep verify should fail on a corrupted blob")
 }
 
+// TestDeepVerifyJSONCountsBlobs checks the verified and missing counts
+// that deep verify reports under --json: on a healthy snapshot, with a
+// damaged blob, and with missing blobs. Deep verify stops at the first
+// damaged blob, so the blobs read before it are the ones verified; when
+// blobs are missing it reads no blob contents, but counts every missing
+// blob.
+func TestDeepVerifyJSONCountsBlobs(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "remote")
+
+	cfg, storer, snapshotID, keys := backUpSeveralBlobs(ctx, t, storeDir)
+
+	healthy, err := deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.NoError(t, err)
+	require.Equal(t, "ok", healthy.Status)
+	require.Len(t, keys, healthy.BlobCount)
+	require.Equal(t, healthy.BlobCount, healthy.Verified)
+	require.Zero(t, healthy.Missing)
+
+	// Damage the last blob read, keeping its length, so every blob before
+	// it is read and passes first.
+	lastPath := filepath.Join(storeDir, keys[len(keys)-1])
+	blob, err := afero.ReadFile(fs, lastPath)
+	require.NoError(t, err)
+
+	blob[len(blob)/2] ^= 0xff
+	require.NoError(t, afero.WriteFile(fs, lastPath, blob, 0o600))
+
+	got, err := deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, healthy.BlobCount-1, got.Verified)
+	require.Zero(t, got.Missing)
+
+	// Delete the first blob and the damaged last one: both must be
+	// counted as missing, not only the first one found.
+	var deletedSize int64
+
+	for _, key := range []string{keys[0], keys[len(keys)-1]} {
+		info, statErr := storer.Stat(ctx, key)
+		require.NoError(t, statErr)
+
+		deletedSize += info.Size
+
+		require.NoError(t, storer.Delete(ctx, key))
+	}
+
+	got, err = deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, 2, got.Missing)
+	require.Equal(t, deletedSize, got.MissingSize)
+	require.Zero(t, got.Verified)
+}
+
+// TestDeepVerifyJSONCountsWrongSizeBlob checks that under --deep --json a
+// blob of the wrong size is counted in mismatched and fails the run, and
+// that the check goes on past it to count a missing blob after it.
+func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "remote")
+
+	cfg, storer, snapshotID, keys := backUpSeveralBlobs(ctx, t, storeDir)
+
+	// Make the first blob checked longer, and delete the last one.
+	firstPath := filepath.Join(storeDir, keys[0])
+	blob, err := afero.ReadFile(fs, firstPath)
+	require.NoError(t, err)
+
+	blob = append(blob, []byte("extra")...)
+	require.NoError(t, afero.WriteFile(fs, firstPath, blob, 0o600))
+	require.NoError(t, storer.Delete(ctx, keys[len(keys)-1]))
+
+	got, err := deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, 1, got.Mismatched)
+	require.Equal(t, 1, got.Missing)
+	require.Zero(t, got.Verified)
+}
+
+// TestDeepVerifyStopsOnBlobThatCannotBeChecked checks that a blob that is
+// present in storage but cannot be checked is not counted as missing:
+// deep verify stops and fails with the storage error.
+func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "remote")
+
+	cfg, storer, snapshotID, keys := backUpSeveralBlobs(ctx, t, storeDir)
+	failing := statFailingStorer{Storer: storer, failKeys: []string{keys[0]}}
+
+	got, err := deepVerifyJSON(ctx, t, cfg, failing, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Zero(t, got.Missing)
+	require.Contains(t, got.ErrorMessage, errStatDenied.Error())
+}
+
+// backUpSeveralBlobs backs up random data, which does not compress or
+// deduplicate, so it fills at least three blobs in a store on disk at
+// storeDir. It returns the blobs' storage keys sorted, which is hash
+// order, the order deep verify reads them in.
+func backUpSeveralBlobs(
+	ctx context.Context, t *testing.T, storeDir string,
+) (*config.Config, *storage.FileStorer, string, []string) {
+	t.Helper()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "source")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	chunkSize := int64(32 * 1024)
+	maxBlobSize := int64(128 * 1024)
+
+	data := make([]byte, maxBlobSize*4)
+	_, err := rand.Read(data)
+	require.NoError(t, err)
+	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(dataDir, "data.bin"), data, 0o644))
+
+	cfg, storer, snapshotID := runFileStorageBackup(
+		ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
+
+	keys := storedBlobKeys(t, storeDir)
+	require.GreaterOrEqual(t, len(keys), 3)
+
+	return cfg, storer, snapshotID, keys
+}
+
+// deepVerifyJSON runs deep verify with --json on snapshotID and returns
+// the decoded result and the error RunDeepVerify returned.
+func deepVerifyJSON(
+	ctx context.Context, t *testing.T,
+	cfg *config.Config, storer storage.Storer, snapshotID string,
+) (vaultik.VerifyResult, error) {
+	t.Helper()
+
+	var out bytes.Buffer
+
+	v := &vaultik.Vaultik{
+		Config:  cfg,
+		Storage: storer,
+		Fs:      afero.NewOsFs(),
+		Stdout:  &out,
+		Stderr:  io.Discard,
+		UI:      ui.NewWithColor(io.Discard, false),
+	}
+	v.SetContext(ctx)
+
+	err := v.RunDeepVerify(snapshotID,
+		&vaultik.VerifyOptions{Deep: true, JSON: true})
+
+	return decodeVerifyResult(t, out.Bytes()), err
+}
+
 // corruptOneBlob flips a middle byte of the first blob file found under
 // blobsDir, leaving the file length unchanged.
 func corruptOneBlob(t *testing.T, fs afero.Fs, blobsDir string) {

internal/vaultik/snapshot_list_test.go

@@ -487,6 +487,7 @@ type listJSONRow struct {
 	RemoteKey        string `json:"remote_key"`
 	Timestamp        string `json:"timestamp"`
 	CompressedSize   int64  `json:"compressed_size"`
+	UncompressedSize int64  `json:"uncompressed_size"`
 	NewChunkSize     int64  `json:"new_chunk_size"`
 	LocallyTracked   bool   `json:"locally_tracked"`
 	RemotePresent    *bool  `json:"remote_present"`

internal/vaultik/uncompressed_size_test.go

@@ -0,0 +1,85 @@
+package vaultik_test
+
+import (
+	"bytes"
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/storage"
+	"sneak.berlin/go/vaultik/internal/vaultik"
+)
+
+// TestUncompressedSizeOfChangedFileStaysFixed covers
+// https://git.eeqj.de/sneak/vaultik/issues/360. A backup that changes a
+// file leaves the earlier snapshot's UNCOMPRESSED SIZE as it was when that
+// snapshot's backup completed. The file's row in the local index is shared
+// by both snapshots and rewritten by the second backup.
+func TestUncompressedSizeOfChangedFileStaysFixed(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+	changedPath := filepath.Join(dataDir, "changed.bin")
+
+	ctx := context.Background()
+	files := writeFaultSourceTree(t, fs, dataDir)
+	cfg := changedFileConfig(dataDir, dbPath)
+
+	appendRandomBytes(t, fs, files, changedPath, appendedFileSize)
+
+	store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
+	require.NoError(t, err)
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = db.Close() })
+
+	repos := database.NewRepositories(db)
+	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
+
+	require.NoError(t, backUp(v, "first"))
+
+	// With nothing to compare against, sizeOfNewChunks totals every
+	// distinct chunk of the files.
+	firstID := localSnapshotID(ctx, t, repos, "first")
+	firstSize := sizeOfNewChunks(t, files, nil)
+	require.Equal(t, firstSize, listedUncompressedSizes(t, v)[firstID],
+		"the first snapshot must count the chunks of every file it holds")
+
+	appendRandomBytes(t, fs, files, changedPath, faultChunkSize)
+	require.NoError(t, backUp(v, "second"))
+
+	secondID := localSnapshotID(ctx, t, repos, "second")
+	sizes := listedUncompressedSizes(t, v)
+	assert.Equal(t, firstSize, sizes[firstID],
+		"a later backup must not change the first snapshot's figure")
+	assert.Equal(t, sizeOfNewChunks(t, files, nil), sizes[secondID],
+		"the second snapshot must count the changed file's chunks")
+}
+
+// listedUncompressedSizes runs `snapshot list --json` and returns each
+// listed snapshot's uncompressed_size by snapshot ID.
+func listedUncompressedSizes(t *testing.T, v *vaultik.Vaultik) map[string]int64 {
+	t.Helper()
+
+	var out bytes.Buffer
+
+	v.Stdout = &out
+	require.NoError(t, v.ListSnapshots(true))
+
+	sizes := make(map[string]int64)
+	for _, row := range decodeListJSON(t, out.String()) {
+		sizes[row.ID] = row.UncompressedSize
+	}
+
+	return sizes
+}

internal/vaultik/verify.go

@@ -17,6 +17,7 @@ import (
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
+	"sneak.berlin/go/vaultik/internal/storage"
 )
 
 // Sentinel errors for snapshot verification failures.
@@ -33,6 +34,8 @@ var (
 	errManifestMissingBlob = errors.New(
 		"manifest omits blob present in database")
 	errBlobSizeMismatch  = errors.New("blob size mismatch")
+	errBlobsNotInStorage = errors.New(
+		"blobs missing from storage or stored with the wrong size")
 )
 
 // verifyStatusFailed is the JSON status value for a failed verification.
@@ -149,7 +152,6 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
 	}
 
 	result.Status = "ok"
-	result.Verified = len(dbBlobs)
 
 	if opts.JSON {
 		return v.outputVerifyJSON(result)
@@ -306,7 +308,7 @@ func (v *Vaultik) runVerificationSteps(
 		v.stdoutf("Checking blob existence in remote storage...\n")
 	}
 
-	err = v.verifyBlobExistenceFromDB(dbBlobs)
+	err = v.verifyBlobExistenceFromDB(dbBlobs, result)
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)
 	}
@@ -317,7 +319,8 @@ func (v *Vaultik) runVerificationSteps(
 			len(dbBlobs), ubytes(totalSize))
 	}
 
-	err = v.performDeepVerificationFromDB(dbBlobs, tdb.db.Conn(), opts, identities)
+	err = v.performDeepVerificationFromDB(
+		dbBlobs, tdb.db.Conn(), opts, result, identities)
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)
 	}
@@ -669,8 +672,14 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
 	return nil
 }
 
-// verifyBlobExistenceFromDB checks that all blobs from database exist in S3
+// verifyBlobExistenceFromDB checks that every blob from the database is
-func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
+// in storage with the size the database records. It checks them all,
+// counting the missing and wrong-size blobs in result, and fails if it
+// found any. Any storage error other than not found stops the check and
+// fails with that error.
+func (v *Vaultik) verifyBlobExistenceFromDB(
+	blobs []snapshot.BlobInfo, result *VerifyResult,
+) error {
 	log.Info("Verifying blob existence in S3", "blob_count", len(blobs))
 
 	for i, blob := range blobs {
@@ -683,17 +692,23 @@ func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
 		// Construct blob path
 		blobPath := fmt.Sprintf("blobs/%s/%s/%s", blob.Hash[:2], blob.Hash[2:4], blob.Hash)
 
-		// Check blob exists
 		stat, err := v.Storage.Stat(v.ctx, blobPath)
-		if err != nil {
+		switch {
-			return fmt.Errorf("blob %s missing from storage: %w", blob.Hash, err)
+		case errors.Is(err, storage.ErrNotFound):
-		}
+			log.Warn("Blob missing from storage", "hash", blob.Hash)
 
-		// Verify size matches
+			result.Missing++
-		if stat.Size != blob.CompressedSize {
+			result.MissingSize += blob.CompressedSize
-			return fmt.Errorf(
+		case err != nil:
-				"%w: blob %s: S3 has %d bytes, database has %d bytes",
+			return fmt.Errorf("checking blob %s in storage: %w",
-				errBlobSizeMismatch, blob.Hash, stat.Size, blob.CompressedSize)
+				blob.Hash, err)
+		case stat.Size != blob.CompressedSize:
+			log.Warn("Blob has the wrong size in storage",
+				"hash", blob.Hash,
+				"storage_size", stat.Size,
+				"database_size", blob.CompressedSize)
+
+			result.Mismatched++
 		}
 
 		// Progress update every 100 blobs
@@ -707,16 +722,22 @@ func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
 		}
 	}
 
+	if result.Missing > 0 || result.Mismatched > 0 {
+		return fmt.Errorf("%w: %d missing, %d with the wrong size",
+			errBlobsNotInStorage, result.Missing, result.Mismatched)
+	}
+
 	log.Info("✓ All blobs exist in storage")
 
 	return nil
 }
 
 // performDeepVerificationFromDB downloads and verifies the content of
-// each blob using the database as source.
+// each blob using the database as source, stopping at the first blob that
+// fails. Each blob that passes is counted in result.Verified.
 func (v *Vaultik) performDeepVerificationFromDB(
 	blobs []snapshot.BlobInfo, db *sql.DB, opts *VerifyOptions,
-	identities []age.Identity,
+	result *VerifyResult, identities []age.Identity,
 ) error {
 	// Calculate total bytes for ETA
 	var totalBytesExpected int64
@@ -739,6 +760,7 @@ func (v *Vaultik) performDeepVerificationFromDB(
 			return fmt.Errorf("blob %s verification failed: %w", blobInfo.Hash, err)
 		}
 
+		result.Verified++
 		bytesProcessed += blobInfo.CompressedSize
 		elapsed := time.Since(startTime)
 		remaining := len(blobs) - (i + 1)