mirror of
https://github.com/pgsty/minio.git
synced 2026-09-14 14:34:03 +03:00
75ba0ce402
The #105 T3 change (PR #156) tried to keep the resident metadata cache monotonic by guarding peer-reload publication on lastUpdate(). But lastUpdate() is the max of per-config timestamps and cannot order whole records: a node caching {policy@20, CORS@10} that receives a newer CORS@15 still has lastUpdate()==20, so the guard rejects the legitimately-newer record and the periodic refresh (same comparator) cannot repair it. A paused reload could also resurrect deleted resident state. Per the maintainer decision, revert the reload publication to its original unconditional (acceptable-until-refresh) behavior: - remove setReloaded and restore the plain Set plus notification/target registry updates in LoadBucketMetadataHandler; - restore refreshBucketsMetadataLoop's own lastUpdate() staleness check and globalEventNotifier.set / globalBucketTargetSys.set publication; - restore the unconditional GetConfig cache-miss publication; - document the known freshness limitation at the reload site (the periodic refresh is best-effort and cannot repair an equal-maximum-timestamp divergence). The T1 lifecycle merge-under-lock (UpdateExpiryLCConfig) and both T2 fixes (DeleteBucket takes metadata.lock before deleting; saveMetadata and loadBucketMetadataParseUnderLock recheck physical bucket existence) are kept fully intact. Tests: - drop the T3 reproductions (overlapping-reload resident-cache test and the peer-reload-preserves-current-targets publication test); - add lockBucketMetadataAcquireHook, a nil-in-production atomic test hook in the shared metadata.lock path, so tests can deterministically observe a caller (notably DeleteBucket, whose lock is taken through its erasureServerPools receiver and is invisible to an injected object layer) reaching the lock; - rewrite the T2 delete-race ghost test to hold metadata.lock MID-SAVE (past saveMetadata's existence recheck) and synchronize on the delete's actual lock attempt via the hook, so it isolates the lock-before-delete fix: removing only DeleteBucket's metadata.lock (recheck kept) now fails it; - rewrite the cancellation test to observe the delete's actual lock attempt, then cancel and await its error while still holding the lock, so a scheduling-delayed delete stopped by the canceled context can no longer pass on a broken tree. Refs #105. Follows #156. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
112 lines
3.9 KiB
Go
112 lines
3.9 KiB
Go
// Copyright 2026 PGSTY contributors.
|
|
// SPDX-License-Identifier: AGPL-3.0-or-later
|
|
|
|
package cmd
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"net/http"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/minio/minio/internal/auth"
|
|
)
|
|
|
|
func TestDeleteBucketMetadataLockCancellation(t *testing.T) {
|
|
defer DetectTestLeak(t)()
|
|
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: testDeleteBucketMetadataLockCancellation})
|
|
}
|
|
|
|
func testDeleteBucketMetadataLockCancellation(obj ObjectLayer, instanceType, bucket string, _ http.Handler, _ auth.Credentials, t *testing.T) {
|
|
_, unlock, err := lockBucketMetadata(t.Context(), obj, bucket)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
release := sync.OnceFunc(unlock)
|
|
defer release()
|
|
|
|
// Observe DeleteBucket's ACTUAL metadata.lock attempt. Set the hook after
|
|
// our own acquisition above so it only trips on the delete.
|
|
delAtLock := make(chan struct{})
|
|
var once sync.Once
|
|
hook := func(b string) {
|
|
if b == bucket {
|
|
once.Do(func() { close(delAtLock) })
|
|
}
|
|
}
|
|
lockBucketMetadataAcquireHook.Store(&hook)
|
|
defer lockBucketMetadataAcquireHook.Store(nil)
|
|
|
|
ctx, cancel := context.WithCancel(t.Context())
|
|
defer cancel()
|
|
done := make(chan error, 1)
|
|
go func() { done <- obj.DeleteBucket(ctx, bucket, DeleteBucketOptions{Force: true, NoLock: true}) }()
|
|
|
|
select {
|
|
case <-delAtLock:
|
|
// Fixed tree: the delete reached metadata.lock and is blocking on the
|
|
// lock we hold. Cancel it and confirm it fails WITHOUT deleting, while we
|
|
// still hold the lock (release stays deferred until after the checks).
|
|
cancel()
|
|
if err := <-done; err == nil {
|
|
t.Errorf("%s: canceled deletion succeeded", instanceType)
|
|
}
|
|
if _, err := obj.GetBucketInfo(t.Context(), bucket, BucketOptions{}); err != nil {
|
|
t.Errorf("%s: bucket disappeared while metadata.lock was held: %v", instanceType, err)
|
|
}
|
|
if _, err := readBucketMetadata(t.Context(), obj, bucket); err != nil {
|
|
t.Errorf("%s: canceled deletion removed metadata: %v", instanceType, err)
|
|
}
|
|
case err := <-done:
|
|
// Broken tree: the delete finished without ever taking metadata.lock,
|
|
// i.e. it did not serialize the destructive operation behind the lock.
|
|
t.Errorf("%s: delete bypassed metadata.lock (err=%v)", instanceType, err)
|
|
}
|
|
}
|
|
|
|
func TestQueuedMetadataUpdateAfterDelete(t *testing.T) {
|
|
defer DetectTestLeak(t)()
|
|
for _, expiry := range []bool{false, true} {
|
|
t.Run(fmt.Sprintf("expiry=%v", expiry), func(t *testing.T) {
|
|
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: func(obj ObjectLayer, instanceType, bucket string, _ http.Handler, _ auth.Credentials, t *testing.T) {
|
|
previous := newObjectLayerFn()
|
|
barrier := &lcMergeBarrier{ObjectLayer: obj, bucket: bucket, mAtLock: make(chan struct{}), mProceed: make(chan struct{})}
|
|
setObjectLayer(barrier)
|
|
defer setObjectLayer(previous)
|
|
release := sync.OnceFunc(func() { close(barrier.mProceed) })
|
|
defer release()
|
|
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
|
|
defer cancel()
|
|
ctx = context.WithValue(ctx, lcMergeWriterKey{}, "M")
|
|
done := make(chan error, 1)
|
|
go func() {
|
|
if expiry {
|
|
done <- globalBucketMetadataSys.UpdateExpiryLCConfig(ctx, bucket, nil, UTCNow())
|
|
return
|
|
}
|
|
_, err := globalBucketMetadataSys.Update(ctx, bucket, bucketTaggingConfig, []byte(`<Tagging><TagSet/></Tagging>`))
|
|
done <- err
|
|
}()
|
|
select {
|
|
case <-barrier.mAtLock:
|
|
case <-ctx.Done():
|
|
t.Fatal("writer did not reach metadata.lock")
|
|
}
|
|
if err := obj.DeleteBucket(t.Context(), bucket, DeleteBucketOptions{Force: true}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
release()
|
|
if err := <-done; !isErrBucketNotFound(err) {
|
|
t.Errorf("%s: queued update should reject a deleted bucket, got %v", instanceType, err)
|
|
}
|
|
if _, err := readBucketMetadata(t.Context(), obj, bucket); !errors.Is(err, errConfigNotFound) && !isErrBucketNotFound(err) {
|
|
t.Errorf("%s: queued update recreated metadata: %v", instanceType, err)
|
|
}
|
|
}})
|
|
})
|
|
}
|
|
}
|