[shurkus]

Related Issues

Fixes #8741

Summary of Changes

CreateBucket for an existing bucket could time out with 503 ServiceUnavailable while overlapping PutObject requests held the bucket lifecycle lock. The duplicate-bucket check previously ran after bucket mutation admission and exclusive storage locks.

Look up the bucket before mutation admission and return the existing duplicate-bucket response when the lookup succeeds. The common response helper preserves owner success in us-east-1, BucketAlreadyOwnedByYou for owners elsewhere, and BucketAlreadyExists for non-owners. Missing buckets, lookup failures, and missing request info retain the existing admitted task and locked mutation path.

The integration with main at 30c1dffe48b2745e03b9198e8d42df5184dad529 preserves LocationConstraint validation before the fast path. An existing bucket must not turn a mismatched location into an idempotent success. Regression tests cover that ordering, ownership responses, missing request info, and actual creation after a missing-bucket lookup. The real-storage tests use the existing large-stack gating fixture.

Verification

Tested source tree: 9d38ebbda97ff043babd8c984f808828ef7d01a3, including the integration with the main commit above. Local verification used stable Rust 1.99.1 on macOS; no RUST_MIN_STACK environment override was set.

• cargo nextest run -p rustfs --lib -E 'test(execute_create_bucket) | test(create_bucket_exists_response) | test(bucket_create_and_delete_use_admitted_fresh_tasks)': 7 passed, 0 failed. This exercises the real four-disk ECStore while holding the lifecycle lock, the error-response cases, real creation of a missing bucket, the duplicate-response helper, and admitted-task routing.
• Controlled negative checks: disabling the fast path makes the lifecycle-lock regression fail with Elapsed(()); placing the fast path before LocationConstraint validation makes the invalid-region assertion fail. Both temporary changes were removed. The restored source passed the same 7 tests with --profile ci and no retries.
• cargo fmt --all --check and git diff --check: passed.

The original head had only successful CLA checks, with no failed build/test workflow logs. Its merge conflict prevented normal pull-request CI from running. Remote checks for this updated head must still pass. Not run locally: full workspace tests, Clippy, Linux execution, distributed/degraded-storage tests, or a real sustained-write benchmark.

Impact

Duplicate CreateBucket requests avoid the bucket mutation queue and exclusive mutation locks when the existing-bucket lookup succeeds. The lookup still performs normal bucket-read validation; it is not a promise that every storage read is lock-free. Actual bucket creation, site-replication ordering, and durability fences retain their existing behavior. No dependency, configuration, or deployment change is required.

The fast path skips the best-effort heal_bucket(recreate) previously attempted after duplicate creation. Background and explicit healing remain the repair routes. A concurrent delete can race the existence observation, so a successful duplicate response does not guarantee that the bucket remains present afterward. Rollback is to revert this fast-path change; it introduces no persisted format migration.

Additional Notes

Two sequential review passes covered the S3-visible change and its integration with current main. No remaining supported production defect was found.

• Correctness: successful existence observation is required for the fast path; invalid location, missing request info, and missing-bucket creation are covered.
• Simplicity: both duplicate branches share one existing-response policy; no new dependency or infrastructure was added.
• Test coverage: positive and negative checks distinguish both the lock-wait regression and validation-order regression; the missing-bucket case verifies persisted existence after real creation.
• Security: S3 access authorization, location-based policy conditions, and additional Object Lock/versioning privileges execute before this usecase; the fast path does not move those checks.
• Concurrency/durability: the fast path performs no namespace mutation; misses/errors retain mutation admission, lifecycle/metadata/namespace fences, and site-replication ordering. Distributed race/fault injection was not run locally.
• Compatibility: duplicate ownership/region responses and the newly merged LocationConstraint validation are preserved; the skipped best-effort heal is explicitly documented above.
• Performance: the held-lock regression proves that the successful existence case avoids exclusive lifecycle-lock waiting. No throughput or degraded/multi-node latency claim is made.

The existing discussion contained a CLA confirmation and an approval without substantive review comments or inline threads. This also complements the operator's HeadBucket-first provisioning change in rustfs/operator#287.

[github-actions]

CLA requirements are satisfied for this pull request.

[houseme]

Reviewed the original head 56c33e5184ed7993ead6007d105ea574d970a443 and its integration with main at 30c1dffe48b2745e03b9198e8d42df5184dad529, using two sequential passes for this S3-visible change.

The original check state contained only successful CLA checks, with no failed build/test logs. The PR conflicted with the newly merged CreateBucket location-constraint validation. The integration keeps that validation before the existing-bucket fast path, so a mismatched location cannot become an idempotent success.

The review traced the S3 access checks, request ownership, default/non-default region responses, storage lookup failures, bucket mutation admission, lifecycle and namespace locks, site-replication ordering, audit completion, and best-effort healing. No additional supported production defect was found. The fast path intentionally skips the old duplicate-create path's best-effort heal; background/explicit healing remains the recovery route. A concurrent delete can race the existence observation, so this response does not guarantee that a bucket remains present afterward.

Pushed integration head: 9d38ebbda97ff043babd8c984f808828ef7d01a3 (a non-rewriting merge of current main into the original PR history). The final diff against that main revision changes only rustfs/src/app/bucket_usecase.rs.

Validation: 7 focused tests passed in each of the default and CI nextest profiles; formatting and whitespace checks passed. Disabling the fast path reproduced the 2-second lifecycle-lock timeout. Moving the shortcut before region validation reproduced an incorrect successful response for a mismatched location. Both negative controls were removed and the restored final source passed the CI-profile run without retries.

The existing-bucket regression at rustfs/src/app/bucket_usecase.rs:4458 now checks the owner response, non-owner response, invalid-region response, and missing request info while the lifecycle lock remains held. The missing-bucket regression at rustfs/src/app/bucket_usecase.rs:4524 verifies that a real bucket was created rather than treating a lookup miss as success.

Verdict: no remaining confirmed findings in the reviewed diff. Full workspace/Clippy, Linux, distributed/degraded-storage, and sustained-write validation were not run locally; remote CI remains the merge gate. The PR body records the individual review-lens verdicts and the exact local verification.