mirror of
https://github.com/pgsty/minio.git
synced 2026-09-25 12:25:57 +03:00
fix: close residual bucket-metadata races (issue #105 audit)
Audit of the three deferred #105 follow-ups. Each reproduces with a deterministic red test in cmd/bucket-metadata-race_test.go, and each fix is the minimal change that turns its test green while preserving the <bucket>.lck -> metadata.lock -> .metadata.bin lock order established by #103. 1. Lifecycle expiry merge lost update (persistent). PeerBucketLCConfigHandler and healBucketILMExpiry read the current lifecycle with an unlocked GetConfigFromDisk, merged the replicated expiry rules with the local transition rules, then wrote the pre-computed blob via Update. Any lifecycle transition change committed between the merge read and the merge write was silently lost. New BucketMetadataSys.UpdateExpiryLCConfig performs the read, merge, and save under one metadata.lock; mergeExpiryWithLCConfig now takes the locked snapshot and validates object-lock retention from it instead of re-reading (avoids a re-entrant metadata load under the lock). 2. DeleteBucket ghost .metadata.bin (persistent). DeleteBucket took only <bucket>.lck while config writers take only metadata.lock, so a writer that was mid-save could re-create .metadata.bin after the prefix purge. The purge now runs under metadata.lock, with a best-effort unlocked fallback so a delete is never blocked from completing. 3. Overlapping peer reloads publishing a stale resident cache (freshness only; the persisted record stays correct). LoadBucketMetadataHandler and the GetConfig cache-miss path published with an unconditional Set, so a reload that read an older revision could overwrite a newer resident record until the next refresh. New BucketMetadataSys.setReloaded (and a matching GetConfig guard) refuses to regress a newer resident record, mirroring refreshBucketsMetadataLoop. Verification: go build -tags kqueue,dev ./...; go vet ./cmd; gofmt clean; rebrand-guard baseline unchanged; go test -tags kqueue,dev ./cmd (207s) green; new tests plus the #103 metadata suite green under -race. Refs #105. Parent #102. Foundation #103. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
@@ -125,6 +125,28 @@ func (sys *BucketMetadataSys) Set(bucket string, meta BucketMetadata) {
|
||||
}
|
||||
}
|
||||
|
||||
// setReloaded publishes a record freshly loaded from disk into the resident
|
||||
// cache without letting an older on-disk revision overwrite a newer resident
|
||||
// one. Overlapping peer reloads (LoadBucketMetadataHandler) and cache-miss
|
||||
// loads can finish out of order, so an unconditional Set can leave the resident
|
||||
// cache a revision behind until the next refresh (issue #105). The authoritative
|
||||
// read-modify-write save path uses Set directly and always wins because it
|
||||
// stamps a fresh updatedAt. Only a shallow copy is stored, exactly like Set.
|
||||
func (sys *BucketMetadataSys) setReloaded(bucket string, meta BucketMetadata) {
|
||||
if isMinioMetaBucketName(bucket) {
|
||||
return
|
||||
}
|
||||
sys.Lock()
|
||||
defer sys.Unlock()
|
||||
if cur, ok := sys.metadataMap[bucket]; ok && !cur.lastUpdate().Before(meta.lastUpdate()) {
|
||||
// A resident revision that is at least as new is already published; do
|
||||
// not regress it to the older reload.
|
||||
return
|
||||
}
|
||||
sys.metadataMap[bucket] = meta
|
||||
sys.clearLoadFailure(bucket)
|
||||
}
|
||||
|
||||
func (sys *BucketMetadataSys) updateAndParse(ctx context.Context, bucket string, configFile string, configData []byte, parse, lifecycleDelete bool) (updatedAt time.Time, err error) {
|
||||
objAPI := newObjectLayerFn()
|
||||
if objAPI == nil {
|
||||
@@ -292,6 +314,57 @@ func (sys *BucketMetadataSys) Update(ctx context.Context, bucket string, configF
|
||||
return sys.updateAndParse(ctx, bucket, configFile, configData, true, false)
|
||||
}
|
||||
|
||||
// UpdateExpiryLCConfig merges a replicated ILM expiry configuration with the
|
||||
// bucket's current lifecycle document and persists the merged result while
|
||||
// holding metadata.lock across the read, merge, and save. The site-replication
|
||||
// expiry heal and peer-apply paths must use this instead of computing the merge
|
||||
// from an unlocked GetConfigFromDisk read and then writing it with Update: that
|
||||
// two-step sequence drops any lifecycle transition change committed in between
|
||||
// (issue #105). Lock order stays <bucket>.lck -> metadata.lock -> .metadata.bin;
|
||||
// the merge and save run under metadata.lock and the peer fan-out runs after it
|
||||
// is released.
|
||||
func (sys *BucketMetadataSys) UpdateExpiryLCConfig(ctx context.Context, bucket string, expLCConfig *string, updatedAt time.Time) error {
|
||||
objAPI := newObjectLayerFn()
|
||||
if objAPI == nil {
|
||||
return errServerNotInitialized
|
||||
}
|
||||
|
||||
if isMinioMetaBucketName(bucket) {
|
||||
return errInvalidArgument
|
||||
}
|
||||
|
||||
notifyCtx := ctx
|
||||
ctx, unlock, err := lockBucketMetadata(ctx, objAPI, bucket)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
err = func() error {
|
||||
defer unlock()
|
||||
meta, err := loadBucketMetadataParse(ctx, objAPI, bucket, true)
|
||||
if err != nil {
|
||||
if !globalIsErasure && !globalIsDistErasure && errors.Is(err, errVolumeNotFound) {
|
||||
// Only single drive mode needs this fallback.
|
||||
meta = newBucketMetadata(bucket)
|
||||
} else {
|
||||
return err
|
||||
}
|
||||
}
|
||||
configData, err := mergeExpiryWithLCConfig(bucket, meta, expLCConfig, updatedAt)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
meta.LifecycleConfigXML = configData
|
||||
meta.LifecycleConfigUpdatedAt = UTCNow()
|
||||
return sys.saveMetadata(ctx, objAPI, meta)
|
||||
}()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
globalNotificationSys.LoadBucketMetadata(bgContext(notifyCtx), bucket) // Do not use caller context here
|
||||
return nil
|
||||
}
|
||||
|
||||
// Get metadata for a bucket.
|
||||
// If no metadata exists errConfigNotFound is returned and a new metadata is returned.
|
||||
// Only a shallow copy is returned, so referenced data should not be modified,
|
||||
@@ -602,7 +675,13 @@ func (sys *BucketMetadataSys) GetConfig(ctx context.Context, bucket string) (met
|
||||
return meta, false, err
|
||||
}
|
||||
sys.Lock()
|
||||
sys.metadataMap[bucket] = meta
|
||||
if cur, ok := sys.metadataMap[bucket]; ok && !cur.lastUpdate().Before(meta.lastUpdate()) {
|
||||
// A concurrent publish installed a resident revision at least as new as
|
||||
// this cache-miss load; return it instead of regressing (issue #105).
|
||||
meta = cur
|
||||
} else {
|
||||
sys.metadataMap[bucket] = meta
|
||||
}
|
||||
sys.clearLoadFailure(bucket)
|
||||
sys.Unlock()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user