Merge branch 'fix/disk' into integration/round-1
This commit is contained in:
@@ -227,58 +227,40 @@ async fn container_label(container_id: &str, label: &str) -> Option<String> {
|
||||
// Migrate
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Project ids with a migration running **in this process right now**.
|
||||
///
|
||||
/// Two things need it. `reconcile_project_statuses` is callable from the
|
||||
/// frontend at any time, not only at startup, and a live migration looks
|
||||
/// exactly like a crashed one from the outside (state file says `in-progress`,
|
||||
/// container carries the label) — without this guard a reconcile mid-run would
|
||||
/// rewrite the phase to `interrupted` underneath a migration that is fine.
|
||||
/// It also makes a second concurrent `migrate_project_to_base` for the same
|
||||
/// project impossible.
|
||||
static ACTIVE_MIGRATIONS: std::sync::OnceLock<
|
||||
std::sync::Mutex<std::collections::HashSet<String>>,
|
||||
> = std::sync::OnceLock::new();
|
||||
|
||||
fn active_migrations() -> &'static std::sync::Mutex<std::collections::HashSet<String>> {
|
||||
ACTIVE_MIGRATIONS.get_or_init(|| std::sync::Mutex::new(std::collections::HashSet::new()))
|
||||
}
|
||||
|
||||
/// Whether a migration for this project is running **in this process right
|
||||
/// now**. Every command that stops, removes or recreates the project's
|
||||
/// container has to consult it: the window between `remove_container` and the
|
||||
/// create that follows looks exactly like "no container", and an ordinary
|
||||
/// Start landing in it creates a second container under the same name.
|
||||
///
|
||||
/// **This is now a view onto [`crate::project_lock`], not a set of its own.**
|
||||
/// It used to be the app's only mutual-exclusion primitive, and it was one-way:
|
||||
/// a migration claimed a project, everything else merely polled this once at
|
||||
/// entry and never claimed anything. Two non-migration writers of
|
||||
/// `triple-c-snapshot-{id}:latest` — a compaction and a recreate — could not
|
||||
/// see each other at all. Folding the set into the shared registry means there
|
||||
/// is exactly one answer to "is something happening to this project", and this
|
||||
/// function is the specialisation of it that reconcile still needs: a *live*
|
||||
/// migration is indistinguishable from a crashed one from the outside, and only
|
||||
/// this process knows which it is looking at.
|
||||
pub(crate) fn is_migrating(project_id: &str) -> bool {
|
||||
active_migrations()
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.contains(project_id)
|
||||
crate::project_lock::is_held_by(project_id, crate::project_lock::ProjectOp::Migration)
|
||||
}
|
||||
|
||||
/// RAII marker: removes the project from [`ACTIVE_MIGRATIONS`] however the
|
||||
/// migration ends, including an early `?`.
|
||||
struct ActiveGuard(String);
|
||||
/// RAII marker: releases the project's [`crate::project_lock`] claim however
|
||||
/// the migration ends, including an early `?`.
|
||||
///
|
||||
/// Kept as a named type rather than using [`crate::project_lock::ProjectGuard`]
|
||||
/// directly so the migration path keeps reading as "take the migration guard",
|
||||
/// and so the one place that decides what a migration's claim *is* stays here.
|
||||
struct ActiveGuard(#[allow(dead_code)] crate::project_lock::ProjectGuard);
|
||||
|
||||
impl ActiveGuard {
|
||||
/// `None` when a migration is already running for this project.
|
||||
/// `None` when a migration — or anything else — already holds this project.
|
||||
fn acquire(project_id: &str) -> Option<Self> {
|
||||
let mut set = active_migrations()
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner());
|
||||
if !set.insert(project_id.to_string()) {
|
||||
return None;
|
||||
}
|
||||
Some(Self(project_id.to_string()))
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ActiveGuard {
|
||||
fn drop(&mut self) {
|
||||
active_migrations()
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.remove(&self.0);
|
||||
crate::project_lock::try_acquire(project_id, crate::project_lock::ProjectOp::Migration)
|
||||
.ok()
|
||||
.map(Self)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1163,7 +1145,23 @@ pub(crate) async fn purge_migration_artifacts(project_id: &str) {
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(None) => return,
|
||||
Ok(None) => {
|
||||
// **`Ok(None)` is not the same as "no file".** `migration_store::load`
|
||||
// now reports an *unparseable* record as absent while deliberately
|
||||
// leaving it on disk, so that `has_record` goes on protecting the
|
||||
// rollback pin it describes. Returning here on that would leave the
|
||||
// file — and therefore a permanently "claimed" pin — behind a Reset
|
||||
// that has just deleted the snapshot and both volumes the record
|
||||
// could possibly refer to.
|
||||
if !migration_store::has_record(project_id).unwrap_or(false) {
|
||||
return;
|
||||
}
|
||||
log::warn!(
|
||||
"Project {} has a migration record that could not be read; removing it anyway \
|
||||
because a Reset supersedes it",
|
||||
project_id
|
||||
);
|
||||
}
|
||||
Err(e) => log::warn!(
|
||||
"Could not read the migration record for {} while cleaning up: {}",
|
||||
project_id,
|
||||
@@ -1172,6 +1170,10 @@ pub(crate) async fn purge_migration_artifacts(project_id: &str) {
|
||||
}
|
||||
let _ = migration_store::clear_staging(project_id);
|
||||
let _ = migration_store::clear(project_id);
|
||||
// The pins this project had are gone with the snapshot; their grace clocks
|
||||
// are meaningless and would otherwise sit in the migrations directory
|
||||
// forever.
|
||||
migration_store::clear_ownerless_for_project(project_id);
|
||||
}
|
||||
|
||||
fn default_docker_socket() -> String {
|
||||
|
||||
@@ -221,20 +221,35 @@ pub async fn start_project_container(
|
||||
app_handle: tauri::AppHandle,
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<Project, String> {
|
||||
// A migration removes the container and creates its replacement moments
|
||||
// later. Starting in that window finds no container, creates a second one
|
||||
// under the same name, and the migration's own create then fails on the
|
||||
// name conflict — which sends it into an auto-rollback that also cannot
|
||||
// create. The UI already refuses (`canMigrate` gates on the container being
|
||||
// stopped and no run being in flight); this is the same gate on the side
|
||||
// that actually owns the invariant.
|
||||
if crate::commands::migration_commands::is_migrating(&project_id) {
|
||||
return Err(
|
||||
"A container base update is running for this project. Wait for it to finish, then start the project."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
// **Acquired, not polled.** A migration removes the container and creates
|
||||
// its replacement moments later. Starting in that window finds no
|
||||
// container, creates a second one under the same name, and the migration's
|
||||
// own create then fails on the name conflict — which sends it into an
|
||||
// auto-rollback that also cannot create. This used to be a one-shot
|
||||
// `is_migrating` read, which covered that case and no other: a start also
|
||||
// commits `triple-c-snapshot-{id}:latest`, so it races a *compaction*
|
||||
// committing the same tag with nothing between them. The claim is held for
|
||||
// the whole start rather than checked at its door.
|
||||
//
|
||||
// The UI already refuses (`canMigrate` gates on the container being stopped
|
||||
// and no run being in flight); this is the same gate on the side that
|
||||
// actually owns the invariant.
|
||||
let _guard =
|
||||
crate::project_lock::try_acquire(&project_id, crate::project_lock::ProjectOp::Recreate)?;
|
||||
start_project_container_locked(project_id, app_handle, state).await
|
||||
}
|
||||
|
||||
/// The body of [`start_project_container`], with **no claim of its own**.
|
||||
///
|
||||
/// Split out for exactly one caller: [`rebuild_project_container`] already
|
||||
/// holds the project under [`crate::project_lock::ProjectOp::Reset`] for its
|
||||
/// whole run, and a Reset that then went through the public command would be
|
||||
/// refused by its own guard. Every other path must go through the command.
|
||||
async fn start_project_container_locked(
|
||||
project_id: String,
|
||||
app_handle: tauri::AppHandle,
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<Project, String> {
|
||||
let mut project = state
|
||||
.projects_store
|
||||
.get(&project_id)
|
||||
@@ -539,6 +554,14 @@ pub async fn stop_project_container(
|
||||
app_handle: tauri::AppHandle,
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<(), String> {
|
||||
// Stop had **no** exclusion at all, which is the one gap CLAUDE.md's "every
|
||||
// command that stops, removes or recreates the container consults
|
||||
// `is_migrating`" rule already named and this function did not honour. A
|
||||
// stop lands on the container a migration is mid-swap on, and it closes the
|
||||
// exec sessions a compaction's config replay is not expecting to lose.
|
||||
let _guard =
|
||||
crate::project_lock::try_acquire(&project_id, crate::project_lock::ProjectOp::Recreate)?;
|
||||
|
||||
let project = state
|
||||
.projects_store
|
||||
.get(&project_id)
|
||||
@@ -571,13 +594,13 @@ pub async fn rebuild_project_container(
|
||||
) -> Result<Project, String> {
|
||||
// Reset deletes both volumes and the snapshot image. Doing that while a
|
||||
// migration is mid-flight pulls the ground out from under it and leaves an
|
||||
// orphan migration record pointing at images that no longer exist.
|
||||
if crate::commands::migration_commands::is_migrating(&project_id) {
|
||||
return Err(
|
||||
"A container base update is running for this project. Wait for it to finish before resetting."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
// orphan migration record pointing at images that no longer exist — and
|
||||
// doing it while a *compaction* is mid-flight is worse, because the
|
||||
// compaction then commits `flat(old)` back over the `:latest` this just
|
||||
// destroyed and resurrects the system layer the user asked to be rid of.
|
||||
// Held for the whole Reset, including the start at the end of it.
|
||||
let _guard =
|
||||
crate::project_lock::try_acquire(&project_id, crate::project_lock::ProjectOp::Reset)?;
|
||||
|
||||
let project = state
|
||||
.projects_store
|
||||
@@ -611,8 +634,9 @@ pub async fn rebuild_project_container(
|
||||
log::warn!("Failed to remove project volumes for project {}: {}", project_id, e);
|
||||
}
|
||||
|
||||
// Start fresh
|
||||
start_project_container(project_id, app_handle, state).await
|
||||
// Start fresh. The locked variant, because `_guard` above is this project's
|
||||
// claim and the public command would be refused by it.
|
||||
start_project_container_locked(project_id, app_handle, state).await
|
||||
}
|
||||
|
||||
/// Reconcile project statuses against actual Docker container state.
|
||||
@@ -656,9 +680,14 @@ pub async fn reconcile_project_statuses(
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
// ...but never for a project this process is actively migrating: the
|
||||
// container is legitimately absent for part of that run.
|
||||
if crate::commands::migration_commands::is_migrating(&project.id) {
|
||||
// ...but never for a project this process is actively working on. A
|
||||
// migration's container is legitimately absent between the
|
||||
// `remove_container` and the create that follows, and so is a Reset's,
|
||||
// and so is a start's before its create returns. Reconciling into any
|
||||
// of those windows writes `Stopped` over a project that is mid-run.
|
||||
// Broadened from `is_migrating` to the whole lock for exactly that
|
||||
// reason — the migration was never the only operation with a gap.
|
||||
if crate::project_lock::held(&project.id).is_some() {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user