Skip to content

Commit a3922e4

Browse files
kalyazinclaude
andcommitted
feat(orch): serve resume from a provisional header during dedup
A resume that overlaps an in-flight pause of the same sandbox can block in the storage-template-memfile span: the local memfile template can't be built until the deduped header resolves, which only happens after dedup's compare+defrag. The in-flight memfd serving added earlier removes the drain wait but not this header wait, because it is upstream of the diff source. At pause, build a provisional local header (identity storage offsets, dirty pages attributed to a fresh provisional build id) from the dirty bitmap and the parent header, and register a memfd-backed diff source for that build id. The local template is built from the provisional header immediately, so a concurrent resume serves dirty pages straight from the still-mapped memfd instead of blocking. Once the deduped header resolves it is swapped in (SwapHeader), routing dirty pages to the compacted deduped diff; the distinct build id makes the swap race-free since build.File resolves the source from the same header snapshot as the offset. The swap is conditional (compare-and-swap from the still-provisional header) so a late swap can't clobber a newer header the upload may have finalized first. The provisional header is local-only: the upload continues to use the deduped header, so the persisted artifact is unchanged. A pause must never parent a snapshot off the provisional header: it maps dirty pages to a synthetic build id with no storage object, so an uploaded header that inherited those mappings (from a resume paused again before the swap) would be unreadable on a cold or cross-node resume. build.File carries the deduped header as a durable-header future, and processMemorySnapshot resolves the parent via DurableHeader (waiting for the deduped header if a swap is still pending) rather than the live, possibly-provisional header. The peer-to-peer header source serves the durable header for the same reason, so a cross-node resume in the window receives the deduped mappings (waiting if a swap is pending) instead of the synthetic provisional build id it cannot resolve. Scheduling metadata (storageTemplate.SchedulingMetadata) reads the durable header too, but NON-BLOCKING (DurableHeaderNow): it runs on the create/resume response path, so while the deduped header is still pending it reports rootfs-only metadata rather than block the response for the whole dedup — and never carries the provisional build id. The dedup goroutine holds the memfd until the swap, bounded by a grace timeout; the swap goroutine signals on every exit (success or error), and buildProvisionalMemfile signals up front when it declines to build a provisional source, so the fallback releases at drain-time and the grace is only a true last-resort backstop — a counter (orchestrator.memfd.swap_grace_elapsed) surfaces it if it ever fires. Both the main memfile diff and the provisional diff are pinned in the build store for the provisional window: they share a DedupedMemfdCache (and memfd), and resume reads refresh only the provisional entry, so disk-pressure eviction of either would break the in-flight serve — evicting the main entry Closes it (cancelling dedup, tearing down the memfd), evicting the provisional entry makes a still-provisional read miss the store and fetch a never-uploaded id. Both are unpinned after the swap. Gated by memfd-dedup-inflight-serve (default off); composes with the in-flight drain serving (provisional covers the compare window, in-flight the drain). Signed-off-by: Nikita Kalyazin <nikita.kalyazin@e2b.dev> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2043ff4 commit a3922e4

11 files changed

Lines changed: 409 additions & 32 deletions

File tree

