Make remote nuke delete leftover .partial uploads (closes #281)
check / check (push) Waiting to run
check / check (push) Waiting to run
The file and rclone listings skip an object whose name ends in `.partial`, the temporary name a `file://` or rclone upload writes before moving the object into place. `remote nuke` deletes only what the listings return, so it left the `.partial` objects killed uploads leave behind and still reported the destination store empty. Storer gains DeletePartialUploads. The file and rclone backends remove every `.partial` object under the prefix; S3 has none to remove, since it shows an object only once its upload completes. `remote nuke` calls it for `metadata/` and `blobs/` as its last step. Empty directories under a `file://` destination are still left behind. Model: opus-5-5
This commit was merged in pull request #283.
This commit is contained in:
@@ -22,6 +22,13 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-08: Made `remote nuke` delete the `.partial` files that
|
||||||
|
uploads cut off part-way leave on the destination store
|
||||||
|
([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The
|
||||||
|
file and rclone listings skip such a file, so the command left it in
|
||||||
|
place and still reported the store empty. It now removes them under
|
||||||
|
`metadata/` and `blobs/` as its last step.
|
||||||
|
|
||||||
- 2026-10-08: Made a local index error while recording a directory or
|
- 2026-10-08: Made a local index error while recording a directory or
|
||||||
symlink stop a backup under `--skip-errors`
|
symlink stop a backup under `--skip-errors`
|
||||||
([issue #284](https://git.eeqj.de/sneak/vaultik/issues/284)). Phase 2
|
([issue #284](https://git.eeqj.de/sneak/vaultik/issues/284)). Phase 2
|
||||||
|
|||||||
@@ -171,6 +171,11 @@ func (f *Storer) ListStream(
|
|||||||
return f.inner.ListStream(ctx, prefix)
|
return f.inner.ListStream(ctx, prefix)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads delegates unchanged.
|
||||||
|
func (f *Storer) DeletePartialUploads(ctx context.Context, prefix string) error {
|
||||||
|
return f.inner.DeletePartialUploads(ctx, prefix)
|
||||||
|
}
|
||||||
|
|
||||||
// Info delegates unchanged.
|
// Info delegates unchanged.
|
||||||
func (f *Storer) Info() storage.Info {
|
func (f *Storer) Info() storage.Info {
|
||||||
return f.inner.Info()
|
return f.inner.Info()
|
||||||
|
|||||||
@@ -242,6 +242,42 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
|
|||||||
return ch
|
return ch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads removes every file under prefix whose name ends in
|
||||||
|
// tempSuffix. A missing prefix has none to remove.
|
||||||
|
func (f *FileStorer) DeletePartialUploads(ctx context.Context, prefix string) error {
|
||||||
|
basePath := f.fullPath(prefix)
|
||||||
|
|
||||||
|
exists, err := afero.Exists(f.fs, basePath)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("checking path: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !exists {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
err = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error {
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if ctx.Err() != nil {
|
||||||
|
return ctx.Err()
|
||||||
|
}
|
||||||
|
|
||||||
|
if info.IsDir() || !strings.HasSuffix(info.Name(), tempSuffix) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return f.fs.Remove(path)
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("walking directory: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// Info returns human-readable storage location information.
|
// Info returns human-readable storage location information.
|
||||||
func (f *FileStorer) Info() Info {
|
func (f *FileStorer) Info() Info {
|
||||||
return Info{
|
return Info{
|
||||||
|
|||||||
@@ -120,3 +120,45 @@ func TestFileStorer_ListSkipsPartialFiles(t *testing.T) {
|
|||||||
t.Fatalf("ListStream should return only the real key, got %v", streamed)
|
t.Fatalf("ListStream should return only the real key, got %v", streamed)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestFileStorer_DeletePartialUploads checks that a leftover temp file is
|
||||||
|
// removed and the object at the real key is kept.
|
||||||
|
func TestFileStorer_DeletePartialUploads(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
base := t.TempDir()
|
||||||
|
|
||||||
|
f, err := storage.NewFileStorer(base)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewFileStorer: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
err = f.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Put: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
leftover := filepath.Join(base, testBlobKey+"-123456.partial")
|
||||||
|
|
||||||
|
err = os.WriteFile(leftover, []byte("half"), 0o600)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("writing leftover temp file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = f.DeletePartialUploads(ctx, "blobs/")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("DeletePartialUploads: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = os.Stat(leftover)
|
||||||
|
if !os.IsNotExist(err) {
|
||||||
|
t.Errorf("leftover temp file was not removed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = f.Stat(ctx, testBlobKey)
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("Stat of the real key: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -213,6 +213,31 @@ func (r *RcloneStorer) ListStream(
|
|||||||
return ch
|
return ch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads removes every object under prefix whose name ends
|
||||||
|
// in tempSuffix.
|
||||||
|
func (r *RcloneStorer) DeletePartialUploads(ctx context.Context, prefix string) error {
|
||||||
|
var partial []fs.Object
|
||||||
|
|
||||||
|
err := operations.ListFn(ctx, r.fsys, func(obj fs.Object) {
|
||||||
|
key := obj.Remote()
|
||||||
|
if strings.HasPrefix(key, prefix) && strings.HasSuffix(key, tempSuffix) {
|
||||||
|
partial = append(partial, obj)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("listing objects: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, obj := range partial {
|
||||||
|
err = obj.Remove(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("removing object: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// Info returns human-readable storage location information.
|
// Info returns human-readable storage location information.
|
||||||
func (r *RcloneStorer) Info() Info {
|
func (r *RcloneStorer) Info() Info {
|
||||||
location := r.remote
|
location := r.remote
|
||||||
|
|||||||
@@ -198,6 +198,47 @@ func TestRcloneStorerListSkipsPartialFiles(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestRcloneStorerDeletePartialUploads checks that a temporary file left
|
||||||
|
// by a killed upload is removed and the object at the real key is kept.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
|
||||||
|
func TestRcloneStorerDeletePartialUploads(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
s, err := storage.NewRcloneStorer(ctx, ":local", dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewRcloneStorer: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = s.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Put: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
leftover := filepath.Join(dir, testBlobKey+"-123456.partial")
|
||||||
|
|
||||||
|
err = os.WriteFile(leftover, []byte("half"), 0o600)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("writing leftover temp file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = s.DeletePartialUploads(ctx, "blobs/")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("DeletePartialUploads: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = os.Stat(leftover)
|
||||||
|
if !os.IsNotExist(err) {
|
||||||
|
t.Errorf("leftover temp file was not removed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = s.Stat(ctx, testBlobKey)
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("Stat of the real key: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// newRcloneStorerOnWrappedLocal registers name as rclone's local backend
|
// newRcloneStorerOnWrappedLocal registers name as rclone's local backend
|
||||||
// wrapped by wrap, and builds an rclone backend on it rooted at a fresh
|
// wrapped by wrap, and builds an rclone backend on it rooted at a fresh
|
||||||
// temp directory. wrap changes the features the local backend reports, so
|
// temp directory. wrap changes the features the local backend reports, so
|
||||||
|
|||||||
@@ -99,6 +99,12 @@ func (s *S3Storer) ListStream(ctx context.Context, prefix string) <-chan ObjectI
|
|||||||
return ch
|
return ch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads has nothing to remove: S3 shows an object only once
|
||||||
|
// its upload has completed, so an upload cut off part-way leaves none.
|
||||||
|
func (s *S3Storer) DeletePartialUploads(_ context.Context, _ string) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// Info returns human-readable storage location information.
|
// Info returns human-readable storage location information.
|
||||||
func (s *S3Storer) Info() Info {
|
func (s *S3Storer) Info() Info {
|
||||||
return Info{
|
return Info{
|
||||||
|
|||||||
@@ -71,6 +71,12 @@ type Storer interface {
|
|||||||
// If an error occurs during listing, the final item will have Err set.
|
// If an error occurs during listing, the final item will have Err set.
|
||||||
ListStream(ctx context.Context, prefix string) <-chan ObjectInfo
|
ListStream(ctx context.Context, prefix string) <-chan ObjectInfo
|
||||||
|
|
||||||
|
// DeletePartialUploads removes every object under prefix that an
|
||||||
|
// upload cut off part-way left under a temporary name ending in
|
||||||
|
// `.partial`. The file and rclone backends' List and ListStream skip
|
||||||
|
// such an object.
|
||||||
|
DeletePartialUploads(ctx context.Context, prefix string) error
|
||||||
|
|
||||||
// Info returns human-readable storage location information.
|
// Info returns human-readable storage location information.
|
||||||
Info() Info
|
Info() Info
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -257,6 +257,10 @@ func (s *stubLister) List(_ context.Context, _ string) ([]string, error) {
|
|||||||
return nil, errStubUnused
|
return nil, errStubUnused
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (s *stubLister) DeletePartialUploads(_ context.Context, _ string) error {
|
||||||
|
return errStubUnused
|
||||||
|
}
|
||||||
|
|
||||||
func (s *stubLister) Info() storage.Info {
|
func (s *stubLister) Info() storage.Info {
|
||||||
return storage.Info{}
|
return storage.Info{}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -155,6 +155,12 @@ func (m *MockStorer) ListStream(
|
|||||||
return ch
|
return ch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads has nothing to remove: Put stores each object
|
||||||
|
// under its key at once.
|
||||||
|
func (m *MockStorer) DeletePartialUploads(_ context.Context, _ string) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (m *MockStorer) Info() storage.Info {
|
func (m *MockStorer) Info() storage.Info {
|
||||||
return storage.Info{
|
return storage.Info{
|
||||||
Type: "mock",
|
Type: "mock",
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
package vaultik_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"io/fs"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/vaultik/internal/log"
|
||||||
|
"sneak.berlin/go/vaultik/internal/snapshot"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
|
||||||
|
// under a file:// destination, including the `.partial` files uploads
|
||||||
|
// killed part-way leave next to a snapshot's metadata and next to where a
|
||||||
|
// blob would have been.
|
||||||
|
func TestNukeRemoteLeavesNoFiles(t *testing.T) {
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
storeDir := filepath.Join(t.TempDir(), "store")
|
||||||
|
v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
|
||||||
|
|
||||||
|
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Len(t, snapshots, 1)
|
||||||
|
|
||||||
|
snapshotKey := snapshot.RemoteSnapshotKey(snapshots[0].ID.String())
|
||||||
|
hash := testBlobHashA
|
||||||
|
leftovers := []string{
|
||||||
|
filepath.Join(storeDir, "metadata", snapshotKey,
|
||||||
|
"db.zst.age-123456.partial"),
|
||||||
|
filepath.Join(storeDir, "blobs", hash[:2], hash[2:4],
|
||||||
|
hash+"-123456.partial"),
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, leftover := range leftovers {
|
||||||
|
require.NoError(t, os.MkdirAll(filepath.Dir(leftover), 0o750))
|
||||||
|
require.NoError(t, os.WriteFile(leftover, []byte("half an upload"), 0o600))
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, v.NukeRemote(true))
|
||||||
|
|
||||||
|
var files []string
|
||||||
|
|
||||||
|
err = filepath.WalkDir(storeDir,
|
||||||
|
func(path string, entry fs.DirEntry, err error) error {
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if !entry.IsDir() {
|
||||||
|
files = append(files, path)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Empty(t, files)
|
||||||
|
}
|
||||||
@@ -24,8 +24,9 @@ var errNukeRequiresForce = errors.New(
|
|||||||
const metadataDirName = "metadata"
|
const metadataDirName = "metadata"
|
||||||
|
|
||||||
// NukeRemote deletes every snapshot's metadata and every blob from remote
|
// NukeRemote deletes every snapshot's metadata and every blob from remote
|
||||||
// storage. After this returns successfully the bucket prefix is empty and
|
// storage, along with any object an upload cut off part-way left under a
|
||||||
// the next backup starts from scratch.
|
// temporary `.partial` name. After this returns successfully the bucket
|
||||||
|
// prefix is empty and the next backup starts from scratch.
|
||||||
//
|
//
|
||||||
// Refuses to run unless force is true. The caller is responsible for
|
// Refuses to run unless force is true. The caller is responsible for
|
||||||
// confirming with the user.
|
// confirming with the user.
|
||||||
@@ -48,6 +49,15 @@ func (v *Vaultik) NukeRemote(force bool) error {
|
|||||||
return fmt.Errorf("pruning blobs: %w", err)
|
return fmt.Errorf("pruning blobs: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The file and rclone listings skip `.partial` objects, so the two
|
||||||
|
// steps above never delete them.
|
||||||
|
for _, prefix := range []string{"metadata/", "blobs/"} {
|
||||||
|
err = v.Storage.DeletePartialUploads(v.ctx, prefix)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("deleting partial uploads: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
v.UI.Completef("Backup destination store is now empty.")
|
v.UI.Completef("Backup destination store is now empty.")
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -125,6 +125,12 @@ func (s *testStorer) ListStream(
|
|||||||
return ch
|
return ch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeletePartialUploads has nothing to remove: Put stores each object
|
||||||
|
// under its key at once.
|
||||||
|
func (s *testStorer) DeletePartialUploads(_ context.Context, _ string) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (s *testStorer) Info() storage.Info {
|
func (s *testStorer) Info() storage.Info {
|
||||||
return storage.Info{
|
return storage.Info{
|
||||||
Type: testLabel,
|
Type: testLabel,
|
||||||
|
|||||||
Reference in New Issue
Block a user