Conversation
Add a portable Foyer Code impl for SplitRangeCacheKey so recovered cache entries stay valid across architectures without enabling Foyer's serde feature.
| ensure_uri_len_in_range(uri_bytes.len())?; | ||
| let uri_len = u64_from_usize(uri_bytes.len(), "object URI is too long")?; |
There was a problem hiding this comment.
The two checks are redundant. The first one passing implies the second one will pass.
The second is REALLY unnecessary. Is the point to guard us in case we compile quickwit on a 128 bit platform?
| writer | ||
| .write_all(uri_bytes) | ||
| .map_err(foyer::Error::io_error)?; | ||
| let start = u64_from_usize(self.byte_range.start, "range start is too large")?; |
There was a problem hiding this comment.
same as above. When does u64_from_usize fail?
| })?; | ||
| let start = usize_from_u64(read_u64(reader)?, "range start does not fit usize")?; | ||
| let end = usize_from_u64(read_u64(reader)?, "range end does not fit usize")?; | ||
| if start > end { |
There was a problem hiding this comment.
And we have a debug_assert in Self::new too.
| impl SplitRangeCacheKey { | ||
| pub(crate) fn new(object_uri: String, byte_range: Range<usize>) -> Self { | ||
| debug_assert!( | ||
| byte_range.start <= byte_range.end, |
There was a problem hiding this comment.
I would drop the Range assert and consider that such a Range is invalid to begin with (responsability of the caller to not build it, the intent of the API is clear and enforced by the contract.
I would also remove the constructor entirely, considering the struct building is actually clearer.
Remove the constructor, usize/u64 conversion errors, URI length cap, and inverted-range decode check. Encode and decode write little-endian u64 fields and only fail on I/O or non-UTF-8 URIs.
|
Superseded by #6781, which preserves this work in a clean five-commit history rebased onto main. |
2 participants
Summary
Codeimpl forSplitRangeCacheKey(object_uri+ exact half-open byte range).u64length/start/end so recovered entries stay valid across architectures, without enabling Foyer's serde/bincode feature.foyerfrom the cargo-machete ignore list now that the crate is used.Stack
Test plan
cargo test -p quickwit-storage --lib split_range_cache_key_codeccargo clippy -p quickwit-storage --all-features --tests -- -D warningscargo macheteMade with Cursor