sandboxd: warm every clone at refill and defer release teardown

docs/deploy.md

@@ -97,6 +97,7 @@ sandboxd reads one JSON file (`-config`, default
 | `tenants` | unset | multi-tenant tokens next to `api_token`: `[{"name": "acme", "token": "…", "max_claims": 50}]`. A tenant token reaches the resource-creating verbs (claim, fork, promote, checkpoint, preview), catalog discovery, and its own sandbox/checkpoint listings; everything it creates is stamped with the tenant name. Root-only surfaces (per-id sandbox reads, `GET /v1/info`, `PUT /v1/pools`, `POST/DELETE /v1/drain`, `/metrics`) answer it 403. `max_claims` (0 = unlimited) caps that tenant's live claims next to the node-wide cap. Requires `api_token` set. Names and tokens must be unique, tokens distinct from `api_token`. On a cluster all nodes must carry the same tenants set (the SDK replays whichever token authorized a redirect), and per-node caps mean a tenant's effective cluster limit is `max_claims` × nodes. Empty = exactly the single-token behavior |
 | `max_fork_count` | 16 | children a single `fork` may create; each is a full-RAM VM, so this bounds one request's memory blast radius to the node's capacity |
 | `refill_concurrency` | 0 (auto) | concurrent VM provisioning budget, shared by warm-pool refills, fork clones, and the reap/hibernate/reconcile engine batches. 0 sizes it from the node: `NumCPU*2/3` clamped to [4, 256] — a 384-core node gets 256; small nodes keep a floor of 4 |
+| `release_delay_seconds` | 0 | seconds a released VM waits in the removal queue before `cocoon vm rm`. The claim is dropped and journaled and its egress listener closed at release; the VM removal, the volume hold release and the egress tap unlock run on the first reap tick (5 s) after the delay, on the `refill_concurrency` budget like every other batch teardown, so a burst of releases does not compete with the claims still running. Until then the VM holds its memory and its volume reservations without counting as a claim. 0 removes inline |
 | `preview_listen` | (off) | address for a preview HTTP server that serves guest ports under signed URLs; needs `preview_secret` |
 | `preview_secret` | — | cluster-shared HMAC secret signing preview tokens (all nodes share one) |
 | `preview_advertise` | = `preview_listen` | the browser-facing preview base URL; nodes behind one TLS proxy may share it, while signed tokens route internally through each owner's `advertise_addr` |
@@ -105,7 +106,7 @@ sandboxd reads one JSON file (`-config`, default
 | `checkpoint_ttl_hours` | 0 (keep forever) | ages out checkpoints older than this; the sweep runs hourly and at startup. Explicit deletes never wait for it. Must be nonzero and match fleet-wide when `checkpoint_peer_heal` is on — it is the expiry eligibility point for a healed replica a delete broadcast missed, after which its next successful hourly sweep removes it; persistent sweep failure extends retention until one succeeds, so it is not a hard ceiling |
 | `checkpoint_peer_heal` | false | on a cluster, lets a node pull a checkpoint it lacks from a peer — found via a live probe, not gossip — rather than failing the branch; see [placement lifecycle](cluster.md#checkpoints-on-a-cluster). Three requirements, all enforced at config load: a nonempty `api_token` (the blob transfer between peers authenticates with it; without one the raw record stream would be open), `mesh.cluster_key` set (the pull presents the fleet `api_token` to an address learned from the peer probe, so the gossip layer carrying that address must itself be authenticated), and `checkpoint_ttl_hours` nonzero (a replica a delete broadcast missed becomes eligible for expiry after it, and its next successful hourly sweep removes it — so it is the finite eligibility point, not an exact ceiling). A shared checkpoint store (`checkpoint_store` kind `s3`) ignores this setting — every node already resolves every checkpoint directly, so there is nothing to heal |
 | `warm_max` (pool entry) | 0 (static) | turns on the demand-adaptive watermark for that pool: the warm target rises from `warm` toward `warm_max` while claims arrive faster than the measured provision lead covers, and decays back over ~a minute of silence |
-| `warmup` (pool entry) | unset | argv run in the golden VM after readiness and before its snapshot, so the files it touches are page-cache-resident in every clone — e.g. `["node", "-e", "0"]` on a Node flavor. It runs under the engine's 2-minute command timeout in silkd's base environment (`PATH`, `TERM`, and the node's proxy variables on the none lane); a non-zero exit or a timeout fails the golden build, so the pool stays unfilled until the config is fixed. Config-owned like `egress`: `PUT /v1/pools` rejects it, and a golden built with a different warmup is rebuilt |
+| `warmup` (pool entry) | unset | argv run in the golden VM after readiness and before its snapshot, so the files it touches are page-cache-resident in every clone, and again in every clone before it joins the warm pool, so those pages are already faulted into the restored VM when the first command runs — e.g. `["node", "-e", "0"]` on a Node flavor. It runs under the engine's 2-minute command timeout in silkd's base environment (`PATH`, `TERM`, and the node's proxy variables on the none lane); a non-zero exit or a timeout fails the golden build, so the pool stays unfilled until the config is fixed. Config-owned like `egress`: `PUT /v1/pools` rejects it, and a golden built with a different warmup is rebuilt |
 | `max_claims` | 0 (unlimited) | node-wide cap on live claims; claim/fork/branch requests beyond it answer 429 with the pool state unharmed (on a cluster, normal warm-candidate placement applies, with volume claims limited to candidates holding every requested volume) |
 | `audit_log` | false | append every relayed request frame's op + addressing fields (never payloads) to `<data_dir>/audit.jsonl`, size-rotated with one `.1` backup. Records are `{t, id, op}` plus whichever addressing fields the op carries (`argv`, `path`, `dest`, `from`, `to`, `url`, `session`, `port`), plus `decision` and `secret` (the ref name, never its value) on `egress` records; preview accesses record as op `preview`, one per request. A request frame whose first line exceeds 4 KiB is skipped, never truncated |
 | `idle_hibernate_seconds` | 0 (off) | node-wide idle policy for unpooled claims (template/checkpoint claims): a none-lane claim with no data-plane connection for this long is hibernated; the next call that reaches the guest wakes it transparently. Per-pool `idle_hibernate_seconds` does the same for that pool's claims; pooled keys ignore the node-wide value, and egress pools reject it because they cannot resume safely. Opt in deliberately: a wake costs latency and the snapshot, so callers with their own idle logic must not pay twice |

sandboxd/config/config.go

@@ -45,7 +45,7 @@ type PoolSpec struct {
 	// Egress is this pool's allow-list, intersected with the tenant's; nil denies all egress.
 	Egress *egress.Policy `json:"egress,omitempty"`
 
-	// Warmup runs in the golden VM before its snapshot, so every clone starts with its page cache.
+	// Warmup runs in the golden VM before its snapshot and again in each clone after restore.
 	Warmup []string `json:"warmup,omitempty"`
 
 	// IdleHibernateSeconds, when >0, hibernates idle claims after that many seconds.
@@ -231,6 +231,9 @@ type Config struct {
 	// RefillConcurrency caps concurrent VM provisioning node-wide; 0 auto-scales with CPUs.
 	RefillConcurrency int `json:"refill_concurrency,omitzero"`
 
+	// ReleaseDelaySeconds, when >0, parks a released VM in the removal queue for that long instead of removing it inline.
+	ReleaseDelaySeconds int `json:"release_delay_seconds,omitzero"`
+
 	// Mesh, when set, joins this node to a memberlist cluster; nil is a mesh of one.
 	Mesh *MeshConfig `json:"mesh,omitempty"`
 
@@ -297,6 +300,20 @@ func (c *Config) applyDefaults() {
 }
 
 func (c *Config) validate() error {
+	for _, field := range []struct {
+		name  string
+		value int
+	}{
+		{"idle_hibernate_seconds", c.IdleHibernateSeconds},
+		{"checkpoint_ttl_hours", c.CheckpointTTLHours},
+		{"max_claims", c.MaxClaims},
+		{"refill_concurrency", c.RefillConcurrency},
+		{"release_delay_seconds", c.ReleaseDelaySeconds},
+	} {
+		if field.value < 0 {
+			return fmt.Errorf("%s must not be negative, got %d", field.name, field.value)
+		}
+	}
 	if err := c.validateAttachment(); err != nil {
 		return err
 	}
@@ -307,21 +324,12 @@ func (c *Config) validate() error {
 	if c.MaxForkCount < 1 {
 		return fmt.Errorf("max_fork_count must be at least 1, got %d", c.MaxForkCount)
 	}
-	if c.RefillConcurrency < 0 {
-		return fmt.Errorf("refill_concurrency must not be negative, got %d", c.RefillConcurrency)
-	}
 	if err := c.RestoreMode.Validate(); err != nil {
 		return fmt.Errorf("restore_mode: %w", err)
 	}
-	if c.MaxClaims < 0 {
-		return fmt.Errorf("max_claims must not be negative, got %d", c.MaxClaims)
-	}
 	if c.PreviewListen != "" && c.PreviewSecret == "" {
 		return fmt.Errorf("preview_listen needs preview_secret")
 	}
-	if c.IdleHibernateSeconds < 0 {
-		return fmt.Errorf("idle_hibernate_seconds must not be negative, got %d", c.IdleHibernateSeconds)
-	}
 	if err := validateArchiveWindow(c.IdleHibernateSeconds, c.ArchiveAfterSeconds, c.ArchiveDeleteAfterSeconds); err != nil {
 		return err
 	}
@@ -336,9 +344,6 @@ func (c *Config) validate() error {
 			return fmt.Errorf("checkpoint_store kind %q: want dir or s3", cs.Kind)
 		}
 	}
-	if c.CheckpointTTLHours < 0 {
-		return fmt.Errorf("checkpoint_ttl_hours must not be negative")
-	}
 	if c.CheckpointPeerHeal && (c.Mesh == nil || c.Mesh.ClusterKey == "") {
 		return fmt.Errorf("checkpoint_peer_heal requires an encrypted mesh (set mesh.cluster_key)")
 	}

sandboxd/pool/claim.go

@@ -169,12 +169,18 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand
 	}
 	td := m.quiesceVolumes(ctx, sb)
 	var err error
-	if vmName == "" {
+	removed := vmName == ""
+	switch {
+	case removed:
 		m.finishVolumeTeardown(ctx, td) // archived: no VM to confirm gone
-	} else if !m.removeOrRetry(ctx, vmName, id, "", td) {
-		err = fmt.Errorf("vm %s survived removal", vmName)
+	case m.releaseDelay > 0:
+		m.queueRemoval(vmName, id, "", td, time.Now().Add(m.releaseDelay))
+	default:
+		if removed = m.removeOrRetry(ctx, vmName, id, "", td); !removed {
+			err = fmt.Errorf("vm %s survived removal", vmName)
+		}
 	}
-	m.disarmEgress(id, err == nil)
+	m.disarmEgress(id, removed)
 	m.dropSnap(ctx, snap)
 	m.counters.releases.Add(1)
 	m.recordUsage(ctx, usageEvent{Event: "release", ID: id, VMName: vmName})

sandboxd/pool/pool.go

@@ -160,6 +160,7 @@ type pendingRemoval struct {
 	tap         string
 	staleCreate bool
 	volumes     volumeTeardown
+	notBefore   time.Time
 }
 
 type pool struct {
@@ -270,6 +271,7 @@ type Manager struct {
 
 	// tenantMax doubles as the set of known tenants; a 0 cap means unlimited.
 	maxClaims    int
+	releaseDelay time.Duration
 	draining     bool // guarded by m.mu; deliberately not persisted
 	tenantMax    map[string]int
 	tenantLive   map[string]int
@@ -345,6 +347,7 @@ func NewManager(ctx context.Context, cfg *config.Config, eng Engine, secrets *eg
 		dataDir:         cfg.DataDir,
 		egress:          cfg.HasEgress(),
 		lockEgress:      len(cfg.Bridges) > 0,
+		releaseDelay:    time.Duration(cfg.ReleaseDelaySeconds) * time.Second,
 		maxFork:         maxFork,
 		store:           newClaimStore(cfg.DataDir),
 		volumes:         make(map[string]catalogVolume, len(cfg.Volumes)),

sandboxd/pool/pool_test.go

@@ -214,6 +214,31 @@ func TestReleaseAfterResolveTearsDownOnce(t *testing.T) {
 	}
 }
 
+func TestReleaseDelayQueuesTeardown(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		eng := newFakeEngine()
+		m := newTestManager(t, eng)
+		m.releaseDelay = 2 * time.Second
+		sb := mustClaim(t, m, testKey)
+
+		if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil {
+			t.Fatalf("release: %v", err)
+		}
+		if _, g := m.Info(); g.Claimed != 0 {
+			t.Fatalf("claimed=%d after release, want 0", g.Claimed)
+		}
+		m.retryRemovals(t.Context()).Wait()
+		if removed := eng.removedNames(); len(removed) != 0 {
+			t.Fatalf("removes=%v before the delay, want none", removed)
+		}
+		time.Sleep(2 * time.Second)
+		m.retryRemovals(t.Context()).Wait()
+		if removed := eng.removedNames(); len(removed) != 1 || removed[0] != sb.VMName {
+			t.Fatalf("removes=%v after the delay, want %s", removed, sb.VMName)
+		}
+	})
+}
+
 func TestReleaseByOperatorCred(t *testing.T) {
 	eng := newFakeEngine()
 	m := newTestManager(t, eng)
@@ -795,6 +820,9 @@ type fakeEngine struct {
 	exportContent                     []byte
 	caInstalls                        []string
 	warmups                           [][]string
+	warmupSocks                       []string
+	warmupAfterSnap                   bool
+	warmupErr                         error
 	staleReconciles                   []string
 	installCAErr                      error
 	diskAttachErr                     error
@@ -1031,14 +1059,13 @@ func (f *fakeEngine) InstallCACert(_ context.Context, vsockSocket string, _ []by
 	return f.installCAErr
 }
 
-func (f *fakeEngine) Warmup(_ context.Context, _ string, argv []string) error {
+func (f *fakeEngine) Warmup(_ context.Context, sock string, argv []string) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	if len(f.snapSaves) > 0 {
-		return fmt.Errorf("warmup after snapshot")
-	}
 	f.warmups = append(f.warmups, argv)
-	return nil
+	f.warmupSocks = append(f.warmupSocks, sock)
+	f.warmupAfterSnap = f.warmupAfterSnap || len(f.snapSaves) > 0
+	return f.warmupErr
 }
 
 func (f *fakeEngine) DiskAttach(_ context.Context, _ string, spec engine.VolumeSpec) error {

sandboxd/pool/refill.go

@@ -90,6 +90,9 @@ func (m *Manager) refillOne(ctx context.Context, p *pool, golden string) {
 		}
 		sb, err = m.readyBounded(ctx, sb, time.Now().Add(warmProbeTimeout))
 	}
+	if err == nil {
+		err = m.warmClone(ctx, sb)
+	}
 	keep := false
 	var fails int
 	var wait time.Duration
@@ -190,15 +193,13 @@ func (m *Manager) buildGoldenSteps(ctx context.Context, key types.PoolKey, name,
 	}
 	caBaked := m.poolIntercepts(key)
 	if caBaked {
-		if err := m.eng.InstallCACert(ctx, sock, m.egressCA.CertPEM()); err != nil {
+		if err = m.eng.InstallCACert(ctx, sock, m.egressCA.CertPEM()); err != nil {
 			return fmt.Errorf("install egress ca: %w", err)
 		}
 	}
-	warmup := m.poolWarmup(key)
-	if len(warmup) > 0 {
-		if err := m.eng.Warmup(ctx, sock, warmup); err != nil {
-			return fmt.Errorf("warmup: %w", err)
-		}
+	warmup, err := m.runWarmup(ctx, key, sock)
+	if err != nil {
+		return fmt.Errorf("warmup: %w", err)
 	}
 	if err := m.eng.SnapshotSave(ctx, name, snap); err != nil {
 		return err
@@ -218,6 +219,23 @@ func (m *Manager) poolWarmup(key types.PoolKey) []string {
 	return m.poolWarmups[key]
 }
 
+func (m *Manager) runWarmup(ctx context.Context, key types.PoolKey, sock string) ([]string, error) {
+	warmup := m.poolWarmup(key)
+	if len(warmup) == 0 {
+		return nil, nil
+	}
+	return warmup, m.eng.Warmup(ctx, sock, warmup)
+}
+
+// a restore maps the golden memory lazily, so the warmup's pages fault in here instead of under the first claim
+func (m *Manager) warmClone(ctx context.Context, sb *types.Sandbox) error {
+	if _, err := m.runWarmup(ctx, sb.Key, sb.VsockSocket); err != nil {
+		m.destroy(ctx, sb.VMName)
+		return fmt.Errorf("clone warmup: %w", err)
+	}
+	return nil
+}
+
 // writeGoldenCASidecar records or clears the baked-CA fingerprint; a rotated CA forces a rebuild.
 func (m *Manager) writeGoldenCASidecar(final string, caBaked bool) error {
 	var stamp string

sandboxd/pool/remove.go

@@ -6,6 +6,7 @@ import (
 	"maps"
 	"slices"
 	"sync"
+	"time"
 
 	"github.com/projecteru2/core/log"
 
@@ -48,13 +49,13 @@ func (m *Manager) removeOrRetry(ctx context.Context, name, sandboxID, tap string
 		m.finishVolumeTeardown(ctx, td)
 		return true
 	}
-	m.queueRemoval(name, sandboxID, tap, td)
+	m.queueRemoval(name, sandboxID, tap, td, time.Time{})
 	return false
 }
 
-func (m *Manager) queueRemoval(name, sandboxID, tap string, td volumeTeardown) {
+func (m *Manager) queueRemoval(name, sandboxID, tap string, td volumeTeardown, notBefore time.Time) {
 	m.mu.Lock()
-	m.pendingRemovals[name] = pendingRemoval{sandboxID: sandboxID, tap: tap, volumes: td}
+	m.pendingRemovals[name] = pendingRemoval{sandboxID: sandboxID, tap: tap, volumes: td, notBefore: notBefore}
 	m.mu.Unlock()
 }
 
@@ -64,16 +65,21 @@ func (m *Manager) queueStaleCreate(name, tap string) {
 	m.mu.Unlock()
 }
 
-// retryRemovals empties the queue to dispatch, so the next tick cannot double-dispatch.
+// retryRemovals moves the due entries out of the queue to dispatch, so the next tick cannot double-dispatch.
 func (m *Manager) retryRemovals(ctx context.Context) *sync.WaitGroup {
+	now := time.Now()
+	batch := map[string]pendingRemoval{}
 	m.mu.Lock()
-	if len(m.pendingRemovals) == 0 {
-		m.mu.Unlock()
-		return new(sync.WaitGroup)
+	for name, pending := range m.pendingRemovals {
+		if !now.Before(pending.notBefore) {
+			batch[name] = pending
+			delete(m.pendingRemovals, name)
+		}
 	}
-	batch := m.pendingRemovals
-	m.pendingRemovals = map[string]pendingRemoval{}
 	m.mu.Unlock()
+	if len(batch) == 0 {
+		return new(sync.WaitGroup)
+	}
 	names := slices.Collect(maps.Keys(batch))
 	return m.runBounded(ctx, len(names), func(ctx context.Context, i int) {
 		m.retryRemoval(ctx, names[i], batch[names[i]])
@@ -96,7 +102,7 @@ func (m *Manager) retryRemoval(ctx context.Context, name string, pending pending
 		}
 	}
 	if !m.removeVM(ctx, name) {
-		m.queueRemoval(name, pending.sandboxID, pending.tap, pending.volumes)
+		m.queueRemoval(name, pending.sandboxID, pending.tap, pending.volumes, time.Time{})
 		return
 	}
 	m.finishRemoval(ctx, pending)

sandboxd/pool/warmup_test.go

@@ -7,6 +7,7 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"testing/synctest"
 
 	"github.com/cocoonstack/sandbox/sandboxd/config"
 )
@@ -22,6 +23,9 @@ func TestGoldenBuildRunsWarmupBeforeSnapshot(t *testing.T) {
 	if len(eng.warmups) != 1 || !slices.Equal(eng.warmups[0], argv) {
 		t.Fatalf("warmups = %v, want [%v]", eng.warmups, argv)
 	}
+	if eng.warmupAfterSnap {
+		t.Fatal("golden warmup ran after the snapshot save")
+	}
 	stamp, err := os.ReadFile(final + warmupSidecarSuffix)
 	if err != nil {
 		t.Fatalf("read warmup sidecar: %v", err)
@@ -87,3 +91,50 @@ func TestPoolSpecRejectsEmptyWarmupArgument(t *testing.T) {
 		t.Errorf("ValidateLimits error = %v, want an empty-argument rejection", err)
 	}
 }
+
+func TestRefillWarmsEveryClone(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		eng := newFakeEngine()
+		argv := []string{"node", "-e", "0"}
+		m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 2, Warmup: argv})
+		m.pools[testKey].goldenDir = "/goldens/x"
+
+		m.refillOnce(t.Context())
+		waitFor(t, func() bool {
+			infos, _ := m.Info()
+			return infos[0].Warm == 2 && infos[0].Refilling == 0
+		})
+		eng.mu.Lock()
+		warmups, socks := slices.Clone(eng.warmups), slices.Clone(eng.warmupSocks)
+		eng.mu.Unlock()
+		if len(warmups) != 2 || !slices.Equal(warmups[0], argv) || !slices.Equal(warmups[1], argv) {
+			t.Fatalf("warmups = %v, want %v once per clone", warmups, argv)
+		}
+		if socks[0] == "" || socks[0] == socks[1] {
+			t.Errorf("warmup sockets = %v, want one per clone", socks)
+		}
+	})
+}
+
+func TestRefillCloneWarmupFailureCleansUp(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		eng := newFakeEngine()
+		eng.warmupErr = errors.New("node: not found")
+		m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Warmup: []string{"node", "-e", "0"}})
+		m.pools[testKey].goldenDir = "/goldens/x"
+
+		m.refillOnce(t.Context())
+		waitFor(t, func() bool {
+			infos, _ := m.Info()
+			return infos[0].Warm == 0 && infos[0].Refilling == 0 && len(eng.removedNames()) == 1
+		})
+		eng.mu.Lock()
+		eng.warmupErr = nil
+		eng.mu.Unlock()
+		m.refillOnce(t.Context())
+		waitFor(t, func() bool {
+			infos, _ := m.Info()
+			return infos[0].Warm == 1 && infos[0].Refilling == 0
+		})
+	})
+}