[CHORE] (cu29_unifiedlog) improve error handling#704
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ddadc65e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn flush(&mut self) -> CuResult<usize> { | ||
| todo!() | ||
| // Flushing is handled at the slab level for mmap-backed storage. | ||
| Ok(self.offset) |
There was a problem hiding this comment.
Make flush actually persist data or return an error
The SectionStorage::flush contract says it “Flushes the section to the underlying storage,” but this implementation now just returns Ok(self.offset) without calling any mmap flush APIs. If a caller relies on get_storage_mut().flush() to force durability (e.g., before a crash or shutdown), they’ll get a false success and the data may still be only in memory. Either call MmapMut::flush[_async]_range here or return an error to keep the unsupported behavior explicit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is super iffy, the flush is indeed taken care upstream IIRC. returning Ok(self.offset) is ok for the mmap version
| fn flush(&mut self) -> CuResult<usize> { | ||
| todo!() | ||
| // Flushing is handled at the slab level for mmap-backed storage. | ||
| Ok(self.offset) |
There was a problem hiding this comment.
this is super iffy, the flush is indeed taken care upstream IIRC. returning Ok(self.offset) is ok for the mmap version
gbin
left a comment
There was a problem hiding this comment.
we will need to test the changes a little bit but it looks good.
* (cu29_unifiedlog) improve error handling * simplify
* Iceorix2 ported as a bridge. * ronfmt lint * simplify * [CHORE] Removed unused `cu29::prelude::*` imports causing warnings in generated workspace templates: (#711) - `templates/cu_full/components/bridges/cu_example_shared_bridge/src/messages.rs` - `templates/cu_full/apps/cu_example_app/src/messages.rs` You can regenerate the workspace and rerun `cargo build` to confirm the warnings are gone. * Resolve cu29_export merge conflict * (cu29_helpers) improve error handling (#703) * (cu29_value) improve error handling (#702) * (cu29_log) improve error handling (#705) * (cu29_soa_derive) improve error handling (#706) * Upgraded the glam dependency to 0.31 in the two crates that declare it, and verified both dependent crates still check cleanly. (#713) Changes: - Updated glam version in `components/libs/cu_transform/Cargo.toml`. - Updated glam version in `components/payloads/cu_spatial_payloads/Cargo.toml`. Checks run: - `cargo +stable check -p cu-transform -p cu-spatial-payloads` Next steps (optional): 1. Run `cargo +stable check --workspace` if you want broader coverage. 2. Run `cargo +stable nextest run -p cu-transform -p cu-spatial-payloads` if you want tests as well. * Updated nix dependency versions to `0.31` in the two crates that directly use it and ran cargo check on those crates. (#714) Changes: - `components/sources/cu_v4l/Cargo.toml` - `examples/cu_msp_bridge_loopback/Cargo.toml` Checks run: - `cargo +stable check -p cu-v4l -p cu-msp-bridge-loopback` If you want, I can also run the broader CI-aligned checks (`just std-ci`) or `cargo +stable nextest run --workspace --all-targets`. * [CHORE] update cudarc deps to 0.19 (#715) * (cu29_log_derive) improve error handling (#707) * [CHORE] (cu29_runtime) improve error handling (#708) * (cu29_runtime) improve error handling * cleared the async processing state and recorded the error when the output mutex is poisoned in CuAsyncTask::process * fmt * [CHORE] (cu29_unifiedlog) improve error handling (#704) * (cu29_unifiedlog) improve error handling * simplify --------- Co-authored-by: Yang Zhou <yangzhou.info@gmail.com>
No description provided.