-
Notifications
You must be signed in to change notification settings - Fork 170
[runtime/buffer] avoid write-locking the Append buffer during blob writes #2679
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
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | 12d03ae | Jan 05 2026, 07:16 PM |
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 PR improves concurrency in the buffer pool by introducing a separate I/O lock (blob_io) for the underlying blob. The key change allows readers to access the buffer pool cache while writes to the blob are in progress, rather than blocking all reads during blob I/O operations.
Key changes:
- Introduces
AsyncMutexwrapper for async mutual exclusion primitives - Adds
blob_ioRwLock to coordinate reads/writes to the underlying blob separately from the buffer pool lock - Implements
try_read_cachedfast path to check cache without acquiring I/O locks
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| runtime/src/utils/mod.rs | Adds AsyncMutex wrapper around async_lock::Mutex for consistency with existing synchronization primitives |
| runtime/src/utils/buffer/pool.rs | Implements try_read_cached method to enable lock-free cache lookups |
| runtime/src/utils/buffer/append.rs | Refactors blob I/O to use separate blob_io lock, allowing concurrent cache reads during blob writes |
1d611a2 to
a3e53db
Compare
Deploying monorepo with
|
| Latest commit: |
12d03ae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://66afdd7c.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://avoid-write-lock-during-blob.monorepo-eu0.pages.dev |
286f4ce to
caffe14
Compare
caffe14 to
b4fdfe9
Compare
runtime/src/utils/buffer/append.rs
Outdated
| // Acquire buffer lock first. | ||
| // NOTE: We MUST acquire the buffer lock before the blob lock to avoid deadlocks with | ||
| // `append`, which acquires buffer then blob (via `flush_internal`). | ||
| let mut guard = self.buffer.write().await; |
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.
| let mut guard = self.buffer.write().await; | |
| let mut buf_guard = self.buffer.write().await; |
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.
done
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2679 +/- ##
==========================================
+ Coverage 92.63% 92.83% +0.20%
==========================================
Files 357 361 +4
Lines 103102 106277 +3175
==========================================
+ Hits 95505 98659 +3154
- Misses 7597 7618 +21
... and 103 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
The append write buffer is currently write-locked while we write data to underlying blob, which prevents any concurrent reads or buffered writes. This PR introduces a separate IO lock for the underlying blob so that buffer reads and appends can be performed concurrently with blob writing.
Impact on existing benchmarks are neutral, however we don't have any benchmarks that perform concurrent reads & writes, which is where this change would shine.
Resolves: #2673