Skip to content

fix(nfsproxy): clean up .nfs* silly-rename orphans on sandbox teardown (rebase of #3533) - #3728

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/nfs-orphan-cleanup-v2
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/nfs-orphan-cleanup-v2

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Rebased follow-up to #3533, which @ryan-sana closed as inactive (76 days) and no longer merging cleanly. The leak is still present on current main (no .nfs* cleanup anywhere in the tree), so this re-does the fix against main's current structure and adds a regression test.

Problem

NFS silly-rename: when an NFS client deletes a file that still has open file descriptors, the client renames it to .nfsXXXX on the server instead of removing it, and only sends the REMOVE once the last descriptor is closed. If the sandbox VM is killed before closing its descriptors, that REMOVE never arrives and the .nfsXXXX file — as large as the original, often several GiB — leaks on the NFS server permanently.

Observed in production: a 5 GiB workspace image lingering as .nfs000000019000ee98..., no process holding it (confirmed via /proc/*/fd), never reclaimed.

Fix

OnNetworkRelease runs after the VM is dead and all of its descriptors are gone, so any .nfs* orphan in the volume is safe to delete there. Add removeNFSOrphans, called per chroot before Close():

  • scans the chroot host-side root (chrooted.Chrooted.ActualRoot, returned by chroot.Root()),
  • removes top-level .nfs* files, skips subdirectories (entry.IsDir() guard, so a directory named .nfs… is never touched),
  • best-effort: failures are logged and never abort teardown.

Adapted to current main: OnNetworkRelease now iterates mounts by lifecycleID and takes the chroot from mounted.chroot; the cleanup is inserted right before chroot.Close().

Test

TestRemoveNFSOrphans (no external deps):

  • .nfs* files removed; real files, non-.nfs dotfiles, and a .nfs… subdirectory kept;
  • a missing directory is a no-op (no panic, no error);
  • an empty directory is a no-op.

go build ./... and go vet pass.

Note on reproduction

Silly-rename only occurs on a real NFS mount, so the mechanism itself isn't reproducible on a non-NFS dev filesystem; the test exercises the cleanup logic given .nfs* orphans, and the mechanism/impact rests on the NFS protocol behavior plus the production observation above.

Fixes #3532. Supersedes #3533.

When an NFS client deletes a file that still has open file descriptors, it
renames the file to .nfsXXXX on the server instead of removing it, and only
sends the REMOVE once the last descriptor is closed. If the sandbox VM is
killed before closing its descriptors, that REMOVE never arrives and the
.nfsXXXX file - as large as the original, often several GiB - leaks on the NFS
server permanently (observed in production: a 5 GiB workspace image lingering
as .nfs000000019000ee98... with no process holding it).

OnNetworkRelease runs after the VM is dead and all of its descriptors are gone,
so any .nfs* orphan in the volume is safe to delete there. Add removeNFSOrphans,
called per chroot before Close(): it scans the chroot host-side root
(chrooted.Chrooted.ActualRoot) and removes top-level .nfs* files, skipping
subdirectories, best-effort (failures logged, teardown never aborted).

Rebased onto current main (OnNetworkRelease now iterates mounts by lifecycleID
and takes the chroot from mounted.chroot). Adds a regression test:
.nfs* removed, real/other dotfiles and a .nfs* subdirectory kept, and a missing
or empty directory is a no-op.

Fixes e2b-dev#3532. Supersedes e2b-dev#3533.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

persistent volume: .nfs* orphan files leak storage when sandbox VMs are forcibly killed

2 participants