From 307068cb419aff319539733f05cda02948c31cf8 Mon Sep 17 00:00:00 2001 From: Babis Chalios Date: Tue, 21 Jul 2026 10:26:12 +0200 Subject: [PATCH 1/3] feat(rootfs): primitives for deferred rootfs export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the building blocks to seal the rootfs diff off the pause critical path: - block.Cache.DiffMetadata() returns the dirty/empty bitmaps from the frozen tracker without copying, so the diff header + scheduling metadata can be built synchronously while the reflink copy runs in the background. - rootfs.Provider.PrepareExportDiff ejects the writable cache, stops the sandbox and waits for the overlay device to release, then returns the frozen ejected cache WITHOUT reflinking it (shares ejectAndStopSandbox with ExportDiff). DirectProvider stubs it (NBD-only). - build.NewDeferredDiff wraps a promise of a Diff: CacheKey/BlockSize are synchronous (DiffStore.Add + upload compress-config need them up front), while data methods block on the promise until the background sealer resolves it. No behavior change yet — nothing calls these. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Babis Chalios --- .../orchestrator/pkg/sandbox/block/cache.go | 55 +++++++++- .../sandbox/block/cache_diffmetadata_test.go | 55 ++++++++++ .../pkg/sandbox/build/deferred_diff.go | 102 ++++++++++++++++++ .../pkg/sandbox/build/deferred_diff_test.go | 92 ++++++++++++++++ .../orchestrator/pkg/sandbox/rootfs/direct.go | 4 + .../orchestrator/pkg/sandbox/rootfs/nbd.go | 43 ++++++-- .../orchestrator/pkg/sandbox/rootfs/rootfs.go | 11 ++ 7 files changed, 352 insertions(+), 10 deletions(-) create mode 100644 packages/orchestrator/pkg/sandbox/block/cache_diffmetadata_test.go create mode 100644 packages/orchestrator/pkg/sandbox/build/deferred_diff.go create mode 100644 packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go diff --git a/packages/orchestrator/pkg/sandbox/block/cache.go b/packages/orchestrator/pkg/sandbox/block/cache.go index e1f555423f..b0d50809e4 100644 --- a/packages/orchestrator/pkg/sandbox/block/cache.go +++ b/packages/orchestrator/pkg/sandbox/block/cache.go @@ -106,18 +106,62 @@ func (c *Cache) isClosed() bool { return c.closed.Load() } +// DiffMetadata returns the dirty/empty diff metadata from the tracker without +// copying any block data. It lets a deferred/background seal build the diff +// header (and scheduling metadata) synchronously while the actual reflink copy +// (ExportToDiff) runs off the critical path. The result matches what +// ExportToDiff computes internally, provided the cache is frozen (no writes) in +// between — which is guaranteed once the sandbox has been stopped and the cache +// ejected. +func (c *Cache) DiffMetadata() (*header.DiffMetadata, error) { + c.mu.RLock() + defer c.mu.RUnlock() + + if c.isClosed() { + return nil, NewErrCacheClosed(c.filePath) + } + + if c.mmap == nil { + return header.NewDiffMetadata(c.blockSize, nil, nil), nil + } + + dirty, empty := c.tracker.Export() + + return header.NewDiffMetadata(c.blockSize, dirty, empty), nil +} + func (c *Cache) ExportToDiff(ctx context.Context, out *os.File) (*header.DiffMetadata, error) { - ctx, childSpan := tracer.Start(ctx, "export-to-diff") - defer childSpan.End() + c.mu.Lock() + defer c.mu.Unlock() + return c.exportToDiffLocked(ctx, out, nil) +} + +// ExportToDiffWithMetadata copies the dirty ranges described by meta to out, +// using meta's bitmap instead of re-reading the tracker. Callers that captured a +// DiffMetadata earlier (e.g. the deferred rootfs seal, which reads it at setup +// and exports later in the background) use this so the copied ranges are +// guaranteed to match a header built from the same bitmap read. +func (c *Cache) ExportToDiffWithMetadata(ctx context.Context, out *os.File, meta *header.DiffMetadata) (*header.DiffMetadata, error) { c.mu.Lock() defer c.mu.Unlock() + return c.exportToDiffLocked(ctx, out, meta) +} + +func (c *Cache) exportToDiffLocked(ctx context.Context, out *os.File, meta *header.DiffMetadata) (*header.DiffMetadata, error) { + ctx, childSpan := tracer.Start(ctx, "export-to-diff") + defer childSpan.End() + if c.isClosed() { return nil, NewErrCacheClosed(c.filePath) } if c.mmap == nil { + if meta != nil { + return meta, nil + } + return header.NewDiffMetadata(c.blockSize, nil, nil), nil } @@ -137,8 +181,11 @@ func (c *Cache) ExportToDiff(ctx context.Context, out *os.File) (*header.DiffMet logger.L().Warn(ctx, "error syncing file", zap.Error(err)) } - dirty, empty := c.tracker.Export() - diffMetadata := header.NewDiffMetadata(c.blockSize, dirty, empty) + diffMetadata := meta + if diffMetadata == nil { + dirty, empty := c.tracker.Export() + diffMetadata = header.NewDiffMetadata(c.blockSize, dirty, empty) + } dst := int(out.Fd()) var writeOffset int64 diff --git a/packages/orchestrator/pkg/sandbox/block/cache_diffmetadata_test.go b/packages/orchestrator/pkg/sandbox/block/cache_diffmetadata_test.go new file mode 100644 index 0000000000..567d540277 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/block/cache_diffmetadata_test.go @@ -0,0 +1,55 @@ +//go:build linux + +package block + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" +) + +// TestCacheDiffMetadata_MatchesExportToDiff verifies DiffMetadata (used to build +// the diff header synchronously) produces the same dirty/empty bitmaps as the +// metadata ExportToDiff computes while copying, so a background seal's header is +// exact. +func TestCacheDiffMetadata_MatchesExportToDiff(t *testing.T) { + t.Parallel() + + blockSize := int64(header.PageSize) + numBlocks := int64(4) + + c, err := NewCache(blockSize*numBlocks, blockSize, t.TempDir()+"/cache", false) + require.NoError(t, err) + t.Cleanup(func() { _ = c.Close() }) + + // Dirty block 0, zero block 2, leave 1 and 3 untouched. + dirtyBlock := make([]byte, blockSize) + for i := range dirtyBlock { + dirtyBlock[i] = 0xAB + } + _, err = c.WriteAt(dirtyBlock, 0) + require.NoError(t, err) + _, err = c.WriteZeroesAt(2*blockSize, blockSize) + require.NoError(t, err) + + // Metadata read without copying. + meta, err := c.DiffMetadata() + require.NoError(t, err) + + // Metadata computed by the actual export. + out, err := os.CreateTemp(t.TempDir(), "diff-*") + require.NoError(t, err) + t.Cleanup(func() { _ = out.Close() }) + + exportMeta, err := c.ExportToDiff(t.Context(), out) + require.NoError(t, err) + + require.True(t, meta.Dirty.Equals(exportMeta.Dirty), "dirty bitmaps must match") + require.True(t, meta.Empty.Equals(exportMeta.Empty), "empty bitmaps must match") + require.Equal(t, exportMeta.BlockSize, meta.BlockSize) + require.EqualValues(t, 1, meta.Dirty.GetCardinality()) + require.EqualValues(t, 1, meta.Empty.GetCardinality()) +} diff --git a/packages/orchestrator/pkg/sandbox/build/deferred_diff.go b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go new file mode 100644 index 0000000000..91392b2853 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go @@ -0,0 +1,102 @@ +//go:build linux + +package build + +import ( + "context" + + "github.com/e2b-dev/infra/packages/shared/pkg/storage" + "github.com/e2b-dev/infra/packages/shared/pkg/utils" +) + +// deferredDiff is a Diff whose backing data is produced asynchronously. It is +// returned synchronously from a pause that seals the rootfs in the background: +// the cache key and block size are known up front (so DiffStore.Add and the +// upload's compress-config validation work immediately), while every data- +// bearing method blocks on the inner promise until the background seal resolves +// the real Diff. +// +// The producer MUST always resolve the promise — with the sealed Diff on success +// or an error on failure — otherwise the data methods (and Close) block forever. +type deferredDiff struct { + cacheKey DiffStoreKey + blockSize int64 + inner *utils.SetOnce[Diff] +} + +var _ Diff = (*deferredDiff)(nil) + +// NewDeferredDiff wraps a promise of a Diff. cacheKey and blockSize are the +// synchronously-known identity of the diff; inner is resolved by the background +// sealer with the materialized Diff (or an error). +func NewDeferredDiff(cacheKey DiffStoreKey, blockSize int64, inner *utils.SetOnce[Diff]) Diff { + return &deferredDiff{ + cacheKey: cacheKey, + blockSize: blockSize, + inner: inner, + } +} + +func (d *deferredDiff) CacheKey() DiffStoreKey { + return d.cacheKey +} + +func (d *deferredDiff) BlockSize() int64 { + return d.blockSize +} + +func (d *deferredDiff) CachePath(ctx context.Context) (string, error) { + inner, err := d.inner.WaitWithContext(ctx) + if err != nil { + return "", err + } + + return inner.CachePath(ctx) +} + +func (d *deferredDiff) ReadAt(ctx context.Context, p []byte, off int64, ft *storage.FrameTable) (int, error) { + inner, err := d.inner.WaitWithContext(ctx) + if err != nil { + return 0, err + } + + return inner.ReadAt(ctx, p, off, ft) +} + +func (d *deferredDiff) Slice(ctx context.Context, off, length int64, ft *storage.FrameTable) ([]byte, error) { + inner, err := d.inner.WaitWithContext(ctx) + if err != nil { + return nil, err + } + + return inner.Slice(ctx, off, length, ft) +} + +func (d *deferredDiff) Size(ctx context.Context) (int64, error) { + inner, err := d.inner.WaitWithContext(ctx) + if err != nil { + return 0, err + } + + return inner.Size(ctx) +} + +func (d *deferredDiff) FileSize(ctx context.Context) (int64, error) { + inner, err := d.inner.WaitWithContext(ctx) + if err != nil { + return 0, err + } + + return inner.FileSize(ctx) +} + +// Close waits for the seal to resolve and closes the materialized diff. If the +// seal failed there is nothing to close (the producer cleans up the partial file +// on error), so only close when the diff actually materialized. +func (d *deferredDiff) Close() error { + if inner, err := d.inner.Wait(); err == nil { + return inner.Close() + } + + return nil +} diff --git a/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go b/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go new file mode 100644 index 0000000000..bad53dc4fc --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go @@ -0,0 +1,92 @@ +//go:build linux + +package build + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/e2b-dev/infra/packages/shared/pkg/utils" +) + +// makeLocalDiff writes `data` to a fresh local diff file and materializes it. +func makeLocalDiff(t *testing.T, blockSize int64, data []byte) Diff { + t.Helper() + + f, err := NewLocalDiffFile(t.TempDir(), "build-id", Rootfs) + require.NoError(t, err) + _, err = f.WriteAt(data, 0) + require.NoError(t, err) + + d, err := f.CloseToDiff(blockSize) + require.NoError(t, err) + + return d +} + +// TestDeferredDiff_IdentityIsSynchronous verifies CacheKey and BlockSize resolve +// without blocking on the (still-unresolved) inner promise, so DiffStore.Add and +// the upload's compress-config validation work the moment the diff is created. +func TestDeferredDiff_IdentityIsSynchronous(t *testing.T) { + t.Parallel() + + inner := utils.NewSetOnce[Diff]() + key := GetDiffStoreKey("build-id", Rootfs) + d := NewDeferredDiff(key, 4096, inner) + + require.Equal(t, key, d.CacheKey()) + require.EqualValues(t, 4096, d.BlockSize()) +} + +// TestDeferredDiff_DelegatesAfterResolve verifies the data methods block until +// the promise resolves, then delegate to the materialized diff. +func TestDeferredDiff_DelegatesAfterResolve(t *testing.T) { + t.Parallel() + + blockSize := int64(4096) + data := make([]byte, blockSize) + for i := range data { + data[i] = 0x5A + } + + materialized := makeLocalDiff(t, blockSize, data) + t.Cleanup(func() { _ = materialized.Close() }) + + inner := utils.NewSetOnce[Diff]() + d := NewDeferredDiff(materialized.CacheKey(), blockSize, inner) + + // Resolve on a goroutine; the reads below block until it lands. + go func() { _ = inner.SetValue(materialized) }() + + path, err := d.CachePath(t.Context()) + require.NoError(t, err) + require.NotEmpty(t, path) + + sz, err := d.Size(t.Context()) + require.NoError(t, err) + require.Equal(t, blockSize, sz) + + buf := make([]byte, blockSize) + _, err = d.ReadAt(t.Context(), buf, 0, nil) + require.NoError(t, err) + require.Equal(t, data, buf) +} + +// TestDeferredDiff_PropagatesError verifies a failed seal surfaces through the +// data methods, and Close is a no-op (the producer cleans up the partial file). +func TestDeferredDiff_PropagatesError(t *testing.T) { + t.Parallel() + + inner := utils.NewSetOnce[Diff]() + d := NewDeferredDiff(GetDiffStoreKey("build-id", Rootfs), 4096, inner) + + sealErr := errors.New("seal failed") + go func() { _ = inner.SetError(sealErr) }() + + _, err := d.CachePath(t.Context()) + require.ErrorIs(t, err, sealErr) + + require.NoError(t, d.Close(), "Close is a no-op when the seal failed") +} diff --git a/packages/orchestrator/pkg/sandbox/rootfs/direct.go b/packages/orchestrator/pkg/sandbox/rootfs/direct.go index a5867ba4e4..3da72c99cd 100644 --- a/packages/orchestrator/pkg/sandbox/rootfs/direct.go +++ b/packages/orchestrator/pkg/sandbox/rootfs/direct.go @@ -113,6 +113,10 @@ func (o *DirectProvider) ExportDiff( return m, nil } +func (o *DirectProvider) PrepareExportDiff(_ context.Context, _ func(context.Context) error) (*block.Cache, error) { + return nil, ErrDeferredExportNotSupported +} + func (o *DirectProvider) Close(ctx context.Context) error { o.finishedOperations <- struct{}{} diff --git a/packages/orchestrator/pkg/sandbox/rootfs/nbd.go b/packages/orchestrator/pkg/sandbox/rootfs/nbd.go index 76fc86def0..96152f74aa 100644 --- a/packages/orchestrator/pkg/sandbox/rootfs/nbd.go +++ b/packages/orchestrator/pkg/sandbox/rootfs/nbd.go @@ -69,14 +69,15 @@ func (o *NBDProvider) Start(ctx context.Context) error { return o.ready.SetValue(nbd.GetDevicePath(deviceIndex)) } -func (o *NBDProvider) ExportDiff( +// ejectAndStopSandbox detaches the writable cache from the overlay, stops the +// sandbox and waits for the overlay device to be released, returning the ejected +// (now standalone, frozen) cache. The caller owns the returned cache and must +// Close it. Shared by the synchronous ExportDiff and the deferred +// PrepareExportDiff. +func (o *NBDProvider) ejectAndStopSandbox( ctx context.Context, - out *os.File, closeSandbox func(ctx context.Context) error, -) (*header.DiffMetadata, error) { - ctx, span := tracer.Start(ctx, "cow-export") - defer span.End() - +) (*block.Cache, error) { cache, err := o.overlay.EjectCache() if err != nil { return nil, fmt.Errorf("error ejecting cache: %w", err) @@ -104,6 +105,22 @@ func (o *NBDProvider) ExportDiff( } telemetry.ReportEvent(ctx, "sandbox stopped") + return cache, nil +} + +func (o *NBDProvider) ExportDiff( + ctx context.Context, + out *os.File, + closeSandbox func(ctx context.Context) error, +) (*header.DiffMetadata, error) { + ctx, span := tracer.Start(ctx, "cow-export") + defer span.End() + + cache, err := o.ejectAndStopSandbox(ctx, closeSandbox) + if err != nil { + return nil, err + } + m, err := cache.ExportToDiff(ctx, out) if err != nil { // Close the cache to avoid leaking the mmaped memory. Log an error @@ -126,6 +143,20 @@ func (o *NBDProvider) ExportDiff( return m, nil } +// PrepareExportDiff ejects the writable cache, stops the sandbox and waits for +// the overlay device to be released, then returns the frozen ejected cache +// WITHOUT reflinking it. The caller reflinks it into a diff in the background and +// Closes it, so a pause returns without paying the reflink stall. +func (o *NBDProvider) PrepareExportDiff( + ctx context.Context, + closeSandbox func(ctx context.Context) error, +) (*block.Cache, error) { + ctx, span := tracer.Start(ctx, "cow-export-prepare") + defer span.End() + + return o.ejectAndStopSandbox(ctx, closeSandbox) +} + func (o *NBDProvider) Close(ctx context.Context) error { ctx, span := tracer.Start(ctx, "cow-close") defer span.End() diff --git a/packages/orchestrator/pkg/sandbox/rootfs/rootfs.go b/packages/orchestrator/pkg/sandbox/rootfs/rootfs.go index 8eb67de833..887aaa5878 100644 --- a/packages/orchestrator/pkg/sandbox/rootfs/rootfs.go +++ b/packages/orchestrator/pkg/sandbox/rootfs/rootfs.go @@ -4,6 +4,7 @@ package rootfs import ( "context" + "errors" "fmt" "os" "syscall" @@ -13,17 +14,27 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/zap" + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block" "github.com/e2b-dev/infra/packages/shared/pkg/logger" "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" ) var tracer = otel.Tracer("github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/rootfs") +// ErrDeferredExportNotSupported is returned by PrepareExportDiff on providers +// that can't defer the rootfs export (e.g. DirectProvider). Callers use it to +// fall back to the synchronous ExportDiff instead of failing the pause. +var ErrDeferredExportNotSupported = errors.New("deferred rootfs export not supported by this provider") + type Provider interface { Start(ctx context.Context) error Close(ctx context.Context) error Path() (string, error) ExportDiff(ctx context.Context, out *os.File, closeSandbox func(context.Context) error) (*header.DiffMetadata, error) + // PrepareExportDiff ejects the writable cache and stops the sandbox, returning + // the frozen ejected cache without reflinking it, so the caller can seal it + // into a diff in the background. Only the NBD provider supports it. + PrepareExportDiff(ctx context.Context, closeSandbox func(context.Context) error) (*block.Cache, error) } // flush flushes the data to the operating system's buffer. From d545ded0d46b8d7f442bf453a136bec2464d347b Mon Sep 17 00:00:00 2001 From: Babis Chalios Date: Tue, 21 Jul 2026 10:26:12 +0200 Subject: [PATCH 2/3] feat(pause): defer rootfs export off the pause critical path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sealing the rootfs diff is a reflink (copy_file_range on XFS) that forces a synchronous host->NVMe writeback of the changed blocks — the stall that makes pause() scale with how much the guest wrote. Move it off the critical path for the suspend path: eject the cache + stop the sandbox, hand back a deferred diff, and reflink it in the background, so pause() returns without the writeback. - WithDeferredRootfsExport pause option + setupDeferredRootfsExport / runDeferredRootfsExport / sealCacheToDiff in Pause: build the header synchronously from the frozen cache's DiffMetadata, resolve the deferred diff from a background reflink. The async upload waits on the deferred diff, so graceful shutdown is gated via the server's upload WaitGroup. - Gated by the defer-rootfs-export flag (default off), and only the Pause (suspend) handler opts in — Checkpoint resumes a fresh sandbox from the new build immediately, so its diff must be materialized synchronously. - Falls back to the synchronous export when off or on a non-NBD provider. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Babis Chalios --- docs/ARCHITECTURE.md | 8 + .../pkg/sandbox/build/deferred_diff.go | 8 + .../pkg/sandbox/build/local_diff.go | 14 +- .../pkg/sandbox/build/local_diff_test.go | 36 +++ .../pkg/sandbox/build_upload_v3.go | 24 +- .../pkg/sandbox/build_upload_v4.go | 14 +- .../pkg/sandbox/deferred_export_test.go | 178 +++++++++++ packages/orchestrator/pkg/sandbox/sandbox.go | 282 ++++++++++++++++-- .../pkg/server/prefetch_harvest.go | 34 ++- packages/orchestrator/pkg/server/sandboxes.go | 22 +- .../orchestrator/pkg/server/upload_retry.go | 2 + .../pkg/server/upload_retry_test.go | 2 + packages/shared/pkg/featureflags/flags.go | 9 + packages/shared/pkg/telemetry/meters.go | 14 +- 14 files changed, 604 insertions(+), 43 deletions(-) create mode 100644 packages/orchestrator/pkg/sandbox/build/local_diff_test.go create mode 100644 packages/orchestrator/pkg/sandbox/deferred_export_test.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0c0c0e60e8..7d665dc01f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -312,6 +312,14 @@ sequenceDiagram orchestrator pauses the VM, snapshots it, diffs memory (dirty-page tracking) and rootfs (COW cache) against the template, caches the snapshot locally, and uploads asynchronously to object storage (with a retry budget). The sandbox leaves the Redis catalog. + - **Deferred rootfs export** (gated by the `deferred-rootfs-export` flag in + `packages/shared/pkg/featureflags`): instead of diffing the rootfs on the pause critical + path, the orchestrator ejects the writable COW cache during pause and returns, then seals it + into the rootfs diff (reflink) in the background. This moves the rootfs-diff latency off the + pause, but the local snapshot's rootfs body isn't materialized until the seal finishes, so the + async upload — and any origin-node resume/prefetch that reads the rootfs diff — waits on the + seal. A seal failure is permanent (it never re-runs), so the upload fails fast rather than + retrying. - **Resume**: same path as creation, but placement prefers the **origin node** — if the snapshot is still in its local cache, resume avoids any object-storage reads. `Checkpoint` is a pause+resume in place used to persist state while keeping the sandbox running. diff --git a/packages/orchestrator/pkg/sandbox/build/deferred_diff.go b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go index 91392b2853..74a7138e26 100644 --- a/packages/orchestrator/pkg/sandbox/build/deferred_diff.go +++ b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go @@ -4,11 +4,19 @@ package build import ( "context" + "errors" "github.com/e2b-dev/infra/packages/shared/pkg/storage" "github.com/e2b-dev/infra/packages/shared/pkg/utils" ) +// ErrDeferredSealFailed marks a deferred rootfs seal that failed. The seal runs +// exactly once, so the failure is permanent: the promise stays settled with this +// error and every subsequent data-bearing method on the diff returns it. Callers +// such as the pause-upload retry loop match on it to stop retrying a diff that +// can never materialize (rather than burning the whole retry budget on it). +var ErrDeferredSealFailed = errors.New("deferred rootfs seal failed") + // deferredDiff is a Diff whose backing data is produced asynchronously. It is // returned synchronously from a pause that seals the rootfs in the background: // the cache key and block size are known up front (so DiffStore.Add and the diff --git a/packages/orchestrator/pkg/sandbox/build/local_diff.go b/packages/orchestrator/pkg/sandbox/build/local_diff.go index 8417b86265..ee02a04af9 100644 --- a/packages/orchestrator/pkg/sandbox/build/local_diff.go +++ b/packages/orchestrator/pkg/sandbox/build/local_diff.go @@ -53,9 +53,21 @@ func (f *LocalDiffFile) Close() error { func (f *LocalDiffFile) CloseToDiff( blockSize int64, -) (Diff, error) { +) (d Diff, e error) { defer f.File.Close() + // On any failure to produce a usable Diff (e.g. an fsync/stat error after the + // bytes were written), remove the partial cache file. Nothing registers it in + // the DiffStore, so otherwise it orphans in the cache dir until process restart + // — disk-pressure eviction can't reclaim a file it doesn't know about. + defer func() { + if e != nil { + if rmErr := os.Remove(f.cachePath); rmErr != nil && !os.IsNotExist(rmErr) { + e = fmt.Errorf("%w; remove partial diff file: %w", e, rmErr) + } + } + }() + err := f.File.Sync() if err != nil { return nil, fmt.Errorf("failed to sync file: %w", err) diff --git a/packages/orchestrator/pkg/sandbox/build/local_diff_test.go b/packages/orchestrator/pkg/sandbox/build/local_diff_test.go new file mode 100644 index 0000000000..b1631b6ab3 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/build/local_diff_test.go @@ -0,0 +1,36 @@ +//go:build linux + +package build + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestLocalDiffFileCloseToDiffRemovesPartialOnError verifies CloseToDiff does not +// leave the partial cache file behind when it fails to produce a usable Diff. +// Nothing registers such a file in the DiffStore, so a leaked orphan would sit in +// the cache dir unreclaimable by disk-pressure eviction until process restart. +func TestLocalDiffFileCloseToDiffRemovesPartialOnError(t *testing.T) { + t.Parallel() + + f, err := NewLocalDiffFile(t.TempDir(), "build-test-id", Rootfs) + require.NoError(t, err) + + // Non-empty so we're past the zero-size NoDiff branch and into materialization. + _, err = f.File.WriteAt(make([]byte, 128), 0) + require.NoError(t, err) + + cachePath := f.cachePath + require.FileExists(t, cachePath) + + // Force a materialization failure: closing the fd makes the Sync inside + // CloseToDiff fail, driving the error path. + require.NoError(t, f.File.Close()) + + diff, err := f.CloseToDiff(blockSize) + require.Error(t, err) + require.Nil(t, diff) + require.NoFileExists(t, cachePath, "partial diff file must be removed on materialization failure") +} diff --git a/packages/orchestrator/pkg/sandbox/build_upload_v3.go b/packages/orchestrator/pkg/sandbox/build_upload_v3.go index 0187a5d697..e6c2dfb0d5 100644 --- a/packages/orchestrator/pkg/sandbox/build_upload_v3.go +++ b/packages/orchestrator/pkg/sandbox/build_upload_v3.go @@ -20,11 +20,6 @@ func (u *Upload) runV3(ctx context.Context) error { return fmt.Errorf("error getting memfile diff path: %w", err) } - rootfsPath, err := u.snap.RootfsDiff.CachePath(ctx) - if err != nil { - return fmt.Errorf("error getting rootfs diff path: %w", err) - } - eg, egCtx := errgroup.WithContext(ctx) eg.Go(func() error { @@ -48,6 +43,17 @@ func (u *Upload) runV3(ctx context.Context) error { return nil } + // Gate the header publish on the rootfs seal (deferred export): the rootfs + // header resolves synchronously at pause time, so without this it could be + // finalized to storage while the background seal is still running — and if + // the seal then fails (it does not retry) storage would keep a completed + // header with no body. The body goroutine below waits on the same seal, so + // both rootfs uploads are gated on it while memfile/snapfile/metadata still + // overlap it. No-op on the synchronous path (CachePath returns immediately). + if _, err := u.snap.RootfsDiff.CachePath(egCtx); err != nil { + return fmt.Errorf("error getting rootfs diff path: %w", err) + } + return storeHeaderWithMetrics(egCtx, u.store, u.paths.RootfsHeader(), uploadFileRootfsHeader, finalizeV3(h), storage.WithMetadata(u.objectMetadata)) }) @@ -77,6 +83,14 @@ func (u *Upload) runV3(ctx context.Context) error { }) eg.Go(func() error { + // Resolve the rootfs diff path inside the group: with deferred rootfs + // export it blocks on the background seal, so doing it here lets the + // memfile/snapfile/metadata uploads overlap the reflink instead of + // waiting behind it. + rootfsPath, err := u.snap.RootfsDiff.CachePath(egCtx) + if err != nil { + return fmt.Errorf("error getting rootfs diff path: %w", err) + } if rootfsPath == "" { return nil } diff --git a/packages/orchestrator/pkg/sandbox/build_upload_v4.go b/packages/orchestrator/pkg/sandbox/build_upload_v4.go index e70dea7283..6f9caa074b 100644 --- a/packages/orchestrator/pkg/sandbox/build_upload_v4.go +++ b/packages/orchestrator/pkg/sandbox/build_upload_v4.go @@ -21,11 +21,6 @@ func (u *Upload) runV4(ctx context.Context) error { return fmt.Errorf("memfile diff path: %w", err) } - rootfsSrc, err := u.snap.RootfsDiff.CachePath(ctx) - if err != nil { - return fmt.Errorf("rootfs diff path: %w", err) - } - eg, ctx := errgroup.WithContext(ctx) eg.Go(func() error { @@ -41,6 +36,15 @@ func (u *Upload) runV4(ctx context.Context) error { }) eg.Go(func() error { + // Resolve the rootfs diff path inside the group: with deferred rootfs + // export it blocks on the background seal, so doing it here lets the + // memfile/snapfile/metadata uploads overlap the reflink instead of + // waiting behind it. + rootfsSrc, err := u.snap.RootfsDiff.CachePath(ctx) + if err != nil { + return fmt.Errorf("rootfs diff path: %w", err) + } + h, err := u.snap.RootfsDiffHeader.WaitWithContext(ctx) if err != nil { return fmt.Errorf("wait rootfs diff header: %w", err) diff --git a/packages/orchestrator/pkg/sandbox/deferred_export_test.go b/packages/orchestrator/pkg/sandbox/deferred_export_test.go new file mode 100644 index 0000000000..be65a28111 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/deferred_export_test.go @@ -0,0 +1,178 @@ +//go:build linux + +package sandbox + +import ( + "context" + "errors" + "os" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/cfg" + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block" + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/build" + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/rootfs" + "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" + "github.com/e2b-dev/infra/packages/shared/pkg/utils" +) + +// failingRootfs embeds rootfs.Provider (nil) so it satisfies the interface while +// only overriding PrepareExportDiff — the single method the error branch under +// test exercises. Any other call would nil-panic, which is the intended guard. +type failingRootfs struct { + rootfs.Provider + + err error +} + +func (f failingRootfs) PrepareExportDiff(context.Context, func(context.Context) error) (*block.Cache, error) { + return nil, f.err +} + +// stubRootfs hands back a pre-built ejected cache from PrepareExportDiff, standing +// in for the NBD provider once it has frozen and ejected the writable COW cache. +type stubRootfs struct { + rootfs.Provider + + cache *block.Cache +} + +func (s stubRootfs) PrepareExportDiff(context.Context, func(context.Context) error) (*block.Cache, error) { + return s.cache, nil +} + +// TestRunDeferredRootfsExport verifies the background lifecycle of the deferred +// rootfs export: the frozen (ejected) cache is reflinked into the deferred diff, +// the diff resolves with the sealed bytes, and the cache is closed — with no +// overlay/provider interaction (the sandbox is already stopped). +func TestRunDeferredRootfsExport(t *testing.T) { + t.Parallel() + + blockSize := int64(header.PageSize) + numBlocks := int64(3) + size := blockSize * numBlocks + + // A standalone frozen cache with block 1 dirtied (stands in for the ejected + // COW cache after the sandbox is stopped). + sealCache, err := block.NewCache(size, blockSize, t.TempDir()+"/ejected", false) + require.NoError(t, err) + blockData := make([]byte, blockSize) + for i := range blockData { + blockData[i] = 0x5C + } + _, err = sealCache.WriteAt(blockData, blockSize) + require.NoError(t, err) + + s := &Sandbox{ + Resources: &Resources{}, + config: cfg.BuilderConfig{DefaultCacheDir: t.TempDir()}, + } + + buildID := uuid.New() + diffPromise := utils.NewSetOnce[build.Diff]() + + // The setup path captures the diff metadata up front and hands it to the + // background seal (so the exported bytes match the header from the same read). + meta, err := sealCache.DiffMetadata() + require.NoError(t, err) + + s.runDeferredRootfsExport(t.Context(), sealCache, buildID, blockSize, meta, diffPromise) + + diff, err := diffPromise.Result() + require.NoError(t, err) + path, err := diff.CachePath(t.Context()) + require.NoError(t, err) + got, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, blockData, got, "deferred diff must contain the dirtied block") + require.NoError(t, diff.Close()) +} + +// TestSetupDeferredRootfsExport_PrepareError verifies the earliest failure branch: +// when ejecting the writable cache fails, setup propagates the error and returns +// no diff/header/seal — so Pause falls back to the synchronous export rather than +// registering a deferred diff whose seal would never run. +func TestSetupDeferredRootfsExport_PrepareError(t *testing.T) { + t.Parallel() + + s := &Sandbox{ + Resources: &Resources{rootfs: failingRootfs{err: errors.New("eject failed")}}, + config: cfg.BuilderConfig{DefaultCacheDir: t.TempDir()}, + } + + cleanup := NewCleanup() + diff, hdr, startSeal, err := s.setupDeferredRootfsExport(t.Context(), uuid.New(), nil, cleanup) + require.Error(t, err) + require.ErrorContains(t, err, "eject failed") + require.Nil(t, diff) + require.Nil(t, hdr) + require.Nil(t, startSeal) +} + +// TestSetupDeferredRootfsExport_AbortCleanupOrder pins the cleanup ordering on the +// abort path (Pause fails before startSeal runs, so `started` stays false and the +// registered abort cleanups fire). The promise-poisoning cleanup MUST run before +// deferredDiff.Close — Close waits on the seal promise with no context, so if the +// order regressed it would block forever on a seal that will never run. The test +// therefore asserts the whole cleanup stack completes promptly (a deadlock is the +// failure it guards against) and that the deferred diff resolves to the abort +// error rather than dangling. +func TestSetupDeferredRootfsExport_AbortCleanupOrder(t *testing.T) { + t.Parallel() + + blockSize := int64(header.PageSize) + numBlocks := int64(3) + size := blockSize * numBlocks + + // A frozen ejected cache with one dirtied block, so setup takes the deferred + // (non-empty diff) branch that registers the abort cleanups. + ejected, err := block.NewCache(size, blockSize, t.TempDir()+"/ejected", false) + require.NoError(t, err) + dirty := make([]byte, blockSize) + for i := range dirty { + dirty[i] = 0xAB + } + _, err = ejected.WriteAt(dirty, blockSize) + require.NoError(t, err) + + originalHeader, err := header.NewHeader( + header.NewTemplateMetadata(uuid.New(), uint64(blockSize), uint64(size)), + nil, + ) + require.NoError(t, err) + + s := &Sandbox{ + Resources: &Resources{rootfs: stubRootfs{cache: ejected}}, + config: cfg.BuilderConfig{DefaultCacheDir: t.TempDir()}, + } + + cleanup := NewCleanup() + rootfsDiff, hdr, startSeal, err := s.setupDeferredRootfsExport(t.Context(), uuid.New(), originalHeader, cleanup) + require.NoError(t, err) + require.NotNil(t, rootfsDiff) + require.NotNil(t, hdr) + require.NotNil(t, startSeal) + + // Do NOT call startSeal: simulate Pause aborting before the seal is handed off. + // Running the cleanups must not deadlock — guard with a generous timeout so a + // wrong ordering surfaces as a clear failure instead of a hung test. + done := make(chan error, 1) + go func() { done <- cleanup.Run(t.Context()) }() + select { + case runErr := <-done: + require.NoError(t, runErr) + case <-time.After(10 * time.Second): + t.Fatal("cleanup deadlocked: deferred diff Close ran before the promise was poisoned") + } + + // The promise was poisoned, so the diff's data methods resolve (to the abort + // error) without blocking instead of waiting on a seal that never runs. The + // specific message also confirms setup took the deferred branch (a NoDiff + // fallback would register no abort cleanup and never poison anything). + _, err = rootfsDiff.FileSize(t.Context()) + require.ErrorContains(t, err, "pause aborted before deferred rootfs export ran") +} diff --git a/packages/orchestrator/pkg/sandbox/sandbox.go b/packages/orchestrator/pkg/sandbox/sandbox.go index 6f663ba550..53acc8cba3 100644 --- a/packages/orchestrator/pkg/sandbox/sandbox.go +++ b/packages/orchestrator/pkg/sandbox/sandbox.go @@ -55,6 +55,7 @@ var ( processMemoryDurationHistogram = utils.Must(telemetry.GetHistogram(meter, telemetry.SnapshotProcessMemoryDurationName)) processRootfsDurationHistogram = utils.Must(telemetry.GetHistogram(meter, telemetry.SnapshotProcessRootfsDurationName)) + rootfsSealDurationHistogram = utils.Must(telemetry.GetHistogram(meter, telemetry.SnapshotRootfsSealDurationName)) uffdStartupPagesHistogram = utils.Must(telemetry.GetHistogram(meter, telemetry.UffdStartupPagesHistogramName)) uffdStartupSourcePagesHistogram = utils.Must(telemetry.GetHistogram(meter, telemetry.UffdStartupSourcePagesHistogramName)) @@ -1403,6 +1404,7 @@ func (s *Sandbox) Shutdown(ctx context.Context) error { type pauseOptions struct { filesystemSnapshot bool + deferRootfsExport bool } type PauseOption func(*pauseOptions) @@ -1415,6 +1417,15 @@ func WithFilesystemSnapshot() PauseOption { return func(o *pauseOptions) { o.filesystemSnapshot = true } } +// WithDeferredRootfsExport seals the rootfs diff off the critical path: the +// sandbox is ejected/stopped and the diff is reflinked in the background, so the +// pause returns without the host->NVMe writeback stall. Only safe when nothing +// reads the diff before the background seal completes — i.e. the suspend (pause) +// path, not a resume-fresh checkpoint. +func WithDeferredRootfsExport() PauseOption { + return func(o *pauseOptions) { o.deferRootfsExport = true } +} + // Pause creates a snapshot of the sandbox. // // Currently the memory snapshotting works like this: @@ -1560,21 +1571,24 @@ func (s *Sandbox) Pause( // harmless and keeps the cleanup ordering identical to the memory path. cleanup.AddNoContext(ctx, mem.Diff.Close) - rootfsDiff, rootfsHeader, err := pauseProcessRootfs( + var ( + rootfsDiff build.Diff + rootfsHeader *header.Header + // startSeal, when non-nil, reflinks the ejected cache into the diff in the + // background; the caller invokes it after the metadata is written. + startSeal func(context.Context) + ) + + rootfsDiff, rootfsHeader, startSeal, err = s.processRootfsSnapshot( ctx, buildID, originalRootfs.Header(), - &RootfsDiffCreator{ - rootfs: s.rootfs, - closeHook: s.Close, - }, - s.config.DefaultCacheDir, - pauseOpts.filesystemSnapshot, + &pauseOpts, + cleanup, ) if err != nil { return nil, fmt.Errorf("error while post processing: %w", err) } - cleanup.AddNoContext(ctx, rootfsDiff.Close) rootfsDiffHeader := NewResolvedDiffHeader(rootfsHeader) // Derive scheduling metadata synchronously so Pause never blocks on the @@ -1582,9 +1596,10 @@ func (s *Sandbox) Pause( // parent header plus the new build, whose exact bytes aren't known yet, so // we pass the pre-dedup dirty size as an upper bound. It is block-granular // (dirty blocks * diff block size) and counts pages before dedup drops the - // base-identical ones, so it over-estimates. The rootfs copy is synchronous - // today, so its new header carries the exact rootfs chain and bytes; if it - // ever becomes async, switch it to the parent plus a dirty proxy like memfile. + // base-identical ones, so it over-estimates. The rootfs header is known + // synchronously even with deferred export — the diff metadata (chain + exact + // bytes) is read up front at pause time and only the reflink seal is deferred — + // so the rootfs half of the scheduling metadata is exact. // mem.header is nil for a filesystem-only pause → rootfs-only metadata. schedulingMetadata := scheduling.FromHeaders(buildID, mem.header, rootfsHeader, mem.newBytes) @@ -1596,6 +1611,12 @@ func (s *Sandbox) Pause( return nil, err } + // The sandbox is stopped and the cache ejected; reflink it to the diff in the + // background so the pause returned without paying the writeback stall. + if startSeal != nil { + startSeal(context.WithoutCancel(ctx)) + } + return &Snapshot{ Snapfile: snapfile, Metafile: metadataFileLink, @@ -1869,29 +1890,79 @@ func buildProvisionalMemfile( return provisionalHeader, provisionalDiff, dc.MarkSwapped } -func pauseProcessRootfs( +func (s *Sandbox) processRootfsSnapshot( ctx context.Context, - buildId uuid.UUID, + buildID uuid.UUID, originalHeader *header.Header, - diffCreator DiffCreator, - cacheDir string, - filesystemOnly bool, -) (d build.Diff, h *header.Header, e error) { + pauseOpts *pauseOptions, + cleanup *Cleanup, +) (d build.Diff, h *header.Header, startSeal func(context.Context), e error) { ctx, span := tracer.Start(ctx, "process-rootfs") defer span.End() - // Duration of the rootfs export+diff, split by fs_only (runs for both pause // kinds) so the fs-only pause latency can be decomposed into quiesce + rootfs. + // This is the pause CRITICAL-PATH rootfs cost: the full export for the + // synchronous path, but only the eject/setup for the deferred path — the + // background reflink seal (runDeferredRootfsExport) is intentionally excluded. + // + // The `deferred` attribute records which of those two populations a sample + // belongs to: without it the (much smaller) deferred setup-only timings would + // be indistinguishable from full synchronous exports. It reflects the path + // actually taken — the fall-through below flips it off when a provider can't + // defer. `success` here is the critical-path outcome only; on the deferred + // path the actual export success/failure is recorded by the seal metric + // (rootfsSealDurationHistogram), so a deferred success=true is setup-only. start := time.Now() defer func() { processRootfsDurationHistogram.Record(ctx, time.Since(start).Milliseconds(), metric.WithAttributes( - attribute.Bool("fs_only", filesystemOnly), + attribute.Bool("fs_only", pauseOpts.filesystemSnapshot), + attribute.Bool("deferred", pauseOpts.deferRootfsExport), attribute.Bool("success", e == nil), )) }() - rootfsDiffFile, err := build.NewLocalDiffFile(cacheDir, buildId.String(), build.Rootfs) + if pauseOpts.deferRootfsExport { + rootfsDiff, rootfsHeader, startSeal, err := s.setupDeferredRootfsExport(ctx, buildID, originalHeader, cleanup) + switch { + case errors.Is(err, rootfs.ErrDeferredExportNotSupported): + // The provider (e.g. DirectProvider) can't defer; fall through to the + // synchronous export below. Safe because PrepareExportDiff returns this + // sentinel before ejecting/stopping anything. + pauseOpts.deferRootfsExport = false + case err != nil: + return nil, nil, nil, fmt.Errorf("deferred rootfs export setup failed: %w", err) + default: + return rootfsDiff, rootfsHeader, startSeal, nil + } + } + + rootfsDiff, rootfsHeader, err := pauseProcessRootfs( + ctx, + buildID, + originalHeader, + &RootfsDiffCreator{ + rootfs: s.rootfs, + closeHook: s.Close, + }, + s.config.DefaultCacheDir, + ) + if err != nil { + return nil, nil, nil, fmt.Errorf("synchronous rootfs export failed: %w", err) + } + cleanup.AddNoContext(ctx, rootfsDiff.Close) + + return rootfsDiff, rootfsHeader, nil, nil +} + +func pauseProcessRootfs( + ctx context.Context, + buildID uuid.UUID, + originalHeader *header.Header, + diffCreator DiffCreator, + cacheDir string, +) (d build.Diff, h *header.Header, e error) { + rootfsDiffFile, err := build.NewLocalDiffFile(cacheDir, buildID.String(), build.Rootfs) if err != nil { return nil, nil, fmt.Errorf("failed to create rootfs diff: %w", err) } @@ -1911,7 +1982,7 @@ func pauseProcessRootfs( } telemetry.ReportEvent(ctx, "converted rootfs diff file to local diff") - rootfsHeader, err := rootfsDiffMetadata.ToDiffHeader(ctx, originalHeader, buildId) + rootfsHeader, err := rootfsDiffMetadata.ToDiffHeader(ctx, originalHeader, buildID) if err != nil { err = errors.Join(err, rootfsDiff.Close()) @@ -1921,6 +1992,175 @@ func pauseProcessRootfs( return rootfsDiff, rootfsHeader, nil } +// setupDeferredRootfsExport ejects the writable cache and stops the sandbox +// (destroy path), then prepares the deferred rootfs diff + header from the frozen +// ejected cache. It returns a startSeal closure that reflinks the cache into the +// diff in the background, so the pause returns without paying the reflink stall. +// Only safe on the suspend path, where nothing reads the diff before the seal +// completes. +func (s *Sandbox) setupDeferredRootfsExport( + ctx context.Context, + buildID uuid.UUID, + originalHeader *header.Header, + cleanup *Cleanup, +) (d build.Diff, h *header.Header, startSeal func(context.Context), e error) { + sealCache, err := s.rootfs.PrepareExportDiff(ctx, s.Close) + if err != nil { + return nil, nil, nil, err + } + + diffMetadata, err := sealCache.DiffMetadata() + if err != nil { + return nil, nil, nil, errors.Join(fmt.Errorf("reading ejected cache metadata: %w", err), sealCache.Close()) + } + // Emit the same rootfs size/ratio metrics the synchronous pauseProcessRootfs + // path does, so deferring the export doesn't blind the snapshot dashboards. + recordSnapshotDiff(ctx, "rootfs", diffMetadata, originalHeader) + + rootfsHeader, err := diffMetadata.ToDiffHeader(ctx, originalHeader, buildID) + if err != nil { + return nil, nil, nil, errors.Join(fmt.Errorf("building rootfs diff header: %w", err), sealCache.Close()) + } + + // No dirty filesystem blocks: the seal would produce an empty diff, i.e. the + // same *NoDiff the synchronous path returns. Return NoDiff directly (and skip + // the background seal — nothing to reflink) so AddSnapshot omits it from the + // DiffStore and peer LookupDiff keeps returning ErrNotAvailable rather than an + // entry whose Slice/Size yield NoDiffError. Close the ejected cache now since + // no seal will own it. + if diffMetadata.Dirty.IsEmpty() { + if err := sealCache.Close(); err != nil { + return nil, nil, nil, fmt.Errorf("closing empty ejected cache: %w", err) + } + + return &build.NoDiff{}, rootfsHeader, func(context.Context) {}, nil + } + + blockSize := int64(originalHeader.Metadata.BlockSize) + diffPromise := utils.NewSetOnce[build.Diff]() + rootfsDiff := build.NewDeferredDiff(build.GetDiffStoreKey(buildID.String(), build.Rootfs), blockSize, diffPromise) + + // The ejected cache and the deferred diff's promise are both owned by the + // background seal once it starts. If Pause aborts before startSeal runs (e.g. + // m.ToFile fails), the goroutine never runs, so on that path we close the + // cache here (else its mmap + backing file leak) and poison the promise (else + // the deferred diff's Close and any waiter block forever on a seal that will + // never resolve). Both are guarded by `started`: once the seal owns them, this + // cleanup is a no-op, so we never double-close the cache nor race a spurious + // SetError against the seal's SetValue (which would drop the sealed diff). + // atomic because startSeal writes it on the pause goroutine and these cleanups + // read it on the cleanup goroutine. + var started atomic.Bool + cleanup.Add(ctx, func(context.Context) error { + if started.Load() { + return nil + } + + return sealCache.Close() + }) + + // LIFO: the abort resolver (added last) runs before rootfsDiff.Close, so on the + // error path the deferred diff's Close never blocks on a seal that won't run. + cleanup.AddNoContext(ctx, rootfsDiff.Close) + cleanup.Add(ctx, func(context.Context) error { + if started.Load() { + return nil + } + _ = diffPromise.SetError(errors.New("pause aborted before deferred rootfs export ran")) + + return nil + }) + + startSeal = func(sealCtx context.Context) { + started.Store(true) + go s.runDeferredRootfsExport(sealCtx, sealCache, buildID, blockSize, diffMetadata, diffPromise) + } + + return rootfsDiff, rootfsHeader, startSeal, nil +} + +// runDeferredRootfsExport reflinks the ejected cache into the rootfs diff and +// closes the cache. The sandbox is already stopped, so there is nothing to fold +// or serialize; the upload waits on the deferred diff, gating shutdown via the +// server's upload WaitGroup. +func (s *Sandbox) runDeferredRootfsExport( + ctx context.Context, + sealCache *block.Cache, + buildID uuid.UUID, + blockSize int64, + meta *header.DiffMetadata, + diffPromise *utils.SetOnce[build.Diff], +) { + ctx, span := tracer.Start(ctx, "deferred-rootfs-export") + defer span.End() + + // Record the background reflink seal latency separately from the + // critical-path process_rootfs.duration, so the deferred export's off-path + // cost stays visible. + start := time.Now() + err := s.sealCacheToDiff(ctx, sealCache, buildID, blockSize, meta, diffPromise) + rootfsSealDurationHistogram.Record(ctx, time.Since(start).Milliseconds(), + metric.WithAttributes(attribute.Bool("success", err == nil))) + if err != nil { + logger.L().Error(ctx, "deferred rootfs export failed", zap.Error(err)) + } else { + telemetry.ReportEvent(ctx, "rootfs diff sealed (deferred)") + } + + // The sandbox is torn down; the ejected cache is ours to close regardless of + // the export outcome. + if err := sealCache.Close(); err != nil { + logger.L().Warn(ctx, "closing ejected rootfs cache", zap.Error(err)) + } +} + +// sealCacheToDiff reflinks the frozen cache into a fresh local diff file and +// resolves diffPromise with the materialized diff. +func (s *Sandbox) sealCacheToDiff( + ctx context.Context, + sealCache *block.Cache, + buildID uuid.UUID, + blockSize int64, + meta *header.DiffMetadata, + diffPromise *utils.SetOnce[build.Diff], +) error { + diffFile, err := build.NewLocalDiffFile(s.config.DefaultCacheDir, buildID.String(), build.Rootfs) + if err != nil { + return s.failRootfsSeal(diffPromise, fmt.Errorf("create rootfs diff file: %w", err)) + } + + // Export using the metadata captured at setup so the sealed data matches the + // header built from the same bitmap read (rather than re-reading the tracker). + if _, err := sealCache.ExportToDiffWithMetadata(ctx, diffFile.File, meta); err != nil { + return s.failRootfsSeal(diffPromise, errors.Join(fmt.Errorf("export rootfs diff: %w", err), diffFile.Close())) + } + telemetry.ReportEvent(ctx, "exported rootfs") + + diff, err := diffFile.CloseToDiff(blockSize) + if err != nil { + return s.failRootfsSeal(diffPromise, fmt.Errorf("materialize rootfs diff: %w", err)) + } + + if err := diffPromise.SetValue(diff); err != nil { + // The promise was already settled (pause aborted); drop the diff so its + // cache file doesn't leak. + return errors.Join(err, diff.Close()) + } + + return nil +} + +// failRootfsSeal settles the deferred diff with err and returns it. +func (s *Sandbox) failRootfsSeal(diffPromise *utils.SetOnce[build.Diff], err error) error { + // Tag the failure with ErrDeferredSealFailed so the upload retry loop can tell + // this permanent, one-shot seal failure apart from transient upload errors and + // stop retrying (the seal never re-runs, so the diff can never materialize). + sealErr := fmt.Errorf("%w: %w", build.ErrDeferredSealFailed, err) + _ = diffPromise.SetError(sealErr) + + return sealErr +} + // createCgroup creates a cgroup for sandbox resource accounting. // The caller is responsible for registering cleanup to remove the cgroup. // diff --git a/packages/orchestrator/pkg/server/prefetch_harvest.go b/packages/orchestrator/pkg/server/prefetch_harvest.go index 55b5a8d802..c6edc18e99 100644 --- a/packages/orchestrator/pkg/server/prefetch_harvest.go +++ b/packages/orchestrator/pkg/server/prefetch_harvest.go @@ -52,10 +52,11 @@ const ( ) var ( - harvestMeter = otel.Meter("github.com/e2b-dev/infra/packages/orchestrator/pkg/server") - harvestAttemptsCounter = utils.Must(telemetry.GetCounter(harvestMeter, telemetry.PauseResumePrefetchHarvestAttempts)) - harvestDurationHistogram = utils.Must(telemetry.GetHistogram(harvestMeter, telemetry.PauseResumePrefetchHarvestDurationName)) - harvestPagesHistogram = utils.Must(telemetry.GetHistogram(harvestMeter, telemetry.PauseResumePrefetchHarvestPagesName)) + harvestMeter = otel.Meter("github.com/e2b-dev/infra/packages/orchestrator/pkg/server") + harvestAttemptsCounter = utils.Must(telemetry.GetCounter(harvestMeter, telemetry.PauseResumePrefetchHarvestAttempts)) + harvestDurationHistogram = utils.Must(telemetry.GetHistogram(harvestMeter, telemetry.PauseResumePrefetchHarvestDurationName)) + harvestPagesHistogram = utils.Must(telemetry.GetHistogram(harvestMeter, telemetry.PauseResumePrefetchHarvestPagesName)) + sealWaitDurationHistogram = utils.Must(telemetry.GetHistogram(harvestMeter, telemetry.PauseResumePrefetchSealWaitDurationName)) ) // harvestResumer resumes the throwaway instance the harvest records its trace @@ -189,8 +190,31 @@ func (s *Server) harvestResumePrefetchAsync( attribute.Bool("consume", consume), ) + // With deferred rootfs export the just-paused snapshot's rootfs diff is + // sealed (reflinked) in the background, and the throwaway warm resume below + // reads the rootfs. Wait for the seal to finish here instead of letting the + // resume block on — and burn its budget against — the reflink. Returns + // immediately for the synchronous and NoDiff paths. If the seal fails, or + // the harvest deadline fires before it completes, skip the harvest (it is + // best-effort and must never touch a half-sealed snapshot). Record the wait + // as its own metric and start the harvest timer after it, so the reflink + // wait doesn't inflate the harvest-duration (slot-hold) histogram. + sealWaitStart := time.Now() + _, sealWaitErr := res.rootfsDiff.CachePath(hCtx) + sealWaitDurationHistogram.Record(hCtx, time.Since(sealWaitStart).Milliseconds()) + start := time.Now() - pages, outcome, err := harvester.run(hCtx, sbx, res.meta, res.upload, buildID, objectMetadata, consume) + var ( + pages int + outcome harvestOutcome + err error + ) + if sealWaitErr != nil { + outcome, err = harvestSkipped, fmt.Errorf("waiting for rootfs seal: %w", sealWaitErr) + } else { + pages, outcome, err = harvester.run(hCtx, sbx, res.meta, res.upload, buildID, objectMetadata, consume) + } + durationMs := time.Since(start).Milliseconds() resultAttr := metric.WithAttributes(attribute.String("result", string(outcome))) diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go index ecb9994f0a..a2eba0fe2e 100644 --- a/packages/orchestrator/pkg/server/sandboxes.go +++ b/packages/orchestrator/pkg/server/sandboxes.go @@ -25,6 +25,7 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox" + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/build" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/fc" sbxtemplate "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/template" buildenvd "github.com/e2b-dev/infra/packages/orchestrator/pkg/template/build/core/envd" @@ -748,8 +749,13 @@ func (s *Server) Pause(ctx context.Context, in *orchestrator.SandboxPauseRequest // Stop the old sandbox in background after we're done defer s.stopSandboxAsync(context.WithoutCancel(ctx), sbx) + // Defer the rootfs reflink off the pause critical path when enabled: pause is a + // suspend, so nothing reads the diff until a later resume (which waits on the + // upload anyway). NBD provider only; falls back to synchronous export otherwise. + deferRootfsExport := s.featureFlags.BoolFlag(ctx, featureflags.DeferRootfsExportFlag) + // Fire and forget - upload completes in the background - res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), map[string]string{storage.ObjectMetadataTemplateID: in.GetTemplateId()}, storage.ObjectOriginPause, in.GetFilesystemOnly()) + res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), map[string]string{storage.ObjectMetadataTemplateID: in.GetTemplateId()}, storage.ObjectOriginPause, in.GetFilesystemOnly(), deferRootfsExport) if err != nil { telemetry.ReportCriticalError(ctx, "error snapshotting sandbox", err, telemetry.WithSandboxID(in.GetSandboxId())) @@ -863,7 +869,9 @@ func (s *Server) Checkpoint(ctx context.Context, in *orchestrator.SandboxCheckpo // Checkpoint always takes a full memory snapshot; filesystem-only checkpoint // (resume-in-place would need to reboot) is not supported yet. - res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), in.GetMetadata(), storage.ObjectOriginSnapshotTemplate, false) + // Checkpoint resumes a fresh sandbox from the new build immediately, so the + // diff must be materialized synchronously — never defer the rootfs export here. + res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), in.GetMetadata(), storage.ObjectOriginSnapshotTemplate, false, false) if err != nil { telemetry.ReportCriticalError(ctx, "error snapshotting sandbox for checkpoint", err, telemetry.WithSandboxID(in.GetSandboxId())) @@ -1021,6 +1029,11 @@ type snapshotResult struct { schedulingMetadata *orchestrator.SchedulingMetadata upload *sandbox.Upload completeUpload func(ctx context.Context, uploadErr error) + // rootfsDiff is the snapshot's rootfs diff. With deferred export it is a + // promise-backed diff that resolves only once the background seal finishes, + // so the prefetch harvest waits on its CachePath before its throwaway resume + // (a warm resume reads the rootfs). Nil-safe callers only: always set here. + rootfsDiff build.Diff // objectMetadata is the storage object metadata the snapshot was uploaded // with. The prefetch harvest reuses it verbatim when re-uploading the // metadata object, so the two can never drift. @@ -1040,6 +1053,7 @@ func (s *Server) snapshotAndCacheSandbox( provenance map[string]string, buildOrigin storage.ObjectOrigin, filesystemOnly bool, + deferRootfsExport bool, ) (*snapshotResult, error) { meta, err := sbx.Template.Metadata() if err != nil { @@ -1056,6 +1070,9 @@ func (s *Server) snapshotAndCacheSandbox( if filesystemOnly { pauseOpts = append(pauseOpts, sandbox.WithFilesystemSnapshot()) } + if deferRootfsExport { + pauseOpts = append(pauseOpts, sandbox.WithDeferredRootfsExport()) + } snapshot, err := sbx.Pause(ctx, meta, sandbox.SnapshotUseCasePause, pauseOpts...) if err != nil { @@ -1131,6 +1148,7 @@ func (s *Server) snapshotAndCacheSandbox( completeUpload: completeUpload, objectMetadata: objectMetadata, filesystemOnly: filesystemOnly, + rootfsDiff: snapshot.RootfsDiff, }, nil } diff --git a/packages/orchestrator/pkg/server/upload_retry.go b/packages/orchestrator/pkg/server/upload_retry.go index dfcbd34cb9..111524a4ca 100644 --- a/packages/orchestrator/pkg/server/upload_retry.go +++ b/packages/orchestrator/pkg/server/upload_retry.go @@ -36,6 +36,8 @@ func isRetryableUploadErr(err error) bool { return false // source vanished; retry cannot recover it case errors.Is(err, context.Canceled): return false // parent cancelled (shutdown) + case errors.Is(err, build.ErrDeferredSealFailed): + return false // deferred rootfs seal ran once and failed; it never re-runs default: // Includes per-attempt context.DeadlineExceeded, GCS 401/503, rate // limiting, and unknown errors — all worth retrying within the budget. diff --git a/packages/orchestrator/pkg/server/upload_retry_test.go b/packages/orchestrator/pkg/server/upload_retry_test.go index 1d0ac73c5a..50a8906e35 100644 --- a/packages/orchestrator/pkg/server/upload_retry_test.go +++ b/packages/orchestrator/pkg/server/upload_retry_test.go @@ -26,6 +26,8 @@ func TestIsRetryableUploadErr(t *testing.T) { {"object not exist", storage.ErrObjectNotExist, false}, {"object not exist wrapped", fmt.Errorf("load: %w", storage.ErrObjectNotExist), false}, {"parent cancelled", context.Canceled, false}, + {"deferred seal failed", build.ErrDeferredSealFailed, false}, + {"deferred seal failed wrapped", fmt.Errorf("rootfs diff path: %w", build.ErrDeferredSealFailed), false}, {"per-attempt deadline", context.DeadlineExceeded, true}, {"gcs 503", errors.New("server error (503)"), true}, {"unknown", errors.New("boom"), true}, diff --git a/packages/shared/pkg/featureflags/flags.go b/packages/shared/pkg/featureflags/flags.go index c438edce04..7a22e6b4c4 100644 --- a/packages/shared/pkg/featureflags/flags.go +++ b/packages/shared/pkg/featureflags/flags.go @@ -186,6 +186,15 @@ var ( // of synchronous. Only safe to enable after PeerToPeerChunkTransferFlag is ON. PeerToPeerAsyncCheckpointFlag = NewBoolFlag("peer-to-peer-async-checkpoint", false) + // DeferRootfsExportFlag moves the rootfs diff seal (the reflink, which forces a + // synchronous host->NVMe writeback) off the pause critical path: pause() ejects + // the cache and stops the sandbox, then reflinks the diff in the background so + // the call returns without the writeback stall. Applied only to the suspend + // (pause) path, where nothing reads the diff until a later resume. Off by + // default; falls back to the synchronous export when off or on a non-NBD + // provider. + DeferRootfsExportFlag = NewBoolFlag("defer-rootfs-export", false) + PersistentVolumesFlag = NewBoolFlag("can-use-persistent-volumes", env.IsDevelopment()) SandboxLabelBasedSchedulingFlag = NewBoolFlag("sandbox-label-based-scheduling", false) OptimisticResourceAccountingFlag = NewBoolFlag("sandbox-placement-optimistic-resource-accounting", false) diff --git a/packages/shared/pkg/telemetry/meters.go b/packages/shared/pkg/telemetry/meters.go index 7567bbb9ff..35353acb0d 100644 --- a/packages/shared/pkg/telemetry/meters.go +++ b/packages/shared/pkg/telemetry/meters.go @@ -125,6 +125,7 @@ const ( PauseDurationHistogramName HistogramType = "orchestrator.sandbox.pause.duration" SnapshotProcessMemoryDurationName HistogramType = "orchestrator.sandbox.snapshot.process_memory.duration" SnapshotProcessRootfsDurationName HistogramType = "orchestrator.sandbox.snapshot.process_rootfs.duration" + SnapshotRootfsSealDurationName HistogramType = "orchestrator.sandbox.snapshot.rootfs_seal.duration" // OrchestratorEnvdUpgradeDurationName is the wall-time of a resume-time envd // live-upgrade (delivery + trigger + WaitForEnvd) = overhead added to the @@ -140,8 +141,9 @@ const ( // duration is the whole throwaway resume-and-persist run (slot-hold cost); // pages is the harvested trace size (distinct 2 MiB blocks), recorded only on // success, so its bottom bucket surfaces the empty-trace (idle-at-pause) rate. - PauseResumePrefetchHarvestDurationName HistogramType = "orchestrator.sandbox.pause_resume_prefetch.harvest.duration" - PauseResumePrefetchHarvestPagesName HistogramType = "orchestrator.sandbox.pause_resume_prefetch.harvest.pages" + PauseResumePrefetchHarvestDurationName HistogramType = "orchestrator.sandbox.pause_resume_prefetch.harvest.duration" + PauseResumePrefetchHarvestPagesName HistogramType = "orchestrator.sandbox.pause_resume_prefetch.harvest.pages" + PauseResumePrefetchSealWaitDurationName HistogramType = "orchestrator.sandbox.pause_resume_prefetch.seal_wait.duration" // Sandbox startup working-set histograms: demand-fault pages/bytes a guest // needed to reach a successful envd init, recorded once per start. Sampled @@ -485,9 +487,11 @@ var histogramDesc = map[HistogramType]string{ PauseDurationHistogramName: "Time taken to pause a sandbox, labeled by fs_only (filesystem-only vs memory) and success", SnapshotProcessMemoryDurationName: "Time to export+diff the memory file during a pause snapshot (memory pauses only), labeled by success", SnapshotProcessRootfsDurationName: "Time to export+diff the rootfs during a pause snapshot, labeled by fs_only and success", + SnapshotRootfsSealDurationName: "Time for the background deferred rootfs reflink seal (off the pause critical path), labeled by success", - PauseResumePrefetchHarvestDurationName: "Time taken for a pause-resume prefetch harvest run (slot-hold cost)", - PauseResumePrefetchHarvestPagesName: "Harvested resume-prefetch trace size in 2 MiB blocks, per successful harvest", + PauseResumePrefetchHarvestDurationName: "Time taken for a pause-resume prefetch harvest run (slot-hold cost)", + PauseResumePrefetchHarvestPagesName: "Harvested resume-prefetch trace size in 2 MiB blocks, per successful harvest", + PauseResumePrefetchSealWaitDurationName: "Time the prefetch harvest waited for the deferred rootfs seal before its warm resume", UffdStartupPagesHistogramName: "Demand-fault pages a guest needed to reach a successful envd init, per start", UffdStartupSourcePagesHistogramName: "Subset of startup demand-fault pages pulled from the source (e.g. GCS), per start", @@ -539,8 +543,10 @@ var histogramUnits = map[HistogramType]string{ PauseDurationHistogramName: "ms", SnapshotProcessMemoryDurationName: "ms", SnapshotProcessRootfsDurationName: "ms", + SnapshotRootfsSealDurationName: "ms", PauseResumePrefetchHarvestDurationName: "ms", PauseResumePrefetchHarvestPagesName: "{page}", + PauseResumePrefetchSealWaitDurationName: "ms", UffdStartupPagesHistogramName: "{page}", UffdStartupSourcePagesHistogramName: "{page}", UffdStartupBytesHistogramName: "{By}", From bec1b513d8556a696c0602f76b693b2bda195a5c Mon Sep 17 00:00:00 2001 From: Babis Chalios Date: Wed, 29 Jul 2026 13:43:22 +0200 Subject: [PATCH 3/3] fix(rootfs): skip unsealed deferred diffs during cache eviction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single disk-space eviction goroutine calls FileSize on the oldest evictable cache entry; for a still-sealing deferred rootfs diff that blocks on the background seal (WaitWithContext with the eviction loop's long-lived ctx), stalling all reclamation during exactly the high-disk-pressure window eviction exists to relieve. Add a non-blocking deferredDiff.sealed() check (via the SetOnce Done channel) and skip not-yet-sealed diffs in eviction, like pinned entries — so eviction neither blocks on the seal nor evicts a fresh, in-flight snapshot; the diff becomes evictable once its seal resolves. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Babis Chalios --- .../orchestrator/pkg/sandbox/build/cache.go | 24 +++++++++++ .../pkg/sandbox/build/cache_test.go | 41 +++++++++++++++++++ .../pkg/sandbox/build/deferred_diff.go | 13 ++++++ .../pkg/sandbox/build/deferred_diff_test.go | 15 +++++++ 4 files changed, 93 insertions(+) diff --git a/packages/orchestrator/pkg/sandbox/build/cache.go b/packages/orchestrator/pkg/sandbox/build/cache.go index 0f3202cb31..daab0ee01f 100644 --- a/packages/orchestrator/pkg/sandbox/build/cache.go +++ b/packages/orchestrator/pkg/sandbox/build/cache.go @@ -98,6 +98,22 @@ func NewDiffStore( // buildData will be deleted by calling buildData.Close() defer ds.resetDelete(item.Key()) + // A deferred diff whose background seal hasn't resolved would block Close() + // on the reflink; close it off the eviction goroutine so this callback + // isn't stalled. Bounded (the seal always resolves) and only reachable if + // the TTL is shortened below the seal time — the disk-pressure eviction + // path already skips unsealed diffs (see deferredDiff.sealed()). + if dd, ok := buildData.(*deferredDiff); ok && !dd.sealed() { + logCtx := context.WithoutCancel(ctx) + go func() { + if closeErr := dd.Close(); closeErr != nil { + logger.L().Warn(logCtx, "failed to close unsealed deferred diff", zap.Any("item_key", item.Key()), zap.Error(closeErr)) + } + }() + + return + } + if closeErr := buildData.Close(); closeErr != nil { logger.L().Warn(ctx, "failed to cleanup build data cache for item", zap.Any("item_key", item.Key()), zap.Error(closeErr)) } @@ -311,6 +327,14 @@ func (s *DiffStore) deleteOldestFromCache(ctx context.Context) (suc bool, e erro return true } + // Skip a deferred diff whose background rootfs seal hasn't resolved yet: + // FileSize below would block on the seal (stalling the sole eviction + // goroutine), and a fresh, still-sealing snapshot is exactly what a + // just-resumed peer needs. It becomes evictable once the seal resolves. + if dd, ok := item.Value().(*deferredDiff); ok && !dd.sealed() { + return true + } + sfSize, err := item.Value().FileSize(ctx) if err != nil { logger.L().Warn(ctx, "failed to get size of deleted item from cache", zap.Error(err)) diff --git a/packages/orchestrator/pkg/sandbox/build/cache_test.go b/packages/orchestrator/pkg/sandbox/build/cache_test.go index 092e2c84ca..fb6ea890fa 100644 --- a/packages/orchestrator/pkg/sandbox/build/cache_test.go +++ b/packages/orchestrator/pkg/sandbox/build/cache_test.go @@ -31,6 +31,7 @@ import ( blockmetrics "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block/metrics" "github.com/e2b-dev/infra/packages/shared/pkg/featureflags" "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" + "github.com/e2b-dev/infra/packages/shared/pkg/utils" ) const ( @@ -716,3 +717,43 @@ func mustParseCfg(t *testing.T) cfg.Config { return c } + +// A deferred rootfs diff whose background seal hasn't resolved must be skipped +// by disk-pressure eviction: its data methods (FileSize) block on the seal, so +// evicting it would stall the sole eviction goroutine, and a fresh, still-sealing +// snapshot is exactly what a just-resumed peer needs. Once the seal resolves the +// entry becomes evictable like any other. +func TestDiffStoreEvictionSkipsUnsealedDeferredDiff(t *testing.T) { + t.Parallel() + cachePath := t.TempDir() + + c, err := cfg.Parse() + require.NoError(t, err) + flags := flagsWithMaxBuildCachePercentage(t, 100) + store, err := NewDiffStore(c, flags, cachePath, 60*time.Second, 4*time.Second) + require.NoError(t, err) + + // Oldest entry: a deferred rootfs diff whose background seal hasn't resolved. + promise := utils.NewSetOnce[Diff]() + deferred := NewDeferredDiff(GetDiffStoreKey("deferred-unsealed", Rootfs), blockSize, promise) + store.Add(deferred) + + // A newer, ordinary diff behind it. + newer := newRootFSDiff(t, cachePath, "seal-newer") + store.Add(newer) + + // Eviction must skip the unsealed deferred diff and fall through to the + // next-oldest instead of blocking on the seal. + ok, err := store.deleteOldestFromCache(t.Context()) + require.NoError(t, err) + assert.True(t, ok) + assert.False(t, store.isBeingDeleted(deferred.CacheKey()), "unsealed deferred diff must be skipped") + assert.True(t, store.isBeingDeleted(newer.CacheKey()), "eviction must fall through to the next-oldest") + + // Once the seal resolves, the deferred diff is evictable like any other entry. + require.NoError(t, promise.SetValue(newRootFSDiff(t, cachePath, "seal-resolved"))) + ok, err = store.deleteOldestFromCache(t.Context()) + require.NoError(t, err) + assert.True(t, ok) + assert.True(t, store.isBeingDeleted(deferred.CacheKey()), "sealed deferred diff must be evictable") +} diff --git a/packages/orchestrator/pkg/sandbox/build/deferred_diff.go b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go index 74a7138e26..fa388db16e 100644 --- a/packages/orchestrator/pkg/sandbox/build/deferred_diff.go +++ b/packages/orchestrator/pkg/sandbox/build/deferred_diff.go @@ -53,6 +53,19 @@ func (d *deferredDiff) BlockSize() int64 { return d.blockSize } +// sealed reports, without blocking, whether the background seal has resolved the +// promise (with the materialized diff or an error). Cache eviction uses it to +// skip a not-yet-sealed diff rather than block on the data methods (which wait +// on the seal) or evict a fresh, still-in-flight snapshot. +func (d *deferredDiff) sealed() bool { + select { + case <-d.inner.Done: + return true + default: + return false + } +} + func (d *deferredDiff) CachePath(ctx context.Context) (string, error) { inner, err := d.inner.WaitWithContext(ctx) if err != nil { diff --git a/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go b/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go index bad53dc4fc..aefaefb425 100644 --- a/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go +++ b/packages/orchestrator/pkg/sandbox/build/deferred_diff_test.go @@ -11,6 +11,21 @@ import ( "github.com/e2b-dev/infra/packages/shared/pkg/utils" ) +// TestDeferredDiff_SealedIsNonBlocking verifies sealed() reports readiness +// without blocking on the promise, so cache eviction can skip an unsealed diff +// instead of stalling on it. +func TestDeferredDiff_SealedIsNonBlocking(t *testing.T) { + t.Parallel() + + inner := utils.NewSetOnce[Diff]() + d := NewDeferredDiff(GetDiffStoreKey("build-id", Rootfs), 4096, inner).(*deferredDiff) + + require.False(t, d.sealed(), "unresolved promise must read as not sealed") + + require.NoError(t, inner.SetValue(&NoDiff{})) + require.True(t, d.sealed(), "resolved promise must read as sealed") +} + // makeLocalDiff writes `data` to a fresh local diff file and materializes it. func makeLocalDiff(t *testing.T, blockSize int64, data []byte) Diff { t.Helper()