One auto-land and one Diffusion bump at a time across hub processes (closes #275) 356183b71fab
leader::exclusive holds a lock named for a proposal or a consumer's pin across every process sharing the database: a transaction-scoped PostgreSQL advisory lock, or on SQLite a lock in the one process. Auto-land holds autoland:ID for each evaluation (beside its TURN), Diffusion diffusion:CONSUMER:PATH for each bump and landing, so two web processes cannot land one proposal twice.
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
agent:claude@andymccall <[email protected]> · keyless, under your grant for changes,refs,seal · key ae4947279dff
- When
- · 2026-10-21 01:12 UTC
- Signature
- ◈ sealed by agent:claude@andymccall
- Hash
356183b71fab8ff2e99271c2f7ed872cfc084c89148f242ee00a92320139dddf- Depends on
- 6b195ed3b637 86a69b53aff0 a7e6e8e6094c b5668108487f e57765dee61a ec84907043c2 f4d6ef418a96 f677d6270298
- Depended on by
- dd327d76062d
- In bookmarks
- api-unknown-fields diffusion-pin-bookmark draft-ready main proposal-list-fast
- In tags
- v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 10 atoms, 0 tree operations, 6 files
Browse the tree at this change
modified CHANGELOG.md +1 −0
| … | 3 unchanged lines |
||
| 4 | 4 | ||
| 5 | 5 | ## Unreleased |
|
| 6 | 6 | ||
| 7 | + | - **One auto-land and one Diffusion bump at a time across hub processes (#275):** auto-land's check of a proposal, and Diffusion's bump and landing of a consumer's pin, were serialised only within a process, so two web processes could land one proposal twice. Each now holds a lock named for the proposal or the pin across every process sharing the database (a PostgreSQL advisory lock; on SQLite, one process, as before). |
|
| 7 | 8 | - **Several web processes can share a data directory (#274):** the transparency log took appends from one process only, so only one process per data directory could have the web role. Each append now holds a lock in the log's directory and starts from the log as the files hold it, readers see every process's entries, and every process serves the newest checkpoint, whichever signed it. |
|
| 8 | 9 | - **A push reuses a job's pass on the same state (#265):** when main reaches a state a proposal's run already tested (landing it), a job that passed there with the same spec and sandbox is not run again: it shows passed, reused from that run, with its receipt, and its check says so. Jobs with an approval gate, an environment, secrets or identity tokens, or whose artifacts another job takes, always run. Maintainers can turn reuse off under Settings › Runs. |
|
| 9 | 10 | - **Drops are backed up (#280):** an owner's backups now include its drops, each copied like a repository and restored as a drop (secret or public, with its title and description); Admin › Backups counts them with the repositories they cover. |
|
| … | 275 unchanged lines |
modified crates/hub/src/autoland.rs +3 −0
| … | 69 unchanged lines |
||
| 70 | 70 | /// wait. |
|
| 71 | 71 | async fn evaluate(app: &Shared, row: &db::repos::RepoRow, p: pdb::ProposalRow) { |
|
| 72 | 72 | let _turn = TURN.lock().await; |
|
| 73 | + | // And one at a time for this proposal across the hub's processes |
|
| 74 | + | // (#275): two web nodes recording its last checks land it once. |
|
| 75 | + | let Ok(_proposal) = crate::leader::exclusive(&app.db, &format!("autoland:{}", p.id)).await else { return }; |
|
| 73 | 76 | // Landed or closed meanwhile, or turned off. |
|
| 74 | 77 | let Ok(Some(p)) = pdb::get(&app.db, row.id, p.number).await else { return }; |
|
| 75 | 78 | if !p.is_open() || p.auto_land_by.is_empty() { |
|
| … | 37 unchanged lines |
modified crates/hub/src/diffusion.rs +4 −0
| … | 1090 unchanged lines |
||
| 1091 | 1091 | if !go { |
|
| 1092 | 1092 | continue; |
|
| 1093 | 1093 | } |
|
| 1094 | + | // One bump or landing of this pin at a time across processes (#275). |
|
| 1095 | + | let Ok(_pin) = crate::leader::exclusive(&app.db, &format!("diffusion:{}:{path}", consumer.id)).await else { continue }; |
|
| 1094 | 1096 | match bump(app, &consumer, &target, &path, upstream, heads.clone()).await { |
|
| 1095 | 1097 | Ok(Bumped::Moved { from, bookmark, heads: bumped, state, .. }) => { |
|
| 1096 | 1098 | // Its pipeline runs as for a push, so the proposal is tested. |
|
| … | 161 unchanged lines |
||
| 1258 | 1260 | /// moves the target as a push would, which may bump and land again. |
|
| 1259 | 1261 | pub fn try_land(app: Shared, consumer: RepoRow, path: String) -> std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>> { |
|
| 1260 | 1262 | Box::pin(async move { |
|
| 1263 | + | // One landing of this pin at a time across processes (#275). |
|
| 1264 | + | let Ok(_pin) = crate::leader::exclusive(&app.db, &format!("diffusion:{}:{path}", consumer.id)).await else { return }; |
|
| 1261 | 1265 | match auto_land(&app, &consumer, &path).await { |
|
| 1262 | 1266 | Ok(_) | Err(Held::NoBump | Held::Manual) => {} |
|
| 1263 | 1267 | Err(why) => tracing::info!("diffusion: {}/{} {path} waits for a person: {why:?}", consumer.owner, consumer.name), |
|
| … | 298 unchanged lines |
modified crates/hub/src/leader.rs +36 −0
| … | 21 unchanged lines |
||
| 22 | 22 | use std::sync::Mutex; |
|
| 23 | 23 | use std::time::Duration; |
|
| 24 | 24 | ||
| 25 | + | /// A lock across every process sharing the hub's database, held while the |
|
| 26 | + | /// guard lives (#275): one evaluation of a proposal's auto-land, one bump |
|
| 27 | + | /// or landing of a consumer's pin, at a time anywhere. On PostgreSQL a |
|
| 28 | + | /// transaction-scoped advisory lock (`pg_advisory_xact_lock`), freed when |
|
| 29 | + | /// the guard drops and its transaction ends, or with its connection; on |
|
| 30 | + | /// SQLite (one process by definition) a lock in this process. |
|
| 31 | + | pub struct Exclusive { |
|
| 32 | + | #[cfg(feature = "postgres")] |
|
| 33 | + | _tx: sqlx::Transaction<'static, sqlx::Postgres>, |
|
| 34 | + | #[cfg(not(feature = "postgres"))] |
|
| 35 | + | _guard: tokio::sync::OwnedMutexGuard<()>, |
|
| 36 | + | } |
|
| 37 | + | ||
| 38 | + | /// Wait for the lock named `name`, then hold it until the guard drops. |
|
| 39 | + | #[cfg(feature = "postgres")] |
|
| 40 | + | pub async fn exclusive(pool: &crate::db::Pool, name: &str) -> sqlx::Result<Exclusive> { |
|
| 41 | + | let mut tx = pool.begin().await?; |
|
| 42 | + | let schema: String = sqlx::query_scalar("SELECT CAST(current_schema() AS TEXT)").fetch_one(&mut *tx).await?; |
|
| 43 | + | sqlx::query("SELECT pg_advisory_xact_lock($1)").bind(pg::key(&schema, name)).execute(&mut *tx).await?; |
|
| 44 | + | Ok(Exclusive { _tx: tx }) |
|
| 45 | + | } |
|
| 46 | + | ||
| 47 | + | /// Wait for the lock named `name`, then hold it until the guard drops. |
|
| 48 | + | #[cfg(not(feature = "postgres"))] |
|
| 49 | + | pub async fn exclusive(_pool: &crate::db::Pool, name: &str) -> sqlx::Result<Exclusive> { |
|
| 50 | + | static LOCAL: Mutex<Option<std::collections::HashMap<String, std::sync::Arc<tokio::sync::Mutex<()>>>>> = Mutex::new(None); |
|
| 51 | + | let lock = { |
|
| 52 | + | let mut map = LOCAL.lock().unwrap_or_else(|p| p.into_inner()); |
|
| 53 | + | let map = map.get_or_insert_with(Default::default); |
|
| 54 | + | // Locks nobody holds or waits for are forgotten. |
|
| 55 | + | map.retain(|_, l| std::sync::Arc::strong_count(l) > 1); |
|
| 56 | + | map.entry(name.to_string()).or_default().clone() |
|
| 57 | + | }; |
|
| 58 | + | Ok(Exclusive { _guard: lock.lock_owned().await }) |
|
| 59 | + | } |
|
| 60 | + | ||
| 25 | 61 | /// How often a process that does not hold a lock asks for it again. |
|
| 26 | 62 | pub const RETRY: Duration = Duration::from_secs(2); |
|
| 27 | 63 | ||
| … | 311 unchanged lines |
modified crates/hub/tests/roles.rs +24 −0
| … | 113 unchanged lines |
||
| 114 | 114 | assert!(app.tlog.append(&[]).is_err()); |
|
| 115 | 115 | } |
|
| 116 | 116 | ||
| 117 | + | /// oxygenhub#275: a named lock is held by one holder at a time across the |
|
| 118 | + | /// database's sessions (on SQLite, the process); other names are free. |
|
| 117 | 119 | #[tokio::test(flavor = "multi_thread")] |
|
| 120 | + | async fn an_exclusive_lock_has_one_holder_at_a_time() { |
|
| 121 | + | hermetic(); |
|
| 122 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 123 | + | let app = oxygenhub::build(config(dir.path(), &[])).await.unwrap(); |
|
| 124 | + | let held = oxygenhub::leader::exclusive(&app.db, "autoland:7").await.unwrap(); |
|
| 125 | + | let (tx, mut rx) = tokio::sync::oneshot::channel(); |
|
| 126 | + | let db = app.db.clone(); |
|
| 127 | + | tokio::spawn(async move { |
|
| 128 | + | let g = oxygenhub::leader::exclusive(&db, "autoland:7").await.unwrap(); |
|
| 129 | + | let _ = tx.send(()); |
|
| 130 | + | drop(g); |
|
| 131 | + | }); |
|
| 132 | + | tokio::time::sleep(Duration::from_millis(300)).await; |
|
| 133 | + | assert!(rx.try_recv().is_err(), "the second holder waits"); |
|
| 134 | + | let other = tokio::time::timeout(Duration::from_secs(2), oxygenhub::leader::exclusive(&app.db, "autoland:8")).await; |
|
| 135 | + | assert!(other.is_ok(), "another name is free"); |
|
| 136 | + | drop(other); |
|
| 137 | + | drop(held); |
|
| 138 | + | assert!(tokio::time::timeout(Duration::from_secs(5), rx).await.is_ok(), "and takes it once it is let go"); |
|
| 139 | + | } |
|
| 140 | + | ||
| 141 | + | #[tokio::test(flavor = "multi_thread")] |
|
| 118 | 142 | async fn a_hub_takes_every_role_by_default() { |
|
| 119 | 143 | hermetic(); |
|
| 120 | 144 | let dir = tempfile::tempdir().unwrap(); |
|
| … | 62 unchanged lines |
modified docs/roles.md +4 −2
| … | 62 unchanged lines |
||
| 63 | 63 | ||
| 64 | 64 | - `autoland::on_check_task` after a check is recorded: serialised today by |
|
| 65 | 65 | a process-wide `TURN` mutex, so two nodes could land one proposal twice. |
|
| 66 | - | Needs a cluster-wide lock (an advisory lock per proposal). |
|
| 66 | + | Built (#275): `leader::exclusive("autoland:ID")`, a transaction-scoped |
|
| 67 | + | advisory lock per proposal, held for each evaluation, beside the TURN. |
|
| 67 | 68 | - `diffusion::propagate` / `try_land` / `catch_up` / `land_ready` after a |
|
| 68 | - | push: idempotent by design, but racing; the same per-proposal lock. |
|
| 69 | + | push: idempotent by design, but racing; built (#275): each bump and |
|
| 70 | + | landing of a consumer's pin holds `diffusion:CONSUMER:PATH`. |
|
| 69 | 71 | - The transparency log's appends in `after_push` ("logged before the push |
|
| 70 | 72 | is answered"): **one writer only**. The log's files live in the data |
|
| 71 | 73 | directory; two processes appending would corrupt them. Needs either an |
|
| … | 174 unchanged lines |