oxidecomputer / oxidecomputer/crucible
Consider merging `DsState::{Faulted, Offline}`
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 260
- Forks
- 34
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 8
Description
The documentation for enum DsState includes this lovely diagram:
* │
* ▼
* │
* ┌────┴──────┐
* ┌───────┐ │ ╞═════◄══════════════════╗
* │ Bad │ │ New ╞═════◄════════════════╗ ║
* │Version├──◄───┤ ├─────◄──────┐ ║ ║
* └───────┘ └────┬───┬──┘ │ ║ ║
* ▼ └───►───┐ │ ║ ║
* ┌────┴──────┐ │ │ ║ ║
* │ Wait │ │ │ ║ ║
* │ Active ├─►┐ │ │ ║ ║
* └────┬──────┘ │ │ ┌────┴───────┐ ║ ║
* ┌───────┐ ┌────┴──────┐ │ └──┤ │ ║ ║
* │ Bad │ │ Wait │ └────┤Disconnected│ ║ ║
* │Region ├──◄───┤ Quorum ├──►────┤ │ ║ ║
* └───────┘ └────┬──────┘ └────┬───────┘ ║ ║
* ........▼.......... │ ║ ║
* ┌─────────┐ : ┌────┴──────┐ : ▲ ║ ║
* │ Failed │ : │ Reconcile │ : │ ╔═╝ ║
* │Reconcile├─◄───┤ ├──►─────────┘ ║ ║
* └─────────┘ : └────┬──────┘ : ║ ║
* Not Active : │ : ▲ ▲ Not Active
* .............. . . . │. . . . ...................║...║............
* Active ▼ ║ ║ Active
* ┌────┴──────┐ ┌──────────╨┐ ║
* ┌─►─┤ Active ├─────►───┤Deactivated│ ║
* │ │ │ ┌──────┤ ├─◄──────┐
* │ └─┬───┬───┬─┘ │ └───────────┘ ║ │
* │ ▼ ▼ ▲ ▲ ║ │
* │ │ │ │ │ ║ │
* │ │ │ │ │ ║ │
* │ │ │ ▲ ┌─┘ ║ │
* │ │ │ ┌─┴──┴──┐ ║ │
* │ │ │ │Replay │ ║ │
* │ │ │ │ ├─►─┐ ║ │
* │ │ │ └─┬──┬──┘ │ ║ │
* │ │ ▼ ▼ ▲ │ ║ │
* │ │ │ │ │ │ ▲ │
* │ │ ┌─┴───┴──┴──┐ │ ┌────────────╨──┐ │
* │ │ │ Offline │ └─►─┤ Faulted │ │
* │ │ │ ├─────►─┤ │ │
* │ │ └───────────┘ └─┬─┬───────┬─┬─┘ │
* │ │ ▲ ▲ ▼ ▲ ▲
* │ └───────────►───────────┘ │ │ │ │
* │ │ │ │ │
* │ ┌────────┴─┐ ┌─┴─┴────┴─┐
* └──────────────────────┤ Live ├─◄─┤ Live │
* │ Repair │ │ Repair │
* │ │ │ Ready │
* └──────────┘ └──────────┘
It took me a while to wrap my head around the difference between Offline and Faulted:
- An
Offlinedownstairs comes back online through replay (i.e. sending it every job that it has missed). This means that if we have any offline downstairs in our set, we can't retire jobs which they have not yet seen. - A
Faulteddownstairs comes back online through live-repair (i.e. getting dirty extents from a different downstairs). We can retire jobs that have not been seen by a faulted downstairs.
Offline versus Faulted is not a property of the actual Downstairs, which is doing its own thing on the other side of the network boundary. Why do we need this distinction at all?
- We want the replay path because it's a fast way to bring back a Downstairs that hasn't diverged too far
- We need the
Faultedstate because we can't buffer arbitrary numbers of jobs forever in the Upstairs
In other words, the Offline → Faulted transition implements policy choices about how many jobs the Upstairs is allowed to keep around. Framed this way, it's not obvious why this logic should be attached to the DsState.
Instead, I'd like to propose attaching that logic to the Upstairs itself:
- Merge
DsState::FaultedandDsState::Offline(and deleteDsState::Replay; it's not doing anything) - If all Downstairs are happy, then the Upstairs should retire jobs on each completed flush (as usual)
- If any Downstairs are offline, then the Upstairs should keep some number of jobs
- Our policy choices about jobs and bytes-in-flight happen here!
- e.g. keeping 10K jobs or 1 GiB of data, whichever is smaller
- When a Downstairs reconnects, based on the jobs buffered in the Upstairs, decide whether it is eligible for replay or has to go through live-repair
This would cut through a bunch of gnarly code (e.g. the coupling between backpressure and transitions to DsState::Faulted), and seems logically consistent.
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 tracing the DsState transitions, especially Offline, Faulted, and Replay, alongside the Upstairs backpressure and job-retirement logic described in the issue. Done means the state distinction and Replay state are removed, policy for buffered jobs belongs to Upstairs, and reconnects choose replay or live-repair based on buffered work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, distributed-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100