Filesystem-only checkpoints#3289
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 86724ef. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 11c3fab. Configure here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for filesystem-only checkpoints, allowing sandboxes to be checkpointed without capturing a full memory snapshot. When a filesystem-only checkpoint is created, the sandbox cold-boots from the rootfs upon resumption. The changes span the API specification, orchestrator, and sandbox packages, replacing the boolean filesystem snapshot flag with a more robust SnapshotMode enum and adding support for thawing the guest filesystem upon resumption. The review feedback highlights three key areas for improvement: resolving a potential nil pointer dereference in build_upload_v3.go when memfileDiffHeader is nil, ensuring the resumed sandbox is shut down if thawing the guest filesystem fails to prevent resource leaks, and updating an outdated comment in sandboxes.go that claims filesystem-only checkpoints are not supported.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Orchestrator now allows creating filesystem only checkpoints. The orchestrator server now accepts a filesystem_only flag in the checkpoint request to control whether it will include a snapshot of the memory. Checkpoint is a bit different than pause, because we need to handle the existing sandbox. The way we do this now is creating a full snapshot, destroying the previous sandbox and starting a fresh one from the freshly created sandbox. Ideally, we should be reusing the existing sandbox and not starting a new one, but this is a bigger change. For the time being, we don't change the way things are done. We create the full snapshot always. If the checkpoint is filesystem only, we mark the Snapshot object as such and use the memory part only locally (we don't upload it). Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Wire the filesystem-only checkpoints through the REST API service. The SandboxSnapshotRequest includes an optional Memory booolean field. The default behaviour remains unmodified (full snapshot) so existing usecases keep working correctly. When Memory is false a filesystem-only checkpoint will be created. Sandboxes created from this checkpoint will start through a clean boot from the checkpointed disk state. The sandbox that we take the checkpoint from continues as is (no reboot). Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Adds the `memory` field to SandboxSnapshotRequest in the integration test client, matching spec/openapi.yml. Fixes generated-code-check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bchalios
force-pushed
the
feat/fs-only-checkpoint
branch
from
72caac0 to
3f9b448
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| eg, egCtx := errgroup.WithContext(ctx) | ||
|
|
||
| eg.Go(func() error { | ||
| // During checkpoint we might mark the snapshot as filesystem only, even | ||
| // though we create the memory snapshot, so that we can resume the existing | ||
| // sandbox that we are checkpointing. | ||
| // Keep the memory snapshot locally and avoid uploading it. | ||
| if u.snap.FilesystemSnapshot { | ||
| return nil | ||
| } | ||
|
|
||
| h, err := u.snap.MemorySnapshot.DiffHeader.WaitWithContext(egCtx) | ||
| if err != nil { | ||
| return fmt.Errorf("wait memfile diff header: %w", err) |
There was a problem hiding this comment.
🔴 Filesystem-only checkpoints (SnapshotModeFsOnlyKeepMemory) still create a full local memory diff (Pause only skips memory work for SnapshotModeFilesystem), but build_upload_v3.go/v4.go skip uploading that memfile's body, header, and ancestor bookkeeping. Because Checkpoint resumes the live sandbox from that same checkpoint build ID, the un-uploaded memfile becomes the sandbox's in-memory template layer; if that sandbox is later paused/checkpointed again with a normal full-memory snapshot (e.g. an auto-pause), the new build'''s memfile diff is computed against the fs-only build, so the new, fully-uploaded build persists a Mapping that references the fs-only build as an ancestor whose memfile bytes exist only in one orchestrator'''s local cache. Reading/resuming that new build on another node (or after local cache eviction) cannot fetch those ranges, corrupting the memory read.
Extended reasoning...
The bug. For a filesystem-only checkpoint (SnapshotModeFsOnlyKeepMemory), Sandbox.Pause in sandbox.go only skips processMemorySnapshot when mode == SnapshotModeFilesystem; for SnapshotModeFsOnlyKeepMemory it still runs the full memory-diff path (this is intentional — the checkpoint needs the live sandbox to resume seamlessly with its in-memory state intact). That diff is added to the local template cache via AddSnapshot. However, build_upload_v3.go and build_upload_v4.go gate on u.snap.FilesystemSnapshot and skip the memfile body upload, the memfile header upload, appendAncestorBuilds for the memfile, and publish for the memfile whenever that flag is set — which it is for SnapshotModeFsOnlyKeepMemory as well as SnapshotModeFilesystem. So the fs-only checkpoint build's memfile diff exists only in the checkpointing orchestrator's local on-disk cache; it is never durably persisted to remote storage.\n\nHow it becomes reachable. Server.Checkpoint (sandboxes.go) resumes the same build ID it just checkpointed (templateCache.GetTemplate(ctx, in.GetBuildId(), ...)), so resumedSbx.Template's memfile is that local-only diff. The resumed sandbox is a completely ordinary live sandbox from that point on — it can be paused again later through the normal Server.Pause path with the default SnapshotModeFull (e.g. via auto-pause on idle timeout, which is the common lifecycle for E2B sandboxes). When that happens, processMemorySnapshot computes the new build's memfile diff against s.Template.Memfile().Header(), i.e. against the fs-only checkpoint build. For any page the fs-only build dirtied that isn't re-dirtied by the time of the next pause, the new build's diff header Mapping still points at the fs-only build's ID as the owning ancestor.\n\nWhy nothing catches it. This new build undergoes a normal (non-fs-only) upload, so appendAncestorBuilds in build_upload_v4.go calls u.uploads.Wait(ctx, fsOnlyBuildID, Memfile) for that ancestor. The fs-only build's upload already finished (its future fired when its — memfile-skipping — upload completed), so Wait returns immediately with the ancestor's locally-cached header and no error; it has no way to know that header's memfile bytes were never actually written to remote storage. Because that header's version is V4/V5 (not V3), the existing V3-sentinel fallback in appendAncestorBuilds (h.Metadata.Version < headers.MetadataVersionV4) does not trigger, so no compensating BuildData entry is written either. The result: the newer, fully-uploaded build persists a memfile Mapping that references an ancestor build ID whose memfile object was never uploaded anywhere but the original checkpointing node's local disk.\n\nImpact. Any later read of that new build's memfile that needs one of those un-redirtied ranges — resuming on a different orchestrator node (the normal case, since client-proxy can route a resume to any node and peer-to-peer chunk transfer targets in-progress uploads, not already-finished-but-incomplete ones), or even the same node after the fs-only build's entry is evicted from the local cache — will fail to fetch the referenced bytes, corrupting or failing the memory restore. This is a data-integrity regression: prior to this PR, every checkpoint always uploaded a full memory snapshot, so every ancestor in a memfile diff chain was guaranteed to be durably persisted; this PR breaks that invariant for the specific case of a full-memory pause built on top of an fs-only checkpoint.\n\nStep-by-step proof.\n1. Client calls checkpoint with filesystem_only=true on sandbox S → build A is created with SnapshotModeFsOnlyKeepMemory. Pause still computes A's real memfile diff+header (mode != SnapshotModeFilesystem) and caches it locally via AddSnapshot.\n2. build_upload_v3/v4.runV3/runV4 skip A's memfile body/header/ancestor-append/publish entirely because u.snap.FilesystemSnapshot is true. A's memfile is never uploaded; A's upload future still fires (rootfs + metadata do upload).\n3. Server.Checkpoint resumes S from build A (GetTemplate(A)); the resumed sandbox's Template.Memfile() is A's local-only diff, and the sandbox keeps running normally under the same execution ID.\n4. Later, S auto-pauses (or is explicitly paused) with the default full-memory mode → build B. processMemorySnapshot diffs against A's header, so B's memfile diff header Mapping contains entries pointing at build A for pages A dirtied that B didn't re-dirty.\n5. B is uploaded normally (not fs-only). appendAncestorBuilds calls uploads.Wait(A, Memfile), which returns A's already-cached local header without error (A's future already fired). Since A's header version is >= V4 and lacks a self BuildData entry (that write was skipped in step 2), no BuildData gets written for A into B's Builds map either.\n6. B finishes uploading successfully and is now the durable, currently-active build — but its memfile Mapping still references A's un-redirtied ranges, and A's memfile bytes exist only on the original node's local disk.\n7. Any future read of B's memfile for one of those ranges from another orchestrator node, or from the same node after A is evicted from its local template cache, cannot locate the data → resume/read failure or silent corruption.\n\nFix direction. Either root the resumed sandbox's memory chain at a build that is guaranteed to be durably persisted (e.g. force a full memory upload on the very first subsequent full-memory pause instead of diffing against the fs-only build), or explicitly reject/flatten a full-memory pause whose base template is an fs-only-checkpoint memfile that was never uploaded.
| // so the old sandbox's cleanup goroutine won't | ||
| // accidentally evict the resumed sandbox from the map. | ||
| var resumeOpts []sandbox.ResumeOption | ||
| if snapshotMode == sandbox.SnapshotModeFsOnlyKeepMemory { | ||
| resumeOpts = append(resumeOpts, sandbox.WithFsThawGuest()) | ||
| } | ||
| resumedSbx, err := s.sandboxFactory.ResumeSandbox( |
There was a problem hiding this comment.
🔴 Server.Checkpoint always requests a guest filesystem thaw on resume for fs-only checkpoints, but the matching freeze in Pause only runs when envd supports the native fsfreeze endpoint (envd >= 0.6.6) — a higher bar than the 0.5.0 minimum the checkpoint feature itself enforces. For any sandbox with envd in [0.5.0, 0.6.6), the guest is synced (never frozen) during pause, yet resume still calls the nonexistent /fsthaw route, which 404s and, per this PR's own error handling, tears down the freshly-resumed sandbox and fails the whole checkpoint.
Extended reasoning...
The bug: In Server.Checkpoint (packages/orchestrator/pkg/server/sandboxes.go:805-811), sandbox.WithFsThawGuest() is appended to resumeOpts whenever snapshotMode == SnapshotModeFsOnlyKeepMemory, with no envd-version check. But the corresponding freeze during Pause (via guestPrepareFsForPause in reclaim.go) is itself conditional: it only calls callEnvdFsfreeze when envdSupportsFsFreeze(ctx) is true, i.e. envd >= MinEnvdVersionForFsFreeze (0.6.6). When envd is below that bar, guestPrepareFsForPause silently falls back to a plain guestSync — no freeze happens at all.
The checkpoint feature's own eligibility gate, however, is much lower: CheckEnvdVersionForSnapshot only requires envd >= MinEnvdVersionForSnapshot (0.5.0) (packages/shared/pkg/utils/version.go:9,38), and this is the same check used by the public POST /sandboxes/{id}/snapshots API handler. So the API happily accepts filesystem-only checkpoint requests for any sandbox with envd anywhere in the range [0.5.0, 0.6.6) — a real, currently-reachable range — while the orchestrator-side thaw call has no matching gate.
The failure path: After Pause completes (with no freeze, for old envd), ResumeSandbox unconditionally calls sbx.callEnvdFsthaw when ropts.fsThawGuest is set (sandbox.go:1099). That routes through callEnvdPostOp → postEnvd, which requires an HTTP 204 response and treats anything else as an error (envd.go:202-206). An envd below 0.6.6 does not expose the /fsthaw route in its chi router at all, so this returns a 404, and postEnvd returns an error. Per this same PR's newly-added error handling at sandbox.go:1099-1102, that thaw failure is joined with sbx.Shutdown(ctx) and returned — tearing down the freshly-resumed sandbox and failing the entire Checkpoint RPC with codes.Internal.
Why nothing else catches this: The codebase already has the correct pattern elsewhere — bestEffortFsthaw (reclaim.go:387-390) explicitly gates its thaw call on envdSupportsFsFreeze(ctx) before calling it, precisely because thawing is only meaningful/safe when a freeze actually happened. The Checkpoint resume-options logic in sandboxes.go omits that same guard, so it issues a thaw against an endpoint it never froze and that may not even exist on the guest.
Step-by-step proof:
- A team's sandbox is running envd 0.6.0 (in-range for
CheckEnvdVersionForSnapshot, but belowMinEnvdVersionForFsFreeze). - Client calls
POST /sandboxes/{id}/snapshotswith{"memory": false}. The API's version check (0.5.0) passes, so the request reaches the orchestrator'sCheckpointRPC withfilesystem_only=true. CheckpointsetssnapshotMode = SnapshotModeFsOnlyKeepMemoryand callssnapshotAndCacheSandbox, which callssbx.Pausewith that mode.- Inside
Pause, since mode !=SnapshotModeFull,guestPrepareFsForPauseruns; it checksenvdSupportsFsFreeze→ false (0.6.0 < 0.6.6) → falls back toguestSynconly, no freeze performed. - Back in
Checkpoint, becausesnapshotMode == SnapshotModeFsOnlyKeepMemory,resumeOptsgetssandbox.WithFsThawGuest()appended — unconditionally, with no check of whether a freeze actually happened. ResumeSandboxis called with thoseresumeOpts. After the guest boots and envd is ready, sinceropts.fsThawGuestis true, it callssbx.callEnvdFsthaw.- That POSTs to
/fsthawon an envd build that doesn't implement the route → 404 →postEnvdreturns a non-nil error. - Per the error handling in
ResumeSandbox, this error is joined withsbx.Shutdown(ctx)and returned as the resume error. Checkpointpropagates this ascodes.Internal, having already marked the original sandbox as stopping. The user's checkpoint attempt fails, and their previously-running sandbox is gone.
The fix is small: gate the sandbox.WithFsThawGuest() append in Checkpoint on the same envdSupportsFsFreeze-equivalent check that guestPrepareFsForPause and bestEffortFsthaw already use, so the thaw is only requested when a freeze actually ran.
All five independent verifiers traced this exact path and confirmed it; there were no refutations.
1 participant
What
Adds filesystem-only checkpoints to the snapshot-template endpoint.
POST /sandboxes/{id}/snapshotsnow accepts{"memory": false}to mint a rootfs-only template — no memory snapshot is persisted. Sandboxes launched from such a template cold-boot (reboot) from disk, while the checkpointed sandbox keeps running uninterrupted. Default stays a full memorysnapshot, so existing callers are unaffected.
Why
Memory snapshots are large to store and transfer. When a template only needs to capture disk state (in-memory state, running processes, and open connections aren't needed on launch), a filesystem-only checkpoint is significantly cheaper to persist. This brings checkpoint in line with the fs-only capability the pause path already had.
How
SnapshotModeenum (Full/Filesystem/FsOnlyKeepMemory) replaces the ad-hoc pause booleans, making the valid combinations explicit.FsOnlyKeepMemory: it still takes a full memory snapshot locally so the live sandbox resumes seamlessly (same execution ID), but marks the persisted template filesystem-only and skips uploading the memfile. The rootfs is quiesced (fsfreeze) before the snapshot so it's self-consistent without the discarded page cache, and the resumedsandbox is thawed afterward.
memoryboolean onSandboxSnapshotRequest→filesystem_onlyon the gRPCSandboxCheckpointRequest; templates are stamped fs-only in metadata so the existing Create path reboots them.Testing
Locally against the dev stack: create a sandbox,
POST /snapshots {"memory": false}, verify (a) the original sandbox stays running with an unchangedboot_id, and (b) a new sandbox from the template has the pre-checkpoint rootfs marker but a freshboot_id(rebooted, memory not restored).Drop the Testing section if you don't want it. Want me to tweak the tone or trim any section further?