astelm/minio
1
0
Fork 0
mirror of https://github.com/pgsty/minio.git synced 2026-11-11 01:55:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(pools): lock single-pool object writes in the object's own erasure set

Browse Source 
In a single pool, DELETE in every form, bulk delete, the CopyObject
destination, SelectObjectContent, batch key rotation and the upload-ID
guards of complete, abort, get-info and list-parts locked through the
first erasure set, while PUT, multipart commit, PutObjectPart, tag,
retention, legal-hold and metadata updates, restore, transition and heal
lock in the set the object hashes to. When the sets of a pool use
different hosts, as every automatic layout of 18 or more hosts does, the
two locks ask different lockers and do not exclude each other, and
concurrent read-modify-writes of xl.meta can lose acknowledged versions
and delete markers or leave an object on a minority of drives.

Route these locks through NewObjectNSLock, which returns the lock of
the object's own set in a single pool and the pools lock otherwise.
Upload guards choose the set by their object and keep the name
object/uploadID. True prefix deletion and the locks of names that are
not objects keep their domain.

Bulk delete groups its names by set and takes one set's lock at a time,
in ascending set index; a failed group fails alone. A single node, whose
sets share one local lock table, keeps its one bulk lock.

The COPY handler takes its source and destination locks before it opens
the source, sorted, and transactionally: bounded waits while holding a
lock, release in reverse order and retry with new lock instances, one
deadline. It reads the source under them, passes NoLock to every nested
call, and runs that work in the locks' context. This also removes a
deadlock of copies in opposite directions wherever both locks were in
one domain, such as a single node or several pools.

