Remove more unnecessary DsState variants (#1550)

- `DsState::Migrating` is not yet implemented - `DsState::Disconnected` is another transient state (see #1526). It's assigned in `DownstairsClient::restart_connection` (which halts the IO task), then we switch to `DsState::New` once the IO task halts. Those two states are never actually handled differently, so we can switch to `DsState::New` immediately
oxidecomputer · Nov 5, 2024 · 6277bf3 · 6277bf3
1 parent a9d614a
commit 6277bf3
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 44 deletions.
diff --git a/cmon/src/main.rs b/cmon/src/main.rs
@@ -92,13 +92,11 @@ fn short_state(dss: DsState) -> String {
         DsState::New => "NEW".to_string(),
         DsState::WaitActive => "WAC".to_string(),
         DsState::WaitQuorum => "WAQ".to_string(),
-        DsState::Disconnected => "DIS".to_string(),
         DsState::Reconcile => "REC".to_string(),
         DsState::Active => "ACT".to_string(),
         DsState::Faulted => "FLT".to_string(),
         DsState::LiveRepairReady => "LRR".to_string(),
         DsState::LiveRepair => "LR".to_string(),
-        DsState::Migrating => "MIG".to_string(),
         DsState::Offline => "OFF".to_string(),
         DsState::Deactivated => "DAV".to_string(),
         DsState::Disabled => "DIS".to_string(),

diff --git a/openapi/crucible-control.json b/openapi/crucible-control.json
@@ -93,13 +93,11 @@
           "new",
           "wait_active",
           "wait_quorum",
-          "disconnected",
           "reconcile",
           "active",
           "faulted",
           "live_repair_ready",
           "live_repair",
-          "migrating",
           "offline",
           "deactivated",
           "disabled",

diff --git a/upstairs/src/client.rs b/upstairs/src/client.rs
@@ -578,7 +578,6 @@ impl DownstairsClient {
 
             DsState::Deactivated
             | DsState::Reconcile
-            | DsState::Disconnected
             | DsState::WaitQuorum
             | DsState::WaitActive
             | DsState::Disabled => Some(DsState::New),
@@ -591,8 +590,6 @@ impl DownstairsClient {
             | DsState::Faulted
             | DsState::New
             | DsState::Replaced => None,
-
-            DsState::Migrating => panic!(),
         };
 
         // Jobs are skipped and replayed in `Downstairs::reinitialize`, which is
@@ -846,7 +843,6 @@ impl DownstairsClient {
         let new_state = match self.state {
             DsState::Active => DsState::Offline,
             DsState::Offline => DsState::Offline,
-            DsState::Migrating => DsState::Faulted,
             DsState::Faulted => DsState::Faulted,
             DsState::Deactivated => DsState::New,
             DsState::Reconcile => DsState::New,
@@ -859,7 +855,7 @@ impl DownstairsClient {
                  * downstairs to receive IO, so we go to the back of the
                  * line and have to re-verify it again.
                  */
-                DsState::Disconnected
+                DsState::New
             }
         };
 
@@ -960,11 +956,9 @@ impl DownstairsClient {
             DsState::New
             | DsState::WaitActive
             | DsState::WaitQuorum
-            | DsState::Disconnected
             | DsState::Reconcile
             | DsState::Deactivated
-            | DsState::Disabled
-            | DsState::Migrating => panic!(
+            | DsState::Disabled => panic!(
                 "enqueue should not be called from state {:?}",
                 self.state
             ),
@@ -1045,7 +1039,6 @@ impl DownstairsClient {
                     }
                 } else if old_state != DsState::New
                     && old_state != DsState::Faulted
-                    && old_state != DsState::Disconnected
                     && old_state != DsState::Replaced
                 {
                     panic!(
@@ -1157,12 +1150,6 @@ impl DownstairsClient {
                 // A move to Disabled can happen at any time we are talking
                 // to a downstairs.
             }
-            _ => {
-                panic!(
-                    "[{}] Missing check for transition {} to {}",
-                    self.client_id, old_state, new_state
-                );
-            }
         }
 
         if old_state != new_state {

diff --git a/upstairs/src/lib.rs b/upstairs/src/lib.rs
@@ -715,17 +715,17 @@ pub(crate) struct RawReadResponse {
  *    ┌─────────────►    New    ╞═════◄════════════════╗ ║
  *    │       ┌─────►           ├─────◄──────┐         ║ ║
  *    │       │     └────┬───┬──┘            │         ║ ║
- *    │       │          ▼   └───►───┐       │         ║ ║
- *    │    bad│     ┌────┴──────┐    │       │         ║ ║
- *    │ region│     │   Wait    │    │       │         ║ ║
+ *    │       │          ▼   └───►───┐ other │         ║ ║
+ *    │    bad│     ┌────┴──────┐    │ failures        ║ ║
+ *    │ region│     │   Wait    │    │       ▲         ║ ║
  *    │       │     │  Active   ├─►┐ │       │         ║ ║
- *    │       │     └────┬──────┘  │ │  ┌────┴───────┐ ║ ║
- *    │       │     ┌────┴──────┐  │ └──┤            │ ║ ║
- *    │       │     │   Wait    │  └────┤Disconnected│ ║ ║
- *    │       └─────┤  Quorum   ├──►────┤            │ ║ ║
- *    │             └────┬──────┘       └────┬───────┘ ║ ║
+ *    │       │     └────┬──────┘  │ │       │         ║ ║
+ *    │       │     ┌────┴──────┐  │ └───────┤         ║ ║
+ *    │       │     │   Wait    │  └─────────┤         ║ ║
+ *    │       └─────┤  Quorum   ├──►─────────┤         ║ ║
+ *    │             └────┬──────┘            │         ║ ║
  *    │          ........▼..........         │         ║ ║
- *    │failed    :  ┌────┴──────┐  :         ▲         ║ ║
+ *    │failed    :  ┌────┴──────┐  :         │         ║ ║
  *    │reconcile :  │ Reconcile │  :         │       ╔═╝ ║
  *    └─────────────┤           ├──►─────────┘       ║   ║
  *               :  └────┬──────┘  :                 ║   ║
@@ -786,12 +786,6 @@ pub enum DsState {
      * Waiting for the minimum number of downstairs to be present.
      */
     WaitQuorum,
-    /*
-     * We were connected, but did not transition all the way to
-     * active before the connection went away. Arriving here means the
-     * downstairs has to go back through the whole negotiation process.
-     */
-    Disconnected,
     /*
      * Initial startup, downstairs are repairing from each other.
      */
@@ -815,10 +809,6 @@ pub enum DsState {
      * This downstairs is undergoing LiveRepair
      */
     LiveRepair,
-    /*
-     * This downstairs is being migrated to a new location
-     */
-    Migrating,
     /*
      * This downstairs was active, but is now no longer connected.
      * We may have work for it in memory, so a replay is possible
@@ -858,9 +848,6 @@ impl std::fmt::Display for DsState {
             DsState::WaitQuorum => {
                 write!(f, "WaitQuorum")
             }
-            DsState::Disconnected => {
-                write!(f, "Disconnected")
-            }
             DsState::Reconcile => {
                 write!(f, "Reconcile")
             }
@@ -876,9 +863,6 @@ impl std::fmt::Display for DsState {
             DsState::LiveRepair => {
                 write!(f, "LiveRepair")
             }
-            DsState::Migrating => {
-                write!(f, "Migrating")
-            }
             DsState::Offline => {
                 write!(f, "Offline")
             }