Parallel Start and Stop: what each would take
- Dominant language
- Go
- Stars
- 3
- Forks
- 0
- Avg merge
- 16m
- Merged PRs (30d)
- 46
Description
## Problem
`Stop` releases a scope's instances one at a time, in reverse build order (`stopAll`, `di.go:2264`). Two services that do not depend on each other are still stopped in sequence, and every `Worker` is cancelled and waited for in turn against the one deadline, so the last worker gets the least of it. `samber/do` shuts children down in parallel and then takes services in waves, each wave being the set with no remaining dependants, run concurrently. Measured on two independent services whose stop hooks each sleep 100 ms: 100 ms there, 200 ms here.
The gain is real only for `Worker` hooks and for hooks that genuinely block, such as an HTTP server waiting out in-flight requests. An `OnStop` that closes a connection takes microseconds, and parallelism buys nothing.
## What holds it
Nothing in the stop machine. `stopIfNeeded` already waits per instance, not per scope, and the phase transitions are per instance under the owner's mutex; several instances stopping at once would not touch it. The obstacle is that the teardown does not know the graph, and that is deliberate.
- **Reverse build order needs no edges.** A dependency is published before the dependant that asked for it, so the reverse of `started` is a topological order for free. A wave needs the question "which instances have no dependant still alive", and that needs edges.
- **The only edges are `instance.deps` (`di.go:300`), recorded for `Explain` and `Graph`.** By rule nothing in the build, start or stop machine reads them, which is what makes a bug in the recorder cosmetic. Feeding them to `Stop` makes the recorder safety-critical.
- **The recorder has holes that reverse order covers and edges would not.** `dependOn` (`di.go:1693`) records only when the asking node has a binding. A `Get` through a scope a constructor kept, or from a goroutine started after it returned, is a fresh path and records nothing. Today that is mostly harmless: if A keeps its scope and later resolves C, and C was built before A, reverse order still stops C after A. With edges, C has no dependant on record and goes into the first wave while A still holds it. `samber/do` has the same hole and answers it with a fallback: when no service is free of dependants it shuts everything down at once in no order. That is not a fallback this library would accept.
- **The oracles predict a total order.** M5 in `lifecyclemodel_test.go` predicts reverse build order exactly, C3 in `concurrent_test.go` is the stop-order oracle, and the fuzzer catches "build order instead of reverse" in 0.06 s precisely because the order is predicted. A partial order over edges means the model tracks edges too, and the missed-deadline exemptions around C3 and C6 get more intricate. This is the layer where every review found defects, and it is most of the work.
- **Determinism.** Joined errors, `Observe` events and the example outputs ("server stopped" before "db closed") stop being reproducible.
The drain phase is a separate and harder problem: its hooks build instances and open scopes mid-sweep, which is why `sweepAll` repeats until a pass does no work. This issue is about the `OnStop` phase, where the scope is already stopped and nothing new appears.
## Options
1. **Accept it.** Document that `Stop` is sequential and deterministic, and that the deadline is shared. Zero cost. The two-server case is bounded by the deadline anyway.
2. **Cancel every `Worker` up front, release sequentially.** All workers of the scope get their cancel at once and the full window to return; `OnStop` hooks keep their order and no edges are needed. The cost is a semantic change: a dependant's worker sees its dependency's loop cancelled before its own is. That is the weaker of the two ordering promises, and it may be acceptable, since a cancelled worker still holds a live value until its `OnStop` runs.
3. **Stop in waves over recorded edges.** Repeatedly take the instances in `started` with no unstopped dependant, run their `stopIfNeeded` concurrently, join. Requires (a) making edges complete, by recording resolutions made through a kept scope, which means attaching the asking instance to the view rather than to the path, or declaring that such a value is unprotected; (b) promoting `deps` to a teardown input, with the tests that implies; (c) extending M5 and C3 to the DAG, and deciding how missed-deadline releases are ordered when several are outstanding at once.
4. **Opt-in.** Option 3 behind a `RunOption` or a per-binding marker, default sequential. Keeps the deterministic order for everyone who did not ask, but two teardown paths is two sets of oracles.
## Recommendation
1 until a use case turns up where the sequential wait is the actual problem rather than the deadline. If one does, 2 first: it addresses the only case where the time adds up, and it needs no edges. 3 only if the edges are made complete first, since a wave built on the current `deps` would run a hook against a value another service still holds, which is the one rule the whole teardown path exists to keep.
## Start
`Start` is sequential in the same way, and with its two phases kept, which they must be, it has the same problem.
**How it runs today.** Two phases. `buildEager` builds the eager bindings one at a time in registration order with the scope not yet running, so constructors run but no hook does. Then `running` is set and `claimNext` walks the `started` lists, this scope's then each child's, running one `OnStart` at a time in build order. The split is deliberate: constructors are meant to be cheap and free of side effects, `OnStart` is where a resource is acquired, so a construction failure anywhere in the graph fails `Start` before a port is bound or a connection opened. Nothing is externally visible from a `Start` that did not succeed.
**Where the ordering comes from after `Start`.** A service built later starts inside the resolution that built it (`startIfRunning`, `di.go:1813`), and a dependant's `await` waits out its dependency's start step. That orders starts at the ask, with no edges, and it is why two goroutines resolving two unbuilt services already start them in parallel today. It works only because a running scope starts as it builds. Setting `running` before the eager builds are done would hand `Start` that ordering for free, and it is not acceptable: a server with few dependencies would bind its port and take traffic into a graph still under construction, and then be rolled back when an unrelated eager constructor failed. The two-phase contract is worth more than the parallelism.
**With the phases kept, the hook phase is the stop problem again.** The values are built and held, so the order of `OnStart` hooks has to be planned from a list, and build order is a total order with no independence in it. Waves need edges, the only edges are `deps`, and the same hole applies in this direction: a kept-scope resolution of an already-built service is protected by build order and would not be by edges.
**The build phase can be parallel safely, and it is worth little.** Resolving the eager bindings concurrently with `running` still off is what concurrent `Get` calls before `Start` do today: build once however many race, cross-branch cycle detection, no hooks. It changes the promise that eager bindings build in registration order, and it may report several constructor failures at once. But constructors that follow the documented rule are cheap, so the time is in the hooks, which stay sequential.
**Recommendation for `Start`** is the same as for `Stop`: accept it. If edges are ever made complete for waves at stop, the same waves apply to the hook phase of `Start`, dependencies first. Parallel eager construction on its own is a small, safe change that can be made whenever the registration-order promise is not needed, and it is not worth making for the time it saves.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading di.go, especially stopAll, stopIfNeeded, dependOn, and startIfRunning, then review lifecyclemodel_test.go and concurrent_test.go. The issue presents several design options and recommends keeping the current sequential behavior, so there is no defined implementation or completion test until a direction is chosen.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100