Lock names and RPCs are unchanged, but a pool is protected only when
every node runs this change: affected pools must be upgraded and rolled
back with a drained restart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-05 03:31:03 +08:00
parent 7f38bf6282
commit 32ab506924
9 changed files with 2194 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/object-handlers.go
+47 -14
View File
@@ -34,6 +34,7 @@ import (
"sort"
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
@@ -183,7 +184,7 @@ func (api objectAPIHandlers) SelectObjectContentHandler(w http.ResponseWriter, r
// Take read lock on object, here so subsequent lower-level
// calls do not need to.
lock := objectAPI.NewNSLock(bucket, object)
lock := objectAPI.NewObjectNSLock(bucket, object)
lkctx, err := lock.GetRLock(ctx, globalOperationTimeout)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
@@ -1470,12 +1471,35 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
return checkCopyObjectPreconditions(ctx, w, r, o)
}
getOpts.CheckPrecondFn = checkCopyPrecondFn
if cpSrcDstSame {
getOpts.NoLock = true
// Lock the source for reading and a local destination for writing before
// the source is opened, both in the lock domain of their writers. Taking
// them together in one order keeps copies in opposite directions from
// holding each other; a copy onto itself takes one write lock. The source
// is then read, preconditions and inline data included, and the
// destination looked up and written, all under these locks and without
// taking them again. That work runs in lockCtx, which ends when either
// lock is lost and so aborts the copy; errors are still reported in the
// request's context. Both locks are released once the copy has been
// written: a synchronous replication below reads the destination with its
// own lock.
locks := []objectLockRequest{{bucket: srcBucket, object: srcObject}}
if !remoteCallRequired {
locks = append(locks, objectLockRequest{bucket: dstBucket, object: dstObject, write: true})
}
lkctx, unlock, err := objectAPI.lockObjects(ctx, locks...)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
unlockCopy := sync.OnceFunc(unlock)
defer unlockCopy()
lockCtx := lkctx.Context()
getOpts.NoLock = true
dstOpts.NoLock = true
var rs *HTTPRangeSpec
gr, err := getObjectNInfo(ctx, srcBucket, srcObject, rs, r.Header, getOpts)
gr, err := getObjectNInfo(lockCtx, srcBucket, srcObject, rs, r.Header, getOpts)
if err != nil {
if isErrPreconditionFailed(err) {
return
@@ -1542,7 +1566,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
length := actualSize
if !cpSrcDstSame {
if err := enforceBucketQuotaHard(ctx, dstBucket, actualSize); err != nil {
if err := enforceBucketQuotaHard(lockCtx, dstBucket, actualSize); err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
@@ -1560,12 +1584,12 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
compressMetadata[ReservedMetadataPrefix+"compression"] = compressionAlgorithmV2
compressMetadata[ReservedMetadataPrefix+"actual-size"] = strconv.FormatInt(actualSize, 10)
reader = etag.NewReader(ctx, reader, nil, nil)
reader = etag.NewReader(lockCtx, reader, nil, nil)
} else {
reader = gr
}
srcInfo.Reader, err = hash.NewReader(ctx, reader, length, "", "", actualSize)
srcInfo.Reader, err = hash.NewReader(lockCtx, reader, length, "", "", actualSize)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -1660,7 +1684,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
}
}
if err = rotateKey(ctx, oldKey, newKeyID, newKey, srcBucket, srcObject, encMetadata, kmsCtx); err != nil {
if err = rotateKey(lockCtx, oldKey, newKeyID, newKey, srcBucket, srcObject, encMetadata, kmsCtx); err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
@@ -1745,7 +1769,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
dstOpts.IndexCB = cb
defer s2c.Close()
reader = etag.Wrap(s2c, checksumReader)
srcInfo.Reader, err = hash.NewReader(ctx, reader, -1, "", "", actualSize)
srcInfo.Reader, err = hash.NewReader(lockCtx, reader, -1, "", "", actualSize)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -1758,7 +1782,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
if isTargetEncrypted && !remoteCallRequired {
var encReader io.Reader
kind, _ := crypto.IsRequested(r.Header)
encReader, objEncKey, err = newEncryptReader(ctx, srcInfo.Reader, kind, newKeyID, newKey, dstBucket, dstObject, encMetadata, kmsCtx)
encReader, objEncKey, err = newEncryptReader(lockCtx, srcInfo.Reader, kind, newKeyID, newKey, dstBucket, dstObject, encMetadata, kmsCtx)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -1773,7 +1797,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
}
// do not try to verify encrypted content
srcInfo.Reader, err = hash.NewReader(ctx, reader, targetSize, "", "", actualSize)
srcInfo.Reader, err = hash.NewReader(lockCtx, reader, targetSize, "", "", actualSize)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -1845,7 +1869,12 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
srcInfo.UserDefined = objectlock.FilterObjectLockMetadata(srcInfo.UserDefined, true, true)
retPerms := isPutActionAllowed(ctx, getRequestAuthType(r), dstBucket, dstObject, r, policy.PutObjectRetentionAction)
holdPerms := isPutActionAllowed(ctx, getRequestAuthType(r), dstBucket, dstObject, r, policy.PutObjectLegalHoldAction)
getObjectInfo := objectAPI.GetObjectInfo
// The destination is locked above: look it up without its lock, in the
// locks' context.
getObjectInfo := func(_ context.Context, bucket, object string, opts ObjectOptions) (ObjectInfo, error) {
opts.NoLock = true
return objectAPI.GetObjectInfo(lockCtx, bucket, object, opts)
}
// apply default bucket configuration/governance headers for dest side.
retentionMode, retentionDate, legalHold, s3Err := checkPutObjectLockAllowed(ctx, r, dstBucket, dstObject, getObjectInfo, retPerms, holdPerms, replicaTrusted)
@@ -2031,9 +2060,11 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
// For an ordinary federated copy, actualSize is the logical plaintext
// length to declare. srcInfo.Size is not a reliable wire length because
// the read path may already have adjusted it.
writeCtx, modified := withFederatedWriteTime(ctx)
writeCtx, modified := withFederatedWriteTime(lockCtx)
remoteObjInfo, rerr := core.PutObject(writeCtx, dstBucket, dstObject, srcInfo.Reader,
actualSize, "", "", opts)
gr.Close()
unlockCopy()
if rerr != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, rerr), r.URL)
return
@@ -2080,7 +2111,9 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
// Copy source object to destination, if source and destination
// object is same then only metadata is updated.
objInfo, err = copyObjectFn(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo, copySrcOpts, dstOpts)
objInfo, err = copyObjectFn(lockCtx, srcBucket, srcObject, dstBucket, dstObject, srcInfo, copySrcOpts, dstOpts)
gr.Close()
unlockCopy()
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return