Runner: lock fetches per repository, not the whole runner ed4aef8b70ca
Workspaces were prepared under one runner-wide lock held from the first fetch to the workspace image, so a slow repository held every job waiting: on 2026-10-16 the hub's fmt job waited 819 s behind metalgear2 jobs fetching from a bucket, and one job's token expired before its fetch began (job 5121). Fetches into a cached repository, and exports from it, now take that repository's own lock; taking cache disks and imaging keep the runner-wide one; a first clone, which passes its token through the process environment, takes a clone lock. The job's log says how long it waited for another job and how long fetching and exporting took. Closes #63
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
Claude Code <[email protected]> · delegate for changes, until 2026-11-13 19:00Z · key ea5f78160088
- When
- · 2026-10-16 19:20 UTC
- Signature
- ✔ signed by Claude Code
- Hash
ed4aef8b70ca73c382d749781ca0fb768997d4da20802ac4ceba96b144dc3baa- Depends on
- 47c1d64dec8a 82cf6e91d398 bb643c252515 c23d519168d8 e0918b887850
- Depended on by
- 1a50d4dae89c 1215f05ff8d0 cedd12342afa 2594278f49b2
- In bookmarks
- main
- In tags
- v0.2.0 v0.3.0 v0.3.1 v0.4.0 v0.5.0 v0.5.1 v0.5.2 v0.5.3
- Edits
- 11 atoms, 0 tree operations, 1 file
Browse the tree at this change
modified crates/flow/src/runner.rs +54 −7
| … | 757 unchanged lines |
||
| 758 | 758 | } |
|
| 759 | 759 | } |
|
| 760 | 760 | ||
| 761 | - | /// Held while a job's workspace is prepared (see `run_claim`). |
|
| 761 | + | /// Held while a job takes its cache disks and images its workspace (see |
|
| 762 | + | /// `run_claim`): one job at a time on the runner's disk and cache pool. |
|
| 762 | 763 | static PREPARE: std::sync::Mutex<()> = std::sync::Mutex::new(()); |
|
| 763 | 764 | ||
| 765 | + | /// Held while the first clone of any repository passes its token through |
|
| 766 | + | /// the process environment. |
|
| 767 | + | static CLONE: std::sync::Mutex<()> = std::sync::Mutex::new(()); |
|
| 768 | + | ||
| 769 | + | /// One lock per cached repository: fetches into it, and exports from it, |
|
| 770 | + | /// never overlap, while jobs of other repositories go on. A slow repository |
|
| 771 | + | /// (one kept in a bucket, say) once held every job on the runner waiting |
|
| 772 | + | /// behind a single global lock. |
|
| 773 | + | fn repo_lock(name: &str) -> std::sync::Arc<std::sync::Mutex<()>> { |
|
| 774 | + | static LOCKS: std::sync::OnceLock<std::sync::Mutex<std::collections::HashMap<String, std::sync::Arc<std::sync::Mutex<()>>>>> = |
|
| 775 | + | std::sync::OnceLock::new(); |
|
| 776 | + | let mut locks = LOCKS.get_or_init(Default::default).lock().unwrap_or_else(|p| p.into_inner()); |
|
| 777 | + | locks.entry(name.to_string()).or_default().clone() |
|
| 778 | + | } |
|
| 779 | + | ||
| 780 | + | /// Seconds, to one decimal place, for the job's log. |
|
| 781 | + | fn secs(d: std::time::Duration) -> String { |
|
| 782 | + | format!("{:.1} s", d.as_secs_f64()) |
|
| 783 | + | } |
|
| 784 | + | ||
| 764 | 785 | fn run_claim(home: &Home, hub: &Hub, cfg: &RunnerConfig, key: &RunnerKey, claim: &Claim) -> Result<Finish> { |
|
| 765 | 786 | let started = Instant::now(); |
|
| 766 | 787 | let mut sink = LogSink { hub, job: claim.job_id, buf: Vec::new(), last: Instant::now(), gone: false }; |
|
| … | 1 unchanged lines |
||
| 768 | 789 | |sink: &mut LogSink, text: String| sink.push(Record { t: started.elapsed().as_millis() as u64, stream: "flow".into(), text }); |
|
| 769 | 790 | let heads: Vec<ChangeHash> = claim.heads.iter().map(|h| ChangeHash::from_hex(h).context("bad head")).collect::<Result<_>>()?; |
|
| 770 | 791 | note(&mut sink, format!("fetching {} for state {}", claim.repo, claim.heads.iter().map(|h| &h[..12]).collect::<Vec<_>>().join("+"))); |
|
| 771 | - | // Preparing a workspace (the cached repositories, the export, the |
|
| 772 | - | // image) is one job at a time: fetches into a cached repository must not |
|
| 773 | - | // overlap, and the first clone passes its token through the process |
|
| 774 | - | // environment. Only the microVMs run side by side. |
|
| 775 | - | let mut preparing = Some(PREPARE.lock().unwrap_or_else(|p| p.into_inner())); |
|
| 792 | + | // Preparing a workspace: fetches into a cached repository, and exports |
|
| 793 | + | // from it, are one job at a time per repository (`repo_lock`); taking |
|
| 794 | + | // cache disks and imaging the workspace are one job at a time on the |
|
| 795 | + | // runner (`PREPARE`); a first clone, whose token goes through the process |
|
| 796 | + | // environment, one at a time (`CLONE`). |
|
| 797 | + | let main_lock = repo_lock(&claim.repo); |
|
| 798 | + | let asked = Instant::now(); |
|
| 799 | + | let mut main_guard = Some(main_lock.lock().unwrap_or_else(|p| p.into_inner())); |
|
| 800 | + | if asked.elapsed().as_secs() >= 2 { |
|
| 801 | + | note(&mut sink, format!("waited {} for another job fetching {}", secs(asked.elapsed()), claim.repo)); |
|
| 802 | + | } |
|
| 803 | + | let fetching = Instant::now(); |
|
| 776 | 804 | let repo = cached_repo(home, cfg, claim)?; |
|
| 805 | + | let fetched = fetching.elapsed(); |
|
| 806 | + | let mut preparing = None; |
|
| 777 | 807 | let scratch = home.root.join("work").join(format!("job-{}", claim.job_id)); |
|
| 778 | 808 | let _ = std::fs::remove_dir_all(&scratch); |
|
| 779 | 809 | std::fs::create_dir_all(scratch.join("tree"))?; |
|
| 780 | 810 | let result = (|| -> Result<Finish> { |
|
| 811 | + | let exporting = Instant::now(); |
|
| 781 | 812 | let mut files = workspace::export_state(&repo, &heads, &scratch.join("tree"))?; |
|
| 813 | + | main_guard.take(); |
|
| 814 | + | note(&mut sink, format!("fetched in {}, {} files exported in {}", secs(fetched), files.len(), secs(exporting.elapsed()))); |
|
| 782 | 815 | // Pinned subrepos (Oxygen spec §7.8), at their paths, as part of the |
|
| 783 | 816 | // workspace and of the scope hash. |
|
| 784 | 817 | for sub in &claim.subrepos { |
|
| … | 9 unchanged lines |
||
| 794 | 827 | sub.path |
|
| 795 | 828 | ), |
|
| 796 | 829 | ); |
|
| 830 | + | let sub_lock = repo_lock(&sub.repo); |
|
| 831 | + | let asked = Instant::now(); |
|
| 832 | + | let sub_guard = sub_lock.lock().unwrap_or_else(|p| p.into_inner()); |
|
| 833 | + | if asked.elapsed().as_secs() >= 2 { |
|
| 834 | + | note(&mut sink, format!("waited {} for another job fetching {}", secs(asked.elapsed()), sub.repo)); |
|
| 835 | + | } |
|
| 797 | 836 | let sub_repo = cached_repo_at(home, cfg, &sub.repo, &sub.clone_url, &claim.token)?; |
|
| 798 | 837 | let sub_heads: Vec<ChangeHash> = |
|
| 799 | 838 | sub.heads.iter().map(|h| ChangeHash::from_hex(h).context("bad subrepo head")).collect::<Result<_>>()?; |
|
| 800 | 839 | let dir = scratch.join("tree").join(&sub.path); |
|
| 801 | 840 | std::fs::create_dir_all(&dir)?; |
|
| 802 | 841 | let sub_files = workspace::export_state(&sub_repo, &sub_heads, &dir)?; |
|
| 842 | + | drop(sub_guard); |
|
| 803 | 843 | files.extend(sub_files.into_iter().map(|(p, h)| (format!("{}/{p}", sub.path), h))); |
|
| 804 | 844 | } |
|
| 805 | 845 | let scope_hash = scope_tree_hash(&claim.job.scope, files.iter().map(|(p, h)| (p.as_str(), *h))); |
|
| … | 4 unchanged lines |
||
| 810 | 850 | // microVM takes a disk already in place as it is. Taken before the |
|
| 811 | 851 | // workspace is imaged: the sources the disk was built from give |
|
| 812 | 852 | // unchanged files their old times back (#52). |
|
| 853 | + | let asked = Instant::now(); |
|
| 854 | + | preparing = Some(PREPARE.lock().unwrap_or_else(|p| p.into_inner())); |
|
| 855 | + | if asked.elapsed().as_secs() >= 2 { |
|
| 856 | + | note(&mut sink, format!("waited {} for another job imaging its workspace", secs(asked.elapsed()))); |
|
| 857 | + | } |
|
| 813 | 858 | let pool = crate::cache::Pool::new(&home.root, cfg.cache_gb); |
|
| 814 | 859 | let cache_ctx = |
|
| 815 | 860 | crate::cache::Ctx { repo: &claim.repo, bookmark: &claim.bookmark, job: &claim.job.name, image: &claim.sandbox.rootfs }; |
|
| … | 287 unchanged lines |
||
| 1103 | 1148 | with_retries(&FETCH_WAITS, || repo.fetch("origin").map(|_| ()).map_err(|e| anyhow::anyhow!("fetch {clone_url}: {e}")))?; |
|
| 1104 | 1149 | return Ok(repo); |
|
| 1105 | 1150 | } |
|
| 1106 | - | // First job for this repository: clone with the job's token. |
|
| 1151 | + | // First job for this repository: clone with the job's token, which goes |
|
| 1152 | + | // through the process environment, so one clone at a time. |
|
| 1153 | + | let _clone = CLONE.lock().unwrap_or_else(|p| p.into_inner()); |
|
| 1107 | 1154 | std::env::set_var("OXY_TOKEN", token); |
|
| 1108 | 1155 | if let Some(ca) = &cfg.ca { |
|
| 1109 | 1156 | std::env::set_var("OXY_CA", ca); |
|
| … | 311 unchanged lines |