Skip files of an unreadable blob under restore --skip-errors (closes #218)
check / check (push) Successful in 12m34s
check / check (push) Successful in 12m34s
A blob that failed to download ended `snapshot restore` even with --skip-errors, after restoring whichever files came first. The download error now goes through the same per-file handling as any other restore error, once for every pending file that references the blob. With --skip-errors those files are reported as failed, the rest are restored, and the command still exits non-zero. Without the flag the restore still aborts; the error now also names one affected file and suggests --skip-errors. A cancelled restore still ends at once. The --skip-errors help text and README now limit the packing and storage caveat to snapshot creation. Model: opus-5-5
This commit was merged in pull request #243.
This commit is contained in:
@@ -178,7 +178,7 @@ vaultik version
|
||||
* `--verbose`, `-v`: Enable verbose output (on stderr — see below)
|
||||
* `--debug`: Enable debug output (on stderr — see below)
|
||||
* `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
|
||||
* `--skip-errors`: Skip files that cannot be read when creating a snapshot, or that cannot be restored when restoring, instead of aborting. Packing and storage errors (which would leave a chunk recorded but not stored) still abort the run.
|
||||
* `--skip-errors`: Skip files that cannot be read when creating a snapshot, or that cannot be restored when restoring, instead of aborting. Packing and storage errors while creating a snapshot (which would leave a chunk recorded but not stored) still abort the run.
|
||||
|
||||
### locking
|
||||
|
||||
|
||||
@@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-06: Made `snapshot restore --skip-errors` skip the files that
|
||||
need a blob it cannot download
|
||||
([issue #218](https://git.eeqj.de/sneak/vaultik/issues/218)). A missing
|
||||
or damaged blob ended the restore even with the flag, after restoring
|
||||
whichever files happened to come first. Every file that needs such a
|
||||
blob is now reported as failed, the rest are restored, and the command
|
||||
still exits non-zero. Without the flag the blob error still aborts.
|
||||
|
||||
- 2026-10-06: Re-vendored the canonical files from `sneak/prompts` at
|
||||
`dd4027b` ([issue #213](https://git.eeqj.de/sneak/vaultik/issues/213)).
|
||||
Linting and testing are now the `lint` and `test` phases of the
|
||||
|
||||
@@ -60,7 +60,8 @@ on the source system.`,
|
||||
cmd.PersistentFlags().BoolVar(&rootFlags.SkipErrors, "skip-errors", false,
|
||||
"Skip files that cannot be read when creating a snapshot, or "+
|
||||
"that cannot be restored when restoring, instead of aborting "+
|
||||
"(packing and storage errors still abort)")
|
||||
"(packing and storage errors while creating a snapshot still "+
|
||||
"abort)")
|
||||
|
||||
// Add subcommands
|
||||
cmd.AddCommand(
|
||||
|
||||
@@ -355,7 +355,7 @@ func (v *Vaultik) runRestoreLoop(
|
||||
|
||||
fileID, ready := plan.popReady()
|
||||
if !ready {
|
||||
downloaded, err := session.downloadNextBlobSet(plan)
|
||||
downloaded, err := session.downloadNextBlobSet(plan, filesByID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -411,7 +411,13 @@ func (v *Vaultik) runRestoreLoop(
|
||||
// blob set and downloads its blobs; after each blob lands, the plan
|
||||
// moves any pending file whose set just emptied onto the ready queue.
|
||||
// Returns false when nothing is pending download (the caller stops).
|
||||
func (s *restoreSession) downloadNextBlobSet(plan *restorePlan) (bool, error) {
|
||||
//
|
||||
// A blob that cannot be downloaded is reported through
|
||||
// handleRestoreFileError for every pending file that references it, so
|
||||
// it aborts the restore unless --skip-errors is set.
|
||||
func (s *restoreSession) downloadNextBlobSet(
|
||||
plan *restorePlan, filesByID map[types.FileID]*database.File,
|
||||
) (bool, error) {
|
||||
s.sweeper.sweep()
|
||||
|
||||
next, ok := plan.pickNextDownload()
|
||||
@@ -433,7 +439,25 @@ func (s *restoreSession) downloadNextBlobSet(plan *restorePlan) (bool, error) {
|
||||
|
||||
err := s.downloadBlobToCache(hash, blob.CompressedSize, blob.UncompressedSize)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("downloading blob %s: %w", shortHash(hash), err)
|
||||
err = fmt.Errorf("downloading blob %s: %w", shortHash(hash), err)
|
||||
|
||||
// On cancel the error says nothing about the blob, so it ends
|
||||
// the restore instead of failing the files that need it.
|
||||
if s.ctx.Err() != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
for _, fileID := range plan.filesReferencingBlob(hash) {
|
||||
fileErr := s.v.handleRestoreFileError(
|
||||
plan, s.opts, s.result, filesByID[fileID], fileID, err)
|
||||
if fileErr != nil {
|
||||
return false, fileErr
|
||||
}
|
||||
}
|
||||
|
||||
// next is among the failed files, so the rest of its blob set
|
||||
// is left for any other file that still needs it.
|
||||
return true, nil
|
||||
}
|
||||
|
||||
s.result.BlobsDownloaded++
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package vaultik //nolint:testpackage // sets ctx/cancel and inspects scratch files
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"path/filepath"
|
||||
@@ -11,6 +12,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/vaultik/internal/log"
|
||||
"sneak.berlin/go/vaultik/internal/storage"
|
||||
@@ -157,3 +159,51 @@ func scratchEntries(t *testing.T, dir string) []string {
|
||||
|
||||
return matches
|
||||
}
|
||||
|
||||
// TestRestoreSkipErrorsCancelDuringBlobDownload cancels a SkipErrors
|
||||
// restore while a blob download is in progress. The download fails only
|
||||
// because of the cancel, so Restore must return context.Canceled without
|
||||
// reporting the file that needs the blob as failed.
|
||||
//
|
||||
//nolint:paralleltest // installs the global logger via log.Initialize
|
||||
func TestRestoreSkipErrorsCancelDuringBlobDownload(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
fs := afero.NewOsFs()
|
||||
tempDir := t.TempDir()
|
||||
|
||||
cfg, storer, snapshotID, srcPath := backupOneFile(context.Background(),
|
||||
t, fs, tempDir, "a.txt", []byte("hello vaultik"), 0o644)
|
||||
|
||||
gate := newBlockingBlobStorer(storer)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
var out bytes.Buffer
|
||||
|
||||
v := newRestoreVaultik(ctx, cfg, gate, fs)
|
||||
v.UI = ui.NewWithColor(&out, false)
|
||||
|
||||
restoreErr := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
restoreErr <- v.Restore(&RestoreOptions{
|
||||
SnapshotID: snapshotID,
|
||||
TargetDir: filepath.Join(tempDir, "restored"),
|
||||
SkipErrors: true,
|
||||
})
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-gate.entered:
|
||||
case <-time.After(30 * time.Second):
|
||||
t.Fatal("restore never reached the blob-download phase")
|
||||
}
|
||||
|
||||
cancel()
|
||||
|
||||
require.ErrorIs(t, <-restoreErr, context.Canceled)
|
||||
assert.NotContains(t, out.String(), srcPath,
|
||||
"the cancel was reported as a failed file")
|
||||
}
|
||||
|
||||
@@ -214,6 +214,20 @@ func (p *restorePlan) blobsNeeded(fileID types.FileID) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// filesReferencingBlob returns the pending files that reference the
|
||||
// named blob, in any order. The result is a copy, so the caller may
|
||||
// finishFile each of them while ranging over it.
|
||||
func (p *restorePlan) filesReferencingBlob(blobHash string) []types.FileID {
|
||||
files := p.blobFiles[blobHash]
|
||||
|
||||
out := make([]types.FileID, 0, len(files))
|
||||
for id := range files {
|
||||
out = append(out, id)
|
||||
}
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
// hasPending reports whether any unfinished files remain.
|
||||
func (p *restorePlan) hasPending() bool {
|
||||
return len(p.fileBlobs) > 0
|
||||
|
||||
@@ -0,0 +1,194 @@
|
||||
package vaultik_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"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/ui"
|
||||
"sneak.berlin/go/vaultik/internal/vaultik"
|
||||
)
|
||||
|
||||
// A file no larger than the chunker's minimum chunk size (a quarter of
|
||||
// faultChunkSize) is stored as one chunk, so it lives in exactly one blob.
|
||||
// More of them than fit in faultMaxBlobSize make the snapshot span two
|
||||
// blobs.
|
||||
const (
|
||||
missingBlobFileBytes = int(faultChunkSize / 4)
|
||||
missingBlobFileCount = 20
|
||||
)
|
||||
|
||||
// missingBlobBackup is a snapshot of single-chunk files spread over two
|
||||
// blobs, with one of those blobs deleted from the store.
|
||||
type missingBlobBackup struct {
|
||||
fs afero.Fs
|
||||
cfg *config.Config
|
||||
storer storage.Storer
|
||||
snapshotID string
|
||||
restoreDir string
|
||||
// files holds the original content by source path.
|
||||
files map[string][]byte
|
||||
// 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. 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.
|
||||
//
|
||||
//nolint:paralleltest // installs the global logger via log.Initialize
|
||||
func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
backup := backupThenDeleteOneBlob(ctx, t)
|
||||
|
||||
var out bytes.Buffer
|
||||
|
||||
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
|
||||
v.UI = ui.NewWithColor(&out, false)
|
||||
|
||||
err := v.Restore(&vaultik.RestoreOptions{
|
||||
SnapshotID: backup.snapshotID,
|
||||
TargetDir: backup.restoreDir,
|
||||
SkipErrors: true,
|
||||
})
|
||||
|
||||
require.Error(t, err, "restore must fail when files were skipped")
|
||||
assert.Contains(t, err.Error(),
|
||||
fmt.Sprintf("%d file(s) failed to restore", len(backup.lost)))
|
||||
|
||||
for path, content := range backup.files {
|
||||
restored := filepath.Join(backup.restoreDir, path)
|
||||
|
||||
if backup.lost[path] {
|
||||
assert.Containsf(t, out.String(), 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 deleted blob", path)
|
||||
assert.Equalf(t, content, got, "%s restored with wrong content", path)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob
|
||||
// still ends the restore with an error when SkipErrors is not set.
|
||||
//
|
||||
//nolint:paralleltest // installs the global logger via log.Initialize
|
||||
func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
backup := backupThenDeleteOneBlob(ctx, t)
|
||||
|
||||
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
|
||||
|
||||
err := v.Restore(&vaultik.RestoreOptions{
|
||||
SnapshotID: backup.snapshotID,
|
||||
TargetDir: backup.restoreDir,
|
||||
})
|
||||
|
||||
require.ErrorIs(t, err, storage.ErrNotFound)
|
||||
}
|
||||
|
||||
// backupThenDeleteOneBlob backs up missingBlobFileCount single-chunk
|
||||
// files, then deletes from the store the blob holding the first of them.
|
||||
func backupThenDeleteOneBlob(
|
||||
ctx context.Context, t *testing.T,
|
||||
) *missingBlobBackup {
|
||||
t.Helper()
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
fs := afero.NewOsFs()
|
||||
tempDir := t.TempDir()
|
||||
dataDir := filepath.Join(tempDir, "src")
|
||||
dbPath := filepath.Join(tempDir, "index.sqlite")
|
||||
cfg := faultTestConfig()
|
||||
|
||||
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
|
||||
|
||||
files := make(map[string][]byte, missingBlobFileCount)
|
||||
|
||||
for i := range missingBlobFileCount {
|
||||
path := filepath.Join(dataDir, fmt.Sprintf("file-%02d.bin", i))
|
||||
files[path] = bytesPattern(
|
||||
fmt.Sprintf("file-%02d-", i), missingBlobFileBytes)
|
||||
require.NoError(t, afero.WriteFile(fs, path, files[path], 0o644))
|
||||
}
|
||||
|
||||
storer, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
|
||||
require.NoError(t, err)
|
||||
|
||||
db, err := database.New(ctx, dbPath)
|
||||
require.NoError(t, err)
|
||||
|
||||
repos := database.NewRepositories(db)
|
||||
|
||||
id := fullFaultBackup(
|
||||
ctx, t, fs, storer, cfg, repos, dataDir, dbPath, "missingblob")
|
||||
|
||||
blobOfFile := make(map[string]string, len(files))
|
||||
for path := range files {
|
||||
blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
|
||||
}
|
||||
|
||||
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 == deleted {
|
||||
lost[path] = true
|
||||
}
|
||||
}
|
||||
|
||||
require.Less(t, len(lost), len(files),
|
||||
"the snapshot must span more than one blob")
|
||||
|
||||
return &missingBlobBackup{
|
||||
fs: fs,
|
||||
cfg: cfg,
|
||||
storer: storer,
|
||||
snapshotID: id,
|
||||
restoreDir: filepath.Join(tempDir, "restored"),
|
||||
files: files,
|
||||
lost: lost,
|
||||
}
|
||||
}
|
||||
|
||||
// blobHashOfSingleChunkFile returns the hash of the blob holding the one
|
||||
// chunk of the file at path, as recorded in the local index.
|
||||
func blobHashOfSingleChunkFile(
|
||||
ctx context.Context, t *testing.T,
|
||||
repos *database.Repositories, path string,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
chunks, err := repos.FileChunks.GetByPath(ctx, path)
|
||||
require.NoError(t, err)
|
||||
require.Lenf(t, chunks, 1, "%s must be a single chunk", path)
|
||||
|
||||
blobChunk, err := repos.BlobChunks.GetByChunkHash(
|
||||
ctx, chunks[0].ChunkHash.String())
|
||||
require.NoError(t, err)
|
||||
require.NotNilf(t, blobChunk, "chunk of %s is in no blob", path)
|
||||
|
||||
blob, err := repos.Blobs.GetByID(ctx, blobChunk.BlobID.String())
|
||||
require.NoError(t, err)
|
||||
|
||||
return blob.Hash.String()
|
||||
}
|
||||
Reference in New Issue
Block a user