From 4061f30cc59f50139bf959fd73284777590df176 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Tue, 20 Oct 2026 18:10:34 +0000 Subject: [PATCH] sbx-mgr: callers use the store's typed reads Convert every production caller of the readers that `db::read` and `db::scan` consolidate, and make those readers non-public outside the store. Exact-equivalent conversions only: each call site now calls the accessor that wraps the reader it called before, so it runs the same statements with the same decoder and missing-row rule. - Strict call sites stay strict: every former `get_sandbox` caller reads `read::sandbox`, including the cleanup admission checks, teardown suspend, pool clone, network runtime, pause and wake lookups that fail closed on a corrupt row. - Narrow projections stay narrow: `node_execution_fence` keeps the previous execution (`sandbox_view(..).node_execution_fence()`), the eviction CAS keeps the raw `state_changed_at` witness (`read::state_changed_at_witness`), and the snapshot-upload reclaim keeps the one-statement suspended-with-pin `EXISTS`. Its two call sites now say why it is a fence: the reclaim holds the connection guard across the unlink, and the earlier due check is only a hint because that guard is released before node-manager is asked. - Receipt reads go through `typed_sandbox_view`, so the outbox is read under the caller's `SandboxId`, as before. The recovery claim check opens its view on the sandbox the claim names. - The State RPC reads its batch through `read::state_page` and rebuilds the same response: state-only rows first, then decoded rows, with the same status codes and messages. One difference: the state-only rows' metric and two warnings are skipped if any later read in the same batch fails (a later row's budget, ambiguity or raw-state read, or the publication, state-change or recovery reads). Batches that finished earlier in the RPC still emit; responses, row order, status codes and messages are unchanged. - The cleanup-obligation loop keeps a `CleanupObligationScan` and only advances it when a page is read, as it kept `recovery_cursor` before. - The test-only `recover_prelaunch_startup_states_in` reads like its production siblings, and `sandbox_eviction/tests.rs` imports the two store counters it calls instead of reaching them through its parent. 79 readers (74 free functions and 5 cursor methods) leave the public API outside the `test-support` feature: `db_private_outside_tests!`, now importable within `db`, declares each as `pub(in crate::db)` in production and `pub` for sbx-mgr's tests, which still compare against them unchanged. That is the narrowest visibility that compiles: the consolidated reads in `db::read` and `db::scan` call every one of them. String literals in every changed store file are byte-identical to the base; only the code around them is re-indented inside the wrappers (review with `git diff -w`). The store's AGENTS.md now says to extend `db::read`/`db::scan` before adding a public reader. --- .../sbx/mgr/server/src/lifecycle_metrics.rs | 2 +- .../sbx/mgr/server/src/local_artifacts.rs | 29 +- .../sbx/mgr/server/src/node_state_sync.rs | 2 +- .../mgr/server/src/route_activation_claims.rs | 10 +- .../sbx/mgr/server/src/sandbox_eviction.rs | 40 +- .../mgr/server/src/sandbox_eviction/tests.rs | 1 + .../server/src/service/activation_support.rs | 20 +- .../sbx/mgr/server/src/service/auth_lookup.rs | 35 +- .../mgr/server/src/service/cache_cleanup.rs | 18 +- .../src/service/cache_cleanup/periodic.rs | 10 +- .../sbx/mgr/server/src/service/cleanup.rs | 145 ++-- .../server/src/service/cleanup_executor.rs | 44 +- .../mgr/server/src/service/cleanup_support.rs | 11 +- .../mgr/server/src/service/cleanup_targets.rs | 13 +- .../libs/sbx/mgr/server/src/service/create.rs | 2 +- .../mgr/server/src/service/create_support.rs | 4 +- .../sbx/mgr/server/src/service/eviction.rs | 46 +- .../server/src/service/execution_startup.rs | 2 +- .../sbx/mgr/server/src/service/grpc_core.rs | 31 +- .../service/grpc_lifecycle/artifact_backup.rs | 18 +- .../service/grpc_lifecycle/pause_resume.rs | 2 +- .../grpc_lifecycle/sandbox_network_policy.rs | 2 +- .../grpc_lifecycle/suspend_snapshot.rs | 7 +- .../grpc_lifecycle/teardown_suspend.rs | 69 +- .../src/service/grpc_pool_wait_clone.rs | 17 +- .../src/service/grpc_sandbox_transfer.rs | 6 +- .../src/service/grpc_snapshot_transfer.rs | 2 +- .../service/grpc_spaced_process_snapshots.rs | 6 +- .../server/src/service/grpc_state_snapshot.rs | 146 ++-- crates/libs/sbx/mgr/server/src/service/mod.rs | 39 +- .../mgr/server/src/service/network_runtime.rs | 2 +- .../sbx/mgr/server/src/service/reconcile.rs | 9 +- .../sbx/mgr/server/src/service/recovery.rs | 55 +- .../src/service/recovery/staging_cleanup.rs | 9 +- .../server/src/service/recovery_cross_node.rs | 75 +- .../mgr/server/src/service/spaced_bridge.rs | 2 +- .../libs/sbx/mgr/server/src/service/start.rs | 13 +- .../server/src/service/startup_resident.rs | 4 +- crates/libs/sbx/mgr/server/src/snapshot_gc.rs | 2 +- .../sbx/mgr/server/src/snapshot_scheduler.rs | 5 +- .../mgr/server/src/snapshot_upload_jobs.rs | 21 +- crates/libs/sbx/mgr/server/src/watchdog.rs | 16 +- crates/libs/sbx/mgr/store/AGENTS.md | 4 + crates/libs/sbx/mgr/store/src/db.rs | 26 +- .../sbx/mgr/store/src/db/auto_lifecycle.rs | 184 ++--- .../src/db/auto_lifecycle_delete_receipts.rs | 255 ++++--- .../sbx/mgr/store/src/db/cleanup_episodes.rs | 534 ++++++------- .../libs/sbx/mgr/store/src/db/cleanup_jobs.rs | 6 +- .../mgr/store/src/db/cleanup_target_scan.rs | 127 ++-- crates/libs/sbx/mgr/store/src/db/eviction.rs | 66 +- .../sbx/mgr/store/src/db/eviction_scan.rs | 203 ++--- .../sbx/mgr/store/src/db/guest_health_scan.rs | 81 +- crates/libs/sbx/mgr/store/src/db/handle.rs | 14 +- crates/libs/sbx/mgr/store/src/db/inventory.rs | 169 +++-- .../sbx/mgr/store/src/db/inventory_read.rs | 227 +++--- .../mgr/store/src/db/local_artifact_leases.rs | 161 ++-- .../sbx/mgr/store/src/db/local_artifacts.rs | 90 +-- .../sbx/mgr/store/src/db/network_policy.rs | 28 +- crates/libs/sbx/mgr/store/src/db/pool.rs | 40 +- .../libs/sbx/mgr/store/src/db/quarantine.rs | 76 +- .../mgr/store/src/db/quarantine_recovery.rs | 206 ++--- .../sbx/mgr/store/src/db/quarantine_scan.rs | 15 +- .../store/src/db/restore_transfer_scopes.rs | 102 +-- .../store/src/db/route_activation_claims.rs | 39 +- crates/libs/sbx/mgr/store/src/db/sandboxes.rs | 714 +++++++++--------- .../libs/sbx/mgr/store/src/db/snapshot_gc.rs | 153 ++-- .../sbx/mgr/store/src/db/startup_recovery.rs | 62 +- .../sbx/mgr/store/src/db/teardown_jobs.rs | 70 +- .../sbx/mgr/store/src/db/teardown_suspend.rs | 294 ++++---- 69 files changed, 2659 insertions(+), 2279 deletions(-) diff --git a/crates/libs/sbx/mgr/server/src/lifecycle_metrics.rs b/crates/libs/sbx/mgr/server/src/lifecycle_metrics.rs index d4187b10a..0301a5137 100644 --- a/crates/libs/sbx/mgr/server/src/lifecycle_metrics.rs +++ b/crates/libs/sbx/mgr/server/src/lifecycle_metrics.rs @@ -66,7 +66,7 @@ pub(crate) async fn run(db: crate::db::SharedConnection, shutdown_token: Cancell continue; } }; - crate::db::sandbox_lifecycle_counts(&db) + crate::db::read::counts(&db, crate::db::read::ByLifecycle) }; let counts = match counts { Ok(counts) => counts, diff --git a/crates/libs/sbx/mgr/server/src/local_artifacts.rs b/crates/libs/sbx/mgr/server/src/local_artifacts.rs index 81c730cc0..55f6f88a4 100644 --- a/crates/libs/sbx/mgr/server/src/local_artifacts.rs +++ b/crates/libs/sbx/mgr/server/src/local_artifacts.rs @@ -207,7 +207,10 @@ impl LocalArtifacts { local_artifact_id: LocalArtifactId, ) -> crate::db::Result> { let conn = self.db_conn()?; - crate::db::get_local_artifact(&conn, local_artifact_id) + crate::db::read::local_artifact( + &conn, + crate::db::read::LocalArtifactKey::Id(local_artifact_id), + ) } /// Read-only fetch by `(sandbox_id, snapshot_name)` for the @@ -218,7 +221,13 @@ impl LocalArtifacts { snapshot_name: &SnapshotName, ) -> crate::db::Result> { let conn = self.db_conn()?; - crate::db::get_named_local_artifact(&conn, sandbox_id, snapshot_name) + crate::db::read::local_artifact( + &conn, + crate::db::read::LocalArtifactKey::Named { + sandbox_id, + snapshot_name, + }, + ) } /// Every sandbox whose **current lifecycle artifact** this node cannot @@ -330,7 +339,7 @@ impl LocalArtifacts { /// with no report, which every consumer already has to handle. pub fn list_pending_backups(&self) -> crate::db::Result> { let conn = self.db_conn()?; - leases::list_pending_backups(&conn) + crate::db::scan::pending_backups(&conn) } /// Inventory-only observation. Callers must still acquire a checkout before @@ -340,7 +349,9 @@ impl LocalArtifacts { sandbox_id: &str, ) -> Result, crate::db::inventory_read::InventoryReadError> { let conn = self.db_conn()?; - leases::list_named_publications(&conn, sandbox_id) + crate::db::read::sandbox_view(&conn, sandbox_id) + .artifacts() + .named() } /// Mark the artifact's remote backup as durably completed. This @@ -603,7 +614,13 @@ impl LocalArtifacts { ) -> Result { let id = { let conn = self.db_conn()?; - let row = crate::db::get_named_local_artifact(&conn, sandbox_id, snapshot_name)?; + let row = crate::db::read::local_artifact( + &conn, + crate::db::read::LocalArtifactKey::Named { + sandbox_id, + snapshot_name, + }, + )?; match row { Some(r) => r.local_artifact_id, None => { @@ -775,7 +792,7 @@ impl LocalArtifacts { /// Retain rows in every state, including historical deletion conflicts. pub(crate) fn gc_staging_is_unowned(&self, id: LocalArtifactId) -> crate::db::Result { let conn = self.db_conn()?; - leases::gc_staging_is_unowned(&conn, &id.to_string()) + crate::db::read::local_artifact_unowned(&conn, id) } /// Examine at most `max_records` raw rows, advancing past owned, present, diff --git a/crates/libs/sbx/mgr/server/src/node_state_sync.rs b/crates/libs/sbx/mgr/server/src/node_state_sync.rs index 8268df572..3b92d8815 100644 --- a/crates/libs/sbx/mgr/server/src/node_state_sync.rs +++ b/crates/libs/sbx/mgr/server/src/node_state_sync.rs @@ -157,7 +157,7 @@ pub(crate) fn start_dirty_outbox_worker( let Ok(db) = connection_or_signal_failure(&db) else { return; }; - match crate::db::list_node_dirty_refs(&db, batch_limit) { + match crate::db::scan::node_dirty_batch(&db, batch_limit) { Ok(rows) => rows, Err(error) => { tracing::warn!(%error, "failed to read node dirty outbox"); diff --git a/crates/libs/sbx/mgr/server/src/route_activation_claims.rs b/crates/libs/sbx/mgr/server/src/route_activation_claims.rs index 984fd1b13..b9b6a0b36 100644 --- a/crates/libs/sbx/mgr/server/src/route_activation_claims.rs +++ b/crates/libs/sbx/mgr/server/src/route_activation_claims.rs @@ -1,6 +1,6 @@ use crate::db::Connection; -use crate::db::route_activation_claims::{OpenRouteActivationClaim, claim_matches}; +use crate::db::route_activation_claims::OpenRouteActivationClaim; #[derive(Debug, thiserror::Error)] pub(crate) enum OpenRouteActivationAdmissionError { @@ -39,10 +39,14 @@ pub(crate) fn validate_cluster_fenced_activation( sandbox_id: &str, claim: &OpenRouteActivationClaim, ) -> Result<(), OpenRouteActivationAdmissionError> { - let provenance = crate::db::activation_route_provenance(conn, sandbox_id)?; + let provenance = + crate::db::read::sandbox_view(conn, sandbox_id).activation_route_provenance()?; let retained = observed_generation_floor(provenance); let requested = claim.requested_generation(); - if claim_matches(conn, sandbox_id, claim)? { + if crate::db::read::sandbox_view(conn, sandbox_id) + .route_claim() + .matches(claim)? + { return Ok(()); } if let Some(retained) = retained diff --git a/crates/libs/sbx/mgr/server/src/sandbox_eviction.rs b/crates/libs/sbx/mgr/server/src/sandbox_eviction.rs index cbb881e22..fc865a27e 100644 --- a/crates/libs/sbx/mgr/server/src/sandbox_eviction.rs +++ b/crates/libs/sbx/mgr/server/src/sandbox_eviction.rs @@ -96,7 +96,6 @@ use tracing::{info, warn}; use uuid::Uuid; pub(crate) use crate::auto_lifecycle::now_secs; -use crate::db::eviction_scan::{count_malformed_suspended_updated_at, count_resident_rows}; use crate::service::SandboxManagerService; /// Runtime configuration for the eviction loop. Loaded from env on @@ -533,12 +532,14 @@ fn query_candidates_after( cursor.map(|cursor| i64::try_from(cursor.idle_witness_secs).unwrap_or(i64::MAX)); let cursor_sandbox_id = cursor.map(|cursor| cursor.sandbox_id.as_str()); let cutoff = i64::try_from(idle_cutoff_epoch_secs).unwrap_or(i64::MAX); - let rows = crate::db::eviction_scan::idle_suspended_candidates( + let rows = crate::db::scan::eviction_candidates( conn, - cutoff, - cursor_witness, - cursor_sandbox_id, - limit, + crate::db::scan::Proofable { + cutoff, + cursor_witness, + cursor_sandbox_id, + limit, + }, )?; Ok(rows .into_iter() @@ -579,7 +580,13 @@ pub(crate) fn list_legacy_reupload_candidates( idle_cutoff_epoch_secs: u64, limit: usize, ) -> crate::db::Result> { - crate::db::eviction_scan::legacy_reupload_candidates(conn, idle_cutoff_epoch_secs, limit) + crate::db::scan::eviction_candidates( + conn, + crate::db::scan::ProoflessLegacy { + idle_cutoff_epoch_secs, + limit, + }, + ) } /// Gather the local half of the eviction evidence plus the sandbox's @@ -598,14 +605,18 @@ pub(crate) fn gather_local_evidence( conn: &Connection, sandbox_id: &str, ) -> crate::db::Result> { - let row = crate::db::eviction_scan::artifact_pin_and_updated_at(conn, sandbox_id)?; + let row = crate::db::read::sandbox_view(conn, sandbox_id) + .pinned_artifact() + .pin_and_updated_at()?; let Some((pin, updated_at)) = row else { return Ok(None); }; let updated_at_secs = updated_at.unsigned(); if let Some(pin) = pin { - let artifact = crate::db::eviction_scan::owned_artifact_evidence(conn, &pin, sandbox_id)?; + let artifact = crate::db::read::sandbox_view(conn, sandbox_id) + .pinned_artifact() + .owned_evidence(&pin)?; let evidence = match artifact { Some((kind, id, backup_marker)) => { match (id.parse(), kind.parse()) { @@ -627,7 +638,7 @@ pub(crate) fn gather_local_evidence( // Unpinned: the newest upload job regardless of status (a failed // newest upload must still count — it proves newer-than-backed // local state exists). - let job = crate::db::eviction_scan::latest_upload_job(conn, sandbox_id)?; + let job = crate::db::read::sandbox_view(conn, sandbox_id).latest_upload()?; let latest_job = job.and_then(|(snapshot_id, created_at)| { let snapshot_id = snapshot_id?.trim().parse::().ok()?; // Invalid time cannot prove recency, even when the sandbox itself @@ -1239,10 +1250,11 @@ async fn evict_tick( let conn = service_for_counts .db_conn() .map_err(|error| format!("eviction row counts db_conn: {error}"))?; - let malformed = count_malformed_suspended_updated_at(&conn) - .map_err(|error| format!("count_malformed_suspended_updated_at: {error}"))?; - let resident = - count_resident_rows(&conn).map_err(|error| format!("count_resident_rows: {error}"))?; + let malformed = + crate::db::read::counts(&conn, crate::db::read::MalformedSuspendedUpdatedAt) + .map_err(|error| format!("count_malformed_suspended_updated_at: {error}"))?; + let resident = crate::db::read::counts(&conn, crate::db::read::Resident) + .map_err(|error| format!("count_resident_rows: {error}"))?; Ok::<_, String>((malformed, resident)) }) .await diff --git a/crates/libs/sbx/mgr/server/src/sandbox_eviction/tests.rs b/crates/libs/sbx/mgr/server/src/sandbox_eviction/tests.rs index 4f772da1f..8a3e2776f 100644 --- a/crates/libs/sbx/mgr/server/src/sandbox_eviction/tests.rs +++ b/crates/libs/sbx/mgr/server/src/sandbox_eviction/tests.rs @@ -1,3 +1,4 @@ +use crate::db::eviction_scan::{count_malformed_suspended_updated_at, count_resident_rows}; use crate::db::fixture::params; use pplx_space_common_core::types::{LocalArtifactId, SnapshotId}; diff --git a/crates/libs/sbx/mgr/server/src/service/activation_support.rs b/crates/libs/sbx/mgr/server/src/service/activation_support.rs index f0e598c7e..bb2531cb1 100644 --- a/crates/libs/sbx/mgr/server/src/service/activation_support.rs +++ b/crates/libs/sbx/mgr/server/src/service/activation_support.rs @@ -364,7 +364,7 @@ impl SandboxManagerService { } let provenance = { let db = self.db_conn()?; - match crate::db::activation_route_provenance(&db, sandbox_id) { + match crate::db::read::sandbox_view(&db, sandbox_id).activation_route_provenance() { Ok(provenance) => { if requested_can_open_claim && let Some(claim) = crate::db::route_activation_claims::OpenRouteActivationClaim::for_cluster_fenced_identity( @@ -454,7 +454,7 @@ impl SandboxManagerService { } let provenance = { let db = self.db_conn()?; - match crate::db::activation_route_provenance(&db, sandbox_id) { + match crate::db::read::sandbox_view(&db, sandbox_id).activation_route_provenance() { Ok(provenance) => provenance, Err(error) if error.is_no_rows() => { return Err( @@ -1003,10 +1003,12 @@ pub(super) async fn configure_process_env_for_activation( let read = || { let conn = crate::node_state_sync::connection_or_signal_failure(db) .map_err(|error| Status::internal(error.to_string()))?; - let sandbox = crate::db::get_sandbox(&conn, sandbox_id) + let sandbox = crate::db::read::sandbox(&conn, sandbox_id) .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("activation sandbox disappeared"))?; - let episode = crate::db::cleanup_episode(&conn, sandbox_id) + let episode = crate::db::read::sandbox_view(&conn, sandbox_id) + .cleanup_episode() + .strict() .map_err(|error| Status::internal(format!("db error: {error}")))?; // Start and restore pre-arm rollback before guest setup. Its presence // is valid until cleanup is requested or another activation owns it. @@ -1116,11 +1118,13 @@ fn activation_failure_status_if_current( .lock() .map_err(|error| Status::internal(format!("database mutex poisoned: {error}"))) .and_then(|db| { - let row = crate::db::get_sandbox(&db, sandbox_id) + let row = crate::db::read::sandbox(&db, sandbox_id) .map_err(|error| Status::internal(format!("db error: {error}")))?; let episode = cleanup_id .map(|cleanup_id| { - crate::db::cleanup_episode(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .strict() .map(|episode| { episode.is_some_and(|episode| { episode.id == cleanup_id && episode.requested_at.is_none() @@ -1295,7 +1299,7 @@ where ); return Err(activation_completion_lost_status(SandboxState::Starting)); } - if let Ok(Some(sandbox)) = crate::db::get_sandbox(&db, sandbox_id) { + if let Ok(Some(sandbox)) = crate::db::read::sandbox(&db, sandbox_id) { emit_guest_observed_specs( sandbox_id, &sandbox.template, @@ -1481,7 +1485,7 @@ async fn mark_restore_after_network_ready( let db = db .lock() .map_err(|error| Status::internal(format!("database mutex poisoned: {error}")))?; - crate::db::get_sandbox(&db, sandbox_id) + crate::db::read::sandbox(&db, sandbox_id) .map_err(|error| Status::internal(format!("read restoring sandbox: {error}")))? .ok_or_else(|| activation_completion_lost_status(SandboxState::Restoring))? .pool_template diff --git a/crates/libs/sbx/mgr/server/src/service/auth_lookup.rs b/crates/libs/sbx/mgr/server/src/service/auth_lookup.rs index f93647229..28918264a 100644 --- a/crates/libs/sbx/mgr/server/src/service/auth_lookup.rs +++ b/crates/libs/sbx/mgr/server/src/service/auth_lookup.rs @@ -548,16 +548,17 @@ impl SandboxManagerService { /// Returns error if not found or ambiguous. pub(super) fn lookup_sandbox(&self, sandbox_id: &str) -> Result { let db = self.db_conn()?; - if let Some(sandbox) = crate::db::get_sandbox(&db, sandbox_id) + if let Some(sandbox) = crate::db::read::sandbox(&db, sandbox_id) .map_err(|e| Status::internal(format!("db error: {e}")))? { return Ok(sandbox); } - let matches = crate::db::find_by_prefix(&db, sandbox_id) - .map_err(|e| Status::internal(format!("db error: {e}")))?; + let matches = + crate::db::scan::resolve_selector(&db, crate::db::scan::RpcSelector, sandbox_id) + .map_err(|e| Status::internal(format!("db error: {e}")))?; match matches.as_slice() { - [id] => crate::db::get_sandbox(&db, id) + [id] => crate::db::read::sandbox(&db, id) .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found(format!("sandbox not found: {id}"))), [] => Err(Status::not_found(format!( @@ -764,7 +765,7 @@ impl SandboxManagerService { // transition is deferred until after the gate succeeds. let sandbox = { let db = self.db_conn()?; - let sandbox = crate::db::get_sandbox(&db, sandbox_id) + let sandbox = crate::db::read::sandbox(&db, sandbox_id) .map_err(|e| Status::internal(format!("db error: {e}")))? .ok_or_else(|| Status::not_found(format!("sandbox not found: {sandbox_id}")))?; sandbox @@ -1031,9 +1032,15 @@ impl SandboxManagerService { uid: u32, ) -> Result { let db = self.db_conn()?; - let mut matches = crate::db::list_by_spaced_host_pid_uid(&db, pid, uid) - .map_err(|e| Status::internal(format!("db error: {e}")))? - .into_iter(); + let mut matches = crate::db::scan::by_peer( + &db, + crate::db::scan::Peer::SpacedHost { + spaced_host_pid: pid, + uid, + }, + ) + .map_err(|e| Status::internal(format!("db error: {e}")))? + .into_iter(); match (matches.next(), matches.next()) { (Some(sandbox), None) => Ok(sandbox), @@ -1059,9 +1066,15 @@ impl SandboxManagerService { // The exact PID/owner lookup below remains mandatory in either case. .unwrap_or(uid); let db = self.db_conn()?; - let mut matches = crate::db::list_by_netgtw_pid_uid(&db, pid, uid) - .map_err(|e| Status::internal(format!("db error: {e}")))? - .into_iter(); + let mut matches = crate::db::scan::by_peer( + &db, + crate::db::scan::Peer::NetGateway { + netgtw_pid: pid, + uid, + }, + ) + .map_err(|e| Status::internal(format!("db error: {e}")))? + .into_iter(); match (matches.next(), matches.next()) { (Some(sandbox), None) => Ok(sandbox), diff --git a/crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs b/crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs index cd5cdc022..071ded584 100644 --- a/crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs +++ b/crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs @@ -832,7 +832,9 @@ where Retirement: Fn(&str) -> Option, RetirementGuard: FnOnce(), { - let rows = db.with_connection(crate::db::list_sandboxes_for_recovery)??; + let rows = db.with_connection(|conn| { + crate::db::scan::all_sandboxes(conn, crate::db::scan::SkipUndecodableRows) + })??; reconcile_inventory_batch( db, paths, @@ -1123,7 +1125,9 @@ where // Periodic DB and directory cursors advance independently: absence from // this DB page is not absence from SQLite. Avoid receiver RPCs for rows // owned by another page; recheck again in the insertion transaction. - if db.with_connection(|conn| crate::db::sandbox_exists(conn, &sandbox_id.to_string()))?? { + if db.with_connection(|conn| { + crate::db::read::sandbox_view(conn, &sandbox_id.to_string()).exists() + })?? { continue; } let observed_presence = SandboxInventoryPresence { @@ -1231,10 +1235,14 @@ where return Ok(OrphanRowMaterialization::InventoryChanged); } let tx = crate::db::Transaction::immediate(conn)?; - if crate::db::get_sandbox(&tx, &sandbox_id_string)?.is_some() { + if crate::db::read::sandbox(&tx, &sandbox_id_string)?.is_some() { return Ok(OrphanRowMaterialization::DatabaseRowAppeared); } - if crate::db::restore_transfer_scope(&tx, &sandbox_id_string)?.is_some() { + if crate::db::read::sandbox_view(&tx, &sandbox_id_string) + .restore_scope() + .scope()? + .is_some() + { return Ok(OrphanRowMaterialization::RestoreTransferScope); } let committed = insert_orphaned_sandbox_row_in_transaction( @@ -1473,7 +1481,7 @@ impl SandboxManagerService { ) -> crate::db::Result<()> { // The caller keeps SQLite locked through retirement, excluding a // same-ID insertion and its new policy obligation. - if !crate::db::sandbox_exists(db, sandbox_id)? { + if !crate::db::read::sandbox_view(db, sandbox_id).exists()? { self.network_policy_reconcile_queue.retire(sandbox_id); } Ok(()) diff --git a/crates/libs/sbx/mgr/server/src/service/cache_cleanup/periodic.rs b/crates/libs/sbx/mgr/server/src/service/cache_cleanup/periodic.rs index 8b9e3d873..e288aee5c 100644 --- a/crates/libs/sbx/mgr/server/src/service/cache_cleanup/periodic.rs +++ b/crates/libs/sbx/mgr/server/src/service/cache_cleanup/periodic.rs @@ -62,9 +62,7 @@ impl PeriodicInventoryScan { paths: SandboxInventoryPaths<'_>, shutdown: &CancellationToken, ) -> Result { - let rows = self - .database - .next_batch(&*service.db_conn()?) + let rows = crate::db::scan::sandbox_pages(&*service.db_conn()?, &mut self.database) .map_err(|error| Status::internal(format!("inventory discovery: {error}")))?; let mut ids = BTreeSet::new(); for (scan, path, is_cache) in [ @@ -197,7 +195,11 @@ impl SandboxManagerService { .map(ToString::to_string) .filter(|id| !listed.contains(id.as_str())) // An unreadable row counts: reconciliation rechecks it. - .filter(|id| !crate::db::sandbox_exists(&db, id).unwrap_or(false)) + .filter(|id| { + !crate::db::read::sandbox_view(&db, id) + .exists() + .unwrap_or(false) + }) .map(|id| paths.cache_dir.join(id)) .collect() }; diff --git a/crates/libs/sbx/mgr/server/src/service/cleanup.rs b/crates/libs/sbx/mgr/server/src/service/cleanup.rs index f15d01c12..540428886 100644 --- a/crates/libs/sbx/mgr/server/src/service/cleanup.rs +++ b/crates/libs/sbx/mgr/server/src/service/cleanup.rs @@ -591,9 +591,12 @@ impl SandboxManagerService { let id = sandbox_id.to_owned(); let scope = self .storage(move |db| { - crate::db::restore_transfer_scope(db, &id).map_err(|error| { - Status::internal(format!("read restore transfer scope: {error}")) - }) + crate::db::read::sandbox_view(db, &id) + .restore_scope() + .scope() + .map_err(|error| { + Status::internal(format!("read restore transfer scope: {error}")) + }) }) .await?; let Some(scope) = scope else { @@ -610,10 +613,12 @@ impl SandboxManagerService { ) -> Result { let still_rowless = { let db = self.db_conn()?; - crate::db::get_sandbox(&db, sandbox_id) + crate::db::read::sandbox(&db, sandbox_id) .map_err(|error| Status::internal(format!("read sandbox row: {error}")))? .is_none() - && crate::db::restore_transfer_scope(&db, sandbox_id) + && crate::db::read::sandbox_view(&db, sandbox_id) + .restore_scope() + .scope() .map_err(|error| { Status::internal(format!("read restore transfer scope: {error}")) })? @@ -683,7 +688,10 @@ impl SandboxManagerService { let Ok(db) = self.db_conn() else { return; }; - match crate::db::rowless_restore_transfer_scopes(&db) { + match crate::db::scan::rowless_restore_scopes( + &db, + crate::db::scan::RowlessRestoreScopes::All, + ) { Ok(scopes) => scopes, Err(error) => { tracing::warn!(%error, "failed to list rowless restore transfer scopes"); @@ -716,7 +724,11 @@ impl SandboxManagerService { let mut cursor = std::mem::take(scan); let batch = self .storage(move |db| { - let batch = cursor.next_batch(db).map_err(|error| { + let batch = crate::db::scan::rowless_restore_scopes( + db, + crate::db::scan::RowlessRestoreScopes::Cursor(&mut cursor), + ) + .map_err(|error| { Status::internal(format!("discover orphan transfer scopes: {error}")) })?; Ok((cursor, batch)) @@ -835,7 +847,8 @@ impl SandboxManagerService { expected_template: Option<&str>, require_template_owner: bool, ) -> Result<(), Status> { - let target = crate::db::cleanup_target(db, sandbox_id) + let target = crate::db::read::sandbox_view(db, sandbox_id) + .cleanup_target() .map_err(|error| Status::unavailable(format!("inspect cleanup template: {error}")))?; if require_template_owner && target @@ -987,9 +1000,12 @@ impl SandboxManagerService { // An unfamiliar durable intent is not a persistence failure that can // authorize best-effort pathname cleanup. Check before any admission // branch can return Unpersisted, including transaction-begin failure. - crate::db::cleanup_episode(&db, &sandbox_id_string).map_err(|_| { - Status::unavailable("cleanup ownership cannot be decoded; retain resources") - })?; + crate::db::read::sandbox_view(&db, &sandbox_id_string) + .cleanup_episode() + .strict() + .map_err(|_| { + Status::unavailable("cleanup ownership cannot be decoded; retain resources") + })?; let transaction = match if mode.requires_immediate_transaction() || require_selection_check { crate::db::Transaction::immediate(&db) @@ -1022,7 +1038,9 @@ impl SandboxManagerService { )?; } - if crate::db::cleanup_episode(&transaction, &sandbox_id_string) + if crate::db::read::sandbox_view(&transaction, &sandbox_id_string) + .cleanup_episode() + .strict() .map_err(|e| Status::internal(format!("read cleanup ownership: {e}")))? .is_some_and(|episode| episode.completion == CleanupCompletion::DeletePoolClone) { @@ -1031,7 +1049,7 @@ impl SandboxManagerService { ); } if let Some(observed_state) = mode.observed_state() { - let current_state = crate::db::get_sandbox(&transaction, &sandbox_id_string) + let current_state = crate::db::read::sandbox(&transaction, &sandbox_id_string) .map_err(|error| { Status::internal(format!( "inspect lifecycle state during cleanup admission failed: {error}" @@ -1046,22 +1064,13 @@ impl SandboxManagerService { } } if target_state == SandboxState::Deleting && completion.deletes_row() { - let erase_safety = crate::db::get_sandbox(&transaction, &sandbox_id_string) + let erase_safety = crate::db::read::sandbox(&transaction, &sandbox_id_string) .map_err(|error| { Status::internal(format!( "inspect suspend publication during delete admission failed: {error}" )) })? - .map(|row| { - ( - row.state, - crate::db::sandbox_erase_safety( - row.state, - row.cleanup_completion, - row.failure_reason.as_deref(), - ), - ) - }); + .map(|row| (row.state, row.erase_safety())); match erase_safety { Some((state, crate::db::SandboxEraseSafety::SuspendPublicationPending)) => { tracing::info!( @@ -1085,7 +1094,9 @@ impl SandboxManagerService { } } let current_execution_generation = - match crate::db::runtime_execution_generation(&transaction, &sandbox_id_string) { + match crate::db::read::sandbox_view(&transaction, &sandbox_id_string) + .runtime_execution_generation() + { Ok(generation) => generation, Err(error) if require_generation_inspection => { return Err(Status::unavailable(format!( @@ -1114,16 +1125,15 @@ impl SandboxManagerService { } if completion.deletes_row() && mode.auto_lifecycle_delete_expectation().is_none() - && crate::db::auto_lifecycle_delete_receipts::pending_receipt_for_sandbox( - &transaction, - sandbox_id, - ) - .map_err(|error| { - Status::unavailable(format!( - "inspect pending auto-lifecycle delete receipt: {error}" - )) - })? - .is_some() + && crate::db::read::typed_sandbox_view(&transaction, sandbox_id) + .delete_receipts() + .pending() + .map_err(|error| { + Status::unavailable(format!( + "inspect pending auto-lifecycle delete receipt: {error}" + )) + })? + .is_some() { return Err(Status::unavailable( "pending auto-lifecycle delete receipt owns sandbox deletion", @@ -1224,7 +1234,8 @@ impl SandboxManagerService { None }; let guest_flush = guest_flush.for_superseded_state( - crate::db::current_state_label(&transaction, &sandbox_id_string) + crate::db::read::sandbox_view(&transaction, &sandbox_id_string) + .current_state_label() .ok() .flatten() .and_then(|state| state.parse().ok()), @@ -1276,7 +1287,9 @@ impl SandboxManagerService { } } if mode.auto_lifecycle_delete_expectation().is_some() - && crate::db::cleanup_episode(&transaction, &sandbox_id_string) + && crate::db::read::sandbox_view(&transaction, &sandbox_id_string) + .cleanup_episode() + .strict() .map_err(|error| { Status::unavailable(format!( "inspect cleanup episode after auto-lifecycle delete claim: {error}" @@ -1388,7 +1401,8 @@ impl SandboxManagerService { } None => (None, None), }; - if let Some(uuid8) = crate::db::cleanup_runtime_label(&transaction, &sandbox_id_string) + if let Some(uuid8) = crate::db::read::sandbox_view(&transaction, &sandbox_id_string) + .cleanup_runtime_label() .map_err(|error| { Status::unavailable(format!("inspect cleanup scheduling identity: {error}")) })? @@ -1468,9 +1482,11 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| Status::internal("database mutex poisoned"))?; - crate::db::runtime_execution_generation(&db, sandbox_id).map_err(|error| { - Status::internal(format!("inspect runtime generation failed: {error}")) - })? + crate::db::read::sandbox_view(&db, sandbox_id) + .runtime_execution_generation() + .map_err(|error| { + Status::internal(format!("inspect runtime generation failed: {error}")) + })? }; if current_execution_generation != expected_execution_generation { return Ok(CleanupDispatchDisposition::Superseded); @@ -1697,16 +1713,16 @@ impl SandboxManagerService { let sandbox_id = sandbox_id .parse::() .map_err(|error| error.to_string())?; - if !crate::db::auto_lifecycle_delete_receipts::pending_receipt_matches_observed_execution( - &db, - sandbox_id, - ) - .map_err(|error| error.to_string())? + if !crate::db::read::typed_sandbox_view(&db, sandbox_id) + .delete_receipts() + .pending_matches_observed_execution() + .map_err(|error| error.to_string())? { return Ok(false); } } - crate::db::cleanup_intent_current(&db, sandbox_id, completion) + crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_intent_current(completion) .map_err(|error| error.to_string()) } @@ -1721,7 +1737,9 @@ impl SandboxManagerService { .lock() .map_err(|error| format!("database mutex poisoned: {error}"))?; - return crate::db::cleanup_episode_current(&db, sandbox_id, cleanup_id) + return crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .current(cleanup_id) .map_err(|error| error.to_string()); } Self::cleanup_attempt_still_current(db, sandbox_id, completion) @@ -1737,7 +1755,9 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| QuarantineRecoveryArtifactError::DatabaseMutexPoisoned)?; - let episode = crate::db::cleanup_episode(&db, sandbox_id)?; + let episode = crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .strict()?; let Some(episode) = episode.filter(|episode| episode.id == cleanup_id) else { return Ok(()); }; @@ -1955,7 +1975,7 @@ impl SandboxManagerService { published_artifact, job_id, } = request; - if crate::db::runtime_execution_generation(conn, sandbox_id)? + if crate::db::read::sandbox_view(conn, sandbox_id).runtime_execution_generation()? != expected_execution_generation { return Ok(CleanupCompletionDisposition::Superseded); @@ -1999,7 +2019,7 @@ impl SandboxManagerService { ); return; }; - let row = match crate::db::get_sandbox(&db, sandbox_id) { + let row = match crate::db::read::sandbox(&db, sandbox_id) { Ok(row) => row, Err(error) => { tracing::warn!( @@ -2295,9 +2315,11 @@ impl SandboxManagerService { Some(admission) => admission.expected_execution_generation(parsed_sandbox_id)?, None => { let db = self.db_conn()?; - crate::db::runtime_execution_generation(&db, sandbox_id).map_err(|error| { - Status::internal(format!("inspect runtime generation failed: {error}")) - })? + crate::db::read::sandbox_view(&db, sandbox_id) + .runtime_execution_generation() + .map_err(|error| { + Status::internal(format!("inspect runtime generation failed: {error}")) + })? } }; let auto_lifecycle_delete_receipt = match admission.as_ref() { @@ -2576,11 +2598,15 @@ impl SandboxManagerService { .lock() .map_err(|_| Status::unavailable("cleanup ownership is unavailable"))?; - crate::db::cleanup_episode(&db, sandbox_id).map_err(|_| { - Status::unavailable("cleanup ownership cannot be decoded; retain resources") - })?; + crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .strict() + .map_err(|_| { + Status::unavailable("cleanup ownership cannot be decoded; retain resources") + })?; request.completion == CleanupCompletion::DeleteRow - && crate::db::current_state_label(&db, sandbox_id) + && crate::db::read::sandbox_view(&db, sandbox_id) + .current_state_label() .map_err(|error| { Status::unavailable(format!( "inspect sandbox after unpersisted delete: {error}" @@ -2805,7 +2831,8 @@ impl SandboxManagerService { BeforeStopFlush::ResumeThenFlush => { let execution_fence = { let db = self.db_conn().map_err(StopFlushError::Database)?; - crate::db::node_execution_fence(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .node_execution_fence() .map_err(StopFlushError::Fence)? .map(Into::into) }; diff --git a/crates/libs/sbx/mgr/server/src/service/cleanup_executor.rs b/crates/libs/sbx/mgr/server/src/service/cleanup_executor.rs index bda46f2c1..495083f6a 100644 --- a/crates/libs/sbx/mgr/server/src/service/cleanup_executor.rs +++ b/crates/libs/sbx/mgr/server/src/service/cleanup_executor.rs @@ -266,8 +266,9 @@ impl SandboxManagerService { { return Ok(None); } - let generation = - crate::db::runtime_execution_generation(&tx, &id).map_err(storage_status)?; + let generation = crate::db::read::sandbox_view(&tx, &id) + .runtime_execution_generation() + .map_err(storage_status)?; if expected_generation.is_some_and(|expected| expected != generation) { return Ok(None); } @@ -341,9 +342,9 @@ impl SandboxManagerService { let _admission = admission; let _fence = fence; let tx = crate::db::Transaction::deferred(db).map_err(storage_status)?; - let generation = - crate::db::runtime_execution_generation(&tx, &sandbox_id.to_string()) - .map_err(storage_status)?; + let generation = crate::db::read::sandbox_view(&tx, &sandbox_id.to_string()) + .runtime_execution_generation() + .map_err(storage_status)?; if expected.is_some_and(|expected| expected != generation) { return Err(Status::aborted("cleanup execution superseded")); } @@ -425,7 +426,7 @@ impl SandboxManagerService { } let capacity = DEFAULT_CLEANUP_CONCURRENCY_CAP; let mut foreground_first = true; - let mut recovery_cursor = None; + let mut recovery_cursor = crate::db::scan::CleanupObligationScan::default(); let mut tasks = tokio::task::JoinSet::new(); let mut tick = tokio::time::interval(Duration::from_secs(1)); tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); @@ -442,15 +443,18 @@ impl SandboxManagerService { if self.is_draining_for_restart() { continue; } - let after = recovery_cursor; + // A failed read keeps the cursor where it was: only the copy advances. + let mut cursor = recovery_cursor; match self .storage(move |db| { - crate::db::cleanup_recovery_page(db, after, 128).map_err(storage_status) + let page = + crate::db::scan::sandbox_pages(db, &mut cursor).map_err(storage_status)?; + Ok((cursor, page)) }) .await { - Ok(page) => { - recovery_cursor = page.next_cursor; + Ok((cursor, page)) => { + recovery_cursor = cursor; let ids = page.ids; if !ids.is_empty() { let permit = Arc::clone(&self.cleanup_io).acquire_owned().await; @@ -509,7 +513,7 @@ impl SandboxManagerService { .map_err(storage_status)? { let Some(job) = - cleanup_jobs::get_job(&tx, &id).map_err(storage_status)? + crate::db::read::cleanup_job(&tx, &id).map_err(storage_status)? else { continue; }; @@ -618,7 +622,7 @@ impl SandboxManagerService { let job_id = id.to_owned(); let selected = self .storage(move |db| { - let job = cleanup_jobs::get_job(db, &job_id) + let job = crate::db::read::cleanup_job(db, &job_id) .map_err(storage_status)? .ok_or_else(|| Status::internal("cleanup job disappeared"))?; if job.status != JobStatus::Running.as_str() { @@ -634,11 +638,9 @@ impl SandboxManagerService { tx.commit().map_err(storage_status)?; return Ok(None); } - let receipt = - crate::db::auto_lifecycle_delete_receipts::pending_receipt_for_sandbox( - db, - work.sandbox_id, - ) + let receipt = crate::db::read::typed_sandbox_view(db, work.sandbox_id) + .delete_receipts() + .pending() .map_err(storage_status)?; Ok(Some((work, receipt))) }) @@ -710,7 +712,7 @@ impl SandboxManagerService { cleanup_jobs::finish(&tx, &id, outcome).map_err(storage_status)?; tx.commit().map_err(storage_status)?; } else { - let stored = cleanup_jobs::get_job(db, &id) + let stored = crate::db::read::cleanup_job(db, &id) .map_err(storage_status)? .ok_or_else(|| Status::internal("cleanup job disappeared"))?; if stored.status != outcome.job_status().as_str() { @@ -721,7 +723,7 @@ impl SandboxManagerService { } } Err(error) => { - let attempts = cleanup_jobs::get_job(db, &id) + let attempts = crate::db::read::cleanup_job(db, &id) .map_err(storage_status)? .map_or(1, |job| job.attempts); let delay = CLEANUP_RETRY_INITIAL_DELAY_SECS @@ -805,7 +807,9 @@ impl SandboxManagerService { loop { let read_id = id; let stored = self - .storage(move |db| cleanup_jobs::get_job(db, &read_id).map_err(storage_status)) + .storage(move |db| { + crate::db::read::cleanup_job(db, &read_id).map_err(storage_status) + }) .await? .ok_or_else(|| Status::internal("cleanup job disappeared"))?; #[cfg(test)] diff --git a/crates/libs/sbx/mgr/server/src/service/cleanup_support.rs b/crates/libs/sbx/mgr/server/src/service/cleanup_support.rs index 40320508d..b21e52fc9 100644 --- a/crates/libs/sbx/mgr/server/src/service/cleanup_support.rs +++ b/crates/libs/sbx/mgr/server/src/service/cleanup_support.rs @@ -98,7 +98,8 @@ pub(super) fn read_node_execution_fence( let db = db .lock() .map_err(|_| Status::unavailable("sandbox database mutex poisoned"))?; - crate::db::node_execution_fence(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .node_execution_fence() .map_err(|error| Status::unavailable(format!("read node execution authority: {error}"))) } @@ -111,7 +112,9 @@ pub(super) fn admit_prepared_clone_disposal_attempt( let db = db .lock() .map_err(|_| Status::internal("sandbox database mutex poisoned"))?; - let episode = crate::db::cleanup_episode(&db, id) + let episode = crate::db::read::sandbox_view(&db, id) + .cleanup_episode() + .strict() .map_err(|e| Status::internal(format!("read clone disposal: {e}")))? .filter(|e| e.completion == CleanupCompletion::DeletePoolClone) .ok_or_else(|| Status::aborted("clone disposal ownership changed"))?; @@ -160,7 +163,9 @@ pub(super) async fn service_cleanup_via_node_with_options( let db = db .lock() .map_err(|_| Status::internal("sandbox database mutex poisoned"))?; - crate::db::cleanup_episode(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .strict() .map_err(|e| Status::internal(format!("read disposal authority: {e}")))? }; if let Some(episode) = episode.filter(|e| e.completion == CleanupCompletion::DeletePoolClone) { diff --git a/crates/libs/sbx/mgr/server/src/service/cleanup_targets.rs b/crates/libs/sbx/mgr/server/src/service/cleanup_targets.rs index 1a7f693b4..5909b9faf 100644 --- a/crates/libs/sbx/mgr/server/src/service/cleanup_targets.rs +++ b/crates/libs/sbx/mgr/server/src/service/cleanup_targets.rs @@ -21,10 +21,9 @@ impl CleanupInventoryCursor { ) -> Result<(Self, CleanupTargetBatch), Status> { let mut targets = Vec::new(); if !self.database.complete() { - let observations = self - .database - .next_batch(&*service.db_conn()?) - .map_err(|error| error.into_status())?; + let observations = + crate::db::scan::sandbox_pages(&*service.db_conn()?, &mut self.database) + .map_err(|error| error.into_status())?; targets.extend(observations.into_iter().map(|row| { CleanupTargetObservation { sandbox_id: row.raw_id, @@ -72,7 +71,8 @@ impl CleanupInventoryCursor { // No filesystem operation runs while SQLite is locked. This // suppresses stable duplicates without retaining all seen IDs; // concurrent deletion can repeat a target, as the wire states. - if crate::db::sandbox_exists(&*service.db_conn()?, &name) + if crate::db::read::sandbox_view(&*service.db_conn()?, &name) + .exists() .map_err(|error| Status::internal(format!("inspect cleanup target: {error}")))? { continue; @@ -249,7 +249,8 @@ fn resolve_lifecycle_target_blocking( let db = db .lock() .map_err(|_| Status::internal("cleanup database mutex poisoned"))?; - crate::db::cleanup_target(&db, &id) + crate::db::read::sandbox_view(&db, &id) + .cleanup_target() .map_err(|error| Status::internal(format!("read cleanup target: {error}")))? }; if row.is_none() { diff --git a/crates/libs/sbx/mgr/server/src/service/create.rs b/crates/libs/sbx/mgr/server/src/service/create.rs index 2cd64a7e1..8a1a65658 100644 --- a/crates/libs/sbx/mgr/server/src/service/create.rs +++ b/crates/libs/sbx/mgr/server/src/service/create.rs @@ -279,7 +279,7 @@ impl SandboxManagerService { ) -> Result, Status> { let requested_id_str = requested_id.to_string(); let db = self.db_conn()?; - crate::db::get_sandbox(&db, &requested_id_str) + crate::db::read::sandbox(&db, &requested_id_str) .map_err(|e| Status::internal(format!("db lookup failed: {e}"))) } diff --git a/crates/libs/sbx/mgr/server/src/service/create_support.rs b/crates/libs/sbx/mgr/server/src/service/create_support.rs index d4abe98bf..6b94a0357 100644 --- a/crates/libs/sbx/mgr/server/src/service/create_support.rs +++ b/crates/libs/sbx/mgr/server/src/service/create_support.rs @@ -617,9 +617,7 @@ pub(super) fn checkpoint_restore_retry_action( // can safely reattach while no runtime, artifact pin, or cleanup intent // was committed. The current request supplies the desired target state // after staging returns the row to Suspended. - SandboxState::Restoring - if !runtime_live && crate::db::sandbox_is_retryable_interrupted_restore(existing) => - { + SandboxState::Restoring if !runtime_live && existing.is_retryable_interrupted_restore() => { CheckpointRestoreRetryAction::ResumeRestoring } SandboxState::Restoring => CheckpointRestoreRetryAction::CleanupRestoring, diff --git a/crates/libs/sbx/mgr/server/src/service/eviction.rs b/crates/libs/sbx/mgr/server/src/service/eviction.rs index 9db19b4b7..f99be5036 100644 --- a/crates/libs/sbx/mgr/server/src/service/eviction.rs +++ b/crates/libs/sbx/mgr/server/src/service/eviction.rs @@ -218,20 +218,20 @@ impl SandboxManagerService { // row still holds this teardown's capture. The claim re-checks all of // it. let claimable = self.db_conn().and_then(|db| { - let running = crate::db::teardown_suspend_operation_running_for( - &db, - sandbox_id, - execution_generation, - ) - .map_err(|error| Status::internal(format!("teardown job lookup: {error}")))?; - let holds_capture = crate::db::get_sandbox(&db, sandbox_id) + let running = crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .running_for(execution_generation) + .map_err(|error| Status::internal(format!("teardown job lookup: {error}")))?; + let holds_capture = crate::db::read::sandbox(&db, sandbox_id) .map_err(|error| Status::internal(format!("sandbox lookup: {error}")))? .is_some_and(|row| { row.state == crate::state::SandboxState::Suspending && row.current_local_artifact_id == Some(staged_artifact) }); let awaiting_backup = if running && holds_capture { - crate::db::other_local_artifacts_awaiting_backup(&db, sandbox_id, staged_artifact) + crate::db::read::sandbox_view(&db, sandbox_id) + .artifacts() + .others_awaiting_backup(staged_artifact) .map_err(|error| Status::internal(format!("local artifact lookup: {error}")))? } else { Vec::new() @@ -337,25 +337,21 @@ impl SandboxManagerService { // Only a claim held back by another artifact's backup is that // case; one that lost because the teardown job moved on is not. let awaiting_backup = self.db_conn().ok().and_then(|db| { - let still_driving = crate::db::teardown_suspend_operation_running_for( - &db, - sandbox_id, - execution_generation, - ) - .ok()? - && crate::db::get_sandbox(&db, sandbox_id) + let still_driving = crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .running_for(execution_generation) + .ok()? + && crate::db::read::sandbox(&db, sandbox_id) .ok()? .is_some_and(|row| { row.state == crate::state::SandboxState::Suspending && row.current_local_artifact_id == Some(staged_artifact) }); - let pending = !crate::db::other_local_artifacts_awaiting_backup( - &db, - sandbox_id, - staged_artifact, - ) - .ok()? - .is_empty(); + let pending = !crate::db::read::sandbox_view(&db, sandbox_id) + .artifacts() + .others_awaiting_backup(staged_artifact) + .ok()? + .is_empty(); Some(still_driving && pending) }); if awaiting_backup == Some(true) { @@ -407,7 +403,7 @@ impl SandboxManagerService { }; let row = { let db = self.db_conn()?; - crate::db::get_sandbox(&db, sandbox_id) + crate::db::read::sandbox(&db, sandbox_id) .map_err(|e| Status::internal(format!("db sandbox lookup failed: {e}")))? .ok_or_else(|| Status::not_found(format!("sandbox {sandbox_id} not found")))? }; @@ -635,10 +631,10 @@ impl SandboxManagerService { // between them fails its CAS rather than being acted on. let (row, state_changed_at) = { let db = self.db_conn()?; - let row = crate::db::get_sandbox(&db, &sandbox_id) + let row = crate::db::read::sandbox(&db, &sandbox_id) .map_err(|e| Status::internal(format!("db sandbox lookup failed: {e}")))? .ok_or_else(|| Status::not_found(format!("sandbox {sandbox_id} not found")))?; - let state_changed_at = crate::db::sandbox_state_changed_at_witness(&db, &sandbox_id) + let state_changed_at = crate::db::read::state_changed_at_witness(&db, &sandbox_id) .map_err(|e| Status::internal(format!("db sandbox lookup failed: {e}")))? .ok_or_else(|| Status::not_found(format!("sandbox {sandbox_id} not found")))?; (row, state_changed_at) diff --git a/crates/libs/sbx/mgr/server/src/service/execution_startup.rs b/crates/libs/sbx/mgr/server/src/service/execution_startup.rs index f512466a8..c03a70131 100644 --- a/crates/libs/sbx/mgr/server/src/service/execution_startup.rs +++ b/crates/libs/sbx/mgr/server/src/service/execution_startup.rs @@ -9,7 +9,7 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| Status::unavailable("sandbox database mutex poisoned"))?; - let unadopted = crate::db::unbound_node_executions(&db).map_err(|error| { + let unadopted = crate::db::scan::unbound_executions(&db).map_err(|error| { Status::unavailable(format!("read execution preflight: {error}")) })?; pplx_space_node_mgr_client::execution_fence::execution_preflight_result(&unadopted) diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_core.rs b/crates/libs/sbx/mgr/server/src/service/grpc_core.rs index 55dab70f9..69ee9e170 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_core.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_core.rs @@ -428,7 +428,7 @@ impl SandboxManagerService { let _guard = wake_lock.lock().await; { let db = self.db_conn()?; - existing = crate::db::get_sandbox(&db, &sandbox_id) + existing = crate::db::read::sandbox(&db, &sandbox_id) .map_err(|e| Status::internal(format!("db lookup failed: {e}")))? .ok_or_else(|| { Status::not_found(format!( @@ -466,7 +466,7 @@ impl SandboxManagerService { existing.state, &auto_lifecycle, )?; - existing = crate::db::get_sandbox(&db, &sandbox_id) + existing = crate::db::read::sandbox(&db, &sandbox_id) .map_err(|e| { Status::internal(format!("db lookup failed: {e}")) })? @@ -829,11 +829,14 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("sandbox database mutex poisoned"))?; - let episode = crate::db::cleanup_episode(&db, id) + let episode = crate::db::read::sandbox_view(&db, id) + .cleanup_episode() + .strict() .map_err(|e| Status::internal(format!("read clone disposal: {e}")))? .filter(|episode| episode.completion == CleanupCompletion::DeletePoolClone); let quarantined = episode.is_some() - && crate::db::current_state_label(&db, id) + && crate::db::read::sandbox_view(&db, id) + .current_state_label() .map_err(|e| Status::internal(format!("read clone disposal state: {e}")))? .as_deref() == Some(SandboxState::Quarantined.as_str()); @@ -1005,7 +1008,7 @@ impl SandboxManagerService { }; let page = self .storage(move |db| { - crate::db::inventory_read::list_page(db, &page_token, limit) + crate::db::scan::list_page(db, &page_token, limit) .map_err(|error| error.into_status()) }) .await?; @@ -1023,8 +1026,13 @@ impl SandboxManagerService { )?; let db_rows = self .storage(|db| { - crate::db::inventory_read::list(db, "ListBuildReferencingSandboxes") - .map_err(|error| error.into_status()) + crate::db::scan::all_sandboxes( + db, + crate::db::scan::RpcListRows { + rpc: "ListBuildReferencingSandboxes", + }, + ) + .map_err(|error| error.into_status()) }) .await?; @@ -1072,9 +1080,12 @@ impl SandboxManagerService { let mut resolved_ids = Vec::with_capacity(sandbox_ids.len()); let mut missing_ids = Vec::new(); for id in sandbox_ids { - if crate::db::sandbox_exists(db, &id).map_err(|error| { - Status::internal(format!("read sandbox identity: {error}")) - })? { + if crate::db::read::sandbox_view(db, &id) + .exists() + .map_err(|error| { + Status::internal(format!("read sandbox identity: {error}")) + })? + { resolved_ids.push(id); } else { missing_ids.push(SbxMissingDiskUsageId { diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/artifact_backup.rs b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/artifact_backup.rs index 081fdd6e1..f2e685226 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/artifact_backup.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/artifact_backup.rs @@ -482,7 +482,7 @@ impl SandboxManagerService { let sandbox = { let db = self.db_conn()?; - crate::db::get_sandbox(&db, sandbox_id) + crate::db::read::sandbox(&db, sandbox_id) .map_err(|err| Status::internal(format!("db sandbox lookup failed: {err}")))? .ok_or_else(|| Status::not_found(format!("sandbox {sandbox_id} not found")))? }; @@ -1258,14 +1258,14 @@ impl SandboxManagerService { return Ok(None); } let db = self.db_conn()?; - Ok( - crate::db::latest_teardown_suspend_operation(&db, &sandbox.id) - .map_err(|error| Status::internal(format!("teardown job lookup: {error}")))? - .filter(|operation| { - operation.status == crate::db::TeardownSuspendOperationStatus::Running - }) - .map(|operation| operation.execution_generation), - ) + Ok(crate::db::read::sandbox_view(&db, &sandbox.id) + .teardown() + .latest() + .map_err(|error| Status::internal(format!("teardown job lookup: {error}")))? + .filter(|operation| { + operation.status == crate::db::TeardownSuspendOperationStatus::Running + }) + .map(|operation| operation.execution_generation)) } } diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/pause_resume.rs b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/pause_resume.rs index 6e327db21..58f4d4baa 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/pause_resume.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/pause_resume.rs @@ -212,7 +212,7 @@ pub(super) async fn pause_with_reconciliation( let found = crate::node_state_sync::connection_or_signal_failure(db) .ok() .and_then(|db| { - crate::db::get_sandbox(&db, &sandbox_id_string) + crate::db::read::sandbox(&db, &sandbox_id_string) .ok() .flatten() }) diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/sandbox_network_policy.rs b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/sandbox_network_policy.rs index cb7fee2d0..c6a6cb908 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/sandbox_network_policy.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/sandbox_network_policy.rs @@ -303,7 +303,7 @@ impl SandboxManagerService { let sandboxes = { let db = self.db_conn()?; - crate::db::list_sandboxes(&db) + crate::db::scan::all_sandboxes(&db, crate::db::scan::StrictRows) .map_err(|error| { Status::internal(format!( "db list failed: {}", diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/suspend_snapshot.rs b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/suspend_snapshot.rs index 9b8bfc60b..da271810d 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/suspend_snapshot.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/suspend_snapshot.rs @@ -296,7 +296,7 @@ impl SandboxManagerService { let Ok(db) = self.db_conn() else { return SuspendSnapshotStagingDisposition::Preserve; }; - match crate::db::get_sandbox(&db, sandbox_id) { + match crate::db::read::sandbox(&db, sandbox_id) { Ok(Some(row)) if row.state == SandboxState::Suspending => row, Ok(Some(row)) => { tracing::warn!( @@ -386,7 +386,10 @@ impl SandboxManagerService { let Ok(db) = self.db_conn() else { return SuspendSnapshotStagingDisposition::Preserve; }; - match crate::db::teardown_suspend_status(&db, sandbox_id) { + match crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .status() + { Ok(Some(status)) => status .operation .filter(|operation| operation.status.is_active()) diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/teardown_suspend.rs b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/teardown_suspend.rs index ef1f87cdf..b65070460 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/teardown_suspend.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/teardown_suspend.rs @@ -285,7 +285,9 @@ impl SandboxManagerService { let sandbox_id_string = sandbox_id.to_string(); let evidence = { let db = self.db_conn()?; - crate::db::teardown_release_evidence(&db, &sandbox_id_string, execution_generation) + crate::db::read::sandbox_view(&db, &sandbox_id_string) + .teardown() + .release_evidence(execution_generation) .map_err(|error| Status::internal(format!("db error: {error}")))? }; let Some(mut status) = evidence.suspend else { @@ -403,7 +405,9 @@ impl SandboxManagerService { } Err(error) => return Err(Status::internal(format!("db error: {error}"))), } - status = crate::db::teardown_suspend_status(&db, &sandbox_id.to_string()) + status = crate::db::read::sandbox_view(&db, &sandbox_id.to_string()) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))?; } @@ -438,7 +442,9 @@ impl SandboxManagerService { } Err(error) => return Err(Status::internal(format!("db error: {error}"))), } - status = crate::db::teardown_suspend_status(&db, &sandbox_id.to_string()) + status = crate::db::read::sandbox_view(&db, &sandbox_id.to_string()) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))?; bound_existing_suspend = true; @@ -463,7 +469,9 @@ impl SandboxManagerService { )?; status = { let db = self.db_conn()?; - crate::db::teardown_suspend_status(&db, &sandbox_id_string) + crate::db::read::sandbox_view(&db, &sandbox_id_string) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))? }; @@ -518,10 +526,12 @@ impl SandboxManagerService { generation: pplx_space_common_core::types::ExecutionGeneration, ) -> Result { let db = self.db_conn()?; - let state = crate::db::get_sandbox(&db, sandbox_id) + let state = crate::db::read::sandbox(&db, sandbox_id) .map_err(|error| Status::internal(format!("db error: {error}")))? .map(|row| row.state); - let operation = crate::db::latest_teardown_suspend_operation(&db, sandbox_id) + let operation = crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .latest() .map_err(|error| Status::internal(format!("db error: {error}")))?; // A failed older generation must not block a new execution. let exact = operation @@ -530,7 +540,12 @@ impl SandboxManagerService { let reason = match state { Some(SandboxState::Quarantined) => Some(crate::db::TeardownSuspendFailure::Quarantined), Some(SandboxState::Deleting) => Some(crate::db::TeardownSuspendFailure::Deleting), - None if crate::db::execution_delete_receipt_covers(&db, sandbox_id, generation) + None if crate::db::now_epoch_i64() + .and_then(|now| { + crate::db::read::sandbox_view(&db, sandbox_id) + .delete_receipts() + .execution_covers(generation, now) + }) .map_err(|error| Status::internal(format!("db error: {error}")))? => { return Ok(TerminalTeardownSuspend::Deleted); @@ -538,10 +553,10 @@ impl SandboxManagerService { None if exact.is_some() => Some(crate::db::TeardownSuspendFailure::Missing), _ if exact.is_some_and(|operation| { operation.status == crate::db::TeardownSuspendOperationStatus::Failed - }) && !crate::db::teardown_suspend_retry_admissible( - &db, sandbox_id, generation, - ) - .map_err(|error| Status::internal(format!("db error: {error}")))? => + }) && !crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .retry_admissible(generation) + .map_err(|error| Status::internal(format!("db error: {error}")))? => { Some(crate::db::TeardownSuspendFailure::Failed) } @@ -566,12 +581,10 @@ impl SandboxManagerService { event_trigger: SandboxEventTrigger, ) -> TeardownSuspendDriverAdmission { let due = self.db_conn().and_then(|db| { - crate::db::teardown_suspend_retry_due( - &db, - &sandbox_id.to_string(), - execution_generation, - ) - .map_err(|error| Status::internal(format!("read teardown retry deadline: {error}"))) + crate::db::read::sandbox_view(&db, &sandbox_id.to_string()) + .teardown() + .retry_due(execution_generation) + .map_err(|error| Status::internal(format!("read teardown retry deadline: {error}"))) }); match due { Ok(true) => {} @@ -881,7 +894,7 @@ impl SandboxManagerService { backup_lookup: Option<&OrgSnapshotConfigResolution>, ) -> Result<(), Status> { let db = self.db_conn()?; - let local = crate::db::get_sandbox(&db, &sandbox_id.to_string()) + let local = crate::db::read::sandbox(&db, &sandbox_id.to_string()) .map_err(|error| Status::internal(format!("db error: {error}")))?; let reason = match local { None => Some(crate::db::TeardownSuspendFailure::Missing), @@ -940,7 +953,9 @@ impl SandboxManagerService { let artifact_guard = artifact_lock.lock_owned().await; let status = { let db = self.db_conn()?; - crate::db::teardown_suspend_status(&db, &sandbox_id_string) + crate::db::read::sandbox_view(&db, &sandbox_id_string) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))? }; @@ -972,7 +987,9 @@ impl SandboxManagerService { .await?; let status = { let db = self.db_conn()?; - crate::db::teardown_suspend_status(&db, &sandbox_id_string) + crate::db::read::sandbox_view(&db, &sandbox_id_string) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))? }; @@ -1125,7 +1142,9 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("database mutex poisoned"))?; - crate::db::teardown_suspend_status(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? }; let owns_runtime = status @@ -1162,7 +1181,9 @@ impl SandboxManagerService { ) -> Result<(), Status> { let completed = { let db = self.db_conn()?; - let status = crate::db::teardown_suspend_status(&db, sandbox_id) + let status = crate::db::read::sandbox_view(&db, sandbox_id) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found("sandbox record no longer exists"))?; let is_ready = status.is_ready_for(execution_generation); @@ -1405,7 +1426,9 @@ impl SandboxManagerService { ) -> Result<(), Status> { let still_owned = { let db = self.db_conn()?; - crate::db::teardown_suspend_status(&db, &sandbox.id) + crate::db::read::sandbox_view(&db, &sandbox.id) + .teardown() + .status() .map_err(|error| Status::internal(format!("db error: {error}")))? .is_some_and(|status| { // Once the artifact is stamped, the next PrepareTeardownSuspend diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_pool_wait_clone.rs b/crates/libs/sbx/mgr/server/src/service/grpc_pool_wait_clone.rs index dd3e5ee28..b7462e869 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_pool_wait_clone.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_pool_wait_clone.rs @@ -217,7 +217,7 @@ impl SandboxManagerService { }; let candidate = { let db = self.db_conn()?; - crate::db::find_pool_sandbox(&db, template, req.content_hash.as_deref(), required_user) + crate::db::scan::pool_donor(&db, template, req.content_hash.as_deref(), required_user) .map_err(|e| Status::internal(format!("db error: {e}")))? }; @@ -606,7 +606,7 @@ impl SandboxManagerService { // Check current state from DB let state = { let db = self.db_conn()?; - crate::db::get_sandbox(&db, &id) + crate::db::read::sandbox(&db, &id) .map_err(|e| Status::internal(format!("db error: {e}")))? .map(|row| row.state) }; @@ -839,11 +839,14 @@ impl SandboxManagerService { }; let current = { let db = self.db_conn()?; - crate::db::cleanup_episode(&db, sandbox_id).map_err(|error| { - cleanup.failure(Status::internal(format!( - "read pool clone episode: {error}" - ))) - })? + crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .strict() + .map_err(|error| { + cleanup.failure(Status::internal(format!( + "read pool clone episode: {error}" + ))) + })? }; if !current .is_some_and(|episode| episode.id == cleanup_id && episode.requested_at.is_none()) diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_sandbox_transfer.rs b/crates/libs/sbx/mgr/server/src/service/grpc_sandbox_transfer.rs index b54b06bbd..4407a4787 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_sandbox_transfer.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_sandbox_transfer.rs @@ -613,7 +613,8 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| Status::internal("sandbox db mutex poisoned"))?; - if crate::db::sandbox_exists(&db, &sandbox_id.to_string()) + if crate::db::read::sandbox_view(&db, &sandbox_id.to_string()) + .exists() .map_err(|error| Status::internal(format!("inspect import identity: {error}")))? { return Err(Status::already_exists( @@ -642,7 +643,8 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("sandbox db mutex poisoned"))?; - if crate::db::sandbox_exists(&db, sandbox_id) + if crate::db::read::sandbox_view(&db, sandbox_id) + .exists() .map_err(|error| Status::internal(format!("inspect import identity: {error}")))? { return Err(Status::already_exists( diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_snapshot_transfer.rs b/crates/libs/sbx/mgr/server/src/service/grpc_snapshot_transfer.rs index e17fb5729..e9ca7718c 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_snapshot_transfer.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_snapshot_transfer.rs @@ -329,7 +329,7 @@ impl SandboxManagerService { let lookup_id = sandbox_id.clone(); let sandbox = self .storage(move |db| { - crate::db::get_sandbox(db, &lookup_id) + crate::db::read::sandbox(db, &lookup_id) .map_err(|error| Status::internal(format!("inspect snapshot target: {error}"))) }) .await? diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_spaced_process_snapshots.rs b/crates/libs/sbx/mgr/server/src/service/grpc_spaced_process_snapshots.rs index 9cff218a3..33fe045d7 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_spaced_process_snapshots.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_spaced_process_snapshots.rs @@ -885,7 +885,7 @@ impl SandboxManagerService { selector: String, ) -> Result { self.storage(move |db| { - crate::db::inventory_read::lookup(db, &selector) + crate::db::scan::resolve_selector(db, crate::db::scan::InventorySelector, &selector) .map_err(|error| error.into_status())? .ok_or_else(|| Status::not_found(format!("sandbox not found: {selector}"))) }) @@ -1236,7 +1236,9 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::unavailable("snapshot ownership is unavailable"))?; - if crate::db::has_cleanup_episode(&db, sandbox_id) + if crate::db::read::sandbox_view(&db, sandbox_id) + .cleanup_episode() + .present() .map_err(|_| Status::unavailable("snapshot cleanup ownership cannot be read"))? { return Err(Status::failed_precondition( diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_state_snapshot.rs b/crates/libs/sbx/mgr/server/src/service/grpc_state_snapshot.rs index 75f3dca8f..367822563 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_state_snapshot.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_state_snapshot.rs @@ -567,8 +567,8 @@ impl SandboxManagerService { async fn state_batch( &self, sandbox_ids: Vec, - mut raw_budget: InventoryBudget, - mut metadata_budget: InventoryBudget, + raw_budget: InventoryBudget, + metadata_budget: InventoryBudget, ) -> Result<(StateResponse, InventoryBudget, InventoryBudget), Status> { let ( rows, @@ -582,33 +582,77 @@ impl SandboxManagerService { raw_budget, metadata_budget, ) = self.storage(move |db| { - crate::db::inventory_read::check_state_metadata(db, &sandbox_ids, &mut metadata_budget).map_err(|error| error.into_status())?; - let mut route_identity_provenance = - crate::db::activation_route_provenance_for_sandboxes(db, &sandbox_ids).map_err( - |error| Status::internal(format!("load route identity provenance: {error}")), - )?; - let mut rows = Vec::with_capacity(sandbox_ids.len()); + use crate::db::read::{StatePageError, StatePageRow}; + let mut budgets = crate::db::read::StateBudgets { + raw: raw_budget, + metadata: metadata_budget, + }; + let page = crate::db::read::state_page(db, &sandbox_ids, &mut budgets).map_err( + |error| match error { + StatePageError::Metadata(error) | StatePageError::Row(error) => { + error.into_status() + } + StatePageError::RouteProvenance(error) => Status::internal(format!( + "load route identity provenance: {error}" + )), + StatePageError::RawLifecycle { sandbox_id, error } => { + Status::internal(format!( + "read raw sandbox state for {sandbox_id}: {error}" + )) + } + StatePageError::Publication(error) => { + tracing::warn!(%error, "failed to load sandbox publication metadata"); + Status::internal(format!("load sandbox publication metadata: {error}")) + } + StatePageError::StateChangedAt(error) => { + Status::internal(format!("load state change times: {error}")) + } + StatePageError::PinnedArtifactCreatedAt(error) => Status::internal( + format!("load pinned artifact capture times: {error}"), + ), + StatePageError::RecoveryObservations(error) => { + tracing::warn!(%error, "failed to load quarantine recovery observations"); + Status::internal(format!("load quarantine recovery observations: {error}")) + } + }, + )?; + let mut rows = Vec::with_capacity(page.len()); let mut missing_ids = Vec::new(); let mut state_only = Vec::new(); - for sandbox_id in sandbox_ids { - match crate::db::inventory_read::get(db, &sandbox_id, &mut raw_budget) { - Ok(Some(sandbox)) => rows.push(sandbox), - Ok(None) => missing_ids.push(sandbox_id), - Err(error @ crate::db::inventory_read::InventoryReadError::Limit(_)) => return Err(error.into_status()), - Err(error @ crate::db::inventory_read::InventoryReadError::Ambiguous) => return Err(error.into_status()), - Err(crate::db::inventory_read::InventoryReadError::Database(error)) => { - let lifecycle = match crate::db::raw_sandbox_lifecycle(db, &sandbox_id) { - Ok(Some(state)) => state, - Ok(None) => { - missing_ids.push(sandbox_id); - continue; - } - Err(state_error) => { - return Err(Status::internal(format!( - "read raw sandbox state for {sandbox_id}: {state_error}" - ))); - } - }; + let mut route_identity_provenance = std::collections::HashMap::new(); + let mut publications = std::collections::BTreeMap::new(); + let mut state_changed_at = std::collections::HashMap::new(); + let mut pinned_artifact_created_at = std::collections::HashMap::new(); + let mut recovery_observations = std::collections::BTreeMap::new(); + for entry in page { + match entry { + StatePageRow::Missing(sandbox_id) => missing_ids.push(sandbox_id), + StatePageRow::Decoded(decoded) => { + let decoded = *decoded; + let id = decoded.row.id.clone(); + if let Some(provenance) = decoded.route_provenance { + route_identity_provenance.insert(id.clone(), provenance); + } + if let Some(publication) = decoded.publication { + publications.insert(id.clone(), publication); + } + if let Some(changed_at) = decoded.state_changed_at { + state_changed_at.insert(id.clone(), changed_at); + } + if let Some(created_at) = decoded.pinned_artifact_created_at { + pinned_artifact_created_at.insert(id.clone(), created_at); + } + if let Some(observation) = decoded.recovery_observation { + recovery_observations.insert(id, observation); + } + rows.push(decoded.row); + } + StatePageRow::StateOnly(crate::db::read::StateOnlyRow { + sandbox_id, + lifecycle, + error, + route_provenance, + }) => { let disposition = pplx_space_metrics::sandbox::publication::StatePublicationDisposition::StateOnly; pplx_space_metrics::sandbox::publication::degraded( pplx_space_metrics::sandbox::publication::StatePublicationComponent::Worker, @@ -621,9 +665,7 @@ impl SandboxManagerService { %disposition, "publishing raw lifecycle without malformed sandbox fields" ); - let route_identity_provenance = match route_identity_provenance - .remove(&sandbox_id) - { + let route_identity_provenance = match route_provenance { Some(Ok(provenance)) => provenance, Some(Err(provenance_error)) => { tracing::warn!( @@ -650,21 +692,6 @@ impl SandboxManagerService { } } } - let row_ids = rows.iter().map(|row| row.id.clone()).collect::>(); - let publications = crate::db::sandbox_state_publication_metadata(db, &row_ids) - .map_err(|error| { - tracing::warn!(%error, "failed to load sandbox publication metadata"); - Status::internal(format!("load sandbox publication metadata: {error}")) - })?; - let state_changed_at = crate::db::sandbox_state_changed_at(db, &row_ids) - .map_err(|error| Status::internal(format!("load state change times: {error}")))?; - let pinned_artifact_created_at = crate::db::pinned_local_artifact_created_at(db, &row_ids) - .map_err(|error| Status::internal(format!("load pinned artifact capture times: {error}")))?; - let recovery_observations = crate::db::quarantine_recovery_observations(db, &row_ids) - .map_err(|error| { - tracing::warn!(%error, "failed to load quarantine recovery observations"); - Status::internal(format!("load quarantine recovery observations: {error}")) - })?; Ok(( rows, missing_ids, @@ -674,8 +701,8 @@ impl SandboxManagerService { route_identity_provenance, state_changed_at, pinned_artifact_created_at, - raw_budget, - metadata_budget, + budgets.raw, + budgets.metadata, )) }) .await?; @@ -1384,8 +1411,14 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("sandbox database mutex poisoned"))?; - crate::db::get_named_local_artifact(&db, &sandbox_id, &snapshot_name) - .map_err(|error| Status::internal(format!("read operation snapshot: {error}")))? + crate::db::read::local_artifact( + &db, + crate::db::read::LocalArtifactKey::Named { + sandbox_id: &sandbox_id, + snapshot_name: &snapshot_name, + }, + ) + .map_err(|error| Status::internal(format!("read operation snapshot: {error}")))? }; if let Some(artifact) = existing.as_ref() && !disk_operation_snapshot_artifact_matches( @@ -1655,11 +1688,16 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("sandbox database mutex poisoned"))?; - crate::db::get_named_local_artifact(&db, &sandbox_id, &snapshot_name).map_err( - |error| { - Status::internal(format!("read concurrent operation snapshot: {error}")) + crate::db::read::local_artifact( + &db, + crate::db::read::LocalArtifactKey::Named { + sandbox_id: &sandbox_id, + snapshot_name: &snapshot_name, }, - )? + ) + .map_err(|error| { + Status::internal(format!("read concurrent operation snapshot: {error}")) + })? }; let matching_artifact_was_published = existing.as_ref().is_some_and(|artifact| { disk_operation_snapshot_artifact_matches( @@ -1782,7 +1820,7 @@ impl SandboxManagerService { let (sandbox, content_hash) = { let db = self.db_conn()?; - let sandbox = crate::db::get_sandbox(&db, &sandbox_id) + let sandbox = crate::db::read::sandbox(&db, &sandbox_id) .map_err(|e| Status::internal(format!("db error: {e}")))? .ok_or_else(|| Status::not_found(format!("sandbox not found: {sandbox_id}")))?; if !Self::sandbox_owned_by_template_manager(&sandbox) { diff --git a/crates/libs/sbx/mgr/server/src/service/mod.rs b/crates/libs/sbx/mgr/server/src/service/mod.rs index 11bee7a6e..8091ffcb7 100644 --- a/crates/libs/sbx/mgr/server/src/service/mod.rs +++ b/crates/libs/sbx/mgr/server/src/service/mod.rs @@ -941,12 +941,14 @@ fn persisted_activation_owns_route_identity( incoming: &RouteStreamEpochIdentity, ) -> Result { let sandbox_id = sandbox_id.to_string(); - let Some(episode) = crate::db::cleanup_episode(db, &sandbox_id) + let Some(episode) = crate::db::read::sandbox_view(db, &sandbox_id) + .cleanup_episode() + .strict() .map_err(|error| Status::internal(format!("db error: {error}")))? else { return Ok(false); }; - let Some(sandbox) = crate::db::get_sandbox(db, &sandbox_id) + let Some(sandbox) = crate::db::read::sandbox(db, &sandbox_id) .map_err(|error| Status::internal(format!("db error: {error}")))? else { return Ok(false); @@ -1246,7 +1248,7 @@ impl SandboxManagerService { RouteIdentityAdoptionReason::Internal, ) })?; - let Some(sandbox) = crate::db::get_sandbox(&db, &sandbox_id.to_string()) + let Some(sandbox) = crate::db::read::sandbox(&db, &sandbox_id.to_string()) .map_err(|error| Status::internal(format!("db error: {error}"))) .inspect_err(|_| { route_identity_adoption( @@ -1277,16 +1279,14 @@ impl SandboxManagerService { local.current_node_id = local.current_node_id.or(self.local_node_id); } let merged = merge_authoritative_route_identity(&local, incoming); - let metadata = crate::db::auto_lifecycle_delete_receipts::route_identity_adoption_metadata( - &db, sandbox_id, - ) - .map_err(|error| Status::internal(format!("inspect route identity metadata: {error}"))) - .inspect_err(|_| { - route_identity_adoption( - RouteIdentityAdoptionOutcome::Rejected, - RouteIdentityAdoptionReason::Internal, - ); - })?; + let metadata = crate::db::read::route_identity_adoption(&db, sandbox_id) + .map_err(|error| Status::internal(format!("inspect route identity metadata: {error}"))) + .inspect_err(|_| { + route_identity_adoption( + RouteIdentityAdoptionOutcome::Rejected, + RouteIdentityAdoptionReason::Internal, + ); + })?; let candidate_provenance = crate::auto_lifecycle_delete_receipts::RouteIdentityProvenance::for_authoritative_adoption( &local, @@ -1315,10 +1315,9 @@ impl SandboxManagerService { // Equality of the route fields does not prove the migration-only // provenance and receipt mode are current. Persist an authoritative // observation even when it does not advance the stream epoch. - let pending_receipt = - crate::db::auto_lifecycle_delete_receipts::pending_receipt_for_sandbox( - &db, sandbox_id, - ) + let pending_receipt = crate::db::read::typed_sandbox_view(&db, sandbox_id) + .delete_receipts() + .pending() .map_err(|error| { route_identity_adoption( RouteIdentityAdoptionOutcome::Rejected, @@ -1460,7 +1459,7 @@ impl SandboxManagerService { } _ => ActivationRouteReplacementAuthority::Unowned, }; - let persisted = crate::db::get_sandbox(&db, &sandbox_id.to_string()) + let persisted = crate::db::read::sandbox(&db, &sandbox_id.to_string()) .map_err(|error| Status::internal(format!("db error: {error}")))?; if let Some(sandbox) = persisted.as_ref() { validate_cold_start_provenance(sandbox, route_identity)?; @@ -1520,7 +1519,7 @@ impl SandboxManagerService { sandbox.space_api_route_identity.clone(), )?; let db = self.db_conn()?; - let current = crate::db::get_sandbox(&db, &sandbox.id) + let current = crate::db::read::sandbox(&db, &sandbox.id) .map_err(|error| Status::internal(format!("db error: {error}")))? .ok_or_else(|| Status::not_found(format!("sandbox not found: {}", sandbox.id)))?; let current_identity = RouteStreamEpochIdentity::from_stream_attachment( @@ -1677,7 +1676,7 @@ impl SandboxManagerService { .as_ref() .map(|bundle| bundle.generation.get()); if options.node_role != Some(NodeRole::Worker) { - match crate::db::auto_lifecycle_delete_receipts::outbox_stats(&conn) { + match crate::db::read::counts(&conn, crate::db::read::DeleteOutbox) { Ok(stats) if stats.pending_count.saturating_add(stats.completed_count) != 0 => { tracing::warn!( node_role = ?options.node_role, diff --git a/crates/libs/sbx/mgr/server/src/service/network_runtime.rs b/crates/libs/sbx/mgr/server/src/service/network_runtime.rs index e554c2851..39348bb79 100644 --- a/crates/libs/sbx/mgr/server/src/service/network_runtime.rs +++ b/crates/libs/sbx/mgr/server/src/service/network_runtime.rs @@ -725,7 +725,7 @@ fn refresh_abandon_reason( sandbox_id: &str, ) -> Result, crate::node_state_sync::DatabaseMutexPoisoned> { let db = crate::node_state_sync::connection_or_signal_failure(db)?; - Ok(match crate::db::get_sandbox(&db, sandbox_id) { + Ok(match crate::db::read::sandbox(&db, sandbox_id) { Ok(None) => Some(RefreshAbandonReason::SandboxRowGone), Ok(Some(sandbox)) if !sandbox.state.can_authorize_net_gateway() => Some( RefreshAbandonReason::StateCannotAuthorizeGateway(sandbox.state), diff --git a/crates/libs/sbx/mgr/server/src/service/reconcile.rs b/crates/libs/sbx/mgr/server/src/service/reconcile.rs index bd2f89094..6c3a26f65 100644 --- a/crates/libs/sbx/mgr/server/src/service/reconcile.rs +++ b/crates/libs/sbx/mgr/server/src/service/reconcile.rs @@ -332,7 +332,7 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("lifecycle database mutex poisoned"))?; - crate::db::get_sandbox(&db, id) + crate::db::read::sandbox(&db, id) .map_err(|error| Status::internal(format!("read auto lifecycle sandbox: {error}")))? .ok_or_else(|| { Status::not_found("sandbox disappeared before auto lifecycle transition") @@ -655,7 +655,7 @@ impl SandboxManagerService { let service = self.clone(); let scan = tokio::task::spawn_blocking(move || { let db = service.db_conn()?; - let selected = crate::db::due_auto_lifecycle_batch( + let selected = crate::db::scan::due_auto_lifecycle( &db, now_secs(), &active, @@ -727,8 +727,7 @@ impl SandboxManagerService { let mut scan = std::mem::take(cursor); let (scan, count) = self .blocking_io(move |service| { - let rows = scan - .next_batch(&*service.db_conn()?) + let rows = crate::db::scan::sandbox_pages(&*service.db_conn()?, &mut scan) .map_err(|error| Status::internal(format!("policy inventory: {error}")))?; let mut count = 0; for sandbox in rows { @@ -1058,7 +1057,7 @@ impl SandboxManagerService { if requested == BootMode::Pool { let has_pool = { let db = self.db_conn()?; - crate::db::find_pool_sandbox(&db, template, None, None) + crate::db::scan::pool_donor(&db, template, None, None) .map_err(|error| Status::internal(format!("resolve pool creation: {error}")))? .is_some() }; diff --git a/crates/libs/sbx/mgr/server/src/service/recovery.rs b/crates/libs/sbx/mgr/server/src/service/recovery.rs index ec53fb7c8..3ae29544a 100644 --- a/crates/libs/sbx/mgr/server/src/service/recovery.rs +++ b/crates/libs/sbx/mgr/server/src/service/recovery.rs @@ -341,7 +341,7 @@ impl SandboxManagerService { // Recover snapshot scheduling for started sandboxes with snapshots_enabled let started = { let db = self.startup_recovery_db()?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; for row in started .into_iter() @@ -375,7 +375,7 @@ impl SandboxManagerService { // and the prelaunch classification that consumes its answers. let rows = { let db = self.startup_recovery_db()?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; let verdicts = self .suspending_retained_source_verdicts(Path::new(CACHE_DIR), &rows) @@ -408,7 +408,7 @@ impl SandboxManagerService { ) -> Result, StartupRecoveryError> { let rows = { let db = self.startup_recovery_db()?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; let failed = AtomicBool::new(false); stream::iter( @@ -454,7 +454,7 @@ impl SandboxManagerService { Output = crate::runtime_reconciliation::FirecrackerVmStateObservation, >, { - if !crate::db::sandbox_has_runtime_fields(&row) { + if !row.has_runtime_fields() { return self .quarantine_interrupted_pause(row, "sandbox row has no retained runtime identity") .await; @@ -666,7 +666,7 @@ impl SandboxManagerService { ) -> Result, StartupRecoveryError> { let rows = { let db = self.startup_recovery_db()?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; self.recover_prelaunch_rows(cache_dir, verdicts, rows) } @@ -683,7 +683,7 @@ impl SandboxManagerService { SandboxState::Creating => { records.push(self.recover_interrupted_create(cache_dir, row)?); } - SandboxState::Starting if !crate::db::sandbox_has_runtime_fields(&row) => { + SandboxState::Starting if !row.has_runtime_fields() => { records.push(self.recover_unlaunched_start(cache_dir, row)?); } SandboxState::Suspending => { @@ -800,13 +800,15 @@ impl SandboxManagerService { }; let owned = self.startup_recovery_db().and_then(|db| { if !crate::db::startup_resident_witness_current(&db, row)? - || crate::db::teardown_suspend_status(&db, &row.id)? + || crate::db::read::sandbox_view(&db, &row.id) + .teardown() + .status()? .and_then(|status| status.operation) .is_some_and(|operation| operation.status.is_active()) { return Ok(None); } - Ok(crate::db::get_sandbox(&db, &row.id)?.filter(unowned_publication)) + Ok(crate::db::read::sandbox(&db, &row.id)?.filter(unowned_publication)) }); let row = match owned { Ok(Some(row)) => row, @@ -868,7 +870,9 @@ impl SandboxManagerService { { let db = self.startup_recovery_db()?; if !crate::db::startup_resident_witness_current(&db, &row)? - || crate::db::teardown_suspend_status(&db, &row.id)? + || crate::db::read::sandbox_view(&db, &row.id) + .teardown() + .status()? .and_then(|status| status.operation) .is_some_and(|operation| operation.status.is_active()) { @@ -899,7 +903,9 @@ impl SandboxManagerService { ) -> Result { let teardown_status = { let db = self.startup_recovery_db()?; - crate::db::teardown_suspend_status(&db, &row.id)? + crate::db::read::sandbox_view(&db, &row.id) + .teardown() + .status()? }; let teardown_generation = teardown_status .as_ref() @@ -1217,7 +1223,10 @@ impl SandboxManagerService { ) -> Result { { let db = self.startup_recovery_db()?; - if crate::db::has_cleanup_episode(&db, &row.id)? { + if crate::db::read::sandbox_view(&db, &row.id) + .cleanup_episode() + .present()? + { // Startup cleanup owns interrupted creates, including partial // unpublished clones. Promoting the row would retire its // armed deletion episode before resources are cleaned. @@ -1259,7 +1268,9 @@ impl SandboxManagerService { ) -> Result { let has_episode = { let db = self.startup_recovery_db()?; - crate::db::has_cleanup_episode(&db, &row.id)? + crate::db::read::sandbox_view(&db, &row.id) + .cleanup_episode() + .present()? }; if has_episode { // Even before runtime allocation, the episode owns rollback and @@ -1451,8 +1462,7 @@ impl SandboxManagerService { let mut cursor = std::mem::take(scan_cursor); let scan = self .storage(move |db| { - let batch = cursor - .next_batch(db) + let batch = crate::db::scan::quarantine_recovery(db, &mut cursor) .map_err(|error| Status::internal(format!("quarantine discovery: {error}")))?; Ok((cursor, batch)) }) @@ -2092,7 +2102,7 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| StartupRecoveryError::DatabaseMutexPoisoned)?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; Self::recover_required_cleanup_targets_from_rows(db, rows, authority) @@ -2110,7 +2120,7 @@ impl SandboxManagerService { .into_iter() .filter_map(|sandbox_id| { let sandbox_id = sandbox_id.to_string(); - match crate::db::get_sandbox(&db, &sandbox_id) { + match crate::db::read::sandbox(&db, &sandbox_id) { Ok(row) => row, Err(error) => { tracing::warn!( @@ -2194,7 +2204,10 @@ impl SandboxManagerService { let db = db .lock() .map_err(|_| StartupRecoveryError::DatabaseMutexPoisoned)?; - match crate::db::cleanup_episode(&db, &row.id) { + match crate::db::read::sandbox_view(&db, &row.id) + .cleanup_episode() + .strict() + { Ok(episode) => episode, Err(error) => { tracing::warn!( @@ -2298,9 +2311,7 @@ impl SandboxManagerService { continue; } let cleanup_intent = match authority { - CleanupRecoveryAuthority::Startup => { - crate::db::cleanup_intent_for_startup_recovery(&row) - } + CleanupRecoveryAuthority::Startup => row.cleanup_intent_for_startup_recovery(), CleanupRecoveryAuthority::RequestedOnly => { // State alone is authoritative for teardown states entered // under the cleanup fence and the settled legacy cleanup @@ -2324,7 +2335,7 @@ impl SandboxManagerService { } (None, None) if matches!(row.state, SandboxState::CleanupFailed) - && crate::db::sandbox_has_runtime_fields(&row) => + && row.has_runtime_fields() => { Some((true, CleanupCompletion::Quarantine)) } @@ -2451,7 +2462,7 @@ impl SandboxManagerService { } let mut cursor = std::mem::take(&mut recovery_cursor); let candidates = match service.storage(move |db| { - let rows = crate::db::periodic_lifecycle_recovery_batch(db, &mut cursor) + let rows = crate::db::scan::sandbox_pages(db, &mut cursor) .map_err(|error| Status::internal(format!("lifecycle discovery: {error}")))?; Ok((cursor, rows)) }).await { diff --git a/crates/libs/sbx/mgr/server/src/service/recovery/staging_cleanup.rs b/crates/libs/sbx/mgr/server/src/service/recovery/staging_cleanup.rs index a5baf20f6..2b425267f 100644 --- a/crates/libs/sbx/mgr/server/src/service/recovery/staging_cleanup.rs +++ b/crates/libs/sbx/mgr/server/src/service/recovery/staging_cleanup.rs @@ -93,7 +93,7 @@ impl SandboxManagerService { }; let eligible = if guards.is_some() { self.startup_recovery_db().and_then(|db| { - crate::db::get_sandbox(&db, &id).map_err(StartupRecoveryError::from) + crate::db::read::sandbox(&db, &id).map_err(StartupRecoveryError::from) }) } else { Ok(None) @@ -289,11 +289,8 @@ impl SandboxManagerService { artifact: LocalArtifactId, ) -> Result { let db = self.startup_recovery_db()?; - Ok(crate::db::sandbox_suspended_with_pinned_artifact( - &db, - sandbox_id, - &artifact.to_string(), - )?) + Ok(crate::db::read::sandbox_view(&db, sandbox_id) + .suspended_with_pinned_artifact(&artifact.to_string())?) } } diff --git a/crates/libs/sbx/mgr/server/src/service/recovery_cross_node.rs b/crates/libs/sbx/mgr/server/src/service/recovery_cross_node.rs index 26e1f5394..701b47635 100644 --- a/crates/libs/sbx/mgr/server/src/service/recovery_cross_node.rs +++ b/crates/libs/sbx/mgr/server/src/service/recovery_cross_node.rs @@ -714,10 +714,11 @@ fn adopt_restoring_row_under_recovery_claim( // The generation CAS below sees the newly written Restoring state, so it // cannot independently prove the prior row was still eligible for recovery. if recovery.is_some() { - let existing_state = - crate::db::current_state_label(&tx, canonical_id).map_err(|source| { - crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source } - })?; + let existing_state = crate::db::read::sandbox_view(&tx, canonical_id) + .current_state_label() + .map_err( + |source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source }, + )?; // An absent row is the orphan case this path exists for; the upsert // inserts it. Only an existing row in an unexpected state is a refusal. // @@ -1063,14 +1064,14 @@ fn claim_exchanged_restore_cleanup( let db = db.lock().map_err(|_| { crate::snapshot_restore::RestoreError::Other("sandbox database mutex poisoned".to_string()) })?; - if crate::db::canonical_restore_reclaim_attempt_is_current( - &db, - sandbox_id, - existing.transfer_scope_id, - execution_generation, - activation_attempt_id, - ) - .map_err(|source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source })? + if crate::db::read::sandbox_view(&db, sandbox_id) + .restore_scope() + .reclaim_attempt_is_current( + existing.transfer_scope_id, + execution_generation, + activation_attempt_id, + ) + .map_err(|source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source })? { return Ok(existing.clone()); } @@ -1087,7 +1088,7 @@ fn claim_exchanged_restore_cleanup( "restore activation attempt is not newer than the durable exchanged-root cleanup owner for {sandbox_id}; retry" ))); } - crate::db::restore_transfer_scope(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id).restore_scope().scope() .map_err(|source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source })? .ok_or_else(|| { crate::snapshot_restore::RestoreError::Other(format!( @@ -1150,9 +1151,13 @@ impl SandboxManagerService { .db .lock() .map_err(|_| crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned)?; - if crate::db::quarantine_recovery_claim_is_current(&db, claimed).map_err(|source| { - crate::snapshot_restore::RestoreError::ReadQuarantineRecoveryClaim { source } - })? { + if crate::db::read::sandbox_view(&db, &claimed.candidate.sandbox_id.to_string()) + .quarantine() + .claim_is_current(claimed) + .map_err(|source| { + crate::snapshot_restore::RestoreError::ReadQuarantineRecoveryClaim { source } + })? + { return Ok(()); } Err( @@ -1303,7 +1308,9 @@ impl SandboxManagerService { let db = self .db_conn() .map_err(|_| crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned)?; - crate::db::restore_transfer_scope(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .restore_scope() + .scope() .map_err( |source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source }, )? @@ -1392,7 +1399,7 @@ impl SandboxManagerService { let db = self .db_conn() .map_err(|_| crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned)?; - crate::db::get_sandbox(&db, &canonical_id) + crate::db::read::sandbox(&db, &canonical_id) .map_err(|source| RestoreError::ReadSandboxRow { source })? }; let mut recovering_orphan_quarantine = false; @@ -1413,7 +1420,9 @@ impl SandboxManagerService { let db = self.db_conn().map_err(|_| { crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned })?; - crate::db::is_orphan_quarantine_recovery_state(&db, &canonical_id, row.state) + crate::db::read::sandbox_view(&db, &canonical_id) + .quarantine() + .is_orphan_recovery_state(row.state) .map_err(|source| RestoreError::ReadSandboxRow { source })? }; if !eligible { @@ -1460,7 +1469,7 @@ impl SandboxManagerService { .result("removed"), ); } - } else if crate::db::sandbox_is_retryable_interrupted_restore(&row) { + } else if row.is_retryable_interrupted_restore() { retrying_interrupted_restore = true; interrupted_restore_source = row .restore_operation @@ -1553,7 +1562,9 @@ impl SandboxManagerService { let db = self.db_conn().map_err(|_| { crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned })?; - crate::db::restore_transfer_scope(&db, &canonical_id) + crate::db::read::sandbox_view(&db, &canonical_id) + .restore_scope() + .scope() .map_err(|source| RestoreError::AdvanceRestorePhase { source })? .filter(|scope| std::path::Path::new(&scope.staging_dir).exists()) }; @@ -1661,7 +1672,9 @@ impl SandboxManagerService { let db = self.db_conn().map_err(|_| { crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned })?; - crate::db::restore_transfer_scope(&db, &canonical_id) + crate::db::read::sandbox_view(&db, &canonical_id) + .restore_scope() + .scope() .map_err(|source| RestoreError::AdvanceRestorePhase { source })? }; if let Some(existing) = existing { @@ -2027,14 +2040,14 @@ impl SandboxManagerService { let db = self.db_conn().map_err(|_| { crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned })?; - crate::db::canonical_restore_reclaim_attempt_is_current( - &db, - &canonical_id, - transfer_scope_id, - execution_generation, - activation_attempt_id, - ) - .map_err(|source| RestoreError::AdvanceRestorePhase { source })? + crate::db::read::sandbox_view(&db, &canonical_id) + .restore_scope() + .reclaim_attempt_is_current( + transfer_scope_id, + execution_generation, + activation_attempt_id, + ) + .map_err(|source| RestoreError::AdvanceRestorePhase { source })? }; if !attempt_is_current { return Err(RestoreError::Other(format!( @@ -2074,7 +2087,7 @@ impl SandboxManagerService { let db = self.db_conn().map_err(|_| { crate::snapshot_restore::RestoreError::SandboxDatabaseMutexPoisoned })?; - crate::db::restore_transfer_scope(&db, &canonical_id) + crate::db::read::sandbox_view(&db, &canonical_id).restore_scope().scope() .map_err(|source| RestoreError::AdvanceRestorePhase { source })? .ok_or_else(|| { RestoreError::Other(format!( diff --git a/crates/libs/sbx/mgr/server/src/service/spaced_bridge.rs b/crates/libs/sbx/mgr/server/src/service/spaced_bridge.rs index 48496a6ac..cfaf5970d 100644 --- a/crates/libs/sbx/mgr/server/src/service/spaced_bridge.rs +++ b/crates/libs/sbx/mgr/server/src/service/spaced_bridge.rs @@ -170,7 +170,7 @@ impl SandboxManagerService { let db = self .db_conn() .inspect_err(record_endpoint_attestation_error)?; - let winning_proof = crate::db::get_sandbox(&db, &sandbox.id) + let winning_proof = crate::db::read::sandbox(&db, &sandbox.id) .map_err(|error| { Status::internal(format!("reload spaced endpoint proof: {error}")) }) diff --git a/crates/libs/sbx/mgr/server/src/service/start.rs b/crates/libs/sbx/mgr/server/src/service/start.rs index 6c753163d..f391f13b0 100644 --- a/crates/libs/sbx/mgr/server/src/service/start.rs +++ b/crates/libs/sbx/mgr/server/src/service/start.rs @@ -581,9 +581,12 @@ async fn prepare_suspend_restore_dir( "sandbox database mutex poisoned".to_string(), ) })?; - crate::db::restore_transfer_scope(&db, &sandbox_id).map_err(|source| { - crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source } - })? + crate::db::read::sandbox_view(&db, &sandbox_id) + .restore_scope() + .scope() + .map_err( + |source| crate::snapshot_restore::RestoreError::AdvanceRestorePhase { source }, + )? }; if let Some(existing_scope) = existing_scope { service @@ -954,7 +957,9 @@ impl SandboxManagerService { .db .lock() .map_err(|_| Status::internal("database mutex poisoned"))?; - let episode = crate::db::cleanup_episode(&db, &sandbox.id) + let episode = crate::db::read::sandbox_view(&db, &sandbox.id) + .cleanup_episode() + .strict() .map_err(|error| Status::internal(format!("load restore cleanup: {error}")))?; Ok(episode .filter(|episode| { diff --git a/crates/libs/sbx/mgr/server/src/service/startup_resident.rs b/crates/libs/sbx/mgr/server/src/service/startup_resident.rs index d29046b0e..28a00ae84 100644 --- a/crates/libs/sbx/mgr/server/src/service/startup_resident.rs +++ b/crates/libs/sbx/mgr/server/src/service/startup_resident.rs @@ -18,7 +18,7 @@ impl SandboxManagerService { ) -> Result, StartupRecoveryError> { let rows = { let db = self.startup_recovery_db()?; - crate::db::list_sandboxes_for_recovery(&db)? + crate::db::scan::all_sandboxes(&db, crate::db::scan::SkipUndecodableRows)? }; let failed = AtomicBool::new(false); stream::iter(rows.into_iter().filter(|row| { @@ -169,7 +169,7 @@ impl SandboxManagerService { detail = "startup recovery found an invalid sandbox ID; operator repair required".into(); } - Ok(id) if crate::db::sandbox_has_runtime_fields(&row) => { + Ok(id) if row.has_runtime_fields() => { use crate::runtime_reconciliation::observe_stable_resident_state; use pplx_space_node_runtime_core::fc_api::FirecrackerVmState; match observe_stable_resident_state(id, probe).await { diff --git a/crates/libs/sbx/mgr/server/src/snapshot_gc.rs b/crates/libs/sbx/mgr/server/src/snapshot_gc.rs index 685519e61..484fe3538 100644 --- a/crates/libs/sbx/mgr/server/src/snapshot_gc.rs +++ b/crates/libs/sbx/mgr/server/src/snapshot_gc.rs @@ -393,7 +393,7 @@ async fn gc_tick( let mut next_scan = *scan; let (next_scan, page) = tokio::task::spawn_blocking(move || { let conn = crate::node_state_sync::connection_or_signal_failure(&db_for_snap)?; - let page = next_scan.next_page(&conn)?; + let page = crate::db::scan::local_artifacts(&conn, &mut next_scan)?; Ok::<_, GcDatabaseReadError>((next_scan, page)) }) .await diff --git a/crates/libs/sbx/mgr/server/src/snapshot_scheduler.rs b/crates/libs/sbx/mgr/server/src/snapshot_scheduler.rs index b4e2e5b76..85ee8fd75 100644 --- a/crates/libs/sbx/mgr/server/src/snapshot_scheduler.rs +++ b/crates/libs/sbx/mgr/server/src/snapshot_scheduler.rs @@ -452,7 +452,8 @@ async fn process_tick( let execution_fence: Option = { let db = lock_db(ctx)?; - crate::db::node_execution_fence(&db, sandbox_id) + crate::db::read::sandbox_view(&db, sandbox_id) + .node_execution_fence() .map_err(|error| error.to_string())? .map(Into::into) }; @@ -805,7 +806,7 @@ pub async fn do_checkpoint( let (previous_state, execution_fence, execution) = { let db = lock_db(ctx)?; let sandbox = - crate::db::get_sandbox(&db, sandbox_id).map_err(|e| format!("db error: {e}"))?; + crate::db::read::sandbox(&db, sandbox_id).map_err(|e| format!("db error: {e}"))?; let sandbox = match sandbox { Some(s) => s, None => return Err(format!("sandbox {sandbox_id} not found in DB")), diff --git a/crates/libs/sbx/mgr/server/src/snapshot_upload_jobs.rs b/crates/libs/sbx/mgr/server/src/snapshot_upload_jobs.rs index 111153178..c26a1cee0 100644 --- a/crates/libs/sbx/mgr/server/src/snapshot_upload_jobs.rs +++ b/crates/libs/sbx/mgr/server/src/snapshot_upload_jobs.rs @@ -883,11 +883,12 @@ fn stream_reclaim_due( return Ok(false); } let sandbox_id = &payload.meta.sandbox_id; - if !crate::db::sandbox_suspended_with_pinned_artifact( - conn, - sandbox_id, - &local_artifact_id.to_string(), - )? { + // Not a fence here: the connection guard is released before node-manager + // is asked, so `reclaim_stream` repeats this check under the hold that + // unlinks. + if !crate::db::read::sandbox_view(conn, sandbox_id) + .suspended_with_pinned_artifact(&local_artifact_id.to_string())? + { pplx_space_metrics::snapshot::stream_reclaim_skipped("not_pinned_suspended"); tracing::info!( sandbox_id, @@ -923,11 +924,11 @@ fn reclaim_stream( return Ok(()); } let sandbox_id = &payload.meta.sandbox_id; - if !crate::db::sandbox_suspended_with_pinned_artifact( - conn, - sandbox_id, - &local_artifact_id.to_string(), - )? { + // A fence only because `conn` is the connection guard every state + // transition serializes on, held here across the unlink below. + if !crate::db::read::sandbox_view(conn, sandbox_id) + .suspended_with_pinned_artifact(&local_artifact_id.to_string())? + { pplx_space_metrics::snapshot::stream_reclaim_skipped("not_pinned_suspended"); tracing::info!( sandbox_id, diff --git a/crates/libs/sbx/mgr/server/src/watchdog.rs b/crates/libs/sbx/mgr/server/src/watchdog.rs index 66d1721f1..e0c7b2852 100644 --- a/crates/libs/sbx/mgr/server/src/watchdog.rs +++ b/crates/libs/sbx/mgr/server/src/watchdog.rs @@ -28,7 +28,7 @@ use pplx_space_metrics::sandbox::worker::{ use pplx_space_node_runtime_core::fc_api::FirecrackerVmState; use crate::auto_lifecycle::AutoLifecycleCoordinator; -use crate::db::guest_health_scan::{SandboxGuestHealthRow, list_guest_health_rows}; +use crate::db::guest_health_scan::SandboxGuestHealthRow; use crate::node_manager_transport::NodeManagerConnector; use crate::node_state_sync::mark_sandbox_dirty; use crate::service::network_policy::NetworkPolicyReconcileQueue; @@ -887,7 +887,7 @@ impl SandboxWatchdog { return; } }; - match crate::db::get_sandbox(&conn, sandbox_id) { + match crate::db::read::sandbox(&conn, sandbox_id) { Ok(Some(row)) => sidecar_info_from_row(&row), Ok(None) => { tracing::debug!(sandbox_id, "sandbox disappeared before watchdog re-arm"); @@ -1084,7 +1084,7 @@ impl SandboxWatchdog { continue; } }; - match crate::db::get_sandbox(&conn, &id) { + match crate::db::read::sandbox(&conn, &id) { Ok(row) => row, Err(error) => { tracing::warn!(sandbox_id = %id, %error, "failed to load live sandbox during watchdog recovery"); @@ -1557,14 +1557,14 @@ fn emit_auto_lifecycle_delete_gauges(db: &crate::db::SharedConnection) { return; } }; - let outbox = match crate::db::auto_lifecycle_delete_receipts::outbox_stats(&db) { + let outbox = match crate::db::read::counts(&db, crate::db::read::DeleteOutbox) { Ok(stats) => stats, Err(error) => { tracing::warn!(%error, "failed to inspect auto-lifecycle delete outbox gauges"); return; } }; - let blocked = match crate::db::auto_lifecycle_delete_receipts::blocked_stats(&db) { + let blocked = match crate::db::read::counts(&db, crate::db::read::DeleteBlocked) { Ok(stats) => stats, Err(error) => { tracing::warn!(%error, "failed to inspect auto-lifecycle delete block gauges"); @@ -1611,7 +1611,7 @@ fn load_guest_health_rows(db: &crate::db::SharedConnection) -> Option Some(rows), Err(error) => { tracing::warn!(%error, "failed to list sandboxes for lifecycle gauges"); @@ -1894,7 +1894,7 @@ fn sandbox_states_for_ids( return None; } }; - let labels = match crate::db::guest_health_scan::state_labels_for_ids(&db, sandbox_ids) { + let labels = match crate::db::read::sandbox_views(&db, sandbox_ids).state_labels() { Ok(labels) => labels, Err(error) => { tracing::warn!(%error, "failed to prepare watchdog probe-state lookup"); @@ -3091,7 +3091,7 @@ fn net_gateway_relaunch_args( let conn = db .lock() .map_err(|error| NetGatewayRelaunchArgsError::DatabaseMutexPoisoned(error.to_string()))?; - let sandbox = crate::db::get_sandbox(&conn, sandbox_id) + let sandbox = crate::db::read::sandbox(&conn, sandbox_id) .map_err(|source| NetGatewayRelaunchArgsError::LoadSandbox { sandbox_id: sandbox_id.to_string(), source, diff --git a/crates/libs/sbx/mgr/store/AGENTS.md b/crates/libs/sbx/mgr/store/AGENTS.md index 39b35a701..3734fa991 100644 --- a/crates/libs/sbx/mgr/store/AGENTS.md +++ b/crates/libs/sbx/mgr/store/AGENTS.md @@ -15,6 +15,10 @@ that nothing else in the server's production graph brings them in. by design. A method that hands one out is declared through `db_private_outside_tests!` (`src/db/handle.rs`), which makes it public only with `test-support`. +- Code outside the store reads through `db::read` (one record or a narrow + view of it) and `db::scan` (named many-row scans). Extend those before + adding a public reader; the readers they consolidate are declared through + `db_private_outside_tests!`, so only tests outside the store still call them. - Write `sandboxes.state` only through `update_state_from` with an explicit edge, or through a named specialized writer. A new edge in `src/db/state_write.rs` needs a kind and a doc comment naming its writer; diff --git a/crates/libs/sbx/mgr/store/src/db.rs b/crates/libs/sbx/mgr/store/src/db.rs index 19155fd1b..df9f0dd3a 100644 --- a/crates/libs/sbx/mgr/store/src/db.rs +++ b/crates/libs/sbx/mgr/store/src/db.rs @@ -34,6 +34,7 @@ use rusqlite::{OptionalExtension as _, params}; mod handle; pub use handle::OptionalExtension; +use handle::db_private_outside_tests; pub use handle::{Connection, ConnectionGuard, Error, Result, SharedConnection, Transaction}; pub mod auto_lifecycle_delete_receipts; @@ -66,8 +67,11 @@ use auto_lifecycle_delete_receipts::discard_pending_receipt_for_execution_advanc pub use teardown_jobs::{ TeardownSuspendFailure, defer_teardown_suspend_retry, fail_teardown_suspend_backup_operation, fail_teardown_suspend_operation, reconcile_terminal_teardown_suspend_jobs, - teardown_suspend_retry_admissible, teardown_suspend_retry_due, }; +#[cfg(not(any(test, feature = "test-support")))] +use teardown_jobs::{teardown_suspend_retry_admissible, teardown_suspend_retry_due}; +#[cfg(any(test, feature = "test-support"))] +pub use teardown_jobs::{teardown_suspend_retry_admissible, teardown_suspend_retry_due}; #[cfg(any(test, feature = "test-support"))] mod test_schema_image_cache; pub use quarantine_scan::PeriodicQuarantineRecoveryScan; @@ -545,15 +549,17 @@ fn record_execution_delete_receipt( Ok(()) } -pub fn execution_delete_receipt_covers( - conn: &Connection, - sandbox_id: &str, - execution_generation: ExecutionGeneration, -) -> Result { - let now = now_epoch_i64()?; - read::sandbox_view(conn, sandbox_id) - .delete_receipts() - .execution_covers(execution_generation, now) +db_private_outside_tests! { + fn execution_delete_receipt_covers( + conn: &Connection, + sandbox_id: &str, + execution_generation: ExecutionGeneration, + ) -> Result { + let now = now_epoch_i64()?; + read::sandbox_view(conn, sandbox_id) + .delete_receipts() + .execution_covers(execution_generation, now) + } } /// Only use at read/decode boundaries. Statement, schema and write failures diff --git a/crates/libs/sbx/mgr/store/src/db/auto_lifecycle.rs b/crates/libs/sbx/mgr/store/src/db/auto_lifecycle.rs index 0d466cfdf..944d04a72 100644 --- a/crates/libs/sbx/mgr/store/src/db/auto_lifecycle.rs +++ b/crates/libs/sbx/mgr/store/src/db/auto_lifecycle.rs @@ -94,112 +94,114 @@ pub fn auto_lifecycle_state_is_settled(state: SandboxState) -> bool { !state.is_transient() } -pub fn due_auto_lifecycle_batch( - conn: &Connection, - now_secs: u64, - active: &[SandboxId], - available: usize, - cursor: &mut AutoLifecycleScan, -) -> Result> { - if available == 0 { - return Ok(Vec::new()); - } - let now = now_secs.min(i64::MAX as u64) as i64; - let mut selected = Vec::new(); - let fresh_slots = if available == 1 { - usize::from(cursor.fresh_turn) - } else { - available / 2 - }; - let eligible = |row: &rusqlite::Row<'_>, - selected: &[DueAutoLifecycleSandbox]| - -> Result> { - let (id, deadline) = match (row.get::<_, String>(1), row.get::<_, Option>(2)) { - (Ok(id), Ok(deadline)) => (id, deadline), - (Err(error), _) | (_, Err(error)) if is_row_decode_error(&error) => { - tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row"); +db_private_outside_tests! { + fn due_auto_lifecycle_batch( + conn: &Connection, + now_secs: u64, + active: &[SandboxId], + available: usize, + cursor: &mut AutoLifecycleScan, + ) -> Result> { + if available == 0 { + return Ok(Vec::new()); + } + let now = now_secs.min(i64::MAX as u64) as i64; + let mut selected = Vec::new(); + let fresh_slots = if available == 1 { + usize::from(cursor.fresh_turn) + } else { + available / 2 + }; + let eligible = |row: &rusqlite::Row<'_>, + selected: &[DueAutoLifecycleSandbox]| + -> Result> { + let (id, deadline) = match (row.get::<_, String>(1), row.get::<_, Option>(2)) { + (Ok(id), Ok(deadline)) => (id, deadline), + (Err(error), _) | (_, Err(error)) if is_row_decode_error(&error) => { + tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row"); + return Ok(None); + } + (Err(error), _) | (_, Err(error)) => return Err(Error::from(error)), + }; + let Some(deadline) = deadline else { + return Ok(None); + }; + let Ok(parsed) = id.parse::() else { + return Ok(None); + }; + // The rowid lane sees every row; a row no action can ever act on is + // not due work (see `AUTO_LIFECYCLE_NEVER_ACTIONABLE_STATES_SQL`). + // A state that does not decode is skipped like a malformed id above; + // a statement-level failure propagates. An unknown-but-decodable + // state string is left to the transition's own lookup error. + let state = match row.get::<_, String>(3) { + Ok(state) => state, + Err(error) if is_row_decode_error(&error) => { + tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row"); + return Ok(None); + } + Err(error) => return Err(Error::from(error)), + }; + if state.parse::().ok() == Some(SandboxState::Deleting) { return Ok(None); } - (Err(error), _) | (_, Err(error)) => return Err(Error::from(error)), - }; - let Some(deadline) = deadline else { - return Ok(None); - }; - let Ok(parsed) = id.parse::() else { - return Ok(None); - }; - // The rowid lane sees every row; a row no action can ever act on is - // not due work (see `AUTO_LIFECYCLE_NEVER_ACTIONABLE_STATES_SQL`). - // A state that does not decode is skipped like a malformed id above; - // a statement-level failure propagates. An unknown-but-decodable - // state string is left to the transition's own lookup error. - let state = match row.get::<_, String>(3) { - Ok(state) => state, - Err(error) if is_row_decode_error(&error) => { - tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row"); + if !(0..=now).contains(&deadline) + || active.contains(&parsed) + || selected.iter().any(|due| due.sandbox_id == parsed) + { return Ok(None); } - Err(error) => return Err(Error::from(error)), + Ok(Some(DueAutoLifecycleSandbox { + storage_id: id, + sandbox_id: parsed, + })) }; - if state.parse::().ok() == Some(SandboxState::Deleting) { - return Ok(None); - } - if !(0..=now).contains(&deadline) - || active.contains(&parsed) - || selected.iter().any(|due| due.sandbox_id == parsed) - { - return Ok(None); - } - Ok(Some(DueAutoLifecycleSandbox { - storage_id: id, - sandbox_id: parsed, - })) - }; - // LIMIT applies to raw index entries, before identity/metadata eligibility. - if fresh_slots > 0 { - let mut statement = conn.prepare(&sql!(format!( - "SELECT rowid, id, next_auto_event_at, state + // LIMIT applies to raw index entries, before identity/metadata eligibility. + if fresh_slots > 0 { + let mut statement = conn.prepare(&sql!(format!( + "SELECT rowid, id, next_auto_event_at, state FROM sandboxes INDEXED BY idx_sandboxes_next_auto_event_at WHERE next_auto_event_at <= ?1 AND state NOT IN {AUTO_LIFECYCLE_NEVER_ACTIONABLE_STATES_SQL} ORDER BY next_auto_event_at DESC, created_at DESC LIMIT 32" - )))?; - let mut rows = statement.query([now])?; - while let Some(row) = rows.next()? { - if let Some(id) = eligible(row, &selected)? { - selected.push(id); - } - if selected.len() == fresh_slots { - break; + )))?; + let mut rows = statement.query([now])?; + while let Some(row) = rows.next()? { + if let Some(id) = eligible(row, &selected)? { + selected.push(id); + } + if selected.len() == fresh_slots { + break; + } } } - } - // The rowid lane must still ADVANCE past `deleting` rows (it is a scan - // position, not a work list), so they stay in the result set of this - // statement and are filtered by `eligible`, not by SQL. - let mut statement = conn.prepare(sql!( - "SELECT rowid, id, next_auto_event_at, state + // The rowid lane must still ADVANCE past `deleting` rows (it is a scan + // position, not a work list), so they stay in the result set of this + // statement and are filtered by `eligible`, not by SQL. + let mut statement = conn.prepare(sql!( + "SELECT rowid, id, next_auto_event_at, state FROM sandboxes WHERE rowid > ?1 ORDER BY rowid LIMIT 128" - ))?; - let mut rows = statement.query([cursor.after])?; - let mut scanned = 0; - while selected.len() < available { - let Some(row) = rows.next()? else { - if scanned < 128 { - cursor.after = i64::MIN; + ))?; + let mut rows = statement.query([cursor.after])?; + let mut scanned = 0; + while selected.len() < available { + let Some(row) = rows.next()? else { + if scanned < 128 { + cursor.after = i64::MIN; + } + break; + }; + scanned += 1; + cursor.after = row.get(0)?; + if let Some(id) = eligible(row, &selected)? { + selected.push(id); } - break; - }; - scanned += 1; - cursor.after = row.get(0)?; - if let Some(id) = eligible(row, &selected)? { - selected.push(id); } + if available == 1 { + cursor.fresh_turn = !cursor.fresh_turn; + } + Ok(selected) } - if available == 1 { - cursor.fresh_turn = !cursor.fresh_turn; - } - Ok(selected) } pub fn update_auto_lifecycle( diff --git a/crates/libs/sbx/mgr/store/src/db/auto_lifecycle_delete_receipts.rs b/crates/libs/sbx/mgr/store/src/db/auto_lifecycle_delete_receipts.rs index e6039aff3..962539a15 100644 --- a/crates/libs/sbx/mgr/store/src/db/auto_lifecycle_delete_receipts.rs +++ b/crates/libs/sbx/mgr/store/src/db/auto_lifecycle_delete_receipts.rs @@ -1,3 +1,4 @@ +use crate::db::db_private_outside_tests; use std::str::FromStr; use crate::db::{Connection, Error, OptionalExtension}; @@ -595,53 +596,55 @@ pub fn record_retained_route_evidence( Ok(updated) } -pub fn route_identity_adoption_metadata( - conn: &Connection, - sandbox_id: SandboxId, -) -> crate::db::Result { - conn.query_row( - "SELECT space_api_route_identity_provenance, +db_private_outside_tests! { + fn route_identity_adoption_metadata( + conn: &Connection, + sandbox_id: SandboxId, + ) -> crate::db::Result { + conn.query_row( + "SELECT space_api_route_identity_provenance, last_observed_execution_generation, last_observed_route_activation_id, last_observed_route_substrate_kind, last_observed_route_identity_provenance FROM sandboxes WHERE id = ?1", - [sandbox_id.to_string()], - |row| { - let current_provenance = row - .get::<_, Option>(0)? - .map(|value| parse_text::(&value, 0)) - .transpose()?; - let generation = row.get::<_, Option>(1)?; - let attempt = row.get::<_, Option>(2)?; - let substrate = row.get::<_, Option>(3)?; - let provenance = row.get::<_, Option>(4)?; - let retained_evidence = match (generation, attempt, substrate, provenance) { - (Some(generation), Some(attempt), Some(substrate), Some(provenance)) => { - Some(RouteEvidence { - execution_generation: ExecutionGeneration::new(generation).map_err( - |error| { - rusqlite::Error::FromSqlConversionFailure( - 1, - Type::Integer, - Box::new(error), - ) - }, - )?, - activation_attempt_id: parse_text(&attempt, 2)?, - substrate: parse_text(&substrate, 3)?, - provenance: parse_text(&provenance, 4)?, - }) - } - _ => None, - }; - Ok(RouteIdentityAdoptionMetadata { - current_provenance, - retained_evidence, - }) - }, - ) + [sandbox_id.to_string()], + |row| { + let current_provenance = row + .get::<_, Option>(0)? + .map(|value| parse_text::(&value, 0)) + .transpose()?; + let generation = row.get::<_, Option>(1)?; + let attempt = row.get::<_, Option>(2)?; + let substrate = row.get::<_, Option>(3)?; + let provenance = row.get::<_, Option>(4)?; + let retained_evidence = match (generation, attempt, substrate, provenance) { + (Some(generation), Some(attempt), Some(substrate), Some(provenance)) => { + Some(RouteEvidence { + execution_generation: ExecutionGeneration::new(generation).map_err( + |error| { + rusqlite::Error::FromSqlConversionFailure( + 1, + Type::Integer, + Box::new(error), + ) + }, + )?, + activation_attempt_id: parse_text(&attempt, 2)?, + substrate: parse_text(&substrate, 3)?, + provenance: parse_text(&provenance, 4)?, + }) + } + _ => None, + }; + Ok(RouteIdentityAdoptionMetadata { + current_provenance, + retained_evidence, + }) + }, + ) + } } fn clear_route_evidence_block(conn: &Connection, sandbox_id: SandboxId) -> crate::db::Result<()> { @@ -711,19 +714,20 @@ pub(crate) fn discard_pending_receipt_for_execution_advance( ) } -/// Returns whether any pending delete receipt still describes every observed -/// execution incarnation for this row. -/// -/// This protects recovery of receipts admitted by an older mixed-version -/// writer: local cleanup must not dispatch after either the current route or -/// its retained high-water mark has advanced to a different generation or -/// same-generation activation attempt. -pub fn pending_receipt_matches_observed_execution( - conn: &Connection, - sandbox_id: SandboxId, -) -> crate::db::Result { - conn.query_row( - "SELECT NOT EXISTS ( +db_private_outside_tests! { + /// Returns whether any pending delete receipt still describes every observed + /// execution incarnation for this row. + /// + /// This protects recovery of receipts admitted by an older mixed-version + /// writer: local cleanup must not dispatch after either the current route or + /// its retained high-water mark has advanced to a different generation or + /// same-generation activation attempt. + fn pending_receipt_matches_observed_execution( + conn: &Connection, + sandbox_id: SandboxId, + ) -> crate::db::Result { + conn.query_row( + "SELECT NOT EXISTS ( SELECT 1 FROM auto_lifecycle_delete_outbox AS receipt JOIN sandboxes AS sandbox ON sandbox.id = receipt.sandbox_id @@ -756,9 +760,10 @@ pub fn pending_receipt_matches_observed_execution( ) ) )", - [sandbox_id.to_string()], - |row| row.get(0), - ) + [sandbox_id.to_string()], + |row| row.get(0), + ) + } } fn record_block( @@ -1091,87 +1096,93 @@ pub fn acknowledge_completed_receipts( Ok(acknowledged) } -pub fn outbox_stats(conn: &Connection) -> crate::db::Result { - let now = crate::db::now_epoch_i64()?; - let (pending, completed, attempts, oldest_pending, oldest_completed) = conn.query_row( - "SELECT +db_private_outside_tests! { + fn outbox_stats(conn: &Connection) -> crate::db::Result { + let now = crate::db::now_epoch_i64()?; + let (pending, completed, attempts, oldest_pending, oldest_completed) = conn.query_row( + "SELECT COALESCE(SUM(state = 'pending'), 0), COALESCE(SUM(state = 'completed'), 0), COALESCE(MAX(CASE WHEN state = 'completed' THEN attempt_count END), 0), MIN(CASE WHEN state = 'pending' THEN CAST(created_at AS INTEGER) END), MIN(CASE WHEN state = 'completed' THEN CAST(completed_at AS INTEGER) END) FROM auto_lifecycle_delete_outbox", - [], - |row| { - Ok(( - row.get::<_, i64>(0)?, - row.get::<_, i64>(1)?, - row.get::<_, i64>(2)?, - row.get::<_, Option>(3)?, - row.get::<_, Option>(4)?, - )) - }, - )?; - let value = |value: i64, column| { - u64::try_from(value).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(column, Type::Integer, Box::new(error)) + [], + |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, i64>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, Option>(3)?, + row.get::<_, Option>(4)?, + )) + }, + )?; + let value = |value: i64, column| { + u64::try_from(value).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(column, Type::Integer, Box::new(error)) + }) + }; + let age = |timestamp: Option, column| { + timestamp + .map(|timestamp| value(now.saturating_sub(timestamp), column)) + .transpose() + .map(Option::unwrap_or_default) + }; + Ok(AutoLifecycleDeleteOutboxStats { + pending_count: value(pending, 0)?, + completed_count: value(completed, 1)?, + max_attempt_count: value(attempts, 2)?, + oldest_pending_age_secs: age(oldest_pending, 3)?, + oldest_completed_age_secs: age(oldest_completed, 4)?, }) - }; - let age = |timestamp: Option, column| { - timestamp - .map(|timestamp| value(now.saturating_sub(timestamp), column)) - .transpose() - .map(Option::unwrap_or_default) - }; - Ok(AutoLifecycleDeleteOutboxStats { - pending_count: value(pending, 0)?, - completed_count: value(completed, 1)?, - max_attempt_count: value(attempts, 2)?, - oldest_pending_age_secs: age(oldest_pending, 3)?, - oldest_completed_age_secs: age(oldest_completed, 4)?, - }) + } } -pub fn blocked_stats(conn: &Connection) -> crate::db::Result> { - let now = crate::db::now_epoch_i64()?; - let mut statement = conn.prepare( - "SELECT auto_lifecycle_delete_block_reason, +db_private_outside_tests! { + fn blocked_stats(conn: &Connection) -> crate::db::Result> { + let now = crate::db::now_epoch_i64()?; + let mut statement = conn.prepare( + "SELECT auto_lifecycle_delete_block_reason, COUNT(*), MIN(CAST(auto_lifecycle_delete_blocked_at AS INTEGER)) FROM sandboxes WHERE auto_lifecycle_delete_block_reason IS NOT NULL GROUP BY auto_lifecycle_delete_block_reason", - )?; - statement - .query_map([], |row| { - let reason = - parse_text::(&row.get::<_, String>(0)?, 0)?; - let count = u64::try_from(row.get::<_, i64>(1)?).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(1, Type::Integer, Box::new(error)) - })?; - let oldest = row.get::<_, Option>(2)?.unwrap_or(now); - let oldest_age_secs = u64::try_from(now.saturating_sub(oldest)).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(2, Type::Integer, Box::new(error)) - })?; - Ok(AutoLifecycleDeleteBlockedStats { - reason, - count, - oldest_age_secs, - }) - })? - .collect::>() - .map_err(crate::db::Error::from) + )?; + statement + .query_map([], |row| { + let reason = + parse_text::(&row.get::<_, String>(0)?, 0)?; + let count = u64::try_from(row.get::<_, i64>(1)?).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(1, Type::Integer, Box::new(error)) + })?; + let oldest = row.get::<_, Option>(2)?.unwrap_or(now); + let oldest_age_secs = u64::try_from(now.saturating_sub(oldest)).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(2, Type::Integer, Box::new(error)) + })?; + Ok(AutoLifecycleDeleteBlockedStats { + reason, + count, + oldest_age_secs, + }) + })? + .collect::>() + .map_err(crate::db::Error::from) + } } -pub fn pending_receipt_for_sandbox( - conn: &Connection, - sandbox_id: SandboxId, -) -> crate::db::Result> { - Ok( - receipt_record_for_sandbox(conn, sandbox_id)?.and_then(|(receipt, state)| { - (state == AutoLifecycleDeleteOutboxState::Pending).then_some(receipt) - }), - ) +db_private_outside_tests! { + fn pending_receipt_for_sandbox( + conn: &Connection, + sandbox_id: SandboxId, + ) -> crate::db::Result> { + Ok( + receipt_record_for_sandbox(conn, sandbox_id)?.and_then(|(receipt, state)| { + (state == AutoLifecycleDeleteOutboxState::Pending).then_some(receipt) + }), + ) + } } fn receipt_record_for_sandbox( diff --git a/crates/libs/sbx/mgr/store/src/db/cleanup_episodes.rs b/crates/libs/sbx/mgr/store/src/db/cleanup_episodes.rs index edcb5a017..7f749c3bb 100644 --- a/crates/libs/sbx/mgr/store/src/db/cleanup_episodes.rs +++ b/crates/libs/sbx/mgr/store/src/db/cleanup_episodes.rs @@ -246,19 +246,22 @@ pub fn bind_prepared_clone_root( )? == 1) } -/// Presence is authoritative even when a newer binary wrote an intent this -/// reader cannot decode. Startup must not promote or recreate its resources. -pub fn has_cleanup_episode(conn: &Connection, id: &str) -> Result { - conn.query_row( - "SELECT EXISTS(SELECT 1 FROM sandbox_cleanup_episodes WHERE sandbox_id = ?1)", - [id], - |row| row.get(0), - ) +db_private_outside_tests! { + /// Presence is authoritative even when a newer binary wrote an intent this + /// reader cannot decode. Startup must not promote or recreate its resources. + fn has_cleanup_episode(conn: &Connection, id: &str) -> Result { + conn.query_row( + "SELECT EXISTS(SELECT 1 FROM sandbox_cleanup_episodes WHERE sandbox_id = ?1)", + [id], + |row| row.get(0), + ) + } } -pub fn cleanup_episode(conn: &Connection, id: &str) -> Result> { - conn.query_row( - "SELECT cleanup_id, completion, keep_filesystem, entered_from_state, +db_private_outside_tests! { + fn cleanup_episode(conn: &Connection, id: &str) -> Result> { + conn.query_row( + "SELECT cleanup_id, completion, keep_filesystem, entered_from_state, activation_state, requested_at, quarantine_reason_code, failure_message, last_cleanup_error, attempt_count, recovery_local_artifact_id, recovery_execution_generation, @@ -266,139 +269,140 @@ pub fn cleanup_episode(conn: &Connection, id: &str) -> Result(0)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) - })?; - let completion = row.get::<_, String>(1)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(1, Type::Text, Box::new(error)) - })?; - let entered_from_state = row - .get::<_, Option>(3)? - .map(|state| { - state.parse::().map_err(|_| { - rusqlite::Error::FromSqlConversionFailure( - 3, - Type::Text, - Box::new(std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!("unknown cleanup entered-from state '{state}'"), - )), - ) - }) - }) - .transpose()?; - let activation_state_raw = row.get::<_, String>(4)?; - let activation_state = activation_state_raw.parse::().map_err(|_| { - rusqlite::Error::FromSqlConversionFailure( - 4, - Type::Text, - Box::new(std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!("unknown cleanup activation state '{activation_state_raw}'"), - )), - ) - })?; - let quarantine_reason_code = row - .get::<_, Option>(6)? - .map(|reason| { - reason.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(6, Type::Text, Box::new(error)) - }) - }) - .transpose()?; - let attempt_count = row.get::<_, i64>(9)?; - let quarantine_recovery = match ( - row.get::<_, Option>(10)?, - row.get::<_, Option>(11)?, - ) { - (None, None) => None, - (Some(local_artifact_id), Some(execution_generation)) => { - Some(QuarantineRecoveryFence { - local_artifact_id: local_artifact_id.parse().map_err(|error| { + params![id], + |row| { + let cleanup_id = row.get::<_, String>(0)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) + })?; + let completion = row.get::<_, String>(1)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(1, Type::Text, Box::new(error)) + })?; + let entered_from_state = row + .get::<_, Option>(3)? + .map(|state| { + state.parse::().map_err(|_| { rusqlite::Error::FromSqlConversionFailure( - 10, + 3, Type::Text, - Box::new(error), + Box::new(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("unknown cleanup entered-from state '{state}'"), + )), ) - })?, - execution_generation: ExecutionGeneration::new(execution_generation) - .map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 11, - Type::Integer, - Box::new(error), - ) - })?, + }) }) - } - _ => { - return Err(rusqlite::Error::FromSqlConversionFailure( - 10, - Type::Null, + .transpose()?; + let activation_state_raw = row.get::<_, String>(4)?; + let activation_state = activation_state_raw.parse::().map_err(|_| { + rusqlite::Error::FromSqlConversionFailure( + 4, + Type::Text, Box::new(std::io::Error::new( std::io::ErrorKind::InvalidData, - "quarantine recovery fence is incomplete", + format!("unknown cleanup activation state '{activation_state_raw}'"), )), - )); - } - }; - Ok(CleanupEpisode { - operation_activation_attempt_id: row - .get::<_, Option>(15)? - .map(|value| value.parse()) - .transpose() - .map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(15, Type::Text, Box::new(error)) - })?, - prepared_clone_root: row - .get::<_, Option>(14)? - .map(|json| { - serde_json::from_str(&json).map_err(|error| { + ) + })?; + let quarantine_reason_code = row + .get::<_, Option>(6)? + .map(|reason| { + reason.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(6, Type::Text, Box::new(error)) + }) + }) + .transpose()?; + let attempt_count = row.get::<_, i64>(9)?; + let quarantine_recovery = match ( + row.get::<_, Option>(10)?, + row.get::<_, Option>(11)?, + ) { + (None, None) => None, + (Some(local_artifact_id), Some(execution_generation)) => { + Some(QuarantineRecoveryFence { + local_artifact_id: local_artifact_id.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 10, + Type::Text, + Box::new(error), + ) + })?, + execution_generation: ExecutionGeneration::new(execution_generation) + .map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 11, + Type::Integer, + Box::new(error), + ) + })?, + }) + } + _ => { + return Err(rusqlite::Error::FromSqlConversionFailure( + 10, + Type::Null, + Box::new(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "quarantine recovery fence is incomplete", + )), + )); + } + }; + Ok(CleanupEpisode { + operation_activation_attempt_id: row + .get::<_, Option>(15)? + .map(|value| value.parse()) + .transpose() + .map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(15, Type::Text, Box::new(error)) + })?, + prepared_clone_root: row + .get::<_, Option>(14)? + .map(|json| { + serde_json::from_str(&json).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 14, + Type::Text, + Box::new(error), + ) + }) + }) + .transpose()?, + id: cleanup_id, + completion, + keep_filesystem: row.get(2)?, + entered_from_state, + activation_state, + requested_at: row + .get::<_, Option>(5)? + .and_then(|value| u64::try_from(value).ok()), + quarantine_reason_code, + failure_message: row.get(7)?, + last_cleanup_error: row.get(8)?, + execution_generation: row + .get::<_, Option>(12)? + .map(ExecutionGeneration::new) + .transpose() + .map_err(|error| { rusqlite::Error::FromSqlConversionFailure( - 14, - Type::Text, + 12, + Type::Integer, Box::new(error), ) - }) - }) - .transpose()?, - id: cleanup_id, - completion, - keep_filesystem: row.get(2)?, - entered_from_state, - activation_state, - requested_at: row - .get::<_, Option>(5)? - .and_then(|value| u64::try_from(value).ok()), - quarantine_reason_code, - failure_message: row.get(7)?, - last_cleanup_error: row.get(8)?, - execution_generation: row - .get::<_, Option>(12)? - .map(ExecutionGeneration::new) - .transpose() - .map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 12, - Type::Integer, - Box::new(error), - ) - })?, - activation_attempt_id: row - .get::<_, Option>(13)? - .map(|value| value.parse()) - .transpose() - .map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(13, Type::Text, Box::new(error)) - })?, - attempt_count: u32::try_from(attempt_count).unwrap_or(u32::MAX), - quarantine_recovery, - }) - }, - ) - .optional() + })?, + activation_attempt_id: row + .get::<_, Option>(13)? + .map(|value| value.parse()) + .transpose() + .map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(13, Type::Text, Box::new(error)) + })?, + attempt_count: u32::try_from(attempt_count).unwrap_or(u32::MAX), + quarantine_recovery, + }) + }, + ) + .optional() + } } /// Retire an armed episode only after another state writer has conclusively @@ -594,13 +598,14 @@ pub fn record_cleanup_episode_failure( Ok(changed == 1) } -pub fn cleanup_episode_current( - conn: &Connection, - sandbox_id: &str, - cleanup_id: SandboxCleanupEpisodeId, -) -> Result { - conn.query_row( - "SELECT EXISTS( +db_private_outside_tests! { + fn cleanup_episode_current( + conn: &Connection, + sandbox_id: &str, + cleanup_id: SandboxCleanupEpisodeId, + ) -> Result { + conn.query_row( + "SELECT EXISTS( SELECT 1 FROM sandbox_cleanup_episodes episode JOIN sandboxes sandbox ON sandbox.id = episode.sandbox_id @@ -611,9 +616,10 @@ pub fn cleanup_episode_current( AND sandbox.cleanup_completion = episode.completion AND sandbox.cleanup_keep_filesystem = episode.keep_filesystem )", - params![sandbox_id, cleanup_id.to_string()], - |row| row.get(0), - ) + params![sandbox_id, cleanup_id.to_string()], + |row| row.get(0), + ) + } } #[derive(Clone, Debug)] @@ -1082,30 +1088,32 @@ pub enum SandboxEraseSafety { RetainedFilesystemOnlyCopy, } -/// Classify whether erasing this record would drop unpublished suspend state. -/// -/// The live and retry-owned states still have a suspend completion that can -/// publish. A terminal missing-artifact failure cannot publish, but its -/// retained rootfs may be the only copy of the sandbox's latest work. The -/// failure reason is the durable witness for that distinct safety condition. -pub fn sandbox_erase_safety( - state: SandboxState, - cleanup_completion: Option, - failure_reason: Option<&str>, -) -> SandboxEraseSafety { - if state == SandboxState::Suspending - || (matches!( - state, - SandboxState::CleanupRequired | SandboxState::CleanupFailed - ) && cleanup_completion == Some(CleanupCompletion::MarkSuspended)) - { - SandboxEraseSafety::SuspendPublicationPending - } else if matches!(state, SandboxState::Failed | SandboxState::Quarantined) - && failure_reason == Some(SUSPEND_PUBLISH_FAILED_NO_ARTIFACT_RUNTIME_GONE) - { - SandboxEraseSafety::RetainedFilesystemOnlyCopy - } else { - SandboxEraseSafety::Safe +db_private_outside_tests! { + /// Classify whether erasing this record would drop unpublished suspend state. + /// + /// The live and retry-owned states still have a suspend completion that can + /// publish. A terminal missing-artifact failure cannot publish, but its + /// retained rootfs may be the only copy of the sandbox's latest work. The + /// failure reason is the durable witness for that distinct safety condition. + fn sandbox_erase_safety( + state: SandboxState, + cleanup_completion: Option, + failure_reason: Option<&str>, + ) -> SandboxEraseSafety { + if state == SandboxState::Suspending + || (matches!( + state, + SandboxState::CleanupRequired | SandboxState::CleanupFailed + ) && cleanup_completion == Some(CleanupCompletion::MarkSuspended)) + { + SandboxEraseSafety::SuspendPublicationPending + } else if matches!(state, SandboxState::Failed | SandboxState::Quarantined) + && failure_reason == Some(SUSPEND_PUBLISH_FAILED_NO_ARTIFACT_RUNTIME_GONE) + { + SandboxEraseSafety::RetainedFilesystemOnlyCopy + } else { + SandboxEraseSafety::Safe + } } } @@ -1751,64 +1759,70 @@ pub struct CleanupTarget { pub user: SandboxOwner, } -pub fn cleanup_target(conn: &Connection, id: &str) -> Result> { - conn.query_row( - sql!("SELECT uuid8, template, user FROM sandboxes WHERE id = ?1"), - [id], - |row| { - Ok(CleanupTarget { - uuid8: SandboxRuntimeLabel::parse(row.get::<_, String>(0)?).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) - })?, - template: row.get(1)?, - user: SandboxOwner::parse(row.get::<_, String>(2)?).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(2, Type::Text, Box::new(error)) - })?, - }) - }, - ) - .optional() -} - -pub fn cleanup_runtime_label(conn: &Connection, id: &str) -> Result> { - conn.query_row( - sql!("SELECT uuid8 FROM sandboxes WHERE id = ?1"), - [id], - |row| { - SandboxRuntimeLabel::parse(row.get::<_, String>(0)?).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) - }) - }, - ) - .optional() +db_private_outside_tests! { + fn cleanup_target(conn: &Connection, id: &str) -> Result> { + conn.query_row( + sql!("SELECT uuid8, template, user FROM sandboxes WHERE id = ?1"), + [id], + |row| { + Ok(CleanupTarget { + uuid8: SandboxRuntimeLabel::parse(row.get::<_, String>(0)?).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) + })?, + template: row.get(1)?, + user: SandboxOwner::parse(row.get::<_, String>(2)?).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(2, Type::Text, Box::new(error)) + })?, + }) + }, + ) + .optional() + } } -pub fn cleanup_intent_current( - conn: &Connection, - id: &str, - completion: CleanupCompletion, -) -> Result { - let intent = conn - .query_row( - sql!("SELECT state, cleanup_completion FROM sandboxes WHERE id = ?1"), +db_private_outside_tests! { + fn cleanup_runtime_label(conn: &Connection, id: &str) -> Result> { + conn.query_row( + sql!("SELECT uuid8 FROM sandboxes WHERE id = ?1"), [id], - |row| Ok((row.get::<_, String>(0)?, row.get::<_, Option>(1)?)), + |row| { + SandboxRuntimeLabel::parse(row.get::<_, String>(0)?).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(0, Type::Text, Box::new(error)) + }) + }, ) - .optional()?; - let Some((state, intent)) = intent else { - return Ok(false); - }; - let Ok(state) = state.parse::() else { - return Ok(false); - }; - let intent = intent - .map(|value| { - value.parse::().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(1, Type::Text, Box::new(error)) + .optional() + } +} + +db_private_outside_tests! { + fn cleanup_intent_current( + conn: &Connection, + id: &str, + completion: CleanupCompletion, + ) -> Result { + let intent = conn + .query_row( + sql!("SELECT state, cleanup_completion FROM sandboxes WHERE id = ?1"), + [id], + |row| Ok((row.get::<_, String>(0)?, row.get::<_, Option>(1)?)), + ) + .optional()?; + let Some((state, intent)) = intent else { + return Ok(false); + }; + let Ok(state) = state.parse::() else { + return Ok(false); + }; + let intent = intent + .map(|value| { + value.parse::().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(1, Type::Text, Box::new(error)) + }) }) - }) - .transpose()?; - Ok(completion.owns_intent(state, intent)) + .transpose()?; + Ok(completion.owns_intent(state, intent)) + } } /// Sandboxes with durable cleanup evidence that a live reconciliation pass may @@ -1876,35 +1890,36 @@ pub struct CleanupRecoveryPage { pub next_cursor: Option, } -pub fn cleanup_recovery_page( - conn: &Connection, - after: Option, - limit: usize, -) -> Result { - let sql_limit = i64::try_from(limit) - .map_err(|error| rusqlite::Error::ToSqlConversionFailure(Box::new(error)))?; - // The rowid is a scan position, never lifecycle authority. - let Some(first) = after.map_or(Some(i64::MIN), |rowid| rowid.checked_add(1)) else { - return Ok(CleanupRecoveryPage { - ids: Vec::new(), - next_cursor: None, - }); - }; - let tx = Transaction::deferred(conn)?; - let mut inventory = tx.prepare(sql!( - "SELECT rowid FROM sandboxes WHERE rowid >= ?1 ORDER BY rowid LIMIT ?2" - ))?; - let range: Vec = inventory - .query_map(params![first, sql_limit], |row| row.get(0))? - .collect::>()?; - let Some(&last) = range.last() else { - return Ok(CleanupRecoveryPage { - ids: Vec::new(), - next_cursor: None, - }); - }; - let mut statement = tx.prepare( - sql!("SELECT sandbox.id +db_private_outside_tests! { + fn cleanup_recovery_page( + conn: &Connection, + after: Option, + limit: usize, + ) -> Result { + let sql_limit = i64::try_from(limit) + .map_err(|error| rusqlite::Error::ToSqlConversionFailure(Box::new(error)))?; + // The rowid is a scan position, never lifecycle authority. + let Some(first) = after.map_or(Some(i64::MIN), |rowid| rowid.checked_add(1)) else { + return Ok(CleanupRecoveryPage { + ids: Vec::new(), + next_cursor: None, + }); + }; + let tx = Transaction::deferred(conn)?; + let mut inventory = tx.prepare(sql!( + "SELECT rowid FROM sandboxes WHERE rowid >= ?1 ORDER BY rowid LIMIT ?2" + ))?; + let range: Vec = inventory + .query_map(params![first, sql_limit], |row| row.get(0))? + .collect::>()?; + let Some(&last) = range.last() else { + return Ok(CleanupRecoveryPage { + ids: Vec::new(), + next_cursor: None, + }); + }; + let mut statement = tx.prepare( + sql!("SELECT sandbox.id FROM sandboxes sandbox LEFT JOIN sandbox_cleanup_episodes episode ON episode.sandbox_id = sandbox.id @@ -1939,13 +1954,14 @@ pub fn cleanup_recovery_page( ) ) ) ORDER BY sandbox.rowid"), - )?; - let ids = statement - .query_map(params![first, last], |row| row.get::<_, String>(0))? - .collect::>()?; - let next_cursor = (range.len() == limit).then_some(last); - drop(statement); - drop(inventory); - tx.commit()?; - Ok(CleanupRecoveryPage { ids, next_cursor }) + )?; + let ids = statement + .query_map(params![first, last], |row| row.get::<_, String>(0))? + .collect::>()?; + let next_cursor = (range.len() == limit).then_some(last); + drop(statement); + drop(inventory); + tx.commit()?; + Ok(CleanupRecoveryPage { ids, next_cursor }) + } } diff --git a/crates/libs/sbx/mgr/store/src/db/cleanup_jobs.rs b/crates/libs/sbx/mgr/store/src/db/cleanup_jobs.rs index c6abe2837..0f904e95b 100644 --- a/crates/libs/sbx/mgr/store/src/db/cleanup_jobs.rs +++ b/crates/libs/sbx/mgr/store/src/db/cleanup_jobs.rs @@ -306,8 +306,10 @@ pub fn reset_inactive_running(conn: &Connection, active_subjects: &[String]) -> )?) } -pub fn get_job(conn: &Connection, job_id: &JobId) -> Result> { - Ok(jobs::get_job(conn.raw(), job_id)?) +db_private_outside_tests! { + fn get_job(conn: &Connection, job_id: &JobId) -> Result> { + Ok(jobs::get_job(conn.raw(), job_id)?) + } } /// Claim a due job; `false` if another claimant won it. diff --git a/crates/libs/sbx/mgr/store/src/db/cleanup_target_scan.rs b/crates/libs/sbx/mgr/store/src/db/cleanup_target_scan.rs index 5e86cd5f8..57531e98c 100644 --- a/crates/libs/sbx/mgr/store/src/db/cleanup_target_scan.rs +++ b/crates/libs/sbx/mgr/store/src/db/cleanup_target_scan.rs @@ -2,6 +2,7 @@ use super::inventory_read::{InventoryReadError, check_row}; use crate::db::Connection; +use crate::db::db_private_outside_tests; use pplx_space_common_core::inventory::{InventoryBudget, MAX_ENCODED_BYTES}; use pplx_space_common_core::types::{SandboxId, SandboxOwner, SandboxRuntimeLabel}; use rusqlite::types::Type; @@ -27,73 +28,75 @@ impl CleanupTargetScan { self.complete } - pub fn next_batch( - &mut self, - conn: &Connection, - ) -> Result, InventoryReadError> { - if self.complete { - return Ok(Vec::new()); - } - if self.next.is_none() { - self.through = - conn.query_row("SELECT MAX(rowid) FROM sandboxes", [], |row| row.get(0))?; - self.next = Some(i64::MIN); - } - let Some(through) = self.through else { - self.complete = true; - return Ok(Vec::new()); - }; - let mut statement = conn.prepare( - "SELECT rowid, id, uuid8, template, user FROM sandboxes + db_private_outside_tests! { + fn next_batch( + &mut self, + conn: &Connection, + ) -> Result, InventoryReadError> { + if self.complete { + return Ok(Vec::new()); + } + if self.next.is_none() { + self.through = + conn.query_row("SELECT MAX(rowid) FROM sandboxes", [], |row| row.get(0))?; + self.next = Some(i64::MIN); + } + let Some(through) = self.through else { + self.complete = true; + return Ok(Vec::new()); + }; + let mut statement = conn.prepare( + "SELECT rowid, id, uuid8, template, user FROM sandboxes WHERE rowid >= ?1 AND rowid <= ?2 ORDER BY rowid LIMIT 128", - )?; - let mut rows = statement.query([self.next.unwrap_or(i64::MIN), through])?; - let mut batch = Vec::new(); - let mut budget = InventoryBudget::new(BATCH_SIZE, MAX_ENCODED_BYTES); - while let Some(row) = rows.next()? { - check_row(row, &mut budget)?; - let rowid: i64 = row.get(0)?; - let raw_id: String = row.get(1)?; - let metadata = (|| { - Ok(CleanupTarget { - uuid8: SandboxRuntimeLabel::parse(row.get::<_, String>(2)?).map_err( - |error| { - rusqlite::Error::FromSqlConversionFailure( - 2, - Type::Text, - Box::new(error), - ) - }, - )?, - template: row.get(3)?, - user: SandboxOwner::parse(row.get::<_, String>(4)?).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure(4, Type::Text, Box::new(error)) - })?, - }) - })(); - let metadata = match metadata { - Ok(metadata) - if raw_id - .parse::() - .is_ok_and(|id| id.to_string() == raw_id) => - { - Some(metadata) + )?; + let mut rows = statement.query([self.next.unwrap_or(i64::MIN), through])?; + let mut batch = Vec::new(); + let mut budget = InventoryBudget::new(BATCH_SIZE, MAX_ENCODED_BYTES); + while let Some(row) = rows.next()? { + check_row(row, &mut budget)?; + let rowid: i64 = row.get(0)?; + let raw_id: String = row.get(1)?; + let metadata = (|| { + Ok(CleanupTarget { + uuid8: SandboxRuntimeLabel::parse(row.get::<_, String>(2)?).map_err( + |error| { + rusqlite::Error::FromSqlConversionFailure( + 2, + Type::Text, + Box::new(error), + ) + }, + )?, + template: row.get(3)?, + user: SandboxOwner::parse(row.get::<_, String>(4)?).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure(4, Type::Text, Box::new(error)) + })?, + }) + })(); + let metadata = match metadata { + Ok(metadata) + if raw_id + .parse::() + .is_ok_and(|id| id.to_string() == raw_id) => + { + Some(metadata) + } + Ok(_) => None, + Err(error) if super::is_row_decode_error(&error) => None, + Err(error) => return Err(error.into()), + }; + batch.push(CleanupTargetObservation { raw_id, metadata }); + if rowid == through { + self.complete = true; + } else { + self.next = Some(rowid + 1); } - Ok(_) => None, - Err(error) if super::is_row_decode_error(&error) => None, - Err(error) => return Err(error.into()), - }; - batch.push(CleanupTargetObservation { raw_id, metadata }); - if rowid == through { + } + if batch.len() < BATCH_SIZE { self.complete = true; - } else { - self.next = Some(rowid + 1); } + Ok(batch) } - if batch.len() < BATCH_SIZE { - self.complete = true; - } - Ok(batch) } } diff --git a/crates/libs/sbx/mgr/store/src/db/eviction.rs b/crates/libs/sbx/mgr/store/src/db/eviction.rs index 3b845c9db..9e195710e 100644 --- a/crates/libs/sbx/mgr/store/src/db/eviction.rs +++ b/crates/libs/sbx/mgr/store/src/db/eviction.rs @@ -325,43 +325,47 @@ fn cancel_live_upload_job(conn: &Connection, artifact: LocalArtifactId) -> Resul Ok(()) } -/// The local artifacts of the sandbox other than `staged_artifact` with no -/// `backup_completed_at` stamp: the rows that hold -/// [`claim_retired_teardown_execution_for_release`] back until each is -/// stamped. -pub fn other_local_artifacts_awaiting_backup( - conn: &Connection, - id: &str, - staged_artifact: LocalArtifactId, -) -> Result> { - let mut statement = conn.prepare( - "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, +db_private_outside_tests! { + /// The local artifacts of the sandbox other than `staged_artifact` with no + /// `backup_completed_at` stamp: the rows that hold + /// [`claim_retired_teardown_execution_for_release`] back until each is + /// stamped. + fn other_local_artifacts_awaiting_backup( + conn: &Connection, + id: &str, + staged_artifact: LocalArtifactId, + ) -> Result> { + let mut statement = conn.prepare( + "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, snapshot_name, local_path FROM local_artifacts WHERE sandbox_id = ?1 AND local_artifact_id <> ?2 AND backup_completed_at IS NULL ORDER BY local_artifact_id", - )?; - Ok(statement - .query_map( - params![id, staged_artifact.to_string()], - row_to_local_artifact, - )? - .collect::>()?) + )?; + Ok(statement + .query_map( + params![id, staged_artifact.to_string()], + row_to_local_artifact, + )? + .collect::>()?) + } } -/// Whether this generation's teardown-suspend job is still running: the other -/// half of what [`claim_retired_teardown_execution_for_release`] requires. -pub fn teardown_suspend_operation_running_for( - conn: &Connection, - id: &str, - execution_generation: ExecutionGeneration, -) -> Result { - Ok( - active_teardown_suspend_operation(conn, id)?.is_some_and(|operation| { - operation.execution_generation == execution_generation - && operation.status == TeardownSuspendOperationStatus::Running - }), - ) +db_private_outside_tests! { + /// Whether this generation's teardown-suspend job is still running: the other + /// half of what [`claim_retired_teardown_execution_for_release`] requires. + fn teardown_suspend_operation_running_for( + conn: &Connection, + id: &str, + execution_generation: ExecutionGeneration, + ) -> Result { + Ok( + active_teardown_suspend_operation(conn, id)?.is_some_and(|operation| { + operation.execution_generation == execution_generation + && operation.status == TeardownSuspendOperationStatus::Running + }), + ) + } } diff --git a/crates/libs/sbx/mgr/store/src/db/eviction_scan.rs b/crates/libs/sbx/mgr/store/src/db/eviction_scan.rs index 7d0dc4d75..a2571d57d 100644 --- a/crates/libs/sbx/mgr/store/src/db/eviction_scan.rs +++ b/crates/libs/sbx/mgr/store/src/db/eviction_scan.rs @@ -1,6 +1,7 @@ //! Suspended-sandbox reads behind `crate::sandbox_eviction`'s candidate //! scans and local eviction evidence. +use crate::db::db_private_outside_tests; use crate::db::{Connection, OptionalExtension, Result}; use pplx_space_sqlite::SqliteU64; use rusqlite::params; @@ -60,18 +61,19 @@ const IDLE_WITNESS_SQL: &str = "COALESCE( CAST(s.updated_at AS INTEGER) )"; -/// `(sandbox_id, idle witness)` of cold suspended sandboxes after the -/// `(cursor_witness, cursor_sandbox_id)` keyset position, in witness then id -/// order. -pub fn idle_suspended_candidates( - conn: &Connection, - cutoff: i64, - cursor_witness: Option, - cursor_sandbox_id: Option<&str>, - limit: usize, -) -> Result> { - let sql = format!( - "SELECT s.id, {IDLE_WITNESS_SQL} FROM sandboxes s +db_private_outside_tests! { + /// `(sandbox_id, idle witness)` of cold suspended sandboxes after the + /// `(cursor_witness, cursor_sandbox_id)` keyset position, in witness then id + /// order. + fn idle_suspended_candidates( + conn: &Connection, + cutoff: i64, + cursor_witness: Option, + cursor_sandbox_id: Option<&str>, + limit: usize, + ) -> Result> { + let sql = format!( + "SELECT s.id, {IDLE_WITNESS_SQL} FROM sandboxes s WHERE s.state = 'suspended' AND {VALID_SUSPENDED_UPDATED_AT_SQL} AND {IDLE_WITNESS_SQL} <= ?1 @@ -97,30 +99,32 @@ pub fn idle_suspended_candidates( ) ORDER BY {IDLE_WITNESS_SQL} ASC, s.id ASC LIMIT ?4" - ); - let mut stmt = conn.prepare(&sql)?; - let rows = stmt - .query_map( - params![cutoff, cursor_witness, cursor_sandbox_id, limit as i64], - |row| { - let sandbox_id: String = row.get(0)?; - let idle_witness_secs = row.get::<_, SqliteU64>(1)?.0; - Ok((sandbox_id, idle_witness_secs)) - }, - )? - .collect::>()?; - Ok(rows) + ); + let mut stmt = conn.prepare(&sql)?; + let rows = stmt + .query_map( + params![cutoff, cursor_witness, cursor_sandbox_id, limit as i64], + |row| { + let sandbox_id: String = row.get(0)?; + let idle_witness_secs = row.get::<_, SqliteU64>(1)?.0; + Ok((sandbox_id, idle_witness_secs)) + }, + )? + .collect::>()?; + Ok(rows) + } } -/// Ids of cold suspended sandboxes with neither a pinned artifact nor an -/// upload job carrying a snapshot id, oldest witness first. -pub fn legacy_reupload_candidates( - conn: &Connection, - idle_cutoff_epoch_secs: u64, - limit: usize, -) -> Result> { - let sql = format!( - "SELECT s.id FROM sandboxes s +db_private_outside_tests! { + /// Ids of cold suspended sandboxes with neither a pinned artifact nor an + /// upload job carrying a snapshot id, oldest witness first. + fn legacy_reupload_candidates( + conn: &Connection, + idle_cutoff_epoch_secs: u64, + limit: usize, + ) -> Result> { + let sql = format!( + "SELECT s.id FROM sandboxes s WHERE s.state = 'suspended' AND {VALID_SUSPENDED_UPDATED_AT_SQL} AND {IDLE_WITNESS_SQL} <= ?1 @@ -139,81 +143,92 @@ pub fn legacy_reupload_candidates( ) ORDER BY {IDLE_WITNESS_SQL} ASC LIMIT ?2" - ); - let mut stmt = conn.prepare(&sql)?; - let rows = stmt - .query_map( - params![SqliteU64(idle_cutoff_epoch_secs), limit as i64], - |r| r.get(0), - )? - .collect::>()?; - Ok(rows) + ); + let mut stmt = conn.prepare(&sql)?; + let rows = stmt + .query_map( + params![SqliteU64(idle_cutoff_epoch_secs), limit as i64], + |r| r.get(0), + )? + .collect::>()?; + Ok(rows) + } } -pub fn count_malformed_suspended_updated_at(conn: &Connection) -> Result { - let sql = format!( - "SELECT COUNT(*) FROM sandboxes s +db_private_outside_tests! { + fn count_malformed_suspended_updated_at(conn: &Connection) -> Result { + let sql = format!( + "SELECT COUNT(*) FROM sandboxes s WHERE s.state = 'suspended' AND NOT ({VALID_SUSPENDED_UPDATED_AT_SQL})" - ); - conn.query_row(&sql, [], |row| { - let count: i64 = row.get(0)?; - Ok(usize::try_from(count).unwrap_or_default()) - }) + ); + conn.query_row(&sql, [], |row| { + let count: i64 = row.get(0)?; + Ok(usize::try_from(count).unwrap_or_default()) + }) + } } -/// Every local `sandboxes` row, whatever its state. node-ctl's full census -/// publishes one entry per row, so this is the count that sizes it. -pub fn count_resident_rows(conn: &Connection) -> Result { - conn.query_row("SELECT COUNT(*) FROM sandboxes", [], |row| { - let count: i64 = row.get(0)?; - usize::try_from(count).map_err(|_| rusqlite::Error::IntegralValueOutOfRange(0, count)) - }) +db_private_outside_tests! { + /// Every local `sandboxes` row, whatever its state. node-ctl's full census + /// publishes one entry per row, so this is the count that sizes it. + fn count_resident_rows(conn: &Connection) -> Result { + conn.query_row("SELECT COUNT(*) FROM sandboxes", [], |row| { + let count: i64 = row.get(0)?; + usize::try_from(count).map_err(|_| rusqlite::Error::IntegralValueOutOfRange(0, count)) + }) + } } -/// The sandbox's pinned artifact id and `updated_at`, `None` without a row. -pub fn artifact_pin_and_updated_at( - conn: &Connection, - sandbox_id: &str, -) -> Result, EpochSeconds)>> { - conn.query_row( - "SELECT current_local_artifact_id, updated_at FROM sandboxes WHERE id = ?1", - params![sandbox_id], - |r| Ok((r.get(0)?, r.get(1)?)), - ) - .optional() +db_private_outside_tests! { + /// The sandbox's pinned artifact id and `updated_at`, `None` without a row. + fn artifact_pin_and_updated_at( + conn: &Connection, + sandbox_id: &str, + ) -> Result, EpochSeconds)>> { + conn.query_row( + "SELECT current_local_artifact_id, updated_at FROM sandboxes WHERE id = ?1", + params![sandbox_id], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .optional() + } } -/// `(artifact_kind, local_artifact_id, backed up)` of the artifact `pin` -/// names, only if `sandbox_id` owns it. -pub fn owned_artifact_evidence( - conn: &Connection, - pin: &str, - sandbox_id: &str, -) -> Result> { - conn.query_row( - "SELECT artifact_kind, local_artifact_id, backup_completed_at IS NOT NULL +db_private_outside_tests! { + /// `(artifact_kind, local_artifact_id, backed up)` of the artifact `pin` + /// names, only if `sandbox_id` owns it. + fn owned_artifact_evidence( + conn: &Connection, + pin: &str, + sandbox_id: &str, + ) -> Result> { + conn.query_row( + "SELECT artifact_kind, local_artifact_id, backup_completed_at IS NOT NULL FROM local_artifacts WHERE local_artifact_id = ?1 AND sandbox_id = ?2", - params![pin, sandbox_id], - |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), - ) - .optional() + params![pin, sandbox_id], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .optional() + } } -/// The payload `snapshot_id` and `created_at` of the sandbox's newest -/// snapshot-upload job, whatever its status. -pub fn latest_upload_job( - conn: &Connection, - sandbox_id: &str, -) -> Result, String)>> { - conn.query_row( - "SELECT json_extract(j.payload, '$.snapshot_id'), j.created_at FROM jobs j +db_private_outside_tests! { + /// The payload `snapshot_id` and `created_at` of the sandbox's newest + /// snapshot-upload job, whatever its status. + fn latest_upload_job( + conn: &Connection, + sandbox_id: &str, + ) -> Result, String)>> { + conn.query_row( + "SELECT json_extract(j.payload, '$.snapshot_id'), j.created_at FROM jobs j WHERE j.kind = 'snapshot_upload' AND j.subject_id = ?1 ORDER BY j.created_at DESC, j.job_id DESC LIMIT 1", - params![sandbox_id], - |r| Ok((r.get(0)?, r.get(1)?)), - ) - .optional() + params![sandbox_id], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .optional() + } } diff --git a/crates/libs/sbx/mgr/store/src/db/guest_health_scan.rs b/crates/libs/sbx/mgr/store/src/db/guest_health_scan.rs index b46030584..749f27cb5 100644 --- a/crates/libs/sbx/mgr/store/src/db/guest_health_scan.rs +++ b/crates/libs/sbx/mgr/store/src/db/guest_health_scan.rs @@ -1,5 +1,6 @@ //! Sandbox rows the watchdog's guest-health and probe passes read. +use crate::db::db_private_outside_tests; use crate::db::{Connection, Error, Result}; use rusqlite::OptionalExtension as _; use rusqlite::types::Type; @@ -19,49 +20,53 @@ struct SandboxStateParseError { value: String, } -pub fn list_guest_health_rows(conn: &Connection) -> Result> { - let mut stmt = conn.prepare(sql!("SELECT id, state, template_hash FROM sandboxes"))?; - let rows = stmt.query_map([], |row| { - let state_value = row.get::<_, String>(1)?; - let state = state_value - .parse::() - .map_err(|_| { - rusqlite::Error::FromSqlConversionFailure( - 1, - Type::Text, - Box::new(SandboxStateParseError { value: state_value }), - ) - })?; - Ok(SandboxGuestHealthRow { - id: row.get(0)?, - state, - template_content_hash: row.get(2)?, - }) - })?; - Ok(rows.collect::>()?) +db_private_outside_tests! { + fn list_guest_health_rows(conn: &Connection) -> Result> { + let mut stmt = conn.prepare(sql!("SELECT id, state, template_hash FROM sandboxes"))?; + let rows = stmt.query_map([], |row| { + let state_value = row.get::<_, String>(1)?; + let state = state_value + .parse::() + .map_err(|_| { + rusqlite::Error::FromSqlConversionFailure( + 1, + Type::Text, + Box::new(SandboxStateParseError { value: state_value }), + ) + })?; + Ok(SandboxGuestHealthRow { + id: row.get(0)?, + state, + template_content_hash: row.get(2)?, + }) + })?; + Ok(rows.collect::>()?) + } } /// One sandbox's `state` label, `None` without a row. pub(crate) type StateLabelRead = Result>; -/// The `state` label of each of `sandbox_ids` through one prepared -/// statement, `None` for a sandbox without a row. Only preparing the -/// statement fails the whole lookup; each id's read fails on its own. -pub fn state_labels_for_ids<'a>( - conn: &Connection, - sandbox_ids: &'a [String], -) -> Result> { - let mut statement = conn.prepare("SELECT state FROM sandboxes WHERE id = ?1")?; - Ok(sandbox_ids - .iter() - .map(|sandbox_id| { - let label = statement - .query_row([sandbox_id], |row| row.get::<_, String>(0)) - .optional() - .map_err(Error::from); - (sandbox_id, label) - }) - .collect()) +db_private_outside_tests! { + /// The `state` label of each of `sandbox_ids` through one prepared + /// statement, `None` for a sandbox without a row. Only preparing the + /// statement fails the whole lookup; each id's read fails on its own. + fn state_labels_for_ids<'a>( + conn: &Connection, + sandbox_ids: &'a [String], + ) -> Result> { + let mut statement = conn.prepare("SELECT state FROM sandboxes WHERE id = ?1")?; + Ok(sandbox_ids + .iter() + .map(|sandbox_id| { + let label = statement + .query_row([sandbox_id], |row| row.get::<_, String>(0)) + .optional() + .map_err(Error::from); + (sandbox_id, label) + }) + .collect()) + } } #[cfg(test)] diff --git a/crates/libs/sbx/mgr/store/src/db/handle.rs b/crates/libs/sbx/mgr/store/src/db/handle.rs index 177c5f225..c0f29ad12 100644 --- a/crates/libs/sbx/mgr/store/src/db/handle.rs +++ b/crates/libs/sbx/mgr/store/src/db/handle.rs @@ -8,11 +8,14 @@ use std::sync::{Arc, LockResult, Mutex, MutexGuard, PoisonError}; use ref_cast::{RefCastCustom, ref_cast_custom}; -/// A method that returns or takes rusqlite values, or that only `db` calls in -/// production: public with the `test-support` feature, for sbx-mgr's fixtures -/// and tripwires outside the store, and private to `db` otherwise, so -/// production code outside the store cannot reach a raw connection, statement -/// or error, or a method only tests need. +/// A function or method that returns or takes rusqlite values, or that only +/// `db` calls in production: public with the `test-support` feature, for +/// sbx-mgr's fixtures and tripwires outside the store, and private to `db` +/// otherwise, so production code outside the store cannot reach a raw +/// connection, statement or error, or a function only tests need. The readers +/// that `db::read` and `db::scan` consolidate are declared through it: tests +/// outside the store still compare against them, production reads through +/// the consolidated API. macro_rules! db_private_outside_tests { ($(#[$attr:meta])* fn $name:ident $($rest:tt)*) => { $(#[$attr])* @@ -24,6 +27,7 @@ macro_rules! db_private_outside_tests { pub(in crate::db) fn $name $($rest)* }; } +pub(in crate::db) use db_private_outside_tests; /// A failed SQLite operation. `Display` and `source` are the underlying /// rusqlite error's, unchanged. diff --git a/crates/libs/sbx/mgr/store/src/db/inventory.rs b/crates/libs/sbx/mgr/store/src/db/inventory.rs index 65f9b038e..546b3eafd 100644 --- a/crates/libs/sbx/mgr/store/src/db/inventory.rs +++ b/crates/libs/sbx/mgr/store/src/db/inventory.rs @@ -1,5 +1,6 @@ //! Bounded local discovery. A batch is never a complete external inventory. +use crate::db::db_private_outside_tests; use crate::db::{Connection, Result}; use super::{SandboxRow, describe_error, describe_raw_error, get_sandbox, is_row_decode_error}; @@ -29,49 +30,51 @@ impl SandboxInventoryScan { self.through.is_none() } - pub fn next_batch(&mut self, conn: &Connection) -> Result> { - if self.through.is_none() { - self.through = - conn.query_row("SELECT MAX(rowid) FROM sandboxes", [], |row| row.get(0))?; - } - let Some(through) = self.through else { - return Ok(Vec::new()); - }; - let mut statement = conn.prepare( - "SELECT rowid, id FROM sandboxes - WHERE rowid >= ?1 AND rowid <= ?2 ORDER BY rowid LIMIT 128", - )?; - let mut rows = statement.query([self.next, through])?; - let mut batch = Vec::new(); - for _ in 0..INVENTORY_BATCH_SIZE { - let Some(row) = rows.next()? else { - *self = Self::default(); - break; - }; - let rowid: i64 = row.get(0)?; - if rowid == through { - *self = Self::default(); - } else { - self.next = rowid + 1; + db_private_outside_tests! { + fn next_batch(&mut self, conn: &Connection) -> Result> { + if self.through.is_none() { + self.through = + conn.query_row("SELECT MAX(rowid) FROM sandboxes", [], |row| row.get(0))?; } - let id = match row.get::<_, String>(1) { - Ok(id) => id, - Err(error) if is_row_decode_error(&error) => { - tracing::warn!(rowid, error = %describe_raw_error(&error), "preserving undecodable inventory identity"); - continue; - } - Err(error) => return Err(crate::db::Error::from(error)), + let Some(through) = self.through else { + return Ok(Vec::new()); }; - match get_sandbox(conn, &id) { - Ok(Some(row)) => batch.push(row), - Ok(None) => {} - Err(error) if is_row_decode_error(error.raw()) => { - tracing::warn!(sandbox_id = %id, error = %describe_error(&error), "preserving undecodable inventory row"); + let mut statement = conn.prepare( + "SELECT rowid, id FROM sandboxes + WHERE rowid >= ?1 AND rowid <= ?2 ORDER BY rowid LIMIT 128", + )?; + let mut rows = statement.query([self.next, through])?; + let mut batch = Vec::new(); + for _ in 0..INVENTORY_BATCH_SIZE { + let Some(row) = rows.next()? else { + *self = Self::default(); + break; + }; + let rowid: i64 = row.get(0)?; + if rowid == through { + *self = Self::default(); + } else { + self.next = rowid + 1; + } + let id = match row.get::<_, String>(1) { + Ok(id) => id, + Err(error) if is_row_decode_error(&error) => { + tracing::warn!(rowid, error = %describe_raw_error(&error), "preserving undecodable inventory identity"); + continue; + } + Err(error) => return Err(crate::db::Error::from(error)), + }; + match get_sandbox(conn, &id) { + Ok(Some(row)) => batch.push(row), + Ok(None) => {} + Err(error) if is_row_decode_error(error.raw()) => { + tracing::warn!(sandbox_id = %id, error = %describe_error(&error), "preserving undecodable inventory row"); + } + Err(error) => return Err(error), } - Err(error) => return Err(error), } + Ok(batch) } - Ok(batch) } } @@ -96,58 +99,60 @@ impl Default for RestoreTransferScopeScan { } impl RestoreTransferScopeScan { - pub fn next_batch( - &mut self, - conn: &Connection, - ) -> Result> { - if self.through.is_none() { - self.through = conn.query_row( - "SELECT MAX(rowid) FROM sandbox_restore_transfer_scopes", - [], - |row| row.get(0), - )?; - } - let Some(through) = self.through else { - return Ok(Vec::new()); - }; - let mut statement = conn.prepare( - "SELECT scopes.rowid, scopes.sandbox_id, scopes.transfer_scope_id, + db_private_outside_tests! { + fn next_batch( + &mut self, + conn: &Connection, + ) -> Result> { + if self.through.is_none() { + self.through = conn.query_row( + "SELECT MAX(rowid) FROM sandbox_restore_transfer_scopes", + [], + |row| row.get(0), + )?; + } + let Some(through) = self.through else { + return Ok(Vec::new()); + }; + let mut statement = conn.prepare( + "SELECT scopes.rowid, scopes.sandbox_id, scopes.transfer_scope_id, scopes.execution_generation, scopes.activation_attempt_id, scopes.staging_dir, EXISTS (SELECT 1 FROM sandboxes WHERE id = scopes.sandbox_id) FROM sandbox_restore_transfer_scopes AS scopes WHERE scopes.rowid >= ?1 AND scopes.rowid <= ?2 ORDER BY scopes.rowid LIMIT 128", - )?; - let mut rows = statement.query([self.next, through])?; - let mut batch = Vec::new(); - let mut examined = 0; - while let Some(row) = rows.next()? { - examined += 1; - let rowid: i64 = row.get(0)?; - if rowid == through { - *self = Self::default(); - } else { - self.next = rowid + 1; - } - if row.get::<_, bool>(6)? { - continue; - } - let decoded = (|| Ok((row.get(1)?, super::restore_transfer_scope_from_row(row, 2)?)))(); - match decoded { - Ok(scope) => batch.push(scope), - Err(error) if is_row_decode_error(&error) => { - tracing::warn!(rowid, error = %describe_raw_error(&error), "preserving malformed orphan transfer scope"); + )?; + let mut rows = statement.query([self.next, through])?; + let mut batch = Vec::new(); + let mut examined = 0; + while let Some(row) = rows.next()? { + examined += 1; + let rowid: i64 = row.get(0)?; + if rowid == through { + *self = Self::default(); + } else { + self.next = rowid + 1; + } + if row.get::<_, bool>(6)? { + continue; + } + let decoded = (|| Ok((row.get(1)?, super::restore_transfer_scope_from_row(row, 2)?)))(); + match decoded { + Ok(scope) => batch.push(scope), + Err(error) if is_row_decode_error(&error) => { + tracing::warn!(rowid, error = %describe_raw_error(&error), "preserving malformed orphan transfer scope"); + } + Err(error) => return Err(error.into()), + } + if batch.len() == 8 { + return Ok(batch); } - Err(error) => return Err(error.into()), } - if batch.len() == 8 { - return Ok(batch); + // A short page means exhaustion; a full page may have more successors. + if examined < INVENTORY_BATCH_SIZE { + *self = Self::default(); } + Ok(batch) } - // A short page means exhaustion; a full page may have more successors. - if examined < INVENTORY_BATCH_SIZE { - *self = Self::default(); - } - Ok(batch) } } diff --git a/crates/libs/sbx/mgr/store/src/db/inventory_read.rs b/crates/libs/sbx/mgr/store/src/db/inventory_read.rs index 2afbe2dc1..7935fa223 100644 --- a/crates/libs/sbx/mgr/store/src/db/inventory_read.rs +++ b/crates/libs/sbx/mgr/store/src/db/inventory_read.rs @@ -2,6 +2,7 @@ //! Unpaged ListBuildReferencingSandboxes is the exception: it reports overruns //! instead. +use crate::db::db_private_outside_tests; use crate::db::{Connection, Error}; use pplx_space_common_core::inventory::{ InventoryBudget, InventoryLimitExceeded, MAX_ITEMS, MAX_RAW_BYTES, @@ -96,22 +97,24 @@ impl UnaryListUsage { const LIST_COLUMNS: &str = "id, state, template, template_hash, user, runtime, created_at, pool_template"; -pub fn list( - conn: &Connection, - rpc: &'static str, -) -> Result, InventoryReadError> { - let mut stmt = conn.prepare(&format!( - "SELECT {LIST_COLUMNS} FROM sandboxes ORDER BY created_at" - ))?; - let mut rows = stmt.query([])?; - let mut usage = UnaryListUsage::default(); - let mut result = Vec::new(); - while let Some(row) = rows.next()? { - usage.observe(row)?; - result.push(super::row_to_list_sandbox(row)?); +db_private_outside_tests! { + fn list( + conn: &Connection, + rpc: &'static str, + ) -> Result, InventoryReadError> { + let mut stmt = conn.prepare(&format!( + "SELECT {LIST_COLUMNS} FROM sandboxes ORDER BY created_at" + ))?; + let mut rows = stmt.query([])?; + let mut usage = UnaryListUsage::default(); + let mut result = Vec::new(); + while let Some(row) = rows.next()? { + usage.observe(row)?; + result.push(super::row_to_list_sandbox(row)?); + } + usage.report(rpc); + Ok(result) } - usage.report(rpc); - Ok(result) } /// One scanned List row. An undecodable row keeps its identity so the caller @@ -140,105 +143,112 @@ pub struct ListPage { pub more: bool, } -/// Scan up to `limit` rows with IDs after `after`, in ID order. The page ends -/// early rather than exceed the raw budget; only a single row over the budget -/// is an error. Keying on the immutable primary key keeps every row present for -/// the whole scan on exactly one page. -pub fn list_page( - conn: &Connection, - after: &str, - limit: usize, -) -> Result { - let mut stmt = conn.prepare_cached(&format!( - "SELECT {LIST_COLUMNS} FROM sandboxes WHERE id > ?1 ORDER BY id LIMIT ?2" - ))?; - let fetch = i64::try_from(limit.saturating_add(1)).unwrap_or(i64::MAX); - let mut rows = stmt.query(rusqlite::params![after, fetch])?; - let mut budget = InventoryBudget::new(limit, MAX_RAW_BYTES); - let mut page = Vec::new(); - while let Some(row) = rows.next()? { - match check_row(row, &mut budget) { - Ok(()) => {} - Err(InventoryReadError::Limit(_)) if !page.is_empty() => { - return Ok(ListPage { - rows: page, - more: true, - }); +db_private_outside_tests! { + /// Scan up to `limit` rows with IDs after `after`, in ID order. The page ends + /// early rather than exceed the raw budget; only a single row over the budget + /// is an error. Keying on the immutable primary key keeps every row present for + /// the whole scan on exactly one page. + fn list_page( + conn: &Connection, + after: &str, + limit: usize, + ) -> Result { + let mut stmt = conn.prepare_cached(&format!( + "SELECT {LIST_COLUMNS} FROM sandboxes WHERE id > ?1 ORDER BY id LIMIT ?2" + ))?; + let fetch = i64::try_from(limit.saturating_add(1)).unwrap_or(i64::MAX); + let mut rows = stmt.query(rusqlite::params![after, fetch])?; + let mut budget = InventoryBudget::new(limit, MAX_RAW_BYTES); + let mut page = Vec::new(); + while let Some(row) = rows.next()? { + match check_row(row, &mut budget) { + Ok(()) => {} + Err(InventoryReadError::Limit(_)) if !page.is_empty() => { + return Ok(ListPage { + rows: page, + more: true, + }); + } + Err(error) => return Err(error), } - Err(error) => return Err(error), + page.push(match super::row_to_list_sandbox(row) { + Ok(row) => ListPageRow::Decoded(row), + Err(error) => ListPageRow::Undecodable { + id: row.get("id")?, + state: row.get::<_, String>("state").ok(), + error: error.into(), + }, + }); } - page.push(match super::row_to_list_sandbox(row) { - Ok(row) => ListPageRow::Decoded(row), - Err(error) => ListPageRow::Undecodable { - id: row.get("id")?, - state: row.get::<_, String>("state").ok(), - error: error.into(), - }, - }); + Ok(ListPage { + rows: page, + more: false, + }) } - Ok(ListPage { - rows: page, - more: false, - }) } -pub fn get( - conn: &Connection, - id: &str, - budget: &mut InventoryBudget, -) -> Result, InventoryReadError> { - let mut stmt = conn.prepare_cached(super::GET_SANDBOX_SQL)?; - let mut rows = stmt.query([id])?; - rows.next()? - .map(|row| { - check_row(row, budget)?; - Ok(super::row_to_sandbox(row)?) - }) - .transpose() +db_private_outside_tests! { + fn get( + conn: &Connection, + id: &str, + budget: &mut InventoryBudget, + ) -> Result, InventoryReadError> { + let mut stmt = conn.prepare_cached(super::GET_SANDBOX_SQL)?; + let mut rows = stmt.query([id])?; + rows.next()? + .map(|row| { + check_row(row, budget)?; + Ok(super::row_to_sandbox(row)?) + }) + .transpose() + } } -pub fn lookup( - conn: &Connection, - selector: &str, -) -> Result, InventoryReadError> { - let mut budget = InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES); - if let Some(row) = get(conn, selector, &mut budget)? { - return Ok(Some(row)); - } - let mut stmt = conn.prepare("SELECT id FROM sandboxes ORDER BY id")?; - let mut rows = stmt.query([])?; - let mut matched = None; - while let Some(row) = rows.next()? { - check_row(row, &mut budget)?; - let id: String = row.get(0)?; - if pplx_space_common_core::ids::sandbox_id_matches(&id, selector) { - if matched.is_some() { - return Err(InventoryReadError::Ambiguous); +db_private_outside_tests! { + fn lookup( + conn: &Connection, + selector: &str, + ) -> Result, InventoryReadError> { + let mut budget = InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES); + if let Some(row) = get(conn, selector, &mut budget)? { + return Ok(Some(row)); + } + let mut stmt = conn.prepare("SELECT id FROM sandboxes ORDER BY id")?; + let mut rows = stmt.query([])?; + let mut matched = None; + while let Some(row) = rows.next()? { + check_row(row, &mut budget)?; + let id: String = row.get(0)?; + if pplx_space_common_core::ids::sandbox_id_matches(&id, selector) { + if matched.is_some() { + return Err(InventoryReadError::Ambiguous); + } + matched = Some(id); } - matched = Some(id); } + matched + .map(|id| { + get( + conn, + &id, + &mut InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES), + ) + }) + .transpose() + .map(Option::flatten) } - matched - .map(|id| { - get( - conn, - &id, - &mut InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES), - ) - }) - .transpose() - .map(Option::flatten) } -/// Check auxiliary State fields before the existing typed readers allocate them. -/// The caller retains the same SQLite mutex across preflight and those reads. -pub fn check_state_metadata( - conn: &Connection, - ids: &[String], - budget: &mut InventoryBudget, -) -> Result<(), InventoryReadError> { - let mut stmt = conn.prepare_cached( - "SELECT s.id, s.state, s.state_changed_at, s.transition_receipt_id, +db_private_outside_tests! { + /// Check auxiliary State fields before the existing typed readers allocate them. + /// The caller retains the same SQLite mutex across preflight and those reads. + fn check_state_metadata( + conn: &Connection, + ids: &[String], + budget: &mut InventoryBudget, + ) -> Result<(), InventoryReadError> { + let mut stmt = conn.prepare_cached( + "SELECT s.id, s.state, s.state_changed_at, s.transition_receipt_id, s.transition_receipt_kind, s.transition_receipt_target_state, s.transition_receipt_execution_generation, s.transition_receipt_reason_code, s.transition_receipt_activation_attempt_id, s.space_api_route_generation, @@ -248,14 +258,15 @@ pub fn check_state_metadata( q.reason_code, q.quarantine_id, q.next_action_owner, q.attempt_count, q.next_action_after FROM sandboxes s LEFT JOIN sandbox_quarantine_state q ON q.sandbox_id = s.id WHERE s.id = ?1" - )?; - for id in ids { - let mut rows = stmt.query([id])?; - if let Some(row) = rows.next()? { - check_row(row, budget)?; + )?; + for id in ids { + let mut rows = stmt.query([id])?; + if let Some(row) = rows.next()? { + check_row(row, budget)?; + } } + Ok(()) } - Ok(()) } #[cfg(test)] diff --git a/crates/libs/sbx/mgr/store/src/db/local_artifact_leases.rs b/crates/libs/sbx/mgr/store/src/db/local_artifact_leases.rs index 87129b291..18ae8159b 100644 --- a/crates/libs/sbx/mgr/store/src/db/local_artifact_leases.rs +++ b/crates/libs/sbx/mgr/store/src/db/local_artifact_leases.rs @@ -2,6 +2,7 @@ //! restore and delete leases. Each runs on the connection or transaction its //! caller holds, so a checkout's reads and its CAS share one transaction. +use crate::db::db_private_outside_tests; use crate::db::{Connection, Error, OptionalExtension, Result}; use pplx_space_common_core::sql; use pplx_space_common_core::types::{ArtifactKind, LocalArtifactId, SnapshotName}; @@ -88,12 +89,13 @@ pub struct NamedArtifactPublication { pub state: ArtifactState, } -/// Every sandbox whose pinned artifact has no backup-completed stamp, or -/// whose pin names no row it owns. The caller, -/// `LocalArtifacts::list_pending_backups`, documents the contract. -pub fn list_pending_backups(conn: &Connection) -> Result> { - conn.prepare( - "SELECT s.id, s.current_local_artifact_id, s.current_local_artifact_kind +db_private_outside_tests! { + /// Every sandbox whose pinned artifact has no backup-completed stamp, or + /// whose pin names no row it owns. The caller, + /// `LocalArtifacts::list_pending_backups`, documents the contract. + fn list_pending_backups(conn: &Connection) -> Result> { + conn.prepare( + "SELECT s.id, s.current_local_artifact_id, s.current_local_artifact_kind FROM sandboxes AS s LEFT JOIN local_artifacts AS a ON a.local_artifact_id = s.current_local_artifact_id @@ -101,72 +103,75 @@ pub fn list_pending_backups(conn: &Connection) -> Result> { WHERE s.current_local_artifact_id IS NOT NULL AND (a.local_artifact_id IS NULL OR a.backup_completed_at IS NULL) ORDER BY s.id", - )? - .query_map([], |row| { - Ok(PendingBackup { - sandbox_id: row.get(0)?, - local_artifact_id: row.get::<_, String>(1)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 1, - rusqlite::types::Type::Text, - Box::new(error), - ) - })?, - artifact_kind: row.get::<_, String>(2)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 2, - rusqlite::types::Type::Text, - Box::new(error), - ) - })?, - }) - })? - .collect::>() - .map_err(crate::db::Error::from) + )? + .query_map([], |row| { + Ok(PendingBackup { + sandbox_id: row.get(0)?, + local_artifact_id: row.get::<_, String>(1)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 1, + rusqlite::types::Type::Text, + Box::new(error), + ) + })?, + artifact_kind: row.get::<_, String>(2)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 2, + rusqlite::types::Type::Text, + Box::new(error), + ) + })?, + }) + })? + .collect::>() + .map_err(crate::db::Error::from) + } } -/// A sandbox's named-snapshot artifacts, within the inventory read budget. -pub fn list_named_publications( - conn: &Connection, - sandbox_id: &str, -) -> std::result::Result, crate::db::inventory_read::InventoryReadError> -{ - use pplx_space_common_core::inventory::{InventoryBudget, MAX_ITEMS, MAX_RAW_BYTES}; - let mut statement = conn.prepare(sql!( - "SELECT snapshot_name, local_path, artifact_kind, state +db_private_outside_tests! { + /// A sandbox's named-snapshot artifacts, within the inventory read budget. + fn list_named_publications( + conn: &Connection, + sandbox_id: &str, + ) -> std::result::Result, crate::db::inventory_read::InventoryReadError> + { + use pplx_space_common_core::inventory::{InventoryBudget, MAX_ITEMS, MAX_RAW_BYTES}; + let mut statement = conn.prepare(sql!( + "SELECT snapshot_name, local_path, artifact_kind, state FROM local_artifacts WHERE sandbox_id = ?1 AND artifact_origin = 'snapshot'" - ))?; - let mut rows = statement.query([sandbox_id])?; - let mut budget = InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES); - let mut publications = Vec::new(); - while let Some(row) = rows.next()? { - crate::db::inventory_read::check_row(row, &mut budget)?; - publications.push(NamedArtifactPublication { - snapshot_name: row.get::<_, String>(0)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 0, - rusqlite::types::Type::Text, - Box::new(error), - ) - })?, - local_path: row.get(1)?, - artifact_kind: row.get::<_, String>(2)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 2, - rusqlite::types::Type::Text, - Box::new(error), - ) - })?, - state: row.get::<_, String>(3)?.parse().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 3, - rusqlite::types::Type::Text, - Box::new(error), - ) - })?, - }); + ))?; + let mut rows = statement.query([sandbox_id])?; + let mut budget = InventoryBudget::new(MAX_ITEMS, MAX_RAW_BYTES); + let mut publications = Vec::new(); + while let Some(row) = rows.next()? { + crate::db::inventory_read::check_row(row, &mut budget)?; + publications.push(NamedArtifactPublication { + snapshot_name: row.get::<_, String>(0)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 0, + rusqlite::types::Type::Text, + Box::new(error), + ) + })?, + local_path: row.get(1)?, + artifact_kind: row.get::<_, String>(2)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 2, + rusqlite::types::Type::Text, + Box::new(error), + ) + })?, + state: row.get::<_, String>(3)?.parse().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 3, + rusqlite::types::Type::Text, + Box::new(error), + ) + })?, + }); + } + Ok(publications) } - Ok(publications) } /// `(local_path, state)` of an artifact row, as stored. @@ -311,17 +316,19 @@ pub fn release_restoring_artifact(conn: &Connection, local_artifact_id: &str) -> ) } -/// Whether neither an artifact row nor a sandbox pin names the id. -pub fn gc_staging_is_unowned(conn: &Connection, local_artifact_id: &str) -> Result { - conn.query_row( - sql!( - "SELECT NOT EXISTS(SELECT 1 FROM local_artifacts WHERE local_artifact_id = ?1) +db_private_outside_tests! { + /// Whether neither an artifact row nor a sandbox pin names the id. + fn gc_staging_is_unowned(conn: &Connection, local_artifact_id: &str) -> Result { + conn.query_row( + sql!( + "SELECT NOT EXISTS(SELECT 1 FROM local_artifacts WHERE local_artifact_id = ?1) AND NOT EXISTS(SELECT 1 FROM sandboxes WHERE current_local_artifact_id = ?1 OR suspend_predecessor_artifact_id = ?1)" - ), - [local_artifact_id], - |row| row.get(0), - ) + ), + [local_artifact_id], + |row| row.get(0), + ) + } } /// The highest artifact rowid, `None` for an empty table. diff --git a/crates/libs/sbx/mgr/store/src/db/local_artifacts.rs b/crates/libs/sbx/mgr/store/src/db/local_artifacts.rs index 85c17acdf..bd9b423ea 100644 --- a/crates/libs/sbx/mgr/store/src/db/local_artifacts.rs +++ b/crates/libs/sbx/mgr/store/src/db/local_artifacts.rs @@ -518,32 +518,34 @@ pub fn mark_local_artifact_backup_completed( Ok(()) } -/// True when the sandbox row is parked `suspended` with -/// `current_local_artifact_id` equal to `local_artifact_id`. -/// -/// This is the safety gate for reclaiming per-artifact files under the -/// published snapshot directory after a backup finalizes ready. Every -/// suspend stages into a sibling directory and flips the row to -/// `suspended` with the freshly minted artifact pinned in one -/// transaction, so while this predicate holds — checked under the same -/// connection mutex every state transition serializes on — the files at -/// the published snapshot path are exactly the pinned artifact's bytes, -/// never a newer suspend's output mid-encode or mid-publish. -pub fn sandbox_suspended_with_pinned_artifact( - conn: &Connection, - sandbox_id: &str, - local_artifact_id: &str, -) -> Result { - conn.query_row( - "SELECT EXISTS( +db_private_outside_tests! { + /// True when the sandbox row is parked `suspended` with + /// `current_local_artifact_id` equal to `local_artifact_id`. + /// + /// This is the safety gate for reclaiming per-artifact files under the + /// published snapshot directory after a backup finalizes ready. Every + /// suspend stages into a sibling directory and flips the row to + /// `suspended` with the freshly minted artifact pinned in one + /// transaction, so while this predicate holds — checked under the same + /// connection mutex every state transition serializes on — the files at + /// the published snapshot path are exactly the pinned artifact's bytes, + /// never a newer suspend's output mid-encode or mid-publish. + fn sandbox_suspended_with_pinned_artifact( + conn: &Connection, + sandbox_id: &str, + local_artifact_id: &str, + ) -> Result { + conn.query_row( + "SELECT EXISTS( SELECT 1 FROM sandboxes WHERE id = ?1 AND state = 'suspended' AND current_local_artifact_id = ?2 )", - params![sandbox_id, local_artifact_id], - |r| r.get(0), - ) + params![sandbox_id, local_artifact_id], + |r| r.get(0), + ) + } } pub fn publish_named_local_artifact( @@ -764,34 +766,38 @@ pub fn pinned_local_artifact_created_at( Ok(created_at) } -pub fn get_local_artifact( - conn: &Connection, - local_artifact_id: LocalArtifactId, -) -> Result> { - conn.query_row( - "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, +db_private_outside_tests! { + fn get_local_artifact( + conn: &Connection, + local_artifact_id: LocalArtifactId, + ) -> Result> { + conn.query_row( + "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, snapshot_name, local_path FROM local_artifacts WHERE local_artifact_id = ?1", - params![local_artifact_id.to_string()], - row_to_local_artifact, - ) - .optional() + params![local_artifact_id.to_string()], + row_to_local_artifact, + ) + .optional() + } } -pub fn get_named_local_artifact( - conn: &Connection, - sandbox_id: &str, - snapshot_name: &SnapshotName, -) -> Result> { - conn.query_row( - "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, +db_private_outside_tests! { + fn get_named_local_artifact( + conn: &Connection, + sandbox_id: &str, + snapshot_name: &SnapshotName, + ) -> Result> { + conn.query_row( + "SELECT local_artifact_id, sandbox_id, artifact_kind, artifact_origin, snapshot_name, local_path FROM local_artifacts WHERE sandbox_id = ?1 AND artifact_origin = 'snapshot' AND snapshot_name = ?2", - params![sandbox_id, snapshot_name.as_str()], - row_to_local_artifact, - ) - .optional() + params![sandbox_id, snapshot_name.as_str()], + row_to_local_artifact, + ) + .optional() + } } pub(super) fn row_to_local_artifact(row: &rusqlite::Row<'_>) -> rusqlite::Result { diff --git a/crates/libs/sbx/mgr/store/src/db/network_policy.rs b/crates/libs/sbx/mgr/store/src/db/network_policy.rs index 8f5f70389..7da4e2838 100644 --- a/crates/libs/sbx/mgr/store/src/db/network_policy.rs +++ b/crates/libs/sbx/mgr/store/src/db/network_policy.rs @@ -379,22 +379,24 @@ pub struct NodeDirtyRefRow { pub revision: i64, } -pub fn list_node_dirty_refs(conn: &Connection, limit: usize) -> Result> { - let mut stmt = conn.prepare( - "SELECT facet, element_kind, element_value, revision +db_private_outside_tests! { + fn list_node_dirty_refs(conn: &Connection, limit: usize) -> Result> { + let mut stmt = conn.prepare( + "SELECT facet, element_kind, element_value, revision FROM node_dirty_refs ORDER BY requested_at ASC LIMIT ?1", - )?; - let rows = stmt.query_map(params![limit.min(i64::MAX as usize) as i64], |row| { - Ok(NodeDirtyRefRow { - facet: row.get(0)?, - element_kind: row.get(1)?, - element_value: row.get(2)?, - revision: row.get(3)?, - }) - })?; - Ok(rows.collect::>()?) + )?; + let rows = stmt.query_map(params![limit.min(i64::MAX as usize) as i64], |row| { + Ok(NodeDirtyRefRow { + facet: row.get(0)?, + element_kind: row.get(1)?, + element_value: row.get(2)?, + revision: row.get(3)?, + }) + })?; + Ok(rows.collect::>()?) + } } /// Acknowledge only the obligations read before sending. A newer enqueue, even diff --git a/crates/libs/sbx/mgr/store/src/db/pool.rs b/crates/libs/sbx/mgr/store/src/db/pool.rs index 63d653766..aadb5bf9e 100644 --- a/crates/libs/sbx/mgr/store/src/db/pool.rs +++ b/crates/libs/sbx/mgr/store/src/db/pool.rs @@ -5,15 +5,16 @@ use super::*; // -- Pool helpers -- // Migration needed: ALTER TABLE sandboxes ADD COLUMN pool_template TEXT; -/// Find one paused sandbox with matching pool membership and optional content hash. -pub fn find_pool_sandbox( - conn: &Connection, - template: &str, - content_hash: Option<&str>, - required_user: Option<&SandboxOwner>, -) -> Result> { - let mut stmt = conn.prepare( - "SELECT id, uuid8, state, template, template_hash, user, runtime, uid, slot, guest_cid, pid, +db_private_outside_tests! { + /// Find one paused sandbox with matching pool membership and optional content hash. + fn find_pool_sandbox( + conn: &Connection, + template: &str, + content_hash: Option<&str>, + required_user: Option<&SandboxOwner>, + ) -> Result> { + let mut stmt = conn.prepare( + "SELECT id, uuid8, state, template, template_hash, user, runtime, uid, slot, guest_cid, pid, spaced_host_pid, netgtw_pid, vsock_uds_path, api_socket_path, created_at, updated_at, snapshots_enabled, rootfs_size_mb, pool_template, guest_uptime_secs, process_env_json, auto_lifecycle_json, auto_lifecycle_state_json, next_auto_event_at, failure_reason, cleanup_completion, cleanup_keep_filesystem, network_policy_override_json, network_tx_bytes, network_rx_bytes, network_policy_generation, applied_network_policy_generation, applied_network_policy_source, network_policy_authority, auto_lifecycle_authority, current_local_artifact_id, current_local_artifact_kind, current_local_artifact_origin, execution_metadata_json, volume_encryption, space_api_route_generation, space_api_route_node_id, space_api_route_execution_generation, space_api_route_pod_uid, restore_origin, restore_phase, restore_storage_uri, space_api_route_substrate_kind, space_api_route_activation_attempt_id, spaced_endpoint_proof_json, disk_io_max_operations_per_sec, disk_io_max_bandwidth_mib_per_sec, node_execution_id FROM sandboxes WHERE pool_template = ?1 @@ -22,16 +23,17 @@ pub fn find_pool_sandbox( AND (?3 IS NULL OR user = ?3) ORDER BY created_at, id LIMIT 1", - )?; - let mut rows = stmt.query_map( - params![ - template, - content_hash, - required_user.map(SandboxOwner::as_str) - ], - row_to_sandbox, - )?; - rows.next().transpose().map_err(Error::from) + )?; + let mut rows = stmt.query_map( + params![ + template, + content_hash, + required_user.map(SandboxOwner::as_str) + ], + row_to_sandbox, + )?; + rows.next().transpose().map_err(Error::from) + } } pub fn claim_pool_sandbox( diff --git a/crates/libs/sbx/mgr/store/src/db/quarantine.rs b/crates/libs/sbx/mgr/store/src/db/quarantine.rs index ed6ca73a2..537fcb10f 100644 --- a/crates/libs/sbx/mgr/store/src/db/quarantine.rs +++ b/crates/libs/sbx/mgr/store/src/db/quarantine.rs @@ -715,16 +715,17 @@ pub fn recover_pause_failed_quarantine( Ok(true) } -/// Load optional quarantine or transition-receipt metadata for inventory -/// publication. Wire degradation for malformed rows happens in the State RPC, -/// so this storage helper exposes the row vocabulary without deciding whether -/// another sandbox may still be published. -pub fn sandbox_state_publication_metadata( - conn: &Connection, - sandbox_ids: &[String], -) -> Result> { - let mut statement = conn.prepare_cached( - "SELECT s.id, s.state, qs.reason_code, +db_private_outside_tests! { + /// Load optional quarantine or transition-receipt metadata for inventory + /// publication. Wire degradation for malformed rows happens in the State RPC, + /// so this storage helper exposes the row vocabulary without deciding whether + /// another sandbox may still be published. + fn sandbox_state_publication_metadata( + conn: &Connection, + sandbox_ids: &[String], + ) -> Result> { + let mut statement = conn.prepare_cached( + "SELECT s.id, s.state, qs.reason_code, s.transition_receipt_id, s.transition_receipt_kind, s.transition_receipt_target_state, @@ -736,36 +737,37 @@ pub fn sandbox_state_publication_metadata( WHERE s.id = ?1 AND (s.state = 'quarantined' OR s.transition_receipt_id IS NOT NULL)", - )?; - let mut publications = BTreeMap::new(); - for sandbox_id in sandbox_ids { - let publication = statement - .query_row(params![sandbox_id], |row| { - Ok(( - row.get::<_, String>(0)?, - RawSandboxStatePublicationMetadata { - quarantine_reason_code: row.get(2)?, - transition_receipt_id: row.get(3)?, - transition_receipt_kind: row.get(4)?, - transition_receipt_target_state: row.get(5)?, - transition_receipt_execution_generation: row.get(6)?, - transition_receipt_reason_code: row.get(7)?, - transition_receipt_activation_attempt_id: row.get(8)?, - }, - )) - }) - .optional(); - match publication { - Ok(Some((id, publication))) => { - publications.insert(id, Ok(publication)); - } - Ok(None) => {} - Err(error) => { - publications.insert(sandbox_id.clone(), Err(error.into())); + )?; + let mut publications = BTreeMap::new(); + for sandbox_id in sandbox_ids { + let publication = statement + .query_row(params![sandbox_id], |row| { + Ok(( + row.get::<_, String>(0)?, + RawSandboxStatePublicationMetadata { + quarantine_reason_code: row.get(2)?, + transition_receipt_id: row.get(3)?, + transition_receipt_kind: row.get(4)?, + transition_receipt_target_state: row.get(5)?, + transition_receipt_execution_generation: row.get(6)?, + transition_receipt_reason_code: row.get(7)?, + transition_receipt_activation_attempt_id: row.get(8)?, + }, + )) + }) + .optional(); + match publication { + Ok(Some((id, publication))) => { + publications.insert(id, Ok(publication)); + } + Ok(None) => {} + Err(error) => { + publications.insert(sandbox_id.clone(), Err(error.into())); + } } } + Ok(publications) } - Ok(publications) } pub fn mark_quarantined_from_state( diff --git a/crates/libs/sbx/mgr/store/src/db/quarantine_recovery.rs b/crates/libs/sbx/mgr/store/src/db/quarantine_recovery.rs index 3962f3944..8d3a16f35 100644 --- a/crates/libs/sbx/mgr/store/src/db/quarantine_recovery.rs +++ b/crates/libs/sbx/mgr/store/src/db/quarantine_recovery.rs @@ -173,19 +173,20 @@ pub struct RawQuarantineRecoveryObservation { pub type RawQuarantineRecoveryObservationRead = std::result::Result; -pub fn is_orphan_quarantine_recovery_state( - conn: &Connection, - sandbox_id: &str, - expected_state: SandboxState, -) -> Result { - if !matches!( - expected_state, - SandboxState::Quarantined | SandboxState::Restoring - ) { - return Ok(false); - } - conn.query_row( - "SELECT EXISTS ( +db_private_outside_tests! { + fn is_orphan_quarantine_recovery_state( + conn: &Connection, + sandbox_id: &str, + expected_state: SandboxState, + ) -> Result { + if !matches!( + expected_state, + SandboxState::Quarantined | SandboxState::Restoring + ) { + return Ok(false); + } + conn.query_row( + "SELECT EXISTS ( SELECT 1 FROM sandboxes JOIN sandbox_quarantine_state qs ON qs.sandbox_id = sandboxes.id @@ -195,14 +196,15 @@ pub fn is_orphan_quarantine_recovery_state( AND qs.source = ?3 AND qs.reason_code = ?4 )", - params![ - sandbox_id, - expected_state.as_str(), - SandboxQuarantineSource::SbxManager.as_str(), - SandboxQuarantineReasonCode::OrphanedSandboxRowMissing.as_str(), - ], - |row| row.get(0), - ) + params![ + sandbox_id, + expected_state.as_str(), + SandboxQuarantineSource::SbxManager.as_str(), + SandboxQuarantineReasonCode::OrphanedSandboxRowMissing.as_str(), + ], + |row| row.get(0), + ) + } } /// Persists a cross-node staging failure on behalf of a recovery claim. @@ -252,17 +254,18 @@ pub fn mark_cross_node_restore_staging_failed_under_recovery_claim( Ok(changed == 1) } -/// Load node-local recovery scheduling facts separately from quarantine -/// classification metadata. The observation covers the automatic-recovery -/// reason class, including owners introduced by a newer release; artifact -/// eligibility is published independently and remains owned by -/// `quarantine_recovery_branch`. -pub fn quarantine_recovery_observations( - conn: &Connection, - sandbox_ids: &[String], -) -> Result> { - let mut statement = conn.prepare_cached( - "SELECT qs.sandbox_id, qs.quarantine_id, qs.next_action_owner, +db_private_outside_tests! { + /// Load node-local recovery scheduling facts separately from quarantine + /// classification metadata. The observation covers the automatic-recovery + /// reason class, including owners introduced by a newer release; artifact + /// eligibility is published independently and remains owned by + /// `quarantine_recovery_branch`. + fn quarantine_recovery_observations( + conn: &Connection, + sandbox_ids: &[String], + ) -> Result> { + let mut statement = conn.prepare_cached( + "SELECT qs.sandbox_id, qs.quarantine_id, qs.next_action_owner, qs.attempt_count, qs.next_action_after, qs.reason_code FROM sandbox_quarantine_state qs JOIN sandboxes s ON s.id = qs.sandbox_id @@ -271,59 +274,60 @@ pub fn quarantine_recovery_observations( AND qs.source = ?2 AND s.state IN ('quarantined', 'restoring') AND cleanup.sandbox_id IS NULL", - )?; - let mut observations = BTreeMap::new(); - for sandbox_id in sandbox_ids { - let observation = statement - .query_row( - params![sandbox_id, SandboxQuarantineSource::SbxManager.as_str()], - |row| { - let reason_raw = row.get::<_, String>(5)?; - let reason = - reason_raw - .parse::() - .map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 5, - Type::Text, - Box::new(error), - ) - })?; - let observation = if reason.automatic_recovery_policy().is_some() { - Some(RawQuarantineRecoveryObservation { - local_quarantine_id: row.get(1)?, - owner: row.get(2)?, - attempt_count: row.get(3)?, - next_attempt_at: { - let value = row.get::<_, rusqlite::types::Value>(4)?; - let stored_type = value.data_type(); - nullable_stored_timestamp_text(value).ok_or_else(|| { - rusqlite::Error::InvalidColumnType( - 4, - "next_action_after".into(), - stored_type, + )?; + let mut observations = BTreeMap::new(); + for sandbox_id in sandbox_ids { + let observation = statement + .query_row( + params![sandbox_id, SandboxQuarantineSource::SbxManager.as_str()], + |row| { + let reason_raw = row.get::<_, String>(5)?; + let reason = + reason_raw + .parse::() + .map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 5, + Type::Text, + Box::new(error), ) - })? - }, - }) - } else { - None - }; - Ok((row.get::<_, String>(0)?, observation)) - }, - ) - .optional(); - match observation { - Ok(Some((id, Some(observation)))) => { - observations.insert(id, Ok(observation)); - } - Ok(Some((_, None)) | None) => {} - Err(error) => { - observations.insert(sandbox_id.clone(), Err(error.into())); + })?; + let observation = if reason.automatic_recovery_policy().is_some() { + Some(RawQuarantineRecoveryObservation { + local_quarantine_id: row.get(1)?, + owner: row.get(2)?, + attempt_count: row.get(3)?, + next_attempt_at: { + let value = row.get::<_, rusqlite::types::Value>(4)?; + let stored_type = value.data_type(); + nullable_stored_timestamp_text(value).ok_or_else(|| { + rusqlite::Error::InvalidColumnType( + 4, + "next_action_after".into(), + stored_type, + ) + })? + }, + }) + } else { + None + }; + Ok((row.get::<_, String>(0)?, observation)) + }, + ) + .optional(); + match observation { + Ok(Some((id, Some(observation)))) => { + observations.insert(id, Ok(observation)); + } + Ok(Some((_, None)) | None) => {} + Err(error) => { + observations.insert(sandbox_id.clone(), Err(error.into())); + } } } + Ok(observations) } - Ok(observations) } /// One scanned row of the legacy-recovery quarantine cohort. @@ -1087,29 +1091,31 @@ pub fn renew_quarantine_recovery_claim( Ok(changed == 1) } -pub fn quarantine_recovery_claim_is_current( - conn: &Connection, - claimed: &ClaimedQuarantineRecoveryCandidate, -) -> Result { - let fence = ExactClaimFence::new( - conn, - claimed, - now_epoch_i64()?, - LeaseRequirement::MustBeLive, - SandboxStateRequirement::MustBeActivatable, - CleanupRequirement::MustBeAbsent, - )?; - let sql = format!( - "SELECT EXISTS ( +db_private_outside_tests! { + fn quarantine_recovery_claim_is_current( + conn: &Connection, + claimed: &ClaimedQuarantineRecoveryCandidate, + ) -> Result { + let fence = ExactClaimFence::new( + conn, + claimed, + now_epoch_i64()?, + LeaseRequirement::MustBeLive, + SandboxStateRequirement::MustBeActivatable, + CleanupRequirement::MustBeAbsent, + )?; + let sql = format!( + "SELECT EXISTS ( SELECT 1 FROM sandbox_quarantine_state AS qs WHERE {}{} )", - fence.quarantine_row_predicate("qs."), - fence.sandbox_guards("qs.sandbox_id"), - ); - let mut statement = conn.prepare_cached(&sql)?; - Ok(statement.query_row(&fence.binds()[..], |row| row.get(0))?) + fence.quarantine_row_predicate("qs."), + fence.sandbox_guards("qs.sandbox_id"), + ); + let mut statement = conn.prepare_cached(&sql)?; + Ok(statement.query_row(&fence.binds()[..], |row| row.get(0))?) + } } fn quarantine_recovery_backoff(local_quarantine_id: uuid::Uuid, attempt_count: u32) -> Duration { diff --git a/crates/libs/sbx/mgr/store/src/db/quarantine_scan.rs b/crates/libs/sbx/mgr/store/src/db/quarantine_scan.rs index 0d469d43d..ab9b71d04 100644 --- a/crates/libs/sbx/mgr/store/src/db/quarantine_scan.rs +++ b/crates/libs/sbx/mgr/store/src/db/quarantine_scan.rs @@ -1,5 +1,6 @@ //! Finite quarantine discovery; page counts are not node-wide backlog samples. +use crate::db::db_private_outside_tests; use std::time::{Duration, Instant}; use crate::db::{Connection, Result}; @@ -60,13 +61,15 @@ impl PeriodicQuarantineRecoveryScan { }) } - pub fn next_batch(&mut self, conn: &Connection) -> Result { - let batch = self.read_batch(conn); - if batch.is_err() { - // Never complete a backlog sample after an unread section of a sweep. - *self = Self::default(); + db_private_outside_tests! { + fn next_batch(&mut self, conn: &Connection) -> Result { + let batch = self.read_batch(conn); + if batch.is_err() { + // Never complete a backlog sample after an unread section of a sweep. + *self = Self::default(); + } + batch } - batch } fn read_batch(&mut self, conn: &Connection) -> Result { diff --git a/crates/libs/sbx/mgr/store/src/db/restore_transfer_scopes.rs b/crates/libs/sbx/mgr/store/src/db/restore_transfer_scopes.rs index 366f6a2ad..04f34e962 100644 --- a/crates/libs/sbx/mgr/store/src/db/restore_transfer_scopes.rs +++ b/crates/libs/sbx/mgr/store/src/db/restore_transfer_scopes.rs @@ -49,46 +49,50 @@ pub(super) fn restore_transfer_scope_from_row( }) } -pub fn restore_transfer_scope( - conn: &Connection, - sandbox_id: &str, -) -> Result> { - conn.query_row( - "SELECT transfer_scope_id, execution_generation, activation_attempt_id, staging_dir +db_private_outside_tests! { + fn restore_transfer_scope( + conn: &Connection, + sandbox_id: &str, + ) -> Result> { + conn.query_row( + "SELECT transfer_scope_id, execution_generation, activation_attempt_id, staging_dir FROM sandbox_restore_transfer_scopes WHERE sandbox_id = ?1", - params![sandbox_id], - |row| restore_transfer_scope_from_row(row, 0), - ) - .optional() + params![sandbox_id], + |row| restore_transfer_scope_from_row(row, 0), + ) + .optional() + } } -pub fn rowless_restore_transfer_scopes( - conn: &Connection, -) -> Result> { - let mut statement = conn.prepare( - "SELECT scopes.sandbox_id, scopes.transfer_scope_id, +db_private_outside_tests! { + fn rowless_restore_transfer_scopes( + conn: &Connection, + ) -> Result> { + let mut statement = conn.prepare( + "SELECT scopes.sandbox_id, scopes.transfer_scope_id, scopes.execution_generation, scopes.activation_attempt_id, scopes.staging_dir FROM sandbox_restore_transfer_scopes AS scopes LEFT JOIN sandboxes ON sandboxes.id = scopes.sandbox_id WHERE sandboxes.id IS NULL", - )?; - let mut rows = statement.query([])?; - let mut scopes = Vec::new(); - while let Some(row) = rows.next()? { - let decoded = (|| Ok((row.get(0)?, restore_transfer_scope_from_row(row, 1)?)))(); - match decoded { - Ok(scope) => scopes.push(scope), - Err(error) if is_row_decode_error(&error) => tracing::warn!( - sandbox_id = ?row.get_ref(0)?, - error = %describe_raw_error(&error), - "retaining malformed rowless restore scope; operator repair required" - ), - Err(error) => return Err(error.into()), + )?; + let mut rows = statement.query([])?; + let mut scopes = Vec::new(); + while let Some(row) = rows.next()? { + let decoded = (|| Ok((row.get(0)?, restore_transfer_scope_from_row(row, 1)?)))(); + match decoded { + Ok(scope) => scopes.push(scope), + Err(error) if is_row_decode_error(&error) => tracing::warn!( + sandbox_id = ?row.get_ref(0)?, + error = %describe_raw_error(&error), + "retaining malformed rowless restore scope; operator repair required" + ), + Err(error) => return Err(error.into()), + } } + Ok(scopes) } - Ok(scopes) } pub fn retire_rowless_restore_transfer_scope( @@ -308,24 +312,26 @@ pub fn retire_restore_transfer_scope_if_matches( Ok(changed == 1) } -pub fn canonical_restore_reclaim_attempt_is_current( - conn: &Connection, - sandbox_id: &str, - transfer_scope_id: pplx_space_common_core::types::TransferScopeId, - execution_generation: ExecutionGeneration, - activation_attempt_id: SandboxActivationAttemptId, -) -> Result { - let Some(row) = get_sandbox(conn, sandbox_id)? else { - return Ok(false); - }; - if !sandbox_is_retryable_interrupted_restore(&row) { - return Ok(false); +db_private_outside_tests! { + fn canonical_restore_reclaim_attempt_is_current( + conn: &Connection, + sandbox_id: &str, + transfer_scope_id: pplx_space_common_core::types::TransferScopeId, + execution_generation: ExecutionGeneration, + activation_attempt_id: SandboxActivationAttemptId, + ) -> Result { + let Some(row) = get_sandbox(conn, sandbox_id)? else { + return Ok(false); + }; + if !sandbox_is_retryable_interrupted_restore(&row) { + return Ok(false); + } + Ok( + restore_transfer_scope(conn, sandbox_id)?.is_some_and(|scope| { + scope.transfer_scope_id == transfer_scope_id + && scope.execution_generation == Some(execution_generation) + && scope.activation_attempt_id == Some(activation_attempt_id) + }), + ) } - Ok( - restore_transfer_scope(conn, sandbox_id)?.is_some_and(|scope| { - scope.transfer_scope_id == transfer_scope_id - && scope.execution_generation == Some(execution_generation) - && scope.activation_attempt_id == Some(activation_attempt_id) - }), - ) } diff --git a/crates/libs/sbx/mgr/store/src/db/route_activation_claims.rs b/crates/libs/sbx/mgr/store/src/db/route_activation_claims.rs index f8c3a096c..01b1bfcd6 100644 --- a/crates/libs/sbx/mgr/store/src/db/route_activation_claims.rs +++ b/crates/libs/sbx/mgr/store/src/db/route_activation_claims.rs @@ -2,6 +2,7 @@ //! (`sandbox_open_route_activations`). use crate::db::Connection; +use crate::db::db_private_outside_tests; use pplx_space_common_core::types::{ ExecutionGeneration, NodeId, NodeManagedBy, PodUid, RouteGeneration, SandboxActivationAttemptId, SpaceApiRouteIdentity, @@ -93,13 +94,14 @@ pub(crate) fn insert_create_claim( write_claim(conn, sandbox_id, claim) } -pub fn claim_matches( - conn: &Connection, - sandbox_id: &str, - claim: &OpenRouteActivationClaim, -) -> crate::db::Result { - conn.query_row( - "SELECT EXISTS ( +db_private_outside_tests! { + fn claim_matches( + conn: &Connection, + sandbox_id: &str, + claim: &OpenRouteActivationClaim, + ) -> crate::db::Result { + conn.query_row( + "SELECT EXISTS ( SELECT 1 FROM sandbox_open_route_activations WHERE sandbox_id = ?1 @@ -111,17 +113,18 @@ pub fn claim_matches( AND activation_attempt_id = ?6 AND identity_provenance = ?7 )", - params![ - sandbox_id, - SqliteU64(claim.route_generation.get()), - claim.node_id.to_string(), - claim.execution_generation.get(), - claim.pod_uid.as_str(), - claim.activation_attempt_id.to_string(), - RouteIdentityProvenance::ClusterFenced.as_str(), - ], - |row| row.get(0), - ) + params![ + sandbox_id, + SqliteU64(claim.route_generation.get()), + claim.node_id.to_string(), + claim.execution_generation.get(), + claim.pod_uid.as_str(), + claim.activation_attempt_id.to_string(), + RouteIdentityProvenance::ClusterFenced.as_str(), + ], + |row| row.get(0), + ) + } } pub fn close_claim(conn: &Connection, sandbox_id: &str) -> crate::db::Result { diff --git a/crates/libs/sbx/mgr/store/src/db/sandboxes.rs b/crates/libs/sbx/mgr/store/src/db/sandboxes.rs index 78253c120..9eee0bcb4 100644 --- a/crates/libs/sbx/mgr/store/src/db/sandboxes.rs +++ b/crates/libs/sbx/mgr/store/src/db/sandboxes.rs @@ -10,18 +10,20 @@ pub struct RawSandboxLifecycle { pub state_changed_at: String, } -pub fn raw_sandbox_lifecycle(conn: &Connection, id: &str) -> Result> { - conn.query_row( - "SELECT state, state_changed_at FROM sandboxes WHERE id = ?1", - [id], - |row| { - Ok(RawSandboxLifecycle { - state: row.get(0)?, - state_changed_at: timestamps::diagnostic_column(row, 1)?, - }) - }, - ) - .optional() +db_private_outside_tests! { + fn raw_sandbox_lifecycle(conn: &Connection, id: &str) -> Result> { + conn.query_row( + "SELECT state, state_changed_at FROM sandboxes WHERE id = ?1", + [id], + |row| { + Ok(RawSandboxLifecycle { + state: row.get(0)?, + state_changed_at: timestamps::diagnostic_column(row, 1)?, + }) + }, + ) + .optional() + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -85,38 +87,42 @@ pub enum InsertRecoveredSuspendedSandboxError { ArtifactPinInvariant { changed: usize }, } -/// Count every durable sandbox-manager row by its bounded lifecycle. This stays -/// in SQL so the sampler's memory use is independent of the number of sandboxes -/// on the node. -pub fn sandbox_lifecycle_counts(conn: &Connection) -> Result> { - let mut statement = conn.prepare( - "SELECT state, COUNT(*) +db_private_outside_tests! { + /// Count every durable sandbox-manager row by its bounded lifecycle. This stays + /// in SQL so the sampler's memory use is independent of the number of sandboxes + /// on the node. + fn sandbox_lifecycle_counts(conn: &Connection) -> Result> { + let mut statement = conn.prepare( + "SELECT state, COUNT(*) FROM sandboxes GROUP BY state ORDER BY state", - )?; - let rows = statement.query_map([], |row| { - let state_label: String = row.get(0)?; - let state = state_label.parse::().map_err(|_| { - rusqlite::Error::FromSqlConversionFailure( - 0, - Type::Text, - format!("unknown sandbox state '{state_label}'").into(), - ) + )?; + let rows = statement.query_map([], |row| { + let state_label: String = row.get(0)?; + let state = state_label.parse::().map_err(|_| { + rusqlite::Error::FromSqlConversionFailure( + 0, + Type::Text, + format!("unknown sandbox state '{state_label}'").into(), + ) + })?; + let count = row.get::<_, SqliteU64>(1)?.0; + Ok(SandboxLifecycleCount { state, count }) })?; - let count = row.get::<_, SqliteU64>(1)?.0; - Ok(SandboxLifecycleCount { state, count }) - })?; - Ok(rows.collect::>()?) + Ok(rows.collect::>()?) + } } -pub fn current_state_label(conn: &Connection, id: &str) -> Result> { - conn.query_row( - "SELECT state FROM sandboxes WHERE id = ?1", - params![id], - |row| row.get(0), - ) - .optional() +db_private_outside_tests! { + fn current_state_label(conn: &Connection, id: &str) -> Result> { + conn.query_row( + "SELECT state FROM sandboxes WHERE id = ?1", + params![id], + |row| row.get(0), + ) + .optional() + } } pub fn record_remote_restore_artifact_staging(conn: &Connection, sandbox_id: &str) -> Result { @@ -247,46 +253,50 @@ pub fn insert_sandbox(conn: &Connection, params: InsertSandboxParams<'_>) -> Res Ok(()) } -pub fn node_execution_fence( - conn: &Connection, - sandbox_id: &str, -) -> Result> { - use pplx_space_node_mgr_client::execution_fence::ExecutionFence; - let (current, previous): (Option, Option) = conn.query_row( - "SELECT node_execution_id, previous_node_execution_id FROM sandboxes WHERE id = ?1", - params![sandbox_id], - |row| Ok((row.get(0)?, row.get(1)?)), - )?; - let parse = |raw: String| { - raw.parse::().map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 0, - rusqlite::types::Type::Text, - Box::new(error), - ) - }) - }; - match current { - Some(current) => Ok(Some(ExecutionFence::new( - parse(current)?, - previous.map(parse).transpose()?, - ))), - None if previous.is_none() => Ok(None), - None => Err(Error::invalid_query()), +db_private_outside_tests! { + fn node_execution_fence( + conn: &Connection, + sandbox_id: &str, + ) -> Result> { + use pplx_space_node_mgr_client::execution_fence::ExecutionFence; + let (current, previous): (Option, Option) = conn.query_row( + "SELECT node_execution_id, previous_node_execution_id FROM sandboxes WHERE id = ?1", + params![sandbox_id], + |row| Ok((row.get(0)?, row.get(1)?)), + )?; + let parse = |raw: String| { + raw.parse::().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 0, + rusqlite::types::Type::Text, + Box::new(error), + ) + }) + }; + match current { + Some(current) => Ok(Some(ExecutionFence::new( + parse(current)?, + previous.map(parse).transpose()?, + ))), + None if previous.is_none() => Ok(None), + None => Err(Error::invalid_query()), + } } } -/// Bounded startup preflight; legacy NULL rows require the adoption baseline. -pub fn unbound_node_executions(conn: &Connection) -> Result> { - let mut statement = conn - .prepare("SELECT id FROM sandboxes WHERE node_execution_id IS NULL ORDER BY id LIMIT ?1")?; - statement - .query_map( - [pplx_space_node_mgr_client::execution_fence::EXECUTION_PREFLIGHT_LIMIT as i64], - |row| row.get(0), - )? - .collect::>() - .map_err(Error::from) +db_private_outside_tests! { + /// Bounded startup preflight; legacy NULL rows require the adoption baseline. + fn unbound_node_executions(conn: &Connection) -> Result> { + let mut statement = conn + .prepare("SELECT id FROM sandboxes WHERE node_execution_id IS NULL ORDER BY id LIMIT ?1")?; + statement + .query_map( + [pplx_space_node_mgr_client::execution_fence::EXECUTION_PREFLIGHT_LIMIT as i64], + |row| row.get(0), + )? + .collect::>() + .map_err(Error::from) + } } /// Import receiver authority only while inserting a previously absent orphan @@ -643,65 +653,71 @@ pub fn startup_resident_witness_current(conn: &Connection, observed: &SandboxRow )?) } -pub fn sandbox_is_retryable_interrupted_restore(row: &SandboxRow) -> bool { - row.state == SandboxState::Restoring - && row - .restore_operation - .as_ref() - .is_some_and(|operation| operation.origin() == RestoreOrigin::Remote) - && !sandbox_has_runtime_fields(row) - && row.current_local_artifact_id.is_none() - && row.cleanup_completion.is_none() - && row.cleanup_keep_filesystem.is_none() +db_private_outside_tests! { + fn sandbox_is_retryable_interrupted_restore(row: &SandboxRow) -> bool { + row.state == SandboxState::Restoring + && row + .restore_operation + .as_ref() + .is_some_and(|operation| operation.origin() == RestoreOrigin::Remote) + && !sandbox_has_runtime_fields(row) + && row.current_local_artifact_id.is_none() + && row.cleanup_completion.is_none() + && row.cleanup_keep_filesystem.is_none() + } } -pub fn sandbox_state_changed_at( - conn: &Connection, - sandbox_ids: &[String], -) -> Result> { - if sandbox_ids.is_empty() { - return Ok(HashMap::new()); - } - let placeholders = std::iter::repeat_n("?", sandbox_ids.len()) - .collect::>() - .join(", "); - let query = format!( - "SELECT id, state_changed_at +db_private_outside_tests! { + fn sandbox_state_changed_at( + conn: &Connection, + sandbox_ids: &[String], + ) -> Result> { + if sandbox_ids.is_empty() { + return Ok(HashMap::new()); + } + let placeholders = std::iter::repeat_n("?", sandbox_ids.len()) + .collect::>() + .join(", "); + let query = format!( + "SELECT id, state_changed_at FROM sandboxes WHERE id IN ({placeholders})" - ); - let mut statement = conn.prepare(&query)?; - let rows = statement.query_map(rusqlite::params_from_iter(sandbox_ids), |row| { - Ok((row.get(0)?, timestamps::diagnostic_column(row, 1)?)) - })?; - Ok(rows.collect::>()?) + ); + let mut statement = conn.prepare(&query)?; + let rows = statement.query_map(rusqlite::params_from_iter(sandbox_ids), |row| { + Ok((row.get(0)?, timestamps::diagnostic_column(row, 1)?)) + })?; + Ok(rows.collect::>()?) + } } -/// Read one row's `state_changed_at` exactly as stored. -/// -/// Raw, unlike [`sandbox_state_changed_at`]'s diagnostic rendering, and for -/// the same reason [`SandboxRow::updated_at`] is raw: the value doubles as a -/// CAS witness, so it has to compare byte-for-byte against the column it came -/// from. Time-based decisions decode it strictly through -/// [`timestamps::EpochSeconds::parse`] instead of trusting the text. -/// -/// `None` means no such row. A non-text value is returned rendered rather -/// than rejected, so a corrupt timestamp reaches the caller's explicit -/// refusal path instead of looking like a missing sandbox. -pub fn sandbox_state_changed_at_witness(conn: &Connection, id: &str) -> Result> { - conn.query_row( - "SELECT state_changed_at FROM sandboxes WHERE id = ?1", - params![id], - |row| { - Ok(match row.get::<_, rusqlite::types::Value>(0)? { - rusqlite::types::Value::Text(raw) => raw, - rusqlite::types::Value::Integer(raw) => raw.to_string(), - rusqlite::types::Value::Real(raw) => raw.to_string(), - rusqlite::types::Value::Null | rusqlite::types::Value::Blob(_) => String::new(), - }) - }, - ) - .optional() +db_private_outside_tests! { + /// Read one row's `state_changed_at` exactly as stored. + /// + /// Raw, unlike [`sandbox_state_changed_at`]'s diagnostic rendering, and for + /// the same reason [`SandboxRow::updated_at`] is raw: the value doubles as a + /// CAS witness, so it has to compare byte-for-byte against the column it came + /// from. Time-based decisions decode it strictly through + /// [`timestamps::EpochSeconds::parse`] instead of trusting the text. + /// + /// `None` means no such row. A non-text value is returned rendered rather + /// than rejected, so a corrupt timestamp reaches the caller's explicit + /// refusal path instead of looking like a missing sandbox. + fn sandbox_state_changed_at_witness(conn: &Connection, id: &str) -> Result> { + conn.query_row( + "SELECT state_changed_at FROM sandboxes WHERE id = ?1", + params![id], + |row| { + Ok(match row.get::<_, rusqlite::types::Value>(0)? { + rusqlite::types::Value::Text(raw) => raw, + rusqlite::types::Value::Integer(raw) => raw.to_string(), + rusqlite::types::Value::Real(raw) => raw.to_string(), + rusqlite::types::Value::Null | rusqlite::types::Value::Blob(_) => String::new(), + }) + }, + ) + .optional() + } } pub fn update_uuid8(conn: &Connection, id: &str, uuid8: &SandboxRuntimeLabel) -> Result<()> { @@ -991,24 +1007,26 @@ fn reset_access_floor_for_execution_change( Ok(()) } -pub fn runtime_execution_generation( - conn: &Connection, - sandbox_id: &str, -) -> Result> { - let generation = conn - .query_row( - "SELECT space_api_route_execution_generation +db_private_outside_tests! { + fn runtime_execution_generation( + conn: &Connection, + sandbox_id: &str, + ) -> Result> { + let generation = conn + .query_row( + "SELECT space_api_route_execution_generation FROM sandboxes WHERE id = ?1", - params![sandbox_id], - |row| row.get::<_, Option>(0), - ) - .optional()? - .flatten(); - generation - .map(ExecutionGeneration::new) - .transpose() - .map_err(|error| Error::decode(0, Type::Integer, error)) + params![sandbox_id], + |row| row.get::<_, Option>(0), + ) + .optional()? + .flatten(); + generation + .map(ExecutionGeneration::new) + .transpose() + .map_err(|error| Error::decode(0, Type::Integer, error)) + } } #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] @@ -1081,12 +1099,13 @@ fn activation_route_provenance_from_row( }) } -pub fn activation_route_provenance( - conn: &Connection, - sandbox_id: &str, -) -> Result { - let values = conn.query_row( - "SELECT space_api_route_generation, +db_private_outside_tests! { + fn activation_route_provenance( + conn: &Connection, + sandbox_id: &str, + ) -> Result { + let values = conn.query_row( + "SELECT space_api_route_generation, space_api_route_execution_generation, space_api_route_substrate_kind, last_observed_route_generation, @@ -1095,24 +1114,26 @@ pub fn activation_route_provenance( last_observed_route_substrate_kind FROM sandboxes WHERE id = ?1", - params![sandbox_id], - |row| activation_route_provenance_from_row(row, 0), - )?; - Ok(values) + params![sandbox_id], + |row| activation_route_provenance_from_row(row, 0), + )?; + Ok(values) + } } -pub fn activation_route_provenance_for_sandboxes( - conn: &Connection, - sandbox_ids: &[String], -) -> Result>> { - if sandbox_ids.is_empty() { - return Ok(HashMap::new()); - } - let placeholders = std::iter::repeat_n("?", sandbox_ids.len()) - .collect::>() - .join(", "); - let query = format!( - "SELECT id, +db_private_outside_tests! { + fn activation_route_provenance_for_sandboxes( + conn: &Connection, + sandbox_ids: &[String], + ) -> Result>> { + if sandbox_ids.is_empty() { + return Ok(HashMap::new()); + } + let placeholders = std::iter::repeat_n("?", sandbox_ids.len()) + .collect::>() + .join(", "); + let query = format!( + "SELECT id, space_api_route_generation, space_api_route_execution_generation, space_api_route_substrate_kind, @@ -1122,15 +1143,16 @@ pub fn activation_route_provenance_for_sandboxes( last_observed_route_substrate_kind FROM sandboxes WHERE id IN ({placeholders})" - ); - let mut statement = conn.prepare(&query)?; - let rows = statement.query_map(rusqlite::params_from_iter(sandbox_ids), |row| { - Ok(( - row.get::<_, String>(0)?, - activation_route_provenance_from_row(row, 1).map_err(Error::from), - )) - })?; - Ok(rows.collect::>()?) + ); + let mut statement = conn.prepare(&query)?; + let rows = statement.query_map(rusqlite::params_from_iter(sandbox_ids), |row| { + Ok(( + row.get::<_, String>(0)?, + activation_route_provenance_from_row(row, 1).map_err(Error::from), + )) + })?; + Ok(rows.collect::>()?) + } } pub fn clear_runtime_fields(conn: &Connection, id: &str) -> Result<()> { @@ -1148,12 +1170,14 @@ pub fn clear_runtime_fields(conn: &Connection, id: &str) -> Result<()> { Ok(()) } -pub fn sandbox_exists(conn: &Connection, id: &str) -> Result { - conn.query_row( - sql!("SELECT EXISTS(SELECT 1 FROM sandboxes WHERE id = ?1)"), - [id], - |row| row.get(0), - ) +db_private_outside_tests! { + fn sandbox_exists(conn: &Connection, id: &str) -> Result { + conn.query_row( + sql!("SELECT EXISTS(SELECT 1 FROM sandboxes WHERE id = ?1)"), + [id], + |row| row.get(0), + ) + } } pub(super) const GET_SANDBOX_SQL: &str = @@ -1161,31 +1185,35 @@ pub(super) const GET_SANDBOX_SQL: &str = spaced_host_pid, netgtw_pid, vsock_uds_path, api_socket_path, created_at, updated_at, snapshots_enabled, rootfs_size_mb, pool_template, guest_uptime_secs, process_env_json, auto_lifecycle_json, auto_lifecycle_state_json, next_auto_event_at, failure_reason, cleanup_completion, cleanup_keep_filesystem, network_policy_override_json, network_tx_bytes, network_rx_bytes, network_policy_generation, applied_network_policy_generation, applied_network_policy_source, network_policy_authority, auto_lifecycle_authority, current_local_artifact_id, current_local_artifact_kind, current_local_artifact_origin, execution_metadata_json, volume_encryption, space_api_route_generation, space_api_route_node_id, space_api_route_execution_generation, space_api_route_pod_uid, restore_origin, restore_phase, restore_storage_uri, space_api_route_substrate_kind, space_api_route_activation_attempt_id, spaced_endpoint_proof_json, disk_io_max_operations_per_sec, disk_io_max_bandwidth_mib_per_sec, node_execution_id FROM sandboxes WHERE id = ?1"; -pub fn get_sandbox(conn: &Connection, id: &str) -> Result> { - // Every guest file operation and process start looks its sandbox up - // several times under the one database connection; parsing this wide - // SELECT each time dominated that work. - let mut stmt = conn.prepare_cached(GET_SANDBOX_SQL)?; - let mut rows = stmt.query_map(params![id], row_to_sandbox)?; - rows.next().transpose().map_err(Error::from) -} - -/// Select at most two identities: two matches prove ambiguity without decoding -/// unrelated or ambiguous sandbox metadata. Callers decode only a unique match. -pub fn find_by_prefix(conn: &Connection, prefix: &str) -> Result> { - let mut stmt = conn.prepare("SELECT id FROM sandboxes ORDER BY id")?; - let mut rows = stmt.query([])?; - let mut matches = Vec::new(); - while let Some(row) = rows.next()? { - let id = row.get::<_, String>(0)?; - if pplx_space_common_core::ids::sandbox_id_matches(&id, prefix) { - matches.push(id); - if matches.len() == 2 { - break; +db_private_outside_tests! { + fn get_sandbox(conn: &Connection, id: &str) -> Result> { + // Every guest file operation and process start looks its sandbox up + // several times under the one database connection; parsing this wide + // SELECT each time dominated that work. + let mut stmt = conn.prepare_cached(GET_SANDBOX_SQL)?; + let mut rows = stmt.query_map(params![id], row_to_sandbox)?; + rows.next().transpose().map_err(Error::from) + } +} + +db_private_outside_tests! { + /// Select at most two identities: two matches prove ambiguity without decoding + /// unrelated or ambiguous sandbox metadata. Callers decode only a unique match. + fn find_by_prefix(conn: &Connection, prefix: &str) -> Result> { + let mut stmt = conn.prepare("SELECT id FROM sandboxes ORDER BY id")?; + let mut rows = stmt.query([])?; + let mut matches = Vec::new(); + while let Some(row) = rows.next()? { + let id = row.get::<_, String>(0)?; + if pplx_space_common_core::ids::sandbox_id_matches(&id, prefix) { + matches.push(id); + if matches.len() == 2 { + break; + } } } + Ok(matches) } - Ok(matches) } const LIST_SANDBOXES_SQL: &str = @@ -1193,28 +1221,32 @@ const LIST_SANDBOXES_SQL: &str = spaced_host_pid, netgtw_pid, vsock_uds_path, api_socket_path, created_at, updated_at, snapshots_enabled, rootfs_size_mb, pool_template, guest_uptime_secs, process_env_json, auto_lifecycle_json, auto_lifecycle_state_json, next_auto_event_at, failure_reason, cleanup_completion, cleanup_keep_filesystem, network_policy_override_json, network_tx_bytes, network_rx_bytes, network_policy_generation, applied_network_policy_generation, applied_network_policy_source, network_policy_authority, auto_lifecycle_authority, current_local_artifact_id, current_local_artifact_kind, current_local_artifact_origin, execution_metadata_json, volume_encryption, space_api_route_generation, space_api_route_node_id, space_api_route_execution_generation, space_api_route_pod_uid, restore_origin, restore_phase, restore_storage_uri, space_api_route_substrate_kind, space_api_route_activation_attempt_id, spaced_endpoint_proof_json, disk_io_max_operations_per_sec, disk_io_max_bandwidth_mib_per_sec, node_execution_id FROM sandboxes ORDER BY created_at"; -pub fn list_sandboxes(conn: &Connection) -> Result> { - let mut stmt = conn.prepare(LIST_SANDBOXES_SQL)?; - let rows = stmt.query_map([], row_to_sandbox)?; - Ok(rows.collect::>()?) -} - -pub fn list_sandboxes_for_recovery(conn: &Connection) -> Result> { - let mut stmt = conn.prepare(LIST_SANDBOXES_SQL)?; - let mut rows = stmt.query([])?; - let mut valid = Vec::new(); - while let Some(row) = rows.next()? { - match row_to_sandbox(row) { - Ok(row) => valid.push(row), - Err(error) if is_row_decode_error(&error) => tracing::warn!( - sandbox_id = ?row.get_ref(0)?, - error = %describe_raw_error(&error), - "skipping undecodable sandbox row during local recovery" - ), - Err(error) => return Err(Error::from(error)), +db_private_outside_tests! { + fn list_sandboxes(conn: &Connection) -> Result> { + let mut stmt = conn.prepare(LIST_SANDBOXES_SQL)?; + let rows = stmt.query_map([], row_to_sandbox)?; + Ok(rows.collect::>()?) + } +} + +db_private_outside_tests! { + fn list_sandboxes_for_recovery(conn: &Connection) -> Result> { + let mut stmt = conn.prepare(LIST_SANDBOXES_SQL)?; + let mut rows = stmt.query([])?; + let mut valid = Vec::new(); + while let Some(row) = rows.next()? { + match row_to_sandbox(row) { + Ok(row) => valid.push(row), + Err(error) if is_row_decode_error(&error) => tracing::warn!( + sandbox_id = ?row.get_ref(0)?, + error = %describe_raw_error(&error), + "skipping undecodable sandbox row during local recovery" + ), + Err(error) => return Err(Error::from(error)), + } } + Ok(valid) } - Ok(valid) } /// A finite sweep: concurrent inserts are visited on the next sweep instead of @@ -1236,130 +1268,136 @@ impl Default for PeriodicLifecycleRecoveryScan { } } -/// Bounded discovery, not a complete inventory or authority to mutate a row. -/// Examine at most 128 raw rowids and return at most eight successfully decoded -/// candidates per tick. Failed decodes do not fill candidate slots, so eligible -/// malformed rows can cause up to 128 individual full-row decode attempts. -/// Advance past malformed/ineligible rows as well as candidates, so a retained -/// or busy first page cannot starve the rest. Rowid is only a scan position; -/// each consumer must revalidate the execution witness under its own fences. -pub fn periodic_lifecycle_recovery_batch( - conn: &Connection, - scan: &mut PeriodicLifecycleRecoveryScan, -) -> Result> { - if scan.through.is_none() { - scan.through = conn.query_row(sql!("SELECT MAX(rowid) FROM sandboxes"), [], |row| { - row.get(0) - })?; - } - let Some(through) = scan.through else { - return Ok(Vec::new()); - }; - let mut statement = conn.prepare(sql!( - "SELECT rowid, id, state, current_local_artifact_id FROM sandboxes - WHERE rowid >= ?1 AND rowid <= ?2 ORDER BY rowid LIMIT 128" - ))?; - let mut rows = statement.query([scan.next, through])?; - let mut candidates = Vec::new(); - for _ in 0..128 { - let Some(row) = rows.next()? else { - *scan = PeriodicLifecycleRecoveryScan::default(); - break; - }; - let rowid: i64 = row.get(0)?; - if rowid == through { - *scan = PeriodicLifecycleRecoveryScan::default(); - } else { - // The upper witness is greater, so this cannot overflow. - scan.next = rowid + 1; +db_private_outside_tests! { + /// Bounded discovery, not a complete inventory or authority to mutate a row. + /// Examine at most 128 raw rowids and return at most eight successfully decoded + /// candidates per tick. Failed decodes do not fill candidate slots, so eligible + /// malformed rows can cause up to 128 individual full-row decode attempts. + /// Advance past malformed/ineligible rows as well as candidates, so a retained + /// or busy first page cannot starve the rest. Rowid is only a scan position; + /// each consumer must revalidate the execution witness under its own fences. + fn periodic_lifecycle_recovery_batch( + conn: &Connection, + scan: &mut PeriodicLifecycleRecoveryScan, + ) -> Result> { + if scan.through.is_none() { + scan.through = conn.query_row(sql!("SELECT MAX(rowid) FROM sandboxes"), [], |row| { + row.get(0) + })?; } - let (id, state) = match (row.get::<_, String>(1), row.get::<_, String>(2)) { - (Ok(id), Ok(state)) => (id, state), - (Err(error), _) | (_, Err(error)) => { - tracing::warn!( - rowid, - error = %describe_raw_error(&error), - "skipping undecodable sandbox key/state during periodic recovery" - ); + let Some(through) = scan.through else { + return Ok(Vec::new()); + }; + let mut statement = conn.prepare(sql!( + "SELECT rowid, id, state, current_local_artifact_id FROM sandboxes + WHERE rowid >= ?1 AND rowid <= ?2 ORDER BY rowid LIMIT 128" + ))?; + let mut rows = statement.query([scan.next, through])?; + let mut candidates = Vec::new(); + for _ in 0..128 { + let Some(row) = rows.next()? else { + *scan = PeriodicLifecycleRecoveryScan::default(); + break; + }; + let rowid: i64 = row.get(0)?; + if rowid == through { + *scan = PeriodicLifecycleRecoveryScan::default(); + } else { + // The upper witness is greater, so this cannot overflow. + scan.next = rowid + 1; + } + let (id, state) = match (row.get::<_, String>(1), row.get::<_, String>(2)) { + (Ok(id), Ok(state)) => (id, state), + (Err(error), _) | (_, Err(error)) => { + tracing::warn!( + rowid, + error = %describe_raw_error(&error), + "skipping undecodable sandbox key/state during periodic recovery" + ); + continue; + } + }; + let eligible = match state.parse::() { + Ok(SandboxState::Paused | SandboxState::Resuming | SandboxState::Checkpointing) => true, + Ok(SandboxState::Suspending) => { + matches!(row.get_ref(3)?, rusqlite::types::ValueRef::Null) + } + _ => false, + }; + if !eligible { continue; } - }; - let eligible = match state.parse::() { - Ok(SandboxState::Paused | SandboxState::Resuming | SandboxState::Checkpointing) => true, - Ok(SandboxState::Suspending) => { - matches!(row.get_ref(3)?, rusqlite::types::ValueRef::Null) + match get_sandbox(conn, &id) { + Ok(Some(row)) => candidates.push(row), + Ok(None) => {} + Err(error) => tracing::warn!( + sandbox_id = %id, + error = %describe_error(&error), + "skipping undecodable sandbox row during periodic recovery" + ), + } + if candidates.len() == 8 { + break; } - _ => false, - }; - if !eligible { - continue; - } - match get_sandbox(conn, &id) { - Ok(Some(row)) => candidates.push(row), - Ok(None) => {} - Err(error) => tracing::warn!( - sandbox_id = %id, - error = %describe_error(&error), - "skipping undecodable sandbox row during periodic recovery" - ), - } - if candidates.len() == 8 { - break; } + Ok(candidates) } - Ok(candidates) } -/// Host-control net-gateway auth lookup: rows whose registered -/// `(netgtw_pid, uid)` matches the caller's peer credentials. -/// -/// Restricted to states where a live gateway can legitimately exist -/// (`SandboxState::can_authorize_net_gateway`): launch-failure and other -/// terminal rows retain the `(uid, netgtw_pid)` of dead processes, and since -/// uids are slot-derived and pids recycle, an unfiltered match would let a -/// stale row shadow — or be served in place of — a later sandbox's gateway. -/// The state list below must stay in sync with that predicate; the -/// per-state lookup test cross-checks them. -pub fn list_by_netgtw_pid_uid( - conn: &Connection, - netgtw_pid: u32, - uid: u32, -) -> Result> { - let mut stmt = conn.prepare( - "SELECT id, uuid8, state, template, template_hash, user, runtime, uid, slot, guest_cid, pid, +db_private_outside_tests! { + /// Host-control net-gateway auth lookup: rows whose registered + /// `(netgtw_pid, uid)` matches the caller's peer credentials. + /// + /// Restricted to states where a live gateway can legitimately exist + /// (`SandboxState::can_authorize_net_gateway`): launch-failure and other + /// terminal rows retain the `(uid, netgtw_pid)` of dead processes, and since + /// uids are slot-derived and pids recycle, an unfiltered match would let a + /// stale row shadow — or be served in place of — a later sandbox's gateway. + /// The state list below must stay in sync with that predicate; the + /// per-state lookup test cross-checks them. + fn list_by_netgtw_pid_uid( + conn: &Connection, + netgtw_pid: u32, + uid: u32, + ) -> Result> { + let mut stmt = conn.prepare( + "SELECT id, uuid8, state, template, template_hash, user, runtime, uid, slot, guest_cid, pid, spaced_host_pid, netgtw_pid, vsock_uds_path, api_socket_path, created_at, updated_at, snapshots_enabled, rootfs_size_mb, pool_template, guest_uptime_secs, process_env_json, auto_lifecycle_json, auto_lifecycle_state_json, next_auto_event_at, failure_reason, cleanup_completion, cleanup_keep_filesystem, network_policy_override_json, network_tx_bytes, network_rx_bytes, network_policy_generation, applied_network_policy_generation, applied_network_policy_source, network_policy_authority, auto_lifecycle_authority, current_local_artifact_id, current_local_artifact_kind, current_local_artifact_origin, execution_metadata_json, volume_encryption, space_api_route_generation, space_api_route_node_id, space_api_route_execution_generation, space_api_route_pod_uid, restore_origin, restore_phase, restore_storage_uri, space_api_route_substrate_kind, space_api_route_activation_attempt_id, spaced_endpoint_proof_json, disk_io_max_operations_per_sec, disk_io_max_bandwidth_mib_per_sec, node_execution_id FROM sandboxes WHERE netgtw_pid = ?1 AND uid = ?2 AND state IN ('starting', 'started', 'stopping', 'pausing', 'paused', 'resuming', 'checkpointing', 'suspending', 'restoring') LIMIT 2", - )?; - let rows = stmt.query_map(params![netgtw_pid, uid], row_to_sandbox)?; - Ok(rows.collect::>()?) + )?; + let rows = stmt.query_map(params![netgtw_pid, uid], row_to_sandbox)?; + Ok(rows.collect::>()?) + } } -/// Host-control spaced-host auth lookup: rows whose registered -/// `(spaced_host_pid, uid)` matches the caller's peer credentials. -pub fn list_by_spaced_host_pid_uid( - conn: &Connection, - spaced_host_pid: u32, - uid: u32, -) -> Result> { - let mut stmt = conn.prepare( - "SELECT id +db_private_outside_tests! { + /// Host-control spaced-host auth lookup: rows whose registered + /// `(spaced_host_pid, uid)` matches the caller's peer credentials. + fn list_by_spaced_host_pid_uid( + conn: &Connection, + spaced_host_pid: u32, + uid: u32, + ) -> Result> { + let mut stmt = conn.prepare( + "SELECT id FROM sandboxes WHERE spaced_host_pid = ?1 AND uid = ?2 LIMIT 2", - )?; - let sandbox_ids = stmt - .query_map(params![spaced_host_pid, uid], |row| row.get::<_, String>(0))? - .collect::>>()?; - drop(stmt); - - sandbox_ids - .into_iter() - .map(|sandbox_id| get_sandbox(conn, &sandbox_id)?.ok_or(Error::no_rows())) - .collect() + )?; + let sandbox_ids = stmt + .query_map(params![spaced_host_pid, uid], |row| row.get::<_, String>(0))? + .collect::>>()?; + drop(stmt); + + sandbox_ids + .into_iter() + .map(|sandbox_id| get_sandbox(conn, &sandbox_id)?.ok_or(Error::no_rows())) + .collect() + } } #[cfg(any(test, feature = "test-support"))] @@ -1474,15 +1512,17 @@ pub fn delete_sandbox_if_inventory_witness_matches( Ok(InventoryStaleRowMutation::Removed) } -pub fn sandbox_has_runtime_fields(row: &SandboxRow) -> bool { - row.uid.is_some() - || row.slot.is_some() - || row.guest_cid.is_some() - || row.pid.is_some() - || row.spaced_host_pid.is_some() - || row.netgtw_pid.is_some() - || row.vsock_uds_path.is_some() - || row.api_socket_path.is_some() +db_private_outside_tests! { + fn sandbox_has_runtime_fields(row: &SandboxRow) -> bool { + row.uid.is_some() + || row.slot.is_some() + || row.guest_cid.is_some() + || row.pid.is_some() + || row.spaced_host_pid.is_some() + || row.netgtw_pid.is_some() + || row.vsock_uds_path.is_some() + || row.api_socket_path.is_some() + } } pub(super) fn sandbox_has_full_suspend_artifact_pin(row: &SandboxRow) -> bool { diff --git a/crates/libs/sbx/mgr/store/src/db/snapshot_gc.rs b/crates/libs/sbx/mgr/store/src/db/snapshot_gc.rs index 4e5f2f3c1..013ddce97 100644 --- a/crates/libs/sbx/mgr/store/src/db/snapshot_gc.rs +++ b/crates/libs/sbx/mgr/store/src/db/snapshot_gc.rs @@ -1,5 +1,6 @@ //! Bounded, progressing artifact discovery; deletion authority stays with its lease. +use crate::db::db_private_outside_tests; use crate::db::{Connection, Result}; use pplx_space_common_core::inventory::{InventoryBudget, MAX_RAW_BYTES}; use pplx_space_common_core::sql; @@ -61,86 +62,88 @@ const PAGE_SQL: &str = sql!( ); impl ArtifactScan { - pub fn next_page( - &mut self, - conn: &Connection, - ) -> std::result::Result { - let through = match self.through { - Some(through) => through, - None => { - let maximum: Option = - conn.query_row(sql!("SELECT MAX(rowid) FROM local_artifacts"), [], |row| { - row.get(0) - })?; - let Some(maximum) = maximum else { - return Ok(ArtifactPage::default()); - }; - maximum - } - }; - let mut statement = conn.prepare_cached(PAGE_SQL)?; - let mut rows = statement.query(params![self.next, through, PAGE_SIZE as i64])?; - let mut page = ArtifactPage::default(); - let mut last = None; - let mut budget = InventoryBudget::new(PAGE_SIZE, MAX_RAW_BYTES); - let mut page_full = false; - while let Some(row) = rows.next()? { - let rowid: i64 = row.get(0)?; - // Oversize evidence is retained without allocation and does not - // prevent later rows progressing. A row that fits alone but not - // this page is retried with a fresh budget on the next tick. - match check_row(row, &mut InventoryBudget::new(1, MAX_RAW_BYTES)) { - Ok(()) => {} - Err(InventoryReadError::Limit(_)) => { - page.examined += 1; - last = Some(rowid); - tracing::warn!( - rowid, - "snapshot reaper: retaining oversized artifact observation" - ); - continue; + db_private_outside_tests! { + fn next_page( + &mut self, + conn: &Connection, + ) -> std::result::Result { + let through = match self.through { + Some(through) => through, + None => { + let maximum: Option = + conn.query_row(sql!("SELECT MAX(rowid) FROM local_artifacts"), [], |row| { + row.get(0) + })?; + let Some(maximum) = maximum else { + return Ok(ArtifactPage::default()); + }; + maximum } - Err(error) => return Err(error), - } - match check_row(row, &mut budget) { - Ok(()) => {} - Err(InventoryReadError::Limit(_)) => { - page_full = true; - break; + }; + let mut statement = conn.prepare_cached(PAGE_SQL)?; + let mut rows = statement.query(params![self.next, through, PAGE_SIZE as i64])?; + let mut page = ArtifactPage::default(); + let mut last = None; + let mut budget = InventoryBudget::new(PAGE_SIZE, MAX_RAW_BYTES); + let mut page_full = false; + while let Some(row) = rows.next()? { + let rowid: i64 = row.get(0)?; + // Oversize evidence is retained without allocation and does not + // prevent later rows progressing. A row that fits alone but not + // this page is retried with a fresh budget on the next tick. + match check_row(row, &mut InventoryBudget::new(1, MAX_RAW_BYTES)) { + Ok(()) => {} + Err(InventoryReadError::Limit(_)) => { + page.examined += 1; + last = Some(rowid); + tracing::warn!( + rowid, + "snapshot reaper: retaining oversized artifact observation" + ); + continue; + } + Err(error) => return Err(error), } - Err(error) => return Err(error), - } - page.examined += 1; - last = Some(rowid); - let observation = (|| -> Result { - Ok(ArtifactObservation { - local_artifact_id: row.get(1)?, - sandbox_id: row.get(2)?, - artifact_origin: row.get(3)?, - artifact_kind: row.get(4)?, - local_path: row.get(5)?, - created_at_secs: row.get::<_, EpochSeconds>(6)?.unsigned(), - last_touched_at_secs: row.get::<_, EpochSeconds>(7)?.unsigned(), - backup_completed: row.get(8)?, - is_ready_state: row.get(9)?, - pinned_current: row.get(10)?, - }) - })(); - match observation { - Ok(observation) => page.artifacts.push(observation), - Err(error) => { - tracing::warn!(rowid, %error, "snapshot reaper: retaining malformed artifact observation") + match check_row(row, &mut budget) { + Ok(()) => {} + Err(InventoryReadError::Limit(_)) => { + page_full = true; + break; + } + Err(error) => return Err(error), + } + page.examined += 1; + last = Some(rowid); + let observation = (|| -> Result { + Ok(ArtifactObservation { + local_artifact_id: row.get(1)?, + sandbox_id: row.get(2)?, + artifact_origin: row.get(3)?, + artifact_kind: row.get(4)?, + local_path: row.get(5)?, + created_at_secs: row.get::<_, EpochSeconds>(6)?.unsigned(), + last_touched_at_secs: row.get::<_, EpochSeconds>(7)?.unsigned(), + backup_completed: row.get(8)?, + is_ready_state: row.get(9)?, + pinned_current: row.get(10)?, + }) + })(); + match observation { + Ok(observation) => page.artifacts.push(observation), + Err(error) => { + tracing::warn!(rowid, %error, "snapshot reaper: retaining malformed artifact observation") + } } } + *self = match last { + Some(last) if (page.examined == PAGE_SIZE || page_full) && last < through => Self { + next: last + 1, + through: Some(through), + }, + _ => Self::default(), + }; + Ok(page) } - *self = match last { - Some(last) if (page.examined == PAGE_SIZE || page_full) && last < through => Self { - next: last + 1, - through: Some(through), - }, - _ => Self::default(), - }; - Ok(page) } } diff --git a/crates/libs/sbx/mgr/store/src/db/startup_recovery.rs b/crates/libs/sbx/mgr/store/src/db/startup_recovery.rs index d23056fc0..e5ebaea63 100644 --- a/crates/libs/sbx/mgr/store/src/db/startup_recovery.rs +++ b/crates/libs/sbx/mgr/store/src/db/startup_recovery.rs @@ -165,38 +165,40 @@ pub fn settle_startup_resident( Ok(true) } -pub fn cleanup_intent_for_startup_recovery(row: &SandboxRow) -> Option<(bool, CleanupCompletion)> { - if let (Some(keep_filesystem), Some(completion)) = - (row.cleanup_keep_filesystem, row.cleanup_completion) - && completion.owns_row_intent(row) - { - return Some((keep_filesystem, completion)); - } - - match row.state { - // Transient cleanup states represent an unfinished lifecycle operation. - // Finish them even if runtime fields were already cleared when the - // target terminal state is self-contained. Suspending is stricter: - // recovery may only mark it suspended once a full suspend artifact is - // already pinned; otherwise service-level startup recovery must either - // finish a complete staged publish or quarantine the row. - SandboxState::Stopping => Some((true, CleanupCompletion::MarkStopped)), - SandboxState::Suspending if sandbox_has_full_suspend_artifact_pin(row) => { - Some((true, CleanupCompletion::MarkSuspended)) - } - SandboxState::Suspending => None, - SandboxState::Deleting => Some((false, CleanupCompletion::DeleteRow)), - SandboxState::CleanupRequired => Some((true, CleanupCompletion::Quarantine)), - SandboxState::Starting if sandbox_has_runtime_fields(row) => { - Some((true, CleanupCompletion::MarkStopped)) - } - SandboxState::Restoring if sandbox_has_runtime_fields(row) => { - Some((true, CleanupCompletion::MarkSuspended)) +db_private_outside_tests! { + fn cleanup_intent_for_startup_recovery(row: &SandboxRow) -> Option<(bool, CleanupCompletion)> { + if let (Some(keep_filesystem), Some(completion)) = + (row.cleanup_keep_filesystem, row.cleanup_completion) + && completion.owns_row_intent(row) + { + return Some((keep_filesystem, completion)); } - SandboxState::CleanupFailed if sandbox_has_runtime_fields(row) => { - Some((true, CleanupCompletion::Quarantine)) + + match row.state { + // Transient cleanup states represent an unfinished lifecycle operation. + // Finish them even if runtime fields were already cleared when the + // target terminal state is self-contained. Suspending is stricter: + // recovery may only mark it suspended once a full suspend artifact is + // already pinned; otherwise service-level startup recovery must either + // finish a complete staged publish or quarantine the row. + SandboxState::Stopping => Some((true, CleanupCompletion::MarkStopped)), + SandboxState::Suspending if sandbox_has_full_suspend_artifact_pin(row) => { + Some((true, CleanupCompletion::MarkSuspended)) + } + SandboxState::Suspending => None, + SandboxState::Deleting => Some((false, CleanupCompletion::DeleteRow)), + SandboxState::CleanupRequired => Some((true, CleanupCompletion::Quarantine)), + SandboxState::Starting if sandbox_has_runtime_fields(row) => { + Some((true, CleanupCompletion::MarkStopped)) + } + SandboxState::Restoring if sandbox_has_runtime_fields(row) => { + Some((true, CleanupCompletion::MarkSuspended)) + } + SandboxState::CleanupFailed if sandbox_has_runtime_fields(row) => { + Some((true, CleanupCompletion::Quarantine)) + } + _ => None, } - _ => None, } } diff --git a/crates/libs/sbx/mgr/store/src/db/teardown_jobs.rs b/crates/libs/sbx/mgr/store/src/db/teardown_jobs.rs index 6aed20bb5..4ed080245 100644 --- a/crates/libs/sbx/mgr/store/src/db/teardown_jobs.rs +++ b/crates/libs/sbx/mgr/store/src/db/teardown_jobs.rs @@ -5,7 +5,7 @@ //! proves runtime release. Periodic reconciliation closes at most 128 terminal //! active teardown jobs per tick, excluding unrelated and retained job history. -use super::{active_teardown_suspend_operation, immediate_transaction}; +use super::{active_teardown_suspend_operation, db_private_outside_tests, immediate_transaction}; use crate::db::{Connection, Result}; use pplx_space_common_core::{ sql, @@ -46,16 +46,17 @@ impl std::fmt::Display for TeardownSuspendFailure { } } -/// A recovered resident may start a fresh attempt; retained artifacts and -/// in-flight cleanup cannot supply that authority, even for the same generation. -pub fn teardown_suspend_retry_admissible( - conn: &Connection, - id: &str, - generation: ExecutionGeneration, -) -> Result { - conn.query_row( - sql!( - "SELECT EXISTS ( +db_private_outside_tests! { + /// A recovered resident may start a fresh attempt; retained artifacts and + /// in-flight cleanup cannot supply that authority, even for the same generation. + fn teardown_suspend_retry_admissible( + conn: &Connection, + id: &str, + generation: ExecutionGeneration, + ) -> Result { + conn.query_row( + sql!( + "SELECT EXISTS ( SELECT 1 FROM sandboxes s WHERE s.id = ?1 AND s.state IN ('started', 'paused') AND s.space_api_route_execution_generation = ?2 @@ -64,10 +65,11 @@ pub fn teardown_suspend_retry_admissible( SELECT 1 FROM sandbox_cleanup_episodes e WHERE e.sandbox_id = s.id ) )" - ), - params![id, generation.get()], - |row| row.get(0), - ) + ), + params![id, generation.get()], + |row| row.get(0), + ) + } } pub fn fail_teardown_suspend_operation( @@ -220,23 +222,25 @@ pub fn defer_teardown_suspend_retry( Ok(()) } -pub fn teardown_suspend_retry_due( - conn: &Connection, - id: &str, - generation: ExecutionGeneration, -) -> Result { - let Some(operation) = active_teardown_suspend_operation(conn, id)? else { - return Ok(true); - }; - if operation.execution_generation != generation { - return Ok(true); - } - conn.query_row( - sql!( - "SELECT next_attempt_at IS NULL OR next_attempt_at <= datetime('now') +db_private_outside_tests! { + fn teardown_suspend_retry_due( + conn: &Connection, + id: &str, + generation: ExecutionGeneration, + ) -> Result { + let Some(operation) = active_teardown_suspend_operation(conn, id)? else { + return Ok(true); + }; + if operation.execution_generation != generation { + return Ok(true); + } + conn.query_row( + sql!( + "SELECT next_attempt_at IS NULL OR next_attempt_at <= datetime('now') FROM jobs WHERE job_id = ?1" - ), - params![operation.job_id], - |row| row.get(0), - ) + ), + params![operation.job_id], + |row| row.get(0), + ) + } } diff --git a/crates/libs/sbx/mgr/store/src/db/teardown_suspend.rs b/crates/libs/sbx/mgr/store/src/db/teardown_suspend.rs index f9df1b63e..ccee8d807 100644 --- a/crates/libs/sbx/mgr/store/src/db/teardown_suspend.rs +++ b/crates/libs/sbx/mgr/store/src/db/teardown_suspend.rs @@ -374,21 +374,23 @@ pub(super) fn active_teardown_suspend_operation( .map_err(Error::from) } -pub fn latest_teardown_suspend_operation( - conn: &Connection, - sandbox_id: &str, -) -> Result> { - conn.query_row( - "SELECT job_id, payload, status +db_private_outside_tests! { + fn latest_teardown_suspend_operation( + conn: &Connection, + sandbox_id: &str, + ) -> Result> { + conn.query_row( + "SELECT job_id, payload, status FROM jobs WHERE kind = ?1 AND subject_id = ?2 ORDER BY created_at DESC, job_id DESC LIMIT 1", - params![TEARDOWN_SUSPEND_JOB_KIND, sandbox_id], - |row| parse_teardown_suspend_operation(row.get(0)?, row.get(1)?, row.get(2)?), - ) - .optional() + params![TEARDOWN_SUSPEND_JOB_KIND, sandbox_id], + |row| parse_teardown_suspend_operation(row.get(0)?, row.get(1)?, row.get(2)?), + ) + .optional() + } } fn parse_teardown_suspend_operation( @@ -417,41 +419,43 @@ pub struct TeardownReleaseEvidence { pub activation_rolled_back: bool, } -/// Release evidence must describe one SQLite snapshot: a historical rollback -/// cannot be combined with ownership from before a replacement activation. -pub fn teardown_release_evidence( - conn: &Connection, - sandbox_id: &str, - execution_generation: ExecutionGeneration, -) -> Result { - let tx = Transaction::deferred(conn)?; - let runtime_generation = runtime_execution_generation(&tx, sandbox_id)?; - let suspend = teardown_suspend_status(&tx, sandbox_id)?; - let deleted = execution_delete_receipt_covers(&tx, sandbox_id, execution_generation)?; - let activation_rolled_back = match &suspend { - Some(status) - if runtime_generation.is_none() - && matches!(status.state, SandboxState::Created | SandboxState::Stopped) - && status.operation.as_ref().is_none_or(|operation| { - operation.status == TeardownSuspendOperationStatus::Completed - }) => - { - completed_activation_rollback_covers( - &tx, - sandbox_id, - status.state, - execution_generation, - )? - } - _ => false, - }; - tx.commit()?; - Ok(TeardownReleaseEvidence { - runtime_generation, - suspend, - deleted, - activation_rolled_back, - }) +db_private_outside_tests! { + /// Release evidence must describe one SQLite snapshot: a historical rollback + /// cannot be combined with ownership from before a replacement activation. + fn teardown_release_evidence( + conn: &Connection, + sandbox_id: &str, + execution_generation: ExecutionGeneration, + ) -> Result { + let tx = Transaction::deferred(conn)?; + let runtime_generation = runtime_execution_generation(&tx, sandbox_id)?; + let suspend = teardown_suspend_status(&tx, sandbox_id)?; + let deleted = execution_delete_receipt_covers(&tx, sandbox_id, execution_generation)?; + let activation_rolled_back = match &suspend { + Some(status) + if runtime_generation.is_none() + && matches!(status.state, SandboxState::Created | SandboxState::Stopped) + && status.operation.as_ref().is_none_or(|operation| { + operation.status == TeardownSuspendOperationStatus::Completed + }) => + { + completed_activation_rollback_covers( + &tx, + sandbox_id, + status.state, + execution_generation, + )? + } + _ => false, + }; + tx.commit()?; + Ok(TeardownReleaseEvidence { + runtime_generation, + suspend, + deleted, + activation_rolled_back, + }) + } } fn completed_activation_rollback_covers( @@ -501,13 +505,14 @@ fn completed_activation_rollback_covers( })) } -pub fn teardown_suspend_status( - conn: &Connection, - sandbox_id: &str, -) -> Result> { - let status = conn - .query_row( - "SELECT s.state, +db_private_outside_tests! { + fn teardown_suspend_status( + conn: &Connection, + sandbox_id: &str, + ) -> Result> { + let status = conn + .query_row( + "SELECT s.state, s.current_local_artifact_id, s.current_local_artifact_kind, s.current_local_artifact_origin, @@ -518,103 +523,104 @@ pub fn teardown_suspend_status( ON a.local_artifact_id = s.current_local_artifact_id AND a.sandbox_id = s.id WHERE s.id = ?1", - params![sandbox_id], - |row| { - let state_text = row.get::<_, String>(0)?; - let state = state_text.parse::().map_err(|_| { - rusqlite::Error::FromSqlConversionFailure( - 0, - Type::Text, - Box::new(std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!("unknown sandbox state {state_text:?}"), - )), - ) - })?; - let local_artifact_id = row - .get::<_, Option>(1)? - .map(|value| { - LocalArtifactId::from_str(&value).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 1, - Type::Text, - Box::new(error), - ) - }) - }) - .transpose()?; - let artifact_kind = row - .get::<_, Option>(2)? - .map(|value| { - ArtifactKind::from_str(&value).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 2, - Type::Text, - Box::new(error), - ) - }) - }) - .transpose()?; - let artifact_origin = row - .get::<_, Option>(3)? - .map(|value| { - ArtifactOrigin::from_str(&value).map_err(|error| { - rusqlite::Error::FromSqlConversionFailure( - 3, - Type::Text, - Box::new(error), - ) - }) - }) - .transpose()?; - let artifact_path = row.get::<_, Option>(4)?; - let backup_completed = row.get::<_, bool>(5)?; - - let artifact = match ( - local_artifact_id, - artifact_kind, - artifact_origin, - artifact_path, - ) { - (None, None, None, None) => None, - ( - Some(local_artifact_id), - Some(ArtifactKind::Full), - Some(ArtifactOrigin::Suspend), - Some(local_path), - ) => Some(LocalArtifactRow { - local_artifact_id, - sandbox_id: sandbox_id.to_string(), - artifact_kind: ArtifactKind::Full, - artifact_origin: ArtifactOrigin::Suspend, - snapshot_name: None, - local_path, - }), - _ => { - return Err(rusqlite::Error::FromSqlConversionFailure( - 1, + params![sandbox_id], + |row| { + let state_text = row.get::<_, String>(0)?; + let state = state_text.parse::().map_err(|_| { + rusqlite::Error::FromSqlConversionFailure( + 0, Type::Text, Box::new(std::io::Error::new( std::io::ErrorKind::InvalidData, - "sandbox suspend artifact pin is incomplete or invalid", + format!("unknown sandbox state {state_text:?}"), )), - )); - } - }; + ) + })?; + let local_artifact_id = row + .get::<_, Option>(1)? + .map(|value| { + LocalArtifactId::from_str(&value).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 1, + Type::Text, + Box::new(error), + ) + }) + }) + .transpose()?; + let artifact_kind = row + .get::<_, Option>(2)? + .map(|value| { + ArtifactKind::from_str(&value).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 2, + Type::Text, + Box::new(error), + ) + }) + }) + .transpose()?; + let artifact_origin = row + .get::<_, Option>(3)? + .map(|value| { + ArtifactOrigin::from_str(&value).map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + 3, + Type::Text, + Box::new(error), + ) + }) + }) + .transpose()?; + let artifact_path = row.get::<_, Option>(4)?; + let backup_completed = row.get::<_, bool>(5)?; - Ok((state, artifact, backup_completed)) - }, - ) - .optional()?; - let Some((state, artifact, backup_completed)) = status else { - return Ok(None); - }; - Ok(Some(TeardownSuspendStatus { - state, - operation: latest_teardown_suspend_operation(conn, sandbox_id)?, - artifact, - backup_completed, - })) + let artifact = match ( + local_artifact_id, + artifact_kind, + artifact_origin, + artifact_path, + ) { + (None, None, None, None) => None, + ( + Some(local_artifact_id), + Some(ArtifactKind::Full), + Some(ArtifactOrigin::Suspend), + Some(local_path), + ) => Some(LocalArtifactRow { + local_artifact_id, + sandbox_id: sandbox_id.to_string(), + artifact_kind: ArtifactKind::Full, + artifact_origin: ArtifactOrigin::Suspend, + snapshot_name: None, + local_path, + }), + _ => { + return Err(rusqlite::Error::FromSqlConversionFailure( + 1, + Type::Text, + Box::new(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "sandbox suspend artifact pin is incomplete or invalid", + )), + )); + } + }; + + Ok((state, artifact, backup_completed)) + }, + ) + .optional()?; + let Some((state, artifact, backup_completed)) = status else { + return Ok(None); + }; + Ok(Some(TeardownSuspendStatus { + state, + operation: latest_teardown_suspend_operation(conn, sandbox_id)?, + artifact, + backup_completed, + })) + } } pub fn complete_teardown_suspend(