From eeff112491f0cd2454af91833b4475c3106fe079 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Thu, 15 Oct 2026 18:18:45 +0800 Subject: [PATCH] test: verify commit metadata loss and write quorum boundaries Signed-off-by: Feng Ruohang --- cmd/erasure-metadata-utils_test.go | 62 ++++++++++++------- cmd/xl-storage-errors_test.go | 97 ++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 22 deletions(-) diff --git a/cmd/erasure-metadata-utils_test.go b/cmd/erasure-metadata-utils_test.go index 917d8249b..559b892bf 100644 --- a/cmd/erasure-metadata-utils_test.go +++ b/cmd/erasure-metadata-utils_test.go @@ -144,14 +144,16 @@ func TestReduceWriteQuorumErrsCommitMetaMissing(t *testing.T) { // from RenameData, enough to drive renameData() in unit tests. type renameDataFailingDisk struct { StorageAPI - err error + err error + undoWrite bool } func (d *renameDataFailingDisk) RenameData(context.Context, string, string, FileInfo, string, string, RenameOptions) (RenameDataResp, error) { return RenameDataResp{}, d.err } -func (d *renameDataFailingDisk) DeleteVersion(context.Context, string, string, FileInfo, bool, DeleteOptions) error { +func (d *renameDataFailingDisk) DeleteVersion(_ context.Context, _, _ string, _ FileInfo, _ bool, opts DeleteOptions) error { + d.undoWrite = opts.UndoWrite return nil } @@ -160,26 +162,42 @@ func (d *renameDataFailingDisk) DeleteVersion(context.Context, string, string, F // while an in-quorum errFileNotFound keeps its meaning so that // PutObject() still reports IncompleteBody for client disconnects. func TestRenameDataCommitMetaMissing(t *testing.T) { - metadata := make([]FileInfo, 16) - for i := range metadata { - metadata[i] = FileInfo{Deleted: true} // valid without erasure info - } - - disks := make([]StorageAPI, 16) - for i := range disks { - disks[i] = &renameDataFailingDisk{err: errCommitMetaMissing} - } - _, _, _, err := renameData(t.Context(), disks, minioMetaTmpBucket, "tmp", metadata, "bucket", "object", 9) - if !errors.Is(err, errErasureWriteQuorum) { - t.Fatalf("expected %s for in-quorum commit failure, got %v", errErasureWriteQuorum, err) - } - - for i := range disks { - disks[i] = &renameDataFailingDisk{err: errFileNotFound} - } - _, _, _, err = renameData(t.Context(), disks, minioMetaTmpBucket, "tmp", metadata, "bucket", "object", 9) - if !errors.Is(err, errFileNotFound) { - t.Fatalf("expected %s for in-quorum staging failure, got %v", errFileNotFound, err) + for _, tc := range []struct { + name string + failures int + diskErr error + wantErr error + }{ + {"all_commit_metadata_missing", 16, errCommitMetaMissing, errErasureWriteQuorum}, + {"quorum_commit_metadata_missing", 9, errCommitMetaMissing, errErasureWriteQuorum}, + {"neither_side_has_quorum", 8, errCommitMetaMissing, errErasureWriteQuorum}, + {"successful_write_quorum", 7, errCommitMetaMissing, nil}, + {"staging_failure", 16, errFileNotFound, errFileNotFound}, + } { + t.Run(tc.name, func(t *testing.T) { + metadata := make([]FileInfo, 16) + disks := make([]StorageAPI, 16) + for i := range disks { + metadata[i] = FileInfo{Deleted: true} // valid without erasure info + disk := &renameDataFailingDisk{} + if i < tc.failures { + disk.err = tc.diskErr + } + disks[i] = disk + } + _, _, _, err := renameData(t.Context(), disks, minioMetaTmpBucket, "tmp", metadata, "bucket", "object", 9) + if !errors.Is(err, tc.wantErr) { + t.Fatalf("expected %v, got %v", tc.wantErr, err) + } + for i, disk := range disks { + // Failed writes undo only the drives that committed. A + // successful quorum must never be rolled back. + wantUndo := tc.wantErr != nil && i >= tc.failures + if got := disk.(*renameDataFailingDisk).undoWrite; got != wantUndo { + t.Errorf("disk %d: UndoWrite = %v, want %v", i, got, wantUndo) + } + } + }) } } diff --git a/cmd/xl-storage-errors_test.go b/cmd/xl-storage-errors_test.go index 0551fed6a..b9d12fc4d 100644 --- a/cmd/xl-storage-errors_test.go +++ b/cmd/xl-storage-errors_test.go @@ -18,11 +18,16 @@ package cmd import ( + "context" "errors" + "net/http" "os" + "path/filepath" "runtime" + "sync" "syscall" "testing" + "time" ) func TestSysErrors(t *testing.T) { @@ -81,4 +86,96 @@ func TestCommitMetaMissingMapping(t *testing.T) { if qerr.Bucket != "bucket" || qerr.Object != "object" { t.Fatalf("unexpected bucket/object %s/%s", qerr.Bucket, qerr.Object) } + apiErr := toAPIError(t.Context(), err) + if apiErr.Code != "SlowDownWrite" || apiErr.HTTPStatusCode != http.StatusServiceUnavailable { + t.Fatalf("expected SlowDownWrite/503, got %+v", apiErr) + } +} + +// missingCommitMetaContext removes staged metadata at an existing cancellation +// check, after WriteAll and (for non-inline objects) the data rename succeeded. +// This deterministically models concurrent cleanup without changing production +// code or relying on a filesystem race. The context itself remains live. +type missingCommitMetaContext struct { + context.Context + mu sync.Mutex + meta, dataDest string + removed bool + removeErr error +} + +func (c *missingCommitMetaContext) Done() <-chan struct{} { + c.mu.Lock() + defer c.mu.Unlock() + if !c.removed { + if _, err := os.Stat(c.meta); err == nil { + dataMoved := c.dataDest == "" + if !dataMoved { + _, err = os.Stat(c.dataDest) + dataMoved = err == nil + } + if dataMoved { + c.removeErr = os.Remove(c.meta) + c.removed = c.removeErr == nil + } + } + } + return c.Context.Done() +} + +func TestXLStorageRenameDataCommitMetaMissing(t *testing.T) { + for _, tc := range []struct { + name string + dataFile bool + }{ + {"empty_object", false}, + {"data_directory", true}, + } { + t.Run(tc.name, func(t *testing.T) { + disk, root, err := newXLStorageTestSetup(t) + if err != nil { + t.Fatal(err) + } + defer disk.Close() + s := disk.storage + if err := s.MakeVol(t.Context(), "bucket"); err != nil { + t.Fatal(err) + } + fi := newFileInfo("object", 2, 2) + fi.DataDir, fi.VersionID = mustGetUUID(), mustGetUUID() + fi.Erasure.Index = 1 + fi.ModTime = time.Now().UTC() + fi.Fresh = true + ctx := &missingCommitMetaContext{ + Context: t.Context(), + meta: filepath.Join(root, minioMetaTmpBucket, "staged", xlStorageFormatFile), + } + if tc.dataFile { + fi.Size = 1 + fi.AddObjectPart(1, "", 1, 1, fi.ModTime, nil, nil) + part := filepath.Join(root, minioMetaTmpBucket, "staged", fi.DataDir, "part.1") + if err := os.MkdirAll(filepath.Dir(part), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(part, []byte("a"), 0o644); err != nil { + t.Fatal(err) + } + ctx.dataDest = filepath.Join(root, "bucket", "object", fi.DataDir) + } + _, err = s.RenameData(ctx, minioMetaTmpBucket, "staged", fi, "bucket", "object", RenameOptions{}) + if !ctx.removed { + t.Fatalf("metadata removal did not execute: removal error = %v, RenameData error = %v", ctx.removeErr, err) + } + if !errors.Is(err, errCommitMetaMissing) { + t.Fatalf("expected commit metadata missing, got %v", err) + } + apiErr := toAPIError(t.Context(), toObjectErr(err, "bucket", "object")) + if apiErr.Code != "SlowDownWrite" || apiErr.HTTPStatusCode != http.StatusServiceUnavailable { + t.Fatalf("expected SlowDownWrite/503, got %+v", apiErr) + } + if _, err := os.Stat(filepath.Join(root, "bucket", "object", xlStorageFormatFile)); !os.IsNotExist(err) { + t.Fatalf("failed commit must not produce destination metadata: %v", err) + } + }) + } }