Repository navigation
Cap the total upload rate of a mirror - #15
Merged
Merged
Conversation
A mirror can take `max_upload_bytes_per_second`. One shared limiter paces every upload in the context: all file workers, all multipart parts and all profiles together. Before, each boto3 transfer ran at full speed, so one sync opened up to max_workers x 10 streams and filled the uplink. `pos3.request_upload()` starts a background sync of every registered upload within a second and returns at once. Each sync resets the interval timer, so `interval` becomes a backstop for a caller that uploads at its own moments. The final sync on context exit now waits for a background sync that is still running. The stop joins the worker for 60 s only, and a capped sync can run longer than that. Version 0.4.0. Ticket: Positronic-Robotics/internal#1637 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ticket: Positronic-Robotics/internal#1637 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A caller does not need to start an upload before the interval. Uploads run on the registration's interval and at the final sync. The final sync still waits for a background sync that is running. Its test now starts that sync from a one-second interval. Ticket: Positronic-Robotics/internal#1637 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The stop's 60 s join can return while a capped background sync still runs. Ticket: Positronic-Robotics/internal#1637 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
v-positronic
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48788443a7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
An error exit with every upload at sync_on_error=False waited on the sync lock for a sync of nothing. It now returns before the lock, so the caller's exception is not held behind a background upload. Ticket: Positronic-Robotics/internal#1637 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c459e9744
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A NaN rate passed the positive check and failed later in time.sleep. An infinite rate stays valid and paces nothing. Ticket: Positronic-Robotics/internal#1596 #refs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pos3.mirror()andpos3.with_mirror()takemax_upload_bytes_per_second. One limiter paces every upload in the context: all file workers, all multipart parts and all profiles together.max_workersand boto3's per-transfer concurrency do not multiply it.None(the default) uploads at full speed. Downloads are not limited. A rate that is not positive, NaN included, is refused. An infinite rate paces nothing.How the cap works
boto3 calls
upload_file'sCallbackin the thread that sends each part, after each read of the file and before those bytes go out. The callback now blocks in a sharedByteRateLimiter(pos3/rate_limit.py) until the bytes fit under the rate. Each call reserves the next slot on one timeline, so the bytes released since the first call never exceed the rate times the elapsed time. An idle limiter builds no credit.boto3's own
TransferConfig(max_bandwidth=...)limits one transfer manager.client.upload_filemakes a new manager on each call, so that limit applies per file, and a shared manager still cannot span two profiles.Tests
tests/test_rate_limit.py: the limiter on a fake clock (one shared timeline, no idle credit, a rewind is not paced, a zero or NaN rate is refused, an infinite rate never waits), and two end-to-end runs through real boto3 against a local S3 stub. Six files through eight workers at a 1 MiB/s cap take at least the time the cap allows. The test fails when the limiter is made per file. With no cap, the same path finishes at full speed.tests/test_s3.py: the final sync does not overlap a background interval sync that outlives the join timeout. The test fails without the sync lock.uv run pytest: 147 passed.Tracked in Positronic-Robotics/internal#1637.
🤖 Generated with Claude Code