Repository navigation
dofs: fingerprint deleted files - #231
Conversation
|
| Name | Type |
|---|---|
| @cloudflare/dofs | Patch |
| @cloudflare/computer-rpc | Patch |
| @cloudflare/computer | Patch |
| @cloudflare/computerd | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "SELECT COUNT(*) FROM vfs_dirents WHERE parent_inode = ?", | ||
| live.inode, | ||
| ); | ||
| if ((children ?? 0) > 0) continue; |
There was a problem hiding this comment.
🔴 Large recursive deletes resurrect directories
When a recursive delete spans pull batches, settleDeferredDirectories discards the parent tombstone while later children remain. Later batches remove those children, but the next push recreates the empty directory remotely.
Learn more
A recursive removal produces a tombstone for the parent directory followed by tombstones for its children. The pull driver applies a bounded batch and advances its fetch cursor before processing the next batch (pullOnce); restartable pulls also apply one block at a time (runPullBlock). When the parent has unseen descendants, its delete is deferred only until the end of that one apply call. If children remain then, the parent tombstone is discarded, yet the fetch cursor advances past it. Later batches can remove every child without revisiting the parent.
Example: The host has unpushed /local and an unacknowledged copy of /dist containing 300 files. The container deletes /dist recursively. A 256-entry pull batch defers /dist and deletes 255 children; the next batch deletes the rest. The host retains empty /dist and pushes it back to the container.
Recommended fix: Preserve deferred directory tombstones across batches or blocks until their descendants have been processed, and decide them before checkpointing past the final relevant descendant. Keep the pending state durable for restartable pulls and ensure the read-only mount and unseen-local-work guards still apply at settlement.
Was this helpful? React with 👍 or 👎 to provide feedback.
…r batch Addresses both Devin review findings on #230. Restarted container (watermarks.ts:168). pullOnce's divergence path reset only the pushRev watermark. readPushCursor still returned the old cursor row, so the window looked clean and the host acknowledged its pull. The receiver raises its fetch cursor to whatever it is sent, so that acknowledgment also covered files the restarted container never got and hid the restart from reconcileWatermarks for good. - openPullWindow now also requires the backend to confirm our push cursor, using the appliedPushCursor that fetchChanges and fetchChangePack already return. No extra round trip, and it holds on every pull path, including pullBlocks, which has no divergence check. - pullOnce's divergence path rewinds the push cursor itself, as reconcileWatermarks does, so push re-sends the host's files. Interrupted pulls (sync-driver.ts:289). pullOnce acknowledged only after the whole stream drained, so a failure after a committed batch left that batch's revs looking like host work. It now acknowledges per batch, as pullBlocks does per block. Only a failure between a batch's commit and its own acknowledgment leaves it unacknowledged; fingerprinted deletes (pr185-delete-fingerprint) cover that case. Tests first, all four failing before the change: - re-sends host files to a container that restarted empty (pullOnce) - entries/pack: an acknowledgment never hides a container that restarted empty (pullBlocks + reconcileWatermarks) - acknowledges each committed batch, so an interrupted pull keeps what it applied The earlier test "applies a container delete pulled during a pushRev reset" modelled a container that forgot its cursor but kept its files. With the push cursor now rewound, step 1 alone keeps that delete and re-sends the file; fingerprinted deletes apply it. The test is replaced by the restart test above, which models a fresh computerd database.
A tombstone now records the fingerprint of the version it removed: type, permission bits, and the file's manifest hash or symlink target, leaving out revs and mtimes. It is stored in vfs_changes.prior (schema v9, nullable) and travels as ChangeEntry.prior in both transports. A pulled delete applies when the local version is seen by the backend (the push cursor covers it) or is exactly the version the backend removed. It is kept only when the local version is both unseen and different. That closes what the acknowledgment cannot: - a pulled file deleted before its pull could be acknowledged - a rewritten name beside an unpushed hardlink - a delete pulled after a divergence reset A directory's fingerprint is its mode and counts only once it is empty. A directory delete held back by unseen descendants is decided again at the end of the apply call, after the descendants' own deletes, and the revs that removal mints are counted so the pull can still be acknowledged. Older peers and bulk directory-rename tombstones send no fingerprint and keep the cursor rule. Push receivers keep the replay rule. Known trade-off, pinned in replay.test.ts: a replayed fingerprinted delete removes an identical recreation made between the original apply and its cursor checkpoint. Tests first: the new fingerprint, migration, tombstone, apply and rpc tests failed before the change; existing exact-shape assertions on delete entries were updated, and one vacuous negative assertion in coalesce.test.ts was rewritten to check by path.
With the push cursor rewound on divergence, the cursor rule treats the host's files as unseen. A container that forgot its cursor but kept its files sends a fingerprinted delete, which matches and applies.
966c7de to
af3623e
Compare
commit: |
1 participant
The change in #230 for the host to acknowledge files only works if the host can confirm the pull. If it can't (for example, the host had unpushed work of its own at the time), pulled files still look like host edits, so the deleted file comes back.
Example:
dist/and the host pulls it, but can't confirm.rm -rf dist.dist/is its own unsaved work, so it ignores the delete.dist/back to the container. ❌Now, every delete carries a hash of the file it removed. If the host's copy matches that fingerprint, deleting it loses nothing, so the delete applies.
rm -rf dist. The delete says "I removed exactly this version."One edge case. A rare crash in the middle of a pull can cause a delete to be applied a second time. If the host had recreated the file with identical content in between, that second delete removes it.