Conversation
* fix: handle idempotency for each operation in journal loader * fix: update unmount operation * fix: appy idempotency for reopen_file,overwrite_file,add_block,complete_file * fix: update unittest * fix: remove unnecessary clone * Update master_fs_test.rs --------- Co-authored-by: thuong <ledacthuong2210>
There was a problem hiding this comment.
Pull request overview
This PR updates the regression test container/docs and adjusts cache-mode read behavior in the unified client to address mtime inconsistencies that can trigger page cache invalidation.
Changes:
- Use PATH lookup (
shutil.which) to detectcargo-llvm-covand avoid false negatives fromcargo llvm-cov --version. - Update the curvine-tests Docker image to include
fio, preinstallllvm-tools-preview, and set a defaultCURVINE_MASTER_HOSTNAME. - In cache-mode reads, override the reader’s reported
mtimeusing the stored UFS mtime.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| curvine-tests/regression/tests/coverage_test.py | Switches cargo-llvm-cov availability detection to PATH lookup. |
| curvine-tests/README.md | Adds Docker guidance for FUSE tests and documents CURVINE_MASTER_HOSTNAME. |
| curvine-tests/Dockerfile | Installs fio, preinstalls llvm-tools-preview, and sets default master hostname env var. |
| curvine-client/src/unified/unified_filesystem.rs | Adjusts cache-mode reader status mtime to align with UFS mtime. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| .await? | ||
| { | ||
| CacheValidity::Valid => { | ||
| blocks.status.mtime = blocks.status.storage_policy.ufs_mtime; |
There was a problem hiding this comment.
In cache-mode, check_cache_validity() returns Valid immediately when mount.info.read_verify_ufs is false, without requiring storage_policy.ufs_mtime to be set. Unconditionally overwriting blocks.status.mtime with storage_policy.ufs_mtime here can therefore set mtime to 0, which will diverge from get_status()/list_status() (they read directly from UFS) and can reintroduce the inconsistency this PR is trying to fix. Consider only overriding mtime when ufs_mtime > 0 (or when read_verify_ufs is enabled / the value has been verified), otherwise keep the existing blocks.status.mtime.
| blocks.status.mtime = blocks.status.storage_policy.ufs_mtime; | |
| let ufs_mtime = blocks.status.storage_policy.ufs_mtime; | |
| if ufs_mtime > 0 { | |
| blocks.status.mtime = ufs_mtime; | |
| } |
fix the inconsistency of mtime returned by list and get_status under cache-mode, which causes page cache invalidation.