diff --git a/src/infrastructure/scheduler/mod.rs b/src/infrastructure/scheduler/mod.rs index a78b9310..f1077050 100644 --- a/src/infrastructure/scheduler/mod.rs +++ b/src/infrastructure/scheduler/mod.rs @@ -33,9 +33,9 @@ pub use engine::SchedulerEngine; pub use handler::JobHandler; pub use pg_job_store::{PgJobStore, PgJobStoreProvider}; pub use recoverable::{ - Finding, JobStore, JobStoreProvider, OpenedRun, ProgressKind, RecoverableAdapter, - RecoverableJobHandler, RunOutcome, RunProgress, RunStatus, RunSummary, derive_progress, - record_or_log, run_or_resume, + CANCEL_INTENT_PARAM, CANCEL_INTENT_TERMINATE, Finding, JobStore, JobStoreProvider, OpenedRun, + ProgressKind, RecoverableAdapter, RecoverableJobHandler, RunOutcome, RunProgress, RunStatus, + RunSummary, derive_progress, record_or_log, run_or_resume, }; pub use registry::{ JobEntry, JobRegistry, JobSummary, PausedRunBrief, RegisterError, StartupTrigger, diff --git a/src/infrastructure/services/backend_migration_service.rs b/src/infrastructure/services/backend_migration_service.rs index aa6b4c15..286320b6 100644 --- a/src/infrastructure/services/backend_migration_service.rs +++ b/src/infrastructure/services/backend_migration_service.rs @@ -580,6 +580,23 @@ impl RecoverableJobHandler for BackendMigrationService { source_missing = source_missing_count, "backend_migration cancelled cooperatively, pausing" ); + // A TERMINAL cancel must give writes back. + // + // Cancel ends the run with no swap, so the source + // stays the active backend and there is nothing left + // to protect. Leaving the gate set stranded the whole + // application read-only with no way out: the flag is + // persisted, so a restart reloaded it rather than + // clearing it, and the only escape was editing + // `admin_settings` by hand. + // + // A plain PAUSE deliberately keeps the gate. The + // cursor stays valid only while nothing writes, so + // resuming after allowing writes could miss a blob + // written below the cursor — see the plan's + // "Why NOT to release the gate on pause". Cancel is + // the escape hatch, and it is the operator's call. + self.release_readonly_on_terminal_cancel(store).await; return RunOutcome::Paused { cursor: cursor .as_ref() @@ -834,6 +851,66 @@ impl RecoverableJobHandler for BackendMigrationService { } impl BackendMigrationService { + /// Clear `migration_readonly` when the cancel was TERMINAL. + /// + /// Cancel ends the run with no swap: the source is still the active + /// backend, so there is nothing left for the write freeze to + /// protect, and leaving it set locks the whole application out of + /// writes. The flag is persisted, so that state survived restarts — + /// the only escape was hand-editing `admin_settings`. + /// + /// **Pause is deliberately not this.** The cursor is a position in a + /// hash-ordered walk, and it stays valid only while nothing writes. + /// Release the gate on pause and a blob written afterwards whose + /// hash sorts BELOW the cursor is never visited, so the run + /// completes, flips the pointer, and reads for that hash 404 against + /// a target that never received it. Cancel is safe precisely because + /// it ENDS the run: a later retry starts fresh and rescans + /// everything. + /// + /// Distinguished by the same `cancel_intent` param the engine reads + /// to decide `Cancelled` vs `Paused`, so the two cannot disagree + /// about which kind of stop this was. + /// + /// Best effort, and deliberately so: a run that has already been + /// cancelled should not be turned into a hard failure by a DB blip + /// while releasing a flag. The in-memory store still happens, so + /// writes resume in THIS process even if the persist fails; the loud + /// warning is what tells an operator the DB copy needs attention. + async fn release_readonly_on_terminal_cancel(&self, store: &dyn JobStore) { + let terminal = store + .get_string_param(crate::infrastructure::scheduler::CANCEL_INTENT_PARAM) + .await + .ok() + .flatten() + .as_deref() + == Some(crate::infrastructure::scheduler::CANCEL_INTENT_TERMINATE); + if !terminal { + return; + } + + if let Err(e) = persist_migration_readonly(self.pool.as_ref(), false).await { + tracing::warn!( + target: "oxicloud::migration", + event = "storage.migration_readonly.release_persist_failed", + run_id = %store.run_id(), + error = %e, + "could not persist migration_readonly=false after a terminal cancel; writes \ + resume in this process but a restart will come up read-only until \ + admin_settings is corrected" + ); + } + self.migration_readonly.store(false, Ordering::Relaxed); + tracing::info!( + target: "audit", + event = "storage.migration_readonly.released", + reason = "migration_cancelled", + run_id = %store.run_id(), + "🚧 migration_readonly released after terminal cancel — writes resume, active \ + backend unchanged" + ); + } + /// Terminal successful path — reached from both Completed sites /// in the batch loop (empty-first-batch and short-batch). /// diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index e6c699ab..15de53a1 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -2879,16 +2879,73 @@ pub async fn cancel_job( .request_terminal_cancel(&name) .await { - Ok(Some(run_id)) => ( - StatusCode::OK, - Json(serde_json::json!({ - "cancelled": true, - "run_id": run_id.to_string(), - "note": "Running row → will land in Cancelled at next batch boundary; \ - Paused row → flipped to Cancelled immediately.", - })), - ) - .into_response(), + Ok(Some(run_id)) => { + // Cancelling a PAUSED migration has to give writes back + // here, because nothing else will. + // + // A Running row re-enters the handler, which releases the + // gate itself at its next cancel poll. A Paused row does + // not: `request_terminal_cancel` flips it straight to + // Cancelled in SQL with no handler in the loop. That is the + // common case — a migration paused by an outage, holding + // `migration_readonly`, which an operator cancels precisely + // TO get writes back. Without this the app stayed read-only + // forever: the flag is persisted, so even a restart reloaded + // it, and the only escape was editing admin_settings by + // hand. + // + // Safe because cancel ends the run with no swap — the source + // is still the active backend, so there is nothing left for + // the freeze to protect. Releasing on PAUSE would not be + // safe; see `release_readonly_on_terminal_cancel`. + // + // Idempotent and harmless for every other job: the flag is + // only ever set by backend_migration, so clearing it when it + // is already false is a no-op. + if name == crate::infrastructure::services::backend_migration_service::BACKEND_MIGRATION_JOB_NAME + && state + .migration_readonly + .load(std::sync::atomic::Ordering::Relaxed) + { + if let Some(pool) = state.db_pool.as_ref() + && let Err(e) = + crate::infrastructure::services::entry_backend::persist_migration_readonly( + pool.as_ref(), + false, + ) + .await + { + tracing::warn!( + target: "oxicloud::migration", + event = "storage.migration_readonly.release_persist_failed", + run_id = %run_id, + error = %e, + "could not persist migration_readonly=false after cancelling a paused \ + migration; writes resume now but a restart will come up read-only" + ); + } + state + .migration_readonly + .store(false, std::sync::atomic::Ordering::Relaxed); + tracing::info!( + target: "audit", + event = "storage.migration_readonly.released", + reason = "paused_migration_cancelled", + run_id = %run_id, + "🚧 migration_readonly released — writes resume, active backend unchanged" + ); + } + ( + StatusCode::OK, + Json(serde_json::json!({ + "cancelled": true, + "run_id": run_id.to_string(), + "note": "Running row → will land in Cancelled at next batch boundary; \ + Paused row → flipped to Cancelled immediately.", + })), + ) + .into_response() + } Ok(None) => ( StatusCode::OK, Json(serde_json::json!({