From a5bd46a461f00df724a6ff778da326528b436ff2 Mon Sep 17 00:00:00 2001 From: Phoebe Goldman Date: Wed, 1 Jul 2026 13:25:40 -0400 Subject: [PATCH] Scaffold `ponder_manual_migrate` and re-work call chain that reaches it --- Cargo.lock | 1 + crates/client-api/src/routes/database.rs | 4 +- crates/core/src/host/host_controller.rs | 9 ++--- crates/core/src/host/module_host.rs | 5 +-- .../src/host/wasm_common/module_host_actor.rs | 4 +- crates/schema/Cargo.toml | 1 + crates/schema/src/def.rs | 20 +++++++++- crates/schema/src/migrate.rs | 40 +++++++++++++++---- crates/schema/src/migrate/auto_migrate.rs | 2 + crates/schema/src/migrate/manual_migrate.rs | 24 +++++++++-- 10 files changed, 86 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e224012f0..d54e70ca7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8737,6 +8737,7 @@ name = "spacetimedb-schema" version = "2.7.0" dependencies = [ "anyhow", + "blake3", "convert_case 0.6.0", "derive_more 0.99.20", "enum-as-inner", diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 7acfbeded4..bd561c88c0 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -1060,7 +1060,7 @@ pub async fn publish( }) }; match maybe_updated { - Some(UpdateDatabaseResult::AutoMigrateError(errs)) => { + Some(UpdateDatabaseResult::MigrationPlanningError(errs)) => { Err(bad_request(format!("Database update rejected: {errs}").into())) } Some(UpdateDatabaseResult::ErrorExecutingMigration(err)) => Err(bad_request( @@ -1282,7 +1282,7 @@ pub async fn pre_publish major_version_upgrade, })) } - MigratePlanResult::AutoMigrationError { + MigratePlanResult::PlanningError { error: e, major_version_upgrade, } => { diff --git a/crates/core/src/host/host_controller.rs b/crates/core/src/host/host_controller.rs index 20cde3d6ef..0230111218 100644 --- a/crates/core/src/host/host_controller.rs +++ b/crates/core/src/host/host_controller.rs @@ -29,7 +29,6 @@ use log::{info, trace, warn}; use parking_lot::Mutex; use scopeguard::defer; use spacetimedb_commitlog::SizeOnDisk; -use spacetimedb_data_structures::error_stream::ErrorStream; use spacetimedb_data_structures::map::{IntMap, IntSet}; use spacetimedb_datastore::db_metrics::data_size::DATA_SIZE_METRICS; use spacetimedb_datastore::db_metrics::DB_METRICS; @@ -42,7 +41,7 @@ use spacetimedb_paths::server::{ModuleLogsDir, ServerDataDir}; use spacetimedb_runtime::AbortHandle; use spacetimedb_sats::hash::Hash; use spacetimedb_schema::def::{ModuleDef, RawModuleDefVersion}; -use spacetimedb_schema::migrate::{ponder_migrate, AutoMigrateError, MigrationPolicy, PrettyPrintStyle}; +use spacetimedb_schema::migrate::{ponder_migrate, MigrationPolicy, PonderMigrateError, PrettyPrintStyle}; use spacetimedb_table::page_pool::PagePool; use std::future::Future; use std::ops::Deref; @@ -1444,7 +1443,7 @@ impl Host { plan: plan.pretty_print(style)?.into(), major_version_upgrade, }, - Err(e) => MigratePlanResult::AutoMigrationError { + Err(e) => MigratePlanResult::PlanningError { error: e, major_version_upgrade, }, @@ -1474,8 +1473,8 @@ pub enum MigratePlanResult { breaks_client: bool, major_version_upgrade: bool, }, - AutoMigrationError { - error: ErrorStream, + PlanningError { + error: PonderMigrateError, major_version_upgrade: bool, }, } diff --git a/crates/core/src/host/module_host.rs b/crates/core/src/host/module_host.rs index 1007fdc537..c534cd8a7a 100644 --- a/crates/core/src/host/module_host.rs +++ b/crates/core/src/host/module_host.rs @@ -43,7 +43,6 @@ use spacetimedb_client_api_messages::energy::FunctionBudget; use spacetimedb_client_api_messages::websocket::common::{ByteListLen as _, RowListLen as _}; use spacetimedb_client_api_messages::websocket::v1::{self as ws_v1}; use spacetimedb_client_api_messages::websocket::v2::{self as ws_v2}; -use spacetimedb_data_structures::error_stream::ErrorStream; use spacetimedb_data_structures::map::{HashCollectionExt as _, HashSet}; use spacetimedb_datastore::error::DatastoreError; use spacetimedb_datastore::execution_context::{Workload, WorkloadType}; @@ -66,7 +65,7 @@ use spacetimedb_sats::raw_identifier::RawIdentifier; use spacetimedb_sats::{AlgebraicType, AlgebraicTypeRef, ProductValue}; use spacetimedb_schema::def::{ModuleDef, ProcedureDef, ReducerDef, ViewDef}; use spacetimedb_schema::identifier::Identifier; -use spacetimedb_schema::migrate::{AutoMigrateError, MigrationPolicy}; +use spacetimedb_schema::migrate::{MigrationPolicy, PonderMigrateError}; use spacetimedb_schema::reducer_name::ReducerName; use spacetimedb_schema::table_name::TableName; use std::collections::VecDeque; @@ -1477,7 +1476,7 @@ pub enum UpdateDatabaseResult { /// `None` if the database is in-memory only. durable_offset: Option, }, - AutoMigrateError(Box>), + MigrationPlanningError(Box), ErrorExecutingMigration(anyhow::Error), } impl UpdateDatabaseResult { diff --git a/crates/core/src/host/wasm_common/module_host_actor.rs b/crates/core/src/host/wasm_common/module_host_actor.rs index 453fa11359..7f1a4334fc 100644 --- a/crates/core/src/host/wasm_common/module_host_actor.rs +++ b/crates/core/src/host/wasm_common/module_host_actor.rs @@ -661,7 +661,9 @@ impl InstanceCommon { Ok(plan) => plan, Err(e) => { return match e { - MigrationPolicyError::AutoMigrateFailure(e) => Ok(UpdateDatabaseResult::AutoMigrateError(e.into())), + MigrationPolicyError::PlanningFailure(e) => { + Ok(UpdateDatabaseResult::MigrationPlanningError(e.into())) + } _ => Ok(UpdateDatabaseResult::ErrorExecutingMigration(e.into())), } } diff --git a/crates/schema/Cargo.toml b/crates/schema/Cargo.toml index 25ff3e3e9c..d72d4f7d4d 100644 --- a/crates/schema/Cargo.toml +++ b/crates/schema/Cargo.toml @@ -17,6 +17,7 @@ spacetimedb-data-structures.workspace = true spacetimedb-memory-usage.workspace = true spacetimedb-sql-parser.workspace = true anyhow.workspace = true +blake3.workspace = true derive_more.workspace = true indexmap.workspace = true itertools.workspace = true diff --git a/crates/schema/src/def.rs b/crates/schema/src/def.rs index 4c9a01f5b4..c160891788 100644 --- a/crates/schema/src/def.rs +++ b/crates/schema/src/def.rs @@ -44,7 +44,7 @@ use spacetimedb_lib::db::raw_def::v9::{ RawUniqueConstraintDataV9, RawViewDefV9, TableAccess, TableType, }; use spacetimedb_lib::db::view::{extract_view_return_product_type_ref, ViewKind}; -use spacetimedb_lib::{ProductType, RawModuleDef}; +use spacetimedb_lib::{bsatn, ProductType, RawModuleDef}; use spacetimedb_primitives::{ ColId, ColList, ColOrCols, ColSet, HttpHandlerId, ProcedureId, ReducerId, TableId, ViewFnPtr, }; @@ -467,6 +467,24 @@ impl ModuleDef { panic!("expected ModuleDef to contain {:?}, but it does not", def.key()); } } + + pub fn manual_migration_function_for_previous_module_def_hash( + &self, + prev_hash: spacetimedb_lib::Hash, + ) -> Option<&ManualMigrationFunctionDef> { + self.manual_migration_functions.get(&prev_hash) + } + + /// Compute the hash of this module def, + /// for use as an identifier a manual migration can use to reference its expected previous state. + /// + /// This takes the Blake3 hash of the BSATN bytes of the `RawModuleDefV10` representation of `self`. + pub fn manual_migration_hash(&self) -> Result { + let raw_def = RawModuleDefV10::from(self.clone()); + let bsatn_bytes = bsatn::to_vec(&raw_def)?; + let hash = blake3::hash(&bsatn_bytes); + Ok(spacetimedb_lib::Hash::from_byte_array(*hash.as_bytes())) + } } impl TryFrom for ModuleDef { diff --git a/crates/schema/src/migrate.rs b/crates/schema/src/migrate.rs index ed1f84a02e..befbeeb001 100644 --- a/crates/schema/src/migrate.rs +++ b/crates/schema/src/migrate.rs @@ -1,6 +1,7 @@ use auto_migrate::{format_plan, ponder_auto_migrate, ColorScheme, TermColorFormatter}; +use manual_migrate::{ponder_manual_migrate, ManualMigrateError}; use spacetimedb_data_structures::error_stream::ErrorStream; -use spacetimedb_lib::{hash_bytes, Identity}; +use spacetimedb_lib::{bsatn, hash_bytes, Identity}; use thiserror::Error; use crate::def::ModuleDef; @@ -119,7 +120,7 @@ impl MigrationPolicy { new_module_hash: spacetimedb_lib::Hash, new_module_def: &'def ModuleDef, ) -> anyhow::Result, MigrationPolicyError> { - let plan = ponder_migrate(old_module_def, new_module_def).map_err(MigrationPolicyError::AutoMigrateFailure)?; + let plan = ponder_migrate(old_module_def, new_module_def).map_err(MigrationPolicyError::PlanningFailure)?; self.permits_migrate_plan(database_identity, old_module_hash, new_module_hash, &plan)?; Ok(plan) } @@ -144,8 +145,8 @@ impl MigrationPolicy { #[derive(Debug, Error)] pub enum MigrationPolicyError { - #[error("Automatic migration planning failed")] - AutoMigrateFailure(ErrorStream), + #[error("Migration planning failed: {0}")] + PlanningFailure(PonderMigrateError), #[error("Token provided is invalid or does not match expected hash")] InvalidToken, @@ -180,14 +181,37 @@ impl MigrationToken { } } +#[derive(thiserror::Error, Debug)] +pub enum PonderMigrateError { + #[error("ModuleDef changes require a manual migration, but new `ModuleDef` does not specify a manual migration function from previous `ModuleDef` {old_hash}.\nManual migration is required because {errors}")] + AutoMigrate { + errors: ErrorStream, + old_hash: spacetimedb_lib::Hash, + }, + #[error("Planning manual migration failed: ")] + ManualMigrate(#[from] ErrorStream), + #[error("Error while BSATN encoding old `ModuleDef` in order to compute its hash when pondering a manual migration: {0}")] + Bsatn(#[from] bsatn::EncodeError), +} + /// Construct a migration plan. /// If `new` has an `__update__` reducer, return a manual migration plan. /// Otherwise, try to plan an automatic migration. This may fail. pub fn ponder_migrate<'def>( old: &'def ModuleDef, new: &'def ModuleDef, -) -> Result, ErrorStream> { - // TODO(1.0): Implement this function. - // Currently we only can do automatic migrations. - ponder_auto_migrate(old, new).map(MigratePlan::Auto) +) -> Result, PonderMigrateError> { + let old_hash = old.manual_migration_hash()?; + if new + .manual_migration_function_for_previous_module_def_hash(old_hash) + .is_some() + { + ponder_manual_migrate(old, old_hash, new) + .map(MigratePlan::Manual) + .map_err(PonderMigrateError::ManualMigrate) + } else { + ponder_auto_migrate(old, new) + .map(MigratePlan::Auto) + .map_err(|errors| PonderMigrateError::AutoMigrate { errors, old_hash }) + } } diff --git a/crates/schema/src/migrate/auto_migrate.rs b/crates/schema/src/migrate/auto_migrate.rs index f7398503f5..d712ca7f52 100644 --- a/crates/schema/src/migrate/auto_migrate.rs +++ b/crates/schema/src/migrate/auto_migrate.rs @@ -95,6 +95,8 @@ pub enum AutoMigrateStep<'def> { RemoveRowLevelSecurity(::Key<'def>), /// Remove an empty table and all its sub-objects (indexes, constraints, sequences). /// Validated at execution time: fails if the table contains data. + /// + /// Will never appear in a [`super::manual_migrate::ManualMigratePlan::auto_migrate_steps_before`]. RemoveTable(::Key<'def>), /// Change the column types of a table, in a layout compatible way. diff --git a/crates/schema/src/migrate/manual_migrate.rs b/crates/schema/src/migrate/manual_migrate.rs index ce892bffee..e7674f806d 100644 --- a/crates/schema/src/migrate/manual_migrate.rs +++ b/crates/schema/src/migrate/manual_migrate.rs @@ -1,17 +1,33 @@ +use spacetimedb_data_structures::error_stream::ErrorStream; + use crate::{ def::{ManualMigrationFunctionDef, ModuleDef, ModuleDefLookup, TableDef}, migrate::auto_migrate::AutoMigrateStep, }; -type TableIdent<'def> = ::Key<'def>; +type Ident<'def, ModuleDefEntry> = ::Key<'def>; /// A plan for a manual migration. #[derive(Debug)] pub struct ManualMigratePlan<'def> { pub old: &'def ModuleDef, pub new: &'def ModuleDef, + pub tables_to_rename_before_and_delete_after: Vec>, + /// Auto-migrate steps to execute before running the manual migration function. + /// + /// This must never contain an [`AutoMigrateStep::RemoveTable`]. + /// Tables to be removed will instead be listed in [`Self::tables_to_rename_before_and_delete_after`]. pub auto_migrate_steps_before: Vec>, - pub tables_to_rename_before: Vec<(TableIdent<'def>, TableIdent<'def>)>, - pub manual_migration_function: ::Key<'def>, - pub tables_to_delete_after: Vec>, + pub manual_migration_function: Ident<'def, ManualMigrationFunctionDef>, +} + +#[derive(thiserror::Error, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum ManualMigrateError {} + +pub fn ponder_manual_migrate<'def>( + old: &'def ModuleDef, + old_hash: spacetimedb_lib::Hash, + new: &'def ModuleDef, +) -> Result, ErrorStream> { + todo!() }