From 2da7ff7fdfdb4812e9634805ffc2750958841b4f Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 11 Sep 2026 11:13:12 +0900 Subject: [PATCH 1/2] sandboxd: run the pool warmup in every clone at refill A restore maps the golden memory lazily, so the pages the golden warmup made resident still fault into each clone on first touch. Under nested virtualization that first touch is the whole time-to-interactive: at 100 concurrent claims the first `node -v` in a fresh clone measured p50 824 ms against 92 ms once the pages were in, while the host sat half idle. Re-running the warmup in the clone before it joins the warm pool moves that cost to refill; the claim path is unchanged. --- docs/deploy.md | 2 +- sandboxd/config/config.go | 2 +- sandboxd/pool/pool_test.go | 12 +++++---- sandboxd/pool/refill.go | 17 ++++++++++++ sandboxd/pool/warmup_test.go | 50 ++++++++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 7 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 5da2140a..f46222bf 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -105,7 +105,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 with only `PATH` in its environment; 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 with only `PATH` in its environment; 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 `/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 | diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index 0fcc72c4..2231e7cb 100644 --- a/sandboxd/config/config.go +++ b/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. diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 6a45df17..19c17fa8 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -795,6 +795,9 @@ type fakeEngine struct { exportContent []byte caInstalls []string warmups [][]string + warmupSocks []string + warmupSnaps []int + warmupErr error staleReconciles []string installCAErr error diskAttachErr error @@ -1031,14 +1034,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.warmupSnaps = append(f.warmupSnaps, len(f.snapSaves)) + return f.warmupErr } func (f *fakeEngine) DiskAttach(_ context.Context, _ string, spec engine.VolumeSpec) error { diff --git a/sandboxd/pool/refill.go b/sandboxd/pool/refill.go index a3d712b6..a0908359 100644 --- a/sandboxd/pool/refill.go +++ b/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, p.key, sb) + } keep := false var fails int var wait time.Duration @@ -218,6 +221,20 @@ func (m *Manager) poolWarmup(key types.PoolKey) []string { return m.poolWarmups[key] } +// warmClone re-runs the pool warmup in a restored clone: a restore maps the golden memory lazily, +// so the pages the warmup touched fault in here, at refill, instead of under the first claim. +func (m *Manager) warmClone(ctx context.Context, key types.PoolKey, sb *types.Sandbox) error { + warmup := m.poolWarmup(key) + if len(warmup) == 0 { + return nil + } + if err := m.eng.Warmup(ctx, sb.VsockSocket, warmup); 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 diff --git a/sandboxd/pool/warmup_test.go b/sandboxd/pool/warmup_test.go index 63620e77..68db93ed 100644 --- a/sandboxd/pool/warmup_test.go +++ b/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.warmupSnaps[0] != 0 { + t.Fatalf("golden warmup ran after %d snapshot saves, want 0", eng.warmupSnaps[0]) + } stamp, err := os.ReadFile(final + warmupSidecarSuffix) if err != nil { t.Fatalf("read warmup sidecar: %v", err) @@ -87,3 +91,49 @@ 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() + defer eng.mu.Unlock() + if len(eng.warmups) != 2 || !slices.Equal(eng.warmups[0], argv) || !slices.Equal(eng.warmups[1], argv) { + t.Fatalf("warmups = %v, want %v once per clone", eng.warmups, argv) + } + if eng.warmupSocks[0] == "" || eng.warmupSocks[0] == eng.warmupSocks[1] { + t.Errorf("warmup sockets = %v, want one per clone", eng.warmupSocks) + } + }) +} + +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 + }) + }) +} From f1d1436d8e4b528f7ce43d8b514c04407df37946 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 11 Sep 2026 11:32:47 +0900 Subject: [PATCH 2/2] sandboxd: add release_delay_seconds to keep teardown out of a claim burst A release drops and journals the claim, then removes the VM inline. In a 100-way burst the early destroys land while other claims still run their first command: with destroys held until the burst ended the same command measured p95 254-293 ms, inline 620-646 ms. With release_delay_seconds set the release returns once the claim is gone and the VM, volume, egress and snapshot cleanup run that many seconds later; teardown failures are logged by the removal queue as before. --- docs/deploy.md | 1 + sandboxd/config/config.go | 29 +++++++++++++++++------------ sandboxd/pool/claim.go | 17 +++++++++++++++-- sandboxd/pool/pool.go | 8 +++++--- sandboxd/pool/pool_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index f46222bf..63645027 100644 --- a/docs/deploy.md +++ b/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 between a release returning and its VM teardown. The claim is dropped and journaled at release; only the `cocoon vm rm`, volume and snapshot cleanup wait, so a burst of releases does not compete with the claims still running. 0 tears down 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` | diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index 2231e7cb..7c24cd8e 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -231,6 +231,9 @@ type Config struct { // RefillConcurrency caps concurrent VM provisioning node-wide; 0 auto-scales with CPUs. RefillConcurrency int `json:"refill_concurrency,omitempty"` + // ReleaseDelaySeconds, when >0, delays a released VM's teardown so it stays out of a claim burst. + ReleaseDelaySeconds int `json:"release_delay_seconds,omitempty"` + // 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 _, n := range []struct { + name string + value int + }{ + {"release_delay_seconds", c.ReleaseDelaySeconds}, + {"refill_concurrency", c.RefillConcurrency}, + {"max_claims", c.MaxClaims}, + {"idle_hibernate_seconds", c.IdleHibernateSeconds}, + {"checkpoint_ttl_hours", c.CheckpointTTLHours}, + } { + if n.value < 0 { + return fmt.Errorf("%s must not be negative, got %d", n.name, n.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)") } diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index f7d248a6..da2582df 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -167,6 +167,21 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand m.purgeArchiveCk(ctx, id, ck, sb.Tenant) // archived: no local VM m.untrack(m.pendingCks, ck) } + m.counters.releases.Add(1) + m.recordUsage(ctx, usageEvent{Event: "release", ID: id, VMName: vmName}) + if m.releaseDelay > 0 && vmName != "" { + time.AfterFunc(m.releaseDelay, func() { + if err := m.teardown(ctx, id, sb, vmName, snap); err != nil { + log.WithFunc("pool.releaseResolved").Errorf(ctx, err, "delayed teardown of %s", id) + } + }) + return nil + } + return m.teardown(ctx, id, sb, vmName, snap) +} + +// teardown frees what a released claim still holds: volumes, the VM, its egress state and snapshot. +func (m *Manager) teardown(ctx context.Context, id string, sb *types.Sandbox, vmName, snap string) error { td := m.quiesceVolumes(ctx, sb) var err error if vmName == "" { @@ -176,8 +191,6 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand } m.disarmEgress(id, err == nil) m.dropSnap(ctx, snap) - m.counters.releases.Add(1) - m.recordUsage(ctx, usageEvent{Event: "release", ID: id, VMName: vmName}) return err } diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 5e921cf3..70135fd9 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -325,9 +325,10 @@ type Manager struct { atCapacityUntil time.Time atCapacityReason string - refillSem chan struct{} - probeSem chan struct{} - refillKick chan struct{} + refillSem chan struct{} + releaseDelay time.Duration + probeSem chan struct{} + refillKick chan struct{} } // NewManager builds a manager from the node config; ctx bounds backend construction. @@ -367,6 +368,7 @@ func NewManager(ctx context.Context, cfg *config.Config, eng Engine, secrets *eg dial: newEgressDialer(parsePrefixes(cfg.EgressInternalAllow)).DialContext, sweep: netfilter.SweepExcept, refillSem: make(chan struct{}, refill), + releaseDelay: time.Duration(cfg.ReleaseDelaySeconds) * time.Second, probeSem: make(chan struct{}, refill), refillKick: make(chan struct{}, 1), healSem: make(chan struct{}, maxConcurrentHeals), diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 19c17fa8..1ccd5f9b 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -214,6 +214,30 @@ func TestReleaseAfterResolveTearsDownOnce(t *testing.T) { } } +func TestReleaseDelayDefersTeardown(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) + } + if removed := eng.removedNames(); len(removed) != 0 { + t.Fatalf("removes=%v before the delay, want none", removed) + } + time.Sleep(2 * time.Second) + synctest.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)