Replace bincode by postcard - #2258
Conversation
nymius
changed the title
4ba69d3 to
007c1d2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2258 +/- ##
==========================================
+ Coverage 78.36% 78.67% +0.30%
==========================================
Files 30 30
Lines 5945 6002 +57
Branches 281 286 +5
==========================================
+ Hits 4659 4722 +63
+ Misses 1210 1206 -4
+ Partials 76 74 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
82c6cae to
28a4592
Compare
`bincode` is no longer maintained. `postcard` is the closest maintained project with >52M downloads on crates.io, regular releases, and activity on its repo. BREAKING CHANGE: - Magic Bytes should be changed to avoid accidentaly modifying old file store blobs. - The internal encoding has changed as a result of using postcard. Old file stores won't be recoverable using the latest file_store version. - As this is a development environment store, we don't provide migration utilities. - `StoreError::Bincode` has been renamed to `StoreError::Decode`, and now contains `postcard::Error`s - From now on, trailing bytes after decoding are rejected. - `append` always tries to attach changesets to the latest valid end of the file.
28a4592 to
1bde283
Compare
|
Breaking compatibility with older bincode-encoded stores without supplying a conversion tool leaves users stranded. Regardless of whether file_store is intended for dev environments, dropping support without providing a migration path or prior notice inevitably leads to silent data loss. |
|
@chukwudiikeh There is no data loss. Everything to persist in |
evanlinjin
left a comment
evanlinjin
left a comment
There was a problem hiding this comment.
I haven't reviewed the tests yet.
EntryIter changes are well written and very thorough - handling all situations elegantly and returning useful errors.
I disagree with the Store::append changes as a corrupted tail becomes unrecoverable.
| let mut payload = Vec::new(); | ||
| // Reserve exactly `len` bytes up front. Fail fast on a corrupt, oversized length prefix. | ||
| // Avoids unnecessary reads and allocations. | ||
| let alloc_failed = usize::try_from(len) | ||
| .map_err(|_| ()) | ||
| .and_then(|len| payload.try_reserve_exact(len).map_err(|_| ())) | ||
| .is_err(); |
There was a problem hiding this comment.
Is there no better way to write this? 😅
There was a problem hiding this comment.
Could we use postcard::from_io instead?
| // Always write at the current end of the file. This handle's cursor may be stale if | ||
| // another handle has appended since we last read, and writing at a stale offset would | ||
| // overwrite those changesets. |
There was a problem hiding this comment.
I don't think this is the behavior we want. If we failed to read the last entry due to DeserializeUnexpectedRead, then further writes will also be corrupted and unreadable due to the len_prefixes being unaligned.
The version on |
This crate is explicitly for testing only, per the README. |
4 participants
bincodeis no longer maintained.postcardis the closest maintained project with >52M downloads on crates.io, regular releases, and activity on its repo.BREAKING CHANGE:
StoreError::Bincodehas been renamed toStoreError::Decode, and now containspostcard::Errorsappendalways tries to attach changesets to the latest valid end of the file.These changes were LLM assisted.
Changelog notice
Changed
bincode(unmaintained) withpostcardfor on-disk (de)serialization.version; callers should bump their magic bytes so old files fail fast with
StoreError::InvalidMagicBytesinstead of a decode error.StoreError::Bincode(bincode::ErrorKind)is replaced byStoreError::Decode(postcard::Error).postcardvarint (1-10 bytes). Covered by the same on-disk format change andmagic-byte-bump guidance above.
Fixed
Store::appendnow seeks to the end of the file before writing, so appending through astale handle no longer overwrites changesets written via another handle. A failed append
also truncates the partial frame, leaving the file unchanged.
Checklists
All Submissions:
New Features:
Bugfixes: