Stop a cancelled backup at once under --skip-errors (closes #286)
check / check (push) Waiting to run
check / check (push) Waiting to run
After Ctrl-C or SIGTERM, phase 2 of a --skip-errors backup treated the cancellation error from each remaining file like an unreadable file: it opened the file, printed an error line, counted it as failed and moved on to the next. The processing loop now checks for cancellation before each file, and an error from a file once the run is cancelled stops the run instead of being skipped. The loop check is what stops a run cancelled while an empty file is open, since an empty file has no chunks and nothing reads the context while it is backed up. The --skip-errors test helper now takes the scan's context. Model: opus-5-5
This commit was merged in pull request #287.
This commit is contained in:
@@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once
|
||||||
|
([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After
|
||||||
|
Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each
|
||||||
|
remaining file like an unreadable file: it opened the file, printed an
|
||||||
|
error line for it, counted it as failed and went on to the next one.
|
||||||
|
The run now stops at the first file after the cancellation, and no
|
||||||
|
file is counted as failed because of it.
|
||||||
|
|
||||||
- 2026-10-08: Made `remote nuke` delete the `.partial` files that
|
- 2026-10-08: Made `remote nuke` delete the `.partial` files that
|
||||||
uploads cut off part-way leave on the destination store
|
uploads cut off part-way leave on the destination store
|
||||||
([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The
|
([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The
|
||||||
|
|||||||
@@ -1312,6 +1312,13 @@ func (s *Scanner) processPhase(
|
|||||||
|
|
||||||
// Process each file
|
// Process each file
|
||||||
for _, fileToProcess := range filesToProcess {
|
for _, fileToProcess := range filesToProcess {
|
||||||
|
// Check context cancellation
|
||||||
|
select {
|
||||||
|
case <-ctx.Done():
|
||||||
|
return ctx.Err()
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
|
||||||
// Update progress
|
// Update progress
|
||||||
if s.progress != nil {
|
if s.progress != nil {
|
||||||
s.progress.GetStats().CurrentFile.Store(fileToProcess.Path)
|
s.progress.GetStats().CurrentFile.Store(fileToProcess.Path)
|
||||||
@@ -1369,6 +1376,11 @@ func (s *Scanner) processFileWithErrorHandling(
|
|||||||
|
|
||||||
err := s.processFileStreaming(ctx, fileToProcess, result)
|
err := s.processFileStreaming(ctx, fileToProcess, result)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
// A cancelled run stops here even under --skip-errors, rather than
|
||||||
|
// counting this file as failed and going on to the next one.
|
||||||
|
if ctx.Err() != nil {
|
||||||
|
return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)
|
||||||
|
}
|
||||||
// A packer/database/encryption/upload failure means the chunk's data
|
// A packer/database/encryption/upload failure means the chunk's data
|
||||||
// may not have been stored. Skipping the file would let the snapshot
|
// may not have been stored. Skipping the file would let the snapshot
|
||||||
// record a file whose chunk is in no blob and cannot be restored, so
|
// record a file whose chunk is in no blob and cannot be restored, so
|
||||||
|
|||||||
@@ -82,6 +82,32 @@ func (f *readFailFs) Open(name string) (afero.File, error) {
|
|||||||
return file, nil
|
return file, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// cancelOnOpenFs cancels the run when the scanner opens the target file to
|
||||||
|
// back it up, as Ctrl-C partway through processing would, and records each
|
||||||
|
// file opened after that.
|
||||||
|
type cancelOnOpenFs struct {
|
||||||
|
afero.Fs
|
||||||
|
|
||||||
|
target string
|
||||||
|
cancel context.CancelFunc
|
||||||
|
cancelled bool
|
||||||
|
openedAfterCancel []string
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:ireturn // afero.Fs.Open is defined to return the interface.
|
||||||
|
func (f *cancelOnOpenFs) Open(name string) (afero.File, error) {
|
||||||
|
if f.cancelled {
|
||||||
|
f.openedAfterCancel = append(f.openedAfterCancel, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
if name == f.target {
|
||||||
|
f.cancel()
|
||||||
|
f.cancelled = true
|
||||||
|
}
|
||||||
|
|
||||||
|
return f.Fs.Open(name)
|
||||||
|
}
|
||||||
|
|
||||||
// linkRemovedAfterLstatFs is the real filesystem, except that the symlink at
|
// linkRemovedAfterLstatFs is the real filesystem, except that the symlink at
|
||||||
// target is removed right after the walk lstats it, as happens when a link is
|
// target is removed right after the walk lstats it, as happens when a link is
|
||||||
// deleted during a backup. The scanner's readlink of it then fails.
|
// deleted during a backup. The scanner's readlink of it then fails.
|
||||||
@@ -128,15 +154,16 @@ func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// runSkipErrorScan scans source on fs with the given skip-errors setting,
|
// runSkipErrorScan scans source on fs under ctx with the given skip-errors
|
||||||
// printing user-facing messages to uiw (nil discards them), and returns the
|
// setting, printing user-facing messages to uiw (nil discards them), and
|
||||||
// repositories (for inspection) and the scan error.
|
// returns the repositories (for inspection) and the scan error.
|
||||||
func runSkipErrorScan(
|
func runSkipErrorScan(
|
||||||
t *testing.T, fs afero.Fs, source string, skipErrors bool, uiw *ui.Writer,
|
ctx context.Context, t *testing.T, fs afero.Fs, source string,
|
||||||
|
skipErrors bool, uiw *ui.Writer,
|
||||||
) (*database.Repositories, error) {
|
) (*database.Repositories, error) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
db, err := database.NewTestDB()
|
db, err := database.New(ctx, ":memory:")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("create test db: %v", err)
|
t.Fatalf("create test db: %v", err)
|
||||||
}
|
}
|
||||||
@@ -161,7 +188,6 @@ func runSkipErrorScan(
|
|||||||
SkipErrors: skipErrors,
|
SkipErrors: skipErrors,
|
||||||
})
|
})
|
||||||
|
|
||||||
ctx := context.Background()
|
|
||||||
snapshotID := "test-snapshot-skip-errors"
|
snapshotID := "test-snapshot-skip-errors"
|
||||||
createTestSnapshotRecord(ctx, t, repos, snapshotID)
|
createTestSnapshotRecord(ctx, t, repos, snapshotID)
|
||||||
|
|
||||||
@@ -185,7 +211,7 @@ func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) {
|
|||||||
writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content")
|
writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content")
|
||||||
writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content")
|
writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content")
|
||||||
|
|
||||||
repos, err := runSkipErrorScan(t, fs, "/source", true, nil)
|
repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected scan to abort on the packer error, got nil")
|
t.Fatal("expected scan to abort on the packer error, got nil")
|
||||||
}
|
}
|
||||||
@@ -212,7 +238,7 @@ func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) {
|
|||||||
fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
|
fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
|
||||||
writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
|
writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
|
||||||
|
|
||||||
_, err := runSkipErrorScan(t, fs, "/source", false, nil)
|
_, err := runSkipErrorScan(context.Background(), t, fs, "/source", false, nil)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected scan to fail on the read error, got nil")
|
t.Fatal("expected scan to fail on the read error, got nil")
|
||||||
}
|
}
|
||||||
@@ -228,7 +254,7 @@ func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) {
|
|||||||
fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
|
fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
|
||||||
writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
|
writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
|
||||||
|
|
||||||
repos, err := runSkipErrorScan(t, fs, "/source", true, nil)
|
repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
|
t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
|
||||||
}
|
}
|
||||||
@@ -267,7 +293,7 @@ func TestScannerUnreadableSymlinkAbortsWithoutSkipErrors(t *testing.T) {
|
|||||||
sourceDir, linkPath := writeSymlinkSource(t)
|
sourceDir, linkPath := writeSymlinkSource(t)
|
||||||
fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}
|
fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}
|
||||||
|
|
||||||
_, err := runSkipErrorScan(t, fs, sourceDir, false, nil)
|
_, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, false, nil)
|
||||||
if !errors.Is(err, os.ErrNotExist) {
|
if !errors.Is(err, os.ErrNotExist) {
|
||||||
t.Fatalf("expected scan to fail on the removed symlink, got %v", err)
|
t.Fatalf("expected scan to fail on the removed symlink, got %v", err)
|
||||||
}
|
}
|
||||||
@@ -283,7 +309,7 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) {
|
|||||||
fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}
|
fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}
|
||||||
uiw := ui.NewWithColor(io.Discard, false)
|
uiw := ui.NewWithColor(io.Discard, false)
|
||||||
|
|
||||||
repos, err := runSkipErrorScan(t, fs, sourceDir, true, uiw)
|
repos, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, true, uiw)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
|
t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
|
||||||
}
|
}
|
||||||
@@ -302,3 +328,56 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) {
|
|||||||
t.Fatalf("expected %s not to be recorded", linkPath)
|
t.Fatalf("expected %s not to be recorded", linkPath)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestScannerCancelStopsSkipErrorsRun checks that a --skip-errors backup
|
||||||
|
// cancelled partway through processing returns the cancellation error,
|
||||||
|
// opens no further file, and reports no file as failed.
|
||||||
|
func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
targetContent string
|
||||||
|
}{
|
||||||
|
// The cancellation lands while the target is being read.
|
||||||
|
{name: "while reading a file", targetContent: "first file content"},
|
||||||
|
// An empty target has no chunks, so the cancellation goes unnoticed
|
||||||
|
// until the run moves on to the next file.
|
||||||
|
{name: "between files", targetContent: ""},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const target = "/source/a.txt"
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
fs := &cancelOnOpenFs{
|
||||||
|
Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
|
||||||
|
}
|
||||||
|
writeSkipErrorTestFile(t, fs, target, tt.targetContent)
|
||||||
|
writeSkipErrorTestFile(t, fs, "/source/b.txt", "second file content")
|
||||||
|
writeSkipErrorTestFile(t, fs, "/source/c.txt", "third file content")
|
||||||
|
|
||||||
|
uiw := ui.NewWithColor(io.Discard, false)
|
||||||
|
|
||||||
|
_, err := runSkipErrorScan(ctx, t, fs, "/source", true, uiw)
|
||||||
|
if !errors.Is(err, context.Canceled) {
|
||||||
|
t.Fatalf("expected the cancellation error, got %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(fs.openedAfterCancel) != 0 {
|
||||||
|
t.Fatalf("expected no file opened after the cancellation, got %v",
|
||||||
|
fs.openedAfterCancel)
|
||||||
|
}
|
||||||
|
|
||||||
|
if uiw.ErrorCount() != 0 {
|
||||||
|
t.Fatalf("expected no file reported as failed, got %d error lines",
|
||||||
|
uiw.ErrorCount())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user