diff --git a/cmd/erasure-metadata-utils_test.go b/cmd/erasure-metadata-utils_test.go index b14cc25df..559b892bf 100644 --- a/cmd/erasure-metadata-utils_test.go +++ b/cmd/erasure-metadata-utils_test.go @@ -20,6 +20,7 @@ package cmd import ( "context" "encoding/hex" + "errors" "fmt" "math/rand" "reflect" @@ -110,6 +111,96 @@ func TestReduceErrs(t *testing.T) { } } +// TestReduceWriteQuorumErrsCommitMetaMissing verifies that an in-quorum +// errCommitMetaMissing reduces to the sentinel, which renameData() converts +// to errErasureWriteQuorum, while below-quorum reduces directly to +// errErasureWriteQuorum. +func TestReduceWriteQuorumErrsCommitMetaMissing(t *testing.T) { + errs := make([]error, 16) + for i := range 9 { + errs[i] = errCommitMetaMissing + } + if err := reduceWriteQuorumErrs(t.Context(), errs, objectOpIgnoredErrs, 9); !errors.Is(err, errCommitMetaMissing) { + t.Errorf("expected %s, got %s", errCommitMetaMissing, err) + } + // The sentinel must be distinguishable from errFileNotFound so that + // commit failures are never mapped to ObjectNotFound (404). + if errors.Is(errCommitMetaMissing, errFileNotFound) { + t.Error("errCommitMetaMissing must not match errFileNotFound") + } + + // Below quorum the reduction must yield a write-quorum failure (503); + // eight sentinels against eight committed drives is not a quorum. + belowQuorum := make([]error, 16) + for i := range 8 { + belowQuorum[i] = errCommitMetaMissing + } + if err := reduceWriteQuorumErrs(t.Context(), belowQuorum, objectOpIgnoredErrs, 9); !errors.Is(err, errErasureWriteQuorum) { + t.Errorf("expected %s, got %s", errErasureWriteQuorum, err) + } +} + +// renameDataFailingDisk is a StorageAPI stub returning a fixed error +// from RenameData, enough to drive renameData() in unit tests. +type renameDataFailingDisk struct { + StorageAPI + 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, _ FileInfo, _ bool, opts DeleteOptions) error { + d.undoWrite = opts.UndoWrite + return nil +} + +// TestRenameDataCommitMetaMissing verifies that renameData() converts an +// in-quorum errCommitMetaMissing into errErasureWriteQuorum (HTTP 503), +// while an in-quorum errFileNotFound keeps its meaning so that +// PutObject() still reports IncompleteBody for client disconnects. +func TestRenameDataCommitMetaMissing(t *testing.T) { + 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) + } + } + }) + } +} + // TestHashOrder - test order of ints in array func TestHashOrder(t *testing.T) { testCases := []struct { diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index a373ea543..b617a1b9a 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -1083,6 +1083,12 @@ func renameData(ctx context.Context, disks []StorageAPI, srcBucket, srcEntry str errs := g.Wait() err := reduceWriteQuorumErrs(ctx, errs, objectOpIgnoredErrs, writeQuorum) + if errors.Is(err, errCommitMetaMissing) { + // Commit metadata went missing on a quorum of drives; the write + // did not commit. Return a 503 class error so clients retry + // instead of treating the object as absent (404). + err = errErasureWriteQuorum + } if err != nil { dg := errgroup.WithNErrs(len(disks)) for index, nerr := range errs { diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go index 7b2b5e404..457970fb0 100644 --- a/cmd/object-api-errors.go +++ b/cmd/object-api-errors.go @@ -162,7 +162,7 @@ func toObjectErr(oerr error, params ...string) error { apiErr.Type = v.Type } return apiErr - case errErasureWriteQuorum.Error(): + case errErasureWriteQuorum.Error(), errCommitMetaMissing.Error(): apiErr := InsufficientWriteQuorum{} if len(params) >= 1 { apiErr.Bucket = params[0] diff --git a/cmd/storage-errors.go b/cmd/storage-errors.go index 69ffcebb0..c0a908eac 100644 --- a/cmd/storage-errors.go +++ b/cmd/storage-errors.go @@ -70,6 +70,11 @@ var errDiskAccessDenied = StorageErr("drive access denied") // errFileNotFound - cannot find the file. var errFileNotFound = StorageErr("file not found") +// errCommitMetaMissing - the staged xl.meta disappeared between the +// data rename and the meta-file commit; a write-quorum failure, not +// a missing object. +var errCommitMetaMissing = StorageErr("commit metadata is missing") + // errFileNotFound - cannot find requested file version. var errFileVersionNotFound = StorageErr("file version not found") diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 76993bc06..0ba573847 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -106,6 +106,8 @@ func toStorageErr(err error) error { return errVolumeExists case errFileNotFound.Error(): return errFileNotFound + case errCommitMetaMissing.Error(): + return errCommitMetaMissing case errFileVersionNotFound.Error(): return errFileVersionNotFound case errFileNameTooLong.Error(): diff --git a/cmd/xl-storage-errors_test.go b/cmd/xl-storage-errors_test.go index 0a4757765..b9d12fc4d 100644 --- a/cmd/xl-storage-errors_test.go +++ b/cmd/xl-storage-errors_test.go @@ -18,10 +18,16 @@ package cmd import ( + "context" + "errors" + "net/http" "os" + "path/filepath" "runtime" + "sync" "syscall" "testing" + "time" ) func TestSysErrors(t *testing.T) { @@ -56,3 +62,120 @@ func TestSysErrors(t *testing.T) { } } } + +// TestCommitMetaMissingMapping verifies the RPC and object-layer mapping +// of errCommitMetaMissing, returned when the staged xl.meta disappears +// between the data rename and the meta-file commit in RenameData(). +func TestCommitMetaMissingMapping(t *testing.T) { + // The sentinel must survive a storage REST/grid round-trip; errors + // cross the wire as strings and are remapped by toStorageErr(). + if got := toStorageErr(errors.New(errCommitMetaMissing.Error())); !errors.Is(got, errCommitMetaMissing) { + t.Fatalf("expected %s, got %s", errCommitMetaMissing, got) + } + if got := toStorageErr(errCommitMetaMissing); got != errCommitMetaMissing { + t.Fatalf("expected %s, got %s", errCommitMetaMissing, got) + } + + // toObjectErr() must map the sentinel to InsufficientWriteQuorum + // (503), never to ObjectNotFound (404). + err := toObjectErr(errCommitMetaMissing, "bucket", "object") + qerr, ok := err.(InsufficientWriteQuorum) + if !ok { + t.Fatalf("expected InsufficientWriteQuorum, got %s (%T)", err, err) + } + 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) + } + }) + } +} diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 92d8c8dec..81e415553 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -2925,7 +2925,15 @@ func (s *xlStorage) RenameData(ctx context.Context, srcVolume, srcPath string, f // if its a partial rename() do not attempt to delete recursively. // this can be healed since all parts are available. s.deleteFile(dstVolumeDir, dstDataPath, false, false) - return res, osErrToFileErr(err) + err = osErrToFileErr(err) + if errors.Is(err, errFileNotFound) { + // The staged xl.meta disappeared between staging and commit. + // This is a write-quorum failure, not a missing object; use a + // dedicated error so the object layer returns 503 instead of + // mapping it to ObjectNotFound (404). + err = errCommitMetaMissing + } + return res, err } if srcVolume != minioMetaMultipartBucket {