[storage & runtime] Switch to page-level checksums instead of record-level checksums #2667
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | b97a138 | Jan 08 2026, 09:31 AM |
Deploying monorepo with
|
| Latest commit: |
b97a138
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://de8bdff4.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://append-crc32.monorepo-eu0.pages.dev |
roberto-bayardo
force-pushed
the
append-crc32
branch
from
b7038cd to
8c9d613
Compare
roberto-bayardo
force-pushed
the
append-crc32
branch
9 times, most recently
from
39fe81d to
515a9e0
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
roberto-bayardo
force-pushed
the
append-crc32
branch
3 times, most recently
from
0d7eb5e to
134fe3b
Compare
roberto-bayardo
force-pushed
the
append-crc32
branch
from
134fe3b to
c3d68ac
Compare
roberto-bayardo
force-pushed
the
append-crc32
branch
3 times, most recently
from
449e157 to
e92b376
Compare
roberto-bayardo
force-pushed
the
append-crc32
branch
3 times, most recently
from
2901be0 to
cf1c179
Compare
| self.sync().await?; | ||
|
|
||
| // Acquire both locks to prevent concurrent operations. | ||
| let mut buf_guard = self.buffer.write().await; |
There was a problem hiding this comment.
Resize race condition can silently lose appended data
High Severity
When shrinking, resize() calls sync() at line 831 which releases the buffer write lock after flushing. Between sync() returning and re-acquiring the buffer lock at line 834, another task can call append() and add data to the buffer. The resize then proceeds to overwrite buf_guard.data at lines 878/881 with data read from disk, silently discarding any data appended during that window. The removed implementation held the buffer lock throughout the operation to prevent this race. The warning only mentions concurrent readers, not that concurrent appends can cause data loss.
Additional Locations (1)
patrick-ogrady
left a comment
patrick-ogrady
left a comment
There was a problem hiding this comment.
I think there is still a lot to optimize here (and am not sure runtime is the right place for this fancy checksummed buffer), but the written disk format (page + checksum record) is correct and we can work with this.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #2667 +/- ##
==========================================
+ Coverage 93.13% 93.18% +0.04%
==========================================
Files 368 371 +3
Lines 110795 112959 +2164
==========================================
+ Hits 103193 105262 +2069
- Misses 7602 7697 +95
... and 25 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This PR extends the buffer pool and its associated blob wrappers to perform integrity verification at a page level, allowing removal of the "application-level" integrity checking from storage structures such as journal which were explicitly CRCing every item.
Note that a good deal of complexity (e.g. dual CRCs per page) pertains to being able to rewrite the final (partial) page without affecting durability of any previously commited version.
Resolves: #2607
Resolves: #1219
Resolves: #2728