Cover fork readiness lifecycle invariants - #325
Merged
Conversation
sjmiller609
marked this pull request as ready for review
August 6, 2026 18:04
hiroTamada
approved these changes
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Performance
20 sequential headless Firecracker forks per version (8 vCPU), measuring the fork request through the successful fork-identity response and verifying public CDP returned 200 on every run. The Envoy fixture used a static listener to isolate local process startup/readiness from xDS and network variance.
759635d0503e9f)All runs passed (20/20 per version). The after p95 leaves 28.3 seconds of headroom in the 30-second API timeout and 23.3 seconds inside the wrapper's 25-second readiness deadline.
But it's OK that this is slower because we are migrating off of envoy and the hot pools avoid hitting this case. Also we have another tier of idle pool which we can start filling with fanout.
Testing
cd server && go vet ./...cd server && go test -race $(go list ./... | grep -v /e2e$)cd server && make buildgit diff --check main...HEADNote
Medium Risk
Touches fork handoff timing and when the applied marker is written; incorrect gating could stall or falsely signal fork readiness to the API.
Overview
Tightens fork-identity readiness so the applied marker is only written when every post-fork probe succeeds, and documents the 5s response margin next to
ApplyTimeoutinforkidentity.writeForkIdentityAppliedMarkercentralizes that gate; readiness polling is split intowaitProbesReadyso tests can assert per-probe behavior (cdp, chromedriver, envoy) without a full boot.Snapshot start page seed Envoy teardown is guaranteed on all exit paths via a defer, with
cleanupSeedEnvoyWithfor unit tests of stop + bootstrap removal.Reviewed by Cursor Bugbot for commit 688cd10. Bugbot is set up for automated code reviews on this repo. Configure here.