fix(blaze): keep lifecycle state bound to opened directories - #2329
Draft
WeissonHan wants to merge 3 commits into
Draft
fix(blaze): keep lifecycle state bound to opened directories#2329WeissonHan wants to merge 3 commits into
WeissonHan wants to merge 3 commits into
Conversation
Lifecycle record I/O and runtime-directory lookup were spread across ServerState, SandboxManager, and API handlers. That left no single boundary where later ownership and recovery rules could be enforced consistently. This enables lifecycle code to apply one state access contract before any object-identity behavior changes. Make StateStore the sole production entry point for scans, loads, writes, and per-sandbox directory lookup, then route existing callers through it. The persisted layout and runtime behavior remain unchanged in this commit. Keeping this mechanical consolidation separate makes the following ownership changes independently reviewable and reversible. Signed-off-by: Weisson <Weisson@linux.alibaba.com>
Lifecycle records and sandbox work directories were addressed by rebuilding state_dir/<uuid> paths for each operation. Replacing the configured root or a UUID entry after daemon startup could redirect later persistence or cleanup, while a failed first publication could leave creation without a trustworthy ownership result. This enables sandbox creation and startup reconciliation to keep lifecycle writes, backend preparation, and orphan cleanup attached to the directory objects accepted by the daemon. Open and exclusively lock the state root, perform record I/O relative to retained descriptors, and publish each sandbox directory through a unique staging directory with no-replace and durability checks. Retain nonterminal directories until their terminal lifecycle record is committed. If publication or rollback remains uncertain, keep the owner retained and report RecoveryRequired instead of discarding the operation. Path-based spawners receive a descriptor-backed alias, and Firecracker derives its backend identifier from the sandbox UUID instead of that alias. The manager uses this protection immediately; transferring the owner into backend instance handles remains in the following commit. Daemon HTTP and wire operations, and the persisted layout, are unchanged. This design retains one state-root descriptor plus one directory descriptor per nonterminal sandbox to keep lifecycle updates and backend cleanup attached to their accepted objects. Fixes: 1f0cfac ("feat(anvil): scaffold local orchestrator crate skeleton") Signed-off-by: Weisson <Weisson@linux.alibaba.com>
Backend spawners received a runtime path but did not own the opened directory behind it. The state store could release its copy after a terminal commit while a successful or partially started backend handle still needed runtime artifacts. Firecracker also needed the descriptor to survive exec when its paths used /proc/self/fd. This enables successful and partially started backend handles to keep runtime artifacts attached to the directory object accepted for the sandbox, even if the configured UUID path is replaced during that daemon lifetime. Keep portable launch parameters in blaze-core and pair them with OwnedRunDir in a daemon-local BackendSpawnRequest. Validate the sandbox identifier, attach the owner to handles returned on both success and partial failure, and pass the typed owner through preparation and orphan cleanup. Firecracker inherits the directory descriptor into its child before exec. The manager drops the backend handle only after terminal retention bookkeeping, so cleanup does not lose its last owner early. Storage-sync test fixtures only adapt to the typed request; production synchronization behavior is unchanged. This tightens an internal Rust contract by removing the duplicate raw run_dir field. It does not change daemon HTTP or wire operations, persisted records, or the restart-reconciliation protocol. Backend handles extend the lifetime of the existing per-sandbox descriptor; Firecracker also inherits it into the child. Fixes: c33b495 ("feat(blaze): own sandbox runtime resources") Signed-off-by: Weisson <Weisson@linux.alibaba.com>
Collaborator
Author
|
@codex review Please review the complete pull request: every commit, the cumulative diff against the base branch, and the implementation and tests as one submitted change. Do not restrict the review to the most recently modified commit. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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.
Why
Lifecycle records and sandbox work directories were addressed by rebuilding
daemon.state_dir/<uuid>paths for later operations. A path says where to look, but it does not keep later persistence, startup compensation, or backend cleanup attached to the directory object the daemon accepted.Replacing the configured root or a UUID entry during the daemon lifetime could therefore redirect later work to another object. A failed first publication could also leave creation without a trustworthy ownership result.
What changed
Before: production scan, load, persistence, sandbox-directory lookup, and backend startup each carried or reconstructed paths. The daemon did not have one ownership boundary spanning the accepted state root, a sandbox directory, and the backend handle that continued using it.
After: the daemon opens and locks the lifecycle-state root before inventory. Record operations are relative to retained directories, new sandbox directories are published without replacing an existing entry, and nonterminal directories remain owned until their terminal record is committed. The accepted
OwnedRunDiris passed into backend startup; both successful and partially started backend handles retain it. Firecracker inherits the required directory descriptor into the child before exec.The resulting flow is:
Commits
32aee3e09 refactor(blaze): centralize daemon state accessStateStore.83899c248 fix(blaze): own persisted state directoriesRecoveryRequiredwhen publication or rollback remains uncertain.5e740cac7 fix(blaze): retain runtime directory ownersOwnedRunDir, validate sandbox identity, retain the owner in success and partial-failure handles, and let Firecracker inherit the descriptor it uses for runtime artifacts.These commits form one PR because they establish one
state_dir/<uuid>continuity invariant. The first commit creates the access boundary, the second binds lifecycle records to opened directories, and the third carries that same directory owner into backend lifetime. Splitting the final two into separate PRs would leave records and runtime artifacts with different ownership contracts.Related issue
closes #2327
Relates to #2254.
User / Agent impact
No new daemon operation, configuration key, or persisted format is introduced. Existing create, restart reconciliation, destroy, and warm paths keep their lifecycle-state and backend-runtime work attached to the directory objects accepted by the daemon. Uncertain publication or cleanup retains recovery responsibility instead of silently dropping ownership.
This PR covers lifecycle-state and backend runtime directories only. Independent storage roots, the earlier startup handoff between peer-root validation and lifecycle-state opening, and checkpoint namespaces remain tracked by #2254. Scan atomicity, directory/record UUID consistency, and staging-cleanup identity checks are tracked by #2328.
Risk and compatibility
The daemon now retains one state-root descriptor plus one directory descriptor per nonterminal sandbox. Backend handles extend the lifetime of the existing per-sandbox descriptor; Firecracker also inherits it into the child.
The internal Rust contract is intentionally tightened: the workspace-only
SpawnRequestno longer carries a duplicate rawrun_dir, andblazeduses a daemon-local typed request. The workspace crates are not published. Daemon HTTP/wire operations, configuration, persisted lifecycle records, and the restart-reconciliation protocol are unchanged.Validation
Validation ran on Linux x86_64, kernel 6.6.102, with rustc/cargo 1.93.1. C1 was tested at its final SHA. C2 and C3 were tested from fresh archives of their exact final source trees; later message-only rewrites changed commit metadata without changing tree OIDs (
3d8c8808281bbcabac47252732e58aa649b4c35band2ff01a2646acbedb6fe8623fa63ce031a4448029). Default and all-features runs used separate initially empty Cargo targets.The following passed for every commit:
cargo fmt --all --checkcargo build --workspace --all-targets --lockedcargo clippy --workspace --all-targets --locked -- -D warningscargo test --workspace --lockedcargo build --workspace --all-targets --all-features --lockedcargo clippy --workspace --all-targets --all-features --locked -- -D warningscargo test --workspace --all-features --lockedRUSTDOCFLAGS="-D warnings"32aee3e0983899c2485e740cac7At the final commit, a real Firecracker create/destroy run replaced the configured UUID pathname while the sandbox was running. Status and destroy operations continued through the accepted directory object; the terminal record and backend stop marker appeared only there, the replacement directory remained empty, the backend process exited, and the daemon remained healthy.
PR Lint, CLA, and the repository CI workflow passed for the submitted head.
Documentation and rollback
No operator documentation changes are required because the daemon API, configuration, and persisted layout are unchanged. Reverting these three commits restores path-based lifecycle-state and backend-runtime handling; existing lifecycle records remain readable.