Exit as interrupted when Ctrl-C lands during a local index write (closes #301)

Once the run's context is cancelled, database/sql rolls an open
transaction back itself, so the rollback in WithTx returned
sql.ErrTxDone. WithTx treated any rollback error as fatal and called
os.Exit(1): an interrupted backup printed "FATAL: failed to rollback
transaction", exited 1 instead of 130 and skipped its deferred cleanup.

WithTx now ignores sql.ErrTxDone from the rollback and returns the
body's error. A rollback that really fails is returned together with
the body's error instead of ending the process; on a panic it is
dropped and the panic carries on. The tests make the rollback really
fail by running ROLLBACK inside the body.

Model: opus-5-5

docs/DATAMODEL.md

@@ -90,7 +90,7 @@ Stores information about packed, compressed, and encrypted blob files.
 - `created_ts` (INTEGER NOT NULL) - Creation timestamp
 - `finished_ts` (INTEGER) - Finalization timestamp (NULL if in progress)
 - `uncompressed_size` (INTEGER NOT NULL DEFAULT 0) - Total size of chunks before compression
-- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption. Once uploaded, the size of the object in storage: when the blob was already stored, under another compression level or other recipients, this can differ from the size of the local copy
+- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption
 - `uploaded_ts` (INTEGER) - Upload completion timestamp (NULL if not uploaded)
 
 **Purpose:** Tracks blob lifecycle from creation through upload. The UUID primary key allows immediate association of chunks with blobs.
@@ -183,7 +183,7 @@ Tracks blob upload metrics.
    - `UPDATE blobs SET blob_hash = ?, finished_ts = ?` - Finalize blob after packing
 
 4. **Upload**
-   - `UPDATE blobs SET uploaded_ts = ?, compressed_size = ?` - Mark blob as uploaded, with the size of the object in storage (the upload is skipped when the blob is already stored)
+   - `UPDATE blobs SET uploaded_ts = ?` - Mark blob as uploaded
    - `INSERT INTO uploads` - Record upload metrics with snapshot_id
    - `INSERT INTO snapshot_blobs` - Associate blob with snapshot
 

internal/database/blobs.go

@@ -159,16 +159,13 @@ func (r *BlobRepository) UpdateFinished(
 	return nil
 }
 
-// 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.
+// UpdateUploaded marks a blob as uploaded
 func (r *BlobRepository) UpdateUploaded(
-	ctx context.Context, tx *sql.Tx, id string, storedSize int64,
+	ctx context.Context, tx *sql.Tx, id string,
 ) error {
 	query := `
-		UPDATE blobs
-		SET uploaded_ts = ?, compressed_size = ?
+		UPDATE blobs 
+		SET uploaded_ts = ?
 		WHERE id = ?
 	`
 
@@ -176,9 +173,9 @@ func (r *BlobRepository) UpdateUploaded(
 
 	var err error
 	if tx != nil {
-		_, err = tx.ExecContext(ctx, query, now, storedSize, id)
+		_, err = tx.ExecContext(ctx, query, now, id)
 	} else {
-		_, err = r.db.ExecWithLog(ctx, query, now, storedSize, id)
+		_, err = r.db.ExecWithLog(ctx, query, now, id)
 	}
 
 	if err != nil {

internal/database/blobs_test.go

@@ -122,8 +122,8 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 		t.Errorf("expected compressed size 500, got %d", updated.CompressedSize)
 	}
 
-	// Test UpdateUploaded, with a stored object larger than the local copy
-	err = repo.UpdateUploaded(ctx, nil, blob.ID.String(), 600)
+	// Test UpdateUploaded
+	err = repo.UpdateUploaded(ctx, nil, blob.ID.String())
 	if err != nil {
 		t.Fatalf("failed to update blob as uploaded: %v", err)
 	}
@@ -141,10 +141,6 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 	if uploaded.UploadedTS.Before(now.Add(-1 * time.Second)) {
 		t.Error("uploaded timestamp should be around test time")
 	}
-
-	if uploaded.CompressedSize != 600 {
-		t.Errorf("expected the stored size 600, got %d", uploaded.CompressedSize)
-	}
 }
 
 func TestBlobRepositoryDuplicate(t *testing.T) {

internal/database/repositories.go

@@ -3,6 +3,7 @@ package database
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"fmt"
 )
 
@@ -60,22 +61,24 @@ func (r *Repositories) WithTx(ctx context.Context, fn TxFunc) error {
 
 	defer func() {
 		if p := recover(); p != nil {
-			rollbackErr := tx.Rollback()
-			if rollbackErr != nil {
-				Fatalf("failed to rollback transaction: %v", rollbackErr)
-			}
+			// The panic is the failure to report, so a rollback error
+			// is dropped.
+			_ = tx.Rollback()
 
 			panic(p)
-		} else if err != nil {
-			rollbackErr := tx.Rollback()
-			if rollbackErr != nil {
-				Fatalf("failed to rollback transaction: %v", rollbackErr)
-			}
 		}
 	}()
 
 	err = fn(ctx, tx)
 	if err != nil {
+		// Once ctx is cancelled, database/sql rolls the transaction
+		// back itself and Rollback returns sql.ErrTxDone.
+		rollbackErr := tx.Rollback()
+		if rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) {
+			return fmt.Errorf("%w; rolling back transaction: %w",
+				err, rollbackErr)
+		}
+
 		return err
 	}
 

internal/database/repositories_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"strings"
 	"testing"
 	"time"
 
@@ -235,6 +236,121 @@ func TestRepositoriesTransactionRollback(t *testing.T) {
 	}
 }
 
+// When the context is cancelled during the body, database/sql rolls the
+// transaction back itself, so WithTx's own rollback finds it already
+// done. WithTx must still return the body's error.
+func TestRepositoriesTransactionContextCancelled(t *testing.T) {
+	t.Parallel()
+
+	db, cleanup := setupTestDB(t)
+	defer cleanup()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	repos := database.NewRepositories(db)
+
+	err := repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
+		file := &database.File{
+			Path:  "/test/cancelled_file.txt",
+			MTime: time.Now().Truncate(time.Second),
+			Size:  1024,
+			Mode:  0644,
+			UID:   1000,
+			GID:   1000,
+		}
+
+		err := repos.Files.Create(ctx, tx, file)
+		if err != nil {
+			return err
+		}
+
+		cancel()
+
+		// database/sql rolls back from a goroutine of its own; wait
+		// until it has, so the test does not depend on that race.
+		for {
+			_, err = tx.ExecContext(context.WithoutCancel(ctx), "SELECT 1")
+			if errors.Is(err, sql.ErrTxDone) {
+				return ctx.Err()
+			}
+
+			time.Sleep(time.Millisecond)
+		}
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected context.Canceled, got: %v", err)
+	}
+
+	file, err := repos.Files.GetByPath(
+		context.Background(), "/test/cancelled_file.txt",
+	)
+	if err != nil {
+		t.Fatalf("error checking for file: %v", err)
+	}
+
+	if file != nil {
+		t.Error("file should not exist after the cancelled transaction")
+	}
+}
+
+// A body that ends the transaction itself with ROLLBACK makes WithTx's
+// own rollback fail. WithTx must return that failure together with the
+// body's error.
+func TestRepositoriesTransactionRollbackFails(t *testing.T) {
+	t.Parallel()
+
+	db, cleanup := setupTestDB(t)
+	defer cleanup()
+
+	ctx := context.Background()
+	repos := database.NewRepositories(db)
+
+	err := repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
+		_, err := tx.ExecContext(ctx, "ROLLBACK")
+		if err != nil {
+			return err
+		}
+
+		return errIntentionalRollback
+	})
+	if !errors.Is(err, errIntentionalRollback) {
+		t.Fatalf("expected the body's error, got: %v", err)
+	}
+
+	if !strings.Contains(err.Error(), "rolling back transaction") {
+		t.Errorf("expected the failed rollback in the error, got: %v", err)
+	}
+}
+
+// When the body ends the transaction itself with ROLLBACK and then
+// panics, WithTx's own rollback fails; the panic must still reach the
+// caller.
+func TestRepositoriesTransactionPanicRollbackFails(t *testing.T) {
+	t.Parallel()
+
+	db, cleanup := setupTestDB(t)
+	defer cleanup()
+
+	ctx := context.Background()
+	repos := database.NewRepositories(db)
+
+	defer func() {
+		if recover() == nil {
+			t.Error("expected the body's panic to reach the caller")
+		}
+	}()
+
+	_ = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
+		_, err := tx.ExecContext(ctx, "ROLLBACK")
+		if err != nil {
+			return err
+		}
+
+		panic(errIntentionalRollback)
+	})
+}
+
 func TestRepositoriesReadTransaction(t *testing.T) {
 	t.Parallel()
 

internal/snapshot/scanner.go

@@ -1509,8 +1509,7 @@ func (s *Scanner) finalizeProcessPhase(ctx context.Context, result *ScanResult)
 			// snapshot references has uploaded_ts set, so deduplication and
 			// interrupted-run repair treat these blobs as trustworthy.
 			err = s.repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
-				err := s.repos.Blobs.UpdateUploaded(
-					ctx, tx, b.ID, b.Compressed)
+				err := s.repos.Blobs.UpdateUploaded(ctx, tx, b.ID)
 				if err != nil {
 					return fmt.Errorf("marking blob uploaded: %w", err)
 				}
@@ -1546,7 +1545,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(
+	blobExists, err := s.uploadBlobIfNeeded(
 		ctx, blobPath, blobWithReader, startTime, result)
 	if err != nil {
 		s.cleanupBlobTempFile(blobWithReader)
@@ -1554,8 +1553,7 @@ func (s *Scanner) handleBlobReady(
 		return fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err)
 	}
 
-	err = s.recordBlobMetadata(
-		ctx, finishedBlob, storedSize, blobExists, startTime)
+	err = s.recordBlobMetadata(ctx, finishedBlob, blobExists, startTime)
 	if err != nil {
 		s.cleanupBlobTempFile(blobWithReader)
 
@@ -1577,22 +1575,20 @@ func (s *Scanner) handleBlobReady(
 }
 
 // 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.
+// exist, returns whether it existed
 func (s *Scanner) uploadBlobIfNeeded(
 	ctx context.Context,
 	blobPath string,
 	blobWithReader *blob.WithReader,
 	startTime time.Time,
 	result *ScanResult,
-) (int64, bool, error) {
+) (bool, error) {
 	finishedBlob := blobWithReader.FinishedBlob
 
 	// Check if blob already exists (deduplication after restart)
 	destination := s.storage.Info().Location
 
-	stored, err := s.storage.Stat(ctx, blobPath)
+	_, err := s.storage.Stat(ctx, blobPath)
 	if err == nil {
 		log.Info("Blob already exists in storage, skipping upload",
 			"hash", finishedBlob.Hash,
@@ -1601,7 +1597,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 			s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
 			s.ui.Path(destination))
 
-		return stored.Size, true, nil
+		return true, nil
 	}
 
 	s.ui.Beginf("Uploading blob %s (%s) to %s.",
@@ -1615,7 +1611,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 	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 false, fmt.Errorf("uploading blob to storage: %w", err)
 	}
 
 	uploadDuration := time.Since(startTime)
@@ -1646,7 +1642,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 		stats.BytesUploaded.Add(finishedBlob.Compressed)
 	}
 
-	return finishedBlob.Compressed, false, nil
+	return false, nil
 }
 
 // makeUploadProgressCallback creates a progress callback for blob uploads.
@@ -1716,7 +1712,6 @@ func (s *Scanner) makeUploadProgressCallback(
 func (s *Scanner) recordBlobMetadata(
 	ctx context.Context,
 	finishedBlob *blob.FinishedBlob,
-	storedSize int64,
 	blobExists bool,
 	startTime time.Time,
 ) error {
@@ -1728,8 +1723,7 @@ func (s *Scanner) recordBlobMetadata(
 	uploadDuration := time.Since(startTime)
 
 	return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error {
-		err := s.repos.Blobs.UpdateUploaded(
-			txCtx, tx, finishedBlob.ID, storedSize)
+		err := s.repos.Blobs.UpdateUploaded(txCtx, tx, finishedBlob.ID)
 		if err != nil {
 			return fmt.Errorf("updating blob upload timestamp: %w", err)
 		}

internal/vaultik/reused_blob_verify_test.go

@@ -1,83 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"io"
-	"path/filepath"
-	"testing"
-
-	"filippo.io/age"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"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) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-
-	dataDir := filepath.Join(tempDir, "source")
-	storeDir := filepath.Join(tempDir, "remote")
-	secondDBPath := filepath.Join(tempDir, "second.sqlite")
-
-	chunkSize := int64(64 * 1024)
-	maxBlobSize := int64(512 * 1024)
-
-	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
-	require.NoError(t, afero.WriteFile(fs,
-		filepath.Join(dataDir, "data.bin"),
-		bytesPattern("reuse-", int(chunkSize*3)), 0o644))
-
-	ctx := context.Background()
-
-	runFileStorageBackup(ctx, t, fs, dataDir, storeDir,
-		filepath.Join(tempDir, "first.sqlite"), chunkSize, maxBlobSize)
-
-	env := setupDedupBackupEnv(
-		ctx, t, fs, storeDir, secondDBPath, chunkSize, maxBlobSize)
-
-	defer func() { _ = env.db.Close() }()
-
-	extra, err := age.GenerateX25519Identity()
-	require.NoError(t, err)
-
-	env.cfg.AgeRecipients = append(env.cfg.AgeRecipients,
-		extra.Recipient().String())
-
-	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.NoError(t, env.db.Close())
-
-	verifier := &vaultik.Vaultik{
-		Config:  env.cfg,
-		Storage: env.storer,
-		Fs:      fs,
-		Stdout:  io.Discard,
-		Stderr:  io.Discard,
-		UI:      ui.NewWithColor(io.Discard, false),
-	}
-	verifier.SetContext(ctx)
-
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID, &vaultik.VerifyOptions{}),
-		"shallow verify should pass")
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID,
-			&vaultik.VerifyOptions{Deep: true}),
-		"deep verify should pass")
-}