Conversation
Mesa DescriptionThis pull request introduces a major architectural refactoring of the MesaCloud filesystem, replacing the synchronous inode cache ( Key architectural changes include:
Description generated by Mesa. Update settings |
3482369 to
9f3fd04
Compare
9f3fd04 to
4f222d5
Compare
… in get_or_resolve
Fix all clippy warnings and dead code warnings introduced during the async icache migration (Tasks 1-7). Zero warnings from cargo clippy.
Also includes prerequisite: make AsyncICache::contains synchronous and add contains_resolved method.
…ntry_or_insert_icb
…cking inc_rc now returns Option<u64> and uses a retry loop to handle concurrent InFlight transitions. Callers propagate None as LookupError::InodeNotFound to maintain correct FUSE ref-count semantics.
38d1b32 to
1ed29f8
Compare
There was a problem hiding this comment.
Performed full review of cc7ead7...1ed29f8
Analysis
-
The new architecture relies heavily on each resolver's
needs_resolvesemantics, creating a potential point of failure if implementations have inconsistent behaviors. -
Premature marking of entries as "resolved" in pre-populated attrs or children (e.g., in
readdir) would prevent the resolver from being invoked again, potentially leading to stale or incomplete data. -
The bidirectional bridges in
CompositeFsare now the single source of truth for inode mappings, creating a tight coupling where inconsistencies between ICB eviction and bridge clearing could leak stale pointers. -
Long-running mounts are particularly vulnerable to memory leaks if the ICBs and bridges fall out of sync, requiring careful coordination between these components.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
18 files reviewed | 2 comments | Edit Agent Settings • Read Docs
…esolver Replace unreachable!() with proper error handling in RepoResolver::resolve when stub is None. Now returns LookupError::InodeNotFound instead of panicking.
…ling back to Root
readdir was caching FileAttr::RegularFile { size: 0, blocks: 0 } for
every file child. Since needs_resolve() returns false for files with
any attr set, subsequent lookups via get_or_resolve would return the
stale size=0 instead of calling the resolver for the real file size.
Only cache directory attrs in readdir. File attrs are left as None so
that lookup triggers the resolver.
delegated_forget was propagating forget(inner_root_ino, nlookups) to the inner filesystem. The inner root's rc=1 is an initialization invariant independent of outer FUSE lookups. When nlookups >= 1, the inner root was evicted, making the inner filesystem non-functional on re-access (readdir/lookup would fail with InodeNotFound). Now child-root inodes (those in child_inodes) skip inner forget propagation entirely.
evict_zero_rc_children called AsyncICache::forget directly, bypassing CompositeFs cleanup. The inode_to_slot and bridge inode_map entries for evicted inodes were never removed, causing unbounded memory growth. Now returns Vec<Inode> of evicted inodes so delegated_readdir can clean up the associated CompositeFs state.
attr_backward silently returned the raw inner inode number when a bridge mapping was missing. Now logs a warning so the issue is visible in traces.
- Document ensure_child_ino TOCTOU invariant (safe due to &mut self) - Add MAX_DEPTH=1024 cycle protection to build_repo_path and path_of_inode - Warn on bridge reset in register_repo_slot (leaks inner icache entries)
No description provided.