mirror of
https://github.com/pgsty/minio.git
synced 2026-10-17 15:16:00 +03:00
fix: return 503 instead of 404 when commit metadata is missing
When the staged xl.meta disappears between the data rename and the meta-file commit inside RenameData(), the affected drives return errFileNotFound. An in-quorum errFileNotFound is reduced by reduceWriteQuorumErrs() and previously surfaced as: - IncompleteBody (400) from PutObject(), misattributing the failure to a client disconnect, or - ObjectNotFound (404 NoSuchKey) from CompleteMultipartUpload(), telling clients the object is absent so they skip retries. Map errFileNotFound at the commit step to a dedicated internal errCommitMetaMissing, convert it to errErasureWriteQuorum in renameData(), and map it to InsufficientWriteQuorum (HTTP 503) in toObjectErr() so clients retry. Below-quorum outcomes already return InsufficientWriteQuorum and are unchanged, as is the IncompleteBody mapping for staging failures of client-disconnected uploads. Signed-off-by: Julien Laurenceau <julien.laurenceau@pepitedata.com>
This commit is contained in:
@@ -20,6 +20,7 @@ package cmd
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/hex"
|
"encoding/hex"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"math/rand"
|
"math/rand"
|
||||||
"reflect"
|
"reflect"
|
||||||
@@ -110,6 +111,35 @@ 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestHashOrder - test order of ints in array
|
// TestHashOrder - test order of ints in array
|
||||||
func TestHashOrder(t *testing.T) {
|
func TestHashOrder(t *testing.T) {
|
||||||
testCases := []struct {
|
testCases := []struct {
|
||||||
|
|||||||
@@ -1072,6 +1072,12 @@ func renameData(ctx context.Context, disks []StorageAPI, srcBucket, srcEntry str
|
|||||||
errs := g.Wait()
|
errs := g.Wait()
|
||||||
|
|
||||||
err := reduceWriteQuorumErrs(ctx, errs, objectOpIgnoredErrs, writeQuorum)
|
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 {
|
if err != nil {
|
||||||
dg := errgroup.WithNErrs(len(disks))
|
dg := errgroup.WithNErrs(len(disks))
|
||||||
for index, nerr := range errs {
|
for index, nerr := range errs {
|
||||||
|
|||||||
@@ -162,7 +162,7 @@ func toObjectErr(oerr error, params ...string) error {
|
|||||||
apiErr.Type = v.Type
|
apiErr.Type = v.Type
|
||||||
}
|
}
|
||||||
return apiErr
|
return apiErr
|
||||||
case errErasureWriteQuorum.Error():
|
case errErasureWriteQuorum.Error(), errCommitMetaMissing.Error():
|
||||||
apiErr := InsufficientWriteQuorum{}
|
apiErr := InsufficientWriteQuorum{}
|
||||||
if len(params) >= 1 {
|
if len(params) >= 1 {
|
||||||
apiErr.Bucket = params[0]
|
apiErr.Bucket = params[0]
|
||||||
|
|||||||
@@ -70,6 +70,11 @@ var errDiskAccessDenied = StorageErr("drive access denied")
|
|||||||
// errFileNotFound - cannot find the file.
|
// errFileNotFound - cannot find the file.
|
||||||
var errFileNotFound = StorageErr("file not found")
|
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.
|
// errFileNotFound - cannot find requested file version.
|
||||||
var errFileVersionNotFound = StorageErr("file version not found")
|
var errFileVersionNotFound = StorageErr("file version not found")
|
||||||
|
|
||||||
|
|||||||
@@ -106,6 +106,8 @@ func toStorageErr(err error) error {
|
|||||||
return errVolumeExists
|
return errVolumeExists
|
||||||
case errFileNotFound.Error():
|
case errFileNotFound.Error():
|
||||||
return errFileNotFound
|
return errFileNotFound
|
||||||
|
case errCommitMetaMissing.Error():
|
||||||
|
return errCommitMetaMissing
|
||||||
case errFileVersionNotFound.Error():
|
case errFileVersionNotFound.Error():
|
||||||
return errFileVersionNotFound
|
return errFileVersionNotFound
|
||||||
case errFileNameTooLong.Error():
|
case errFileNameTooLong.Error():
|
||||||
|
|||||||
@@ -18,6 +18,7 @@
|
|||||||
package cmd
|
package cmd
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"runtime"
|
"runtime"
|
||||||
"syscall"
|
"syscall"
|
||||||
@@ -56,3 +57,28 @@ 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+9
-1
@@ -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.
|
// if its a partial rename() do not attempt to delete recursively.
|
||||||
// this can be healed since all parts are available.
|
// this can be healed since all parts are available.
|
||||||
s.deleteFile(dstVolumeDir, dstDataPath, false, false)
|
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 {
|
if srcVolume != minioMetaMultipartBucket {
|
||||||
|
|||||||
Reference in New Issue
Block a user