Skip to content

fix(cleanup): use batch-level rate limiting to bypass remove_stream buffered(20)#6

Merged
dpruijt merged 3 commits into
mainfrom
devin/1779914174-fix-delete-rate-limit
May 28, 2026
Merged

fix(cleanup): use batch-level rate limiting to bypass remove_stream buffered(20)#6
dpruijt merged 3 commits into
mainfrom
devin/1779914174-fix-delete-rate-limit

Conversation

@jan-exa

@jan-exa jan-exa commented May 27, 2026

Copy link
Copy Markdown

Summary

The delete_rate_limit parameter was ineffective because the S3 object_store crate's delete_stream implementation uses .try_chunks(1000).buffered(20), running up to 20 concurrent DeleteObjects batch requests. The previous approach — wrapping the input path stream with an IntervalStream ticker — was defeated by this internal buffering.

Fix: Replace per-path stream rate limiting with manual batch-level control:

  1. Collect unreferenced paths into batches of up to 1000
  2. Feed each batch to remove_stream individually (producing exactly 1 DeleteObjects API call per batch)
  3. Sleep for ceil(1000/rate) ms between batches

This gives precise control over the actual S3 API request rate. With delete_rate_limit=3, the delete phase now sends exactly 3 DeleteObjects requests per second (3000 keys/s), well under S3's 3,500 DELETE/s per-prefix limit.

Also renames calculate_durationcalculate_batch_interval with clearer semantics.

Review & Testing Checklist for Human

  • Verify the batch loop correctly handles the final partial batch (< 1000 paths)
  • Confirm calculate_batch_interval math: rate=3 → 334ms, rate=1 → 1000ms, rate=0 → clamped to 1000ms
  • Test against atlas_v3.lance cleanup: run with delete_rate_limit=3 and confirm no 503 errors in the delete phase

Notes

Root cause: object_store-0.12.5/src/aws/mod.rs:272-290delete_stream does .try_chunks(1_000).buffered(20). The buffered(20) pre-fetches chunks from the rate-limited input stream faster than the ticker can gate them, allowing bursts of concurrent API calls that trigger S3 503 throttling.

The non-rate-limited path (when delete_rate_limit is None) is unchanged — it still uses remove_stream directly for maximum throughput.

Link to Devin session: https://app.devin.ai/sessions/bb810ab5769542e0a12b08c2505d2ae1
Requested by: @jan-exa

…uffered(20)

The S3 object_store crate's delete_stream uses .try_chunks(1000).buffered(20),
which runs up to 20 concurrent DeleteObjects batch requests. The previous
per-path IntervalStream rate limiter was defeated by this internal buffering.

Replace with manual batch collection: collect paths into batches of 1000,
send each batch to remove_stream individually (producing exactly 1 API call
per batch), and sleep between batches. This gives precise control over the
actual S3 API request rate.

Co-Authored-By: Jan van der Vegt <jan@exa.ai>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions Bot added the bug Something isn't working label May 27, 2026
devin-ai-integration Bot and others added 2 commits May 27, 2026 21:30
For S3 (batch_size=1000) and Azure (batch_size=256), batch files
together matching the DeleteObjects API limit. For local/other
backends (batch_size=1), rate-limit per file, matching the old
per-path IntervalStream behavior.

Co-Authored-By: Jan van der Vegt <jan@exa.ai>
Co-Authored-By: Jan van der Vegt <jan@exa.ai>
@dpruijt
dpruijt merged commit c92e3fa into main May 28, 2026
9 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants