feat(sandbox): pre-pause guest reclaim via envd#2551
feat(sandbox): pre-pause guest reclaim via envd#2551ValentaTomas wants to merge 6 commits intomainfrom
Conversation
Run sync, drop_caches, compact_memory, and fstrim -av on the live VM through envd's Process service immediately before pause to shrink the memfile/rootfs diff snapshot. Composed as a single bash chain with ';' separators so each step is best-effort, the orchestrator owns the deadline via Connect-Timeout-Ms, and all failures are non-fatal. Gated by reclaim-on-pause-timeout-ms (LD int flag, ms; default 0 = disabled). resume-build gains a matching --reclaim-timeout-ms override for local exercise.
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit db30f55. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Wraps each reclaim step (sync, drop_caches, compact_memory, fstrim) in its own `timeout -s KILL`. A stuck step (most realistically a slow sync on a large dirty backlog) cannot starve the rest, so compact_memory — the diff-critical step — always runs as long as its cap is > 0. Per-step ceilings are runtime-configurable via four new IntFlags: - reclaim-sync-timeout-ms (default 500) - reclaim-drop-caches-timeout-ms (default 200) - reclaim-compact-memory-timeout-ms (default 1000) - reclaim-fstrim-timeout-ms (default 500) Setting any per-step cap to 0 skips that step. The outer reclaim-on-pause-timeout-ms remains the master enable + Connect-Timeout-Ms cap. Pausing FC is unaffected by an in-flight guest sync that we time out: FC only drains in-flight virtio I/O before completing the pause; any unflushed dirty pages stay in the memfile snapshot and converge on resume. Per-step timeouts trade reclaim payoff, never correctness.
…uffix and join Three fixes triggered by Cursor Bugbot review of the previous commit and a follow-up question on the master flag: 1. Drop reclaim-on-pause-timeout-ms. Per-step caps (defaulting to 0) already encode "disabled by default": when every cap is 0 the script is empty and bestEffortReclaim short-circuits without calling envd. The outer Connect-Timeout-Ms is now derived from the sum of per-step caps + 500ms slack. 2. `timeout` accepts s/m/h/d (or fractional seconds), not `ms`. Format each cap as `%.3f` seconds (e.g. 500ms → 0.500). Without this, every step would silently fail with "invalid time interval". 3. Join parts with `; ` (not a single space) and append one trailing `true`. With space-joining, bash parsed `; true timeout ...` as `true` swallowing subsequent steps as args, so only `sync` ever ran. 4. Add `--foreground` to `timeout`. Without it, the SIGKILL doesn't reliably reach a stuck child when run from a non-interactive bash invoked by envd's Process service (verified empirically with `sh -c "sleep 5"` running its full 5s despite a 0.5s timeout). resume-build CLI: replace --reclaim-timeout-ms with a `--reclaim` bool that flips the per-step caps to sane local-test defaults (500/200/1000/500).
The previous refactor commit referenced Sandbox.StartEnvdProcess from reclaim.go and resume-build/main.go but the helper file itself was never tracked, breaking the orchestrator build (typecheck) on CI.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db30f55. Configure here.
| pc := processconnect.NewProcessClient(&http.Client{Transport: sandboxHttpClient.Transport}, addr) | ||
|
|
||
| req := connect.NewRequest(&process.StartRequest{ | ||
| Process: &process.ProcessConfig{Cmd: "/bin/bash", Args: []string{"-c", script}}, |
There was a problem hiding this comment.
Login shell flag dropped during refactor to shared helper
Medium Severity
The old runCommandInSandbox used Args: []string{"-l", "-c", command} to invoke bash as a login shell, sourcing /etc/profile and user profile scripts. The new shared StartEnvdProcess uses Args: []string{"-c", script}, dropping the -l flag. This means user-provided commands via --cmd, --cmd-pause, or --cmd-signal-pause in the resume-build CLI no longer get a login shell environment, potentially breaking commands that depend on PATH or environment variables set in profile scripts.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit db30f55. Configure here.
Adds `vm.compaction_proactiveness=0` to the base template's `/etc/sysctl.conf` so kcompactd no longer runs background page migrations in the guest. With 2 MiB host-side hugepage backing of guest RAM, every migration dirties a destination hugepage from the host UFFD's perspective and lands in the next memfile diff — with no snapshot-aligned benefit. The pre-pause `compact_memory` write (#2551) does the work deterministically right before we capture state. Existing templates inherit the change on rebuild.
2 participants
Adds an opt-in pre-pause step that runs
sync,drop_caches,compact_memory, andfstrim -avon the live VM via envd's Process service to shrink the memfile/rootfs diff. Each step is wrapped intimeout --foreground -s KILL, so a stuck step (most realistically a slowsyncon a large dirty backlog) cannot starve the rest —compact_memoryalways runs as long as its own cap is> 0.Pausing FC is unaffected by an in-flight guest
syncwe time out: FC only drains in-flight virtio I/O before completing the pause; any unflushed dirty pages stay in the memfile snapshot and converge on resume. Per-step timeouts trade reclaim payoff, never correctness.Disabled by default — every per-step cap defaults to 0, so the chain is empty until an operator opts in step by step. The orchestrator skips the envd call entirely when the chain is empty. The outer
Connect-Timeout-Msis derived from the sum of per-step caps plus a small slack.LD flags (all int, ms;
0skips that step):reclaim-sync-timeout-msreclaim-drop-caches-timeout-msreclaim-compact-memory-timeout-msreclaim-fstrim-timeout-msPairs cleanly with #2553 (disable proactive compaction in the guest base image), but is independent of it and of FPH (#2552). Split out from #2550.