packages/orchestrator/pkg/sandbox/block/memfd.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import (
1111
"time"
1212

1313
"github.com/RoaringBitmap/roaring/v2"
14+
"go.opentelemetry.io/otel"
1415
"go.opentelemetry.io/otel/attribute"
16+
"go.opentelemetry.io/otel/metric"
1517
"go.opentelemetry.io/otel/trace"
1618
"go.uber.org/zap"
1719
"golang.org/x/sys/unix"
@@ -265,6 +267,15 @@ type DedupedMemfdCache struct {
265267
// the compare), so this grace is a backstop, not the common path.
266268
const memfdSwapGrace = 30 * time.Second
267269

270+
// swapGraceElapsedCounter counts memfd releases that fell back to the grace
271+
// timeout because no swap signal arrived. Expected to be ~0 (the swap normally
272+
// signals before the drain finishes); a nonzero rate means swaps are being lost
273+
// — e.g. pauses aborting before AddSnapshot spawns the swap goroutine — and the
274+
// memfd (guest-RAM-sized) is being held the full grace on those pauses.
275+
var swapGraceElapsedCounter = utils.Must(otel.Meter("github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block").
276+
Int64Counter("orchestrator.memfd.swap_grace_elapsed",
277+
metric.WithDescription("Dedup memfd releases that timed out on the swap grace without a swap signal")))
278+
268279
func NewCacheFromMemfdDeduped(
269280
ctx context.Context,
270281
base ReadonlyDevice,
@@ -413,17 +424,20 @@ func (d *DedupedMemfdCache) runDedup(
413424

414425
recordDedupAttrs(ctx, plan, scanEmptyPages, compareDur, writeDur)
415426
// Resolve the drained cache immediately so upload and post-swap reads don't
416-
// wait. Then, when inflight serving is active, keep the memfd mapped until
417-
// the local template swaps off the provisional header (MarkSwapped) so a
427+
// wait. Then, when inflight serving is active, keep the memfd mapped until the
428+
// local template swaps off the provisional header (MarkSwapped) so a
418429
// provisional read never hits a released memfd. The swap only waits on the
419-
// compare, so it has usually already fired by now; ctx and the grace bound
420-
// the wait so a failed or absent swap can't leak the mapping.
430+
// compare, so it has usually already fired by now; ctx and the grace bound the
431+
// wait so a failed or absent swap can't leak the mapping. When no provisional
432+
// source is built, buildProvisionalMemfile calls MarkSwapped up front, so this
433+
// returns immediately and releases at drain-time like the non-inflight path.
421434
logSetOnceErr(ctx, "dedup done", d.done.SetResult(cache, err))
422435
if d.inflight {
423436
select {
424437
case <-d.swapped.Done:
425438
case <-ctx.Done():
426439
case <-time.After(memfdSwapGrace):
440+
swapGraceElapsedCounter.Add(ctx, 1)
427441
logger.L().Warn(ctx, "memfd swap grace elapsed; releasing memfd without a swap signal")
428442
}
429443
}

packages/orchestrator/pkg/sandbox/build/build.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ var (
5252
)
5353

5454
type File struct {
55-
header atomic.Pointer[header.Header]
55+
header atomic.Pointer[header.Header]
56+
// durable, when set, is the future for the header this file will eventually
57+
// settle on (the deduped header while a provisional header is being served).
58+
// DurableHeader waits on it so a pause never parents a snapshot off a
59+
// provisional (local-only) header — see SetDurableHeader.
60+
durable atomic.Pointer[utils.SetOnce[*header.Header]]
5661
store *DiffStore
5762
fileType DiffType
5863
persistence storage.StorageProvider
@@ -91,6 +96,56 @@ func (b *File) SwapHeader(h *header.Header) {
9196
b.header.Store(h)
9297
}
9398

99+
// SwapHeaderIfCurrent atomically replaces the header with next only if it is
100+
// still old, reporting whether it did. The provisional→deduped swap uses this so
101+
// that, if it runs late, it can't clobber a newer header already installed by
102+
// another writer (e.g. Upload.publish finalizing the build, which swaps
103+
// unconditionally) with the older, still-incomplete deduped header.
104+
func (b *File) SwapHeaderIfCurrent(old, next *header.Header) bool {
105+
return b.header.CompareAndSwap(old, next)
106+
}
107+
108+
// SetDurableHeader records the future for the durable header this file will
109+
// settle on, so DurableHeader can return it instead of the currently-installed
110+
// (possibly provisional) header. Set once, before the file is shared.
111+
func (b *File) SetDurableHeader(f *utils.SetOnce[*header.Header]) {
112+
b.durable.Store(f)
113+
}
114+
115+
// DurableHeader returns the header safe to persist as a parent: the durable
116+
// future's value if one was set (waiting for it), otherwise the currently
117+
// installed header. Callers building an *uploaded* snapshot header (a Pause)
118+
// must use this, never Header(), so they never inherit a provisional header's
119+
// synthetic build-id mappings, which have no storage object.
120+
func (b *File) DurableHeader(ctx context.Context) (*header.Header, error) {
121+
if f := b.durable.Load(); f != nil {
122+
return f.WaitWithContext(ctx)
123+
}
124+
125+
return b.Header(), nil
126+
}
127+
128+
// DurableHeaderNow is the non-blocking form of DurableHeader: it returns
129+
// (header, true) when the durable header is already known — no provisional swap
130+
// pending, or the deduped future has resolved — and (nil, false) while a swap is
131+
// still pending. Latency-sensitive callers (e.g. scheduling metadata on the
132+
// create/resume response path) use it so they neither block on the dedup nor
133+
// observe the provisional header's synthetic build id.
134+
func (b *File) DurableHeaderNow() (*header.Header, bool) {
135+
f := b.durable.Load()
136+
if f == nil {
137+
return b.Header(), true
138+
}
139+
select {
140+
case <-f.Done:
141+
h, err := f.Wait()
142+
143+
return h, err == nil
144+
default:
145+
return nil, false
146+
}
147+
}
148+
94149
// ReadAt times file.read_at around readAt; Slice composes via readAt directly to
95150
// avoid double counting.
96151
func (b *File) ReadAt(ctx context.Context, p []byte, off int64) (n int, err error) {

packages/orchestrator/pkg/sandbox/build/cache.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ type DiffStore struct {
5353
pdDelay time.Duration
5454

5555
insertionTimes sync.Map // map[DiffStoreKey]time.Time — tracks when each diff was cached
56+
57+
// pinned entries are skipped by disk-pressure eviction (TTL eviction still
58+
// applies). Used to protect a diff whose Close would tear down state another
59+
// live entry depends on — e.g. the memfile diff whose DedupedMemfdCache is
60+
// also serving an in-flight provisional resume.
61+
pinned sync.Map // map[DiffStoreKey]struct{}
5662
}
5763

5864
func NewDiffStore(
@@ -284,6 +290,12 @@ func (s *DiffStore) deleteOldestFromCache(ctx context.Context) (suc bool, e erro
284290
return true
285291
}
286292

293+
// Skip pinned entries (e.g. a memfile diff still backing an in-flight
294+
// provisional resume); closing them would tear down shared state.
295+
if s.isPinned(item.Key()) {
296+
return true
297+
}
298+
287299
sfSize, err := item.Value().FileSize(ctx)
288300
if err != nil {
289301
logger.L().Warn(ctx, "failed to get size of deleted item from cache", zap.Error(err))
@@ -324,6 +336,19 @@ func (s *DiffStore) isBeingDeleted(key DiffStoreKey) bool {
324336
return f
325337
}
326338

339+
// Pin protects a cached entry from disk-pressure eviction (TTL eviction still
340+
// applies). Idempotent; pair every Pin with an Unpin.
341+
func (s *DiffStore) Pin(key DiffStoreKey) { s.pinned.Store(key, struct{}{}) }
342+
343+
// Unpin lifts a Pin, making the entry eligible for disk-pressure eviction again.
344+
func (s *DiffStore) Unpin(key DiffStoreKey) { s.pinned.Delete(key) }
345+
346+
func (s *DiffStore) isPinned(key DiffStoreKey) bool {
347+
_, ok := s.pinned.Load(key)
348+
349+
return ok
350+
}
351+
327352
func (s *DiffStore) scheduleDelete(ctx context.Context, key DiffStoreKey, dSize int64) {
328353
s.pdMu.Lock()
329354
defer s.pdMu.Unlock()

packages/orchestrator/pkg/sandbox/sandbox.go

Lines changed: 95 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,17 @@ func (s *Sandbox) Pause(
14351435
type MemorySnapshot struct {
14361436
Diff build.Diff
14371437
DiffHeader *DiffHeader
1438+
// ProvisionalDiffHeader + ProvisionalDiff, when non-nil, let the local
1439+
// template serve immediately from the still-mapped memfd via a distinct
1440+
// provisional build id while dedup runs, instead of blocking a concurrent
1441+
// resume in storage-template-memfile on the deduped header. They feed only
1442+
// the local AddSnapshot path; the upload still uses DiffHeader (deduped).
1443+
ProvisionalDiffHeader *header.Header
1444+
ProvisionalDiff build.Diff
1445+
// ProvisionalSwapDone, when non-nil, is invoked by the AddSnapshot swap
1446+
// goroutine once it has swapped the deduped header in; it lets the dedup
1447+
// goroutine release the memfd the provisional source was serving from.
1448+
ProvisionalSwapDone func()
14381449
// BlockSize is captured synchronously at Pause time because NewUpload's
14391450
// compression validation needs it before the async dedup header resolves;
14401451
// the dedup memfile path produces a page-granular Diff.BlockSize() that
@@ -1456,7 +1467,21 @@ func (s *Sandbox) processMemorySnapshot(ctx context.Context, buildID uuid.UUID)
14561467
if err != nil {
14571468
return MemorySnapshot{}, fmt.Errorf("failed to get original memfile: %w", err)
14581469
}
1470+
// Parent off the durable (deduped) header, never a provisional (local-only)
1471+
// one: a provisional header maps dirty pages to a synthetic build id with no
1472+
// storage object, so an uploaded header inheriting those mappings would be
1473+
// unreadable on a cold or cross-node resume. DurableHeader waits for the
1474+
// deduped header if a provisional swap is still pending; devices without one
1475+
// return their current header immediately.
14591476
memfileHeader := originalMemfile.Header()
1477+
if dh, ok := originalMemfile.(interface {
1478+
DurableHeader(ctx context.Context) (*header.Header, error)
1479+
}); ok {
1480+
memfileHeader, err = dh.DurableHeader(ctx)
1481+
if err != nil {
1482+
return MemorySnapshot{}, fmt.Errorf("failed to resolve durable memfile header: %w", err)
1483+
}
1484+
}
14601485

14611486
memfileDiffMetadata, err := s.Resources.memory.DiffMetadata(ctx, s.process)
14621487
if err != nil {
@@ -1481,7 +1506,7 @@ func (s *Sandbox) processMemorySnapshot(ctx context.Context, buildID uuid.UUID)
14811506
}
14821507
}
14831508

1484-
memfileDiff, memfileDiffHeader, err := pauseProcessMemory(
1509+
memfileDiff, memfileDiffHeader, provMemfileHeader, provMemfileDiff, provMemfileSwapDone, err := pauseProcessMemory(
14851510
ctx,
14861511
buildID,
14871512
memfileHeader,
@@ -1501,11 +1526,14 @@ func (s *Sandbox) processMemorySnapshot(ctx context.Context, buildID uuid.UUID)
15011526
}
15021527

15031528
return MemorySnapshot{
1504-
Diff: memfileDiff,
1505-
DiffHeader: memfileDiffHeader,
1506-
BlockSize: memfileHeader.Metadata.BlockSize,
1507-
header: memfileHeader,
1508-
newBytes: memfileDiffMetadata.Dirty.GetCardinality() * uint64(memfileDiffMetadata.BlockSize),
1529+
Diff: memfileDiff,
1530+
DiffHeader: memfileDiffHeader,
1531+
ProvisionalDiffHeader: provMemfileHeader,
1532+
ProvisionalDiff: provMemfileDiff,
1533+
ProvisionalSwapDone: provMemfileSwapDone,
1534+
BlockSize: memfileHeader.Metadata.BlockSize,
1535+
header: memfileHeader,
1536+
newBytes: memfileDiffMetadata.Dirty.GetCardinality() * uint64(memfileDiffMetadata.BlockSize),
15091537
}, nil
15101538
}
15111539

@@ -1539,7 +1567,7 @@ func pauseProcessMemory(
15391567
dedupDirectIO bool,
15401568
dedupBudget block.DedupBudget,
15411569
dedupInflightServe bool,
1542-
) (d build.Diff, h *DiffHeader, e error) {
1570+
) (d build.Diff, h *DiffHeader, provisionalHeader *header.Header, provisionalDiff build.Diff, provisionalSwapDone func(), e error) {
15431571
ctx, span := tracer.Start(ctx, "process-memory")
15441572
defer span.End()
15451573

@@ -1552,17 +1580,24 @@ func pauseProcessMemory(
15521580
dedupInflightServe,
15531581
)
15541582
if err != nil {
1555-
return nil, nil, fmt.Errorf("failed to export memory: %w", err)
1583+
return nil, nil, nil, nil, nil, fmt.Errorf("failed to export memory: %w", err)
15561584
}
15571585

15581586
diff, err := build.NewLocalDiffFromCache(
15591587
build.GetDiffStoreKey(buildID.String(), build.Memfile),
15601588
cache,
15611589
)
15621590
if err != nil {
1563-
return nil, nil, fmt.Errorf("failed to create local diff from cache: %w", errors.Join(err, cache.Close()))
1591+
return nil, nil, nil, nil, nil, fmt.Errorf("failed to create local diff from cache: %w", errors.Join(err, cache.Close()))
15641592
}
15651593

1594+
// Provisional local header: while the deduped header is still
1595+
// being computed, let a same-node resume serve dirty pages from the memfd via
1596+
// a distinct provisional build id at identity offsets. Gated on the memfd
1597+
// dedup path + the inflight-serve flag; best-effort (fall back to the deduped
1598+
// header on any error). The upload always uses the deduped header below.
1599+
provisionalHeader, provisionalDiff, provisionalSwapDone = buildProvisionalMemfile(ctx, cache, dedupInflightServe, originalMemfile, originalHeader, diffMetadata)
1600+
15661601
// Build the diff header on a goroutine so Pause returns without waiting
15671602
// on memfd-dedup compare. ExportMemory resolves metaOut sync for every
15681603
// other path, so Wait there is non-blocking; the goroutine is harmless.
@@ -1589,7 +1624,57 @@ func pauseProcessMemory(
15891624
setHeader(meta.ToDiffHeader(ctx, originalHeader, buildID))
15901625
}()
15911626

1592-
return diff, headerOut, nil
1627+
return diff, headerOut, provisionalHeader, provisionalDiff, provisionalSwapDone, nil
1628+
}
1629+
1630+
// buildProvisionalMemfile builds the provisional local header + its memfd-backed
1631+
// diff source, plus a swap-done callback the AddSnapshot swap goroutine invokes
1632+
// once it has swapped the deduped header in (it lets the dedup goroutine release
1633+
// the memfd). Returns (nil, nil, nil) — falling back to the deduped header —
1634+
// when the path doesn't apply or on any error, so it never blocks a pause. The
1635+
// provisional source is keyed by a fresh build id so a header swap to the
1636+
// deduped header (after dedup) is race-free (see MemfdIdentitySource).
1637+
func buildProvisionalMemfile(
1638+
ctx context.Context,
1639+
cache block.DiffSource,
1640+
enabled bool,
1641+
originalMemfile block.ReadonlyDevice,
1642+
originalHeader *header.Header,
1643+
diffMetadata *header.DiffMetadata,
1644+
) (*header.Header, build.Diff, func()) {
1645+
dc, ok := cache.(*block.DedupedMemfdCache)
1646+
if !ok {
1647+
return nil, nil, nil
1648+
}
1649+
// From here dc != nil. On every path that declines to build a provisional
1650+
// source, signal MarkSwapped so runDedup's inflight memfd-hold releases at
1651+
// drain-time instead of waiting out the swap grace — nothing will serve from
1652+
// the memfd, so there's no reason to hold it.
1653+
if !enabled || originalMemfile == nil || originalHeader == nil || diffMetadata == nil {
1654+
dc.MarkSwapped()
1655+
1656+
return nil, nil, nil
1657+
}
1658+
1659+
provisionalBuildID := uuid.New()
1660+
provisionalHeader, err := diffMetadata.ToProvisionalDiffHeader(ctx, originalHeader, provisionalBuildID)
1661+
if err != nil {
1662+
logger.L().Warn(ctx, "build provisional memfile header; using deduped header", zap.Error(err))
1663+
dc.MarkSwapped()
1664+
1665+
return nil, nil, nil
1666+
}
1667+
1668+
provisionalSource := block.NewMemfdIdentitySource(dc, int64(originalHeader.Metadata.Size))
1669+
provisionalDiff, err := build.NewLocalDiffFromCache(build.GetDiffStoreKey(provisionalBuildID.String(), build.Memfile), provisionalSource)
1670+
if err != nil {
1671+
logger.L().Warn(ctx, "build provisional memfile diff; using deduped header", zap.Error(err))
1672+
dc.MarkSwapped()
1673+
1674+
return nil, nil, nil
1675+
}
1676+
1677+
return provisionalHeader, provisionalDiff, dc.MarkSwapped
15931678
}
15941679

15951680
func pauseProcessRootfs(

0 commit comments

Comments
 (0)