oxidecomputer / oxidecomputer/crucible

Consider merging `DsState::{Faulted, Offline}`

Open
#1,258 1 comment 0 reactions 0 assignees View on GitHub

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 Offline downstairs 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 Faulted downstairs 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 Faulted state because we can't buffer arbitrary numbers of jobs forever in the Upstairs

In other words, the OfflineFaulted 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::Faulted and DsState::Offline (and delete DsState::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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.