Skip to content

test(s3): cover bulk delete failures and contention recovery - #147

Merged
majinghe merged 2 commits into
mainfrom
cxymds/bulk-delete-regression-cases
Nov 12, 2026
Merged

majinghe merged 2 commits into
mainfrom
cxymds/bulk-delete-regression-cases

Conversation

@cxymds

@cxymds cxymds commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Bulk deletes failed with per-object ServiceUnavailable during the main VM campaign, and benchmark preparation/cleanup hid the phase that exhausted its budget. The existing standalone probe was not part of the complete S3 suite and did not cover duplicate entries, version states or recovery after same-key writes.

Add four registered S3 cases with 24 subcases per topology (72 across the full three-topology suite): 1/32/128/500/1000-key batches, duplicate/nonexistent keys, immutable GET load and finite same-key PUT/DELETE contention. Check seed inventory, current-key absence on every configured endpoint, sampled historical bytes, cold-key progress and fresh same-key operations after all writers join. These are black-box checks; exact-ID cleanup and delayed-peer ownership remain covered by the RustFS lock tests.

Loaded cases warm up every worker, wait for post-warmup SDK activity, and require at least one successful load request to overlap the bulk delete's client invocation. Record monotonic intervals, successful overlap counts and worker counts; GET intervals include consuming the response body. Warmup-only activity, operations completed before the delete, client construction and response logging cannot establish overlap. Load sessions reuse their clients while the configured 10-second read timeout fits the remaining budget; existing performance sessions retain their 60-second timeout.

Disable SDK retries for the original request and keep HTTP 503 responses separate from per-object error entries. Persist parsed acknowledgments and sanitized errors before rejecting incomplete responses. Keep the primary and recovery verdicts independent: a recovery-only failure fails the overall case without rewriting primary PASS, while streamed verification timeouts and incomplete reads still trigger same-key recovery. Preserve phase timings, request IDs, cleanup history and NOT_RUN entries. Version/marker/multipart cleanup only operates on fresh receipt-owned buckets, and cleanup retries cannot make a failed case pass. Add a Chinese case guide and Markdown summaries; upload all detailed evidence in the S3 workflow and install its client in an isolated task venv.

Validation:

  • S3 script tests: 54 passed, including 30 bulk-delete tool tests and real HTTP 200 mixed-error / HTTP 503 protocol fixtures. Regression cases cover missing overlap, client reuse, incomplete-response evidence, recovery-only failures, streamed-read exceptions and response-body cleanup.
  • Existing performance lifecycle tests: 43 passed after the shared Session change.
  • Canonical S3 shell regression, Python AST, shell/workflow syntax and diff checks passed.
  • Functional chain tests (20) and sensitive-diagnostics tests (4) passed during the initial implementation; those surfaces were unchanged by this review follow-up.
  • No VM deployment or live RustFS run was performed. Client overlap evidence does not prove server lock overlap. Strict contention failures remain failures and require investigation; these tests do not establish that all 503s share one root cause.

Copilot AI balanced review requested due to automatic review settings November 11, 2026 02:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The loaded regression cases can pass without any GET/PUT activity overlapping bulk deletion, so they do not reliably verify behavior under concurrent load.

5 open issues
What changed in this PR

Adds bulk-delete failure and contention-recovery checks to the complete S3 compatibility suite, covering 24 subcases per topology and 72 across all three topologies.

Changes:

  • Registers four cases covering batch sizes, duplicate keys, GET load, and same-key write contention across three versioning states.
  • Adds failure evidence, recovery checks, receipt-owned cleanup, and JSON/Markdown reports.
  • Adds mutation and HTTP protocol tests, workflow integration, and a Chinese case guide.
File Description
test_s3_bulk_delete_regressions.py Tests failure detection, recovery, cleanup, protocol handling, and runner integration.
scripts/​s3_bulk_delete_regressions.py Implements the bulk-delete campaign and evidence reporting.
rustfs-s3-compat-test.sh Registers four cases and forwards endpoints, credentials, and output settings.
README.md Introduces the regressions and links the case guide.
docs/​s3-bulk-delete-regressions.zh-CN.md Documents case contracts, execution, evidence, and validation limits.
.github/​workflows/​s3-script-tests.yml Adds client dependencies and regression tests to CI.
.github/​workflows/​rustfs-s3-compat-test.yml Installs an isolated client and uploads bulk-delete evidence.

Review effort: Balanced

Comment thread scripts/s3_bulk_delete_regressions.py Resolved
Comment thread scripts/s3_bulk_delete_regressions.py Resolved
Comment thread scripts/s3_bulk_delete_regressions.py Resolved
Comment thread scripts/s3_bulk_delete_regressions.py Resolved
Comment thread scripts/s3_bulk_delete_regressions.py Resolved
@majinghe
majinghe merged commit 4cb4f35 into main Nov 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants