sbx-mgr: pause writes only over the state it expects

architecture-baseline.toml

@@ -81,7 +81,7 @@
 "crates/libs/sbx/mgr/server/src/db/state_transitions.rs" = 1140
 "crates/libs/sbx/mgr/server/src/run.rs" = 1101
 "crates/libs/sbx/mgr/server/src/sandbox_eviction.rs" = 1273
-"crates/libs/sbx/mgr/server/src/service/activation_support.rs" = 1592
+"crates/libs/sbx/mgr/server/src/service/activation_support.rs" = 1574
 "crates/libs/sbx/mgr/server/src/service/auth_lookup.rs" = 1221
 "crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs" = 1455
 "crates/libs/sbx/mgr/server/src/service/cleanup.rs" = 2704

crates/libs/sbx/mgr/server/src/db/state_write.rs

@@ -13,8 +13,7 @@
 //! - A specialized compare-and-set writer's from-set is its SQL predicate,
 //!   narrowed to the states its production callers reach.
 //! - A generic `update_state` caller writes from any state the generic guards
-//!   admit. Its edge lists the states the caller relies on, and
-//!   [`EdgeKind::KnownRace`] edges list the other states it can overwrite.
+//!   admit. Its edge lists the states the caller relies on.
 //! - An empty from-set is an insert: no row existed.
 //! - A write over a row already in the target state changes no state and has
 //!   no edge.
@@ -110,11 +109,6 @@ pub enum EdgeKind {
     Compensation,
     /// Startup, inventory, quarantine or cleanup repair.
     Recovery,
-    /// A stale or unvalidated write a generic `update_state` caller can make:
-    /// a concurrent transition moved the row after the caller read it, or the
-    /// caller never checked the state. Not intended, and `update_state_from`
-    /// refuses to take one: converting the caller removes the edge.
-    KnownRace,
 }
 
 /// One family of persisted transitions: every state in `from` to `to`.
@@ -231,14 +225,15 @@ edge_table! {
 
     // Pause and resident resume.
 
-    /// `pause` and the suspend auto-pause: `update_state(Pausing)` after
-    /// `transition(Pausing)` on the read.
+    /// `pause` and the suspend auto-pause, after `transition(Pausing)` on the
+    /// read.
     PAUSE_BEGIN: Request, of(&[Started]) => Pausing;
-    /// `pause` and the suspend auto-pause: `update_state(Paused)`.
+    /// `pause` and the suspend auto-pause, after node-manager confirms the
+    /// pause.
     PAUSE_COMPLETE: Completion, of(&[Pausing]) => Paused;
-    /// `update_state(Started)` after a failed guest preparation in `pause`
-    /// and the suspend auto-pause; `update_state_best_effort(Started)` after a
-    /// node connect failure and in `pause_with_reconciliation`.
+    /// `pause` and the suspend auto-pause after a failed guest preparation;
+    /// `pause` after a node connect failure, and `pause_with_reconciliation`,
+    /// best effort.
     PAUSE_ROLLBACK: Compensation, of(&[Pausing]) => Started;
     /// `begin_resident_resume_with_policy`, and `begin_resident_resume`'s
     /// `update_state_from` without an activation policy.
@@ -425,14 +420,6 @@ edge_table! {
     /// `restore_adopt_orphan_or_insert_sandbox_in_transaction`'s insert
     /// branch from orphan materialization of an interrupted restore.
     ORPHAN_RESTORE_INSERT: Recovery, INSERT => Restoring;
-
-    // Known races.
-
-    /// Pause rollback (`update_state(Started)`, best effort or not) after a
-    /// concurrent Stop settled `stopped`.
-    PAUSE_ROLLBACK_AFTER_STOP: KnownRace, of(&[Stopped]) => Started;
-    /// Pause completion (`update_state(Paused)`) after a concurrent Stop.
-    PAUSE_COMPLETE_AFTER_STOP: KnownRace, of(&[Stopped]) => Paused;
 }
 
 pub use edges::EDGES;
@@ -522,16 +509,16 @@ pub enum StateWriteOutcome {
 /// autocommit it opens its own `IMMEDIATE` transaction, so the guard reads,
 /// the write and the job failure commit together; it never nests one.
 ///
-/// Inserts and [`EdgeKind::KnownRace`] edges, and targets `update_state`
-/// rejects (`quarantined`, `failed`), are `InvalidQuery`. A write that matches
+/// Inserts, and targets `update_state` rejects (`quarantined`, `failed`), are
+/// `InvalidQuery`. A write that matches
 /// no row although the row was read in a source state is
 /// `StatementChangedRows(0)`.
 pub fn update_state_from(
     conn: &Connection,
     id: &str,
     edge: &'static Edge,
 ) -> Result<StateWriteOutcome> {
-    if edge.kind == EdgeKind::KnownRace || edge.from.is_empty() {
+    if edge.from.is_empty() {
         return Err(rusqlite::Error::InvalidQuery);
     }
     write_state_from(conn, id, edge.from, edge.to)

crates/libs/sbx/mgr/server/src/db/tests/state_write_table.rs

@@ -14,11 +14,11 @@ use crate::state::SandboxState::{self, *};
 enum Disagreement {
     /// A request edge writes the pair; `can_transition_to` forbids it.
     RequestForbidden,
-    /// Only completion, compensation, recovery or known-race edges write the
-    /// pair; `can_transition_to` forbids it.
+    /// Only completion, compensation or recovery edges write the pair;
+    /// `can_transition_to` forbids it.
     WriteForbidden,
     /// `can_transition_to` allows the pair, but only completion,
-    /// compensation, recovery or known-race edges write it.
+    /// compensation or recovery edges write it.
     AllowedNotRequested,
     /// `can_transition_to` allows the pair; no writer implements it.
     AllowedUnwritten,
@@ -64,9 +64,7 @@ const ALLOWLIST: &[(SandboxState, SandboxState, Disagreement, &str)] = &[
     (Stopping,        CleanupRequired, WriteForbidden,      "an interrupted stop is handed to cleanup"),
     (Stopping,        CleanupFailed,   WriteForbidden,      "stop cleanup failed"),
     (Stopping,        Quarantined,     AllowedNotRequested, RECOVERY_QUARANTINE),
-    (Stopped,         Started,         WriteForbidden,      "known race: pause rollback after Stop"),
     (Stopped,         Stopping,        RequestForbidden,    STOP_SETTLED_SOURCE),
-    (Stopped,         Paused,          WriteForbidden,      "known race: pause completion after Stop"),
     (Stopped,         Suspended,       RequestForbidden,    "warm start and process-snapshot rollback restage a stopped row"),
     (Stopped,         Quarantined,     AllowedUnwritten,    NO_QUARANTINE_WRITER),
     (Deleting,        CleanupRequired, WriteForbidden,      "an interrupted delete is handed to cleanup"),

crates/libs/sbx/mgr/server/src/db/tests/update_state_from.rs

@@ -11,8 +11,7 @@ use super::update_state_matrix::{
     sweep_lifecycle_pairs_and_cleanup_shapes, sweep_quarantine_episode_and_missing_row, write_from,
 };
 use crate::db::{
-    EDGES, EdgeKind, StateSet, StateWriteOutcome, StateWriteRefusal, edges, test_conn,
-    update_state_from,
+    EDGES, StateSet, StateWriteOutcome, StateWriteRefusal, edges, test_conn, update_state_from,
 };
 use crate::state::SandboxState;
 
@@ -108,8 +107,7 @@ fn a_from_set_without_the_row_state_refuses_and_keeps_every_row() {
 }
 
 fn writable(edge: &crate::db::Edge) -> bool {
-    edge.kind() != EdgeKind::KnownRace
-        && !edge.from().is_empty()
+    !edge.from().is_empty()
         && !matches!(edge.to(), SandboxState::Quarantined | SandboxState::Failed)
 }
 
@@ -137,25 +135,15 @@ fn an_edge_writes_its_target_only_from_its_sources() {
     }
 }
 
-/// Known races, inserts and quarantine or failure targets are refused before
-/// the row is read. Known races are tried over a source the generic guards
-/// admit; inserts and quarantine targets over `started`.
+/// Inserts and quarantine or failure targets are refused before the row is
+/// read. Each is tried over `started`.
 #[test]
-fn known_race_insert_and_rejected_target_edges_are_refused() {
+fn insert_and_rejected_target_edges_are_refused() {
     let refused: Vec<_> = EDGES.iter().filter(|edge| !writable(edge)).collect();
-    assert!(
-        refused
-            .iter()
-            .any(|edge| edge.kind() == EdgeKind::KnownRace)
-    );
     assert!(refused.iter().any(|edge| edge.from().is_empty()));
     for edge in refused {
         let to = edge.to();
-        let from = edge
-            .from()
-            .iter()
-            .find(|from| cell(PLAIN[index(*from)], to) == Expected::Applied)
-            .unwrap_or(SandboxState::Started);
+        let from = SandboxState::Started;
         let case = format!("{}: {from} -> {to}", edge.name());
         let conn = test_conn();
         let id = seed(

crates/libs/sbx/mgr/server/src/service/activation_support.rs

@@ -1816,42 +1816,6 @@ fn node_manager_failure_is_terminal(code: tonic::Code) -> bool {
     }
 }
 
-pub(super) fn update_state_best_effort(
-    db: &Arc<Mutex<rusqlite::Connection>>,
-    sandbox_id: &str,
-    state: SandboxState,
-) {
-    let updated = {
-        let Ok(db) = crate::node_state_sync::connection_or_signal_failure(db) else {
-            return;
-        };
-        match crate::db::update_state(&db, sandbox_id, state) {
-            Ok(outcome) if outcome.is_applied() => true,
-            Ok(outcome) => {
-                tracing::warn!(
-                    sandbox_id,
-                    state = state.as_str(),
-                    %outcome,
-                    "sandbox state write was superseded"
-                );
-                false
-            }
-            Err(error) => {
-                tracing::warn!(
-                    sandbox_id,
-                    state = state.as_str(),
-                    %error,
-                    "failed to update sandbox state"
-                );
-                false
-            }
-        }
-    };
-    if updated {
-        mark_sandbox_dirty(db, sandbox_id);
-    }
-}
-
 pub(super) async fn notify_resume_via_spaced_host(
     sandbox_id: &str,
     reason: pplx_space_spaced_host_client::proto::ResumeReason,

crates/libs/sbx/mgr/server/src/service/cleanup_support.rs

@@ -327,9 +327,9 @@ impl StateWriteRejected {
     }
 }
 
-/// `update_state_best_effort` for [`crate::db::update_state_from`]: a refusal
-/// or an error is logged and leaves the row as it is, and only an applied
-/// write marks the sandbox dirty. `None` is a poisoned connection or a
+/// The best-effort form of [`crate::db::update_state_from`]: a refusal or an
+/// error is logged and leaves the row as it is, and only an applied write
+/// marks the sandbox dirty. `None` is a poisoned connection or a
 /// database error.
 pub(super) fn update_state_from_best_effort(
     db: &Arc<Mutex<rusqlite::Connection>>,

crates/libs/sbx/mgr/server/src/service/grpc_lifecycle.rs

@@ -5,7 +5,7 @@ use super::activation_support::{
     grpc_timeout_budget, operation_activation_attempt_id_from_wire, sandbox_event_trigger,
     space_api_route_identity_for_runtime_activation,
     space_api_route_identity_from_wake_start_context, space_api_route_identity_to_wire,
-    update_state_best_effort, wake_target_state_from_proto,
+    wake_target_state_from_proto,
 };
 use super::cleanup::{
     AsyncCleanupRequest, CleanupAdmission, CleanupDispatchDisposition, SynchronousCleanupRequest,