diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index 5854fa64d..54511b851 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -1027,7 +1027,6 @@ func putRestoreOpts(rreq *RestoreObjectRequest, objInfo ObjectInfo, opts ObjectO restoreExpiry := lifecycle.ExpectedExpiryTime(time.Now().UTC(), rreq.Days) meta[xhttp.AmzRestore] = completedRestoreObj(restoreExpiry).String() delete(meta, restoreAttemptKey) - delete(meta, restoreUploadKey) return ObjectOptions{ Versioned: opts.Versioned, VersionSuspended: opts.VersionSuspended, diff --git a/cmd/bucket-object-lock.go b/cmd/bucket-object-lock.go index 293042fc3..397b039ef 100644 --- a/cmd/bucket-object-lock.go +++ b/cmd/bucket-object-lock.go @@ -251,9 +251,8 @@ func enforceRetentionBypassForPut(ctx context.Context, r *http.Request, oi Objec // checkObjectLockedForOverwrite re-checks under the object write lock whether an // in-place write to the null version (such as a directory object) would -// overwrite a locked version in an Object Lock bucket. Restore context keeps -// failed reads from deleting the version being rehydrated. -func checkObjectLockedForOverwrite(ctx context.Context, bucket, object string, getObjectInfoFn GetObjectInfoFn, transition TransitionOptions) error { +// overwrite a locked version in an Object Lock bucket. +func checkObjectLockedForOverwrite(ctx context.Context, bucket, object string, getObjectInfoFn GetObjectInfoFn) error { if isMinioMetaBucketName(bucket) { return nil } @@ -266,7 +265,6 @@ func checkObjectLockedForOverwrite(ctx context.Context, bucket, object string, g } objInfo, err := getObjectInfoFn(ctx, bucket, object, ObjectOptions{ VersionID: nullVersionID, - Transition: transition, NoLock: true, NoAuditLog: true, }) diff --git a/cmd/erasure-multipart-cleanup_test.go b/cmd/erasure-multipart-cleanup_test.go index 14c012968..26a95db53 100644 --- a/cmd/erasure-multipart-cleanup_test.go +++ b/cmd/erasure-multipart-cleanup_test.go @@ -123,20 +123,7 @@ func (d *multipartCleanupDisk) Endpoint() Endpoint { func (d *multipartCleanupDisk) ReadVersion(context.Context, string, string, string, string, ReadOptions) (FileInfo, error) { d.reads++ - if d.readErr != nil { - return FileInfo{}, d.readErr - } - fi := newFileInfo("obj", 1, 0) - fi.Erasure.Index, fi.ModTime = 1, d.modTime - return fi, nil -} - -// multipartCleanupSet is a set of one drive, which also answers the -// metadata reads that establish the restore marker of an expired upload. -// Its locks are local: stale cleanup decides a removal under the upload's -// commit lock. -func multipartCleanupSet(disk StorageAPI) erasureObjects { - return erasureObjects{getDisks: func() []StorageAPI { return []StorageAPI{disk} }, nsMutex: newNSLock(false)} + return FileInfo{ModTime: d.modTime}, d.readErr } func (d *multipartCleanupDisk) StatVol(ctx context.Context, volume string) (VolInfo, error) { @@ -156,10 +143,9 @@ func TestCleanupStaleUploadsOnDisk(t *testing.T) { }{ {name: "recent", prefix: uuid, age: expiry / 2, readErr: errFileNotFound}, {name: "plaintext_base64_collision", prefix: "4eacd477-e733-4851-b821-6fd42785ff87", age: expiry / 2, readErr: errFileNotFound}, - // Only an expired upload is read on every drive, for its restore marker. - {name: "expired", prefix: uuid, age: 2 * expiry, readErr: errFileNotFound, wantReads: 1, wantDeleted: true}, + {name: "expired", prefix: uuid, age: 2 * expiry, readErr: errFileNotFound, wantDeleted: true}, {name: "legacy_recent", prefix: uuid, age: expiry / 2, legacy: true, wantReads: 1}, - {name: "legacy_expired", prefix: uuid, age: 2 * expiry, legacy: true, wantReads: 2, wantDeleted: true}, + {name: "legacy_expired", prefix: uuid, age: 2 * expiry, legacy: true, wantReads: 1, wantDeleted: true}, {name: "legacy_metadata_missing", prefix: uuid, age: 2 * expiry, legacy: true, readErr: errFileNotFound, wantReads: 1}, {name: "malformed_metadata_missing", prefix: "not-a-uuid", age: 2 * expiry, readErr: errFileNotFound, wantReads: 1}, } { @@ -180,7 +166,7 @@ func TestCleanupStaleUploadsOnDisk(t *testing.T) { if err := os.Chtimes(filepath.Dir(uploadDir), modTime, modTime); err != nil { t.Fatal(err) } - multipartCleanupSet(disk).cleanupStaleUploadsOnDisk(t.Context(), disk) + erasureObjects{}.cleanupStaleUploadsOnDisk(t.Context(), disk) if disk.reads != tc.wantReads { t.Errorf("ReadVersion calls = %d, want %d", disk.reads, tc.wantReads) } @@ -213,7 +199,7 @@ func TestCleanupStaleUploadsEmptyHashDirectory(t *testing.T) { if err := os.Chtimes(shaDir, modTime, modTime); err != nil { t.Fatal(err) } - multipartCleanupSet(disk).cleanupStaleUploadsOnDisk(t.Context(), disk) + erasureObjects{}.cleanupStaleUploadsOnDisk(t.Context(), disk) if _, err := os.Stat(shaDir); !os.IsNotExist(err) { t.Fatalf("empty hash directory must be removed regardless of age: %v", err) } @@ -238,7 +224,7 @@ func TestCleanupStaleUploadsSharedHashDirectory(t *testing.T) { t.Fatal(err) } - multipartCleanupSet(disk).cleanupStaleUploadsOnDisk(t.Context(), disk) + erasureObjects{}.cleanupStaleUploadsOnDisk(t.Context(), disk) if _, err := os.Stat(filepath.Join(shaDir, expiredID)); !os.IsNotExist(err) { t.Errorf("expired upload must be removed: %v", err) } diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index c4f5d97ff..6d717df26 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -71,7 +71,6 @@ func (er erasureObjects) getMultipartSHADir(bucket, object string) string { } // checkUploadIDExists - verify if a given uploadID exists and is valid. -// The metadata read from each drive is returned also with an error. func (er erasureObjects) checkUploadIDExists(ctx context.Context, bucket, object, uploadID string, write bool) (fi FileInfo, metArr []FileInfo, err error) { defer func() { if errors.Is(err, errFileNotFound) { @@ -89,15 +88,15 @@ func (er erasureObjects) checkUploadIDExists(ctx context.Context, bucket, object readQuorum, writeQuorum, err := objectQuorumFromMeta(ctx, partsMetadata, errs, er.defaultParityCount) if err != nil { - return fi, partsMetadata, err + return fi, nil, err } if readQuorum < 0 { - return fi, partsMetadata, errErasureReadQuorum + return fi, nil, errErasureReadQuorum } if writeQuorum < 0 { - return fi, partsMetadata, errErasureWriteQuorum + return fi, nil, errErasureWriteQuorum } quorum := readQuorum @@ -114,7 +113,7 @@ func (er erasureObjects) checkUploadIDExists(ctx context.Context, bucket, object err = reduceReadQuorumErrs(ctx, errs, objectOpIgnoredErrs, readQuorum) } if err != nil { - return fi, partsMetadata, err + return fi, nil, err } // Pick one from the first valid metadata. @@ -217,55 +216,12 @@ func (er erasureObjects) cleanupStaleUploadsOnDisk(ctx context.Context, disk Sto hasSkippedUploads = true return nil } - // A completion holds this lock from staging its commit in the - // upload until the commit ends: decide on a removal only while - // holding it, and skip an upload whose commit is running. Only - // uploads past the stale age pay for the lock and the read below. - lk := er.restoreCommitLock(uploadIDPath) - lkctx, err := lk.GetLock(ctx, newDynamicTimeout(time.Second, time.Second)) - if err != nil { - hasSkippedUploads = true - return nil - } - // The rename below releases the lock when it ends, also when it - // outlives its deadline; every other exit releases it here. - renaming := false - defer func() { - if !renaming { - lk.Unlock(lkctx) - } - }() - // A native restore can run past the stale age: keep its upload - // while its attempt still owns the version, and keep any upload - // whose marker the drives cannot establish. - view := er.readRestoreUpload(lkctx.Context(), "", uploadIDPath) - if view.committing { - // What a commit staged, with no commit running: only a staged - // copy itself shows when it staged, only an old one is a - // leftover of a failed commit, and a copy not staged yet - // tells nothing. - local, err := disk.ReadVersion(lkctx.Context(), "", minioMetaMultipartBucket, uploadIDPath, "", ReadOptions{}) - st, serr := os.Stat(pathJoin(drivePath, minioMetaMultipartBucket, uploadIDPath, xlStorageFormatFile)) - if err != nil || serr != nil || !restoreUploadStaged(local) || time.Since(st.ModTime()) < restoreCommitGrace { - hasSkippedUploads = true - return nil - } - } else if view.live && (!view.known || (view.marker != nil && er.restoreUploadActive(lkctx.Context(), view))) { - hasSkippedUploads = true - return nil - } - renaming = true w := xioutil.NewDeadlineWorker(globalDriveConfig.GetMaxTimeout()) return w.Run(func() error { wait := deleteMultipartCleanupSleeper.Timer(ctx) pathUUID := mustGetUUID() targetPath := pathJoin(drivePath, minioMetaTmpDeletedBucket, pathUUID) - err := renameAll(pathJoin(drivePath, minioMetaMultipartBucket, uploadIDPath), targetPath, pathJoin(drivePath, minioMetaBucket)) - lk.Unlock(lkctx) - if err != nil { - // The next cleanup round tries again. - storageLogIf(ctx, fmt.Errorf("Unable to remove stale upload %s on %s: %w", uploadIDPath, drivePath, err)) - } + renameAll(pathJoin(drivePath, minioMetaMultipartBucket, uploadIDPath), targetPath, pathJoin(drivePath, minioMetaBucket)) wait() return nil }) @@ -506,13 +462,6 @@ func (er erasureObjects) listMultipartUploadsExact(ctx context.Context, bucket, if populatedUploadIDs.Contains(uploadID) { continue } - // A native restore's upload is private to its attempt; list only an - // upload that the drives establish as an ordinary one. - metas, errs := readAllFileInfo(ctx, er.getDisks(), bucket, minioMetaMultipartBucket, - pathJoin(er.getMultipartSHADir(bucket, object), uploadID), "", false, false) - if !restoreUploadState(metas, errs).visible(ObjectOptions{}) { - continue - } var fallback time.Time if _, ok := multipartUploadTime(uploadID); !ok { fi, err := disk.ReadVersion(ctx, bucket, minioMetaMultipartBucket, @@ -612,11 +561,6 @@ func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string, if opts.PreserveETag != "" { userDefined["etag"] = opts.PreserveETag } - // Only a native restore marks its upload private, never caller metadata. - delete(userDefined, restoreUploadKey) - if a := opts.Transition.attempt; a != nil { - userDefined[restoreUploadKey] = a.String() - } onlineDisks := er.getDisks() // Get parity and data drive count based on storage class metadata @@ -798,10 +742,7 @@ func (er erasureObjects) PutObjectPart(ctx context.Context, bucket, object, uplo uploadIDPath := er.getUploadIDDir(bucket, object, uploadID) // Validates if upload ID exists. - fi, metas, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, true) - if err == nil && !restoreUploadVisible(metas, opts) { - err = errUploadIDNotFound - } + fi, _, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, true) if err != nil { if errors.Is(err, errVolumeNotFound) { return pi, toObjectErr(err, bucket) @@ -1038,10 +979,7 @@ func (er erasureObjects) GetMultipartInfo(ctx context.Context, bucket, object, u UploadID: uploadID, } - fi, metas, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, false) - if err == nil && !restoreUploadVisible(metas, opts) { - err = errUploadIDNotFound - } + fi, _, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, false) if err != nil { if errors.Is(err, errVolumeNotFound) { return result, toObjectErr(err, bucket) @@ -1119,10 +1057,7 @@ func (er erasureObjects) ListObjectParts(ctx context.Context, bucket, object, up auditObjectErasureSet(ctx, "ListObjectParts", object, &er) } - fi, metas, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, false) - if err == nil && !restoreUploadVisible(metas, opts) { - err = errUploadIDNotFound - } + fi, _, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, false) if err != nil { return result, toObjectErr(err, bucket, object, uploadID) } @@ -1335,7 +1270,7 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str auditObjectErasureSet(ctx, "CompleteMultipartUpload", object, &er) } - if opts.CheckPrecondFn != nil || opts.ReplicaLockReconcile || opts.Transition.attempt != nil { + if opts.CheckPrecondFn != nil || opts.ReplicaLockReconcile { if !opts.NoLock { ns := er.NewNSLock(bucket, object) lkctx, err := ns.GetLock(ctx, globalOperationTimeout) @@ -1368,24 +1303,7 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str } } - if opts.Transition.attempt != nil { - // A native restore holds the upload write lock from validation - // through commit, after the object lock and in the lock domain of - // PutObjectPart, which takes a part lock and then this lock to read. - // NoLock only says that the caller holds the object lock. - uploadIDLock := er.NewNSLock(bucket, pathJoin(object, uploadID)) - ulkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout) - if err != nil { - return oi, err - } - ctx = ulkctx.Context() - defer uploadIDLock.Unlock(ulkctx) - } - fi, partsMetadata, err := er.checkUploadIDExists(ctx, bucket, object, uploadID, true) - if err == nil && !restoreUploadVisible(partsMetadata, opts) { - err = errUploadIDNotFound - } if err != nil { if errors.Is(err, errVolumeNotFound) { return oi, toObjectErr(err, bucket) @@ -1699,7 +1617,6 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str delete(fi.Metadata, hash.MinIOMultipartChecksumType) delete(fi.Metadata, multipartMetaBucket) delete(fi.Metadata, multipartMetaObject) - delete(fi.Metadata, restoreUploadKey) // Save the final object size and modtime. fi.Size = objectSize @@ -1776,36 +1693,17 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str getObjectInfo = opts.replicaObjectInfo } if fi.VersionID == "" || fi.VersionID == nullVersionID { - if err := checkObjectLockedForOverwrite(ctx, bucket, object, getObjectInfo, opts.Transition); err != nil { + if err := checkObjectLockedForOverwrite(ctx, bucket, object, getObjectInfo); err != nil { return oi, toObjectErr(err, bucket, object) } - if a := opts.Transition.attempt; a != nil { - // A native restore writes back only the null version it targets. - current, gerr := getObjectInfo(ctx, bucket, object, ObjectOptions{ - VersionID: nullVersionID, - Transition: opts.Transition, - NoLock: true, - }) - if gerr != nil { - return oi, toObjectErr(gerr, bucket, object) - } - if !a.rehydrates(current, fi) { - return oi, toObjectErr(errAuthentication, bucket, object) - } - } } else { - current, gerr := getObjectInfo(ctx, bucket, object, ObjectOptions{ + _, gerr := getObjectInfo(ctx, bucket, object, ObjectOptions{ VersionID: fi.VersionID, Versioned: opts.Versioned, VersionSuspended: opts.VersionSuspended, - Transition: opts.Transition, NoLock: true, }) switch { - case gerr == nil && opts.Transition.attempt.rehydrates(current, fi): - // A native restore's own private upload rehydrates this same - // tiered version. Any other completion must still reject a - // version overwrite. case gerr == nil, errors.As(gerr, &MethodNotAllowed{}): return oi, toObjectErr(errAuthentication, bucket, object) case isErrVersionNotFound(gerr) || isErrObjectNotFound(gerr): @@ -1815,16 +1713,6 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str } } - // Stale cleanup and abort judge an upload whose commit staged object - // metadata in it by this lock: hold it from staging through the commit. - commitLock := er.restoreCommitLock(uploadIDPath) - clkctx, err := commitLock.GetLock(ctx, globalOperationTimeout) - if err != nil { - return oi, err - } - ctx = clkctx.Context() - defer commitLock.Unlock(clkctx) - er.cleanupMultipartPath(ctx, paths...) // cleanup all part.N.meta, and skipped part.N's before final rename(). defer func() { @@ -1896,38 +1784,7 @@ func (er erasureObjects) abortMultipartUpload(ctx context.Context, bucket, objec if !ok || internalID == "" || internalID == "." || internalID == ".." || strings.ContainsAny(internalID, "/\\") { return false, InvalidUploadID{Bucket: bucket, Object: object, UploadID: uploadID} } - // Authorize before the unconditional cleanup below. A native restore's - // upload is absent to every other caller, and an upload whose marker the - // drives cannot establish is not removed by anyone. - uploadIDPath := er.getUploadIDDir(bucket, object, uploadID) - view := er.readRestoreUpload(ctx, bucket, uploadIDPath) - if view.committing { - // What a failed commit left behind, also after stale cleanup could - // not remove it: only with no commit running, and once every copy - // shows a staging older than the grace. - lk := er.restoreCommitLock(uploadIDPath) - lkctx, err := lk.GetLock(ctx, newDynamicTimeout(restoreTakeoverWait, restoreTakeoverWait)) - if err != nil { - return false, toObjectErr(errErasureReadQuorum, bucket, object, uploadID) - } - defer lk.Unlock(lkctx) - if !er.restoreCommitAbandoned(lkctx.Context(), bucket, uploadIDPath) { - return false, toObjectErr(errErasureReadQuorum, bucket, object, uploadID) - } - er.deleteAll(lkctx.Context(), minioMetaMultipartBucket, uploadIDPath) - return true, nil - } - switch { - case view.visible(opts): - case view.known && view.marked: - return false, nil - case view.known && opts.Transition.attempt != nil: - // A native restore's own abort, of an upload no longer its own. - return false, nil - default: - return false, toObjectErr(errErasureReadQuorum, bucket, object, uploadID) - } - defer er.deleteAll(ctx, minioMetaMultipartBucket, uploadIDPath) + defer er.deleteAll(ctx, minioMetaMultipartBucket, er.getUploadIDDir(bucket, object, uploadID)) _, _, err = er.checkUploadIDExists(ctx, bucket, object, uploadID, false) err = toObjectErr(err, bucket, object, uploadID) if _, absent := err.(InvalidUploadID); absent { diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index f3c8cfb42..8a0de02ad 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -45,7 +45,6 @@ import ( "github.com/minio/minio/internal/crypto" "github.com/minio/minio/internal/event" "github.com/minio/minio/internal/grid" - "github.com/minio/minio/internal/hash" xhttp "github.com/minio/minio/internal/http" xioutil "github.com/minio/minio/internal/ioutil" "github.com/minio/minio/internal/logger" @@ -156,7 +155,7 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d if dstOpts.replicaObjectInfo != nil { getObjectInfo = dstOpts.replicaObjectInfo } - if err := checkObjectLockedForOverwrite(ctx, dstBucket, dstObject, getObjectInfo, dstOpts.Transition); err != nil { + if err := checkObjectLockedForOverwrite(ctx, dstBucket, dstObject, getObjectInfo); err != nil { return oi, toObjectErr(err, dstBucket, dstObject) } } @@ -1673,7 +1672,7 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st if opts.replicaObjectInfo != nil { getObjectInfo = opts.replicaObjectInfo } - if err := checkObjectLockedForOverwrite(ctx, bucket, object, getObjectInfo, opts.Transition); err != nil { + if err := checkObjectLockedForOverwrite(ctx, bucket, object, getObjectInfo); err != nil { return ObjectInfo{}, toObjectErr(err, bucket, object) } } @@ -2665,180 +2664,6 @@ func (er erasureObjects) TransitionObject(ctx context.Context, bucket, object st return err } -// RestoreTransitionedObject - restore transitioned object content locally on this cluster. -// This is similar to PostObjectRestore from AWS GLACIER -// storage class. When PostObjectRestore API is called, a temporary copy of the object -// is restored locally to the bucket on source cluster until the restore expiry date. -// The copy that was transitioned continues to reside in the transitioned tier. -func (er erasureObjects) RestoreTransitionedObject(ctx context.Context, bucket, object string, opts ObjectOptions) error { - return er.restoreTransitionedObject(ctx, bucket, object, opts) -} - -// getRestoreLock waits for a restore's object lock for as long as ctx lives. -// The POST handler has already marked the version ongoing, and only the -// restore clears that marker. A restore queued behind another version's -// download must therefore keep waiting rather than give up after one timeout. -func getRestoreLock(ctx context.Context, lk RWLocker) (LockContext, error) { - for { - lkctx, err := lk.GetLock(ctx, globalRestoreLockTimeout) - if !errors.As(err, &OperationTimedOut{}) || ctx.Err() != nil { - return lkctx, err - } - } -} - -// restoreTransitionedObject for multipart object chunks the file stream from remote tier into the same number of parts -// as in the xl.meta for this version and rehydrates the part.n into the fi.DataDir for this version as in the xl.meta -// -// A POST restore publishes its attempt on the version's ongoing marker. The -// restore rehydrates the version only while that attempt owns the marker, and -// on failure clears the marker only while it still does. A direct caller -// publishes no attempt; its attempt only names the private upload. -func (er erasureObjects) restoreTransitionedObject(ctx context.Context, bucket string, object string, opts ObjectOptions) error { - if !opts.NoLock { - // Single-pool and direct callers must protect the metadata snapshot - // through download, rehydration and failure cleanup, just like pools. - lk := er.NewNSLock(bucket, object) - lkctx, err := getRestoreLock(ctx, lk) - if err != nil { - return restoreResetError{err} - } - ctx = lkctx.Context() - defer lk.Unlock(lkctx) - opts.NoLock = true - } - - published := opts.Transition.attempt - // Restore the authoritative generation of the version, never a revision - // that only some drives hold. - gen, err := er.readRestoreGeneration(ctx, bucket, object, opts.VersionID) - if err != nil { - err = toObjectErr(err, bucket, object, opts.VersionID) - if isErrObjectNotFound(err) || isErrVersionNotFound(err) { - // No version was read, so there is no restore marker to clear. - return err - } - return restoreResetError{err} - } - actualfi := gen.fi - if actualfi.Deleted { - return toObjectErr(errMethodNotAllowed, bucket, object) - } - - oi := actualfi.ToObjectInfo(bucket, object, opts.Versioned || opts.VersionSuspended) - attempt := published - if attempt == nil { - attempt = newRestoreAttempt(oi) - } else if !attempt.owns(oi) { - // A later POST took the version over, or it was replaced. - return errRestoreSuperseded - } - opts.Transition.attempt = attempt - versionID := restoreVersionID(actualfi.VersionID) - fail := func(cause error) error { - return er.restoreFailed(ctx, bucket, object, versionID, published, cause) - } - - ropts := putRestoreOpts(opts.Transition.RestoreRequest, oi, opts) - ropts.NoLock = opts.NoLock - ropts.Transition = opts.Transition - // Rehydration copies encoded bytes without changing their content identity. - ropts.PreserveETag = oi.ETag - // Even a one-part multipart upload must retain its original part number: - // encrypted multipart data uses that number to derive its decryption key. - if len(oi.Parts) == 1 && !oi.isMultipart() { - actualSize, err := oi.GetActualSize() - if err != nil { - return fail(err) - } - ropts.IndexCB = func() []byte { return oi.Parts[0].Index } - var rs *HTTPRangeSpec - gr, err := getTransitionedObjectReader(ctx, bucket, object, rs, http.Header{}, oi, opts) - if err != nil { - return fail(toObjectErr(err, bucket, object)) - } - defer gr.Close() - hashReader, err := hash.NewReader(ctx, gr, gr.ObjInfo.Size, "", "", actualSize) - if err != nil { - return fail(toObjectErr(err, bucket, object)) - } - pReader := NewPutObjReader(hashReader) - if _, err = er.PutObject(ctx, bucket, object, pReader, ropts); err != nil { - return fail(toObjectErr(err, bucket, object)) - } - return nil - } - - // Every server must keep the upload private, also a queued restore's. - if err := nativeRestoreAvailable(ctx); err != nil { - return fail(err) - } - // The upload records when it starts, which the stale-upload cleanup ages; - // completion still commits the version's own ModTime. - uopts := ropts - uopts.MTime = time.Time{} - res, err := er.NewMultipartUpload(ctx, bucket, object, uopts) - if err != nil { - return fail(err) - } - completed := false - defer func() { - if !completed { - er.abortRestoreUpload(bucket, object, res.UploadID, attempt) - } - }() - - var uploadedParts []CompletePart - var rs *HTTPRangeSpec - // get reader from the warm backend - note that even in the case of encrypted objects, this stream is still encrypted. - gr, err := getTransitionedObjectReader(ctx, bucket, object, rs, http.Header{}, oi, opts) - if err != nil { - return fail(err) - } - defer gr.Close() - - // rehydrate the parts back on disk as per the original xl.meta prior to transition - for _, partInfo := range oi.Parts { - // Old uncompressed parts may not record ActualSize. Encrypted parts - // derive it from the written ciphertext in PutObjectPart; compressed - // parts need their recorded logical length, including valid zero sizes. - actualSize := partInfo.Size - if oi.IsCompressed() { - actualSize = partInfo.ActualSize - } - hr, err := hash.NewReader(ctx, io.LimitReader(gr, partInfo.Size), partInfo.Size, "", "", actualSize) - if err != nil { - return fail(err) - } - pInfo, err := er.PutObjectPart(ctx, bucket, object, res.UploadID, partInfo.Number, NewPutObjReader(hr), ObjectOptions{ - IndexCB: func() []byte { return partInfo.Index }, - Transition: TransitionOptions{attempt: attempt}, - }) - if err != nil { - return fail(err) - } - if pInfo.Size != partInfo.Size { - return fail(InvalidObjectState{Bucket: bucket, Object: object}) - } - uploadedParts = append(uploadedParts, CompletePart{ - PartNumber: pInfo.PartNumber, - ETag: pInfo.ETag, - }) - } - _, err = er.CompleteMultipartUpload(ctx, bucket, object, res.UploadID, uploadedParts, ObjectOptions{ - MTime: oi.ModTime, - NoLock: opts.NoLock, - Versioned: opts.Versioned, - VersionSuspended: opts.VersionSuspended, - Transition: opts.Transition, - }) - if err != nil { - return fail(err) - } - completed = true - return nil -} - // DecomTieredObject - moves tiered object to another pool during decommissioning. func (er erasureObjects) DecomTieredObject(ctx context.Context, bucket, object string, fi FileInfo, opts ObjectOptions) error { if opts.UserDefined == nil { diff --git a/cmd/erasure-server-pool-lock-domain_test.go b/cmd/erasure-server-pool-lock-domain_test.go index 85b0360e1..13249b213 100644 --- a/cmd/erasure-server-pool-lock-domain_test.go +++ b/cmd/erasure-server-pool-lock-domain_test.go @@ -760,8 +760,10 @@ func TestLockDomainBulkDeleteMapping(t *testing.T) { } } -// A restore holds the object lock of its set for its download. DELETE and COPY -// of the object must wait for it. +// A restore takes the object lock of its set only to commit, not for its +// download. DELETE and COPY of the object wait for that commit, also where +// the sets of the pool lock with lockers of their own, and the restore still +// completes. func TestLockDomainRestoreExcludesDeleteAndCopy(t *testing.T) { for _, mode := range ldModes { for _, op := range []string{"delete", "copy"} { @@ -769,60 +771,41 @@ func TestLockDomainRestoreExcludesDeleteAndCopy(t *testing.T) { r := newRestoreSafetyTest(t, 1, versioning.Enabled) ldSplit(t, r.z, mode) r.object = ldKey(t, r.z, "restore-"+op, 1) - oi, _ := nativeRestoreSeed(t, r, 0, false, false) - blocked := &nativeRestoreBlockedTier{WarmBackend: globalTierConfigMgr.drivercache["WARM"], entered: make(chan struct{}), release: make(chan struct{})} - globalTierConfigMgr.drivercache["WARM"] = blocked - var once sync.Once - unblock := func() { once.Do(func() { close(blocked.release) }) } - defer unblock() - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) - defer cancel() - restored := make(chan error, 1) - go func() { - restored <- r.z.RestoreTransitionedObject(ctx, r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) - }() - select { - case <-blocked.entered: - case <-ctx.Done(): - t.Fatal("restore never started its download") - } - done := make(chan error, 1) - go func() { + oi, body := nativeRestoreSeed(t, r, 0, false, false) + waited, restoreErr, opErr := ldOverlapRun(t, func(ctx context.Context) error { + return r.z.RestoreTransitionedObject(ctx, r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + }, func(ctx context.Context) error { switch op { case "delete": _, err := r.z.DeleteObject(ctx, r.bucket, r.object, r.opts("")) - done <- err - case "copy": + return err + default: ropts := r.opts(oi.VersionID) ropts.NoLock = true cur, err := r.z.GetObjectInfo(ctx, r.bucket, r.object, ropts) if err != nil { - done <- err - return + return err } cur.metadataOnly = true cur.UserDefined = cloneMSS(cur.UserDefined) cur.UserDefined["x-amz-meta-copied"] = "1" vopts := r.opts(oi.VersionID) _, err = r.z.CopyObject(ctx, r.bucket, r.object, r.bucket, r.object, cur, vopts, vopts) - done <- err + return err } - }() - var opErr error - select { - case opErr = <-done: - t.Errorf("%s ran during the restore (%v)", op, opErr) - case <-time.After(ldWait): - unblock() - opErr = <-done + }) + if !waited { + t.Errorf("%s ran during the restore's commit (%v)", op, opErr) } - unblock() - if err := <-restored; err != nil { - t.Fatalf("restore: %v", err) + if restoreErr != nil { + t.Fatalf("restore: %v", restoreErr) } if opErr != nil { t.Fatalf("%s after the restore: %v", op, opErr) } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } }) } } diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index 221156480..0811edebc 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -2393,22 +2393,21 @@ func (z *erasureServerPools) CompleteMultipartUpload(ctx context.Context, bucket } }() - // Keep object -> upload lock ordering for single-pool completion too: - // native restore holds the object lock while publishing upload parts. - // Both locks are in the object's lock domain, the set-local one of - // single-pool writes. - if !opts.NoLock { - lk := z.NewObjectNSLock(bucket, encodeDirObject(object)) - lkctx, err := lk.GetLock(ctx, globalOperationTimeout) - if err != nil { - return objInfo, err - } - ctx = lkctx.Context() - defer lk.Unlock(lkctx) - } - opts.NoLock = true - + // Several pools take the object lock first, then the upload lock below. + // A single pool takes the upload lock here and the set takes the object + // lock, in the object's own erasure set, only right before it commits. if !z.SinglePool() { + if !opts.NoLock { + lk := z.NewObjectNSLock(bucket, encodeDirObject(object)) + lkctx, err := lk.GetLock(ctx, globalOperationTimeout) + if err != nil { + return objInfo, err + } + ctx = lkctx.Context() + defer lk.Unlock(lkctx) + } + opts.NoLock = true + // A conditional completion must be evaluated against the logical // latest object across pools, under the object write lock held for // this operation. The pool hosting the upload may only hold a stale @@ -2460,8 +2459,9 @@ func (z *erasureServerPools) CompleteMultipartUpload(ctx context.Context, bucket // Hold write locks to verify uploaded parts, also disallows any // parallel PutObjectPart() requests. - // An empty upload ID aliases the object lock already held above. - // Let the storage layer return its normal missing bucket/upload error. + // An empty upload ID would name the object lock itself, which this + // completion holds or takes later: let the storage layer return its + // normal missing bucket/upload error instead. if uploadID != "" { uploadIDLock := z.uploadIDNSLock(bucket, object, uploadID) wlkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout) @@ -3398,48 +3398,26 @@ func (z *erasureServerPools) TransitionObject(ctx context.Context, bucket, objec return z.serverPools[idx].TransitionObject(ctx, bucket, object, opts) } -// RestoreTransitionedObject - restore transitioned object content locally on this cluster. -func (z *erasureServerPools) RestoreTransitionedObject(ctx context.Context, bucket, object string, opts ObjectOptions) (err error) { - object = encodeDirObject(object) - defer func() { - // The restore stopped before it could clear POST's ongoing marker. - var reset restoreResetError - if errors.As(err, &reset) { - z.recoverRestoreStatus(bucket, object, opts) - err = reset.error - } - }() - if z.SinglePool() { - return z.serverPools[0].RestoreTransitionedObject(ctx, bucket, object, opts) - } - - if !opts.NoLock { - // Acquire write lock before restoring transitioned object - lk := z.NewNSLock(bucket, object) - lkctx, err := getRestoreLock(ctx, lk) - if err != nil { - return restoreResetError{err} - } - ctx = lkctx.Context() - defer lk.Unlock(lkctx) - } - - // Avoid restoring object from a pool being decommissioned. - opts.SkipDecommissioned = true - opts.NoLock = true - idx, err := z.restorePoolIdx(ctx, bucket, object, opts) - if err != nil { - return err - } - - return z.serverPools[idx].RestoreTransitionedObject(ctx, bucket, object, opts) -} - func (z *erasureServerPools) CheckAbandonedParts(ctx context.Context, bucket, object string, opts madmin.HealOpts) error { object = encodeDirObject(object) if z.SinglePool() { return z.serverPools[0].CheckAbandonedParts(ctx, bucket, object, opts) } + // Several pools: remove abandoned data under the pools lock, which every + // write of the object takes, as healObjectInPool does. The lock of one + // pool's set would not exclude a write that has moved its data directory + // in place but not yet published the metadata that references it, such + // as a native restore's commit. + if opts.Remove && !opts.DryRun && !opts.NoLock { + lk := z.NewObjectNSLock(bucket, object) + lkctx, err := lk.GetLock(ctx, globalOperationTimeout) + if err != nil { + return err + } + ctx = lkctx.Context() + defer lk.Unlock(lkctx) + opts.NoLock = true + } errs := make([]error, len(z.serverPools)) var wg sync.WaitGroup for idx, pool := range z.serverPools { diff --git a/cmd/object-api-interface.go b/cmd/object-api-interface.go index 11e867171..307268adb 100644 --- a/cmd/object-api-interface.go +++ b/cmd/object-api-interface.go @@ -178,9 +178,14 @@ type TransitionOptions struct { RestoreExpiry time.Time ExpireRestored bool - // attempt is the native restore a call belongs to. Only that restore - // sees its private upload, see restoreUploadVisible. + // attempt is the native restore a call belongs to: the attempt that + // POST marked on the version, see restoreRequest. attempt *restoreAttempt + // owner is the restore lock of the version, which POST hands to the + // worker that runs attempt, see erasureServerPools.RestoreTransitionedObject. + owner *restoreOwner + // restoreDate is the request date POST recorded with attempt. + restoreDate string } // MakeBucketOptions represents bucket options for ObjectLayer bucket operations diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index a1133be7e..4f9100df2 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -3988,8 +3988,8 @@ func (api objectAPIHandlers) PostRestoreObjectHandler(w http.ResponseWriter, r * // Preserve a failed restore read even before the request body is parsed. opts.Transition.RestoreRequest = &RestoreObjectRequest{} - // Resolve the target without the object lock: a running restore holds it - // for its download, and the status step bounds its own wait for it. + // Resolve the target without the object lock; the status step below + // takes the restore lock and then the object lock. preOpts := opts preOpts.NoLock = true objInfo, err := getObjectInfo(ctx, bucket, object, preOpts) @@ -4023,10 +4023,15 @@ func (api objectAPIHandlers) PostRestoreObjectHandler(w http.ResponseWriter, r * restoreExpiry := lifecycle.ExpectedExpiryTime(time.Now().UTC(), rreq.Days) // update self with restore metadata - var attempt *restoreAttempt + var ( + attempt *restoreAttempt + owner *restoreOwner + restoreDate string + ) if rreq.Type != SelectRestoreRequest { - // Native restore needs every server to keep its upload private; - // refuse it, writing nothing, while that is not established. + // Native restore needs every server to follow the restore lock and + // the staged commit; refuse it, writing nothing, while that is not + // established. if err := nativeRestoreAvailable(ctx); err != nil { apiErr := errorCodes.ToAPIErr(ErrBusy) apiErr.Code, apiErr.Description = "ServiceUnavailable", err.Error() @@ -4060,6 +4065,8 @@ func (api objectAPIHandlers) PostRestoreObjectHandler(w http.ResponseWriter, r * return } attempt = status.attempt + // The restore lock goes to the worker below, which releases it. + owner, restoreDate = status.owner, status.date } restoreObject := mustGetUUID() @@ -4079,6 +4086,9 @@ func (api objectAPIHandlers) PostRestoreObjectHandler(w http.ResponseWriter, r * // now process the restore in background go func() { rctx := GlobalContext + // The restore releases the lock it was handed; this covers a path + // that does not reach it. Releasing twice is a no-op. + defer owner.release() if !rreq.SelectParameters.IsEmpty() { actualSize, err := objInfo.GetActualSize() if err != nil { @@ -4130,6 +4140,8 @@ func (api objectAPIHandlers) PostRestoreObjectHandler(w http.ResponseWriter, r * RestoreRequest: rreq, RestoreExpiry: restoreExpiry, attempt: attempt, + owner: owner, + restoreDate: restoreDate, }, VersionID: attempt.VersionID, Versioned: opts.Versioned, diff --git a/cmd/object-retention-overwrite_test.go b/cmd/object-retention-overwrite_test.go index 7c864e733..83ce43ebe 100644 --- a/cmd/object-retention-overwrite_test.go +++ b/cmd/object-retention-overwrite_test.go @@ -57,7 +57,7 @@ func TestObjectLockOverwriteConfigUnavailable(t *testing.T) { err := checkObjectLockedForOverwrite(t.Context(), "lock-config-unavailable", "object", func(context.Context, string, string, ObjectOptions) (ObjectInfo, error) { t.Fatal("object must not be read without a bucket configuration") return ObjectInfo{}, nil - }, TransitionOptions{}) + }) if !errors.Is(err, errServerNotInitialized) { t.Fatalf("unavailable lock configuration must fail closed, got %v", err) } diff --git a/cmd/restore-attempt.go b/cmd/restore-attempt.go index e95903dd3..ec2d01ecb 100644 --- a/cmd/restore-attempt.go +++ b/cmd/restore-attempt.go @@ -24,6 +24,7 @@ import ( "fmt" "maps" "net/http" + "slices" "strconv" "strings" "time" @@ -33,24 +34,16 @@ import ( xhttp "github.com/minio/minio/internal/http" ) -// A native restore keeps two private records. The version that POST marks -// ongoing names the attempt that owns the marker; that attempt's multipart -// upload names the attempt and the version it rehydrates. Clients cannot -// set either: requests with reserved metadata are refused, it is never -// extracted from headers, and uploads only get the marker from the restore. -const ( - restoreAttemptKey = ReservedMetadataPrefixLower + "restore-attempt" - restoreUploadKey = ReservedMetadataPrefixLower + "restore-upload" -) +// The version that POST marks ongoing names the native restore attempt that +// owns the marker. Clients cannot set it: requests with reserved metadata are +// refused, and it is never extracted from headers. +const restoreAttemptKey = ReservedMetadataPrefixLower + "restore-attempt" const ( - // restoreTakeoverWait bounds how long a POST on an ongoing version waits - // for the object lock. A running restore holds it for its download. + // restoreTakeoverWait bounds how long a POST waits for the restore lock + // of its version. A running restore holds it for its whole run. restoreTakeoverWait = 2 * time.Second - // restoreCommitGrace is how long stale cleanup keeps an upload whose - // completion staged object metadata in it; a commit takes seconds. - restoreCommitGrace = time.Hour - // restoreCleanupTimeout bounds each status reset and upload abort. + // restoreCleanupTimeout bounds each status reset and convergence pass. restoreCleanupTimeout = time.Minute ) @@ -63,7 +56,7 @@ var ( // errRestoreSuperseded: the version no longer belongs to this attempt. // The attempt must not rehydrate, complete or reset anything. errRestoreSuperseded = errors.New("restore attempt superseded") - // errRestoreInProgress: a restore holds the object lock of the version. + // errRestoreInProgress: a restore holds the restore lock of the version. errRestoreInProgress = errors.New("restore already in progress") // errRestoreUnchanged: a restore status update found nothing to write. errRestoreUnchanged = errors.New("restore status unchanged") @@ -107,19 +100,6 @@ func (a *restoreAttempt) String() string { return string(b) } -func parseRestoreAttempt(s string) (*restoreAttempt, bool) { - var a restoreAttempt - if s == "" || json.Unmarshal([]byte(s), &a) != nil || a.ID == "" || a.VersionID == "" { - return nil, false - } - return &a, true -} - -func (a *restoreAttempt) equal(b *restoreAttempt) bool { - return a != nil && b != nil && a.ID == b.ID && a.VersionID == b.VersionID && - a.ETag == b.ETag && a.Size == b.Size && a.ModTime.Equal(b.ModTime) -} - // targets reports whether oi is still the version the attempt was made for. // A replacement under the same version ID differs in ETag, size or ModTime. func (a *restoreAttempt) targets(oi ObjectInfo) bool { @@ -133,162 +113,16 @@ func (a *restoreAttempt) owns(oi ObjectInfo) bool { return a.targets(oi) && oi.RestoreOngoing && oi.UserDefined[restoreAttemptKey] == a.ID } -// rehydrates reports whether completing upload fi, the attempt's private -// upload, writes back the very version current. -func (a *restoreAttempt) rehydrates(current ObjectInfo, fi FileInfo) bool { - return a.targets(current) && fi.Metadata["etag"] == a.ETag && fi.Size == a.Size && fi.ModTime.Equal(a.ModTime) -} - -// restoreUploadView is the restore marker of an upload as the drives of -// its set establish it. A marked upload is written to a write quorum, at -// least a majority of the drives, and its copies never change their marker. -type restoreUploadView struct { - // marker is the attempt of a native restore's upload: one marker that a - // majority of the drives hold. - marker *restoreAttempt - // marked: some readable drive holds a marker. - marked bool - // known: the marker state is established. Without it, callers fail - // closed: nobody lists, uses, aborts or removes the upload. - known bool - // live: enough drives hold the upload or cannot tell that a written - // upload may exist; otherwise only remnants of one are left. - live bool - // committing: some drive holds object metadata in the upload's place. - // Completion stages the committed version's object metadata there, - // without the marker, before it renames the upload into the object. - committing bool - // bucket and object the upload of an established marker belongs to. - bucket, object string -} - -// restoreUploadState establishes the marker of an upload from the metadata of -// every drive. errs may be nil, then a drive without valid metadata is one -// that cannot tell. A marker is established on a majority of the drives; its -// absence only when no readable drive shows one and the drives that cannot -// tell are too few to hold a marked upload. -func restoreUploadState(metas []FileInfo, errs []error) restoreUploadView { - n := len(metas) - majority := n/2 + 1 - markers := make(map[string]int) - var unmarked, unknown, committing int - for i, fi := range metas { - if errs != nil && errs[i] != nil { - if !errors.Is(errs[i], errFileNotFound) && !errors.Is(errs[i], errFileVersionNotFound) && - !errors.Is(errs[i], errVolumeNotFound) && !errors.Is(errs[i], errUploadIDNotFound) { - unknown++ - } - continue - } - if !fi.IsValid() { - unknown++ - continue - } - if restoreUploadStaged(fi) { - committing++ - continue - } - if m := fi.Metadata[restoreUploadKey]; m != "" { - markers[m]++ - } else { - unmarked++ - } +// ownsGeneration reports whether the attempt owns the authoritative +// generation of the version. Where the drives disagree only in their restore +// fields, see restoreGeneration, it owns the version only when its marker is +// the only ongoing marker there: holding one of several is not enough. +func (a *restoreAttempt) ownsGeneration(gen restoreGeneration, bucket, object string) bool { + oi := gen.fi.ToObjectInfo(bucket, object, false) + if !gen.split { + return a.owns(oi) } - present := unmarked - for _, c := range markers { - present += c - } - v := restoreUploadView{marked: len(markers) > 0, live: present+unknown+committing >= majority, committing: committing > 0} - if v.committing { - // Nobody but the completion may use or remove it now. - return v - } - for m, c := range markers { - if c >= majority { - v.marker, v.known = parseRestoreAttempt(m) - if v.known { - // The marker also names the upload's own target. - for _, fi := range metas { - if !fi.IsValid() || fi.Metadata[restoreUploadKey] != m { - continue - } - if restoreVersionID(fi.VersionID) != v.marker.VersionID || fi.Metadata["etag"] != v.marker.ETag { - v.marker, v.known = nil, false - break - } - v.bucket, v.object = fi.Metadata[multipartMetaBucket], fi.Metadata[multipartMetaObject] - } - } - return v - } - } - // A marker without a majority is established only as a remnant. - v.known = !v.live || (!v.marked && unknown < majority) - return v -} - -// restoreUploadStaged reports whether an upload's record is object metadata -// that a completion staged there: an upload record has no parts and no -// other versions. -func restoreUploadStaged(fi FileInfo) bool { - return fi.Deleted || len(fi.Parts) > 0 || fi.NumVersions > 1 -} - -// visible reports whether a caller sees the upload: a native restore its own -// upload, any other caller only an upload established as unmarked. -func (v restoreUploadView) visible(opts ObjectOptions) bool { - if a := opts.Transition.attempt; a != nil { - return v.marker.equal(a) - } - return v.known && !v.marked -} - -// restoreUploadVisible reports whether a caller sees an upload whose metadata -// is metas, one entry for each drive. -func restoreUploadVisible(metas []FileInfo, opts ObjectOptions) bool { - return restoreUploadState(metas, nil).visible(opts) -} - -// readRestoreUpload reads the metadata of an upload from every drive. -func (er erasureObjects) readRestoreUpload(ctx context.Context, bucket, uploadIDPath string) restoreUploadView { - metas, errs := readAllFileInfo(ctx, er.getDisks(), bucket, minioMetaMultipartBucket, uploadIDPath, "", false, false) - return restoreUploadState(metas, errs) -} - -// restoreCommitLock is held by a completion from staging through its commit; -// it is named by the upload's own path, which an upload whose copies all -// hold staged object metadata still has. -func (er erasureObjects) restoreCommitLock(uploadIDPath string) RWLocker { - return er.NewNSLock(minioMetaMultipartBucket, uploadIDPath) -} - -// restoreCommitAbandoned reports whether an upload holds only what a failed -// commit left: every drive answers, and every copy that is there is staged -// object metadata, see restoreUploadStaged, older than restoreCommitGrace by -// that drive's own metadata file. A copy that cannot be read, or that is not -// staged yet, may belong to a commit still running. The caller holds the -// upload's restoreCommitLock. -func (er erasureObjects) restoreCommitAbandoned(ctx context.Context, bucket, uploadIDPath string) bool { - disks := er.getDisks() - metas, errs := readAllFileInfo(ctx, disks, bucket, minioMetaMultipartBucket, uploadIDPath, "", false, false) - staged := 0 - for i, fi := range metas { - if errs[i] != nil { - if errors.Is(errs[i], errFileNotFound) || errors.Is(errs[i], errFileVersionNotFound) { - continue - } - return false - } - if !fi.IsValid() || !restoreUploadStaged(fi) { - return false - } - st, err := disks[i].StatInfoFile(ctx, minioMetaMultipartBucket, pathJoin(uploadIDPath, xlStorageFormatFile), false) - if err != nil || len(st) == 0 || time.Since(st[0].ModTime) < restoreCommitGrace { - return false - } - staged++ - } - return staged > 0 + return a.targets(oi) && len(gen.ongoing) == 1 && gen.ongoing[a.ID] } // restoreGeneration is the metadata generation of one version that the drives @@ -304,12 +138,18 @@ func (er erasureObjects) restoreCommitAbandoned(ctx context.Context, bucket, upl // out: fi has no restore fields, and ongoing names the attempts whose // ongoing marker some of its drives hold ("" for one without an attempt). // Every other field, protection included, still needs the quorum. +// +// revs has, for every drive that holds the version, the full revision of the +// entry it holds, whether or not that revision is the generation; nil where a +// drive does not hold the version or cannot be read. A native restore's +// commit compares the drives against it later, see restorePublication. type restoreGeneration struct { fi FileInfo drives []FileInfo disks []StorageAPI split bool ongoing map[string]bool + revs []*revisionKey } // restoreFieldKeys are the metadata keys a restore status write sets. @@ -464,7 +304,7 @@ func (er erasureObjects) readRestoreGeneration(ctx context.Context, bucket, obje return bestKey, best != nil && best.count >= majority && best.count >= er.versionReadQuorum(ctx, bucket, object, best.ver, best.metaV, make([]FileInfo, n), make([]error, n)) } - gen := restoreGeneration{drives: make([]FileInfo, n), disks: make([]StorageAPI, n)} + gen := restoreGeneration{drives: make([]FileInfo, n), disks: make([]StorageAPI, n), revs: slices.Clone(keys)} bestKey, ok := accepted(revisions) if !ok { if bestKey, ok = accepted(neutrals); !ok { @@ -593,60 +433,6 @@ func (er erasureObjects) clearRestoreStatus(ctx context.Context, bucket, object, return err } -// restoreFailed clears the ongoing marker of a failed attempt while the -// object lock is held. A lost lock or an ended context leaves that to the -// bounded retries of the pools layer, which take the lock afresh. -func (er erasureObjects) restoreFailed(ctx context.Context, bucket, object, versionID string, published *restoreAttempt, cause error) error { - if ctx.Err() != nil { - return restoreResetError{cause} - } - if err := er.clearRestoreStatus(ctx, bucket, object, versionID, published); err != nil { - storageLogIf(ctx, fmt.Errorf("Unable to clear the restore status of %s/%s(%s): %w", bucket, object, versionID, err)) - return restoreResetError{cause} - } - return cause -} - -// abortRestoreUpload removes the private upload of a failed attempt, or -// leaves it to stale-upload cleanup. The restore's own context may have -// ended, so it uses a fresh bounded one, and the upload lock as any abort. -// It matches the upload to its own attempt rather than to the version, which -// a newer attempt may own by now, and it never touches the version. -func (er erasureObjects) abortRestoreUpload(bucket, object, uploadID string, a *restoreAttempt) { - ctx, cancel := context.WithTimeout(GlobalContext, restoreCleanupTimeout) - defer cancel() - lk := er.NewNSLock(bucket, pathJoin(object, uploadID)) - lkctx, err := lk.GetLock(ctx, globalOperationTimeout) - if err == nil { - defer lk.Unlock(lkctx) - _, err = er.abortMultipartUpload(lkctx.Context(), bucket, object, uploadID, ObjectOptions{ - NoAuditLog: true, - Transition: TransitionOptions{attempt: a}, - }) - } - if err != nil { - storageLogIf(ctx, fmt.Errorf("Unable to abort the restore upload of %s/%s: %w", bucket, object, err)) - } -} - -// restoreUploadActive reports whether the restore that owns a stale-aged -// upload still runs: its attempt owns the version's ongoing marker. A long -// download keeps its upload; one whose attempt ended loses it, and an -// ongoing marker alone keeps nothing. -func (er erasureObjects) restoreUploadActive(ctx context.Context, v restoreUploadView) bool { - gen, err := er.readRestoreGeneration(ctx, v.bucket, v.object, v.marker.VersionID) - if err != nil { - // An unreadable version may still be restoring; retry next round. - return !errors.Is(err, errFileVersionNotFound) && !errors.Is(err, errFileNotFound) - } - if gen.split { - // Drives disagree on the restore status: that the attempt ended - // is not established. - return gen.ongoing[v.marker.ID] - } - return v.marker.owns(gen.fi.ToObjectInfo(v.bucket, v.object, false)) -} - // restoreLock is the object lock that restores and their status updates // share: the lock every write of the object takes. func (z *erasureServerPools) restoreLock(bucket, object string) RWLocker { @@ -659,36 +445,50 @@ type restoreRequestStatus struct { // attempt is the restore that must rehydrate the version; nil when a // restored copy was still there and its expiry was renewed. attempt *restoreAttempt + // owner is the restore lock of the version, held for attempt. The + // caller hands it to the worker that runs attempt, or releases it. + owner *restoreOwner + // date is the request date recorded with attempt. + date string } // restoreRequest records a POST restore on target, the version that POST // resolved, on a fresh read under the object lock. The version must still // be target: a replacement under the same version ID is refused. A copy that // still holds its restored data is renewed. Otherwise the version is marked -// ongoing for a new attempt; that also takes over a marker no running -// restore holds, as a running restore holds the object lock for its whole -// download. Only the copy that is to be restored is written, so no copy +// ongoing for a new attempt, which also takes over a marker that no running +// restore holds. Only the copy that is to be restored is written, so no copy // claims a restored copy it does not hold. -func (z *erasureServerPools) restoreRequest(ctx context.Context, bucket, object string, opts ObjectOptions, target ObjectInfo) (restoreRequestStatus, error) { +// +// It takes the restore lock R of the version before the object lock O. A +// running restore holds R for its whole run, so a POST that cannot take R +// within the takeover wait answers from the drives: in progress when they +// show the version ongoing, and a retryable timeout otherwise, such as while +// another POST of the version registers. ctx must outlive the request: a new +// attempt keeps R in the returned status. +func (z *erasureServerPools) restoreRequest(ctx context.Context, bucket, object string, opts ObjectOptions, target ObjectInfo) (status restoreRequestStatus, err error) { object = encodeDirObject(object) versionID := restoreVersionID(target.VersionID) - lk := z.restoreLock(bucket, object) - // Wait only briefly first, whatever the lockless read before saw: a - // restore of this version holds the lock for its whole download. Then - // wait as usual only if the drives show no restore of it running. - lkctx, err := lk.GetLock(ctx, newDynamicTimeout(restoreTakeoverWait, restoreTakeoverWait)) - if errors.As(err, &OperationTimedOut{}) && ctx.Err() == nil { - running, rerr := z.restoreRunning(ctx, bucket, object, versionID) - switch { - case rerr != nil: - // Whether it is restoring is unknown: answer now, not after - // the lock frees. - return restoreRequestStatus{}, rerr - case running: - return restoreRequestStatus{}, errRestoreInProgress + owner, err := z.takeRestoreOwner(ctx, bucket, object, versionID, newDynamicTimeout(restoreTakeoverWait, restoreTakeoverWait)) + if err != nil { + if errors.As(err, &OperationTimedOut{}) && ctx.Err() == nil { + running, rerr := z.restoreRunning(ctx, bucket, object, versionID) + switch { + case rerr != nil: + return restoreRequestStatus{}, rerr + case running: + return restoreRequestStatus{}, errRestoreInProgress + } } - lkctx, err = lk.GetLock(ctx, globalOperationTimeout) + return restoreRequestStatus{}, err } + defer func() { + if status.attempt == nil { + owner.release() + } + }() + lk := z.restoreLock(bucket, object) + lkctx, err := lk.GetLock(owner.ctx, globalOperationTimeout) if err != nil { return restoreRequestStatus{}, err } @@ -750,7 +550,10 @@ func (z *erasureServerPools) restoreRequest(ctx context.Context, bucket, object } return restoreRequestStatus{}, err } - return restoreRequestStatus{objInfo: oi, attempt: attempt}, nil + if attempt == nil { + return restoreRequestStatus{objInfo: oi}, nil + } + return restoreRequestStatus{objInfo: oi, attempt: attempt, owner: owner, date: date}, nil } // restoreRunning reports, without the object lock and within the takeover @@ -801,7 +604,7 @@ func (z *erasureServerPools) restorePoolIdx(ctx context.Context, bucket, object return -1, restoreResetError{toObjectErr(err, bucket, object, a.VersionID)} } found = true - if !z.IsSuspended(idx) && a.owns(gen.fi.ToObjectInfo(bucket, object, false)) { + if !z.IsSuspended(idx) && a.ownsGeneration(gen, bucket, object) { return idx, nil } } diff --git a/cmd/restore-capability.go b/cmd/restore-capability.go index 3594a6dc9..311c1c6b3 100644 --- a/cmd/restore-capability.go +++ b/cmd/restore-capability.go @@ -36,7 +36,7 @@ import ( // or does not know the question, as an older server does not, blocks it. // nativeRestoreCapability is the native restore protocol this server speaks. -const nativeRestoreCapability = "1" +const nativeRestoreCapability = "2" const ( peerRESTRestoreCapability = "restore" diff --git a/cmd/restore-native-generation_test.go b/cmd/restore-native-generation_test.go index 1cea3faf7..b7383ef8f 100644 --- a/cmd/restore-native-generation_test.go +++ b/cmd/restore-native-generation_test.go @@ -21,17 +21,14 @@ import ( "bytes" "context" "crypto/rand" - "encoding/base64" "errors" "fmt" "maps" "net/http" "net/http/httptest" - "os" "reflect" "runtime" "strings" - "sync" "sync/atomic" "testing" "time" @@ -40,7 +37,6 @@ import ( "github.com/minio/madmin-go/v3" "github.com/minio/minio/internal/bucket/lifecycle" "github.com/minio/minio/internal/bucket/versioning" - "github.com/minio/minio/internal/config/drive" xhttp "github.com/minio/minio/internal/http" ) @@ -56,7 +52,7 @@ func (d restoreGenerationDisk) WriteMetadata(ctx context.Context, orig, vol, pat if d.write != nil && d.write.Load() && vol == d.bucket && path == d.object { return errFaultyDisk } - return d.WriteMetadata(ctx, orig, vol, path, fi) + return d.StorageAPI.WriteMetadata(ctx, orig, vol, path, fi) } func (d restoreGenerationDisk) UpdateMetadata(ctx context.Context, vol, path string, fi FileInfo, opts UpdateMetadataOpts) error { @@ -99,7 +95,7 @@ func (d restoreGenerationOffline) WriteMetadata(ctx context.Context, orig, bucke if d.fail.Load() && bucket == d.bucket && object == d.object { return errFaultyDisk } - return d.WriteMetadata(ctx, orig, bucket, object, fi) + return d.StorageAPI.WriteMetadata(ctx, orig, bucket, object, fi) } // restoreGenerationMetadata returns the version's metadata on each drive. @@ -259,168 +255,6 @@ func TestRestoreAttemptGenerationFencing(t *testing.T) { } } -// restoreGenerationUploadPath is the stored path of an upload. -func restoreGenerationUploadPath(t *testing.T, r *restoreTest, id string) string { - t.Helper() - b, err := base64.RawURLEncoding.DecodeString(id) - if err != nil { - t.Fatal(err) - } - _, uploadUUID, ok := strings.Cut(string(b), ".") - if !ok { - t.Fatalf("upload ID without UUID: %q", id) - } - return pathJoin(r.z.serverPools[0].getHashedSet(r.object).getMultipartSHADir(r.bucket, r.object), uploadUUID) -} - -// The restore marker is established on a majority of the drives. One drive -// whose copy of the upload lacks it does not open the upload to clients, -// and the restore still completes. -func TestRestoreUploadMarkerQuorum(t *testing.T) { - r := newRestoreSafetyTest(t, 2, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - oi, body := restoreIsolationSeed(t, r, 0, false, true) - tier := newRestoreIsolationGateTier(5<<20, nil) - globalTierConfigMgr.drivercache["WARM"] = tier - defer tier.unblock() - layer := newRestoreIsolationLayer(r) - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatal(rec.Body.String()) - } - restoreIsolationEntered(t, tier) - id, _ := restoreIsolationUpload(t, r, 0) - disk := r.z.serverPools[0].getHashedSet(r.object).getDisks()[0] - path := restoreGenerationUploadPath(t, r, id) - fi, err := disk.ReadVersion(t.Context(), "", minioMetaMultipartBucket, path, "", ReadOptions{}) - if err != nil { - t.Fatal(err) - } - delete(fi.Metadata, restoreUploadKey) - if err := disk.WriteMetadata(t.Context(), "", minioMetaMultipartBucket, path, fi); err != nil { - t.Fatal(err) - } - poison := bytes.Repeat([]byte("p"), 5<<20) - if _, err := r.z.PutObjectPart(t.Context(), r.bucket, r.object, id, 1, - mustGetPutObjReader(t, bytes.NewReader(poison), int64(len(poison)), "", ""), ObjectOptions{}); !errors.As(err, &InvalidUploadID{}) { - t.Errorf("public part with one unmarked copy of the upload: %v, want NoSuchUpload", err) - } - for _, op := range []func() error{ - func() error { - _, err := r.z.ListObjectParts(t.Context(), r.bucket, r.object, id, 0, 10, ObjectOptions{}) - return err - }, - func() error { - _, err := r.z.GetMultipartInfo(t.Context(), r.bucket, r.object, id, ObjectOptions{}) - return err - }, - } { - if err := op(); !errors.As(err, &InvalidUploadID{}) { - t.Errorf("public read with one unmarked copy of the upload: %v, want NoSuchUpload", err) - } - } - if uploads, err := r.z.ListMultipartUploads(t.Context(), r.bucket, r.object, "", "", "", 10); err != nil || len(uploads.Uploads) != 0 { - t.Errorf("listing with one unmarked copy of the upload: %+v, %v", uploads.Uploads, err) - } - tier.unblock() - if err := layer.wait(t); err != nil { - t.Fatal(err) - } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents: %v", err) - } -} - -// When the drives cannot tell an upload's marker, no client lists it, no -// abort removes it and stale cleanup keeps it; the restore completes once -// the drives answer again. -func TestRestoreUploadUnreadableFailsClosed(t *testing.T) { - for _, mode := range []string{"list", "abort", "stale"} { - t.Run(mode, func(t *testing.T) { - r := newRestoreSafetyTest(t, 2, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - oi, body := restoreIsolationSeed(t, r, 0, false, true) - er := r.z.serverPools[0].getHashedSet(r.object) - disks := er.getDisks() - var fail atomic.Bool - // Stale cleanup with a short age also removes in-flight temporary - // parts: hold that restore in completion, after its last part and - // before its commit, and the others in their download, outside - // the upload lock. - gate := &restoreIsolationCompletionGate{entered: make(chan struct{}), release: make(chan struct{})} - defer gate.unblock() - tier := newRestoreIsolationGateTier(5<<20, nil) - defer tier.unblock() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - disk := d - if mode == "stale" { - disk = restoreIsolationValidationDisk{d, gate} - } - wrapped[i] = restoreGenerationDisk{StorageAPI: disk, uploadRead: &fail} - } - er.getDisks = func() []StorageAPI { return wrapped } - if mode != "stale" { - globalTierConfigMgr.drivercache["WARM"] = tier - } - layer := newRestoreIsolationLayer(r) - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatal(rec.Body.String()) - } - if mode == "stale" { - select { - case <-gate.entered: - case <-time.After(restoreTestWait): - t.Fatal("restore never reached completion") - } - } else { - restoreIsolationEntered(t, tier) - } - stored := restoreIsolationUploads(t, r, 0) - if len(stored) != 1 { - t.Fatalf("restore upload on disk: %v", stored) - } - var id string - for uploadUUID := range stored { - id = base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, "%s.%s", globalDeploymentID(), uploadUUID)) - } - fail.Store(true) - switch mode { - case "list": - if uploads, err := r.z.ListMultipartUploads(t.Context(), r.bucket, r.object, "", "", "", 100); len(uploads.Uploads) != 0 { - t.Errorf("an upload with an unknown marker is listed: %+v, %v", uploads.Uploads, err) - } - case "abort": - if err := r.z.AbortMultipartUpload(t.Context(), r.bucket, r.object, id, ObjectOptions{}); err == nil { - t.Error("abort of an upload with an unknown marker succeeded") - } - case "stale": - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - for _, disk := range wrapped { - er.cleanupStaleUploadsOnDisk(t.Context(), disk) - } - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - } - fail.Store(false) - if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 1 { - t.Errorf("%s removed an upload with an unknown marker: %v", mode, uploads) - } - gate.unblock() - tier.unblock() - if err := layer.wait(t); err != nil { - t.Fatalf("restore after %s: %v", mode, err) - } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents: %v", err) - } - }) - } -} - // POST restores the version it resolved with that version's identity. A // replacement under the same version ID between its read and its status // write is refused, and nothing restores it. @@ -633,190 +467,6 @@ func TestRestoreGenerationSplitNeedsQuorum(t *testing.T) { } } -// restoreGenerationStageDisk holds a completion that is staging: as -// xlStorage.RenameData does, a drive that stages writes the committed -// object metadata into the upload before it renames the upload into the -// object. -type restoreGenerationStageDisk struct { - StorageAPI - bucket, object string - stage bool - arrived *atomic.Int32 - entered chan struct{} - release chan struct{} - disks int32 -} - -func (d restoreGenerationStageDisk) RenameData(ctx context.Context, srcVolume, srcPath string, fi FileInfo, dstVolume, dstPath string, opts RenameOptions) (RenameDataResp, error) { - if srcVolume == minioMetaMultipartBucket && dstVolume == d.bucket && dstPath == d.object { - if d.stage { - if err := d.WriteMetadata(ctx, "", srcVolume, srcPath, fi); err != nil { - return RenameDataResp{}, err - } - } - if d.arrived.Add(1) == d.disks { - close(d.entered) - } - select { - case <-d.release: - case <-ctx.Done(): - return RenameDataResp{}, ctx.Err() - } - } - return d.StorageAPI.RenameData(ctx, srcVolume, srcPath, fi, dstVolume, dstPath, opts) -} - -// While a completion stages its commit, the upload holds object metadata -// without the marker on the drives that staged. Nobody lists, aborts or -// removes it then, also when the copies not staged yet are old. -func TestRestoreUploadStagingFailsClosed(t *testing.T) { - for _, mode := range []string{"staged", "partly-staged", "stalled-commit"} { - t.Run(mode, func(t *testing.T) { testRestoreUploadStaging(t, mode) }) - } -} - -func testRestoreUploadStaging(t *testing.T, mode string) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - oi, body := restoreIsolationSeed(t, r, 0, false, true) - er := r.z.serverPools[0].getHashedSet(r.object) - disks := er.getDisks() - var arrived atomic.Int32 - entered, release := make(chan struct{}), make(chan struct{}) - released := false - defer func() { - if !released { - close(release) - } - }() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - wrapped[i] = restoreGenerationStageDisk{ - StorageAPI: d, bucket: r.bucket, object: r.object, stage: mode != "partly-staged" || i == 0, - arrived: &arrived, entered: entered, release: release, disks: int32(len(disks)), - } - } - er.getDisks = func() []StorageAPI { return wrapped } - layer := newRestoreIsolationLayer(r) - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatal(rec.Body.String()) - } - select { - case <-entered: - case <-time.After(restoreTestWait): - t.Fatal("completion never staged its commit") - } - stored := restoreIsolationUploads(t, r, 0) - var id string - for uploadUUID := range stored { - id = base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, "%s.%s", globalDeploymentID(), uploadUUID)) - if mode != "staged" { - // partly-staged: a long restore, the copies not staged yet are - // old. stalled-commit: drives that stall the running commit - // past the grace make its staged copies look abandoned. - old := UTCNow().Add(-48 * time.Hour) - for _, d := range disks[1:] { - if err := os.Chtimes(pathJoin(d.Endpoint().Path, minioMetaMultipartBucket, er.getMultipartSHADir(r.bucket, r.object), uploadUUID, xlStorageFormatFile), old, old); err != nil { - t.Fatal(err) - } - } - } - } - if uploads, err := r.z.ListMultipartUploads(t.Context(), r.bucket, r.object, "", "", "", 10); err != nil || len(uploads.Uploads) != 0 { - t.Errorf("a staged restore upload is listed: %+v, %v", uploads.Uploads, err) - } - // The set's own abort: the pools layer would wait for the upload lock - // the completion holds. - if err := er.AbortMultipartUpload(t.Context(), r.bucket, r.object, id, ObjectOptions{}); err == nil { - t.Error("abort of a staged restore upload succeeded") - } - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - for _, disk := range wrapped { - er.cleanupStaleUploadsOnDisk(t.Context(), disk) - } - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - if after := restoreIsolationUploads(t, r, 0); !reflect.DeepEqual(after, stored) { - t.Errorf("a staged restore upload was removed: %v -> %v", stored, after) - } - released = true - close(release) - if err := layer.wait(t); err != nil { - t.Fatalf("restore after its staging was attacked: %v", err) - } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents: %v", err) - } -} - -// When a drive of an acknowledged attempt becomes unreadable, the drives that -// answer disagree on the restore status. That the attempt ended is then not -// established, and stale cleanup keeps its upload. -func TestRestoreSplitKeepsRunningUpload(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - oi, body := restoreIsolationSeed(t, r, 0, false, true) - er := r.z.serverPools[0].getHashedSet(r.object) - disks := er.getDisks() - var missed, offline atomic.Bool - gate := &restoreIsolationCompletionGate{entered: make(chan struct{}), release: make(chan struct{})} - defer gate.unblock() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - var disk StorageAPI = restoreIsolationValidationDisk{d, gate} - switch i { - case 0: - disk = restoreGenerationDisk{StorageAPI: disk, bucket: r.bucket, object: r.object, write: &missed} - case 1: - disk = restoreGenerationOffline{StorageAPI: disk, bucket: r.bucket, object: r.object, fail: &offline} - } - wrapped[i] = disk - } - er.getDisks = func() []StorageAPI { return wrapped } - layer := newRestoreIsolationLayer(r) - missed.Store(true) - rec := restoreIsolationPOST(t, r, layer, oi.VersionID) - missed.Store(false) - if rec.Code != http.StatusOK { - t.Fatal(rec.Body.String()) - } - select { - case <-gate.entered: - case <-time.After(restoreTestWait): - t.Fatal("restore never reached completion") - } - stored := restoreIsolationUploads(t, r, 0) - offline.Store(true) - if gen, err := er.readRestoreGeneration(t.Context(), r.bucket, r.object, oi.VersionID); err != nil || !gen.split { - t.Fatalf("fixture: no split generation: %v", err) - } - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - for _, disk := range wrapped { - er.cleanupStaleUploadsOnDisk(t.Context(), disk) - } - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - offline.Store(false) - if after := restoreIsolationUploads(t, r, 0); !reflect.DeepEqual(after, stored) { - t.Errorf("stale cleanup removed a running restore's upload: %v -> %v", stored, after) - } - gate.unblock() - if err := layer.wait(t); err != nil { - t.Fatalf("restore after the split: %v", err) - } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents: %v", err) - } -} - // restoreGenerationCleanupDisk misses restored-copy cleanups and metadata // writes of the object while its toggles are set. type restoreGenerationCleanupDisk struct { @@ -836,7 +486,7 @@ func (d restoreGenerationCleanupDisk) WriteMetadata(ctx context.Context, orig, b if bucket == d.bucket && object == d.object && d.metaWrites.Load() { return errFaultyDisk } - return d.WriteMetadata(ctx, orig, bucket, object, fi) + return d.StorageAPI.WriteMetadata(ctx, orig, bucket, object, fi) } // Restored-copy expiry decides on the version's authoritative generation. A @@ -1033,268 +683,6 @@ func TestRestoreHealNeedsRestoreQuorum(t *testing.T) { } } -// restoreGenerationStageFails fails the commit before it stages. -type restoreGenerationStageFails struct { - StorageAPI - bucket, object string -} - -func (d restoreGenerationStageFails) RenameData(ctx context.Context, srcVolume, srcPath string, fi FileInfo, dstVolume, dstPath string, opts RenameOptions) (RenameDataResp, error) { - if srcVolume == minioMetaMultipartBucket && dstVolume == d.bucket && dstPath == d.object { - return RenameDataResp{}, errFaultyDisk - } - return d.StorageAPI.RenameData(ctx, srcVolume, srcPath, fi, dstVolume, dstPath, opts) -} - -// restoreGenerationFailedCommit stages the committed metadata into the -// upload, as RenameData does, and then fails the commit. -type restoreGenerationFailedCommit struct { - StorageAPI - bucket, object string -} - -func (d restoreGenerationFailedCommit) RenameData(ctx context.Context, srcVolume, srcPath string, fi FileInfo, dstVolume, dstPath string, opts RenameOptions) (RenameDataResp, error) { - if srcVolume == minioMetaMultipartBucket && dstVolume == d.bucket && dstPath == d.object { - if err := d.WriteMetadata(ctx, "", srcVolume, srcPath, fi); err != nil { - return RenameDataResp{}, err - } - return RenameDataResp{}, errFaultyDisk - } - return d.StorageAPI.RenameData(ctx, srcVolume, srcPath, fi, dstVolume, dstPath, opts) -} - -// restoreGenerationLeftover runs a restore whose commit fails after it -// staged on every drive, or on every drive but drive 0, which fails before -// it stages. It returns the upload left behind and its on-disk UUID. -func restoreGenerationLeftover(t *testing.T, r *restoreTest, unstaged bool) (map[string]int, string) { - t.Helper() - initConfigSubsystem(t.Context(), r.z) - oi, _ := restoreIsolationSeed(t, r, 0, false, true) - er := r.z.serverPools[0].getHashedSet(r.object) - disks := er.getDisks() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - if unstaged && i == 0 { - wrapped[i] = restoreGenerationStageFails{StorageAPI: d, bucket: r.bucket, object: r.object} - continue - } - wrapped[i] = restoreGenerationFailedCommit{StorageAPI: d, bucket: r.bucket, object: r.object} - } - er.getDisks = func() []StorageAPI { return wrapped } - layer := newRestoreIsolationLayer(r) - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatal(rec.Body.String()) - } - if err := layer.wait(t); err == nil { - t.Fatal("fixture: the failed commit succeeded") - } - er.getDisks = func() []StorageAPI { return disks } - stored := restoreIsolationUploads(t, r, 0) - if len(stored) != 1 { - t.Fatalf("fixture: no leftover of the failed commit: %v", stored) - } - for uploadUUID := range stored { - return stored, uploadUUID - } - return nil, "" -} - -// restoreGenerationAgeUpload makes every drive's copy of an upload's -// metadata look written d ago. -func restoreGenerationAgeUpload(t *testing.T, r *restoreTest, uploadUUID string, d time.Duration) { - t.Helper() - er := r.z.serverPools[0].getHashedSet(r.object) - old := UTCNow().Add(-d) - for _, disk := range er.getDisks() { - if err := os.Chtimes(pathJoin(disk.Endpoint().Path, minioMetaMultipartBucket, er.getMultipartSHADir(r.bucket, r.object), uploadUUID, xlStorageFormatFile), old, old); err != nil { - t.Fatal(err) - } - } -} - -// What a failed commit leaves in an upload can always be removed once every -// copy is staged and older than the commit grace: abort removes it, also -// when stale cleanup could not. A recent staging, a copy that cannot be read -// and a copy not staged yet are refused, and nothing is removed then. -func TestRestoreAbandonedCommitAbort(t *testing.T) { - for _, age := range []string{"recent", "abandoned", "unreadable-copy", "unstaged-copy"} { - t.Run(age, func(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - stored, uploadUUID := restoreGenerationLeftover(t, r, age == "unstaged-copy") - if age != "recent" { - restoreGenerationAgeUpload(t, r, uploadUUID, 2*restoreCommitGrace) - } - id := base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, "%s.%s", globalDeploymentID(), uploadUUID)) - er := r.z.serverPools[0].getHashedSet(r.object) - disks := er.getDisks() - var unreadable atomic.Bool - if age == "unreadable-copy" { - wrapped := append([]StorageAPI(nil), disks...) - wrapped[0] = restoreGenerationDisk{StorageAPI: disks[0], uploadRead: &unreadable} - er.getDisks = func() []StorageAPI { return wrapped } - unreadable.Store(true) - } - err := r.z.AbortMultipartUpload(t.Context(), r.bucket, r.object, id, ObjectOptions{}) - unreadable.Store(false) - left := restoreIsolationUploads(t, r, 0) - if age != "abandoned" { - if err == nil || !reflect.DeepEqual(left, stored) { - t.Errorf("abort of an upload that may still commit (%s): %v, left %v", age, err, left) - } - return - } - if err != nil || len(left) != 0 { - t.Errorf("abort of an abandoned commit: %v, left %v", err, left) - } - }) - } -} - -// With no commit running, stale cleanup removes what a failed commit left -// drive by drive: a staged copy older than the commit grace. A recent -// staging and a copy that never staged stay. -func TestRestoreFailedCommitStaleCleanup(t *testing.T) { - for _, mode := range []string{"recent", "abandoned", "unstaged-copy"} { - t.Run(mode, func(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - stored, uploadUUID := restoreGenerationLeftover(t, r, mode == "unstaged-copy") - if mode != "recent" { - // The copy that never staged is as old as the others. - restoreGenerationAgeUpload(t, r, uploadUUID, 2*restoreCommitGrace) - } - er := r.z.serverPools[0].getHashedSet(r.object) - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - for _, disk := range er.getDisks() { - er.cleanupStaleUploadsOnDisk(t.Context(), disk) - } - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - want := map[string]int{} - switch mode { - case "recent": - want = stored - case "unstaged-copy": - want = map[string]int{uploadUUID: 1} - } - if left := restoreIsolationUploads(t, r, 0); !reflect.DeepEqual(left, want) { - t.Errorf("stale cleanup of a failed commit's leftover: %v -> %v, want %v", stored, left, want) - } - }) - } -} - -// restoreGenerationSnapshotKey marks the context of a stale cleanup whose -// reads of upload metadata restoreGenerationSnapshotDisk holds. -type restoreGenerationSnapshotKey struct{} - -// restoreGenerationSnapshotDisk reads upload metadata at once, and hands the -// reply to a marked stale cleanup only on release. -type restoreGenerationSnapshotDisk struct { - StorageAPI - reads *atomic.Int32 - release chan struct{} -} - -func (d restoreGenerationSnapshotDisk) ReadVersion(ctx context.Context, origvolume, volume, path, versionID string, opts ReadOptions) (FileInfo, error) { - fi, err := d.StorageAPI.ReadVersion(ctx, origvolume, volume, path, versionID, opts) - if volume == minioMetaMultipartBucket && ctx.Value(restoreGenerationSnapshotKey{}) != nil { - d.reads.Add(1) - select { - case <-d.release: - case <-ctx.Done(): - return FileInfo{}, ctx.Err() - } - } - return fi, err -} - -// Stale cleanup decides on a removal under the commit lock. A completion -// that starts while cleanup reads an upload past the stale age waits for -// it, and a cleanup that comes later skips the running commit: the upload, -// an ordinary one, still commits with its contents. -func TestCleanupStaleUploadsSkipsRunningCommit(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - er := r.z.serverPools[0].getHashedSet(r.object) - mp, err := r.z.NewMultipartUpload(t.Context(), r.bucket, r.object, ObjectOptions{Versioned: true, MTime: UTCNow().Add(-48 * time.Hour)}) - if err != nil { - t.Fatal(err) - } - body := []byte("an ordinary upload past the stale age") - part, err := r.z.PutObjectPart(t.Context(), r.bucket, r.object, mp.UploadID, 1, mustGetPutObjReader(t, bytes.NewReader(body), int64(len(body)), "", ""), ObjectOptions{}) - if err != nil { - t.Fatal(err) - } - disks := er.getDisks() - var reads, staged atomic.Int32 - readRelease := make(chan struct{}) - stageEntered, stageRelease := make(chan struct{}), make(chan struct{}) - var readOnce, stageOnce sync.Once - defer readOnce.Do(func() { close(readRelease) }) - defer stageOnce.Do(func() { close(stageRelease) }) - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - stage := restoreGenerationStageDisk{ - StorageAPI: d, bucket: r.bucket, object: r.object, stage: true, - arrived: &staged, entered: stageEntered, release: stageRelease, disks: int32(len(disks)), - } - wrapped[i] = restoreGenerationSnapshotDisk{StorageAPI: stage, reads: &reads, release: readRelease} - } - er.getDisks = func() []StorageAPI { return wrapped } - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - defer func() { - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - }() - // Every drive's cleanup at once, each reading the upload on every drive. - ctx := context.WithValue(t.Context(), restoreGenerationSnapshotKey{}, true) - var cleanups sync.WaitGroup - for _, disk := range wrapped { - cleanups.Go(func() { er.cleanupStaleUploadsOnDisk(ctx, disk) }) - } - cleanupDone := make(chan struct{}) - go func() { cleanups.Wait(); close(cleanupDone) }() - // The completion starts once every cleanup that may decide holds its - // read of the upload from before the commit. - for deadline := time.Now().Add(2 * time.Second); reads.Load() < int32(len(disks)*len(disks)) && time.Now().Before(deadline); { - time.Sleep(10 * time.Millisecond) - } - completion := make(chan ObjectInfo, 1) - completionErr := make(chan error, 1) - go func() { - oi, err := r.z.CompleteMultipartUpload(t.Context(), r.bucket, r.object, mp.UploadID, []CompletePart{{PartNumber: 1, ETag: part.ETag}}, ObjectOptions{Versioned: true}) - completion <- oi - completionErr <- err - }() - select { - case <-stageEntered: - case <-time.After(3 * time.Second): - } - readOnce.Do(func() { close(readRelease) }) - select { - case <-cleanupDone: - case <-time.After(restoreTestWait): - t.Fatal("stale cleanup never returned") - } - stageOnce.Do(func() { close(stageRelease) }) - oi := <-completion - if err := <-completionErr; err != nil { - t.Fatalf("completion of an upload stale cleanup read: %v", err) - } - er.getDisks = func() []StorageAPI { return disks } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("completed contents: %q, %v", got, err) - } -} - // restoreGenerationRand draws UUIDs from crypto/rand for the whole test // binary, installed before any goroutine runs. While a pause is set, it holds // the UUID that stale cleanup draws in its deadline-bound worker, for the @@ -1334,91 +722,3 @@ func restoreGenerationInCleanupRename() bool { } } } - -// Stale cleanup holds the commit lock until its rename ends, also when the -// rename outlives the drive deadline and cleanup has moved on: a completion -// does not stage while that rename can still remove its upload. -func TestCleanupStaleUploadsHoldsLockThroughRename(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - er := r.z.serverPools[0].getHashedSet(r.object) - mp, err := r.z.NewMultipartUpload(t.Context(), r.bucket, r.object, ObjectOptions{Versioned: true, MTime: UTCNow().Add(-48 * time.Hour)}) - if err != nil { - t.Fatal(err) - } - body := []byte("an upload whose stale cleanup outlives the drive deadline") - part, err := r.z.PutObjectPart(t.Context(), r.bucket, r.object, mp.UploadID, 1, mustGetPutObjReader(t, bytes.NewReader(body), int64(len(body)), "", ""), ObjectOptions{}) - if err != nil { - t.Fatal(err) - } - disks := er.getDisks() - stored := restoreIsolationUploads(t, r, 0) - prev := globalDriveConfig.GetMaxTimeout() - // The shortest drive deadline the configuration takes. - globalDriveConfig.Update(drive.Config{MaxTimeout: 1100 * time.Millisecond}) - defer globalDriveConfig.Update(drive.Config{MaxTimeout: prev}) - pause := &restoreGenerationPause{entered: make(chan struct{}), release: make(chan struct{})} - var paused sync.Once - defer paused.Do(func() { close(pause.release) }) - restoreGenerationRand.pause.Store(pause) - defer restoreGenerationRand.pause.CompareAndSwap(pause, nil) - - cleanupDone := make(chan struct{}) - go func() { - er.cleanupStaleUploadsOnDisk(t.Context(), disks[0]) - close(cleanupDone) - }() - select { - case <-pause.entered: - case <-time.After(5 * time.Second): - t.Fatal("stale cleanup never reached its rename") - } - select { - case <-cleanupDone: - case <-time.After(5 * time.Second): - t.Fatal("stale cleanup did not move on after the drive deadline") - } - if left := restoreIsolationUploads(t, r, 0); !reflect.DeepEqual(left, stored) { - t.Fatalf("fixture: the held rename already ran: %v -> %v", stored, left) - } - - var staged atomic.Int32 - stageEntered, stageRelease := make(chan struct{}), make(chan struct{}) - var stageOnce sync.Once - defer stageOnce.Do(func() { close(stageRelease) }) - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - wrapped[i] = restoreGenerationStageDisk{ - StorageAPI: d, bucket: r.bucket, object: r.object, stage: true, - arrived: &staged, entered: stageEntered, release: stageRelease, disks: int32(len(disks)), - } - } - er.getDisks = func() []StorageAPI { return wrapped } - completion := make(chan ObjectInfo, 1) - completionErr := make(chan error, 1) - go func() { - oi, err := r.z.CompleteMultipartUpload(t.Context(), r.bucket, r.object, mp.UploadID, []CompletePart{{PartNumber: 1, ETag: part.ETag}}, ObjectOptions{Versioned: true}) - completion <- oi - completionErr <- err - }() - select { - case <-stageEntered: - t.Error("a completion staged while stale cleanup's rename could still run") - case <-time.After(2 * time.Second): - } - paused.Do(func() { close(pause.release) }) - select { - case <-stageEntered: - case <-time.After(restoreTestWait): - t.Fatal("completion never staged after the rename ended") - } - stageOnce.Do(func() { close(stageRelease) }) - oi := <-completion - if err := <-completionErr; err != nil { - t.Fatalf("completion after the rename ended: %v", err) - } - er.getDisks = func() []StorageAPI { return disks } - if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { - t.Fatalf("completed contents: %q, %v", got, err) - } -} diff --git a/cmd/restore-native-isolation_test.go b/cmd/restore-native-isolation_test.go index 6e5e412da..2690aefea 100644 --- a/cmd/restore-native-isolation_test.go +++ b/cmd/restore-native-isolation_test.go @@ -20,7 +20,6 @@ package cmd import ( "bytes" "context" - "encoding/base64" "errors" "fmt" "io" @@ -41,12 +40,9 @@ import ( "github.com/minio/mux" ) -// The private metadata of a native restore, as stored. Literal names keep -// these regressions meaningful on a tree without the keys. -const ( - restoreIsolationAttemptKey = "x-minio-internal-restore-attempt" - restoreIsolationUploadKey = "x-minio-internal-restore-upload" -) +// The private attempt metadata of a native restore, as stored. A literal name +// keeps these regressions meaningful on a tree without the key. +const restoreIsolationAttemptKey = "x-minio-internal-restore-attempt" // restoreIsolationLayer hooks the object layer a POST restore handler uses. // afterRead runs once POST has read the version, before POST records its @@ -177,30 +173,6 @@ func restoreIsolationUploads(t *testing.T, r *restoreTest, pool int) map[string] return found } -// restoreIsolationUpload finds the single upload of the object in a pool on -// disk, with its first part, and returns its public upload ID. -func restoreIsolationUpload(t *testing.T, r *restoreTest, pool int) (string, ObjectPartInfo) { - t.Helper() - uploads := restoreIsolationUploads(t, r, pool) - if len(uploads) != 1 { - t.Fatalf("want one upload on disk, found %v", uploads) - } - var uploadUUID string - for uploadUUID = range uploads { - } - er := r.z.serverPools[pool].getHashedSet(r.object) - uploadPath := pathJoin(er.getMultipartSHADir(r.bucket, r.object), uploadUUID) - fi, err := er.getDisks()[0].ReadVersion(t.Context(), "", minioMetaMultipartBucket, uploadPath, "", ReadOptions{}) - if err != nil { - t.Fatal(err) - } - parts, err := readParts(t.Context(), er.getDisks(), minioMetaMultipartBucket, []string{pathJoin(uploadPath, fi.DataDir, "part.1.meta")}, []int{1}, 1) - if err != nil || len(parts) != 1 || parts[0].Error != "" { - t.Fatalf("restore must have published its first part: %+v, %v", parts, err) - } - return base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, "%s.%s", globalDeploymentID(), uploadUUID)), parts[0] -} - // restoreIsolationGateTier holds a download after its first bytes and can // fail it once released. type restoreIsolationGateTier struct { @@ -319,207 +291,6 @@ func installRestoreIsolationFaults(r *restoreTest, pool int, failing int) (reads return reads, writes } -// A public client must not replace a part of a native restore's upload, -// before or after the restore validated it: the bytes would become the -// COMPLIANCE-protected version under its original ETag. -func TestRestoreUploadIsolatedFromPublicParts(t *testing.T) { - r := newRestoreSafetyTest(t, 2, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - enableBucketObjectLock(t, r.bucket) - previous := newObjectLayerFn() - t.Cleanup(func() { setObjectLayer(previous) }) - router := initTestAPIEndPoints(r.z, []string{"PutObjectPart", "GetObject"}) - oi, body := nativeRestoreSeed(t, r, 0, false, true) - if rec := nativeRestoreLockRequest(t.Context(), t, r, oi.VersionID, "retention", retentionBody("COMPLIANCE", UTCNow().Add(48*time.Hour))); rec.Code != http.StatusOK { - t.Fatalf("lock: %d %s", rec.Code, rec.Body.String()) - } - // Hold completion after it validated the parts, before it commits. - er := r.z.serverPools[0].getHashedSet(r.object) - original := er.getDisks - gate := &restoreIsolationCompletionGate{entered: make(chan struct{}), release: make(chan struct{})} - defer gate.unblock() - disks := original() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - wrapped[i] = restoreIsolationCompletionDisk{d, gate} - } - er.getDisks = func() []StorageAPI { return wrapped } - defer func() { er.getDisks = original }() - ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) - defer cancel() - restored := make(chan error, 1) - go func() { - restored <- r.z.RestoreTransitionedObject(ctx, r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) - }() - select { - case <-gate.entered: - case err := <-restored: - t.Fatalf("restore stopped before validation: %v", err) - case <-ctx.Done(): - t.Fatal("no completion gate") - } - id, _ := restoreIsolationUpload(t, r, 0) - poison := bytes.Repeat([]byte("z"), 5<<20) - // The request may wait for the upload lock that completion holds. - uploaded := make(chan *httptest.ResponseRecorder, 1) - go func() { - uploaded <- attributesPartsSignedRequest(t, router, globalActiveCred, http.MethodPut, "/"+r.bucket+"/"+r.object+"?uploadId="+url.QueryEscape(id)+"&partNumber=1", poison, nil) - }() - var rec *httptest.ResponseRecorder - select { - case rec = <-uploaded: - case <-time.After(500 * time.Millisecond): - } - gate.unblock() - if err := <-restored; err != nil { - t.Fatalf("restore: %v", err) - } - if rec == nil { - rec = <-uploaded - } - if rec.Code != http.StatusNotFound || !strings.Contains(rec.Body.String(), "NoSuchUpload") { - t.Errorf("public UploadPart on a native restore upload: %d %s, want NoSuchUpload", rec.Code, rec.Body.String()) - } - got := attributesPartsSignedRequest(t, router, globalActiveCred, http.MethodGet, "/"+r.bucket+"/"+r.object+"?versionId="+oi.VersionID, nil, nil) - if got.Code != http.StatusOK || !bytes.Equal(got.Body.Bytes(), body) { - t.Errorf("COMPLIANCE version holds other bytes after restore: GET=%d poisoned=%v", got.Code, bytes.HasPrefix(got.Body.Bytes(), poison)) - } -} - -type restoreIsolationCompletionGate struct { - entered, release chan struct{} - enter, open sync.Once -} - -func (g *restoreIsolationCompletionGate) unblock() { g.open.Do(func() { close(g.release) }) } - -type restoreIsolationCompletionDisk struct { - StorageAPI - gate *restoreIsolationCompletionGate -} - -// Completion removes the part metadata it validated just before it renames -// the upload into place. -func (d restoreIsolationCompletionDisk) DeleteBulk(ctx context.Context, bucket string, paths ...string) error { - if bucket == minioMetaMultipartBucket && len(paths) == 2 && strings.HasSuffix(paths[0], ".meta") && strings.HasSuffix(paths[1], ".meta") { - d.gate.enter.Do(func() { close(d.gate.entered) }) - select { - case <-d.gate.release: - case <-ctx.Done(): - return ctx.Err() - } - } - return d.StorageAPI.DeleteBulk(ctx, bucket, paths...) -} - -// restoreIsolationValidationDisk holds a completion where it reads the parts -// it validates: after its object and upload locks, before its commit lock. -// Stale cleanup then decides on the upload itself, not on a running commit. -type restoreIsolationValidationDisk struct { - StorageAPI - gate *restoreIsolationCompletionGate -} - -func (d restoreIsolationValidationDisk) ReadParts(ctx context.Context, bucket string, partMetaPaths ...string) ([]*ObjectPartInfo, error) { - if bucket == minioMetaMultipartBucket { - d.gate.enter.Do(func() { close(d.gate.entered) }) - select { - case <-d.gate.release: - case <-ctx.Done(): - return nil, ctx.Err() - } - } - return d.StorageAPI.ReadParts(ctx, bucket, partMetaPaths...) -} - -// Every public multipart entry point finds no native restore upload, and -// none of them can change it: the restore still completes with its own data. -func TestRestoreUploadHiddenFromPublicEntryPoints(t *testing.T) { - for _, pools := range []int{1, 2} { - t.Run(fmt.Sprintf("pools=%d", pools), func(t *testing.T) { - r := newRestoreSafetyTest(t, pools, versioning.Enabled) - oi, body := nativeRestoreSeed(t, r, 0, false, true) - tier := newRestoreIsolationGateTier(5<<20, nil) - globalTierConfigMgr.drivercache["WARM"] = tier - defer tier.unblock() - ctx, cancel := context.WithTimeout(t.Context(), 20*time.Second) - defer cancel() - restored := make(chan error, 1) - go func() { - restored <- r.z.RestoreTransitionedObject(ctx, r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) - }() - restoreIsolationEntered(t, tier) - id, part := restoreIsolationUpload(t, r, 0) - stored := restoreIsolationUploads(t, r, 0) - - for _, prefix := range []string{r.object, ""} { - uploads, err := r.z.ListMultipartUploads(t.Context(), r.bucket, prefix, "", "", "", 100) - if err != nil { - t.Fatal(err) - } - for _, u := range uploads.Uploads { - if u.UploadID == id { - t.Errorf("ListMultipartUploads(prefix=%q) shows the native restore upload", prefix) - } - } - } - poison := bytes.Repeat([]byte("z"), 5<<20) - noUpload := func(op string, err error) { - t.Helper() - if !errors.As(err, &InvalidUploadID{}) { - t.Errorf("%s on a native restore upload: %v, want NoSuchUpload", op, err) - } - } - // With one pool, part writes first wait for the object lock the - // restore holds; with more they reach the upload at once. - written := make(chan error, 2) - go func() { - _, err := r.z.PutObjectPart(ctx, r.bucket, r.object, id, 1, mustGetPutObjReader(t, bytes.NewReader(poison), int64(len(poison)), "", ""), ObjectOptions{}) - written <- err - }() - go func() { - src := ObjectInfo{PutObjReader: mustGetPutObjReader(t, bytes.NewReader(poison), int64(len(poison)), "", "")} - _, err := r.z.CopyObjectPart(ctx, r.bucket, r.object, r.bucket, r.object, id, 1, 0, int64(len(poison)), src, ObjectOptions{}, ObjectOptions{}) - written <- err - }() - var writes []error - if pools > 1 { - writes = append(writes, <-written, <-written) - } - _, err := r.z.ListObjectParts(t.Context(), r.bucket, r.object, id, 0, 10, ObjectOptions{}) - noUpload("ListObjectParts", err) - _, err = r.z.GetMultipartInfo(t.Context(), r.bucket, r.object, id, ObjectOptions{}) - noUpload("GetMultipartInfo", err) - err = r.z.AbortMultipartUpload(t.Context(), r.bucket, r.object, id, ObjectOptions{}) - noUpload("AbortMultipartUpload", err) - if after := restoreIsolationUploads(t, r, 0); !reflect.DeepEqual(after, stored) { - t.Errorf("public calls changed the restore upload on disk: %v -> %v", stored, after) - } - // Completion waits for the object lock the restore holds. - completed := make(chan error, 1) - go func() { - _, err := r.z.CompleteMultipartUpload(ctx, r.bucket, r.object, id, []CompletePart{{PartNumber: 1, ETag: part.ETag}}, r.opts(oi.VersionID)) - completed <- err - }() - tier.unblock() - if err := <-restored; err != nil { - t.Fatalf("restore after public calls: %v", err) - } - for len(writes) < 2 { - writes = append(writes, <-written) - } - for _, err := range writes { - noUpload("PutObjectPart/CopyObjectPart", err) - } - noUpload("CompleteMultipartUpload", <-completed) - _, got, err := r.read(oi.VersionID) - if err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents after public calls: %v", err) - } - }) - } -} - // POST records its status on a fresh read under the object lock, so an // Object Lock update it did not see survives. func TestRestorePOSTKeepsConcurrentProtection(t *testing.T) { @@ -721,6 +492,10 @@ func TestRestoreSupersededAttempt(t *testing.T) { t.Cleanup(unblock) older := newRestoreIsolationLayer(r) older.restore = func(ctx context.Context, bucket, object string, opts ObjectOptions) error { + // The queued worker loses the restore lock POST handed it, + // as when its refresh found it gone: the marker then belongs + // to no running restore. + opts.Transition.owner.release() <-release return r.z.RestoreTransitionedObject(ctx, bucket, object, opts) } @@ -984,204 +759,6 @@ func TestRestoreDuplicatePoolCacheStates(t *testing.T) { } } -// Stale-upload cleanup ages an upload by when it started, not by the -// version's ModTime, keeps the upload of a running restore beyond the stale -// age, and still removes ordinary stale uploads and those of ended attempts. -func TestRestoreUploadOutlivesStaleAge(t *testing.T) { - cleanup := func(t *testing.T, r *restoreTest) { - er := r.z.serverPools[0].getHashedSet(r.object) - for _, disk := range er.getDisks() { - er.cleanupStaleUploadsOnDisk(t.Context(), disk) - } - } - shortExpiry := func(t *testing.T) { - globalAPIConfig.mu.Lock() - prev := globalAPIConfig.staleUploadsExpiry - globalAPIConfig.staleUploadsExpiry = time.Nanosecond - globalAPIConfig.mu.Unlock() - t.Cleanup(func() { - globalAPIConfig.mu.Lock() - globalAPIConfig.staleUploadsExpiry = prev - globalAPIConfig.mu.Unlock() - }) - } - for _, mode := range []string{"old-version", "past-stale-age", "public-abort", "ordinary-stale", "unowned-marker"} { - t.Run(mode, func(t *testing.T) { - r := newRestoreSafetyTest(t, 2, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - if mode == "ordinary-stale" { - // Control: an abandoned client upload, old by its upload ID. - if _, err := r.z.NewMultipartUpload(t.Context(), r.bucket, r.object, ObjectOptions{MTime: UTCNow().Add(-72 * time.Hour)}); err != nil { - t.Fatal(err) - } - cleanup(t, r) - if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 0 { - t.Errorf("ordinary stale upload survived cleanup: %v", uploads) - } - return - } - var oi ObjectInfo - var body []byte - if mode == "past-stale-age" { - oi, body = restoreIsolationSeed(t, r, 0, false, true) - } else { - oi, body = nativeRestoreSeed(t, r, 0, false, true) - } - old := r.opts(oi.VersionID) - old.MTime = UTCNow().Add(-72 * time.Hour) - old.EvalMetadataFn = func(*ObjectInfo, error) (ReplicateDecision, error) { return ReplicateDecision{}, nil } - if _, err := r.z.PutObjectMetadata(t.Context(), r.bucket, r.object, old); err != nil { - t.Fatal(err) - } - // The cleanup with a short stale age also removes in-flight - // temporary parts; hold such restores in completion instead, - // before their commit. - gated := mode == "past-stale-age" || mode == "unowned-marker" - tier := newRestoreIsolationGateTier(5<<20, nil) - if !gated { - globalTierConfigMgr.drivercache["WARM"] = tier - } - defer tier.unblock() - gate := &restoreIsolationCompletionGate{entered: make(chan struct{}), release: make(chan struct{})} - defer gate.unblock() - if gated { - er := r.z.serverPools[0].getHashedSet(r.object) - original := er.getDisks - disks := original() - wrapped := make([]StorageAPI, len(disks)) - for i, d := range disks { - wrapped[i] = restoreIsolationValidationDisk{d, gate} - } - er.getDisks = func() []StorageAPI { return wrapped } - defer func() { er.getDisks = original }() - } - result := make(chan error, 1) - layer := newRestoreIsolationLayer(r) - if mode == "past-stale-age" { - // A POST publishes the attempt that a running restore owns. - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatalf("POST: %d %s", rec.Code, rec.Body.String()) - } - } else { - go func() { - result <- r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) - }() - } - if gated { - select { - case <-gate.entered: - case <-time.After(restoreTestWait): - t.Fatal("restore never reached completion") - } - } else { - restoreIsolationEntered(t, tier) - } - switch mode { - case "past-stale-age", "unowned-marker": - shortExpiry(t) - cleanup(t, r) - kept := len(restoreIsolationUploads(t, r, 0)) == 1 - if mode == "unowned-marker" && kept { - // The version is ongoing, but not for this direct restore's - // upload: an ongoing marker alone does not keep an upload. - t.Error("stale cleanup kept an upload no attempt owns") - } - if mode == "past-stale-age" && !kept { - t.Error("stale cleanup removed the upload of a running restore") - } - case "old-version": - cleanup(t, r) - case "public-abort": - id, _ := restoreIsolationUpload(t, r, 0) - if err := r.z.AbortMultipartUpload(t.Context(), r.bucket, r.object, id, ObjectOptions{}); !errors.As(err, &InvalidUploadID{}) { - t.Errorf("public abort of a native restore upload: %v, want NoSuchUpload", err) - } - } - tier.unblock() - gate.unblock() - var err error - if mode == "past-stale-age" { - err = layer.wait(t) - } else { - err = <-result - } - if mode == "unowned-marker" { - if err == nil { - t.Error("restore completed without its upload") - } - return - } - if err != nil { - t.Fatalf("restore lost its upload: %v", err) - } - _, got, err := r.read(oi.VersionID) - if err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents: %v", err) - } - }) - } -} - -// A failed multipart restore removes its own upload from the drives, also -// when a newer attempt owns the version by then, and leaves that newer -// attempt's marker alone. -func TestRestoreFailureAbortsUpload(t *testing.T) { - for _, mode := range []string{"download-failure", "superseded-mid-download"} { - t.Run(mode, func(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - initConfigSubsystem(t.Context(), r.z) - failure := errors.New("injected remote failure") - if mode == "download-failure" { - oi, _ := nativeRestoreSeed(t, r, 0, false, true) - tier := globalTierConfigMgr.drivercache["WARM"].(*restoreTestTier) - globalTierConfigMgr.drivercache["WARM"] = nativeRestoreFailTier{tier, failure} - if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); !errors.Is(err, failure) { - t.Fatalf("restore error=%v, want %v", err, failure) - } - if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 0 { - t.Errorf("failed native restore left its upload on disk: %v", uploads) - } - return - } - oi, _ := restoreIsolationSeed(t, r, 0, false, true) - tier := newRestoreIsolationGateTier(5<<20, failure) - globalTierConfigMgr.drivercache["WARM"] = tier - defer tier.unblock() - layer := newRestoreIsolationLayer(r) - if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { - t.Fatalf("POST: %d %s", rec.Code, rec.Body.String()) - } - restoreIsolationEntered(t, tier) - if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 1 { - t.Fatalf("restore upload on disk: %v", uploads) - } - // Model a newer generation that owns the version by now, such as - // one that took over after this restore lost its lock. - newer := r.opts(oi.VersionID) - newer.MTime = oi.ModTime - newer.NoLock = true - newer.EvalMetadataFn = func(o *ObjectInfo, _ error) (ReplicateDecision, error) { - o.UserDefined[restoreIsolationAttemptKey] = "newer-generation" - return ReplicateDecision{}, nil - } - if _, err := r.z.serverPools[0].getHashedSet(r.object).PutObjectMetadata(t.Context(), r.bucket, r.object, newer); err != nil { - t.Fatal(err) - } - tier.unblock() - if err := layer.wait(t); !errors.Is(err, failure) { - t.Fatalf("restore error=%v, want %v", err, failure) - } - if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 0 { - t.Errorf("superseded restore left its upload on disk: %v", uploads) - } - after := r.fileInfo(oi.VersionID) - if !strings.Contains(after.Metadata[xhttp.AmzRestore], "true") || after.Metadata[restoreIsolationAttemptKey] != "newer-generation" { - t.Errorf("failed restore cleared a newer attempt's marker: %q %q", after.Metadata[xhttp.AmzRestore], after.Metadata[restoreIsolationAttemptKey]) - } - }) - } -} - // A restored copy expires only by its own reason, on a fresh read: its // restore deadline passed and nothing else. A renewed deadline survives // another rule falling due and an expiry checked across all pools. @@ -1246,58 +823,6 @@ func TestExpireRestoredNeedsOwnReason(t *testing.T) { } } -// Caller metadata cannot make an upload a native restore upload: the marker -// is dropped, the upload stays an ordinary one and nothing commits it. -func TestRestoreUploadMarkerSpoofing(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - oi, _ := nativeRestoreSeed(t, r, 0, false, true) - forged := fmt.Sprintf(`{"id":%q,"v":%q,"e":%q,"s":%d,"m":%q}`, mustGetUUID(), oi.VersionID, oi.ETag, oi.Size, oi.ModTime.UTC().Format(time.RFC3339Nano)) - for _, target := range []string{"existing-version", "new-object"} { - t.Run(target, func(t *testing.T) { - opts := ObjectOptions{Versioned: true, UserDefined: map[string]string{restoreIsolationUploadKey: forged}} - object := r.object - if target == "existing-version" { - opts.VersionID = oi.VersionID - opts.MTime = oi.ModTime - opts.PreserveETag = oi.ETag - } else { - object = "spoofed-" + r.object - } - upload, err := r.z.NewMultipartUpload(t.Context(), r.bucket, object, opts) - if err != nil { - t.Fatal(err) - } - uploads, err := r.z.ListMultipartUploads(t.Context(), r.bucket, object, "", "", "", 100) - if err != nil || len(uploads.Uploads) != 1 || uploads.Uploads[0].UploadID != upload.UploadID { - t.Fatalf("a caller's upload must stay listed: %+v, %v", uploads.Uploads, err) - } - data := []byte("spoofed part") - part, err := r.z.PutObjectPart(t.Context(), r.bucket, object, upload.UploadID, 1, mustGetPutObjReader(t, bytes.NewReader(data), int64(len(data)), "", ""), ObjectOptions{}) - if err != nil { - t.Fatal(err) - } - before := nativeRestoreSnapshot(t, r) - done, err := r.z.CompleteMultipartUpload(t.Context(), r.bucket, object, upload.UploadID, []CompletePart{{PartNumber: 1, ETag: part.ETag}}, ObjectOptions{Versioned: true, MTime: opts.MTime}) - if target == "existing-version" { - if !errors.Is(err, errAuthentication) { - t.Errorf("spoofed upload overwrote the transitioned version: %v", err) - } - if !reflect.DeepEqual(before, nativeRestoreSnapshot(t, r)) { - t.Error("rejected spoofed completion changed the version") - } - return - } - if err != nil { - t.Fatal(err) - } - info, err := r.z.GetObjectInfo(t.Context(), r.bucket, object, ObjectOptions{VersionID: done.VersionID}) - if err != nil || info.UserDefined[restoreIsolationUploadKey] != "" { - t.Errorf("caller metadata committed a restore upload marker: %q, %v", info.UserDefined[restoreIsolationUploadKey], err) - } - }) - } -} - // Controls: an ordinary POST restore, a renewal, and a client's multipart // upload of the same key while a restore runs. func TestRestorePOSTControls(t *testing.T) { diff --git a/cmd/restore-native_test.go b/cmd/restore-native_test.go index 44676ac7c..61733e094 100644 --- a/cmd/restore-native_test.go +++ b/cmd/restore-native_test.go @@ -373,10 +373,9 @@ func TestNativeRestorePOSTVersionIdentity(t *testing.T) { } } -// A restore request is internal context, not an exception for normal S3 -// multipart overwrites or for a version that has never completed transition. -// Only the upload of a native restore attempt, whose marker names the very -// version being completed, may write that version back. +// A restore request is internal context, not an exception for multipart +// overwrites: a native restore never completes an upload, so no completion, +// whatever restore context it carries, writes an existing version back. func TestNativeRestoreMultipartOverwriteBoundary(t *testing.T) { for _, mode := range []string{"matching-restore", "ordinary-complete", "restore-without-attempt", "other-attempt", "not-tiered", "changed-time", "changed-size", "changed-etag"} { t.Run(mode, func(t *testing.T) { @@ -446,24 +445,7 @@ func TestNativeRestoreMultipartOverwriteBoundary(t *testing.T) { } before := nativeRestoreSnapshot(t, r) _, err = pool.CompleteMultipartUpload(t.Context(), r.bucket, r.object, upload.UploadID, complete, opts) - if mode == "matching-restore" { - if err != nil { - t.Fatalf("matching restore rejected: %v", err) - } - _, got, err := r.read(oi.VersionID) - if err != nil || !bytes.Equal(got, body) { - t.Fatalf("matching restore contents: %v", err) - } - if after := r.fileInfo(oi.VersionID); after.Metadata[restoreUploadKey] != "" || after.Metadata[restoreAttemptKey] != "" { - t.Error("restore committed its private upload or attempt marker") - } - return - } - if mode == "other-attempt" { - if !errors.As(err, &InvalidUploadID{}) { - t.Fatalf("another attempt completed a private upload: %v", err) - } - } else if !errors.Is(err, errAuthentication) { + if !errors.Is(err, errAuthentication) { t.Fatalf("existing version overwrite: %v", err) } if !reflect.DeepEqual(before, nativeRestoreSnapshot(t, r)) { @@ -525,7 +507,10 @@ func nativeRestoreLockRequest(ctx context.Context, t *testing.T, r *restoreTest, return rec } -func TestNativeRestoreSerializesObjectLock(t *testing.T) { +// A native restore downloads without the object lock. An Object Lock +// update made meanwhile commits at once, and the restore, whether it +// completes or fails, keeps it. +func TestNativeRestoreKeepsObjectLockUpdate(t *testing.T) { for _, kind := range []string{"legal-hold", "retention"} { for _, fail := range []bool{false, true} { t.Run(fmt.Sprintf("%s/failure=%v", kind, fail), func(t *testing.T) { @@ -567,22 +552,19 @@ func TestNativeRestoreSerializesObjectLock(t *testing.T) { case <-ctx.Done(): t.Fatal("restore never started download") } - updateCtx, updateCancel := context.WithTimeout(t.Context(), 200*time.Millisecond) + // The download holds no object lock: an Object Lock update + // commits at once, and the restore's commit keeps it. + updateCtx, updateCancel := context.WithTimeout(t.Context(), 5*time.Second) rec := nativeRestoreLockRequest(updateCtx, t, r, oi.VersionID, kind, updated) updateCancel() - if rec.Code == http.StatusOK { - t.Error("Object Lock update committed during restore's metadata snapshot") - // On the unfixed path this update commits, and the restore - // must not subsequently erase the acknowledged protection. - readOpts := r.opts(oi.VersionID) - readOpts.NoLock = true - before, err = r.z.GetObjectInfo(t.Context(), r.bucket, r.object, readOpts) - if err != nil { - t.Fatal(err) - } + if rec.Code != http.StatusOK { + t.Fatalf("Object Lock update during the restore download: %d %s", rec.Code, rec.Body.String()) } - if rec.Code != http.StatusOK && (rec.Code != http.StatusServiceUnavailable || !strings.Contains(rec.Body.String(), "RequestTimeout")) { - t.Errorf("update failed for an unrelated reason: %d %s", rec.Code, rec.Body.String()) + readOpts := r.opts(oi.VersionID) + readOpts.NoLock = true + during, err := r.z.GetObjectInfo(t.Context(), r.bucket, r.object, readOpts) + if err != nil { + t.Fatal(err) } unblock() select { @@ -598,26 +580,19 @@ func TestNativeRestoreSerializesObjectLock(t *testing.T) { t.Fatal(err) } for _, key := range keys { - if before.UserDefined[key] == "" || after.UserDefined[key] != before.UserDefined[key] { - t.Errorf("restore changed protection %s: %q -> %q", key, before.UserDefined[key], after.UserDefined[key]) + if during.UserDefined[key] == "" || after.UserDefined[key] != during.UserDefined[key] { + t.Errorf("restore changed protection %s: %q -> %q", key, during.UserDefined[key], after.UserDefined[key]) } } - if rec := nativeRestoreLockRequest(t.Context(), t, r, oi.VersionID, kind, updated); rec.Code != http.StatusOK { - t.Fatalf("protection update after restore: %d %s", rec.Code, rec.Body.String()) - } - final, err := r.z.GetObjectInfo(t.Context(), r.bucket, r.object, r.opts(oi.VersionID)) - if err != nil { - t.Fatal(err) - } - if !final.ModTime.Equal(before.ModTime) || final.ETag != before.ETag || final.Size != before.Size { - t.Error("Object Lock update changed content identity") - } changed := keys[0] if kind == "retention" { changed = keys[1] } - if final.UserDefined[changed] == before.UserDefined[changed] || final.UserDefined[keys[len(keys)-1]] == before.UserDefined[keys[len(keys)-1]] { - t.Error("new protection or its timestamp was not stored") + if after.UserDefined[changed] == before.UserDefined[changed] { + t.Error("the update made during the download was not kept") + } + if !after.ModTime.Equal(before.ModTime) || after.ETag != before.ETag || after.Size != before.Size { + t.Error("Object Lock update changed content identity") } }) } @@ -704,80 +679,6 @@ func TestNativeRestoreQueuedBehindDownload(t *testing.T) { } } -func TestNativeRestoreConcurrentCompletion(t *testing.T) { - for _, separateSets := range []bool{false, true} { - t.Run(fmt.Sprintf("separate-set-locks=%v", separateSets), func(t *testing.T) { - r := newRestoreSafetyTest(t, 1, versioning.Enabled) - if separateSets { - // Local production sets share one map. Independent maps model the - // distinct locker groups of distributed sets while reusing test disks. - pool := r.z.serverPools[0] - sets, count := pool.sets, pool.setCount - t.Cleanup(func() { pool.sets, pool.setCount = sets, count }) - first, second := *sets[0], *sets[0] - first.nsMutex, second.nsMutex = newNSLock(false), newNSLock(false) - second.setIndex = 1 - pool.sets, pool.setCount = []*erasureObjects{&first, &second}, 2 - for i := 0; ; i++ { - r.object = fmt.Sprintf("second-set-%d", i) - if pool.getHashedSetIndex(r.object) == 1 { - break - } - } - } - oi, body := nativeRestoreSeed(t, r, 0, false, true) - blocked := &nativeRestoreBlockedTier{WarmBackend: globalTierConfigMgr.drivercache["WARM"], entered: make(chan struct{}), release: make(chan struct{}), afterFirstPart: true} - globalTierConfigMgr.drivercache["WARM"] = blocked - var once sync.Once - unblock := func() { once.Do(func() { close(blocked.release) }) } - defer unblock() - ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) - defer cancel() - restored := make(chan error, 1) - go func() { - restored <- r.z.RestoreTransitionedObject(ctx, r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) - }() - select { - case <-blocked.entered: - case <-ctx.Done(): - t.Fatal("restore never started") - } - // A client that learns the internal upload ID, which listings no - // longer show, can try to complete the first part while native - // restore owns the object. - id, part := restoreIsolationUpload(t, r, 0) - completed := make(chan struct{}) - var completeErr error - go func() { - _, completeErr = r.z.CompleteMultipartUpload(ctx, r.bucket, r.object, id, []CompletePart{{PartNumber: 1, ETag: part.ETag}}, r.opts(oi.VersionID)) - close(completed) - }() - waitForListingReadLock(t, r.z.serverPools[0].getHashedSet(r.object).nsMutex, r.bucket, r.object, completed) - unblock() - select { - case err := <-restored: - if err != nil { - t.Errorf("restore blocked by concurrent completion: %v", err) - } - case <-time.After(5 * time.Second): - t.Fatal("restore did not finish") - } - select { - case <-completed: - case <-time.After(5 * time.Second): - t.Fatal("completion did not finish") - } - if !errors.As(completeErr, &InvalidUploadID{}) { - t.Errorf("completion after native restore: %v, want InvalidUploadID", completeErr) - } - _, got, err := r.read(oi.VersionID) - if err != nil || !bytes.Equal(got, body) { - t.Fatalf("restore contents after concurrent completion: %v", err) - } - }) - } -} - type nativeRestorePartGateReader struct { io.ReadCloser gate *nativeRestoreBlockedTier diff --git a/cmd/restore-stage.go b/cmd/restore-stage.go new file mode 100644 index 000000000..1bc2a443e --- /dev/null +++ b/cmd/restore-stage.go @@ -0,0 +1,900 @@ +// Copyright (c) 2026 Feng Ruohang +// +// This file is part of Silo Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "context" + "errors" + "fmt" + "io" + "maps" + "net/http" + "slices" + "strconv" + "sync" + "time" + + "github.com/google/uuid" + "github.com/minio/minio/internal/bucket/lifecycle" + xhttp "github.com/minio/minio/internal/http" + "github.com/pgsty/silo-pkg/v3/sync/errgroup" +) + +// A native restore runs in two phases. It downloads the tier copy of a +// version and erasure-codes it into the tmp volume of the version's set +// without the object lock O, then takes O only to check the version again +// and publish the staged copy drive by drive, as heal does. Hot versions of +// the key stay readable and writable during the download. +// +// The restore lock R of the version tells whether a restore of it runs: a +// POST takes R before O, and the worker that rehydrates the version holds R +// for its whole run. Nothing else takes R, so nothing holds O while it waits +// for R. R is a liveness hint, not an exclusion: a lost refresh can let two +// workers overlap. Only the checks under O decide what is written. + +// restoreOwnerPrefix names the restore locks under minioMetaBucket. +const restoreOwnerPrefix = "restore" + +// restorePublicationDelays are the in-memory retries of a convergence after +// the one under the commit's own lock hold, see recoverRestorePublication. +var restorePublicationDelays = []time.Duration{100 * time.Millisecond, time.Second, 5 * time.Second, 30 * time.Second, 2 * time.Minute} + +var ( + // errRestoreMoved: the version moved to another pool after the restore + // staged its copy in this one. + errRestoreMoved = errors.New("restored version moved to another pool") + // errRestoreNotConverged: a publication that reached some drives but not + // a write quorum could not be brought back to the unrestored version yet. + errRestoreNotConverged = errors.New("restore publication did not converge") +) + +// restoreOwner is the restore lock R of one version, held by a POST that +// registers an attempt and then by the worker that runs it. ctx ends when the +// lock is released or when its holder learns that it lost it. +type restoreOwner struct { + lk RWLocker + lkctx LockContext + ctx context.Context + cancel context.CancelFunc + once sync.Once +} + +// release unlocks R once. It is a no-op on a nil owner. +func (o *restoreOwner) release() { + if o == nil { + return + } + o.once.Do(func() { + o.cancel() + o.lk.Unlock(o.lkctx) + }) +} + +// restoreOwnerResource is the name of the restore lock of a version. Bucket +// names hold no slash, object names no ".." element, and the version ID has +// a fixed form, so the name is unambiguous. +func restoreOwnerResource(bucket, object, versionID string) string { + return pathJoin(restoreOwnerPrefix, bucket, object, restoreVersionID(versionID)) +} + +// takeRestoreOwner takes the restore lock of a version. With a nil timeout it +// waits for as long as ctx lives, as a direct caller does; otherwise it gives +// up after timeout. ctx must outlive the restore that the lock is held for. +func (z *erasureServerPools) takeRestoreOwner(ctx context.Context, bucket, object, versionID string, timeout *dynamicTimeout) (*restoreOwner, error) { + for { + // A new instance for each try: a distributed unlock retries in the + // background and could release a later try of the same instance. + lk := z.NewNSLock(minioMetaBucket, restoreOwnerResource(bucket, object, versionID)) + wait := timeout + if wait == nil { + wait = globalRestoreLockTimeout + } + lkctx, err := lk.GetLock(ctx, wait) + if err != nil { + if timeout == nil && errors.As(err, &OperationTimedOut{}) && ctx.Err() == nil { + continue + } + return nil, err + } + octx, cancel := context.WithCancel(lkctx.Context()) + return &restoreOwner{lk: lk, lkctx: lkctx, ctx: octx, cancel: cancel}, nil + } +} + +// restoreStage is the copy of one version that a native restore encoded into +// the tmp volume of the version's set, under tmpID/dataDir, before it takes +// the object lock. +type restoreStage struct { + versionID string + // fi is the version as the download read it: the commit only publishes + // the copy for a version that still has this content, see + // restoreSameContent. + fi FileInfo + tmpID string + dataDir string + // staged tells, in the order of the version's erasure distribution, + // which drives hold every part of the copy. + staged []bool + expiry time.Time + // request is set for an attempt that POST registered: days and date + // are what it recorded. A direct caller keeps the version's own. + request bool + days int + date string +} + +// restoreSameContent reports whether two reads of a version name the same +// encoded bytes, laid out the same way: the version, its tier copy, its parts +// and its erasure layout. The tier copy is never rewritten under the same +// name and remote version, so these decide the bytes. Every other field, such +// as tags, Object Lock, replication state or a rewrapped object key, is taken +// from the version as the commit reads it. +func restoreSameContent(a, b FileInfo) bool { + if a.VersionID != b.VersionID || a.Deleted || b.Deleted || !a.ModTime.Equal(b.ModTime) || + a.Metadata["etag"] != b.Metadata["etag"] || a.Size != b.Size || + a.TransitionStatus != lifecycle.TransitionComplete || b.TransitionStatus != lifecycle.TransitionComplete || + a.TransitionTier != b.TransitionTier || a.TransitionedObjName != b.TransitionedObjName || + a.TransitionVersionID != b.TransitionVersionID { + return false + } + if len(a.Parts) != len(b.Parts) { + return false + } + for i := range a.Parts { + if a.Parts[i].Number != b.Parts[i].Number || a.Parts[i].Size != b.Parts[i].Size { + return false + } + } + ea, eb := a.Erasure, b.Erasure + return ea.Algorithm == eb.Algorithm && ea.DataBlocks == eb.DataBlocks && ea.ParityBlocks == eb.ParityBlocks && + ea.BlockSize == eb.BlockSize && slices.Equal(ea.Distribution, eb.Distribution) +} + +// restoreInlineKey marks a version whose data lives in its xl.meta. +const restoreInlineKey = ReservedMetadataPrefixLower + "inline-data" + +// stageRestore downloads the tier copy of the version opts address and +// erasure-codes it, part by part, into the tmp volume of the drives of this +// set, as heal writes the parts it rebuilds. It holds no object lock. A drive +// that fails to write any part, or that misses a part afterwards, for +// example because its tmp volume was moved away when its server restarted, +// takes no further part and no part in the commit. +func (er erasureObjects) stageRestore(ctx context.Context, bucket, object string, opts ObjectOptions) (*restoreStage, error) { + rreq := opts.Transition.RestoreRequest + if rreq == nil { + return nil, errInvalidArgument + } + gen, err := er.readRestoreGeneration(ctx, bucket, object, opts.VersionID) + if err != nil { + return nil, toObjectErr(err, bucket, object, opts.VersionID) + } + fi := gen.fi + if fi.Deleted { + return nil, toObjectErr(errMethodNotAllowed, bucket, object) + } + if a := opts.Transition.attempt; a != nil && !a.ownsGeneration(gen, bucket, object) { + // A later POST took the version over, or it was replaced. + return nil, errRestoreSuperseded + } + disks := er.getDisks() + var total int64 + for _, p := range fi.Parts { + if p.Size < 0 { + return nil, toObjectErr(errFileCorrupt, bucket, object) + } + total += p.Size + } + if fi.TransitionStatus != lifecycle.TransitionComplete || len(fi.Parts) == 0 || total != fi.Size || + len(fi.Erasure.Distribution) != len(disks) { + return nil, InvalidObjectState{Bucket: bucket, Object: object} + } + erasure, err := NewErasure(ctx, fi.Erasure.DataBlocks, fi.Erasure.ParityBlocks, fi.Erasure.BlockSize) + if err != nil { + return nil, toObjectErr(err, bucket, object) + } + writeQuorum := fi.WriteQuorum(er.defaultWQuorum()) + st := &restoreStage{ + versionID: restoreVersionID(fi.VersionID), + fi: fi, + tmpID: mustGetUUID(), + dataDir: mustGetUUID(), + expiry: lifecycle.ExpectedExpiryTime(time.Now().UTC(), rreq.Days), + } + if opts.Transition.restoreDate != "" { + st.request, st.days, st.date = true, rreq.Days, opts.Transition.restoreDate + } + staged := false + defer func() { + if !staged { + er.deleteAll(context.Background(), minioMetaTmpBucket, st.tmpID) + } + }() + + // The tier copy is the encoded stream of the parts, encrypted or + // compressed as stored: the parts get exactly these bytes back. + oi := fi.ToObjectInfo(bucket, object, opts.Versioned || opts.VersionSuspended) + gr, err := getTransitionedObjectReader(ctx, bucket, object, nil, http.Header{}, oi, opts) + if err != nil { + return nil, toObjectErr(err, bucket, object) + } + defer gr.Close() + + var buffer []byte + pool := globalBytePoolCap.Load() + if int64(pool.Width()) < fi.Erasure.BlockSize { + buffer = make([]byte, fi.Erasure.BlockSize) + } else { + buffer = pool.Get() + defer pool.Put(buffer) + } + buffer = buffer[:fi.Erasure.BlockSize] + + // Slots follow the erasure distribution: the drive in slot s gets shard + // s+1 of every part, and the commit gives it erasure index s+1. + slots := shuffleDisks(disks, fi.Erasure.Distribution) + for _, part := range fi.Parts { + partPath := pathJoin(st.tmpID, st.dataDir, fmt.Sprintf("part.%d", part.Number)) + writers := make([]io.Writer, len(slots)) + for s, disk := range slots { + if disk != nil { + writers[s] = newBitrotWriter(disk, bucket, minioMetaTmpBucket, partPath, + erasure.ShardFileSize(part.Size), DefaultBitrotAlgorithm, erasure.ShardSize()) + } + } + n, err := erasure.Encode(ctx, io.LimitReader(gr, part.Size), writers, buffer, writeQuorum) + closeErrs := closeBitrotWriters(writers) + if err != nil { + return nil, toObjectErr(err, bucket, object) + } + if n != part.Size { + return nil, IncompleteBody{Bucket: bucket, Object: object} + } + for s := range slots { + if writers[s] == nil || closeErrs[s] != nil { + slots[s] = nil + } + } + if countOnlineDisks(slots) < writeQuorum { + return nil, toObjectErr(errErasureWriteQuorum, bucket, object) + } + } + // The tier copy must end with the last part. + var extra [1]byte + if n, _ := io.ReadFull(gr, extra[:]); n != 0 { + return nil, InvalidObjectState{Bucket: bucket, Object: object} + } + + // A tmp volume that was moved away and recreated by a later part lacks the + // earlier parts: find them before anything is published. CheckParts sees a + // part that is present and not shorter than its shards; a staged drive + // that wrote every part without an error has exactly that. + check := fi + check.DataDir = st.dataDir + g := errgroup.WithNErrs(len(slots)) + for s, disk := range slots { + if disk == nil { + continue + } + g.Go(func() error { + resp, err := disk.CheckParts(ctx, minioMetaTmpBucket, st.tmpID, check) + if err != nil { + return err + } + for _, result := range resp.Results { + if result != checkPartSuccess { + return errFileCorrupt + } + } + return nil + }, s) + } + st.staged = make([]bool, len(slots)) + count := 0 + for s, err := range g.Wait() { + if slots[s] != nil && err == nil { + st.staged[s] = true + count++ + } + } + if count < writeQuorum { + return nil, toObjectErr(errErasureWriteQuorum, bucket, object) + } + staged = true + return st, nil +} + +// fileInfoRevisions are the revisions the entry that writing fi creates can +// have, as readRestoreGeneration and the convergence identify the entries +// they read. The header flag that tells whether a transitioned version uses +// its data directory follows whether its restored copy has expired at the +// moment a drive writes the entry, so a restore whose copy expires while it +// publishes can find either value: both are this entry. Every other field of +// the header and the full metadata digest still have to match. +func fileInfoRevisions(fi FileInfo) ([]revisionKey, error) { + var x xlMetaV2 + if err := x.AddVersion(fi); err != nil { + return nil, err + } + buf, err := x.AppendTo(nil) + if err != nil { + return nil, err + } + var y xlMetaV2 + if err := y.Load(buf); err != nil { + return nil, err + } + if len(y.versions) != 1 { + return nil, errFileCorrupt + } + ver := y.versions[0] + other := ver + other.header.Flags ^= xlFlagUsesDataDir + return []revisionKey{versionRevision(ver, y.metaV), versionRevision(other, y.metaV)}, nil +} + +// restorePublication remembers, for a commit that published the staged copy +// on some drives but not on a write quorum, what each drive held before the +// commit (Sᵢ), what the commit wrote there (Pᵢ) and what a convergence writes +// there (Vᵢ), in the order of the version's erasure distribution. Only the +// worker that published knows them; a convergence touches a version only +// while every drive holds one of them. +type restorePublication struct { + bucket, object string + versionID string + uv [16]byte + dataDir string + attempt *restoreAttempt + quorum int + dist []int + before []*revisionKey + anyBefore map[revisionKey]bool + published map[revisionKey]bool + conv []FileInfo + convKeys map[revisionKey]bool +} + +// restorePublished is the entry a commit publishes on the drive in slot s: +// the version as the commit read it on that drive, with the staged data +// directory and a completed restore. Restore fields are written alike on +// every drive: the request POST recorded, or for a direct caller the one +// the authoritative generation auth holds. +func restorePublished(base, auth FileInfo, st *restoreStage, slot int) FileInfo { + fi := base + fi.Metadata = maps.Clone(base.Metadata) + if fi.Metadata == nil { + fi.Metadata = make(map[string]string) + } + for _, k := range restoreFieldKeys { + delete(fi.Metadata, k) + } + // An earlier restore may have inlined the data, and expiring that copy + // leaves the inline marker behind; this copy lives in its data directory. + delete(fi.Metadata, restoreInlineKey) + fi.Metadata[xhttp.AmzRestore] = completedRestoreObj(st.expiry).String() + if st.request { + fi.Metadata[xhttp.AmzRestoreExpiryDays] = strconv.Itoa(st.days) + fi.Metadata[xhttp.AmzRestoreRequestDate] = st.date + } else { + for _, k := range []string{xhttp.AmzRestoreExpiryDays, xhttp.AmzRestoreRequestDate} { + if v, ok := auth.Metadata[k]; ok { + fi.Metadata[k] = v + } + } + } + fi.DataDir = st.dataDir + fi.Data = nil + fi.Parts = slices.Clone(base.Parts) + fi.Erasure.Index = slot + 1 + // The staged parts use the default streaming bitrot algorithm. + fi.Erasure.Checksums = nil + return fi +} + +// restoreUnrestored is the entry a convergence writes on the drive in slot s: +// the version as the commit read it, with no restore fields, so that its tier +// copy serves reads again and a later POST can restore it. +func restoreUnrestored(base FileInfo, slot int) FileInfo { + fi := base + fi.Metadata = maps.Clone(base.Metadata) + for _, k := range restoreFieldKeys { + delete(fi.Metadata, k) + } + delete(fi.Metadata, restoreInlineKey) + fi.Data = nil + fi.Erasure.Index = slot + 1 + return fi +} + +// commitRestore publishes a staged copy while the caller holds the object +// lock. It reads the version again, checks that the attempt still owns it and +// that it still has the content the copy was staged for, then publishes the +// copy on every staged drive that holds the version, with RenameData on each +// drive, as heal does: never through erasure renameData, whose undo of a +// replaced version could delete the new data and keep metadata that points +// at it. +// +// It returns nil when the copy reached a write quorum. Otherwise it returns +// an error, and a publication when some drive may already have the copy; the +// caller must then converge, see convergeRestore. +func (er erasureObjects) commitRestore(ctx context.Context, bucket, object string, st *restoreStage, opts ObjectOptions, placed func(context.Context) error) (*restorePublication, error) { + if placed != nil { + if err := placed(ctx); err != nil { + return nil, err + } + } + gen, err := er.readRestoreGeneration(ctx, bucket, object, st.versionID) + if err != nil { + return nil, toObjectErr(err, bucket, object, st.versionID) + } + if gen.fi.Deleted { + return nil, toObjectErr(errMethodNotAllowed, bucket, object) + } + a := opts.Transition.attempt + if a != nil && !a.ownsGeneration(gen, bucket, object) { + return nil, errRestoreSuperseded + } + if !restoreSameContent(st.fi, gen.fi) { + // Replaced, or its tier copy or layout changed since the download. + return nil, InvalidObjectState{Bucket: bucket, Object: object} + } + disks := er.getDisks() + dist := gen.fi.Erasure.Distribution + if len(dist) != len(disks) || len(gen.revs) != len(disks) { + return nil, InvalidObjectState{Bucket: bucket, Object: object} + } + n := len(disks) + writeQuorum := gen.fi.WriteQuorum(er.defaultWQuorum()) + + pub := &restorePublication{ + bucket: bucket, object: object, versionID: st.versionID, dataDir: st.dataDir, + attempt: a, quorum: writeQuorum, dist: slices.Clone(dist), + before: make([]*revisionKey, n), + anyBefore: make(map[revisionKey]bool), + published: make(map[revisionKey]bool), + conv: make([]FileInfo, n), + convKeys: make(map[revisionKey]bool), + } + if st.versionID != nullVersionID { + id, err := uuid.Parse(st.versionID) + if err != nil { + return nil, toObjectErr(errFileVersionNotFound, bucket, object, st.versionID) + } + pub.uv = id + } + targets := make([]StorageAPI, n) + files := make([]FileInfo, n) + for i := range disks { + s := dist[i] - 1 + if s < 0 || s >= n { + return nil, InvalidObjectState{Bucket: bucket, Object: object} + } + if gen.revs[i] != nil { + pub.before[s] = gen.revs[i] + pub.anyBefore[*gen.revs[i]] = true + } + v := restoreUnrestored(gen.fi, s) + keys, err := fileInfoRevisions(v) + if err != nil { + return nil, toObjectErr(err, bucket, object) + } + pub.conv[s] = v + // The unrestored entry has no restore header: its flag never varies. + pub.convKeys[keys[0]] = true + if gen.disks[i] == nil || !st.staged[s] { + continue + } + fi := restorePublished(gen.drives[i], gen.fi, st, s) + if keys, err = fileInfoRevisions(fi); err != nil { + return nil, toObjectErr(err, bucket, object) + } + for _, key := range keys { + pub.published[key] = true + } + targets[s], files[s] = gen.disks[i], fi + } + if countOnlineDisks(targets) < writeQuorum { + // Nothing is published. + return nil, toObjectErr(errErasureWriteQuorum, bucket, object) + } + + errs := make([]error, n) + olds := make([]string, n) + var wg sync.WaitGroup + for s, disk := range targets { + if disk == nil { + errs[s] = errDiskNotFound + continue + } + wg.Add(1) + go func() { + defer wg.Done() + resp, err := disk.RenameData(ctx, minioMetaTmpBucket, st.tmpID, files[s], bucket, object, RenameOptions{}) + olds[s], errs[s] = resp.OldDataDir, err + }() + } + wg.Wait() + + if countErrs(errs, nil) < writeQuorum { + return pub, toObjectErr(errErasureWriteQuorum, bucket, object) + } + // The data directory of the replaced entry, which held a former restored + // copy, goes only when every drive that published reports the same one: + // RenameData reports none where another version shares it, and then the + // directory stays everywhere. It goes only from the drives that reported + // it. + old := "" + cleanup := make([]StorageAPI, n) + for s := range targets { + if errs[s] != nil { + continue + } + if olds[s] == "" || (old != "" && olds[s] != old) { + old = "" + break + } + old = olds[s] + cleanup[s] = targets[s] + } + if old != "" && old != st.dataDir { + er.commitRenameDataDir(ctx, bucket, object, old, cleanup, 1) + } + if countErrs(errs, nil) < n { + er.addPartial(bucket, object, gen.fi.VersionID) + } + return nil, nil +} + +// restoreConvergence is the outcome of one convergence pass. +type restoreConvergence int + +const ( + // restoreIncomplete: the drives are not settled yet; try again. + restoreIncomplete restoreConvergence = iota + // restoreConverged: every drive that holds the version holds it + // unrestored again. + restoreConverged + // restoreCommitted: the copy reached a write quorum after all, such as + // when a drive applied a publication whose answer was lost. + restoreCommitted + // restoreStopped: some drive holds an entry that neither this attempt + // nor the version before it wrote: someone else changed the version. + restoreStopped +) + +// classify reads the entry of the version on every drive and sorts it into +// the slot of its drive: held tells which slots hold an entry this +// publication knows, published and converged count the drives that hold +// what the commit and the convergence write. A drive that cannot be read is +// left out. An entry the publication does not know stops it. +func (pub *restorePublication) classify(ctx context.Context, disks []StorageAPI) (held []bool, published, converged int, err error) { + raws, errs := readAllRawFileInfo(ctx, disks, pub.bucket, pub.object, false) + held = make([]bool, len(disks)) + for i := range disks { + if errs[i] != nil || i >= len(pub.dist) { + continue + } + var xl xlMetaV2 + if xl.LoadOrConvert(raws[i].Buf) != nil { + continue + } + s := pub.dist[i] - 1 + for _, ver := range xl.versions { + if ver.header.VersionID != pub.uv || ver.header.FreeVersion() { + continue + } + key := versionRevision(ver, xl.metaV) + switch { + case pub.published[key]: + published++ + case pub.convKeys[key]: + converged++ + case pub.before[s] != nil && *pub.before[s] == key: + case pub.before[s] == nil && pub.anyBefore[key]: + default: + return nil, 0, 0, errRestoreSuperseded + } + held[s] = true + break + } + } + return held, published, converged, nil +} + +// convergeRestore settles a publication while the caller holds the object +// lock. When every drive still holds what the version held before the commit, +// what the commit wrote, or what an earlier pass wrote, it either finds the +// copy on a write quorum, or writes the unrestored version to every drive +// that holds the version. It deletes no directory: an unreferenced staged +// directory is left to abandoned-data cleanup. Any other entry stops it, so +// that it never overwrites what someone else wrote between two passes. +func (er erasureObjects) convergeRestore(ctx context.Context, pub *restorePublication) (restoreConvergence, error) { + disks := er.getDisks() + held, published, _, err := pub.classify(ctx, disks) + if err != nil { + return restoreStopped, err + } + if published >= pub.quorum { + er.addPartial(pub.bucket, pub.object, pub.versionID) + return restoreCommitted, nil + } + slots := shuffleDisks(disks, pub.dist) + files := make([]FileInfo, len(slots)) + for s := range slots { + if !held[s] { + slots[s] = nil + continue + } + files[s] = pub.conv[s] + } + _, werr := writeUniqueFileInfo(ctx, slots, "", pub.bucket, pub.object, files, pub.quorum) + + held, published, converged, err := pub.classify(ctx, disks) + if err != nil { + return restoreStopped, err + } + holding := 0 + for _, h := range held { + if h { + holding++ + } + } + if published == 0 && converged == holding && converged >= pub.quorum { + return restoreConverged, nil + } + if werr == nil { + werr = errErasureWriteQuorum + } + return restoreIncomplete, werr +} + +// recoverRestorePublication retries a convergence in memory, for a bounded +// time, each pass under the object lock taken afresh. A restart loses the +// retries; the version then stays as the drives hold it, see the design's +// excluded residuals. +func (er erasureObjects) recoverRestorePublication(pub *restorePublication, lock func(context.Context) (LockContext, func(), error)) { + go func() { + err := errRestoreNotConverged + for _, delay := range restorePublicationDelays { + select { + case <-GlobalContext.Done(): + return + case <-time.After(delay): + } + ctx, cancel := context.WithTimeout(GlobalContext, restoreCleanupTimeout) + lkctx, unlock, lerr := lock(ctx) + if lerr != nil { + cancel() + err = lerr + continue + } + var res restoreConvergence + res, err = er.convergeRestore(lkctx.Context(), pub) + unlock() + cancel() + if res == restoreConverged || res == restoreCommitted { + return + } + if res == restoreStopped { + break + } + } + er.logRestoreNotConverged(pub, err) + }() +} + +func (er erasureObjects) logRestoreNotConverged(pub *restorePublication, err error) { + attempt := "" + if pub.attempt != nil { + attempt = pub.attempt.ID + } + storageLogIf(GlobalContext, fmt.Errorf("%w: %s/%s(%s), attempt %q, data dir %s: %v", + errRestoreNotConverged, pub.bucket, pub.object, pub.versionID, attempt, pub.dataDir, err)) +} + +// restoreHooks tie a restore of one set to its caller. +type restoreHooks struct { + // lock takes the object lock O of the version, in the lock domain of the + // object's writes, and returns its release. + lock func(ctx context.Context) (LockContext, func(), error) + // placed tells, under O, whether the version still lives in this set. + placed func(ctx context.Context) error + // clear clears the attempt's ongoing marker after a failure that + // published nothing, taking O itself, and retries in the background. + clear func() +} + +// restoreVersion rehydrates the transitioned version opts address in this +// set: it stages the copy without the object lock, then commits it under the +// lock. A failure that published nothing clears the attempt's marker. A +// publication that reached some drives but not a write quorum converges +// under the same lock hold, and in bounded background retries after it. +func (er erasureObjects) restoreVersion(ctx context.Context, bucket, object string, opts ObjectOptions, h restoreHooks) error { + st, err := er.stageRestore(ctx, bucket, object, opts) + if err != nil { + return restoreAbandon(err, h) + } + defer er.deleteAll(context.Background(), minioMetaTmpBucket, st.tmpID) + + lkctx, unlock, err := h.lock(ctx) + if err != nil { + return restoreAbandon(err, h) + } + pub, err := er.commitRestore(lkctx.Context(), bucket, object, st, opts, h.placed) + if err == nil { + unlock() + return nil + } + if pub == nil { + unlock() + return restoreAbandon(err, h) + } + res, cerr := er.convergeRestore(lkctx.Context(), pub) + unlock() + switch res { + case restoreCommitted: + return nil + case restoreConverged: + return err + case restoreStopped: + er.logRestoreNotConverged(pub, cerr) + return err + default: + er.recoverRestorePublication(pub, h.lock) + return err + } +} + +// restoreAbandon clears the marker of an attempt that published nothing, +// unless another attempt owns the version. A not-found error may come from the +// tier rather than from the version, and the clear checks the marker's owner +// and finds nothing to do for a version that is gone, so it runs then too. +func restoreAbandon(err error, h restoreHooks) error { + if errors.Is(err, errRestoreSuperseded) { + return err + } + if h.clear != nil { + h.clear() + } + return err +} + +// RestoreTransitionedObject - restore transitioned object content locally on +// this cluster. This is similar to PostObjectRestore from AWS GLACIER storage +// class. When PostObjectRestore API is called, a temporary copy of the object +// is restored locally to the bucket on source cluster until the restore +// expiry date. The copy that was transitioned continues to reside in the +// transitioned tier. +// +// The set entry serves direct callers of one set. It takes the object lock in +// the set's own domain and no restore lock; native restores of a cluster go +// through erasureServerPools. +func (er erasureObjects) RestoreTransitionedObject(ctx context.Context, bucket, object string, opts ObjectOptions) error { + lock := func(ctx context.Context) (LockContext, func(), error) { + lk := er.NewNSLock(bucket, object) + lkctx, err := lk.GetLock(ctx, globalOperationTimeout) + if err != nil { + return LockContext{}, nil, err + } + return lkctx, func() { lk.Unlock(lkctx) }, nil + } + clearMarker := func() { + ctx, cancel := context.WithTimeout(GlobalContext, restoreCleanupTimeout) + defer cancel() + lkctx, unlock, err := lock(ctx) + if err != nil { + storageLogIf(ctx, fmt.Errorf("Unable to clear the restore status of %s/%s(%s): %w", bucket, object, opts.VersionID, err)) + return + } + defer unlock() + versionID := opts.VersionID + if a := opts.Transition.attempt; a != nil { + versionID = a.VersionID + } + if err := er.clearRestoreStatus(lkctx.Context(), bucket, object, versionID, opts.Transition.attempt); err != nil && + !isErrObjectNotFound(err) && !isErrVersionNotFound(err) && !isErrMethodNotAllowed(err) { + storageLogIf(ctx, fmt.Errorf("Unable to clear the restore status of %s/%s(%s): %w", bucket, object, versionID, err)) + } + } + return er.restoreVersion(ctx, bucket, object, opts, restoreHooks{lock: lock, clear: clearMarker}) +} + +// RestoreTransitionedObject - restore transitioned object content locally on +// this cluster, see erasureObjects.RestoreTransitionedObject. +// +// A POST hands over the restore lock R of the version it registered in +// opts.Transition.owner; a direct caller takes R here, waiting while ctx +// lives. The restore runs while R is held, and releases it when it returns. +func (z *erasureServerPools) RestoreTransitionedObject(ctx context.Context, bucket, object string, opts ObjectOptions) error { + object = encodeDirObject(object) + owner := opts.Transition.owner + opts.Transition.owner = nil + if owner != nil && owner.ctx.Err() != nil { + // The lock POST handed over ended before the restore ran, such as + // when its holder lost it while the restore was queued: take it again + // as a direct caller does. The attempt still has to own the version. + owner = nil + } + defer func() { owner.release() }() + + // Avoid restoring object from a pool being decommissioned. + opts.SkipDecommissioned = true + opts.NoLock = true + clearMarker := func() { + if err := z.resetRestoreStatus(bucket, object, opts); err != nil { + z.recoverRestoreStatus(bucket, object, opts) + } + } + idx, err := z.restorePoolIdx(ctx, bucket, object, opts) + if err != nil { + var reset restoreResetError + if errors.As(err, &reset) { + clearMarker() + return reset.error + } + if isErrObjectNotFound(err) && opts.VersionID != "" { + err = VersionNotFound{Bucket: bucket, Object: decodeDirObject(object), VersionID: opts.VersionID} + } + return err + } + // A server may have been replaced by an older binary while the restore + // was queued: such a server knows neither the restore lock nor the + // staged commit. + if err := nativeRestoreAvailable(ctx); err != nil { + clearMarker() + return err + } + er := z.serverPools[idx].getHashedSet(object) + if owner == nil { + if opts.VersionID == "" { + // Restore the version that is latest now, by its own ID. + gen, err := er.readRestoreGeneration(ctx, bucket, object, "") + if err != nil { + return toObjectErr(err, bucket, object) + } + opts.VersionID = restoreVersionID(gen.fi.VersionID) + } + if owner, err = z.takeRestoreOwner(ctx, bucket, object, opts.VersionID, nil); err != nil { + return err + } + } + return er.restoreVersion(owner.ctx, bucket, object, opts, restoreHooks{ + lock: func(ctx context.Context) (LockContext, func(), error) { + lk := z.restoreLock(bucket, object) + lkctx, err := lk.GetLock(ctx, globalOperationTimeout) + if err != nil { + return LockContext{}, nil, err + } + return lkctx, func() { lk.Unlock(lkctx) }, nil + }, + placed: func(ctx context.Context) error { + got, err := z.restorePoolIdx(ctx, bucket, object, opts) + var reset restoreResetError + if errors.As(err, &reset) { + return reset.error + } + if err != nil { + return err + } + if got != idx { + return errRestoreMoved + } + return nil + }, + clear: clearMarker, + }) +} diff --git a/cmd/restore-two-phase_test.go b/cmd/restore-two-phase_test.go new file mode 100644 index 000000000..ba3e981f1 --- /dev/null +++ b/cmd/restore-two-phase_test.go @@ -0,0 +1,858 @@ +// Copyright (c) 2026 Feng Ruohang +// +// This file is part of Silo Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/bucket/versioning" + "github.com/minio/minio/internal/config/storageclass" + xhttp "github.com/minio/minio/internal/http" +) + +// A native restore downloads its tier copy without the object lock: while +// the download of an old version is paused, the hot versions of the same key +// stay readable and writable. Only the short commit takes the object lock. +func TestRestoreDownloadKeepsKeyAvailable(t *testing.T) { + for _, pools := range []int{1, 2} { + for _, multipart := range []bool{false, true} { + t.Run(fmt.Sprintf("pools=%d/multipart=%v", pools, multipart), func(t *testing.T) { + r := newRestoreSafetyTest(t, pools, versioning.Enabled) + initConfigSubsystem(t.Context(), r.z) + oi, body := nativeRestoreSeed(t, r, 0, false, multipart) + hot := putConsistencyObject(t, r.z, r.bucket, r.object, 0, "hot latest", ObjectOptions{Versioned: true, MTime: oi.ModTime.Add(time.Hour)}) + putConsistencyObject(t, r.z, r.bucket, "unrelated", 0, "unrelated hot object", ObjectOptions{Versioned: true}) + gate := newRestoreIsolationGateTier(1, nil) + globalTierConfigMgr.drivercache["WARM"] = gate + defer gate.unblock() + done := make(chan error, 1) + go func() { + done <- r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + }() + restoreIsolationEntered(t, gate) + + within := func(name string, fn func(ctx context.Context) error) { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + start := time.Now() + if err := fn(ctx); err != nil { + t.Fatalf("%s during a paused restore download: %v after %s", name, err, time.Since(start)) + } + } + for _, key := range []string{"unrelated", r.object} { + within("GET "+key, func(ctx context.Context) error { + gr, err := r.z.GetObjectNInfo(ctx, r.bucket, key, nil, http.Header{}, ObjectOptions{}) + if err != nil { + return err + } + defer gr.Close() + _, err = io.ReadAll(gr) + return err + }) + } + within("HEAD hot latest", func(ctx context.Context) error { + got, err := r.z.GetObjectInfo(ctx, r.bucket, r.object, ObjectOptions{}) + if err == nil && got.VersionID != hot.VersionID { + err = fmt.Errorf("latest is %s, want %s", got.VersionID, hot.VersionID) + } + return err + }) + within("PutObjectTags hot latest", func(ctx context.Context) error { + _, err := r.z.PutObjectTags(ctx, r.bucket, r.object, "during=restore", ObjectOptions{VersionID: hot.VersionID, Versioned: true}) + return err + }) + // The download publishes nothing: no upload, public or not. + if uploads := restoreIsolationUploads(t, r, 0); len(uploads) != 0 { + t.Errorf("restore created uploads: %v", uploads) + } + within("PUT new version", func(ctx context.Context) error { + data := []byte("written during restore") + _, err := r.z.PutObject(ctx, r.bucket, r.object, + mustGetPutObjReader(t, bytes.NewReader(data), int64(len(data)), "", ""), ObjectOptions{Versioned: true}) + return err + }) + + gate.unblock() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Minute): + t.Fatal("restore did not complete after releasing the tier") + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored version mismatch: %v", err) + } + }) + } + } +} + +// restoreTwoPhaseStagedCommitFailure makes a multipart completion stage its +// object metadata in the upload and then fail before moving any data. +type restoreTwoPhaseStagedCommitFailure struct { + StorageAPI + bucket, object string +} + +func (d restoreTwoPhaseStagedCommitFailure) RenameData(ctx context.Context, sv, sp string, fi FileInfo, dv, dp string, opts RenameOptions) (RenameDataResp, error) { + if sv == minioMetaMultipartBucket && dv == d.bucket && dp == d.object { + if err := d.WriteMetadata(ctx, "", sv, sp, fi); err != nil { + return RenameDataResp{}, err + } + return RenameDataResp{}, errFaultyDisk + } + return d.StorageAPI.RenameData(ctx, sv, sp, fi, dv, dp, opts) +} + +// An ordinary upload whose completion failed after staging stays usable as +// on main: the client re-uploads its parts and completes, or aborts. +func TestMultipartFailedCommitRetriesLikeMain(t *testing.T) { + for _, abort := range []bool{false, true} { + t.Run(fmt.Sprintf("abort=%v", abort), func(t *testing.T) { + z, bucket := consistencyPools(t) + initConfigSubsystem(t.Context(), z) + const object = "public-upload" + er := z.serverPools[0].getHashedSet(object) + ctx := t.Context() + mp, err := er.NewMultipartUpload(ctx, bucket, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + body := []byte("ordinary multipart data") + putPart := func() (PartInfo, error) { + return er.PutObjectPart(ctx, bucket, object, mp.UploadID, 1, + mustGetPutObjReader(t, bytes.NewReader(body), int64(len(body)), "", ""), ObjectOptions{}) + } + p, err := putPart() + if err != nil { + t.Fatal(err) + } + disks := er.getDisks() + wrapped := make([]StorageAPI, len(disks)) + for i, d := range disks { + wrapped[i] = restoreTwoPhaseStagedCommitFailure{d, bucket, object} + } + previous := er.getDisks + er.getDisks = func() []StorageAPI { return wrapped } + _, err = er.CompleteMultipartUpload(ctx, bucket, object, mp.UploadID, []CompletePart{{PartNumber: 1, ETag: p.ETag}}, ObjectOptions{}) + er.getDisks = previous + if err == nil { + t.Fatal("fault injection failed") + } + if _, err := er.GetMultipartInfo(ctx, bucket, object, mp.UploadID, ObjectOptions{}); err != nil { + t.Fatalf("upload after a failed commit: %v", err) + } + if abort { + if err := er.AbortMultipartUpload(ctx, bucket, object, mp.UploadID, ObjectOptions{}); err != nil { + t.Fatalf("abort after a failed commit: %v", err) + } + if _, err := er.GetMultipartInfo(ctx, bucket, object, mp.UploadID, ObjectOptions{}); !errors.As(err, &InvalidUploadID{}) { + t.Fatalf("aborted upload still answers: %v", err) + } + return + } + if p, err = putPart(); err != nil { + t.Fatalf("re-upload a part after a failed commit: %v", err) + } + if _, err := er.CompleteMultipartUpload(ctx, bucket, object, mp.UploadID, []CompletePart{{PartNumber: 1, ETag: p.ETag}}, ObjectOptions{}); err != nil { + t.Fatalf("complete after re-uploading: %v", err) + } + gr, err := er.GetObjectNInfo(ctx, bucket, object, nil, http.Header{}, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + defer gr.Close() + if got, err := io.ReadAll(gr); err != nil || !bytes.Equal(got, body) { + t.Fatalf("completed object = %q, %v", got, err) + } + }) + } +} + +// restoreTwoPhaseFaults are the faults restoreTwoPhaseDisk injects, shared by +// a test and its drives so that the test can turn them off. +type restoreTwoPhaseFaults struct { + // rename: "" passes the publication through, "fail" fails it without + // applying it, "apply-fail" applies it and then reports a failure. + rename atomic.Value + // oldDataDir, when set, replaces the old data directory a publication + // reports. + oldDataDir atomic.Pointer[string] + // writeFails fails this many metadata writes of the object. + writeFails atomic.Int32 + // writes counts the metadata writes of the object. + writes atomic.Int32 + // recreateTmp drops the staging directory before the second part is + // written, as a server restart moves the tmp volume away. + recreateTmp atomic.Bool +} + +func (f *restoreTwoPhaseFaults) renameMode() string { + m, _ := f.rename.Load().(string) + return m +} + +type restoreTwoPhaseDisk struct { + StorageAPI + bucket, object string + f *restoreTwoPhaseFaults +} + +func (d restoreTwoPhaseDisk) RenameData(ctx context.Context, sv, sp string, fi FileInfo, dv, dp string, opts RenameOptions) (RenameDataResp, error) { + if sv != minioMetaTmpBucket || dv != d.bucket || dp != d.object { + return d.StorageAPI.RenameData(ctx, sv, sp, fi, dv, dp, opts) + } + switch d.f.renameMode() { + case "fail": + return RenameDataResp{}, errFaultyDisk + case "apply-fail": + if _, err := d.StorageAPI.RenameData(ctx, sv, sp, fi, dv, dp, opts); err != nil { + return RenameDataResp{}, err + } + return RenameDataResp{}, errFaultyDisk + } + resp, err := d.StorageAPI.RenameData(ctx, sv, sp, fi, dv, dp, opts) + if old := d.f.oldDataDir.Load(); old != nil { + resp.OldDataDir = *old + } + return resp, err +} + +func (d restoreTwoPhaseDisk) WriteMetadata(ctx context.Context, orig, vol, path string, fi FileInfo) error { + if vol == d.bucket && path == d.object { + d.f.writes.Add(1) + if d.f.writeFails.Load() > 0 { + d.f.writeFails.Add(-1) + return errFaultyDisk + } + } + return d.StorageAPI.WriteMetadata(ctx, orig, vol, path, fi) +} + +func (d restoreTwoPhaseDisk) CreateFile(ctx context.Context, origvolume, volume, path string, size int64, reader io.Reader) error { + if volume == minioMetaTmpBucket && strings.HasSuffix(path, "/part.2") && d.f.recreateTmp.Load() { + tmpID, _, _ := strings.Cut(path, "/") + if err := os.RemoveAll(filepath.Join(d.Endpoint().Path, minioMetaTmpBucket, tmpID)); err != nil { + return err + } + } + return d.StorageAPI.CreateFile(ctx, origvolume, volume, path, size, reader) +} + +// restoreTwoPhaseFaulty wraps the drives of the object's set in the first +// pool; faults[i] is nil for a drive left as it is. It returns the plain +// drives, for reads that see what is stored. +func restoreTwoPhaseFaulty(t *testing.T, r *restoreTest, faults ...*restoreTwoPhaseFaults) []StorageAPI { + t.Helper() + er := r.z.serverPools[0].getHashedSet(r.object) + disks := er.getDisks() + previous := er.getDisks + t.Cleanup(func() { er.getDisks = previous }) + wrapped := append([]StorageAPI(nil), disks...) + for i, f := range faults { + if f != nil { + wrapped[i] = restoreTwoPhaseDisk{StorageAPI: disks[i], bucket: r.bucket, object: r.object, f: f} + } + } + er.getDisks = func() []StorageAPI { return wrapped } + return disks +} + +// restoreTwoPhaseEntries reads the version from each drive. +func restoreTwoPhaseEntries(t *testing.T, r *restoreTest, disks []StorageAPI, versionID string) []FileInfo { + t.Helper() + infos, errs := readAllFileInfo(t.Context(), disks, "", r.bucket, r.object, restoreTestVersionKey(versionID), false, false) + for i, err := range errs { + if err != nil { + t.Fatalf("drive %d: %v", i, err) + } + } + return infos +} + +func restoreTwoPhaseUnrestored(fi FileInfo) bool { + return fi.Metadata[xhttp.AmzRestore] == "" && fi.Metadata[restoreAttemptKey] == "" && fi.IsRemote() +} + +func restoreTwoPhaseRestored(fi FileInfo) bool { + st, err := parseRestoreObjStatus(fi.Metadata[xhttp.AmzRestore]) + return err == nil && !st.Ongoing() && !fi.IsRemote() && fi.Metadata[restoreAttemptKey] == "" +} + +// restoreTwoPhaseDelays shortens the convergence retries for a test. +func restoreTwoPhaseDelays(t *testing.T, delays ...time.Duration) { + previous := restorePublicationDelays + restorePublicationDelays = delays + t.Cleanup(func() { restorePublicationDelays = previous }) +} + +// A publication that reaches fewer drives than the write quorum is brought +// back to the unrestored version on every drive, under the same lock hold. +// No directory is deleted: the staged one is left to abandoned-data cleanup. +// The version restores again once the drives work. +func TestRestorePublicationBelowQuorumReverts(t *testing.T) { + for _, multipart := range []bool{false, true} { + t.Run(fmt.Sprintf("multipart=%v", multipart), func(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, body := nativeRestoreSeed(t, r, 0, false, multipart) + faults := &restoreTwoPhaseFaults{} + faults.rename.Store("fail") + disks := restoreTwoPhaseFaulty(t, r, faults, faults) + before := restoreTwoPhaseEntries(t, r, disks, oi.VersionID) + err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + if err == nil { + t.Fatal("a publication on two of four drives succeeded") + } + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if !restoreTwoPhaseUnrestored(fi) || fi.DataDir != before[i].DataDir { + t.Errorf("drive %d after a failed publication: restore %q, remote %v, data dir %s (was %s)", + i, fi.Metadata[xhttp.AmzRestore], fi.IsRemote(), fi.DataDir, before[i].DataDir) + } + } + // The drives that published keep the staged directory. + for i := 2; i < len(disks); i++ { + entries, err := os.ReadDir(filepath.Join(disks[i].Endpoint().Path, r.bucket, r.object)) + if err != nil { + t.Fatal(err) + } + dirs := 0 + for _, e := range entries { + if e.IsDir() && e.Name() != before[i].DataDir { + dirs++ + } + } + if dirs == 0 { + t.Errorf("drive %d: the published data directory was deleted", i) + } + } + faults.rename.Store("") + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err != nil { + t.Fatalf("restore after the drives recovered: %v", err) + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } + }) + } +} + +// A publication whose drives applied it but did not answer is found on a +// write quorum when the worker reads the drives again: the restore stands. +func TestRestorePublicationUncertainCommits(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, body := nativeRestoreSeed(t, r, 0, false, true) + faults := &restoreTwoPhaseFaults{} + faults.rename.Store("apply-fail") + disks := restoreTwoPhaseFaulty(t, r, faults, faults) + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err != nil { + t.Fatalf("restore whose answers were lost: %v", err) + } + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if !restoreTwoPhaseRestored(fi) { + t.Errorf("drive %d: not restored: %q", i, fi.Metadata[xhttp.AmzRestore]) + } + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } +} + +// A convergence that cannot write every drive under the commit's lock hold +// finishes in a background retry. +func TestRestoreConvergenceRetries(t *testing.T) { + restoreTwoPhaseDelays(t, 50*time.Millisecond, 50*time.Millisecond, 50*time.Millisecond) + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, _ := nativeRestoreSeed(t, r, 0, false, false) + failing, flaky := &restoreTwoPhaseFaults{}, &restoreTwoPhaseFaults{} + failing.rename.Store("fail") + flaky.writeFails.Store(1) + disks := restoreTwoPhaseFaulty(t, r, failing, failing, flaky) + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err == nil { + t.Fatal("a publication on two of four drives succeeded") + } + deadline := time.Now().Add(restoreTestWait) + for { + settled := true + for _, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + settled = settled && restoreTwoPhaseUnrestored(fi) + } + if settled { + return + } + if time.Now().After(deadline) { + t.Fatal("background convergence did not finish") + } + time.Sleep(20 * time.Millisecond) + } +} + +// A convergence retry never overwrites what someone else wrote between two +// lock holds: a metadata update or a newer attempt's marker stops it. +func TestRestoreConvergenceStopsOnOtherWrites(t *testing.T) { + for _, other := range []string{"tags", "successor"} { + t.Run(other, func(t *testing.T) { + restoreTwoPhaseDelays(t, 300*time.Millisecond, 300*time.Millisecond, 300*time.Millisecond) + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + initConfigSubsystem(t.Context(), r.z) + oi, body := restoreIsolationSeed(t, r, 0, false, false) + failing, stuck := &restoreTwoPhaseFaults{}, &restoreTwoPhaseFaults{} + failing.rename.Store("fail") + stuck.writeFails.Store(1 << 20) + disks := restoreTwoPhaseFaulty(t, r, failing, failing, stuck) + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err == nil { + t.Fatal("a publication on two of four drives succeeded") + } + var successor string + release := make(chan struct{}) + layer := newRestoreIsolationLayer(r) + switch other { + case "tags": + if _, err := r.z.PutObjectTags(t.Context(), r.bucket, r.object, "kept=yes", r.opts(oi.VersionID)); err != nil { + t.Fatal(err) + } + case "successor": + layer.restore = func(ctx context.Context, bucket, object string, opts ObjectOptions) error { + <-release + return r.z.RestoreTransitionedObject(ctx, bucket, object, opts) + } + if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { + close(release) + t.Fatalf("successor POST: %d %s", rec.Code, rec.Body.String()) + } + successor = restoreTwoPhaseEntries(t, r, disks, oi.VersionID)[0].Metadata[restoreAttemptKey] + if successor == "" { + close(release) + t.Fatal("successor POST recorded no attempt") + } + } + writes := stuck.writes.Load() + time.Sleep(time.Second) + if got := stuck.writes.Load(); got != writes { + t.Errorf("a stopped convergence kept writing: %d -> %d writes", writes, got) + } + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if i == 2 { + continue // the drive the convergence could not write + } + switch other { + case "tags": + if fi.Metadata[xhttp.AmzObjectTagging] != "kept=yes" { + t.Errorf("drive %d: the tags written between retries were overwritten: %q", i, fi.Metadata[xhttp.AmzObjectTagging]) + } + case "successor": + if fi.Metadata[restoreAttemptKey] != successor { + t.Errorf("drive %d: the successor's marker was overwritten: %q", i, fi.Metadata[restoreAttemptKey]) + } + } + } + if other == "successor" { + failing.rename.Store("") + stuck.writeFails.Store(0) + close(release) + if err := layer.wait(t); err != nil { + t.Fatalf("successor restore: %v", err) + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } + } + }) + } +} + +// The data directory of the replaced entry goes only where every drive that +// reports it agrees on it; a drive that does not report it, as when another +// version shares the directory, keeps the directory everywhere. +func TestRestoreRemovesOldDataDirOnlyWhenUnanimous(t *testing.T) { + for _, mixed := range []bool{false, true} { + t.Run(fmt.Sprintf("mixed=%v", mixed), func(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, body := nativeRestoreSeed(t, r, 0, false, true) + old := r.fileInfo(oi.VersionID).DataDir + if old == "" { + t.Fatal("fixture: the transitioned version has no data directory") + } + faults := &restoreTwoPhaseFaults{} + if mixed { + none := "" + faults.oldDataDir.Store(&none) + } + restoreTwoPhaseFaulty(t, r, faults) + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err != nil { + t.Fatal(err) + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } + n := r.dataDirs(old) + if mixed && n != 4 { + t.Errorf("drives disagreed on the old data directory, but %d of 4 kept it", n) + } + if !mixed && n != 0 { + t.Errorf("the unanimous old data directory remained on %d drives", n) + } + }) + } +} + +// An earlier restore of a small object may have inlined its data, and the +// expiry of that copy leaves the inline marker behind. A new restore writes +// a data directory and must not publish the marker with it. +func TestRestoreDropsStaleInlineMarker(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, body := nativeRestoreSeed(t, r, 0, false, false) + disks := r.z.serverPools[0].getHashedSet(r.object).getDisks() + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + fi.SetInlineData() + if err := disks[i].WriteMetadata(t.Context(), "", r.bucket, r.object, fi); err != nil { + t.Fatal(err) + } + } + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err != nil { + t.Fatal(err) + } + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if fi.InlineData() { + t.Errorf("drive %d: the restored copy is marked inline", i) + } + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } +} + +// A drive whose staging directory was moved away and recreated by a later +// part misses the earlier parts: it is left out of the commit, and when too +// few drives are left nothing is published at all. +func TestRestoreStagingDropsRecreatedTmp(t *testing.T) { + for _, recreated := range []int{1, 2} { + t.Run(fmt.Sprintf("drives=%d", recreated), func(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, body := nativeRestoreSeed(t, r, 0, false, true) + faults := &restoreTwoPhaseFaults{} + faults.recreateTmp.Store(true) + wrap := make([]*restoreTwoPhaseFaults, recreated) + for i := range wrap { + wrap[i] = faults + } + disks := restoreTwoPhaseFaulty(t, r, wrap...) + err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + entries := restoreTwoPhaseEntries(t, r, disks, oi.VersionID) + if recreated == 2 { + if err == nil { + t.Fatal("a copy staged completely on two of four drives was published") + } + for i, fi := range entries { + if !restoreTwoPhaseUnrestored(fi) { + t.Errorf("drive %d after a failed staging: %q", i, fi.Metadata[xhttp.AmzRestore]) + } + } + return + } + if err != nil { + t.Fatal(err) + } + if restoreTwoPhaseRestored(entries[0]) { + t.Error("the drive with missing parts got the restored copy") + } + for i := 1; i < len(entries); i++ { + if !restoreTwoPhaseRestored(entries[i]) { + t.Errorf("drive %d: not restored", i) + } + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } + }) + } +} + +// A POST while the worker of an earlier POST holds the restore lock answers +// that a restore is in progress; it starts no second worker. +func TestRestoreSecondPOSTWhileRunning(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + initConfigSubsystem(t.Context(), r.z) + oi, body := restoreIsolationSeed(t, r, 0, false, true) + release := make(chan struct{}) + layer := newRestoreIsolationLayer(r) + layer.restore = func(ctx context.Context, bucket, object string, opts ObjectOptions) error { + <-release + return r.z.RestoreTransitionedObject(ctx, bucket, object, opts) + } + if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { + t.Fatalf("first POST: %d %s", rec.Code, rec.Body.String()) + } + rec := restoreIsolationPOST(t, r, layer, oi.VersionID) + close(release) + if rec.Code != http.StatusConflict || !strings.Contains(rec.Body.String(), "RestoreAlreadyInProgress") { + t.Errorf("second POST: %d %s", rec.Code, rec.Body.String()) + } + if err := layer.wait(t); err != nil { + t.Fatal(err) + } + select { + case err := <-layer.done: + t.Fatalf("a second restore ran: %v", err) + default: + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } +} + +// A version deleted while its tier copy downloads is not brought back. +func TestRestoreTargetDeletedDuringDownload(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + initConfigSubsystem(t.Context(), r.z) + oi, _ := nativeRestoreSeed(t, r, 0, false, false) + gate := newRestoreIsolationGateTier(1, nil) + globalTierConfigMgr.drivercache["WARM"] = gate + defer gate.unblock() + done := make(chan error, 1) + go func() { + done <- r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + }() + restoreIsolationEntered(t, gate) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + _, err := r.z.DeleteObject(ctx, r.bucket, r.object, r.opts(oi.VersionID)) + cancel() + if err != nil { + t.Fatalf("delete during the download: %v", err) + } + gate.unblock() + select { + case err := <-done: + if err == nil { + t.Fatal("the restore of a deleted version succeeded") + } + case <-time.After(restoreTestWait): + t.Fatal("restore did not finish") + } + if _, err := r.z.GetObjectInfo(t.Context(), r.bucket, r.object, r.opts(oi.VersionID)); !isErrVersionNotFound(err) && !isErrObjectNotFound(err) { + t.Fatalf("deleted version after the restore: %v", err) + } +} + +// With several pools, abandoned-data cleanup takes the pools lock that every +// write of the object takes, also where a pool's sets have lockers of their +// own: while a writer holds it, cleanup removes nothing, such as a data +// directory that a commit moved in place and has not yet referenced. +func TestCheckAbandonedPartsTakesPoolsLock(t *testing.T) { + r := newRestoreSafetyTest(t, 2, versioning.Enabled) + nativeRestoreSeed(t, r, 1, false, false) + for _, set := range r.z.serverPools[1].sets { + previous := set.nsMutex + set.nsMutex = newNSLock(false) + t.Cleanup(func() { set.nsMutex = previous }) + } + disks := r.z.serverPools[1].getHashedSet(r.object).getDisks() + moved := mustGetUUID() + for _, disk := range disks { + dir := filepath.Join(disk.Endpoint().Path, r.bucket, r.object, moved) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "part.1"), []byte("staged"), 0o644); err != nil { + t.Fatal(err) + } + } + present := func() int { + n := 0 + for _, disk := range disks { + if _, err := os.Stat(filepath.Join(disk.Endpoint().Path, r.bucket, r.object, moved)); err == nil { + n++ + } + } + return n + } + lk := r.z.NewObjectNSLock(r.bucket, r.object) + lkctx, err := lk.GetLock(t.Context(), globalOperationTimeout) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + r.z.CheckAbandonedParts(ctx, r.bucket, r.object, madmin.HealOpts{Remove: true}) + cancel() + held := present() + lk.Unlock(lkctx) + if held != len(disks) { + t.Fatalf("abandoned-data cleanup removed a moved directory from %d drives while a writer held the object lock", len(disks)-held) + } + if err := r.z.CheckAbandonedParts(t.Context(), r.bucket, r.object, madmin.HealOpts{Remove: true}); err != nil { + t.Fatalf("abandoned-data cleanup without a writer: %v", err) + } + if n := present(); n != 0 { + t.Fatalf("abandoned-data cleanup left the unreferenced directory on %d drives", n) + } +} + +// The header flag that tells whether a transitioned version uses its data +// directory follows whether its restored copy has expired when a drive writes +// the entry. A publication's own entry is recognized with either value. +func TestRestorePublishedRevisionAcrossExpiry(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + oi, _ := nativeRestoreSeed(t, r, 0, false, false) + fi := r.fileInfo(oi.VersionID) + expiry := time.Now().Add(2 * time.Second).Truncate(time.Second).Add(time.Second) + st := &restoreStage{dataDir: mustGetUUID(), expiry: expiry} + published := restorePublished(fi, fi, st, 0) + keys, err := fileInfoRevisions(published) + if err != nil { + t.Fatal(err) + } + stored := func() revisionKey { + var x xlMetaV2 + if err := x.AddVersion(published); err != nil { + t.Fatal(err) + } + buf, err := x.AppendTo(nil) + if err != nil { + t.Fatal(err) + } + var y xlMetaV2 + if err := y.Load(buf); err != nil { + t.Fatal(err) + } + return versionRevision(y.versions[0], y.metaV) + } + before := stored() + time.Sleep(time.Until(expiry) + 100*time.Millisecond) + after := stored() + if before == after { + t.Fatal("fixture: the stored flag did not change at the expiry") + } + for _, got := range []revisionKey{before, after} { + if got != keys[0] && got != keys[1] { + t.Errorf("a publication's own entry is not recognized: %v not in %v", got, keys) + } + } +} + +// A tier that lost the copy answers not found. That is no reason to keep the +// version's ongoing marker: the attempt clears it, and a later POST can +// restore the version again. +func TestRestoreTierNotFoundClearsMarker(t *testing.T) { + r := newRestoreSafetyTest(t, 1, versioning.Enabled) + initConfigSubsystem(t.Context(), r.z) + oi, _ := restoreIsolationSeed(t, r, 0, false, false) + tier := globalTierConfigMgr.drivercache["WARM"] + globalTierConfigMgr.drivercache["WARM"] = nativeRestoreFailTier{tier, ObjectNotFound{Bucket: r.bucket, Object: r.object}} + layer := newRestoreIsolationLayer(r) + if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK { + t.Fatalf("POST: %d %s", rec.Code, rec.Body.String()) + } + if err := layer.wait(t); !isErrObjectNotFound(err) { + t.Fatalf("restore from a tier without the copy: %v", err) + } + info, err := r.z.GetObjectInfo(t.Context(), r.bucket, r.object, r.opts(oi.VersionID)) + if err != nil || info.RestoreOngoing || info.UserDefined[xhttp.AmzRestore] != "" { + t.Fatalf("version after the failed restore: %q, %v", info.UserDefined[xhttp.AmzRestore], err) + } +} + +// newRestoreWideTest is newRestoreSafetyTest with one pool of twelve drives, +// whose default layout is eight data and four parity shards: a version's +// metadata reaches its read quorum on seven drives, one short of the write +// quorum of eight that its shards need. +func newRestoreWideTest(t *testing.T) *restoreTest { + t.Helper() + prevTier := globalTierConfigMgr + t.Cleanup(func() { globalTierConfigMgr = prevTier }) + dirs, err := getRandomDisks(12) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { removeRoots(dirs) }) + obj, _, err := initObjectLayer(t.Context(), mustGetPoolEndpoints(0, dirs...)) + if err != nil { + t.Fatal(err) + } + z := obj.(*erasureServerPools) + t.Cleanup(func() { z.Shutdown(context.Background()) }) + // Other tests leave a storage class behind; fix the layout to 8+4. + saved := globalStorageClass + globalStorageClass.Update(storageclass.Config{Standard: storageclass.StorageClass{Parity: 4}}) + t.Cleanup(func() { globalStorageClass.Update(saved) }) + globalTierConfigMgr = NewTierConfigMgr() + globalTierConfigMgr.drivercache["WARM"] = &restoreTestTier{tierExpiryTestBackend: &tierExpiryTestBackend{objects: map[string][]byte{}}} + r := &restoreTest{t: t, z: z, bucket: getRandomBucketName(), object: "obj", labels: map[string]string{}, bodies: map[string][]byte{}} + if err := z.MakeBucket(t.Context(), r.bucket, MakeBucketOptions{VersioningEnabled: true}); err != nil { + t.Fatal(err) + } + meta, err := globalBucketMetadataSys.Get(r.bucket) + if err != nil { + t.Fatal(err) + } + meta.LifecycleConfigXML = []byte(restoreSafetyRules) + if err := meta.Save(t.Context(), z); err != nil { + t.Fatal(err) + } + globalBucketMetadataSys.Set(r.bucket, meta) + return r +} + +// On twelve drives with eight data shards, a publication that reached seven +// drives has the metadata read quorum but not the eight shards a read needs. +// It is not a restore: the worker brings every drive back to the unrestored +// version, and the version reads from its tier copy. +func TestRestorePublicationBelowWriteQuorumOnWideSet(t *testing.T) { + r := newRestoreWideTest(t) + oi, body := nativeRestoreSeed(t, r, 0, false, true) + fi := r.fileInfo(oi.VersionID) + if fi.Erasure.DataBlocks != 8 || fi.Erasure.ParityBlocks != 4 { + t.Fatalf("fixture layout %d+%d, want 8+4", fi.Erasure.DataBlocks, fi.Erasure.ParityBlocks) + } + faults := &restoreTwoPhaseFaults{} + faults.rename.Store("fail") + disks := restoreTwoPhaseFaulty(t, r, faults, faults, faults, faults, faults) + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err == nil { + t.Fatal("a publication on seven of twelve drives succeeded") + } + for i, e := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if !restoreTwoPhaseUnrestored(e) || e.DataDir != fi.DataDir { + t.Errorf("drive %d after a publication on seven drives: restore %q, remote %v", i, e.Metadata[xhttp.AmzRestore], e.IsRemote()) + } + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("the version no longer reads from its tier copy: %v", err) + } + faults.rename.Store("") + if err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)); err != nil { + t.Fatalf("restore after the drives recovered: %v", err) + } + for i, e := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if !restoreTwoPhaseRestored(e) { + t.Errorf("drive %d: not restored", i) + } + } +}