Write file:// blobs atomically via temp file and rename (closes #130)
The file:// backend streamed each object straight to its final key, so an upload cut off mid-stream left a truncated object there. The next backup saw that Stat succeeded, recorded the blob as complete, and produced a snapshot that reported success but could not be restored. Writes now go to a temporary file with a .partial suffix in the destination directory, are synced, then renamed onto the key. List and ListStream skip .partial files, so a leftover is never trusted as a blob and is overwritten when the key is written again. S3 PutObject is already atomic. Disclosure: the containing directory is not synced after the rename, so a host crash right after it could still lose the object on some filesystems. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
This commit was merged in pull request #143.
This commit is contained in:
@@ -0,0 +1,119 @@
|
||||
package storage_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/vaultik/internal/storage"
|
||||
)
|
||||
|
||||
// errStreamInterrupted stands in for an upload cut off mid-stream.
|
||||
var errStreamInterrupted = errors.New("connection reset mid-upload")
|
||||
|
||||
// failingReader yields its data once, then fails.
|
||||
type failingReader struct {
|
||||
data []byte
|
||||
done bool
|
||||
}
|
||||
|
||||
func (r *failingReader) Read(p []byte) (int, error) {
|
||||
if r.done {
|
||||
return 0, errStreamInterrupted
|
||||
}
|
||||
|
||||
n := copy(p, r.data)
|
||||
r.done = true
|
||||
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// TestFileStorer_InterruptedWriteLeavesNoTrustedObject checks that a write
|
||||
// cut off mid-stream leaves nothing at the destination key, so a later run
|
||||
// cannot Stat a truncated object and trust it as a complete blob.
|
||||
func TestFileStorer_InterruptedWriteLeavesNoTrustedObject(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
f, err := storage.NewFileStorer(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatalf("NewFileStorer: %v", err)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
key := "blobs/aa/bb/aabbccddeeff"
|
||||
|
||||
err = f.PutWithProgress(ctx, key, &failingReader{data: []byte("partial")}, 4096, nil)
|
||||
if err == nil {
|
||||
t.Fatal("expected the interrupted write to fail, got nil")
|
||||
}
|
||||
|
||||
_, err = f.Stat(ctx, key)
|
||||
if !errors.Is(err, storage.ErrNotFound) {
|
||||
t.Fatalf("expected key absent after interrupted write, got Stat err %v", err)
|
||||
}
|
||||
|
||||
keys, err := f.List(ctx, "blobs/")
|
||||
if err != nil {
|
||||
t.Fatalf("List: %v", err)
|
||||
}
|
||||
|
||||
if len(keys) != 0 {
|
||||
t.Fatalf("expected no keys listed after interrupted write, got %v", keys)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileStorer_ListSkipsPartialFiles checks that a leftover temp file (the
|
||||
// storage layer names them with a ".partial" suffix) is never surfaced as a
|
||||
// key by List or ListStream.
|
||||
func TestFileStorer_ListSkipsPartialFiles(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
base := t.TempDir()
|
||||
|
||||
f, err := storage.NewFileStorer(base)
|
||||
if err != nil {
|
||||
t.Fatalf("NewFileStorer: %v", err)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
realKey := "blobs/aa/bb/aabbccddeeff"
|
||||
|
||||
err = f.Put(ctx, realKey, strings.NewReader("blob-bytes"))
|
||||
if err != nil {
|
||||
t.Fatalf("Put: %v", err)
|
||||
}
|
||||
|
||||
// A stray temp file, as an interrupted write would leave behind.
|
||||
leftover := filepath.Join(base, "blobs/aa/bb/aabbccddeeff-123456.partial")
|
||||
|
||||
err = os.WriteFile(leftover, []byte("half"), 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("writing leftover temp file: %v", err)
|
||||
}
|
||||
|
||||
keys, err := f.List(ctx, "blobs/")
|
||||
if err != nil {
|
||||
t.Fatalf("List: %v", err)
|
||||
}
|
||||
|
||||
if len(keys) != 1 || keys[0] != realKey {
|
||||
t.Fatalf("List should return only the real key, got %v", keys)
|
||||
}
|
||||
|
||||
var streamed []string
|
||||
|
||||
for obj := range f.ListStream(ctx, "blobs/") {
|
||||
if obj.Err != nil {
|
||||
t.Fatalf("ListStream: %v", obj.Err)
|
||||
}
|
||||
|
||||
streamed = append(streamed, obj.Key)
|
||||
}
|
||||
|
||||
if len(streamed) != 1 || streamed[0] != realKey {
|
||||
t.Fatalf("ListStream should return only the real key, got %v", streamed)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user