Skip to content

Commit aa1e732

Browse files
authored
core: Remove init_maybe_update_module_host from host controller (#3246)
The method is no longer used. Note that this patch must merge **after** the private companion. # API and ABI breaking changes Internal API change. # Expected complexity level and risk 1 # Testing In private.
1 parent 123fc72 commit aa1e732

1 file changed

Lines changed: 1 addition & 116 deletions

File tree

crates/core/src/host/host_controller.rs

Lines changed: 1 addition & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::subscription::module_subscription_manager::{spawn_send_worker, Subscr
1616
use crate::util::asyncify;
1717
use crate::util::jobs::{JobCore, JobCores};
1818
use crate::worker_metrics::WORKER_METRICS;
19-
use anyhow::{anyhow, ensure, Context};
19+
use anyhow::{anyhow, Context};
2020
use async_trait::async_trait;
2121
use durability::{Durability, EmptyHistory};
2222
use log::{info, trace, warn};
@@ -448,117 +448,6 @@ impl HostController {
448448
host.migrate_plan(host_type, program, style).await
449449
}
450450

451-
/// Start the host `replica_id` and conditionally update it.
452-
///
453-
/// If the host was not initialized before, it is initialized with the
454-
/// program [`Database::initial_program`], which is loaded from the
455-
/// controller's [`ProgramStorage`].
456-
///
457-
/// If it was already initialized and its stored program hash matches
458-
/// [`Database::initial_program`], no further action is taken.
459-
///
460-
/// Otherwise, if `expected_hash` is `Some` and does **not** match the
461-
/// stored hash, an error is returned.
462-
///
463-
/// Otherwise, the host is updated to [`Database::initial_program`], loading
464-
/// the program data from the controller's [`ProgramStorage`].
465-
///
466-
/// > Note that this ascribes different semantics to [`Database::initial_program`]
467-
/// > than elsewhere, where the [`Database`] value is provided by the control
468-
/// > database. The method is mainly useful for bootstrapping the control
469-
/// > database itself.
470-
pub async fn init_maybe_update_module_host(
471-
&self,
472-
database: Database,
473-
replica_id: u64,
474-
expected_hash: Option<Hash>,
475-
) -> anyhow::Result<watch::Receiver<ModuleHost>> {
476-
trace!("custom bootstrap {}/{}", database.database_identity, replica_id);
477-
478-
let db_addr = database.database_identity;
479-
let host_type = database.host_type;
480-
let program_hash = database.initial_program;
481-
482-
let mut guard = self.acquire_write_lock(replica_id).await;
483-
484-
// `HostController::clone` is fast,
485-
// as all of its fields are either `Copy` or wrapped in `Arc`.
486-
let this = self.clone();
487-
488-
// `try_init_host` is not cancel safe, as it will spawn other async tasks
489-
// which hold a filesystem lock past when `try_init_host` returns or is cancelled.
490-
// This means that, if `try_init_host` is cancelled, subsequent calls will fail.
491-
//
492-
// The rest of this future is also not cancel safe, as it will `Option::take` out of the guard
493-
// at the start of the block and then store back into it at the end.
494-
//
495-
// This is problematic because Axum will cancel its handler tasks if the client disconnects,
496-
// and this method is called from Axum handlers, e.g. for the publish route.
497-
// `tokio::spawn` a task to update the contents of `guard`,
498-
// so that it will run to completion even if the caller goes away.
499-
//
500-
// Note that `tokio::spawn` only cancels its tasks when the runtime shuts down,
501-
// at which point we won't be calling `try_init_host` again anyways.
502-
let module = tokio::spawn(async move {
503-
let mut host = match guard.take() {
504-
Some(host) => host,
505-
None => this.try_init_host(database, replica_id).await?,
506-
};
507-
let module = host.module.subscribe();
508-
509-
// The program is now either:
510-
//
511-
// - the desired one from [Database], in which case we do nothing
512-
// - `Some` expected hash, in which case we update to the desired one
513-
// - `None` expected hash, in which case we also update
514-
let stored_hash = stored_program_hash(host.db())?
515-
.with_context(|| format!("[{db_addr}] database improperly initialized"))?;
516-
if stored_hash == program_hash {
517-
info!("[{db_addr}] database up-to-date with {program_hash}");
518-
*guard = Some(host);
519-
} else {
520-
if let Some(expected_hash) = expected_hash {
521-
ensure!(
522-
expected_hash == stored_hash,
523-
"[{}] expected program {} found {}",
524-
db_addr,
525-
expected_hash,
526-
stored_hash
527-
);
528-
}
529-
info!("[{db_addr}] updating database from `{stored_hash}` to `{program_hash}`");
530-
let program = load_program(&this.program_storage, program_hash).await?;
531-
let update_result = host
532-
.update_module(
533-
this.runtimes.clone(),
534-
host_type,
535-
program,
536-
MigrationPolicy::Compatible,
537-
this.energy_monitor.clone(),
538-
this.unregister_fn(replica_id),
539-
this.db_cores.take(),
540-
)
541-
.await?;
542-
match update_result {
543-
UpdateDatabaseResult::NoUpdateNeeded | UpdateDatabaseResult::UpdatePerformed => {
544-
*guard = Some(host);
545-
}
546-
UpdateDatabaseResult::AutoMigrateError(e) => {
547-
return Err(anyhow::anyhow!(e));
548-
}
549-
UpdateDatabaseResult::ErrorExecutingMigration(e) => {
550-
return Err(e);
551-
}
552-
}
553-
}
554-
555-
Ok::<_, anyhow::Error>(module)
556-
})
557-
.await??;
558-
559-
Ok(module)
560-
}
561-
562451
/// Release all resources of the [`ModuleHost`] identified by `replica_id`,
563452
/// and deregister it from the controller.
564453
#[tracing::instrument(level = "trace", skip_all)]
@@ -1115,10 +1004,6 @@ impl Host {
11151004

11161005
Ok(res)
11171006
}
1118-
1119-
fn db(&self) -> &RelationalDB {
1120-
&self.replica_ctx.relational_db
1121-
}
11221007
}
11231008

11241009
impl Drop for Host {

0 commit comments

Comments
 (0)