From b3a6d5dbf85e0251837aa81db3f3c6c8b01dde6d Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 2 Sep 2026 01:16:10 +0800 Subject: [PATCH] fix: bound bucket metadata migration locking Use a short dedicated migration lock timeout and fall back to the already loaded legacy or target configuration in memory when persistence is contended. Never fall back to an unlocked metadata save, and keep peer fan-out contexts free of stale lock markers.\n\nRefs: #102 Signed-off-by: Feng Ruohang --- cmd/bucket-metadata-sys.go | 9 +++++++-- cmd/bucket-metadata.go | 28 +++++++++++++++++++++------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cmd/bucket-metadata-sys.go b/cmd/bucket-metadata-sys.go index 7432eb038..ab2c4ea3d 100644 --- a/cmd/bucket-metadata-sys.go +++ b/cmd/bucket-metadata-sys.go @@ -123,6 +123,7 @@ func (sys *BucketMetadataSys) updateAndParse(ctx context.Context, bucket string, if isMinioMetaBucketName(bucket) { return updatedAt, errInvalidArgument } + notifyCtx := ctx ctx, unlock, err := lockBucketMetadata(ctx, objAPI, bucket) if err != nil { return updatedAt, err @@ -195,7 +196,7 @@ func (sys *BucketMetadataSys) updateAndParse(ctx context.Context, bucket string, if err != nil { return updatedAt, err } - globalNotificationSys.LoadBucketMetadata(bgContext(ctx), bucket) // Do not use caller context here + globalNotificationSys.LoadBucketMetadata(bgContext(notifyCtx), bucket) // Do not use caller context here return updatedAt, nil } @@ -228,8 +229,12 @@ func (sys *BucketMetadataSys) saveMetadata(ctx context.Context, objAPI ObjectLay } func lockBucketMetadata(ctx context.Context, objectAPI ObjectLayer, bucket string) (context.Context, func(), error) { + return lockBucketMetadataWithTimeout(ctx, objectAPI, bucket, globalOperationTimeout) +} + +func lockBucketMetadataWithTimeout(ctx context.Context, objectAPI ObjectLayer, bucket string, timeout *dynamicTimeout) (context.Context, func(), error) { lock := objectAPI.NewNSLock(minioMetaBucket, pathJoin(bucketMetaPrefix, bucket, "metadata.lock")) - lkctx, err := lock.GetLock(ctx, globalOperationTimeout) + lkctx, err := lock.GetLock(ctx, timeout) if err != nil { return nil, nil, err } diff --git a/cmd/bucket-metadata.go b/cmd/bucket-metadata.go index 53099cf62..9a3024357 100644 --- a/cmd/bucket-metadata.go +++ b/cmd/bucket-metadata.go @@ -250,10 +250,13 @@ func loadBucketMetadataParse(ctx context.Context, objectAPI ObjectLayer, bucket if len(configs) > 0 { if !bucketMetadataLockHeld(ctx, bucket) { - return loadBucketMetadataParseUnderLock(ctx, objectAPI, bucket, parse) - } - // Old bucket without bucket metadata. Hence we migrate existing settings. - if err = b.convertLegacyConfigs(ctx, objectAPI, configs); err != nil { + migrated, lockErr := loadBucketMetadataParseUnderLock(ctx, objectAPI, bucket, parse) + if lockErr == nil { + return migrated, nil + } + internalLogOnceIf(ctx, fmt.Errorf("unable to persist bucket metadata migration for %s, using the legacy configuration in memory: %w", bucket, lockErr), "bucket-metadata-migration-lock-"+bucket) + b.applyLegacyConfigs(configs) + } else if err = b.convertLegacyConfigs(ctx, objectAPI, configs); err != nil { return b, err } } @@ -274,7 +277,12 @@ func loadBucketMetadataParse(ctx context.Context, objectAPI ObjectLayer, bucket // migrate unencrypted remote targets if len(b.BucketTargetsConfigJSON) != 0 && GlobalKMS != nil && len(b.BucketTargetsConfigMetaJSON) == 0 && !bucketMetadataLockHeld(ctx, bucket) { - return loadBucketMetadataParseUnderLock(ctx, objectAPI, bucket, parse) + migrated, lockErr := loadBucketMetadataParseUnderLock(ctx, objectAPI, bucket, parse) + if lockErr == nil { + return migrated, nil + } + internalLogOnceIf(ctx, fmt.Errorf("unable to persist encrypted bucket target metadata for %s, using the existing configuration in memory: %w", bucket, lockErr), "bucket-metadata-migration-lock-"+bucket) + return b, nil } if err = b.migrateTargetConfig(ctx, objectAPI); err != nil { return b, err @@ -284,7 +292,7 @@ func loadBucketMetadataParse(ctx context.Context, objectAPI ObjectLayer, bucket } func loadBucketMetadataParseUnderLock(ctx context.Context, objectAPI ObjectLayer, bucket string, parse bool) (BucketMetadata, error) { - ctx, unlock, err := lockBucketMetadata(ctx, objectAPI, bucket) + ctx, unlock, err := lockBucketMetadataWithTimeout(ctx, objectAPI, bucket, bucketMetadataMigrationTimeout) if err != nil { return newBucketMetadata(bucket), err } @@ -292,6 +300,8 @@ func loadBucketMetadataParseUnderLock(ctx context.Context, objectAPI ObjectLayer return loadBucketMetadataParse(ctx, objectAPI, bucket, parse) } +var bucketMetadataMigrationTimeout = newDynamicTimeout(5*time.Second, time.Second) + // loadBucketMetadata loads and migrates to bucket metadata. func loadBucketMetadata(ctx context.Context, objectAPI ObjectLayer, bucket string) (BucketMetadata, error) { return loadBucketMetadataParse(ctx, objectAPI, bucket, true) @@ -455,7 +465,7 @@ func (b *BucketMetadata) getAllLegacyConfigs(ctx context.Context, objectAPI Obje return configs, nil } -func (b *BucketMetadata) convertLegacyConfigs(ctx context.Context, objectAPI ObjectLayer, configs map[string][]byte) error { +func (b *BucketMetadata) applyLegacyConfigs(configs map[string][]byte) { for legacyFile, configData := range configs { switch legacyFile { case legacyBucketObjectLockEnabledConfigFile: @@ -487,6 +497,10 @@ func (b *BucketMetadata) convertLegacyConfigs(ctx context.Context, objectAPI Obj } } b.defaultTimestamps() +} + +func (b *BucketMetadata) convertLegacyConfigs(ctx context.Context, objectAPI ObjectLayer, configs map[string][]byte) error { + b.applyLegacyConfigs(configs) if err := b.Save(ctx, objectAPI); err != nil { return err