Repository navigation
fix(nfs): spill AppleDouble companion bodies to disk and expand workspace capacity to 65,536 - #636
Open
jiangkoumo wants to merge 20 commits into
Open
jiangkoumo wants to merge 20 commits into
jiangkoumo wants to merge 20 commits into
Conversation
…budget (lp73) - Keep AppleDouble companion index entries in export memory (HELD = 65,536, FILE_BYTES = 1 MiB) and bound resident payload bytes at RESIDENT_BYTES (32 MiB); when a write or resize crosses the resident budget, spill least recently touched resident bodies to the export's scratch directory (<mounts>/.appledouble/<namespace>/<serial>) and serve spilled reads/writes via pread/pwrite outside the companion lock, issuing 0 engine or SQLite owner jobs. - Drop the paired ._<name> companion (and unlink its spilled file) when a main file is removed, silly-renamed, or replaced by RENAME; when HELD is reached, reclaim orphan companions whose main file is absent from Table::by_name first and return NFS3ERR_ACCES only if all 65,536 slots belong to live files. - Clean any leftover companion spill directory on Attach/export start and remove the directory when the export stops. LayerFS production LOC: 143682 -> 144159 (delta +477) Production LOC: 143,682 -> 144,159 (delta +477) Scope: core 39,491 -> 39,491 (0), engine 66,277 -> 66,754 (+477), control 33,969 -> 33,969 (0), api 3,945 -> 3,945 (0); reference absent (retired R9c; last counted 65,417 at tree 498dd19) Method: python3 -B tools/production_loc.py (parent 716197c -> committed tree)
…n out under another call (lp73) - Close the window the review found: spill_self switched a companion to the file it wrote out without comparing the `used` tick, so a resident body another connection wrote to during the off-lock disk write could be replaced by the older snapshot. Both spill paths now go through spill_one -> Companions::adopt_spilled, which switches only when spill_applies(used, file.used, resident) holds and otherwise unlinks the file just written so the newer body stays and the caller decides again. - Fold the redundant state machines: spill_for and spill_self become one spill_one(serial, keep); companion_write and companion_resize share Edit (Write/Size), Room (Resident/Spilled/Grow), edit_resident (in the lock, within the resident budget), edit_companion (one off-lock disk pass, deciding again when another call changed the body) and record_edit; companion_resize is removed and SETATTR resizes through edit_companion. Unlink/removal boilerplate is shared by unlink_paths, companion_take and Kernel::holdings. - Remove ExportConfig::with_companion_spill_dir (no caller) and Released::companions_spilled (read only by tests), counting the spill directory in the tests instead. ExportConfig keeps companion_resident_bytes and companions_held, exactly as lookups_held already does, because both bounds have to be reachable cheaply from tests; deleting them needs a doc(hidden) setter and saves about four lines net. - Set the modes instead of relying on the daemon's umask: the scratch directory is created 0700 and each spilled file 0600. The daemon's startup reclaim now removes a leftover <mounts>/.appledouble from a daemon that ended without unmounting, before it serves any export, and touches nothing else under the mount home. Tests: tests/write.rs asserts the scratch directory and spilled-file mode bits, and that a spill replaces a companion body only when no call changed it meanwhile; tests/daemon.rs asserts the startup reclaim clears the leftover spill tree and leaves its siblings alone. No test is deleted. LayerFS production LOC: 143682 -> 144039 (delta +357) Production LOC: 144159 -> 144039 (delta -120) for this commit (parent 24fa155); the +357 line above is the branch against its base 716197c, as the task asked. Scope at the committed tree: core 39,491, engine 66,634, control 33,969, api 3,945 (this commit: apple_double.rs 715 -> 614, kernel.rs 796 -> 783, server.rs 300 -> 296, control/export.rs 841 -> 839); reference absent (retired R9c; last counted 65,417 at tree 498dd19) Method: python3 -B tools/production_loc.py --files at the parent 24fa155 and the committed tree; first-party product source under core/engine/control/api, tests, docs and tools excluded.
…wo spill races (lp73) Every spilled companion body was held open in `Body::Spilled(Arc<File>)`, so an export with more spilled companions than the process's own `RLIMIT_NOFILE` answered WRITE with `NFS3ERR_IO` (`openat EMFILE`): with Linux's default 1024, `write::apple_double_companions_spill_preserve_all_bytes_reclaim_orphans_and_clean_up` failed after about a thousand companions. `Body::Spilled` now only marks a body as on disk, its path derived from the serial, and READ, WRITE and SETATTR open it for their own `pread`/`pwrite`/`set_len` and close it again, so descriptors follow concurrent operations instead of spilled bodies. No descriptor limit is raised to pass a test; nothing about the file format or directory layout changes. Two multi-connection windows are closed with it: - `edit_resident` records a body's bytes, length, recency tick and time under the same lock, so a call can no longer see new bytes with an old length (a READ could slice past the bytes and panic) or a stale `used` tick (a spill could take the older snapshot for the current body). `Room::Resident` carries the stat afterwards, so `record_edit` now only serves the disk half. READ clamps its slice to the bytes it holds instead of panicking. - A spill writes `<serial>.<n>.tmp` (`create_new`) and adoption renames it to `<serial>` under the lock, so two calls spilling the same companion never truncate each other's file, and the call whose file is not adopted unlinks only its own temporary file. New test `write::spilled_companions_outnumber_the_open_file_limit` lowers the soft `RLIMIT_NOFILE` (restored by a `Drop` guard, `panic` included) to at most 512 and creates four times as many spilled companions, then reads all of them byte for byte and writes and truncates one: `NFS3ERR_IO` before this change, all green after. `libc` becomes a dev-dependency of `layerfs-nfs` for `getrlimit`/ `setrlimit`; it is already in the workspace at 0.2.189, so the lock gains one dependency edge and no package. LayerFS production LOC: 144039 -> 144079 (delta +40) Scope and method: `python3 -B tools/production_loc.py` (combined total) on the first parent and the committed tree; only `engine/crates/layerfs-nfs/src/apple_double.rs` changed in production scope (614 -> 654 production lines, physical 723 -> 785, under the 999-line ceiling). Tests, the Cargo manifest/lock and `docs/architecture/engine/kernel-routes/nfs.md` are outside that count.
…B-FIX) Keep companion name lookup, capacity reclamation and serial insertion under one lock. Preserve previously observed hard-link names in by_name and remove every key for an inode when its synthetic custody is retired. Keep HELD at 65,536. Refuse an exhausted companion index with NFS3ERR_NOSPC, count each refusal and log export/namespace and capacity at most once per minute. Add concurrent CREATE and hard-link reclamation regressions; update nfs.md. Validation: layerfs-nfs fast tests 32 passed and slow test 1 passed; write under ulimit -n 1024: 13 passed; named_mount: 1 passed; Clippy all targets for layerfs-nfs/layerfs-daemon with -D warnings; rustfmt; product boundary guard and its 37 self-tests; release CLI/daemon build. Both new tests failed with their respective fixes temporarily reverted. No kernel mount testing, push, PR or issue. LayerFS production LOC: 144079 -> 144094 (delta +15) Scope: first-party core/engine/control/API production source and runtime SQL; tests, docs, tools and manifests excluded. Engine 66674 -> 66689; other production scopes unchanged. Relative to 716197c (143682): delta +412. Method: unchanged tools/production_loc.py at ca153a7, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; python3 -B tools/production_loc.py --root <snapshot>, identical source scope from git archives of first parent ca153a7 and final staged tree 86c9888. The committed tree must match it. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
…s (lp73) A `._name` companion the export creates while it knows its main file now belongs to that inode rather than to one name: every hard-link alias reaches it, RENAME of the main file keeps it reachable, unlink or a replacing RENAME only unbinds the paired `._` name, and the inode's last name (or its silly rename) drops it. Its FinderInfo and extended attributes are kept under that inode's serial, once per inode and not once per name. After every resident write, resize or rename of such a companion, the new codec (`apple_double_codec`) decides whether the body is a whole image in the client's own layout: an AppleDouble v2 header whose FinderInfo entry runs into the ATTR table, values packed in entry order, the client's empty resource fork or none, the whole length a number of 4 KiB units. It decodes the image and requires that encoding the decoded form gives back exactly the same bytes, so a synthesized READ is byte for byte what the client wrote. Each attribute entry keeps the alignment padding the client left after its name (it does not clear what a shorter name replaced), and nothing else. A whole image takes no slot of the 65,536-entry index bound, which is what keeps a tree's attribute copies from exhausting it; it is kept decoded while the decoded budget takes it (512 MiB in all, 128 KiB each), and otherwise keeps its bytes as written. Every state of the client's write sequence is served as written: a header, a delete and rebuild, an edit that leaves the layout incomplete, a truncation or a half-written image stay the client's own bytes, and only a whole image is replaced by its decoded form. A resource fork, another layout, a companion whose main file the export never saw, or an edit to a spilled body keeps the Ephemeral-AI-Lab#636 stored-as-written path and its slot, with the bound, NOSPC, the rate-limited log and orphan reclamation unchanged. No attribute is dropped: provenance, quarantine, FinderInfo and user attributes are all kept. Persistence is unchanged: companions remain export-local and never reach the Store or a Commit. Released reports slotted/decoded companions, decoded bytes, and releasing an export with companions logs them. The spill-file helpers move unchanged into `apple_double_spill` to keep `apple_double.rs` under the file ceiling. Tests: new `tests/apple_double_codec.rs` (hand-built images, 0 B to 100 KiB values, many attributes, FinderInfo, resource forks, kept alignment padding, truncated, altered and foreign bytes) and `tests/apple_double_decode.rs` (the client's write sequence, whole images taking no slot, hard links, renames and replacements, the silly rename, the slot bound and the stored-as-written fallback). No test deleted. Production LOC: 156721 -> 157237 (delta +516); scope core/engine/control/api first-party product source; counted with tools/production_loc.py on git-archive snapshots of the parent commit 640a304 and the staged tree; tests, docs and tools excluded. engine 73491 -> 74007. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…e (lp73) Three review findings on fac9124: - `adopt_spilled` takes a decoded body too: an edit that leaves a whole image not whole (a mid-image write, a header rewrite) now spills the encoded image and applies the write to that file, instead of answering NFS3ERR_IO after three rounds. A decoded body's resident need is its edited length rather than its image length, so truncating it needs no room for the image and evicts no other body. - `keep_decoded` and `classify` hold `file.inode.is_some()`: a companion whose main file the export does not know is never a whole image. It keeps the client's bytes as written, takes a `HELD` slot and can be reclaimed as an orphan, including after a RENAME onto a name whose main file is unknown (a decoded body is written back out as the client's bytes there). - `Kernel::renamed` takes the source name as well and removes only `(from, name)` from `Table::by_name`, so an inode's other names, its hard links, keep reaching its companion after another name is renamed. Tests: three new cases in `tests/apple_double_decode.rs`: a decoded companion edited under a 1 KiB resident budget (mid-image write, truncation, rewrite, and an unrelated resident body that must not be evicted for it); a companion with no main file not decoded and reclaimed by the index bound; hard links a and b, RENAME a -> c, `._b` still LOOKUPs before and after c goes. Reverting each fix alone makes its test fail (NFS3ERR_IO; decoded=2/slotted=0; NFS3ERR_NOENT). No tests deleted. Docs: nfs.md records that a RENAME leaves the inode's other names reaching the companion and that a body which stops being decodable is written back out. Production LOC: 157237 -> 157253 (delta +16); scope core/crates, engine/crates, control/crates and api/crates product sources (src/ and shipped SQL) counted by tools/production_loc.py on the parent tree fac9124 and this committed tree; test, doc and tooling changes are excluded.
…p73) Docs only: rewraps the paragraph the previous commit extended, so its lines keep the file's ~80 column prose wrapping. No wording or behaviour change. Production LOC: 157253 -> 157253 (delta 0); docs only, counted with tools/production_loc.py on the parent tree and this tree.
Collaborator
Author
|
Pushed three more commits on top of the spill work (head
Results on a native macOS NFSv3 mount: 70,000-file Tests: macOS |
…appledouble-spill
The AppleDouble section describes the companion machinery but not its remaining cost. State the known limitation: a create still pays extra `._` companion round-trips (about 5-6 RPCs per file) because macOS auto-tags new files with com.apple.provenance, and no server-side fix keeps xattr semantics. Production LOC: 160911 -> 160911 (delta 0) Scope: first-party product source in the four core, ten engine, eight control and four api crates; tests, docs, tools and third-party code excluded. Method: python3 tools/production_loc.py (files unchanged by this commit; a docs-only change keeps the combined total and its delta at 0).
This was referenced Nov 13, 2026
Contributor
|
Review (static, at c0c83e3). Request changes. The spill design holds up: Blocking
To fix
Rules
Conflicts with #644 in |
Production LOC: 160911 -> 160921 (delta +10) Scope: first-party core/engine/control/api production source and shipped runtime SQL; tests, docs, tools and manifests excluded. Method: python3 -B tools/production_loc.py (SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6) on git archives of the first parent 3ed05e4 and the merged tree 2a7dc2b98; the delta is the merged upstream's own (control +10). Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
`Kernel::drop_unlinked` retired an inode's names by scanning the whole `Table::by_name` index (`retain(|_, held| *held != serial)`) under the table lock, and `entry_taken` never dropped a serial's previous name, so a name a lookup replaced stayed until the inode's last name went. `rm -rf` of N files therefore cost N x |by_name| -- up to the index's 4M names (LP636 review, blocking 1). - `Table::aliases` (serial -> names) remembers a hard-linked inode's names beyond `Table::names`, so `unbind_serial` drops exactly one serial's keys and nothing scans the index. - `entry_taken` retires the serial's old key at once when the engine reports one name (nlink 1): that inode cannot be reached under a second name. A hard-linked inode (nlink > 1) still keeps its aliases, which is why orphan reclamation keeps a companion when a second name of its main file was seen. - `renamed` and the failed resolution of a collected serial drop their key through the same helpers; `finish_unlink` unbinds through `Table::unbind_any` so the alias record goes with the name. The release counters and `Released` moved to `kernel/released.rs` (kernel.rs would otherwise pass the 999-line ceiling); the move changes no behavior. Tests: existing `layerfs-nfs` write/decode suites cover hard-link aliases, rename and reclamation (13 write + 9 decode cases, all pass). Production LOC: 160921 -> 160984 (delta +63) Scope: first-party core/engine/control/api production source and shipped runtime SQL; tests, docs, tools and manifests excluded. Method: python3 -B tools/production_loc.py (SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6) on git archives of the first parent 3ab79db (160921) and this committed tree 900a8dc (160984). Engine 75363 -> 75426; other scopes unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Four findings from the LP636 review of the AppleDouble companions, all in the export's own index. - Blocking 2. At the `HELD` bound every `._` CREATE copied all of `Table::by_name` into two HashSets (keys and values) -- a namespace-sized allocation on each refused create, taken under the table lock. Reclamation now walks the companion index instead: `Kernel::orphans` takes the table lock and, inside it, the companion lock (never the other order), asks each slotted companion whether its main inode still holds a name (O(1) through `Table::names`/`Table::aliases`, which the previous commit added) or, for one whose main file was never seen, whether its main name is still in `by_name`, and returns only the orphans. Work is bounded by the index, not the namespace. - A spilled whole image survived a RENAME onto a name whose main file is unknown as `inode=None, whole=true`, taking no slot and bypassing `HELD`. `companion_rename` classifies every body again under its destination, and `classify` keeps a body on disk's class only while the inode is known: an unknown destination makes it slotted. (A spilled body cannot be decoded there; that would read its file under the companion lock.) - `edit_companion` re-decided up to three optimistic rounds and then answered `NFS3ERR_IO` with no failure behind it, and `spill_for` looped with no bound. Each companion now carries its own edit gate (`Companion::editing`), held for the whole edit -- resident or disk -- so no other edit of that companion interleaves. `spill_for` bounds its rounds by the index and the caller writes out its own body (through `spill_gated`, which the gate makes authoritative) when it still does not fit, so no edit is refused for contention. - The spilled-edit length race (`ca153a7f6` fixed it for resident bodies): a concurrent truncate and WRITE left `len` at 10 with a 0-byte file, and every read then failed `NFS3ERR_IO`. With the gate, each edit's length is recorded with the bytes its own edit left, so the pair can only serialize. The body path (READ, WRITE, SETATTR, spill) moved to `apple_double/edit.rs`; `apple_double.rs` stays under the 999-line ceiling. New tests: - `apple_double_decode::a_spilled_body_renamed_off_its_inode_takes_its_slot` (spilled whole image renamed onto an unknown main must take its slot). - `write::a_spilled_companion_keeps_its_length_and_bytes_together` (a truncate and a write of one spilled companion, 64 rounds: the reported length always matches the bytes served). Docs: nfs.md records the reverse index, the per-companion edit gate, the bounded spill rounds and the RENAME reclassification. Tests: `layerfs-nfs` write 14 passed, decode 10 passed (nextest). Clippy `-p layerfs-nfs -p layerfs-daemon --all-targets -- -D warnings` clean; rustfmt clean for the changed files (the two upstream files listed in the LP636-CLOSE report still differ, unchanged here). Production LOC: 160984 -> 161033 (delta +49) Scope: first-party core/engine/control/api production source and shipped runtime SQL; tests, docs, tools and manifests excluded. Method: python3 -B tools/production_loc.py (SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6) on git archives of the first parent 5851b52 (160984) and this committed tree 8dffa04 (161033). Engine 75426 -> 75475; other scopes unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
`spilled_companions_outnumber_the_open_file_limit` lowered the process-wide soft `RLIMIT_NOFILE` to 512 and asserted the process descriptor count while the other tests of the same binary ran in threads beside it: either assertion could flake (LP636 review). The test now re-executes its own binary with `--exact` for this one test in a child process (`LAYERFS_NFS_FD_LIMIT_CHILD` marks the child), so the limit and the descriptor count belong to a process running nothing else. The parent asserts the child's exit status. Libtest and nextest both start the same test binary, so the child is selected by name in either. Two regression tests for the companion fixes: - `a_spilled_body_renamed_off_its_inode_takes_its_slot`: a whole image kept as written (attribute above the decoded-entry bound), evicted to disk by another companion's body, then renamed onto a name whose main file the export never saw. Before the reclassification it kept `whole` and took no slot. - `a_spilled_companion_keeps_its_length_and_bytes_together`: a truncate and a write of one spilled companion start together 64 times; the length read back always matches the bytes served. Without the per-companion edit gate the length can be left at the write's size over a truncated file, and the read then answers NFS3ERR_IO. Tests: `layerfs-nfs` write 14 passed, decode 10 passed (nextest); the other layerfs-nfs binaries were run separately (31 passed). Production LOC: 161033 -> 161033 (delta 0) Scope: first-party core/engine/control/api production source and shipped runtime SQL; tests, docs, tools and manifests excluded. Method: python3 -B tools/production_loc.py (SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6) on git archives of the first parent b4d0222 and this committed tree; only test files changed. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…nk (lp73) `Companion` is not in the body module's scope, so the module documentation linked a private item that rustdoc cannot resolve there. The gate is named in the parent module anyway. Production LOC: 161033 -> 161033 (delta 0) Scope: first-party core/engine/control/api production source and shipped runtime SQL; tests, docs, tools and manifests excluded. Method: python3 -B tools/production_loc.py (SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6); only comment lines changed, so the count is unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…aliases (lp73) - N1: at the bound CREATE releases the companion lock to reclaim orphans; it now reads the name and its main file again and inserts under the lock it holds once more, so a concurrent GUARDED CREATE of one name yields one serial and one NFS3ERR_EXIST (before: a second serial for the same name, or a spurious NFS3ERR_NOSPC once the other call had inserted). - N2: renamed() records the recalled name it overwrites, when that name did not move and is still one of the inode's, as an alias, so retiring the serial drops every key of by_name it holds. - N3: reclamation walks Companions::slots, the companions that take a slot, instead of the whole index. A whole image takes no slot and its number is bounded by the decoded bytes, not by HELD, so it no longer sets the walk's cost. nfs.md's CREATE, alias and reclamation paragraphs now say that. - Tests: a_guarded_create_reclaims_the_last_slot_for_one_entry_only and a_renamed_hard_link_alias_keeps_the_inodes_recalled_name_bound fail on the unfixed code (round 20 answers NOSPC instead of EXIST; the companion is no longer found), whole_images_take_no_slot_and_the_bound_reclaims_ only_slots pins the slot accounting reclamation reads. Production LOC: 161033 -> 161046 (delta +13) Counting: tools/production_loc.py --root <snapshot> --json, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; scope core/engine/control/api src plus shipped SQL, tests, docs and tools excluded; parents a7b6ffd (161033) and the staged tree (161046); layerfs-nfs apple_double.rs 549 -> 562, kernel.rs 777 -> 777. reference absent (retired R9c; last counted 65,417 at tree 498dd19).
cargo +1.85.1 fmt -p layerfs-nfs. The other files this branch changes were rustfmt'd in the commits that changed them; after this commit cargo fmt --all -- --check reports only the two upstream files (api/crates/layerfs-cli/tests/ concurrent_reads.rs, control/crates/layerfs-store/src/lease.rs), neither of which this branch touches. Production LOC: 161046 -> 161046 (delta 0) Counting: tools/production_loc.py --root <snapshot> --json, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; scope core/engine/control/api src plus shipped SQL, tests, docs and tools excluded; parents the previous commit's tree (161046) and the staged tree (161046); layerfs-nfs apple_double/edit.rs 298 -> 298 (physical lines 367). reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Whole images exceeding DECODED_ENTRY_BYTES or DECODED_BYTES stay as written bytes rather than Body::Decoded, so DECODED_BYTES bounds only the decoded subset while all whole images stay outside HELD and Companions::slots. Production LOC: 161046 -> 161046 (delta 0) Counting: tools/production_loc.py --root <snapshot> --json, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; scope core/engine/control/api src plus shipped SQL, tests, docs and tools excluded; parents 0a4dfdd (161046) and the staged tree (161046); comment-only change in layerfs-nfs apple_double.rs. reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…(lp73) Brings in upstream/layerfs_v2_0_0 at de1d77e (Ephemeral-AI-Lab#671 architecture docs reorganization and two daemon test updates). Conflict in docs/architecture/engine/kernel-routes/nfs.md keeps the upstream Ephemeral-AI-Lab#671 structure here; the AppleDouble spill/decode doc updates from this branch are ported in the next commit. Production LOC: 161046 -> 161046 (delta 0) Counting: tools/production_loc.py --root <snapshot> --json, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; scope core/engine/control/api src plus shipped SQL, tests, docs and tools excluded; first-parent 778d831 (161046) and merged tree (161046); upstream brought docs and test-only changes. reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…I-Lab#671 nfs.md (lp73) Topics updated: engine/kernel-routes (nfs.md). Ports this branch's ExportConfig fields, companion wcc_data rule, and AppleDouble spill/decode/concurrency/reclamation bounds and behavior into the restructured Ephemeral-AI-Lab#671 nfs.md layout, including the N3 clarification that whole images kept as written bytes above the decoded budget also take no HELD slot. Production LOC: 161046 -> 161046 (delta 0) Counting: tools/production_loc.py --root <snapshot> --json, SHA-256 d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6; scope core/engine/control/api src plus shipped SQL, tests, docs and tools excluded; parents 22b8df2 (161046) and the staged tree (161046); docs-only change. reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Collaborator
Author
|
感谢审阅。逐条回复如下（当前头 Blocking 1（unlink 扫 by_name）——已改。 Blocking 2（槽满时克隆整个 by_name）——已改。 新增 Blocking 3（描述与人为上限）——描述已按当前头重写；
To fix 1（companion_rename 对 spilled whole image）——已改。 To fix 2（edit_companion 3 轮后 NFS3ERR_IO、spill_for 无界）——已改。 每个伴随带自己的 To fix 3（spilled 编辑的长度竞态）——已改。 同一把门覆盖磁盘写与长度记录（ To fix 4（RLIMIT 与并行测试）——已改。 Rules。 新建的合并提交（ #644 冲突。 在当前头 |
yifanxuaaa
requested changes
Nov 14, 2026
yifanxuaaa
left a comment
yifanxuaaa
left a comment
Contributor
There was a problem hiding this comment.
Review record
- Head reviewed:
2e09f0295f. It merges cleanly withlayerfs_v2_0_0at27920d3bb6. - Tier: P1, gate: yes. Risks:
risk:kernel;risk:deps(newlibcdev-dependency oflayerfs-nfs);risk:security(newunsafegetrlimit/setrlimitintests/write.rs);risk:delete, added by hand (remove_dir_allof<mounts>/.appledoubleat reclaim, Attach and release).
- Read: the whole production diff, and the tests scanned for weakened assertions (none found). Re-run:
tools/check_product_boundary.pyPASS;tools/production_loc.pygives 159977 -> 161046 (+1069, all engine). Not run: cargo, any mount.
Verified as correct:
- Codec: byte-exact round trip (
compactrequiresencode() == input), and parsing is bounds-safe. - Spill files: paths are built from numeric ids only, so no traversal; files are
0700/0600, with no fsync and no space reserved ahead. - Concurrency:
Server::stopjoins connections before release, and the lock order is consistent. - Reclaim: limited to the per-Sandbox mount home.
Owner decisions needed before code review continues
- Artificial caps.
HELD= 65,536 andFILE_BYTES= 1 MiB (apple_double.rs:68-70) are total caps. The repository rules forbid these without owner approval (your options A/B/C). - New on-disk storage class.
<mounts>/.appledouble/lives outside the Workspace overlay database and daemon memory. - Resident memory that is never evicted.
DECODED_BYTES= 512 MiB per export (:74).
Findings
1. The slots ≤ HELD bound is not enforced (blocking).
- The doc comment (
apple_double.rs:148-152) andnfs.mdsay the walk isHELD-bounded. attach(:202-205) inserts every slotted companion with no admission check. It is reached fromedit_resident/classify(whole becomes false on a partial or non-compact body), fromrecord_edit(edit.rs:253), and from a rename onto an unknown main.- Scenario: copy 100k files (all whole images, 0 slots), then add resource forks or rewrite them.
slotsgrows pastHELDwith noNOSPC, and everyorphans()walk (:443) under the table lock then scales with it. - Fix: admit at the point a companion becomes slotted, or drop the bound claim and bound the walk another way.
2. Eviction is an O(n) scan under the companion lock, repeated (blocking).
oldest_resident(:369) scans every companion, whole images included.spill_for(edit.rs:271) can repeat it withinSPILL_ROUNDS(edit.rs:36), so the worst case is O(n²) work while holding the lock.- A lost adoption race in
spill_foris discarded and the pick is made again. The no-retry rule may cover that re-attempt. - Fix: keep an ordered recency index (for example a
BTreeMap<tick, serial>) of resident bodies.
3. Rename can exceed the resident budget. When a decoded body is renamed onto an unknown main, classify (:294) turns it back into resident bytes with no budget check or spill.
4. Live data can be reclaimed as an orphan. A companion created while its main name is missing from by_name (the name index full) gets inode = None. orphans() then treats it as dead and reclaims it while the main file still exists, losing its attributes.
5. Disk I/O under the companion lock. adopt_spilled (:339) runs fs::rename under the companion lock. The module doc and nfs.md say file I/O never holds it.
6. Spurious READ errors. companion_read uses read_exact_at (edit.rs:77) with a length read before the edit gate. A concurrent truncate gives NFS3ERR_IO. Use read_at and return a short read.
7. Full re-encode on every READ. A decoded image is re-encoded in full under the lock on each READ.
Size and history
- At +1069 production lines in gate-risk code, the PR is past the policy's ~800-line guide. Please split it, for example: codec, then spill, then edit gate and name index.
5851b529amixes movingReleasedintokernel/released.rswith a behaviour change. Land the move as its own relocation commit.- The title says "expand workspace capacity". These attributes live only in daemon memory and spill files and never reach the Workspace or a Commit, so a title like "AppleDouble companions beyond 4,096 per export" fits better.
Policy
- The PR body has no
Tier:line. - The macOS evidence is from
c0c83e3d3, from before the edit-gate, alias and reclamation rewrites. The Linux mount lane does not exercise the macOS AppleDouble path, so the feature has no real-client proof at this head.
Reviewer: Claude Opus 5.5 (independent-context review, finding 1 spot-checked against the code). Requesting changes for findings 1 and 2, the split, and the owner decisions above.
Contributor
Owner decisions for #636Context: LayerFS does not support extended attributes (the FUSE route answers 1.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
摘要
本 PR 为原生 macOS loopback NFSv3 导出提供 AppleDouble（
._*）伴随文件的内存索引 + 磁盘溢写，并把客户端写出的整份 AppleDouble 镜像解码为主 inode 的属性，去掉「复制大量文件后
could not copy extended attributes: Operation not permitted」的 4,096 上限问题（#635）。范围与数字（当前头
2e09f0295）layerfs_v2_0_0@de1d77e13。58c548b3e、三次合入上游）。tools/production_loc.py，core/engine/control/api 的 src + 随包 SQL，测试/文档/工具不计）：上游
159,977→ 本分支161,046（delta +1069，全部在 engine：74,419→75,488；core/control/api 不变）。reference absent (retired R9c; last counted 65,417 at tree 498dd1917812)。79e378e4f」是更早 head 的状态；之后fac912440（解码）、0ecb4c578（编辑与重新分类）、640a30483（合入58c548b3e）及两轮 review 修复均已在分支里。「65,536」是每个 export 的占槽伴随索引数（
HELD），不是 Workspace 容量；「≈20 MB/export」不再成立（见下「内存与准入」）。
行为
RESIDENT_BYTES）时，最久未用的 body 溢写到
<mounts>/.appledouble/<namespace>/<serial>（目录 0700、文件 0600，临时名
<serial>.<n>.tmp后rename收养，pread/pwrite每次按路径开、用完关，不按 body 持 fd）。的条目只保留 FinderInfo + 属性，READ 时按原字节重新编码；超出解码预算的 whole image 保留客户端原字节）；
所有 whole image 不占
HELD槽。资源叉、其它布局、半写入、主文件未知的伴随按客户端字节保存并占槽。关键路径外的孤儿在
HELD满时回收；release 与 daemon 启动reclaim删除整个<mounts>/.appledouble（崩溃残留），不碰 mount home 下其它东西。（并发 GUARDED CREATE 只成功一个，其余答
NFS3ERR_EXIST）；Table::aliases（serial → 硬链接别名，含
renamed覆写时保留的旧当前名）让 unlink/rename 只删自己的键，不再扫 4M 条by_name；HELD满时的回收只遍历Companions::slots（上界HELD，表锁内取伴随锁，不复制命名空间、也不遍历 whole image）；每个伴随的编辑由
Companion::editing门串行，溢写轮次有上界，溢写编辑把自己的 body 写出后就地编辑，竞争不再回
NFS3ERR_IO；溢写后的长度与字节在一次编辑内一起记录（truncate 与 WRITE 不会留下不一致的长度）。CREATE ._*答NFS3ERR_NOSPC（不再是ACCES），每次拒绝计数、日志每 export 至少 60 秒一条。
内存与准入（如实说明）
RESIDENT_BYTES（32 MiB）、DECODED_BYTES（512 MiB）、DECODED_ENTRY_BYTES（128 KiB）都是缓存预算：超出只改变保存形态（溢写到磁盘 / 按客户端原字节保存），不拒绝、不丢数据；
ENOSPC/EDQUOT→NFS3ERR_NOSPC，EFBIG、EACCES、EPERM、EROFS、EIO各自映射），占槽伴随最坏为HELD × FILE_BYTES = 65,536 × 1 MiB = 64 GiB/export（不占槽的 whole image 受
FILE_BYTES与文件系统容量约束，不受HELD约束）；HELD（65,536 槽）与FILE_BYTES（1 MiB/伴随）仍是人为总量上限，AGENTS.md 不允许其长期存在。去掉它们需要把伴随索引从内存搬到磁盘（CREATE/LOOKUP/REMOVE/RENAME/回收/释放全部改写），本轮未做、不做半改。三个方案请 owner 定夺：
<mounts>/.appledouble/<namespace>/index.sqlite（表
companion/name/inode，synchronous=OFF、单写入者），内存只留可回收的 LRU 缓存；HELD退化为缓存提示，FILE_BYTES删除、改为物理容量准入（ENOSPC/EDQUOT→NFS3ERR_NOSPC）。代价：每次 CREATE/LOOKUP/REMOVE多一次 DB 操作；
layerfs-nfs目前无第三方依赖，需新增rusqlite；索引文件需补生命周期与权限（0700/0600）。(parent,name)→ 定长头 + 名字 + 扩展块，append + 桶级 compact）。不改依赖，但崩溃一致性、删除、回收、并发 compaction 都要自写，代码量与出错面都比 A 大。
HELD/FILE_BYTES，在文档中明确为导出资源准入。不满足 AGENTS.md，需要 owner 明确豁免。Breaking change
layerfs_nfs::ExportConfig新增三个公开字段（companion_spill_dir、companion_resident_bytes、companions_held）与构造函数ExportConfig::writable(uid, gid)；用结构体字面量构造的调用方需要补字段或改用构造函数。daemon 已改用构造函数。
ExportServing::reclaim与 release 时删除<mounts>/.appledouble；CREATE ._*满额的新状态码是
NFS3ERR_NOSPC。验证
2e09f0295，cargo +1.85.1 --locked）：rc=0，1922/1922 passed（2 slow, 1 leaky，26 skipped，62.440s）；rc=0，146/146 passed（2 slow，11 skipped，61.361s）；/dev/fuse）：45/45 binaries passed（pass=45 fail=0）；NOT_RUN=0）；cargo +1.85.1 clippy --locked -p layerfs-nfs -p layerfs-daemon --all-targets -- -D warnings干净；cargo +1.85.1 fmt --all -- --check仅上游已知基线api/crates/layerfs-cli/tests/concurrent_reads.rs:16有 diff，本 PR 改动文件干净；python3 -B tools/check_fuser_integrity.py、python3 -B tools/check_product_boundary.py --skip-sdk、python3 -B -m unittest discover -s tools -p 'test_*.py'（182 tests）全部通过；spilled_companions_outnumber_the_open_file_limit额外独立复跑 2 次均通过（0.82s、0.65s）。NOT_RUN — disabled by owner until explicit reauthorization。cp/ditto/tar、4,300 个 4 KiB 与 4,300 个 1 MiB 文件）是79e378e4f时代的结论；当前头
2e09f0295未在 macOS 上复测（本轮在 Linux 上完成）。已知限制
._伴随往返（com.apple.provenance自动打标，每文件约 5–6 个 RPC）；#644（b49690d11）在engine/crates/layerfs-daemon/src/control/export.rs存在 git 文本冲突（
CONFLICT (content)，实测git merge-tree --write-tree 2e09f0295 b49690d11rc=1）：冲突在
export.rs的spill_root/companion_spill_dirvs#644的is_mounted以及reclaim()中的mounted_under签名；ExportConfig接线与两处remove_dir(serving.spill_root())无冲突。合并时按常规手工解决，保留spill_root()接线并用#644的新mounted_under(&self.config.mounts, self.config.mount_table.as_deref())签名即可；mounted_install::writes_during_a_commit_stay_live_and_reach_the_next_commit的既有偶发失败与基线同率（旧描述里的 220 次抽样统计仍有效；本轮全量 mount 45/45 一次通过）。
Fixes #635
Refs Ephemeral-AI-Lab/layerfs-project#73