From 55f0fbf364ca64ee2016d3fed6b8163936f3155d Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 6 Dec 2025 21:37:25 -0800 Subject: s3: optimize DELETE by skipping lock check for buckets without Object Lock (#7642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This optimization avoids an expensive filer gRPC call for every DELETE operation on buckets that don't have Object Lock enabled. Before this change, enforceObjectLockProtections() would always call getObjectEntry() to fetch object metadata to check for retention/legal hold, even for buckets that never had Object Lock configured. Changes: 1. Add early return in enforceObjectLockProtections() if bucket has no Object Lock config or bucket doesn't exist 2. Add isObjectLockEnabled() helper function to check if a bucket has Object Lock configured 3. Fix validateObjectLockHeaders() to check ObjectLockConfig instead of just versioningEnabled - this ensures object-lock headers are properly rejected on buckets without Object Lock enabled, which aligns with AWS S3 semantics 4. Make bucket creation with Object Lock atomic - set Object Lock config in the same CreateEntry call as bucket creation, preventing race conditions where bucket exists without Object Lock enabled 5. Properly handle Object Lock setup failures during bucket creation - if StoreObjectLockConfigurationInExtended fails, roll back the bucket creation and return an error instead of leaving a bucket without the requested Object Lock configuration This significantly improves DELETE latency for non-Object-Lock buckets, which is the common case (lockCheck time reduced from 1-10ms to ~1µs). --- test/s3/retention/s3_object_lock_headers_test.go | 2 +- test/s3/retention/s3_retention_test.go | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/test/s3/retention/s3_object_lock_headers_test.go b/test/s3/retention/s3_object_lock_headers_test.go index bf7283617..fad9e6fbb 100644 --- a/test/s3/retention/s3_object_lock_headers_test.go +++ b/test/s3/retention/s3_object_lock_headers_test.go @@ -236,7 +236,7 @@ func TestObjectLockHeadersNonVersionedBucket(t *testing.T) { bucketName := getNewBucketName() // Create regular bucket without object lock/versioning - createBucket(t, client, bucketName) + createBucketWithoutObjectLock(t, client, bucketName) defer deleteBucket(t, client, bucketName) key := "test-non-versioned" diff --git a/test/s3/retention/s3_retention_test.go b/test/s3/retention/s3_retention_test.go index 8477a50bf..4abdf6d87 100644 --- a/test/s3/retention/s3_retention_test.go +++ b/test/s3/retention/s3_retention_test.go @@ -69,8 +69,19 @@ func getNewBucketName() string { return fmt.Sprintf("%s%d", defaultConfig.BucketPrefix, timestamp) } -// createBucket creates a new bucket for testing +// createBucket creates a new bucket for testing with Object Lock enabled +// Object Lock is required for retention and legal hold functionality per AWS S3 specification func createBucket(t *testing.T, client *s3.Client, bucketName string) { + _, err := client.CreateBucket(context.TODO(), &s3.CreateBucketInput{ + Bucket: aws.String(bucketName), + ObjectLockEnabledForBucket: aws.Bool(true), + }) + require.NoError(t, err) +} + +// createBucketWithoutObjectLock creates a new bucket without Object Lock enabled +// Use this only for tests that specifically need to verify non-Object-Lock bucket behavior +func createBucketWithoutObjectLock(t *testing.T, client *s3.Client, bucketName string) { _, err := client.CreateBucket(context.TODO(), &s3.CreateBucketInput{ Bucket: aws.String(bucketName), }) -- cgit v1.2.3