-
Notifications
You must be signed in to change notification settings - Fork 11
Fix backpressure #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix backpressure #46
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
===========================================
- Coverage 83.77% 55.97% -27.81%
===========================================
Files 8 29 +21
Lines 1923 7048 +5125
===========================================
+ Hits 1611 3945 +2334
- Misses 312 3103 +2791 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes a backpressure issue in the async Zarr storage implementation by introducing a bounded queue for write operations. The main changes include replacing the unbounded Vec<JoinHandle> with a bounded JoinSet protected by a mutex, and implementing a backpressure mechanism that blocks when too many writes are queued.
Key changes:
- Introduced a bounded write queue using
Arc<Mutex<JoinSet>>with configurable limit - Implemented
queue_writefunction that enforces backpressure by waiting for tasks when limit is reached - Updated
finalizeandflushmethods to properly join all pending writes - Bumped
pulpdependency from 0.21.4 to 0.22.2
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/storage/zarr/async_impl.rs | Implements backpressure mechanism with bounded write queue, replaces Vec of JoinHandles with Arc<Mutex>, adds queue_write helper function |
| Cargo.toml | Updates pulp dependency to version 0.22.2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fa0d34f to
2f503eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2f503eb to
126348b
Compare
fixes pymc-devs/nutpie#262