refactor: centralize persisted sandbox lifecycle traversal
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 8.7k
- Forks
- 1.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 253
Description
Description
Evaluate a small refactor that centralizes the common traversal mechanics used by persisted sandbox lifecycle sweeps without hiding their distinct behavior.
start_persisted_sandboxes, stop_persisted_sandboxes_on_shutdown, and persisted lifecycle recovery repeat the same general pattern:
- Capture persisted sandbox IDs.
- Acquire the per-sandbox lifecycle gate.
- Re-read the sandbox record while holding the gate.
- Handle records that disappeared or could not be read.
- Inspect the current phase.
- Perform an asynchronous, phase-specific action while retaining the gate.
Consider extracting only these common mechanics into a small traversal or map helper. For example, the caller could select the concurrency policy and provide an asynchronous callback. Start-, stop-, and recovery-specific RPCs, outcome handling, state mutations, aggregation, and logging should remain explicit at their call sites.
The refactor must preserve:
- the shutdown stop sweep's bounded concurrency;
- startup's current ordering and error semantics;
- the lifecycle gate's lifetime across each operation; and
- the correctness property that the sandbox record is re-read only after acquiring its gate.
This is a readability and maintainability follow-up to #2743. It should not block #2743.
This refactor does not solve the race in which a new sandbox can be created after the shutdown sweep captures its inventory. It does not replace #2826, which tracks quiescing and draining in-flight lifecycle requests before shutdown cleanup.
Context
PR #2743 expanded persisted sandbox start and shutdown-stop reconciliation. Review of its lifecycle loops found that their shared concurrency and correctness mechanics are difficult to compare, increasing the risk that future changes cause the implementations to drift.
The abstraction should remain deliberately narrow. A single generic lifecycle sweep that obscures the meaningful differences between start, stop, and recovery would make the code harder to review.
Related:
- #2743
- #2826
Definition of Done
- Common traversal mechanics are centralized only if the resulting code is easier to read and review.
- Start-, stop-, and recovery-specific RPCs, outcome handling, state mutations, aggregation, and logging remain visible at their call sites.
- Stop preserves bounded concurrency, and start preserves its current ordering and error semantics.
- The lifecycle gate remains held for the full per-sandbox operation, with the persisted record re-read after gate acquisition.
- Tests cover gate/re-read behavior, disappeared-record skipping, read failures, and the caller-selected concurrency choices.
- Existing lifecycle behavior has no regression.
- The change does not claim to resolve or replace the shutdown-draining work tracked by #2826.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating start_persisted_sandboxes, stop_persisted_sandboxes_on_shutdown, and persisted lifecycle recovery, then compare their traversal mechanics and existing lifecycle tests. Preserve the gate lifetime, post-gate record reread, disappeared-record and read-failure handling, bounded stop concurrency, and startup ordering while keeping caller-specific behavior visible. Done means the shared mechanics are centralized only if the code is easier to review and all listed lifecycle behaviors remain covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100