Compare commits

..
Author SHA1 Message Date
sneak 2c6af49bb6 Return errors instead of exiting on rollback, rows close and blob temp file cleanup (closes #345)
check / check (push) Waiting to run
WithReadTx ended the process when its rollback failed, including after
Ctrl-C, when database/sql has already rolled the transaction back and
Rollback returns sql.ErrTxDone. It now handles the rollback as WithTx
does. The query functions ended the process when closing their rows
failed; they now ignore the deferred close, because database/sql reports
a close failure through rows.Err(), which each returns, once the rows
are read to the end. The unused database.Fatalf is removed.
cleanupBlobTempFile returns its close or remove failure and the upload
step returns it, so such a failure now fails the backup.

No test makes a rows close fail: SQLite fails it only with the error the
last read already returned.

Model: opus-5-5
2026-10-13 14:42:31 +00:00
26 changed files with 235 additions and 570 deletions
+4 -4
View File
@@ -293,8 +293,8 @@ needed.
**`snapshot list`**: Show every snapshot known to this host — the union
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).
uncompressed chunk size; size of chunks newly referenced by that
snapshot).
Listing the destination store does **not** require the age secret key,
so it works in vaultik's intended configuration, where the backed-up
@@ -307,8 +307,8 @@ only the hash, so hostname and snapshot name exist solely in the local
index and in the encrypted per-snapshot database. Snapshots found only
on the destination store are therefore listed as
`<remote only:<abbreviated remote key>>` and show `<remote only>` in
the uncompressed and "new chunk" columns, which come only from the
local index. Their timestamp and compressed size are real,
the uncompressed and "new chunk" columns, which can only be computed
from the local index. Their timestamp and compressed size are real,
read from the manifest.
Snapshots in the local index with no counterpart on the destination
+2 -3
View File
@@ -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.
@@ -119,7 +119,6 @@ Tracks backup snapshots.
- `completed_at` (INTEGER) - Completion timestamp (NULL if in progress)
- `file_count` (INTEGER) - Number of files in snapshot
- `chunk_count` (INTEGER) - Number of chunks this snapshot stored that were not stored before
- `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
- `blob_size` (INTEGER) - Total compressed size of all referenced blobs
@@ -184,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
+1 -6
View File
@@ -58,12 +58,7 @@ func (r *BlobChunkRepository) GetByBlobID(
return nil, fmt.Errorf("querying blob chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var blobChunks []*BlobChunk
+7 -15
View File
@@ -83,12 +83,7 @@ func (r *BlobRepository) GetAll(ctx context.Context) (map[string]*Blob, error) {
return nil, fmt.Errorf("querying blobs: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
out := make(map[string]*Blob)
@@ -159,16 +154,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 +168,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 {
+2 -6
View File
@@ -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) {
+3 -18
View File
@@ -61,12 +61,7 @@ func (r *ChunkFileRepository) GetByChunkHash(
return nil, fmt.Errorf("querying chunk files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanChunkFiles(rows)
}
@@ -87,12 +82,7 @@ func (r *ChunkFileRepository) GetByFilePath(
return nil, fmt.Errorf("querying chunk files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanChunkFiles(rows)
}
@@ -112,12 +102,7 @@ func (r *ChunkFileRepository) GetByFileID(
return nil, fmt.Errorf("querying chunk files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanChunkFiles(rows)
}
+2 -12
View File
@@ -107,12 +107,7 @@ func (r *ChunkRepository) GetByHashes(
return nil, fmt.Errorf("querying chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var chunks []*Chunk
@@ -152,12 +147,7 @@ func (r *ChunkRepository) ListUnpacked(
return nil, fmt.Errorf("querying unpacked chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var chunks []*Chunk
+1 -6
View File
@@ -38,12 +38,7 @@ func (r *ChunkRepository) list(ctx context.Context, query string) ([]*Chunk, err
return nil, fmt.Errorf("querying chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var chunks []*Chunk
-12
View File
@@ -1,12 +0,0 @@
package database
import (
"fmt"
"os"
)
// Fatalf prints an error message to stderr and exits with status 1
func Fatalf(format string, args ...any) {
fmt.Fprintf(os.Stderr, "FATAL: "+format+"\n", args...)
os.Exit(1)
}
+3 -18
View File
@@ -62,12 +62,7 @@ func (r *FileChunkRepository) GetByPath(
return nil, fmt.Errorf("querying file chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanFileChunks(rows)
}
@@ -88,12 +83,7 @@ func (r *FileChunkRepository) GetByFileID(
return nil, fmt.Errorf("querying file chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanFileChunks(rows)
}
@@ -117,12 +107,7 @@ func (r *FileChunkRepository) GetByPathTx(
return nil, fmt.Errorf("querying file chunks: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
fileChunks, err := r.scanFileChunks(rows)
LogSQL("GetByPathTx", "Complete", path, "count", len(fileChunks))
+4 -24
View File
@@ -175,12 +175,7 @@ func (r *FileRepository) ListModifiedSince(
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var files []*File
@@ -256,12 +251,7 @@ func (r *FileRepository) ListUnderPath(
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var files []*File
@@ -304,12 +294,7 @@ func (r *FileRepository) ListIDsWithChunksNotInUploadedBlobs(
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var ids []types.FileID
@@ -340,12 +325,7 @@ func (r *FileRepository) ListAll(ctx context.Context) ([]*File, error) {
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var files []*File
+11 -9
View File
@@ -106,22 +106,24 @@ func (r *Repositories) WithReadTx(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
}
+38
View File
@@ -411,3 +411,41 @@ func TestRepositoriesReadTransaction(t *testing.T) {
t.Error("expected to retrieve file in read transaction")
}
}
// When the context is cancelled during a read transaction's body,
// database/sql rolls the transaction back itself, so WithReadTx's own
// rollback finds it already done. WithReadTx must return the body's error
// alone, without that rollback.
func TestRepositoriesReadTransactionContextCancelled(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.WithReadTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
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)
}
if errors.Is(err, sql.ErrTxDone) {
t.Fatalf("the already-done rollback was reported: %v", err)
}
}
-1
View File
@@ -86,7 +86,6 @@ CREATE TABLE IF NOT EXISTS snapshots (
completed_at INTEGER,
file_count INTEGER NOT NULL DEFAULT 0,
chunk_count INTEGER NOT NULL DEFAULT 0,
new_chunk_size INTEGER NOT NULL DEFAULT 0,
blob_count INTEGER NOT NULL DEFAULT 0,
total_size INTEGER NOT NULL DEFAULT 0,
blob_size INTEGER NOT NULL DEFAULT 0,
+38 -44
View File
@@ -129,7 +129,6 @@ func (r *SnapshotRepository) UpdateExtendedStats(
compressionLevel int,
uploadBytes int64,
uploadDurationMs int64,
newChunkSize int64,
) error {
compressionRatio, err := r.extendedCompressionRatio(
ctx, tx, snapshotID, blobUncompressedSize,
@@ -144,19 +143,18 @@ func (r *SnapshotRepository) UpdateExtendedStats(
compression_ratio = ?,
compression_level = ?,
upload_bytes = ?,
upload_duration_ms = ?,
new_chunk_size = ?
upload_duration_ms = ?
WHERE id = ?
`
if tx != nil {
_, err = tx.ExecContext(ctx, query,
blobUncompressedSize, compressionRatio, compressionLevel,
uploadBytes, uploadDurationMs, newChunkSize, snapshotID)
uploadBytes, uploadDurationMs, snapshotID)
} else {
_, err = r.db.ExecWithLog(ctx, query,
blobUncompressedSize, compressionRatio, compressionLevel,
uploadBytes, uploadDurationMs, newChunkSize, snapshotID)
uploadBytes, uploadDurationMs, snapshotID)
}
if err != nil {
@@ -241,12 +239,7 @@ func (r *SnapshotRepository) GetOnlySnapshot(ctx context.Context) (*Snapshot, er
return nil, fmt.Errorf("querying snapshots: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
snapshots, err := r.scanSnapshotRows(rows)
if err != nil {
@@ -281,12 +274,7 @@ func (r *SnapshotRepository) ListRecent(
return nil, fmt.Errorf("querying snapshots: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanSnapshotRows(rows)
}
@@ -501,12 +489,7 @@ func (r *SnapshotRepository) GetBlobHashes(
return nil, fmt.Errorf("querying blob hashes: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
var blobs []string
@@ -596,24 +579,45 @@ func (r *SnapshotRepository) GetSnapshotUncompressedChunkSize(
return totalSize, nil
}
// GetSnapshotNewChunkSize returns the total plaintext size of the chunks the
// snapshot's backup stored that were not stored before, as the backup
// recorded it. It is not derived from the snapshot's files: a file's row is
// shared by every snapshot that lists it and is rewritten when the file
// changes, so it shows only the file's latest chunks.
// GetSnapshotNewChunkSize returns the sum of plaintext sizes of chunks that are
// referenced by this snapshot but not by any earlier completed snapshot known to
// the local database. The result is the marginal uncompressed data this snapshot
// added to the dedup pool — i.e., the delta from prior snapshots.
func (r *SnapshotRepository) GetSnapshotNewChunkSize(
ctx context.Context, snapshotID string,
) (int64, error) {
query := `SELECT new_chunk_size FROM snapshots WHERE id = ?`
query := `
WITH this_snap_chunks AS (
SELECT DISTINCT fc.chunk_hash
FROM snapshot_files sf
JOIN file_chunks fc ON sf.file_id = fc.file_id
WHERE sf.snapshot_id = ?
),
prior_chunks AS (
SELECT DISTINCT fc.chunk_hash
FROM snapshots s
JOIN snapshot_files sf ON sf.snapshot_id = s.id
JOIN file_chunks fc ON fc.file_id = sf.file_id
WHERE s.completed_at IS NOT NULL
AND s.id != ?
AND s.started_at < (SELECT started_at FROM snapshots WHERE id = ?)
)
SELECT COALESCE(SUM(c.size), 0)
FROM chunks c
JOIN this_snap_chunks t ON c.chunk_hash = t.chunk_hash
WHERE c.chunk_hash NOT IN (SELECT chunk_hash FROM prior_chunks)
`
var newChunkSize int64
var totalSize int64
err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&newChunkSize)
err := r.db.conn.QueryRowContext(
ctx, query, snapshotID, snapshotID, snapshotID,
).Scan(&totalSize)
if err != nil {
return 0, fmt.Errorf("querying new chunk size: %w", err)
}
return newChunkSize, nil
return totalSize, nil
}
// GetIncompleteSnapshots returns all snapshots that haven't been completed
@@ -634,12 +638,7 @@ func (r *SnapshotRepository) GetIncompleteSnapshots(
return nil, fmt.Errorf("querying incomplete snapshots: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
return r.scanSnapshotRows(rows)
}
@@ -662,12 +661,7 @@ func (r *SnapshotRepository) GetIncompleteByHostname(
return nil, fmt.Errorf("querying incomplete snapshots: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
defer func() { _ = rows.Close() }()
// Same column set as every other multi-row snapshot query, so the
// shared scanner applies — including its timestamp normalization.
+62
View File
@@ -0,0 +1,62 @@
//nolint:testpackage // exercises the unexported cleanupBlobTempFile
package snapshot
import (
"errors"
"os"
"testing"
"github.com/spf13/afero"
"sneak.berlin/go/vaultik/internal/blob"
)
// A blob temp file that cannot be closed or removed must make the cleanup
// return an error, not end the process.
func TestCleanupBlobTempFileReturnsErrors(t *testing.T) {
t.Parallel()
fs := afero.NewOsFs()
s := &Scanner{fs: fs}
tests := []struct {
name string
// prepare runs before the cleanup and makes its close or remove
// fail.
prepare func(tempFile afero.File) error
want error
}{
{
name: "close fails",
prepare: func(tempFile afero.File) error { return tempFile.Close() },
want: os.ErrClosed,
},
{
name: "remove fails",
prepare: func(tempFile afero.File) error {
return fs.Remove(tempFile.Name())
},
want: os.ErrNotExist,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tempFile, err := afero.TempFile(fs, t.TempDir(), "blob-*.tmp")
if err != nil {
t.Fatal(err)
}
err = tt.prepare(tempFile)
if err != nil {
t.Fatal(err)
}
err = s.cleanupBlobTempFile(&blob.WithReader{TempFile: tempFile})
if !errors.Is(err, tt.want) {
t.Fatalf("expected an error wrapping %v, got: %v", tt.want, err)
}
})
}
}
+28 -32
View File
@@ -135,9 +135,7 @@ type ScannerConfig struct {
// are counted per file: BytesScanned is the size of the new and changed
// files, BytesSkipped that of the unchanged ones. FilesFailed counts the
// new and changed files that phase 2 could not store; FilesScanned
// includes them and BytesScanned does not. ChunksCreated counts the
// chunks that were not already stored, and NewChunkSize is their total
// size.
// includes them and BytesScanned does not.
type ScanResult struct {
FilesScanned int
FilesSkipped int
@@ -147,7 +145,6 @@ type ScanResult struct {
BytesSkipped int64
BytesDeleted int64
ChunksCreated int
NewChunkSize int64
BlobsCreated int
BlobsUploaded int
BytesUploaded int64
@@ -1517,8 +1514,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)
}
@@ -1554,24 +1550,27 @@ 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)
cleanupErr := s.cleanupBlobTempFile(blobWithReader)
return fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err)
return errors.Join(
fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err), cleanupErr)
}
err = s.recordBlobMetadata(
ctx, finishedBlob, storedSize, blobExists, startTime)
err = s.recordBlobMetadata(ctx, finishedBlob, blobExists, startTime)
if err != nil {
s.cleanupBlobTempFile(blobWithReader)
cleanupErr := s.cleanupBlobTempFile(blobWithReader)
return errors.Join(err, cleanupErr)
}
err = s.cleanupBlobTempFile(blobWithReader)
if err != nil {
return err
}
s.cleanupBlobTempFile(blobWithReader)
// Chunks from this blob are now committed to DB - remove from pending set
s.removePendingChunkHashes(blobWithReader.InsertedChunkHashes)
@@ -1585,22 +1584,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,
@@ -1609,7 +1606,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.",
@@ -1623,7 +1620,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)
@@ -1654,7 +1651,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.
@@ -1724,7 +1721,6 @@ func (s *Scanner) makeUploadProgressCallback(
func (s *Scanner) recordBlobMetadata(
ctx context.Context,
finishedBlob *blob.FinishedBlob,
storedSize int64,
blobExists bool,
startTime time.Time,
) error {
@@ -1736,8 +1732,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)
}
@@ -1768,20 +1763,22 @@ func (s *Scanner) recordBlobMetadata(
}
// cleanupBlobTempFile closes and removes the blob's temporary file
func (s *Scanner) cleanupBlobTempFile(blobWithReader *blob.WithReader) {
func (s *Scanner) cleanupBlobTempFile(blobWithReader *blob.WithReader) error {
if blobWithReader.TempFile != nil {
tempName := blobWithReader.TempFile.Name()
err := blobWithReader.TempFile.Close()
if err != nil {
log.Fatal("Failed to close temp file", "file", tempName, "error", err)
return fmt.Errorf("closing blob temp file: %w", err)
}
err = s.fs.Remove(tempName)
if err != nil {
log.Fatal("Failed to remove temp file", "file", tempName, "error", err)
return fmt.Errorf("removing blob temp file: %w", err)
}
}
return nil
}
// streamingChunkInfo tracks chunk metadata collected during streaming
@@ -1875,12 +1872,11 @@ func (s *Scanner) processFileStreaming(
return nil
}
// updateChunkStats counts a chunk that was not already stored, and its
// size. The scan result's file counts, BytesScanned and BytesSkipped are
// not touched here: the scan phase counts each file once.
// updateChunkStats counts a chunk that was not already stored. The scan
// result's file counts, BytesScanned and BytesSkipped are not touched
// here: the scan phase counts each file once.
func (s *Scanner) updateChunkStats(chunkSize int64, result *ScanResult) {
result.ChunksCreated++
result.NewChunkSize += chunkSize
if s.progress != nil {
s.progress.GetStats().ChunksCreated.Add(1)
-2
View File
@@ -199,7 +199,6 @@ func (sm *SnapshotManager) UpdateSnapshotStatsExtended(
stats.CompressionLevel,
stats.BytesUploaded,
stats.UploadDurationMs,
stats.NewChunkSize,
)
})
}
@@ -907,7 +906,6 @@ type ExtendedBackupStats struct {
BlobUncompressedSize int64 // Total uncompressed size of all referenced blobs
CompressionLevel int // Compression level used for this snapshot
UploadDurationMs int64 // Total milliseconds spent uploading to S3
NewChunkSize int64 // Total size of the chunks counted in ChunksCreated
}
// deleteOtherSnapshots deletes all snapshots except the current one
-133
View File
@@ -1,133 +0,0 @@
package vaultik_test
import (
"bytes"
"context"
"maps"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/chunker"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestNewChunkSizeOfChangedFileStaysFixed covers
// https://git.eeqj.de/sneak/vaultik/issues/311. A backup whose only change
// is a modified file lists the size of the chunks that change added as its
// NEW CHUNK SIZE, and a later backup that changes the file again leaves
// every earlier snapshot's figure as it was. The file's row in the local
// index is shared by all three snapshots and rewritten by each backup.
func TestNewChunkSizeOfChangedFileStaysFixed(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"))
firstFiles := maps.Clone(files)
firstID := localSnapshotID(ctx, t, repos, "first")
appendRandomBytes(t, fs, files, changedPath, faultChunkSize)
require.NoError(t, backUp(v, "second"))
secondID := localSnapshotID(ctx, t, repos, "second")
secondAdded := sizeOfNewChunks(t, files, firstFiles)
require.Positive(t, secondAdded)
assert.Equal(t, secondAdded, listedNewChunkSizes(t, v)[secondID],
"the second snapshot must count the chunks the change added")
appendRandomBytes(t, fs, files, changedPath, faultChunkSize)
require.NoError(t, backUp(v, "third"))
sizes := listedNewChunkSizes(t, v)
assert.Equal(t, sizeOfNewChunks(t, firstFiles, nil), sizes[firstID],
"a later backup must not change the first snapshot's figure")
assert.Equal(t, secondAdded, sizes[secondID],
"a later backup must not change the second snapshot's figure")
}
// sizeOfNewChunks returns the total size of the distinct chunks of the
// files in after that no file in before has, cut the way the backup cuts
// them.
func sizeOfNewChunks(t *testing.T, after, before map[string][]byte) int64 {
t.Helper()
known := make(map[string]bool)
for _, chunk := range chunksOf(t, before) {
known[chunk.Hash] = true
}
var total int64
for _, chunk := range chunksOf(t, after) {
if !known[chunk.Hash] {
known[chunk.Hash] = true
total += chunk.Size
}
}
return total
}
// chunksOf returns the chunks of every file in files.
func chunksOf(t *testing.T, files map[string][]byte) []chunker.Chunk {
t.Helper()
c := chunker.NewChunker(faultChunkSize)
var all []chunker.Chunk
for _, content := range files {
chunks, err := c.ChunkReader(bytes.NewReader(content))
require.NoError(t, err)
all = append(all, chunks...)
}
return all
}
// listedNewChunkSizes runs `snapshot list --json` and returns each listed
// snapshot's new_chunk_size by snapshot ID.
func listedNewChunkSizes(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.NewChunkSize
}
return sizes
}
+1 -12
View File
@@ -309,22 +309,11 @@ func (v *Vaultik) restoreAllFiles(
return nil, err
}
plan, unplanned, err := newRestorePlan(
v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
plan, err := newRestorePlan(v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
if err != nil {
return nil, fmt.Errorf("building restore plan: %w", err)
}
// Ranging over files rather than the map reports them in a stable order.
for _, file := range files {
if planErr, ok := unplanned[file.ID]; ok {
err = v.handleRestoreFileError(plan, opts, result, file, file.ID, planErr)
if err != nil {
return nil, err
}
}
}
filesByID, totalBytesExpected := indexRestoreFiles(files)
v.UI.Beginf("Restoring %s files (%s) to %s.",
@@ -61,8 +61,8 @@ func craftedFile(
}
// TestRestoreShortChunkHashInNoBlob proves a file whose short chunk hash
// has no blob_chunks row is left out of the restore plan with an error,
// and fails the chunk write with an error.
// has no blob_chunks row fails restore planning and the chunk write with
// an error.
func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
t.Parallel()
@@ -77,11 +77,9 @@ func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
chunkToBlobMap, err := v.buildChunkToBlobMap(ctx, repos)
require.NoError(t, err)
plan, unplanned, err := newRestorePlan(ctx, repos, []*database.File{file},
_, err = newRestorePlan(ctx, repos, []*database.File{file},
chunkToBlobMap, map[string]string{})
require.NoError(t, err)
require.ErrorIs(t, unplanned[file.ID], errPlanChunkMissing)
require.False(t, plan.hasPending())
require.ErrorIs(t, err, errPlanChunkMissing)
fileChunks, err := repos.FileChunks.GetByFileID(ctx, file.ID)
require.NoError(t, err)
+8 -17
View File
@@ -41,25 +41,20 @@ type restorePlan struct {
// newRestorePlan builds the file→blob index for the given files. Files
// whose chunks reference no blobs (symlinks, directories) start in the
// ready queue immediately. A file with a chunk that no blob holds cannot
// be restored: it is left out of the plan and returned, by ID, with the
// reason.
// ready queue immediately.
func newRestorePlan(
ctx context.Context,
repos *database.Repositories,
files []*database.File,
chunkToBlobMap map[string]*database.BlobChunk,
blobIDToHash map[string]string,
) (*restorePlan, map[types.FileID]error, error) {
) (*restorePlan, error) {
p := &restorePlan{
fileBlobs: make(map[types.FileID]map[string]struct{}, len(files)),
blobFiles: make(map[string]map[types.FileID]struct{}),
ready: make([]types.FileID, 0, len(files)),
cached: make(map[string]struct{}),
}
unplanned := make(map[types.FileID]error)
nextFile:
for _, f := range files {
if f.IsSymlink() || f.Mode&uint32(os.ModeDir) != 0 {
// No chunks to fetch — restore can run immediately.
@@ -71,7 +66,7 @@ nextFile:
fileChunks, err := repos.FileChunks.GetByFileID(ctx, f.ID)
if err != nil {
return nil, nil, fmt.Errorf("planning %s: %w", f.Path, err)
return nil, fmt.Errorf("planning %s: %w", f.Path, err)
}
blobs := make(map[string]struct{})
@@ -79,18 +74,14 @@ nextFile:
for _, fc := range fileChunks {
bc, ok := chunkToBlobMap[fc.ChunkHash.String()]
if !ok {
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
continue nextFile
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
}
hash, ok := blobIDToHash[bc.BlobID.String()]
if !ok {
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanBlobIDMissing, bc.BlobID)
continue nextFile
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanBlobIDMissing, bc.BlobID)
}
blobs[hash] = struct{}{}
@@ -112,7 +103,7 @@ nextFile:
}
}
return p, unplanned, nil
return p, nil
}
// markBlobCached records that the named blob is now resident in the
+16 -103
View File
@@ -28,8 +28,7 @@ const (
)
// missingBlobBackup is a snapshot of single-chunk files spread over two
// blobs, with one of those blobs lost: deleted from the store, or missing
// from the snapshot metadata.
// blobs, with one of those blobs deleted from the store.
type missingBlobBackup struct {
fs afero.Fs
cfg *config.Config
@@ -38,39 +37,20 @@ type missingBlobBackup struct {
restoreDir string
// files holds the original content by source path.
files map[string][]byte
// lost holds the source paths whose chunk is in the lost blob.
// lost holds the source paths whose chunk is in the deleted blob.
lost map[string]bool
}
// TestRestoreSkipErrorsSkipsFilesOfMissingBlob restores with SkipErrors
// after one blob of a two-blob snapshot was deleted from the store.
// after one blob of a two-blob snapshot was deleted. Every file stored in
// that blob must be reported as failed and left absent, every other file
// must be restored intact, and Restore must still return an error.
func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
assertRestoreSkipsLostFiles(ctx, t, backupThenDeleteOneBlob(ctx, t))
}
// TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob restores with
// SkipErrors a snapshot whose metadata lists chunks that no blob holds, as
// earlier backup defects left behind.
func TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
assertRestoreSkipsLostFiles(ctx, t, backupThenDropOneBlobFromMetadata(ctx, t))
}
// assertRestoreSkipsLostFiles restores backup with SkipErrors. Every file
// stored in the lost blob must be reported as failed and left absent,
// every other file must be restored intact, and Restore must still return
// an error.
func assertRestoreSkipsLostFiles(
ctx context.Context, t *testing.T, backup *missingBlobBackup,
) {
t.Helper()
backup := backupThenDeleteOneBlob(ctx, t)
var out bytes.Buffer
@@ -92,14 +72,14 @@ func assertRestoreSkipsLostFiles(
if backup.lost[path] {
assert.Containsf(t, out.String(), path,
"%s needs the lost blob and must be reported", path)
"%s needs the deleted blob and must be reported", path)
assert.NoFileExists(t, restored)
continue
}
got, err := afero.ReadFile(backup.fs, restored)
require.NoErrorf(t, err, "%s does not need the lost blob", path)
require.NoErrorf(t, err, "%s does not need the deleted blob", path)
assert.Equalf(t, content, got, "%s restored with wrong content", path)
}
}
@@ -123,30 +103,6 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
require.ErrorIs(t, err, storage.ErrNotFound)
}
// TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held
// by no blob still ends the restore, before any file is written, when
// SkipErrors is not set.
func TestRestoreChunkInNoBlobAbortsWithoutSkipErrors(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
backup := backupThenDropOneBlobFromMetadata(ctx, t)
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
err := v.Restore(&vaultik.RestoreOptions{
SnapshotID: backup.snapshotID,
TargetDir: backup.restoreDir,
})
require.ErrorContains(t, err, "chunk missing from blob map")
for path := range backup.files {
assert.NoFileExists(t, filepath.Join(backup.restoreDir, path))
}
}
// backupThenDeleteOneBlob backs up missingBlobFileCount single-chunk
// files, then deletes from the store the blob holding the first of them.
func backupThenDeleteOneBlob(
@@ -154,54 +110,6 @@ func backupThenDeleteOneBlob(
) *missingBlobBackup {
t.Helper()
backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
require.NoError(t, db.Close())
require.NoError(t, backup.storer.Delete(ctx, fmt.Sprintf(
"blobs/%s/%s/%s", lostBlob[:2], lostBlob[2:4], lostBlob)))
return backup
}
// backupThenDropOneBlobFromMetadata backs up missingBlobFileCount
// single-chunk files, then removes the blob holding the first of them from
// the snapshot and exports the snapshot metadata again. The exported
// metadata then lists chunks that no blob holds.
func backupThenDropOneBlobFromMetadata(
ctx context.Context, t *testing.T,
) *missingBlobBackup {
t.Helper()
backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
// The uploads row refers to the blob row, so it would keep the export
// from deleting it.
for _, query := range []string{
"DELETE FROM snapshot_blobs WHERE blob_hash = ?",
"DELETE FROM uploads WHERE blob_hash = ?",
} {
_, err := db.Conn().ExecContext(ctx, query, lostBlob)
require.NoError(t, err)
}
sm := newFaultSnapshotManager(
backup.fs, backup.storer, backup.cfg, database.NewRepositories(db))
require.NoError(t,
sm.ExportSnapshotMetadata(ctx, db.Path(), backup.snapshotID))
require.NoError(t, db.Close())
return backup
}
// backupSingleChunkFiles backs up missingBlobFileCount single-chunk files.
// It returns the backup, with lost naming the files stored in the same
// blob as the first of them, the still open local index, and the hash of
// that blob.
func backupSingleChunkFiles(
ctx context.Context, t *testing.T,
) (*missingBlobBackup, *database.DB, string) {
t.Helper()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
@@ -235,11 +143,16 @@ func backupSingleChunkFiles(
blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
}
lostBlob := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
require.NoError(t, db.Close())
deleted := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
require.NoError(t, storer.Delete(ctx, fmt.Sprintf(
"blobs/%s/%s/%s", deleted[:2], deleted[2:4], deleted)))
lost := make(map[string]bool)
for path, hash := range blobOfFile {
if hash == lostBlob {
if hash == deleted {
lost[path] = true
}
}
@@ -255,7 +168,7 @@ func backupSingleChunkFiles(
restoreDir: filepath.Join(tempDir, "restored"),
files: files,
lost: lost,
}, db, lostBlob
}
}
// blobHashOfSingleChunkFile returns the hash of the blob holding the one
@@ -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")
}
-3
View File
@@ -209,7 +209,6 @@ type snapshotStats struct {
totalFiles int
totalBytes int64
totalChunks int
newChunkSize int64
totalBlobs int
totalBytesSkipped int64
totalFilesSkipped int
@@ -342,7 +341,6 @@ func (v *Vaultik) scanAllDirectories(
stats.totalFiles += result.FilesScanned
stats.totalBytes += result.BytesScanned
stats.totalChunks += result.ChunksCreated
stats.newChunkSize += result.NewChunkSize
stats.totalBlobs += result.BlobsCreated
stats.totalFilesSkipped += result.FilesSkipped
stats.totalFilesFailed += result.FilesFailed
@@ -405,7 +403,6 @@ func (v *Vaultik) finalizeSnapshotMetadata(
BlobUncompressedSize: stats.blobUncompressedSize,
CompressionLevel: v.Config.CompressionLevel,
UploadDurationMs: stats.uploadDuration.Milliseconds(),
NewChunkSize: stats.newChunkSize,
}
err = v.SnapshotManager.UpdateSnapshotStatsExtended(v.ctx, snapshotID, extStats)
-1
View File
@@ -487,7 +487,6 @@ type listJSONRow struct {
RemoteKey string `json:"remote_key"`
Timestamp string `json:"timestamp"`
CompressedSize int64 `json:"compressed_size"`
NewChunkSize int64 `json:"new_chunk_size"`
LocallyTracked bool `json:"locally_tracked"`
RemotePresent *bool `json:"remote_present"`
}