diff --git a/ROADMAP.md b/ROADMAP.md index 83e86c9c..2c000fd0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -707,7 +707,7 @@ and reconciler, collaborators, branches) and F2b (smart-HTTP git transport via ` `pre-receive` ref policy, admission control, threat-model pass #1) and F3a (content reads: refs, tags, commits, trees, contents, archives) and F3b (contents writes: create, update, delete as server-made commits) and F4a (branch protection enforced on push, contents API and branch delete; commit statuses) and F4b (pull requests, reviews, comments, retention -refs) and F4c (merge with a crash-safe journal, required-check gating, kill -9 and protection-matrix tests) are built. Hosted repos are +refs) and F4c (merge with a crash-safe journal, required-check gating, kill -9 and protection-matrix tests) and F5a (two-stage gc, merge-commit retention, runtime cleanup of stale locks and quarantine directories) are built. Hosted repos are bare Git and get none of the `.sc/` features. Other follow-up work is tracked as GitHub issues and listed under Deferred diff --git a/crates/forge/src/api.rs b/crates/forge/src/api.rs index 632fe7d1..1122b804 100644 --- a/crates/forge/src/api.rs +++ b/crates/forge/src/api.rs @@ -412,6 +412,16 @@ pub fn admin_list_users(ctx: &mut Ctx) -> Resp { } } +/// `POST /admin/cron/{task}`: run a maintenance task now. Only `git_gc_repos` exists; it answers 204 +/// at once and the pass runs on the maintenance thread (a pass already queued or running covers it). +pub fn admin_cron(ctx: &mut Ctx) -> Resp { + if ctx.param("task") != "git_gc_repos" { + return error(&ctx.app.base, 404, "unknown cron task"); + } + ctx.app.maint.request(); + Resp::no_content() +} + /// `POST /admin/users`: create a user; `must_change_password` defaults to true as in Gitea, and `admin` is ignored (set it with PATCH). pub fn admin_create_user(ctx: &mut Ctx) -> Resp { let b = match body(ctx) { diff --git a/crates/forge/src/cgi.rs b/crates/forge/src/cgi.rs index 918eaf83..e515db83 100644 --- a/crates/forge/src/cgi.rs +++ b/crates/forge/src/cgi.rs @@ -512,6 +512,8 @@ fn launch(l: Launch) -> Result { .registry .register( repo_id, + // Only a fetch is known to write no lock and no quarantine directory. + l.plan.service != Some(Service::Upload), || matches!(repos::find_by_id(&conn, repo_id), Ok(Some(r)) if r.state == "live"), ) .ok_or_else(|| { diff --git a/crates/forge/src/cli.rs b/crates/forge/src/cli.rs index 07ebf007..1c5e68c4 100644 --- a/crates/forge/src/cli.rs +++ b/crates/forge/src/cli.rs @@ -7,6 +7,9 @@ use std::net::SocketAddr; use std::path::PathBuf; use std::time::Duration; +/// The largest duration flag accepted: ten years, far beyond any sensible window. +const MAX_DURATION_SECS: u64 = 10 * 365 * 86_400; + #[derive(Parser)] #[command(name = "sc-forge", version, about = "A Gitea-compatible Git forge")] struct Cli { @@ -91,6 +94,16 @@ struct ServeArgs { /// Memory limit of each git transport child, bytes, Linux only (0 = none). #[arg(long, default_value_t = 8 << 30)] git_memory_limit: u64, + /// Seconds between garbage-collection passes over all repositories (0 = only on request through + /// `POST /api/v1/admin/cron/git_gc_repos`). + #[arg(long, default_value_t = 86_400)] + maintenance_interval: u64, + /// Seconds an unreachable object survives garbage collection (at least twice `--git-timeout` and an hour). + #[arg(long, default_value_t = 14 * 86_400)] + gc_prune_expire: u64, + /// Wall-clock limit of the garbage collection of one repository, seconds. + #[arg(long, default_value_t = 3_600)] + gc_timeout: u64, /// Concurrent password verifications (Argon2 is memory-hard). #[arg(long, default_value_t = 4)] argon2_concurrency: usize, @@ -264,11 +277,30 @@ fn run_serve(a: ServeArgs) -> Result<()> { ("--git-idle-timeout", a.git_idle_timeout), ("--max-pack-size", a.max_pack_size), ("--max-request-bytes", a.max_request_bytes), + ("--gc-timeout", a.gc_timeout), ] { if v == 0 { bail!("{flag} must be at least 1"); } } + let min_expire = scl_forge::maintenance::min_prune_expire(Duration::from_secs(a.git_timeout)); + if Duration::from_secs(a.gc_prune_expire) < min_expire { + bail!( + "--gc-prune-expire must be at least {} seconds (twice --git-timeout, and an hour): a shorter window can delete objects an operation in flight still needs", + min_expire.as_secs() + ); + } + // Durations are added to `Instant::now()` and subtracted from a unix time: an absurd value + // would panic the runner or, worse, turn a long prune window into "prune now". + for (flag, v) in [ + ("--maintenance-interval", a.maintenance_interval), + ("--gc-prune-expire", a.gc_prune_expire), + ("--gc-timeout", a.gc_timeout), + ] { + if v > MAX_DURATION_SECS { + bail!("{flag} must be at most {MAX_DURATION_SECS} seconds (ten years)"); + } + } bind_gate(&a.listen, a.behind_proxy, a.allow_plaintext).map_err(anyhow::Error::msg)?; let cidrs = a .trusted_proxies @@ -335,6 +367,16 @@ fn run_serve(a: ServeArgs) -> Result<()> { std::sync::Arc::get_mut(&mut app) .expect("the app has no other owner yet") .transports = scl_forge::cgi::Transports::new(tcfg); + std::sync::Arc::get_mut(&mut app) + .expect("the app has no other owner yet") + .maint = scl_forge::maintenance::Maintenance::new(scl_forge::maintenance::Cfg { + interval: Duration::from_secs(a.maintenance_interval), + prune_expire: Duration::from_secs(a.gc_prune_expire), + gc_timeout: Duration::from_secs(a.gc_timeout), + min_free: a.min_free_bytes, + ..Default::default() + }); + let _maintenance = scl_forge::maintenance::spawn(app.clone()); let limits = Limits { max_body: a.max_body_bytes, max_connections: a.max_connections, diff --git a/crates/forge/src/gitproc.rs b/crates/forge/src/gitproc.rs index 9d807c45..cdb1aba0 100644 --- a/crates/forge/src/gitproc.rs +++ b/crates/forge/src/gitproc.rs @@ -8,6 +8,7 @@ //! through [`Identity::new`]); this module never builds a revision from client text. use crate::limits::Gate; +use crate::registry::Cancel; use std::ffi::OsStr; use std::io::{Read, Write}; use std::os::unix::process::CommandExt; @@ -32,6 +33,9 @@ pub enum GitError { /// More output than the cap; the process group was killed. #[error("git produced too much output")] OutputTooLarge, + /// The caller's [`Cancel`] was set; the process group was killed. + #[error("git was cancelled")] + Cancelled, #[error("could not run git: {0}")] Spawn(#[from] std::io::Error), /// git ran and failed in a way that means "something is wrong", not "not found". @@ -105,6 +109,9 @@ pub struct Spec<'a> { /// Extra environment for this spawn, added after the pinned one (the `pre-receive` hook passes /// the object directories of git's quarantine this way). pub extra_env: &'a [(&'a str, &'a str)], + /// Ends the spawn (group killed, [`GitError::Cancelled`]) once set; checked before the spawn + /// and on every poll. + pub cancel: Option<&'a Cancel>, } /// The runner. @@ -184,6 +191,9 @@ impl Git { ("core.fsyncMethod".into(), "fsync".into()), ("core.useReplaceRefs".into(), "false".into()), ("gc.auto".into(), "0".into()), + // A maintenance `pack-refs` holds `packed-refs.lock` for a moment; a ref delete waits + // for it (git's default is one second). + ("core.packedRefsTimeout".into(), "5000".into()), ("receive.fsckObjects".into(), "true".into()), ]; config.extend(extra.iter().map(|(k, v)| (k.to_string(), v.clone()))); @@ -219,6 +229,9 @@ impl Git { .gate .acquire_timeout(self.admission) .ok_or(GitError::Busy)?; + if spec.cancel.is_some_and(Cancel::is_set) { + return Err(GitError::Cancelled); + } let mut cmd = Command::new(program); cmd.env_clear() .envs(self.env(spec.identity)) @@ -298,6 +311,9 @@ impl Git { if over.load(Ordering::SeqCst) { break Some(GitError::OutputTooLarge); } + if spec.cancel.is_some_and(Cancel::is_set) { + break Some(GitError::Cancelled); + } if Instant::now() >= deadline { break Some(GitError::Timeout { stdout: Vec::new() }); } @@ -679,6 +695,44 @@ mod tests { let _ = std::fs::remove_dir_all(d); } + #[test] + fn a_cancel_kills_the_group_and_a_set_one_prevents_the_spawn() { + let (g, d) = git("cancel"); + let cancel = Cancel::default(); + let setter = cancel.clone(); + let t = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(300)); + setter.cancel(); + }); + let started = Instant::now(); + let err = g + .run_program( + OsStr::new("/bin/sh"), + &Spec { + args: &["-c", "sleep 60 & echo $!; wait"], + cancel: Some(&cancel), + ..Default::default() + }, + ) + .unwrap_err(); + t.join().unwrap(); + assert!(matches!(err, GitError::Cancelled), "{err:?}"); + assert!(started.elapsed() < Duration::from_secs(10)); + // Already set: nothing starts (the marker file would exist if it had). + let marker = d.join("started"); + let again = g.run_program( + OsStr::new("/bin/sh"), + &Spec { + args: &["-c", &format!("touch {}", marker.display())], + cancel: Some(&cancel), + ..Default::default() + }, + ); + assert!(matches!(again, Err(GitError::Cancelled))); + assert!(!marker.exists()); + let _ = std::fs::remove_dir_all(d); + } + #[test] fn a_normal_exit_still_ends_a_lingering_background_child() { let (g, d) = git("linger"); diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index c9019083..797090c7 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -23,6 +23,7 @@ pub mod hook; pub mod http; pub mod lifecycle; pub mod limits; +pub mod maintenance; pub mod merges; pub mod names; pub mod orphans; diff --git a/crates/forge/src/lifecycle.rs b/crates/forge/src/lifecycle.rs index 41d6fa88..37c17898 100644 --- a/crates/forge/src/lifecycle.rs +++ b/crates/forge/src/lifecycle.rs @@ -189,6 +189,51 @@ impl Lifecycle { }) } + /// Run `f` as the only server-side writer of repository `id`: the write gate only, not the + /// lifecycle lock, so a `delete` is never held up behind it (maintenance cancels itself through + /// the registry instead). `None`: the gate stayed taken for `wait`. + pub fn with_gate(&self, id: i64, wait: Duration, f: impl FnOnce() -> T) -> Option { + let gate = self + .gates + .lock() + .unwrap_or_else(|e| e.into_inner()) + .entry(id) + .or_insert_with(|| Arc::new(crate::limits::Gate::new(1))) + .clone(); + let _writer = gate.acquire_timeout(wait)?; + Some(f()) + } + + /// Run `f` (a server-made write) under the shared lifecycle lock if it can be had right now, + /// never waiting: a delete queued for the exclusive lock would otherwise wait for a caller that + /// is itself waiting for the lock. Take the gate first. `None`: the lock was not free. + pub fn try_shared(&self, id: i64, f: impl FnOnce() -> T) -> Option { + let lock = self.lock_for(id); + let held = match lock.try_read() { + Ok(g) => g, + Err(std::sync::TryLockError::Poisoned(e)) => e.into_inner(), + Err(std::sync::TryLockError::WouldBlock) => return None, + }; + let r = f(); + drop(held); + Some(r) + } + + /// Run `f` while holding the repository's exclusive lifecycle lock, if it is free right now + /// (never waits): default-branch changes and branch deletes write refs under that lock alone, + /// so a cleanup that unlinks lock files needs it too. Take the gate first (see the lock order). + pub fn try_exclusive(&self, id: i64, f: impl FnOnce() -> T) -> Option { + let lock = self.lock_for(id); + let held = match lock.try_write() { + Ok(g) => g, + Err(std::sync::TryLockError::Poisoned(e)) => e.into_inner(), + Err(std::sync::TryLockError::WouldBlock) => return None, + }; + let r = f(); + drop(held); + Some(r) + } + /// `repos/.git`: always built from the integer id. pub fn repo_dir(&self, id: i64) -> PathBuf { self.data_dir.join("repos").join(format!("{id}.git")) diff --git a/crates/forge/src/maintenance.rs b/crates/forge/src/maintenance.rs new file mode 100644 index 00000000..751bb4aa --- /dev/null +++ b/crates/forge/src/maintenance.rs @@ -0,0 +1,416 @@ +//! Maintenance: garbage collection and the cleanup of what killed git writers leave behind. +//! +//! One pass visits every live repository in turn. For each it runs two stages (plan: +//! `docs/superpowers/plans/2026-10-24-p38-f5a-maintenance.md`): +//! +//! 1. `git gc --prune=never`, no gate: packs what refs reach and loosens unreachable packed objects, +//! deletes nothing, so it runs beside every other operation. +//! 2. Under the repository's write gate: every retention ref is reconciled (a failure skips the +//! prune), `git prune --expire=` removes the aged loose unreachable objects, and stale +//! ref locks and quarantine directories go. The gate keeps the API writers (which copy existing, +//! possibly old, objects into new trees) from referencing an object prune is about to unlink. +//! +//! Both stages run under one registry registration, like a transport: `delete` cancels the run +//! and waits for it before it renames the directory. The run never takes the lifecycle lock. + +use crate::gitproc::{GitError, Spec}; +use crate::registry::Cancel; +use crate::routes::App; +use crate::{db, merges, orphans, pulls, repos, timefmt, transport}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Condvar, Mutex}; +use std::time::{Duration, Instant}; + +/// Settings of the maintenance pass. +#[derive(Clone, Debug)] +pub struct Cfg { + /// Time between passes; zero means only on request. + pub interval: Duration, + /// Unreachable loose objects younger than this survive `prune`. + pub prune_expire: Duration, + /// Wall-clock limit of the gc of one repository. + pub gc_timeout: Duration, + /// Wall-clock limit of the prune, which holds the write gate (API writers wait on it). + pub prune_timeout: Duration, + /// Free disk that must remain beyond what a repack needs. + pub min_free: u64, + /// How long stage 2 waits for the repository's write gate before skipping the repository. + pub gate_wait: Duration, + /// Leftover locks and quarantine directories younger than this are left alone. + pub stale_age: Duration, +} + +impl Default for Cfg { + fn default() -> Cfg { + Cfg { + interval: Duration::from_secs(86_400), + prune_expire: Duration::from_secs(14 * 86_400), + gc_timeout: Duration::from_secs(3_600), + prune_timeout: Duration::from_secs(600), + min_free: 1 << 30, + gate_wait: Duration::from_secs(30), + stale_age: Duration::from_secs(600), + } + } +} + +/// The shortest `--gc-prune-expire` that keeps in-flight work safe: twice the longest git +/// operation, and at least an hour. +pub fn min_prune_expire(git_timeout: Duration) -> Duration { + (git_timeout * 2).max(Duration::from_secs(3_600)) +} + +#[derive(Default)] +struct State { + requested: bool, + running: bool, +} + +/// The maintenance schedule: a timer and on-demand requests share one runner thread ([`spawn`]). +pub struct Maintenance { + pub cfg: Cfg, + state: Mutex, + wake: Condvar, + passes: AtomicU64, +} + +impl Maintenance { + pub fn new(cfg: Cfg) -> Maintenance { + Maintenance { + cfg, + state: Mutex::default(), + wake: Condvar::new(), + passes: AtomicU64::new(0), + } + } + + fn state(&self) -> std::sync::MutexGuard<'_, State> { + self.state.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Ask for a pass now. `false` when one is already queued or running (it covers this request). + pub fn request(&self) -> bool { + let mut st = self.state(); + if st.requested || st.running { + return false; + } + st.requested = true; + self.wake.notify_all(); + true + } + + /// Passes finished since the server started. + pub fn passes(&self) -> u64 { + self.passes.load(Ordering::SeqCst) + } + + /// Is a pass queued or running? + pub fn busy(&self) -> bool { + let st = self.state(); + st.requested || st.running + } +} + +/// How one repository's maintenance ended. +#[derive(Debug, PartialEq, Eq)] +pub enum Outcome { + /// Both stages ran; sizes are disk usage in KiB. + Done { + before_kib: u64, + after_kib: u64, + }, + /// Stage 2 did not delete anything. + Skipped(&'static str), + /// The repository is gone or being deleted, or the server is stopping. + Gone, + Failed(String), +} + +/// What a pass did. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Pass { + pub repos: usize, + pub done: usize, + pub skipped: usize, + pub failed: usize, +} + +/// Start the runner thread: a pass every `cfg.interval` (if set) and whenever one is requested, +/// until the server stops. Passes run one at a time. +pub fn spawn(app: Arc) -> std::thread::JoinHandle<()> { + std::thread::spawn(move || { + let m = &app.maint; + let mut next = (!m.cfg.interval.is_zero()).then(|| Instant::now() + m.cfg.interval); + loop { + let requested = { + let mut st = m.state(); + if !st.requested { + st = m + .wake + .wait_timeout(st, Duration::from_secs(1)) + .unwrap_or_else(|e| e.into_inner()) + .0; + } + let r = std::mem::take(&mut st.requested); + st.running = r || next.is_some_and(|t| Instant::now() >= t); + r + }; + if app.life.registry.is_stopped() { + return; + } + if !m.state().running { + continue; + } + let pass = run_pass(&app); + eprintln!("sc-forge: maintenance pass finished: {pass:?} (requested: {requested})"); + // Not running again before the count moves, so a request that follows an observed + // increment is always accepted. + m.state().running = false; + m.passes.fetch_add(1, Ordering::SeqCst); + next = (!m.cfg.interval.is_zero()).then(|| Instant::now() + m.cfg.interval); + } + }) +} + +/// One pass over every live repository. +pub fn run_pass(app: &App) -> Pass { + let mut pass = Pass::default(); + let conn = match db::connect(&app.data_dir) { + Ok(c) => c, + Err(e) => { + eprintln!("sc-forge: maintenance cannot open the database: {e}"); + return pass; + } + }; + let ids: Vec = match repos::list_in_state(&conn, "live") { + Ok(r) => r.into_iter().map(|r| r.id).collect(), + Err(e) => { + eprintln!("sc-forge: maintenance cannot list the repositories: {e}"); + return pass; + } + }; + for id in ids { + if app.life.registry.is_stopped() { + break; + } + pass.repos += 1; + match maintain_repo(app, &conn, id) { + Outcome::Done { + before_kib, + after_kib, + } => { + pass.done += 1; + eprintln!( + "sc-forge: maintenance of repository {id}: {before_kib} KiB -> {after_kib} KiB" + ); + } + Outcome::Skipped(why) => { + pass.skipped += 1; + eprintln!("sc-forge: maintenance of repository {id} skipped: {why}"); + } + Outcome::Gone => {} + Outcome::Failed(e) => { + pass.failed += 1; + eprintln!("sc-forge: maintenance of repository {id} failed: {e}"); + } + } + } + pass +} + +fn fail(e: GitError) -> Outcome { + match e { + GitError::Cancelled => Outcome::Gone, + e => Outcome::Failed(e.to_string()), + } +} + +/// Disk usage of a repository in KiB: `size` (loose) plus `size-pack` from `count-objects -v`. +fn usage_kib(app: &App, dir: &std::path::Path, cancel: &Cancel) -> Result { + let out = app.life.git.run(&Spec { + repo: Some(dir), + args: &["count-objects", "-v"], + cancel: Some(cancel), + ..Default::default() + })?; + if !out.ok() { + return Err(GitError::Failed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )); + } + Ok(out + .text() + .lines() + .filter_map(|l| l.split_once(": ")) + .filter(|(k, _)| matches!(*k, "size" | "size-pack")) + .filter_map(|(_, v)| v.trim().parse::().ok()) + .sum()) +} + +/// Collect one repository. See the module documentation for the stages and why they are safe. +pub fn maintain_repo(app: &App, conn: &rusqlite::Connection, id: i64) -> Outcome { + let life = &app.life; + let cfg = &app.maint.cfg; + let dir = life.repo_dir(id); + let live = || matches!(repos::find_by_id(conn, id), Ok(Some(r)) if r.state == "live"); + let Some(reg) = life.registry.register(id, true, live) else { + return Outcome::Gone; + }; + let cancel = reg.cancel.clone(); + let run = |args: &[&str], timeout: Option| { + life.git.run(&Spec { + repo: Some(&dir), + args, + timeout, + cancel: Some(&cancel), + ..Default::default() + }) + }; + let check = |r: Result| match r { + Ok(o) if o.ok() => Ok(()), + Ok(o) => Err(Outcome::Failed( + String::from_utf8_lossy(&o.stderr).trim().to_string(), + )), + Err(e) => Err(fail(e)), + }; + + // Leftovers of killed writers go first: a stale ref lock would fail the gc and the reconcile + // below. Owners are excluded: the gate is held and ours is the only registration. + match in_gate(life, id, &cancel, cfg.gate_wait, || { + // The gate excludes the API writers and the registry the transports; default-branch and + // branch-delete writers hold only the exclusive lifecycle lock, so take that too (gate + // first, as everywhere). Anything still running means the leftovers wait for the next pass. + life.try_exclusive(id, || { + life.registry.with_only(®, || { + // Under the registry's server-wide lock: rename only, delete below. + orphans::clean_repo_deferred(&dir, Some(cfg.stale_age)) + }) + }) + }) { + Ok(Some(Some(_))) => { + orphans::remove_dead(&dir); + } + Ok(_) => eprintln!( + "sc-forge: repository {id}: stale lock cleanup deferred (a writer may be running)" + ), + // Optional work: a busy gate must not cost the repository its gc. + Err(Outcome::Gone) => return Outcome::Gone, + Err(_) => {} + } + + let before = match usage_kib(app, &dir, &cancel) { + Ok(k) => k, + Err(e) => return fail(e), + }; + + // Stage 1: nothing is deleted. A repack writes the new pack before it drops the old ones. + let free = transport::free_bytes(&app.data_dir).unwrap_or(0); + let mut note = None; + if free < cfg.min_free.saturating_add(before.saturating_mul(1024)) { + note = Some("not enough free disk for a repack"); + } else if let Err(o) = check(run( + &[ + "-c", + "gc.autoDetach=false", + "-c", + "gc.cruftPacks=false", + "-c", + "pack.threads=2", + "-c", + "pack.windowMemory=256m", + "gc", + "--prune=never", + "--force", + "--quiet", + ], + Some(cfg.gc_timeout), + )) { + if o == Outcome::Gone { + return o; + } + // Stage 2 does not depend on stage 1: it only removes aged loose objects. + eprintln!("sc-forge: gc of repository {id} failed: {o:?}"); + note = Some("gc failed"); + } + + // Stage 2, under the write gate. + if let Some(n) = note { + // Without a completed stage 1 reachable objects may still be loose and old; git's own gc + // does not prune after a failed repack either. + eprintln!("sc-forge: repository {id}: stage 1 did not complete ({n}); nothing pruned"); + return Outcome::Skipped("stage 1 did not complete"); + } + let stage2 = || -> Outcome { + if cancel.is_set() { + return Outcome::Gone; + } + let now = timefmt::now(); + // The reconcile writes refs and rows as a gated writer, so it holds the shared lifecycle + // lock like every other one (`delete_branch` relies on that); it never waits for it. + // A sync logs and skips a pull request it cannot repair, so success of the reconcile is not + // enough: the roots are verified separately. + let Some(roots) = life.try_shared(id, || { + pulls::reconcile_repo(conn, &life.git, &dir, id, now) + .map_err(|e| e.to_string()) + .and_then(|()| pulls::verify_roots(conn, &life.git, &dir, id)) + }) else { + return Outcome::Skipped("the repository is being changed (delete, default branch)"); + }; + if let Err(e) = roots { + // Without every retention ref confirmed, deleting could destroy the only copy. + eprintln!("sc-forge: repository {id}: retention refs could not be reconciled ({e}); nothing pruned"); + return Outcome::Skipped("retention refs could not be reconciled"); + } + // A merge intent still pending after the reconcile has a commit only the journal names. + // Fail closed: an unreadable journal is treated as a pending merge. + if !matches!(merges::pending_checked(conn, id), Ok(false)) { + return Outcome::Skipped("a merge is pending or the journal is unreadable"); + } + // Saturating, and never a cutoff in the future: an absurd window must not prune everything. + let window = i64::try_from(cfg.prune_expire.as_secs()).unwrap_or(i64::MAX); + let cutoff = now.saturating_sub(window); + if cutoff < 1 { + return Outcome::Skipped("the prune window reaches before the epoch"); + } + let expire = format!("--expire={cutoff}"); + if let Err(o) = check(run(&["prune", &expire], Some(cfg.prune_timeout))) { + return o; + } + match usage_kib(app, &dir, &cancel) { + Ok(after_kib) => Outcome::Done { + before_kib: before, + after_kib, + }, + Err(e) => fail(e), + } + }; + match in_gate(life, id, &cancel, cfg.gate_wait, stage2) { + Ok(o) => o, + Err(o) => o, + } +} + +/// Run `f` under repository `id`'s write gate. The wait is sliced so a cancel (a delete, shutdown) +/// is noticed; `Err` carries the outcome when `f` never ran. +fn in_gate( + life: &crate::lifecycle::Lifecycle, + id: i64, + cancel: &Cancel, + wait: Duration, + f: impl FnOnce() -> T, +) -> Result { + let mut once = Some(f); + let deadline = Instant::now() + wait; + loop { + if cancel.is_set() { + return Err(Outcome::Gone); + } + if let Some(r) = life.with_gate(id, Duration::from_millis(250), || (once.take().unwrap())()) + { + return Ok(r); + } + if Instant::now() >= deadline { + return Err(Outcome::Skipped("the write gate stayed taken")); + } + } +} diff --git a/crates/forge/src/merges.rs b/crates/forge/src/merges.rs index 37ec290d..feda595c 100644 --- a/crates/forge/src/merges.rs +++ b/crates/forge/src/merges.rs @@ -95,8 +95,14 @@ pub fn pending(conn: &Connection, repo_id: i64) -> rusqlite::Result> rows } -/// Is a merge of this repository unfinished? +/// Is a merge of this repository unfinished? A database error reads as "no": callers that must +/// fail closed use [`pending_checked`]. pub fn has_pending(conn: &Connection, repo_id: i64) -> bool { + pending_checked(conn, repo_id).unwrap_or(false) +} + +/// [`has_pending`] that reports a database error instead of answering "no". +pub fn pending_checked(conn: &Connection, repo_id: i64) -> rusqlite::Result { conn.query_row( "SELECT 1 FROM merge_intents WHERE repo_id = ?1 AND state = 'pending' LIMIT 1", [repo_id], @@ -104,7 +110,6 @@ pub fn has_pending(conn: &Connection, repo_id: i64) -> bool { ) .optional() .map(|r| r.is_some()) - .unwrap_or(false) } fn insert(conn: &Connection, i: &Intent, now: i64) -> rusqlite::Result { @@ -225,6 +230,14 @@ fn finish( i.pull_number ); } + // The marker goes below; from here on this ref is what keeps the merge commit alive when the + // base is rewritten away from it. + if let Err(e) = pulls::retain_at(git, dir, &pulls::merge_ref(i.pull_number), &i.merge_oid) { + eprintln!( + "sc-forge: could not retain the merge commit of pull request {}: {e}", + i.pull_number + ); + } let mut last = None; for k in 0..3 { match finalize_rows(conn, i, recovered) { @@ -252,7 +265,7 @@ fn finish( } /// Does the commit exist in the repository? -fn commit_exists(git: &Git, dir: &Path, oid: &str) -> Result { +pub(crate) fn commit_exists(git: &Git, dir: &Path, oid: &str) -> Result { let out = git.run(&Spec { repo: Some(dir), args: &["cat-file", "-e", &format!("{oid}^{{commit}}")], @@ -295,7 +308,8 @@ fn mark_landed(git: &Git, dir: &Path, i: &Intent) { /// Is a git writer holding the base ref right now (its lock file exists)? At run time that is a /// live writer (a push, or our own publish): the intent waits rather than being judged under it. -/// The lock of a *dead* writer is cleared at startup (`orphans::sweep`), not guessed at here. +/// The lock of a *dead* writer is cleared at startup (`orphans::sweep`) or by the maintenance pass +/// once its writers are excluded (`orphans::clean_repo`), not guessed at here. fn base_is_locked(dir: &Path, base_ref: &str) -> bool { dir.join("refs") .join("heads") @@ -448,8 +462,8 @@ fn publish(git: &Git, dir: &Path, i: &Intent) -> Result { }) { Ok(o) => Ok(o.ok()), // Killed by the runner's deadline. SIGKILL leaves the locks of the refs it had prepared; they - // stay until the next restart's cleanup, because a lock cannot be told from a live writer's - // at run time (the intent stays pending while the base is locked). + // stay until the next restart or maintenance pass cleans them, because a lock cannot be told + // from a live writer's here (the intent stays pending while the base is locked). Err(GitError::Timeout { .. }) => Ok(false), Err(e) => Err(e), } diff --git a/crates/forge/src/orphans.rs b/crates/forge/src/orphans.rs index c55e4c2d..184e70e2 100644 --- a/crates/forge/src/orphans.rs +++ b/crates/forge/src/orphans.rs @@ -7,8 +7,8 @@ //! server (see `lifecycle::DirLock`), so at startup every git process that names a path under this //! data directory's `repos/` belongs to a dead predecessor; they are killed, and the `*.lock` files //! a killed git leaves in `refs/` (nothing else holds them now) are removed, before any recovery -//! looks at the repositories. At run time nothing here runs: a live lock then belongs to a live -//! writer. +//! looks at the repositories. At run time only `clean_repo` runs, from the maintenance pass, and +//! only when the repository's writers are excluded (see `maintenance.rs`). use std::path::Path; use std::time::{Duration, Instant}; @@ -184,7 +184,18 @@ pub fn kill_orphan_git(data_dir: &Path, me: i32, git_bin: Option<&Path>) -> Opti } } -fn remove_locks(dir: &Path, depth: usize) -> usize { +/// Is `p` at least `min_age` old (by modification time)? No age means "always"; an unreadable +/// time counts as young, so a runtime cleanup leaves it alone. +fn old_enough(p: &Path, min_age: Option) -> bool { + let Some(min) = min_age else { return true }; + std::fs::symlink_metadata(p) + .and_then(|m| m.modified()) + .ok() + .and_then(|t| t.elapsed().ok()) + .is_some_and(|age| age >= min) +} + +fn remove_locks(dir: &Path, depth: usize, min_age: Option) -> usize { let Ok(rd) = std::fs::read_dir(dir) else { return 0; }; @@ -193,9 +204,10 @@ fn remove_locks(dir: &Path, depth: usize) -> usize { let p = e.path(); let Ok(t) = e.file_type() else { continue }; if t.is_dir() && depth < 32 { - n += remove_locks(&p, depth + 1); + n += remove_locks(&p, depth + 1, min_age); } else if t.is_file() && p.extension().is_some_and(|x| x == "lock") + && old_enough(&p, min_age) && std::fs::remove_file(&p).is_ok() { eprintln!( @@ -208,21 +220,75 @@ fn remove_locks(dir: &Path, depth: usize) -> usize { n } -/// Remove the leftover ref locks of every repository (`refs/**/*.lock`, `packed-refs.lock`, -/// `HEAD.lock`). Only after [`kill_orphan_git`], and only at startup. -pub fn remove_stale_locks(data_dir: &Path) -> usize { - let mut n = 0; - let Ok(rd) = std::fs::read_dir(data_dir.join("repos")) else { +/// The name receive-pack gives its quarantine directory under `objects/` (git 2.35 and later; +/// `MIN_GIT` is 2.40): `tmp_objdir-incoming-XXXXXX`. A test pins it against a real push. +pub const QUARANTINE_PREFIX: &str = "tmp_objdir-incoming-"; + +/// Remove what a killed git writer left in one repository: ref locks (`refs/**/*.lock`, +/// `packed-refs.lock`, `HEAD.lock`), the commit-graph locks a killed `gc` leaves (every later gc +/// fails on them) and receive-pack's quarantine directories. +/// With `min_age` only items at least that old go. The caller must know no git writer of the +/// repository is alive (startup: after [`kill_orphan_git`]; at run time: the write gate is held and +/// no transport is registered). +pub fn clean_repo(repo: &Path, min_age: Option) -> usize { + let n = clean_inner(repo, min_age, false); + n + remove_dead(repo) +} + +/// [`clean_repo`] for a caller that holds a server-wide lock: quarantine directories (which can be +/// gigabytes) are only renamed to `objects/dead-*`, and [`remove_dead`] deletes them afterwards. +pub fn clean_repo_deferred(repo: &Path, min_age: Option) -> usize { + clean_inner(repo, min_age, true) +} + +/// Delete the directories [`clean_repo_deferred`] set aside. +pub fn remove_dead(repo: &Path) -> usize { + let Ok(rd) = std::fs::read_dir(repo.join("objects")) else { return 0; }; - for e in rd.flatten() { - let repo = e.path(); - if !repo.is_dir() { - continue; + rd.flatten() + .filter(|e| e.file_name().to_string_lossy().starts_with("dead-")) + .filter(|e| e.file_type().is_ok_and(|t| t.is_dir())) + .filter(|e| std::fs::remove_dir_all(e.path()).is_ok()) + .count() +} + +fn clean_inner(repo: &Path, min_age: Option, rename_only: bool) -> usize { + let mut n = remove_locks(&repo.join("refs"), 0, min_age); + for f in [ + "packed-refs.lock", + "HEAD.lock", + "objects/info/commit-graph.lock", + "objects/info/commit-graphs/commit-graph-chain.lock", + ] { + let p = repo.join(f); + if old_enough(&p, min_age) && std::fs::remove_file(&p).is_ok() { + n += 1; } - n += remove_locks(&repo.join("refs"), 0); - for f in ["packed-refs.lock", "HEAD.lock"] { - if std::fs::remove_file(repo.join(f)).is_ok() { + } + if let Ok(rd) = std::fs::read_dir(repo.join("objects")) { + for e in rd.flatten() { + let p = e.path(); + let named = e + .file_name() + .to_string_lossy() + .starts_with(QUARANTINE_PREFIX); + if named + && e.file_type().is_ok_and(|t| t.is_dir()) + && old_enough(&p, min_age) + && if rename_only { + let to = repo + .join("objects") + .join(format!("dead-{}", e.file_name().to_string_lossy())); + std::fs::rename(&p, to).is_ok() + } else { + std::fs::remove_dir_all(&p).is_ok() + } + { + eprintln!( + "sc-forge: removed the quarantine {} left by a killed git", + p.display() + ); n += 1; } } @@ -230,6 +296,19 @@ pub fn remove_stale_locks(data_dir: &Path) -> usize { n } +/// [`clean_repo`] for every repository, regardless of age. Only after [`kill_orphan_git`], and +/// only at startup. +pub fn remove_stale_locks(data_dir: &Path) -> usize { + let Ok(rd) = std::fs::read_dir(data_dir.join("repos")) else { + return 0; + }; + rd.flatten() + .map(|e| e.path()) + .filter(|p| p.is_dir()) + .map(|p| clean_repo(&p, None)) + .sum() +} + /// The startup cleanup. `data_dir` must be the canonical data directory; `git_bin` is the git /// executable the server runs. Locks are removed only /// when no matching process is left alive: a lock of a writer that could not be killed is not ours @@ -356,4 +435,45 @@ mod tests { std::fs::remove_dir_all(&d).unwrap(); assert!(!d.exists()); } + + #[test] + fn quarantine_directories_go_with_the_locks_and_age_is_honoured() { + let d = std::env::temp_dir().join(format!("scl-forge-orphans-q-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + let repo = d.join("repos").join("1.git"); + std::fs::create_dir_all(repo.join("objects/tmp_objdir-incoming-aB3xYz/pack")).unwrap(); + std::fs::create_dir_all(repo.join("objects/pack")).unwrap(); + std::fs::create_dir_all(repo.join("refs/heads")).unwrap(); + std::fs::write( + repo.join("objects/tmp_objdir-incoming-aB3xYz/pack/tmp"), + b"x", + ) + .unwrap(); + std::fs::write(repo.join("refs/heads/a.lock"), b"").unwrap(); + std::fs::create_dir_all(repo.join("objects/info/commit-graphs")).unwrap(); + std::fs::write(repo.join("objects/info/commit-graph.lock"), b"").unwrap(); + // Young items stay when an age is given; the unconditional startup form removes them. + assert_eq!(clean_repo(&repo, Some(Duration::from_secs(3600))), 0); + assert!(repo.join("objects/tmp_objdir-incoming-aB3xYz").exists()); + // The deferred form only sets the quarantine aside; removing what it set aside is separate. + assert_eq!(clean_repo_deferred(&repo, None), 3); + assert!(!repo.join("objects/tmp_objdir-incoming-aB3xYz").exists()); + assert!(repo + .join("objects/dead-tmp_objdir-incoming-aB3xYz") + .exists()); + assert_eq!(remove_dead(&repo), 1); + assert!(!repo + .join("objects/dead-tmp_objdir-incoming-aB3xYz") + .exists()); + std::fs::create_dir_all(repo.join("objects/tmp_objdir-incoming-q2/pack")).unwrap(); + std::fs::write(repo.join("refs/heads/a.lock"), b"").unwrap(); + assert_eq!(remove_stale_locks(&d), 2); + assert!(!repo.join("objects/tmp_objdir-incoming-q2").exists()); + assert!( + repo.join("objects/pack").exists(), + "only quarantine directories go" + ); + std::fs::remove_dir_all(&d).unwrap(); + assert!(!d.exists()); + } } diff --git a/crates/forge/src/pulls.rs b/crates/forge/src/pulls.rs index 08df78f5..a5830334 100644 --- a/crates/forge/src/pulls.rs +++ b/crates/forge/src/pulls.rs @@ -23,6 +23,18 @@ pub fn retention_ref(n: i64) -> String { format!("refs/forge/pull/{n}/head") } +/// The ref that keeps the base commit a pull request was last opened or retargeted against, for +/// when its base branch is gone and the stored commit is all that remains. +pub fn base_ref(n: i64) -> String { + format!("refs/forge/pull/{n}/base") +} + +/// The ref that keeps the merge commit of merged pull request `n` alive after the base moves on +/// (the landing marker is retired once the merge is final). +pub fn merge_ref(n: i64) -> String { + format!("refs/forge/pull/{n}/merge") +} + /// A stored pull request. #[derive(Clone, Debug)] pub struct Pull { @@ -398,10 +410,14 @@ pub fn refs_under( /// Point PR `n`'s retention ref at `oid` (a commit that exists), whatever it holds now. For the /// authoritative writers: PR creation, close, and the record of a head that is about to be deleted. pub fn retain(git: &Git, dir: &Path, n: i64, oid: &str) -> Result<(), GitError> { - let r = retention_ref(n); + retain_at(git, dir, &retention_ref(n), oid) +} + +/// Point `r` (one of the `refs/forge/pull//…` refs) at `oid`, whatever it holds now. +pub fn retain_at(git: &Git, dir: &Path, r: &str, oid: &str) -> Result<(), GitError> { let out = git.run(&Spec { repo: Some(dir), - args: &["update-ref", &r, oid], + args: &["update-ref", r, oid], ..Default::default() })?; if out.ok() { @@ -573,13 +589,47 @@ pub fn reconcile_repo( crate::merges::drop_stale_receipts(conn, git, dir, repo_id)?; sync_locked(conn, git, dir, repo_id, now)?; let kept = refs_under(git, dir, "refs/forge/pull")?; - let mut st = - conn.prepare("SELECT number, state, final_head_sha FROM pulls WHERE repo_id = ?1")?; - let rows: Vec<(i64, String, Option)> = st - .query_map([repo_id], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)))? + let mut st = conn.prepare( + "SELECT number, state, final_head_sha, merge_commit_sha, base_sha FROM pulls WHERE repo_id = ?1", + )?; + // (number, state, final head, merge commit, base) + type Row = (i64, String, Option, Option, String); + let rows: Vec = st + .query_map([repo_id], |r| { + Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?, r.get(4)?)) + })? .collect::>()?; - let known: HashMap)> = - rows.into_iter().map(|(n, s, h)| (n, (s, h))).collect(); + let known: HashMap)> = rows + .iter() + .map(|(n, s, h, _, _)| (*n, (s.clone(), h.clone()))) + .collect(); + // A merged pull request's merge commit stays reachable even after the base is rewritten. A + // repair that fails is remembered, not returned at once: the other repairs still run. + let mut failed = None; + for (n, state, _, merge, base) in &rows { + if let (true, Some(m)) = (state == "merged", merge) { + if kept.get(&merge_ref(*n)) != Some(m) { + if let Err(e) = retain_at(git, dir, &merge_ref(*n), m) { + failed.get_or_insert(e); + } + } + } + // The stored base is what an unmerged PR shows once its base branch is gone. A commit that + // no longer exists cannot be kept (and nothing is lost by not keeping it). + if state != "merged" && kept.get(&base_ref(*n)) != Some(base) { + match crate::merges::commit_exists(git, dir, base) { + Ok(true) => { + if let Err(e) = retain_at(git, dir, &base_ref(*n), base) { + failed.get_or_insert(e); + } + } + Ok(false) => {} + Err(e) => { + failed.get_or_insert(e); + } + } + } + } for p in open_pulls(conn, repo_id)? { // Reopened without its branch: the stored head is the only anchor left. if p.head_gone && kept.get(&retention_ref(p.number)) != Some(&p.head_sha) { @@ -598,7 +648,11 @@ pub fn reconcile_repo( for name in kept.keys() { let n = name .strip_prefix("refs/forge/pull/") - .and_then(|r| r.strip_suffix("/head")) + .and_then(|r| { + r.strip_suffix("/head") + .or_else(|| r.strip_suffix("/merge")) + .or_else(|| r.strip_suffix("/base")) + }) .and_then(|n| n.parse::().ok()); if n.is_some_and(|n| !known.contains_key(&n)) { let _ = git.run(&Spec { @@ -608,6 +662,55 @@ pub fn reconcile_repo( }); } } + match failed { + Some(e) => Err(e.into()), + None => Ok(()), + } +} + +/// Does a ref still hold every commit a pull request row names? The check before anything is +/// pruned, independent of how [`reconcile_repo`] went (a sync logs and skips a PR it cannot +/// repair): an open PR needs its head branch or its retention ref at the stored head; a closed or +/// merged one its retention ref at the final head; a merged one also its merge ref. +pub fn verify_roots(conn: &Connection, git: &Git, dir: &Path, repo_id: i64) -> Result<(), String> { + let e = |e: &dyn std::fmt::Display| e.to_string(); + let kept = refs_under(git, dir, "refs/forge/pull").map_err(|x| e(&x))?; + let heads = refs_under(git, dir, "refs/heads").map_err(|x| e(&x))?; + let mut st = conn + .prepare("SELECT number FROM pulls WHERE repo_id = ?1") + .map_err(|x| e(&x))?; + let numbers: Vec = st + .query_map([repo_id], |r| r.get(0)) + .map_err(|x| e(&x))? + .collect::>() + .map_err(|x| e(&x))?; + for n in numbers { + let Some(p) = find(conn, repo_id, n).map_err(|x| e(&x))? else { + continue; + }; + let held = |r: String, want: &str| kept.get(&r).is_some_and(|have| have == want); + let ok = match p.state.as_str() { + "open" => { + (!p.head_gone && head_tip(&heads, &p.head_ref).is_some()) + || held(retention_ref(n), &p.head_sha) + } + _ => p + .final_head + .as_deref() + .is_none_or(|h| held(retention_ref(n), h)), + }; + let merge_ok = p.state != "merged" + || p.merge_commit + .as_deref() + .is_none_or(|m| held(merge_ref(n), m)); + // A base commit that exists must be held; one that is already gone cannot be. + let base_ok = p.state == "merged" + || held(base_ref(n), &p.base_sha) + || !crate::merges::commit_exists(git, dir, &p.base_sha).map_err(|x| e(&x))?; + if !ok || !merge_ok || !base_ok { + return Err(format!("pull request {n} is not held by a ref")); + } + } Ok(()) } diff --git a/crates/forge/src/registry.rs b/crates/forge/src/registry.rs index 26a9ce37..ecad9673 100644 --- a/crates/forge/src/registry.rs +++ b/crates/forge/src/registry.rs @@ -62,7 +62,8 @@ impl Drop for CancelOnDrop { #[derive(Default)] struct Slot { closed: bool, - live: HashMap, + /// Cancel handle and whether the run may write (a push, a maintenance pass). + live: HashMap, } /// Live transports by repository id. @@ -94,6 +95,7 @@ impl Registry { pub fn register( self: &Arc, repo: i64, + writes: bool, still_live: impl FnOnce() -> bool, ) -> Option { let mut slots = self.lock(); @@ -109,7 +111,7 @@ impl Registry { .entry(repo) .or_default() .live - .insert(key, cancel.clone()); + .insert(key, (cancel.clone(), writes)); Some(Registration { reg: self.clone(), repo, @@ -123,7 +125,7 @@ impl Registry { let mut slots = self.lock(); let slot = slots.entry(repo).or_default(); slot.closed = true; - slot.live.values().for_each(Cancel::cancel); + slot.live.values().for_each(|(c, _)| c.cancel()); } /// Allow transports on `repo` again (a delete that did not go through). @@ -154,6 +156,24 @@ impl Registry { true } + /// Run `f` only if every other registration on `mine`'s repository is read-only (an upload-pack + /// writes no locks and no quarantine) and the slot is open, holding the registry lock for the + /// duration, so no writer can register meanwhile. `f` must be short (file system calls only) + /// and must not call back into the registry. + pub fn with_only(&self, mine: &Registration, f: impl FnOnce() -> T) -> Option { + let slots = self.lock(); + match slots.get(&mine.repo) { + Some(s) + if !s.closed + && s.live.contains_key(&mine.key) + && s.live.iter().all(|(k, (_, w))| *k == mine.key || !*w) => + { + Some(f()) + } + _ => None, + } + } + /// Drop the (closed) slot of a repository that no longer exists. pub fn forget(&self, repo: i64) { self.lock().remove(&repo); @@ -172,7 +192,7 @@ impl Registry { slots .values() .flat_map(|s| s.live.values()) - .for_each(Cancel::cancel); + .for_each(|(c, _)| c.cancel()); } /// Wait up to `limit` until no transport at all is registered. @@ -218,13 +238,13 @@ mod tests { #[test] fn a_closed_slot_refuses_and_cancels() { let r = Arc::new(Registry::default()); - let a = r.register(1, || true).expect("open"); + let a = r.register(1, true, || true).expect("open"); assert!(!a.cancel.is_set()); r.close(1); assert!(a.cancel.is_set(), "close cancels what is live"); - assert!(r.register(1, || true).is_none(), "closed refuses"); + assert!(r.register(1, true, || true).is_none(), "closed refuses"); assert!( - r.register(2, || true).is_some(), + r.register(2, true, || true).is_some(), "other repositories are unaffected" ); assert!( @@ -234,20 +254,20 @@ mod tests { drop(a); assert!(r.wait_idle(1, Duration::from_millis(20))); r.reopen(1); - assert!(r.register(1, || true).is_some()); + assert!(r.register(1, true, || true).is_some()); } #[test] fn the_liveness_check_runs_under_the_lock_and_can_refuse() { let r = Arc::new(Registry::default()); - assert!(r.register(1, || false).is_none()); + assert!(r.register(1, true, || false).is_none()); assert_eq!(r.live(), 0); } #[test] fn wait_idle_wakes_when_the_last_transport_leaves() { let r = Arc::new(Registry::default()); - let a = r.register(7, || true).unwrap(); + let a = r.register(7, true, || true).unwrap(); r.close(7); let t = std::thread::spawn({ let r = r.clone(); @@ -261,8 +281,8 @@ mod tests { #[test] fn shutdown_cancels_everything() { let r = Arc::new(Registry::default()); - let a = r.register(1, || true).unwrap(); - let b = r.register(2, || true).unwrap(); + let a = r.register(1, true, || true).unwrap(); + let b = r.register(2, true, || true).unwrap(); r.cancel_all(); assert!(a.cancel.is_set() && b.cancel.is_set()); assert!(!r.wait_all_idle(Duration::from_millis(10))); diff --git a/crates/forge/src/routes.rs b/crates/forge/src/routes.rs index ce90b3b0..3ea02ceb 100644 --- a/crates/forge/src/routes.rs +++ b/crates/forge/src/routes.rs @@ -28,6 +28,8 @@ pub struct App { pub life: Lifecycle, /// Admission and limits of the git transport. pub transports: crate::cgi::Transports, + /// Garbage collection and cleanup; the runner thread is started by `serve`. + pub maint: crate::maintenance::Maintenance, } impl App { @@ -48,6 +50,7 @@ impl App { proxies, life, transports: crate::cgi::Transports::new(crate::cgi::Cfg::default()), + maint: crate::maintenance::Maintenance::new(crate::maintenance::Cfg::default()), } } @@ -222,6 +225,14 @@ pub static ROUTES: &[Route] = &[ SiteAdmin, api::admin_patch_user, ), + r( + "POST", + "/admin/cron/{task}", + Token, + n(Cat::Admin, Level::Write), + SiteAdmin, + api::admin_cron, + ), r( "POST", "/orgs", diff --git a/crates/forge/tests/common/mod.rs b/crates/forge/tests/common/mod.rs index ed79e0f7..597404a9 100644 --- a/crates/forge/tests/common/mod.rs +++ b/crates/forge/tests/common/mod.rs @@ -31,6 +31,13 @@ pub struct Srv { thread: Option>, } +/// Optional extras of a test server. +#[derive(Default)] +pub struct Extra { + pub maint: Option, + pub git_bin: Option, +} + pub struct Resp { pub status: u16, pub headers: HashMap, @@ -97,16 +104,32 @@ impl Srv { /// Start with a runner of `slots` concurrent git children and the given admission deadline. pub fn start_git(slots: usize, admission: std::time::Duration) -> Srv { - Srv::start_full(Limits::default(), None, Some((slots, admission)), None) + Srv::start_full( + Limits::default(), + None, + Some((slots, admission)), + None, + Extra::default(), + ) } fn start_cfg(limits: Limits, backoff: Option) -> Srv { - Srv::start_full(limits, backoff, None, None) + Srv::start_full(limits, backoff, None, None, Extra::default()) } /// Start with custom git transport limits. pub fn start_transport(cfg: scl_forge::cgi::Cfg) -> Srv { - Srv::start_full(Limits::default(), None, None, Some(cfg)) + Srv::start_full(Limits::default(), None, None, Some(cfg), Extra::default()) + } + + /// Start with the maintenance runner (timer off, passes on request) and, optionally, a + /// different git executable (a wrapper that slows chosen subcommands). + pub fn start_maint(cfg: scl_forge::maintenance::Cfg, git_bin: Option) -> Srv { + let extra = Extra { + maint: Some(cfg), + git_bin, + }; + Srv::start_full(Limits::default(), None, None, None, extra) } fn start_full( @@ -114,6 +137,7 @@ impl Srv { backoff: Option, git_cfg: Option<(usize, std::time::Duration)>, tcfg: Option, + extra: Extra, ) -> Srv { let dir = std::env::temp_dir().join(format!( "scl-forge-e2e-{}-{}", @@ -141,9 +165,16 @@ impl Srv { let data_lock = scl_forge::lifecycle::DirLock::acquire(&dir).unwrap(); let (slots, admission) = git_cfg.unwrap_or((8, std::time::Duration::from_secs(5))); let git = std::sync::Arc::new( - scl_forge::gitproc::Git::new(&dir, scl_forge::gitproc::find_git().expect("git"), slots) - .unwrap() - .with_limits(std::time::Duration::from_secs(30), admission, 16 << 20), + scl_forge::gitproc::Git::new( + &dir, + extra + .git_bin + .clone() + .unwrap_or_else(|| scl_forge::gitproc::find_git().expect("git")), + slots, + ) + .unwrap() + .with_limits(std::time::Duration::from_secs(30), admission, 16 << 20), ); scl_forge::hook::install( git.data_dir(), @@ -158,8 +189,16 @@ impl Srv { if let Some(c) = tcfg { app.transports = scl_forge::cgi::Transports::new(c); } + let with_runner = extra.maint.is_some(); + if let Some(c) = extra.maint { + app.maint = scl_forge::maintenance::Maintenance::new(c); + } let app = std::sync::Arc::new(app); let app_handle = app.clone(); + if with_runner { + // The runner thread lives until the registry stops (server drop). + let _ = scl_forge::maintenance::spawn(app.clone()); + } let (tx, rx) = tokio::sync::oneshot::channel(); let thread = std::thread::spawn(move || { let rt = tokio::runtime::Builder::new_multi_thread() diff --git a/crates/forge/tests/e2e_maintenance.rs b/crates/forge/tests/e2e_maintenance.rs new file mode 100644 index 00000000..4910fe40 --- /dev/null +++ b/crates/forge/tests/e2e_maintenance.rs @@ -0,0 +1,768 @@ +//! F5a: garbage collection and the cleanup of what killed git writers leave behind. +#![cfg(unix)] + +mod common; +use common::*; +use scl_forge::maintenance::{self, Cfg, Outcome}; +use serde_json::{json, Value}; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::time::{Duration, SystemTime}; + +static N: AtomicUsize = AtomicUsize::new(0); + +/// A git wrapper that logs every invocation and, while `hold` exists, blocks the first +/// invocation that has `sub` among its arguments (touching `started` when it begins to wait). +struct Wrap { + dir: PathBuf, +} + +impl Wrap { + fn new(sub: &str) -> Wrap { + let dir = std::env::temp_dir().join(format!( + "scl-maint-wrap-{}-{}", + std::process::id(), + N.fetch_add(1, Ordering::SeqCst) + )); + std::fs::create_dir_all(&dir).unwrap(); + let real = scl_forge::gitproc::find_git().expect("git"); + let d = dir.display(); + let script = format!( + "#!/bin/sh\necho \"$@\" >> {d}/log\nfor a in \"$@\"; do\n if [ \"$a\" = {sub} ] && [ -e {d}/hold ]; then\n touch {d}/started\n while [ -e {d}/hold ]; do sleep 0.05; done\n fi\ndone\nexec {} \"$@\"\n", + real.display() + ); + let bin = dir.join("git"); + std::fs::write(&bin, script).unwrap(); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).unwrap(); + Wrap { dir } + } + fn bin(&self) -> PathBuf { + self.dir.join("git") + } + fn hold(&self) { + std::fs::write(self.dir.join("hold"), "").unwrap(); + } + fn release(&self) { + let _ = std::fs::remove_file(self.dir.join("hold")); + } + fn wait_started(&self) { + wait_for("the wrapped git to start waiting", || { + self.dir.join("started").exists() + }); + } + fn log(&self) -> String { + std::fs::read_to_string(self.dir.join("log")).unwrap_or_default() + } +} + +impl Drop for Wrap { + fn drop(&mut self) { + self.release(); + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +fn wait_for(what: &str, mut f: impl FnMut() -> bool) { + let end = std::time::Instant::now() + Duration::from_secs(60); + while !f() { + assert!( + std::time::Instant::now() < end, + "timed out waiting for {what}" + ); + std::thread::sleep(Duration::from_millis(25)); + } +} + +fn cfg() -> Cfg { + Cfg { + interval: Duration::ZERO, + min_free: 0, + gate_wait: Duration::from_secs(5), + ..Cfg::default() + } +} + +struct W { + s: Srv, + auth: String, + id: i64, +} + +fn world_with(cfg: Cfg, git_bin: Option) -> W { + let s = Srv::start_maint(cfg, git_bin); + let auth = token(&s.admin); + let r = s.post( + "/api/v1/user/repos", + &auth, + json!({"name": "w", "default_branch": "main", "auto_init": true}), + ); + assert_eq!(r.status, 201, "{}", r.msg()); + let id = r.json()["id"].as_i64().unwrap(); + W { s, auth, id } +} + +fn world() -> W { + world_with(cfg(), None) +} + +impl W { + fn api(&self, method: &str, path: &str, body: Option) -> Resp { + self.s.send( + method, + &format!("/api/v1/repos/siteadmin/w{path}"), + Some(&self.auth), + body.as_ref(), + ) + } + fn git(&self, args: &[&str]) -> String { + self.s.git(self.id, args, None) + } + fn exists(&self, oid: &str) -> bool { + self.s.git_try(self.id, &["cat-file", "-e", oid]).is_some() + } + fn repo(&self) -> PathBuf { + self.s.dir.join("repos").join(format!("{}.git", self.id)) + } + fn tip(&self, branch: &str) -> String { + self.git(&["rev-parse", &format!("refs/heads/{branch}")]) + } + /// An unreferenced blob, loose. + fn loose(&self, text: &str) -> String { + self.s.git( + self.id, + &["hash-object", "-w", "--stdin"], + Some(text.as_bytes()), + ) + } + fn loose_path(&self, oid: &str) -> PathBuf { + self.repo().join("objects").join(&oid[..2]).join(&oid[2..]) + } + fn commit(&self, branch: &str, file: &str, text: &str, from: Option<&str>) -> String { + let mut body = json!({"content": b64(text), "message": format!("add {file}"), "branch": from.unwrap_or(branch)}); + if from.is_some() { + body["new_branch"] = json!(branch); + } + let r = self.api("POST", &format!("/contents/{file}"), Some(body)); + assert_eq!(r.status, 201, "{}", r.msg()); + self.tip(branch) + } + /// One pass on request, waited for. + fn pass(&self) { + let before = self.s.app.maint.passes(); + let r = self.s.send( + "POST", + "/api/v1/admin/cron/git_gc_repos", + Some(&self.auth), + None, + ); + assert_eq!(r.status, 204, "{}", r.s_text()); + wait_for("the maintenance pass", || { + self.s.app.maint.passes() > before + }); + } + /// One repository's maintenance, in this thread. + fn maintain(&self) -> Outcome { + let conn = scl_forge::db::connect(&self.s.dir).unwrap(); + maintenance::maintain_repo(&self.s.app, &conn, self.id) + } + fn fsck(&self) { + let out = self + .s + .git_try(self.id, &["fsck", "--strict", "--no-dangling"]); + assert!(out.is_some(), "git fsck failed"); + } +} + +fn age(p: &Path, days: u64) { + // Loose objects are read-only files; setting the time needs only ownership. + let f = std::fs::File::open(p).unwrap(); + f.set_modified(SystemTime::now() - Duration::from_secs(days * 86_400)) + .unwrap(); +} + +fn age_dir(p: &Path, days: u64) { + // A directory cannot be opened for writing: use the std handle on a read-only open. + let f = std::fs::File::open(p).unwrap(); + f.set_modified(SystemTime::now() - Duration::from_secs(days * 86_400)) + .unwrap(); +} + +#[test] +fn aged_unreferenced_objects_go_young_and_reachable_ones_stay() { + let w = world(); + let head = w.commit("f", "f.txt", "f\n", Some("main")); + let old_loose = w.loose("old loose"); + age(&w.loose_path(&old_loose), 30); + // An old unreferenced object inside a pack: reach it through a temporary ref while packing + // (git 2.40 only packs loose unreachable objects next to an existing pack), then drop the ref. + let old_packed = w.loose("old packed"); + w.git(&["update-ref", "refs/tmp/keep", &old_packed]); + w.git(&["repack", "-a", "-d", "-l"]); + w.git(&["update-ref", "-d", "refs/tmp/keep"]); + for e in std::fs::read_dir(w.repo().join("objects/pack")) + .unwrap() + .flatten() + { + age(&e.path(), 30); + } + assert!(!w.loose_path(&old_packed).exists(), "the object is packed"); + let young_loose = w.loose("young loose"); + assert!(w.exists(&old_packed)); + w.pass(); + assert!( + !w.exists(&old_loose), + "an old loose object must be collected" + ); + assert!( + !w.exists(&old_packed), + "an old packed object must be collected" + ); + assert!(w.exists(&young_loose), "a young object survives the window"); + assert!(w.exists(&head), "a branch tip survives"); + assert_eq!(w.tip("f"), head); + w.fsck(); + // The data is intact end to end: the file reads back through the API. + let r = w.api("GET", "/contents/f.txt?ref=f", None); + assert_eq!(r.status, 200, "{}", r.msg()); +} + +#[test] +fn a_merged_pull_keeps_its_merge_commit_after_the_base_is_rewritten() { + let w = world(); + let base = w.tip("main"); + w.commit("f", "f.txt", "f\n", Some("main")); + let r = w.api( + "POST", + "/pulls", + Some(json!({"title": "PR f", "head": "f", "base": "main"})), + ); + assert_eq!(r.status, 201, "{}", r.msg()); + let n = r.json()["number"].as_i64().unwrap(); + assert_eq!( + w.api( + "POST", + &format!("/pulls/{n}/merge"), + Some(json!({"Do": "merge"})) + ) + .status, + 200 + ); + let m = w.tip("main"); + assert_eq!( + w.git(&["rev-parse", &format!("refs/forge/pull/{n}/merge")]), + m + ); + // The base is rewritten away from the merge and the objects are old. + w.git(&["update-ref", "refs/heads/main", &base]); + let tree = w.git(&["rev-parse", &format!("{m}^{{tree}}")]); + for oid in [&m, &tree] { + age(&w.loose_path(oid), 30); + } + let junk = w.loose("junk"); + age(&w.loose_path(&junk), 30); + w.pass(); + assert!(w.exists(&m), "the merge commit is kept by its ref"); + assert!(w.exists(&tree)); + assert!(!w.exists(&junk), "the pass really pruned"); + let pr = w.api("GET", &format!("/pulls/{n}"), None).json(); + assert_eq!(pr["merge_commit_sha"], m.as_str()); + // A missing ref is recreated from the database before anything is pruned. + w.git(&["update-ref", "-d", &format!("refs/forge/pull/{n}/merge")]); + w.pass(); + assert!(w.exists(&m)); + assert_eq!( + w.git(&["rev-parse", &format!("refs/forge/pull/{n}/merge")]), + m + ); + w.fsck(); +} + +#[test] +fn a_retention_ref_that_cannot_be_repaired_stops_the_prune() { + let w = world(); + w.commit("f", "f.txt", "f\n", Some("main")); + let r = w.api( + "POST", + "/pulls", + Some(json!({"title": "PR f", "head": "f", "base": "main"})), + ); + let n = r.json()["number"].as_i64().unwrap(); + assert_eq!( + w.api( + "PATCH", + &format!("/pulls/{n}"), + Some(json!({"state": "closed"})) + ) + .status, + 201 + ); + // Lose the retention ref and make it unwritable with a lock that is not yet stale. + let head_ref = format!("refs/forge/pull/{n}/head"); + w.git(&["update-ref", "-d", &head_ref]); + let lock = w.repo().join(format!("{head_ref}.lock")); + std::fs::create_dir_all(lock.parent().unwrap()).unwrap(); + std::fs::write(&lock, "x").unwrap(); + let junk = w.loose("junk"); + age(&w.loose_path(&junk), 30); + let o = w.maintain(); + assert!(matches!(o, Outcome::Skipped(_)), "{o:?}"); + assert!( + w.exists(&junk), + "nothing is pruned while a root is unconfirmed" + ); + // Once the lock is old it is cleaned first, the ref is repaired and the prune runs. + age(&lock, 1); + let o = w.maintain(); + assert!(matches!(o, Outcome::Done { .. }), "{o:?}"); + assert!(!lock.exists()); + assert!(!w.exists(&junk)); + assert!(w.git_try_ref(&head_ref)); +} + +impl W { + fn git_try_ref(&self, r: &str) -> bool { + self.s + .git_try(self.id, &["rev-parse", "--verify", "--quiet", r]) + .is_some() + } +} + +#[test] +fn stage_two_excludes_the_api_writers_while_it_prunes() { + let wrap = Wrap::new("prune"); + let w = world_with(cfg(), Some(wrap.bin())); + w.commit("f", "f.txt", "f\n", Some("main")); + wrap.hold(); + let before = w.s.app.maint.passes(); + let r = w.s.send( + "POST", + "/api/v1/admin/cron/git_gc_repos", + Some(&w.auth), + None, + ); + assert_eq!(r.status, 204); + wrap.wait_started(); + let done = AtomicBool::new(false); + std::thread::scope(|sc| { + let writer = sc.spawn(|| { + let r = w.api( + "POST", + "/contents/g.txt", + Some(json!({"content": b64("g\n"), "message": "g", "branch": "f"})), + ); + done.store(true, Ordering::SeqCst); + r.status + }); + std::thread::sleep(Duration::from_millis(700)); + assert!( + !done.load(Ordering::SeqCst), + "a contents write must wait for the prune" + ); + wrap.release(); + assert_eq!(writer.join().unwrap(), 201); + }); + wait_for("the pass", || w.s.app.maint.passes() > before); + w.fsck(); +} + +#[test] +fn deleting_a_repository_cancels_a_gc_in_either_stage() { + for stage in ["gc", "prune"] { + let wrap = Wrap::new(stage); + let w = world_with(cfg(), Some(wrap.bin())); + wrap.hold(); + let outcome = std::thread::scope(|sc| { + let run = sc.spawn(|| w.maintain()); + wrap.wait_started(); + let started = std::time::Instant::now(); + let r = + w.s.send("DELETE", "/api/v1/repos/siteadmin/w", Some(&w.auth), None); + assert_eq!(r.status, 204, "{stage}: {}", r.s_text()); + assert!( + started.elapsed() < Duration::from_secs(9), + "{stage}: the delete must not wait out its limit" + ); + run.join().unwrap() + }); + assert_eq!(outcome, Outcome::Gone, "{stage}"); + assert!(!w.repo().exists()); + } +} + +#[test] +fn a_running_transport_defers_the_cleanup_and_age_keeps_young_leftovers() { + let w = world(); + let stale = w.repo().join("refs/heads/zz.lock"); + let young = w.repo().join("refs/heads/yy.lock"); + let quarantine = w.repo().join("objects/tmp_objdir-incoming-abc123"); + std::fs::write(&stale, "x").unwrap(); + std::fs::write(&young, "x").unwrap(); + std::fs::create_dir_all(quarantine.join("pack")).unwrap(); + std::fs::write(quarantine.join("pack/tmp"), "x").unwrap(); + age(&stale, 1); + age_dir(&quarantine, 1); + { + let _transport = + w.s.app + .life + .registry + .register(w.id, true, || true) + .expect("registers"); + assert!(matches!(w.maintain(), Outcome::Done { .. })); + assert!(stale.exists() && quarantine.exists(), "an owner may exist"); + } + assert!(matches!(w.maintain(), Outcome::Done { .. })); + assert!(!stale.exists(), "a stale lock goes once no owner can exist"); + assert!(!quarantine.exists(), "so does a stale quarantine directory"); + assert!(young.exists(), "a young lock is left alone"); +} + +#[test] +fn too_little_free_disk_skips_the_repack_and_so_the_prune() { + let wrap = Wrap::new("none"); + let w = world_with( + Cfg { + min_free: u64::MAX / 2, + ..cfg() + }, + Some(wrap.bin()), + ); + let junk = w.loose("junk"); + age(&w.loose_path(&junk), 30); + assert!(matches!(w.maintain(), Outcome::Skipped(_))); + let log = wrap.log(); + assert!(!log.split_whitespace().any(|a| a == "gc"), "gc ran: {log}"); + assert!( + !log.split_whitespace().any(|a| a == "prune"), + "pruned without a stage 1: {log}" + ); + assert!(w.exists(&junk)); +} + +#[test] +fn the_cron_route_is_for_site_admins_with_the_admin_scope() { + let w = world(); + let url = "/api/v1/admin/cron/git_gc_repos"; + // Anonymous callers get Gitea's answer for a token-only route. + assert!([401, 403].contains(&w.s.send("POST", url, None, None).status)); + let pw = w.s.mk_user("bob"); + let bob = token(&w.s.mk_token("bob", &pw, "t", &["write:repository"])); + assert_eq!(w.s.send("POST", url, Some(&bob), None).status, 403); + let narrow = scl_forge::admin::generate_access_token( + &w.s.dir, + "siteadmin", + "narrow", + &["read:repository".to_string()], + ) + .unwrap(); + assert_eq!( + w.s.send("POST", url, Some(&token(&narrow)), None).status, + 403, + "an admin token without the admin scope" + ); + assert_eq!( + w.s.send("POST", "/api/v1/admin/cron/nope", Some(&w.auth), None) + .status, + 404 + ); + assert_eq!(w.s.send("POST", url, Some(&w.auth), None).status, 204); + wait_for("the queue to drain", || !w.s.app.maint.busy()); +} + +#[test] +fn requests_during_a_running_pass_start_nothing_more() { + let wrap = Wrap::new("gc"); + let w = world_with(cfg(), Some(wrap.bin())); + let before = w.s.app.maint.passes(); + wrap.hold(); + let url = "/api/v1/admin/cron/git_gc_repos"; + assert_eq!(w.s.send("POST", url, Some(&w.auth), None).status, 204); + wrap.wait_started(); + for _ in 0..3 { + assert_eq!(w.s.send("POST", url, Some(&w.auth), None).status, 204); + } + assert!( + !w.s.app.maint.request(), + "a running pass covers the request" + ); + wrap.release(); + wait_for("the pass", || w.s.app.maint.passes() > before); + std::thread::sleep(Duration::from_millis(1500)); + assert_eq!(w.s.app.maint.passes(), before + 1, "exactly one pass ran"); +} + +#[test] +fn a_prune_window_shorter_than_the_safe_minimum_is_refused_at_startup() { + let dir = std::env::temp_dir().join(format!("scl-maint-flag-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let out = std::process::Command::new(env!("CARGO_BIN_EXE_sc-forge")) + .args(["serve", "--data-dir"]) + .arg(&dir) + .args([ + "--listen", + "127.0.0.1:0", + "--public-url", + "http://127.0.0.1:1", + "--gc-prune-expire", + "60", + ]) + .output() + .unwrap(); + let _ = std::fs::remove_dir_all(&dir); + assert!(!out.status.success()); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains("--gc-prune-expire must be at least"), "{err}"); +} + +#[test] +fn passes_beside_writers_lose_nothing() { + let w = world(); + w.commit("f", "f.txt", "f\n", Some("main")); + // Everything that exists is old, so the passes really prune and the writers reuse old objects. + let junk: Vec = (0..20).map(|i| w.loose(&format!("junk {i}"))).collect(); + let mut stack = vec![w.repo().join("objects")]; + while let Some(d) = stack.pop() { + for e in std::fs::read_dir(&d).unwrap().flatten() { + let p = e.path(); + if p.is_dir() { + stack.push(p); + } else if d.file_name().is_some_and(|n| n.len() == 2) { + age(&p, 30); + } + } + } + let stop = AtomicBool::new(false); + let acknowledged = std::sync::Mutex::new(Vec::::new()); + std::thread::scope(|sc| { + let passes = sc.spawn(|| { + while !stop.load(Ordering::SeqCst) { + w.pass(); + } + }); + let writers: Vec<_> = (0..3) + .map(|t| { + let (w, ack) = (&w, &acknowledged); + sc.spawn(move || { + for i in 0..8 { + let file = format!("t{t}-{i}.txt"); + let r = w.api( + "POST", + &format!("/contents/{file}"), + Some(json!({"content": b64(&format!("{t}{i}\n")), "message": file, "branch": "f"})), + ); + if r.status == 201 { + ack.lock().unwrap().push(file); + } else { + // Contention on one branch is a refusal, never a lost write. + assert!([409, 422, 503].contains(&r.status), "{} {}", r.status, r.msg()); + } + } + }) + }) + .collect(); + for h in writers { + h.join().unwrap(); + } + stop.store(true, Ordering::SeqCst); + passes.join().unwrap(); + }); + assert!(!acknowledged.lock().unwrap().is_empty()); + assert!( + junk.iter().all(|j| !w.exists(j)), + "the passes pruned the old junk" + ); + w.fsck(); + for file in acknowledged.lock().unwrap().iter() { + let r = w.api("GET", &format!("/contents/{file}?ref=f"), None); + assert_eq!(r.status, 200, "{file} was acknowledged and must read back"); + } +} + +#[test] +fn a_branch_or_default_branch_writer_defers_the_cleanup() { + let w = world(); + let stale = w.repo().join("refs/heads/zz.lock"); + std::fs::write(&stale, "x").unwrap(); + age(&stale, 1); + { + // The exclusive lifecycle lock is what default-branch changes and branch deletes hold. + let lock = w.s.app.life.lock_for(w.id); + let _writer = lock.write().unwrap(); + // Neither the cleanup nor the reconcile (a gated writer) runs beside it. + assert!(matches!(w.maintain(), Outcome::Skipped(_))); + assert!(stale.exists(), "a lifecycle writer may own that lock"); + } + assert!(matches!(w.maintain(), Outcome::Done { .. })); + assert!(!stale.exists()); +} + +#[test] +fn an_open_pull_with_no_branch_and_no_retention_ref_stops_the_prune() { + let w = world(); + w.commit("f", "f.txt", "f\n", Some("main")); + let r = w.api( + "POST", + "/pulls", + Some(json!({"title": "PR f", "head": "f", "base": "main"})), + ); + let n = r.json()["number"].as_i64().unwrap(); + // Lost branch and retention ref, and a fresh lock that keeps the repair from succeeding: the + // sync logs and skips this pull request, yet the prune must still refuse to run. + w.git(&["update-ref", "-d", "refs/heads/f"]); + w.git(&["update-ref", "-d", &format!("refs/forge/pull/{n}/head")]); + let lock = w.repo().join(format!("refs/forge/pull/{n}/head.lock")); + std::fs::create_dir_all(lock.parent().unwrap()).unwrap(); + std::fs::write(&lock, "x").unwrap(); + let junk = w.loose("junk"); + age(&w.loose_path(&junk), 30); + let o = w.maintain(); + assert!(matches!(o, Outcome::Skipped(_)), "{o:?}"); + assert!(w.exists(&junk)); +} + +#[test] +fn a_fetch_does_not_defer_the_cleanup_but_a_push_does() { + let w = world(); + let stale = w.repo().join("refs/heads/zz.lock"); + std::fs::write(&stale, "x").unwrap(); + age(&stale, 1); + let reg = &w.s.app.life.registry; + let push = reg.register(w.id, true, || true).expect("registers"); + assert!(matches!(w.maintain(), Outcome::Done { .. })); + assert!(stale.exists(), "a push may own the lock"); + drop(push); + let _fetch = reg.register(w.id, false, || true).expect("registers"); + assert!(matches!(w.maintain(), Outcome::Done { .. })); + assert!(!stale.exists(), "a read-only run owns no lock"); +} + +#[test] +fn the_quarantine_name_matches_what_git_really_creates() { + use scl_forge::orphans::QUARANTINE_PREFIX; + let w = world(); + let hook = w.repo().join("hooks"); + std::fs::create_dir_all(&hook).unwrap(); + let seen = w.s.dir.join("seen"); + // A pre-receive hook sees the quarantine: record what lives under objects/ at that moment. + let script = format!("#!/bin/sh\nls \"$GIT_DIR/objects\" > {}\n", seen.display()); + // The server's own hooks directory is configured globally; use a per-repository override. + let own = w.s.dir.join("own-hooks"); + std::fs::create_dir_all(&own).unwrap(); + std::fs::write(own.join("pre-receive"), script).unwrap(); + std::fs::set_permissions( + own.join("pre-receive"), + std::fs::Permissions::from_mode(0o755), + ) + .unwrap(); + let work = w.s.dir.join("work"); + let git = |dir: &Path, args: &[&str]| { + let o = std::process::Command::new("git") + .current_dir(dir) + .args(args) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") + .output() + .unwrap(); + assert!( + o.status.success(), + "{args:?}: {}", + String::from_utf8_lossy(&o.stderr) + ); + }; + std::fs::create_dir_all(&work).unwrap(); + git(&work, &["init", "-q", "-b", "main"]); + std::fs::write(work.join("a"), "a").unwrap(); + git(&work, &["add", "a"]); + git( + &work, + &[ + "-c", + "user.name=t", + "-c", + "user.email=t@x", + "commit", + "-qm", + "m", + ], + ); + let bare = w.s.dir.join("q.git"); + git(&w.s.dir, &["init", "-q", "--bare", bare.to_str().unwrap()]); + git(&bare, &["config", "core.hooksPath", own.to_str().unwrap()]); + git(&work, &["push", "-q", bare.to_str().unwrap(), "main"]); + let listing = std::fs::read_to_string(&seen).unwrap(); + assert!( + listing.lines().any(|l| l.starts_with(QUARANTINE_PREFIX)), + "git names its quarantine differently now: {listing}" + ); +} + +#[test] +fn an_absurd_prune_window_prunes_nothing() { + let w = world_with( + Cfg { + prune_expire: Duration::from_secs(u64::MAX), + ..cfg() + }, + None, + ); + let junk = w.loose("junk"); + age(&w.loose_path(&junk), 30); + assert!(matches!(w.maintain(), Outcome::Skipped(_))); + assert!(w.exists(&junk)); +} + +#[test] +fn duration_flags_beyond_ten_years_are_refused() { + for (flag, value) in [ + ("--gc-prune-expire", "9999999999"), + ("--maintenance-interval", "18446744073709551615"), + ("--gc-timeout", "9999999999"), + ] { + let dir = std::env::temp_dir().join(format!("scl-maint-bound-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let out = std::process::Command::new(env!("CARGO_BIN_EXE_sc-forge")) + .args(["serve", "--data-dir"]) + .arg(&dir) + .args([ + "--listen", + "127.0.0.1:0", + "--public-url", + "http://127.0.0.1:1", + ]) + .args([flag, value]) + .output() + .unwrap(); + let _ = std::fs::remove_dir_all(&dir); + assert!(!out.status.success(), "{flag}"); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains("at most"), "{flag}: {err}"); + } +} + +#[test] +fn a_base_branch_deleted_under_an_open_pull_keeps_its_last_commit() { + let w = world(); + let base_tip = w.commit("b", "b.txt", "b\n", Some("main")); + w.commit("f", "f.txt", "f\n", Some("main")); + let r = w.api( + "POST", + "/pulls", + Some(json!({"title": "PR f", "head": "f", "base": "b"})), + ); + assert_eq!(r.status, 201, "{}", r.msg()); + let n = r.json()["number"].as_i64().unwrap(); + w.git(&["update-ref", "-d", "refs/heads/b"]); + let tree = w.git(&["rev-parse", &format!("{base_tip}^{{tree}}")]); + for oid in [&base_tip, &tree] { + age(&w.loose_path(oid), 30); + } + w.pass(); + assert!(w.exists(&base_tip), "the stored base must survive"); + assert_eq!( + w.git(&["rev-parse", &format!("refs/forge/pull/{n}/base")]), + base_tip + ); + let r = w.api("GET", &format!("/pulls/{n}"), None); + assert_eq!(r.status, 200, "{}", r.msg()); +} diff --git a/docs/THREAT-MODEL.md b/docs/THREAT-MODEL.md index 24723577..1912fb40 100644 --- a/docs/THREAT-MODEL.md +++ b/docs/THREAT-MODEL.md @@ -1093,10 +1093,57 @@ SIGKILL during merges, restarted and checked), and the `bypass_matrix_*` tests i CI); a later change to it needs a new migration. - **Known windows.** A rule created while a push is between `pre-receive` and its ref update is not stopped (the push does not take the gate). A merge computes its tree before it takes the base lock in git, so a base that moves under it only costs a retry, - not a wrong merge (the compare-and-swap refuses). Objects of aborted or lost merges stay unreferenced until F5's gc. + not a wrong merge (the compare-and-swap refuses). Objects of aborted or lost merges stay unreferenced until the maintenance + pass collects them (F5a, below). - **Identity.** The merge commit is authored and committed by the authenticated user (name and stored email, or `@noreply.localhost`), as for contents writes; a client cannot choose it. +### Maintenance: garbage collection and leftover cleanup (F5a) + +`maintenance.rs` deletes data, so it is the part of the forge where a mistake loses an acknowledged write. What it deletes, and +why each deletion is safe: + +- **Unreferenced objects, in two stages.** Stage 1 (`git gc --prune=never --force`, no gate, `gc.cruftPacks=false` pinned) packs + what refs reach and loosens unreachable packed objects with their old pack's mtime; it deletes nothing. Stage 2 runs under the + repository's write gate: it first confirms every retention ref (`pulls::reconcile_repo` recreates a missing head, final-head or + merge-commit ref from the database, and `pulls::verify_roots` then checks each pull request row independently, because a sync + logs and skips a PR it cannot repair; any error, or a merge intent still pending, skips the prune; the reconcile runs under the + shared lifecycle lock like every gated writer, taken with `try_read` so a queued delete cannot deadlock it), then `git prune --expire=` removes loose unreachable + objects older than the window (`--gc-prune-expire`, default 14 days, refused below twice `--git-timeout` and an hour). Holding the + gate keeps the API writers away: a contents write or merge copies existing, possibly old, trees and blobs into new objects + (`mktree` does not touch the referenced objects), and without the gate prune could unlink one between that read and the + ref update. A first draft held no gate; Sol found this interleaving and the draft was dropped. +- **Merge commits.** A finished merge used to keep only its head alive; rewriting the base afterwards orphaned the merge commit that + `merge_commit_sha` still names. `refs/forge/pull//merge` now holds it (written by `finish`, recreated by the reconciler). +- **Leftover locks and quarantine directories.** `refs/**/*.lock`, `packed-refs.lock`, `HEAD.lock`, the commit-graph locks a killed gc leaves and receive-pack's `objects/tmp_objdir-incoming-*` quarantine directories go at + startup unconditionally (after orphaned git processes are killed) and at run time only when the write gate is held, the + exclusive lifecycle lock is free (default-branch changes and branch deletes write refs under that lock alone), the + repository has no registered run that can write (`Registry::with_only` ignores read-only fetches and holds the registry lock so + no writer can register meanwhile; under that lock a quarantine is only renamed to `objects/dead-*` and deleted afterwards) + and the item is older than 10 minutes. Age alone is never enough: a stepped clock or a suspended host can make a live owner's file + look old. +- **Delete.** A run registers like a transport and carries the registry's cancel flag into every git spawn, so a repository delete + cancels it (group kill) and waits for it before the rename. The run never waits for the lifecycle lock (`try_read`/`try_write` + only); the gate wait is sliced so a cancel is noticed. The reconcile between the two does not carry the cancel flag; it is a handful of short git commands, and a + delete that outwaits it answers 503 and reopens the repository rather than renaming under it. +- **Cruft packs are off on purpose.** Stage 1 pins `gc.cruftPacks=false`: with git's default, `gc --prune=never` writes unreachable + objects into a cruft pack and `git prune` never touches packed objects, so nothing would be collected (a test packs an old + object and fails if the pin goes). The cost: unreachable objects are stored loose, without deltas, for up to the window. Not + measured on a realistic repository. +- **Prune holds the gate** for at most 10 minutes (its own limit). It runs only after a complete stage 1 (git's own gc does not + prune after a failed repack either): without it reachable objects may still be loose and old, and the walk is slower. A repository + that is too full to repack therefore keeps its garbage until space is freed. +- **Base commits.** An unmerged PR whose base branch is gone shows its stored `base_sha`; `refs/forge/pull//base` keeps that + commit (recreated by the reconciler while it exists). Duration flags above ten years are refused at startup (a cutoff that wrapped + would have meant "prune now"), and the pass also refuses a cutoff before the epoch. +- **Remaining exposure.** A push built on an object older than the window while a concurrent force-push has just unreferenced it + and stage 2 runs, is not excluded (receive-pack does not take the gate); its objects would have to be older than 14 days and + unreachable in that instant. A pending merge intent whose merge commit stays unreferenced for the whole window reads as "not + landed" and aborts (a landed merge is reachable from the base or its marker). `git gc` is bounded by `--gc-timeout` and the + cgroup, not by `RLIMIT`s; a repack needs roughly the repository's size in free disk (the pass skips the repack below + `--min-free-bytes` plus that size). Writers to a repository wait on the gate (10 s) while stage 2 runs and see 503 if it + outlasts that; stage 2 is a reachability walk and a scan of loose objects, not a repack. + ## Reporting Security reports go to **toni@git-agentic.com** or GitHub private vulnerability diff --git a/docs/superpowers/plans/2026-10-24-p38-f5a-maintenance.md b/docs/superpowers/plans/2026-10-24-p38-f5a-maintenance.md new file mode 100644 index 00000000..66a8f011 --- /dev/null +++ b/docs/superpowers/plans/2026-10-24-p38-f5a-maintenance.md @@ -0,0 +1,156 @@ +# P38 F5a: maintenance (gc, runtime cleanup) + +F5 (ops) is split in two PRs. F5a is the part that deletes data: garbage collection and the cleanup of +what killed git processes leave behind. F5b (logs, drain, bind-gate tests, container image, ops doc) follows. + +Not in F5: per-owner byte quotas and per-user transport limits. Engine's clones come from one service +account, so a per-user limit needs a deliberate budget; both stay in #297. + +## Problem + +Unreferenced objects accumulate: refused or lost contents writes, aborted merge commits, force-pushed +history. `gc.auto=0` is pinned, so nothing ever collects them. A git writer killed at run time (timeout, +cancel) can leave `refs/**/*.lock` or `objects/incoming-*` behind until the next restart's sweep. + +## Design + +### 1. Where gc runs + +Inside the server (the data-directory lock makes an offline CLI impossible). Two triggers, one code path +(`maintenance::run_all`): + +- a timer thread: `--maintenance-interval` seconds (default 86400, 0 = off), first run one interval after start; +- `POST /api/v1/admin/cron/{task}` (Gitea route, site admin, `admin` write scope). Task `git_gc_repos` answers + 204 and runs the pass on a background thread; an unknown task is 404; a pass already running is not started + twice (still 204, as Gitea). Other Gitea task names are 404 (not recorded in the fixtures, Engine does not call them). + +One pass at a time, repositories sequentially, `live` rows only. + +### 2. What one repository gets (two stages) + +Sol's review of the first draft (no gate, `git gc --prune`) found real losses, so collection is split into a +non-destructive stage and a short destructive one under the write gate. + +**Stage 1, no gate, deletes nothing:** `git gc --prune=never --force` with +`-c gc.autoDetach=false -c pack.threads=2 -c pack.windowMemory=256m`. With `never`, git packs the reachable +set, loosens unreachable packed objects (keeping the old pack's mtime) and prunes nothing. It runs through the +`Git` runner (admission slot, process group, output cap) with `--gc-timeout` (default 3600 s). The runner sets +no `RLIMIT`s (only transports do); the bound is the timeout, `pack.*` settings and the container's cgroup limits. + +**Stage 2, under the repository's write gate (`Lifecycle::with_gate`, gate only, no lifecycle lock):** + +1. `pulls::reconcile_repo` runs first and must succeed: it recreates every missing retention ref (open head, + closed or merged final head, and the new merge-commit ref, below). On any error the repository is skipped + and logged; nothing is deleted. +2. `git prune --expire=<--gc-prune-expire>` (default 14 days, rejected below `max(2 * --git-timeout, 1 h)`). + After stage 1 every unreachable object is loose, so `prune` is complete. Holding the gate excludes the API + writers (contents write, merge, PR sync), the producers that copy existing, possibly old, objects into new + trees (`gitwrite.rs` `mktree`) and so could otherwise reference an object that prune is about to unlink. + A push cannot do that: receive-pack's objects arrive fresh, and its connectivity check needs objects + reachable from existing refs. Left over, and documented in the threat model: a push built on a commit that a + concurrent force-push has just unreferenced, whose objects are all older than the window, while stage 2 runs. +3. Runtime cleanup (below), then the gate is released. + +A writer that arrives during stage 2 waits on the gate (its normal 10 s) and answers 503 only if prune outlasts it. +Stage 2 is a reachability walk (commit-graph backed) plus a scan of the loose objects, so it is far shorter than stage 1. +Stage 1 is concurrent-safe in git itself (`pack-refs`, `repack -d` take their own locks, readers re-scan packs). + +- **`--force`** because a `gc.pid` left by a killed gc must not block the repository forever; only this module + runs gc, one run per repository at a time (a per-repository `AtomicBool`). +- **Delete safety.** `Spec` gains `cancel: Option<&Cancel>`, polled by the runner's wait loop (new + `GitError::Cancelled`, group killed and reaped as for a timeout). Both stages run under one + `Registry::register` registration (liveness checked by a direct database read, as `cgi.rs` does, never by a + lifecycle lock inside the registry callback), held through every filesystem operation including cleanup, so + `delete` (which takes the exclusive lifecycle lock, then closes the slot and waits) cancels the run and waits + for it before the rename. The run never takes the lifecycle lock, and releases the registration before any + follow-up that does. A cancelled run starts no further command or cleanup. Shutdown cancels it. +- **Free space.** Skip stage 1 (logged) when `free < min_free + repository size` (`repack` writes the new pack + before it removes the old ones); size is `size-pack + size` from `git count-objects -v` (KiB). + +### 3. Completed merges keep their merge commit + +Sol: after a merge finishes, the landing marker is deleted and only the head is retained, so a later rewrite of +the base orphans the merge commit that `merge_commit_sha` still reports, and gc would delete it. `finish` +therefore writes `refs/forge/pull//merge` (the merge commit) before retiring the marker, and +`reconcile_repo` recreates it for every merged PR from `merge_commit_sha` and removes `/merge` refs of unknown PRs. +Nothing is deployed from before F4c, so there is no legacy data to backfill. + +### 4. Runtime cleanup of stale locks and quarantine directories + +Age alone does not prove an owner has ended (wall-clock steps, host suspend), so deletion requires +ownership to be excluded and age is only a second condition: + +- the repository's write gate is held (no API writer) and the registry slot has no live registration + (`Registry::with_idle`, which runs the cleanup under the registry mutex so no transport can register meanwhile; + it is a few `readdir`/`unlink` calls); +- and the item's mtime is older than 600 s. + +Items: `objects/incoming-*` directories; `refs/**/*.lock`, `packed-refs.lock`, `HEAD.lock`. Cleanup runs even +if gc failed (the failure may be caused by such a lock). Startup keeps removing all of them unconditionally +(`orphans::sweep` after it kills orphans) and gains `objects/incoming-*`. + +### 4a. Additions after the diff review + +- Cleanup also needs the exclusive lifecycle lock (`Lifecycle::try_exclusive`, never waits, taken after the gate): default-branch + changes and branch deletes write refs under that lock without the gate. +- `pulls::verify_roots` checks every pull request row against the refs after the reconcile (a sync logs and skips a PR it cannot + repair, so reconcile success alone proved nothing). +- The cancel flag is checked after the gate is taken; the reconcile does not carry it (short commands, a slow delete answers 503). + +### 4b. Additions after the Opus review + +- git names the quarantine `objects/tmp_objdir-incoming-XXXXXX` (since 2.35), not `incoming-*`; the first cut matched nothing. A test + pins the prefix against a real push. +- A killed gc leaves `objects/info/commit-graph.lock` (and the chain lock), which fails every later gc: `clean_repo` removes them. +- Stage 1 depends on `gc.cruftPacks=false` (see the threat model). Cleanup runs before stage 1 (a stale `packed-refs.lock` makes gc + print an error and still exit 0). +- Registrations carry a `writes` flag; fetches do not defer the cleanup. The reconcile holds the shared lifecycle lock (`try_read`). + A pending merge intent skips the prune. The prune has its own 10-minute limit. `core.packedRefsTimeout` is 5 s so a ref delete + survives a concurrent `pack-refs`. The reconcile finishes its other repairs when the merge-ref repair fails. +- Stage 2 is not purely file-system work: the reconcile can finalize or abort merge intents and close pull requests. + +### 4c. Additions after the second Opus review + +- `refs/forge/pull//base` keeps the stored base commit of unmerged PRs (a commit that is already gone is skipped, not an error). +- The prune is skipped when stage 1 did not complete; duration flags are capped at ten years; the cutoff is checked in the pass too. +- A quarantine is renamed under the registry lock and deleted after it; a gate timeout in the cleanup step no longer skips the repository. +- The plan's `run_all`, per-repository `AtomicBool` and `with_idle` were not built: one runner thread serializes passes, and + `with_only` replaced `with_idle`. + +### 5. Failure and recovery + +gc killed at any point: stage 1 leaves the old packs until the new one is complete; git's temp files +(`objects/pack/tmp_*`, `gc.pid`) are removed by the next gc (`--force`, `prune`). Stage 2 deletes only +unreachable, aged loose objects. No database state changes, so nothing needs reconciling. A pass interrupted by +shutdown just ends. + +### 6. Tests + +Deterministic interleavings come from `SC_FORGE_GIT` pointing at a wrapper script that sleeps on chosen +subcommands (`gc`, `prune`), so a test can hold stage 1 or stage 2 open. + +- old unreferenced loose object and old unreferenced packed object: both gone after a pass; young ones stay; +- reachable data stays: branch tips, a merged PR's frozen head, an open PR's retention ref after its head branch + was deleted, a closed PR's `final_head_sha`, merge markers; `git fsck --strict` clean and a clone of every + branch succeeds (descendant bytes present, not just refs); +- merge commit survives: merge, force-push the base away from it, backdate the objects, run a pass, the merge + commit still exists and `GET pulls/{n}` still reports it; +- repair failure skips pruning: delete a retention ref and lock it (`.lock`), backdate, run a pass; nothing is deleted; +- stage 2 excludes API writers: with `prune` held open by the wrapper a contents write waits on the gate and + completes after; a write holding the gate delays stage 2 (it never prunes under a running write); +- a transport in flight on the repository defers cleanup (the registry is not idle), a stale lock then goes on + the next pass; with the age under 600 s a lock stays; +- delete during stage 1 and during stage 2 cancels the run and the delete completes; no run touches the directory afterwards; +- the aborted-merge commit (old) is collected after its journal row is `aborted`; +- startup removes `incoming-*` and locks regardless of age; +- cron route: non-admin 403, token without `admin` scope 403, anonymous 401, unknown task 404, admin 204; a + second call during a pass starts nothing; +- free-space skip with a huge `--min-free-bytes`; a prune window below the minimum is refused at startup; +- runner: a cancelled `Spec` kills its group (with a grandchild) and returns `Cancelled`. + +### 7. Out of scope, recorded + +- `refs/forge/*` growth: `gc` packs them. +- An idle repository is gc'd every interval (a clean one is cheap); no `last_gc` state in the database. +- Repository disk usage is not reported by an API; the logs of F5b carry the size before and after each pass. +- Pushes that build on an object older than the window while a force-push unreferences it (see 2). diff --git a/docs/superpowers/specs/2026-10-22-p38-forge-design.md b/docs/superpowers/specs/2026-10-22-p38-forge-design.md index 43b1dc43..30697ddb 100644 --- a/docs/superpowers/specs/2026-10-22-p38-forge-design.md +++ b/docs/superpowers/specs/2026-10-22-p38-forge-design.md @@ -905,6 +905,27 @@ F3b is the write half of the content API (plan: `docs/superpowers/plans/2026-10- deterministic interfering writer); the F4 `refs/forge/pull//head` refresh for a contents write to a PR branch; portable-name collision rules; a disk reservation sized to the exact rebuilt trees. +### F5a decisions (as built) + +F5a is the maintenance half of F5 (plan: `docs/superpowers/plans/2026-10-24-p38-f5a-maintenance.md`); logs, drain, the bind-gate +tests, the image and the operations guide are F5b. + +- **Two-stage gc** (`maintenance.rs`): `git gc --prune=never` without the gate, then under the write gate the retention refs are + reconciled and `git prune --expire=<--gc-prune-expire>` runs. The spec said "under the repo lock" for the whole gc; holding the + gate through a repack would answer 503 to every merge and contents write for minutes, and the unguarded `--prune` draft + could drop an old object a writer was about to reference, so the prune alone holds the gate. One pass at a time, repositories in + turn; a failure is logged and the pass continues. +- **Triggers.** `--maintenance-interval` (default 86400 s, 0 = off) and `POST /api/v1/admin/cron/git_gc_repos` (Gitea's route; + 204 at once, site admin, `admin` write scope, 404 for any other task). Both wake one runner thread; a request during a queued or + running pass starts nothing more. +- **Retention.** Merged pull requests keep `refs/forge/pull//merge`; the reconciler recreates it from `merge_commit_sha`. +- **Cleanup.** Stale ref locks and `objects/incoming-*` are removed at startup unconditionally and at run time only with the gate + held, no other registered run on the repository and an age over 10 minutes. This closes the "lock left by a git writer killed at + run time" gap. +- **Not built.** Per-owner byte quotas and per-user transport limits (they need a budget decision first; Engine's clones come from + one service account; #297). No last-gc state: an idle repository is collected every interval. `Git::run` gained `Spec::cancel` + but still sets no `RLIMIT`s (only transports do). + ### F4c decisions (as built) F4c adds migration 5 (`merge_intents`) and `POST /pulls/{n}/merge`; the gate now replays G, T, U, O and Q completely (the F4