diff --git a/docs/cleanup.md b/docs/cleanup.md index 2fbe4a22..bcd57816 100644 --- a/docs/cleanup.md +++ b/docs/cleanup.md @@ -344,6 +344,64 @@ everything else on the machine. Deleting them is a decision with your own containers on the other side of it, and `docker system df` is the tool that shows you what it costs. +### The bare caches' loose refs, and who packs them + +| artifact | who reclaims it | what makes that safe | +| --- | --- | --- | +| loose ref files under a bare cache's `refs/` | the background freshness sweep, with one `git pack-refs --all` after each fetch that succeeded | packing changes how a ref is stored and not which refs exist, so there is nothing to prove and nothing to consent to | + +Every ref a fetch updates is written as a file, and a file costs a whole +filesystem block: about 4096 bytes each, against the 81 or so the same ref takes +as a line in `packed-refs`. Nothing used to collapse them. `pack-refs --auto` is a +documented no-op on git's `files` ref backend, and `dl` runs no `gc` on a bare, so +`gc.auto` never gets the chance either. + +The cost is one block per **ref**, which means it tracks how many branches and +tags a remote leaves open and has almost nothing to do with how big the +repository is. Measured across ten real remotes with `git ls-remote`, +`torvalds/linux` carries 1887 refs against `microsoft/vscode`'s 5342 and +`rust-lang/rust`'s 334, with a median around 370. So a whole cache of 20 to 40 +repositories holds something like 30 MB to 60 MB of loose refs, a couple of +percent of one bare's own size. **Disk is not the reason this is here.** + +What carries it is placement. The broad sweep is the only thing in `dl` that +fetches every head and tag, so it is the only thing that makes loose refs in +quantity, and it already holds the repository's lock while it does. Packing there +costs one more bounded `git` call in a scope that just spent its whole network +budget, and it happens only on a pass that actually fetched. Measured on git +2.51.1 over a bare of 551 refs, 301 of them loose, those files held 1204 KiB of +blocks against a 30 KiB `packed-refs` for all 551, and the pack itself took 23 ms. + +There is a second payment, and it is banked rather than collected. A guard that +walks every ref on the bare to decide whether a clone is safe to remove reads one +file instead of thousands once the refs are packed, and such a probe measured +2.8 ms against 5.3 ms on that same bare. **No shipped code collects that yet.** +The bare-side reachability guard is decided and not built, and what ships today +asks the clone instead. So the saving is a reason to keep this once that guard +arrives, and it is not a reason this is here now. + +**A pack that refuses is not a fetch that failed.** The fetch is the point of the +sweep and the packing is the optional half, so a refusal becomes a notice +carrying the repository and git's own words, the record's freshness stamp still +lands, and the next sweep tries again. Withholding the stamp would make every +later pass re-fetch the whole repository forever on account of a representation +change that did not come off. + +That notice reaches nobody today, and the honest reading of why is that the sweep +runs detached with its output discarded, so every notice it raises goes to a null +descriptor and this is simply the first one that anybody would want to read. What +a refusal costs while it stays unread is bounded: loose refs are one file per ref +rewritten in place rather than appended, so a pack that keeps failing holds the +ref count flat at what one sweep writes instead of growing it. + +**Packing does not change what a later prune may delete.** A ref the remote +retracts is removed whether it was loose or packed: git rewrites `packed-refs` +through the same ref transaction that unlinks a loose file, and a ref that was +loose over a stale packed line loses both, so nothing comes back at an old sha. +The only difference is cost, and it falls on the prune rather than here, since +removing a packed ref rewrites the whole file where removing a loose one unlinks a +single path. + ### Reconciling records that disagree `dl` keeps its own record of every workspace, and devpod keeps one too. They diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index 99f4062e..0fcf7134 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -2079,6 +2079,10 @@ pub devlaunch_core::flows::repo_manager::CacheNotice::RefNotFetched::branch: all pub devlaunch_core::flows::repo_manager::CacheNotice::RefNotFetched::owner: alloc::string::String pub devlaunch_core::flows::repo_manager::CacheNotice::RefNotFetched::reason: devlaunch_core::flows::repo_manager::NotRefreshed pub devlaunch_core::flows::repo_manager::CacheNotice::RefNotFetched::repo: alloc::string::String +pub devlaunch_core::flows::repo_manager::CacheNotice::RefsNotPacked +pub devlaunch_core::flows::repo_manager::CacheNotice::RefsNotPacked::owner: alloc::string::String +pub devlaunch_core::flows::repo_manager::CacheNotice::RefsNotPacked::reason: alloc::string::String +pub devlaunch_core::flows::repo_manager::CacheNotice::RefsNotPacked::repo: alloc::string::String pub devlaunch_core::flows::repo_manager::CacheNotice::TrackedFilesNotListed pub devlaunch_core::flows::repo_manager::CacheNotice::TrackedFilesNotListed::reason: alloc::string::String pub devlaunch_core::flows::repo_manager::CacheNotice::WorkspaceCloneRemoved diff --git a/rust/devlaunch-core/src/clients/git.rs b/rust/devlaunch-core/src/clients/git.rs index ae8c8885..240daf21 100644 --- a/rust/devlaunch-core/src/clients/git.rs +++ b/rust/devlaunch-core/src/clients/git.rs @@ -586,6 +586,50 @@ impl<'r> Git<'r> { self.captured("fetch", &spec) } + /// Collapse every loose ref in the bare into `packed-refs`. + /// + /// A ref git writes as a file costs a whole filesystem block, typically 4096 + /// bytes against the ~81 the packed line takes, and [`Git::fetch_all`] writes + /// one per ref it updates. Nothing else in devlaunch ever packed them: + /// `pack-refs --auto` is a documented no-op on the `files` backend, and no + /// `gc` is run on the bare, so `gc.auto` never gets the chance either. + /// Measured on git 2.51.1 over a bare with 301 loose refs of 551, the loose + /// files held 1204 KiB of blocks against a 30 KiB `packed-refs` for all 551, + /// and the pack took 23 ms. + /// + /// `--all` rather than the default, and the difference is the whole verb here + /// rather than a nicety: measured on 2.51.1 against a bare holding 301 loose + /// heads and 101 loose tags, a bare `pack-refs` took the tags to zero and left + /// all 301 heads exactly where they were. Heads are the population a broad + /// sweep of a real repository mostly makes. + /// + /// **Pure, in the sense that decides where this is allowed to live.** Packing + /// changes how a ref is stored and not which refs exist, so it can lose no + /// work — and it does not change what a later `--prune` may delete either. + /// Measured on 2.51.1: a prune of a packed ref rewrites `packed-refs` through + /// the same ref transaction, deletes nothing else, and a ref that was loose + /// over a stale packed line loses *both*, so nothing is resurrected at the old + /// sha. The only difference is cost, and it falls on the prune: removing a + /// packed ref rewrites the whole file where removing a loose one unlinks + /// a single path. + /// + /// Bounded at [`ABOUT_ONE_REPO`] rather than left unbounded like the sweep's + /// fetch: this touches no network, so a pack that has not finished in thirty + /// seconds is a stuck filesystem rather than a slow remote, and the caller it + /// runs under is a detached child whose whole point is that nobody is watching + /// it. + pub(crate) fn pack_refs(&self, bare: &Path) -> GitAnswer { + self.captured( + "pack-refs", + &SpawnSpec::new( + Invocation::new(PROGRAM) + .with_args(["pack-refs", "--all"]) + .with_cwd(bare.to_path_buf()), + ) + .with_timeout(ABOUT_ONE_REPO), + ) + } + /// Fetch exactly one branch into the bare cache. /// /// The launch path's entire network budget, so the time it can hold the repo diff --git a/rust/devlaunch-core/src/clients/git/tests.rs b/rust/devlaunch-core/src/clients/git/tests.rs index 73ecdec4..f80a9cbb 100644 --- a/rust/devlaunch-core/src/clients/git/tests.rs +++ b/rust/devlaunch-core/src/clients/git/tests.rs @@ -503,6 +503,21 @@ fn the_background_sweep_s_bound_reaches_the_spawn() { assert_eq!(timeout(&fake), Some(Duration::from_secs(60))); } +#[test] +fn packing_collapses_every_loose_ref_in_the_bare_under_a_bound() { + // `--all` and not the default, which packs tags alone and would leave every + // head the sweep just fetched sitting loose. The bound is there because this + // touches no network: thirty seconds of `pack-refs` is a stuck filesystem, and + // the caller is a detached child nobody is watching. + let fake = ScriptedRunner::new(); + + Git::new(&fake).pack_refs(Path::new("/cache/o/r/.bare")); + + assert_eq!(strs(&argv(&fake)), ["git", "pack-refs", "--all"]); + assert_eq!(cwd(&fake).as_deref(), Some(Path::new("/cache/o/r/.bare"))); + assert_eq!(timeout(&fake), Some(Duration::from_secs(30))); +} + #[test] fn fetching_one_ref_moves_exactly_that_ref_in_the_c_locale() { let fake = ScriptedRunner::new(); @@ -967,6 +982,7 @@ fn nothing_here_spawns_more_than_once_per_verb() { git.clone_bare("url", Path::new("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/cache/.bare")); git.fetch_all(Path::new("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/cache/.bare"), None); + git.pack_refs(Path::new("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/cache/.bare")); git.fetch_ref(Path::new("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/cache/.bare"), "feature"); git.symbolic_ref(Path::new("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/cache/.bare"), "HEAD"); git.remote_branch_listing(Path::new("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/cache/.bare")); @@ -993,7 +1009,7 @@ fn nothing_here_spawns_more_than_once_per_verb() { git.status_porcelain(Path::new("/ws")); git.unpushed_commits(Path::new("/ws")); - assert_eq!(fake.call_count(), 27, "one spawn per verb, 27 verbs"); + assert_eq!(fake.call_count(), 28, "one spawn per verb, 28 verbs"); assert!( fake.calls() .iter() diff --git a/rust/devlaunch-core/src/flows/repo_manager.rs b/rust/devlaunch-core/src/flows/repo_manager.rs index fa521570..6c300246 100644 --- a/rust/devlaunch-core/src/flows/repo_manager.rs +++ b/rust/devlaunch-core/src/flows/repo_manager.rs @@ -246,6 +246,18 @@ pub enum CacheNotice { base: String, reason: NotRefreshed, }, + /// The loose refs the sweep's own fetch wrote could not be collapsed into + /// `packed-refs`. The fetch itself succeeded, so this is a notice and never a + /// [`FetchRepoError`]: the cache is fresh either way and the only cost is that + /// the bare keeps a block per ref until the next sweep tries again. + /// + /// Names the repository because the sweep walks every repository in one + /// detached process, so a line that did not say which one would be unactionable. + RefsNotPacked { + owner: String, + repo: String, + reason: String, + }, /// The cache's git-lfs store could not be filled. Best-effort: the workspace /// falls through to the network phase. LfsCacheNotFilled { reason: String }, @@ -1398,6 +1410,35 @@ impl<'r> RepositoryManager<'r> { }); } + // Where the loose refs the fetch just wrote get collapsed into + // `packed-refs`. Here rather than anywhere else because `fetch_all` has + // exactly one production caller, so "after the fetch" *is* the detached + // background sweep by construction, in the repo-lock scope it already + // holds. + // + // A second payment is banked here rather than collected: a guard that + // walks every ref on the bare reads one file instead of thousands once + // they are packed (measured on git 2.51.1, 2.8 ms against 5.3 ms over + // 551 refs, 301 of them loose). Nothing collects it yet. That guard is + // decided and unbuilt, and the probe that ships asks the *clone*, not + // the bare. Placement is what carries this call today. + // + // **A refusal here is not a failed fetch.** The fetch happened, the cache + // is fresh, and the stamp below has to land or the sweep re-fetches this + // repository every interval forever on account of a representation change + // that did not come off. Packing is the optional half of this function and + // the fetch is the point of it. Its own arm rather than `let _ =`, for the + // reason [`RecordUpdate::Absent`] gets one below: a caller that cannot say + // what it does with an answer is the shape that turns a no-op into a + // reported success. + if let Some(refused) = self.git.pack_refs(&bare).refusal() { + notices.say(CacheNotice::RefsNotPacked { + owner: owner.to_owned(), + repo: repo.to_owned(), + reason: refused.reason().to_owned(), + }); + } + // The stamp is the only field this touches, so it moves inside the // metadata lock rather than riding back in a copy of the whole record // taken before the lock existed. @@ -2039,6 +2080,32 @@ pub(crate) mod tests { refs } + /// Every loose ref file under a repository's `refs/`, sorted. + /// + /// A ref git has packed is a line in `packed-refs` and no file here, so an + /// empty answer beside a non-empty [`refs_of`] is what "packed" looks like + /// from the filesystem. Directories are walked rather than listed, because a + /// ref name carries slashes and `refs/heads/feature/test` is two levels down. + pub(crate) fn loose_refs_of(repo: &Path) -> Vec { + fn walk(dir: &Path, found: &mut Vec) { + let Ok(listing) = std::fs::read_dir(dir) else { + return; + }; + for entry in listing.filter_map(Result::ok) { + let path = entry.path(); + if path.is_dir() { + walk(&path, found); + } else { + found.push(path); + } + } + } + let mut found = Vec::new(); + walk(&repo.join("refs"), &mut found); + found.sort(); + found + } + /// A real git client over this process's `git`. pub(crate) fn real_git() -> ProcessRunner { ProcessRunner::new() @@ -2450,7 +2517,7 @@ pub(crate) mod tests { .fetch_repo(&mut cache.storage, "owner", "repo", None, &mut ignoring()) .expect("fetched"); - let call = fake.only_call(); + let call = fake.calls().swap_remove(0); assert_eq!( as_strs(&[call.argv()])[0], [ @@ -2474,6 +2541,105 @@ pub(crate) mod tests { ); } + #[test] + fn the_sweep_packs_the_refs_after_the_fetch_and_before_it_stamps_the_record() { + // Both halves of the order are load-bearing. After the fetch is what makes + // the refs it packs the ones *this* pass wrote loose. Before the stamp is + // what puts a refusal on stderr ahead of the line that closes the block, + // since `FetchedUpdates` means the fetch and its bookkeeping are both done + // and a warning arriving after it would read as being about the next + // repository the sweep walks. + let mut cache = a_cache(); + let bare = cache.given_bare_clone("owner", "repo"); + cache.given_record("owner", "repo"); + let fake = FakeGit::new(); + let manager = a_manager(&cache, Git::new(&fake)); + + manager + .fetch_repo(&mut cache.storage, "owner", "repo", None, &mut ignoring()) + .expect("fetched"); + + let argvs = fake.argvs(); + assert_eq!( + as_strs(&argvs), + [ + vec![ + "git", + "fetch", + "origin", + "+refs/heads/*:refs/heads/*", + "+refs/tags/*:refs/tags/*", + "--prune", + ], + vec!["git", "pack-refs", "--all"], + ] + ); + assert_eq!( + fake.calls()[1].invocation().cwd.as_deref(), + Some(bare.as_path()), + "the pack is about the bare the fetch just wrote into" + ); + } + + #[test] + fn a_pack_that_refused_is_reported_and_the_fetch_still_counts_as_done() { + // Principle 1 decides this and it is not close: the fetch happened, so the + // cache is fresh and nothing is at risk. Reporting a failed fetch here + // would be a lie about the network, and withholding the stamp would make + // every later sweep re-fetch the whole repository forever on account of a + // representation change that did not come off. Packing is the optional + // half of this function; the fetch is the point of it. + let mut cache = a_cache(); + cache.given_bare_clone("owner", "repo"); + cache.given_record("owner", "repo"); + let fake = FakeGit::new().with_script( + ["git", "pack-refs"], + Response::failed( + 1, + "fatal: unable to create 'packed-refs.lock': Permission denied\n", + ), + ); + let manager = a_manager(&cache, Git::new(&fake)); + let mut notices = ignoring(); + + manager + .fetch_repo(&mut cache.storage, "owner", "repo", None, &mut notices) + .expect("a pack that refused is not a fetch that failed"); + + // In this order, which is what the placement buys: the refusal names the + // repository and lands *before* the line that closes the block, so a sweep + // walking a whole cache cannot read as though the warning belonged to the + // next repository along. + assert_eq!( + notices, + vec![ + CacheNotice::FetchingUpdates { + owner: "owner".to_owned(), + repo: "repo".to_owned(), + }, + CacheNotice::RefsNotPacked { + owner: "owner".to_owned(), + repo: "repo".to_owned(), + reason: "fatal: unable to create 'packed-refs.lock': Permission denied" + .to_owned(), + }, + CacheNotice::FetchedUpdates { + owner: "owner".to_owned(), + repo: "repo".to_owned(), + }, + ] + ); + assert!( + cache + .storage + .get_repository("owner", "repo") + .expect("the record") + .last_fetched + .is_some(), + "the stamp is about the fetch, and the fetch happened" + ); + } + #[test] fn a_sweep_says_which_repository_it_is_fetching_and_that_it_finished() { // Python's two `logger.info` lines around the fetch, and both are worth @@ -2567,7 +2733,9 @@ pub(crate) mod tests { .expect("fetched"); assert_eq!(BACKGROUND_FETCH_TIMEOUT, Duration::from_secs(300)); - match fake.only_call() { + // The fetch, which is the call the bound is about. The pack beside it + // carries its own thirty seconds and never the network's budget. + match fake.calls().swap_remove(0) { devlaunch_test_support::Call::Capture(spec) => { assert_eq!(spec.timeout, Some(BACKGROUND_FETCH_TIMEOUT)); } @@ -2873,7 +3041,10 @@ pub(crate) mod tests { .expect("fetched"), Fetched::Fetched ); - assert_eq!(fake.call_count(), 1); + // The fetch and the pack that follows it. A skipped pass below spawns + // neither, which is what makes the pack cost nothing on a pass that did + // not fetch. + assert_eq!(fake.call_count(), 2); // The fetch above wrote `last_fetched`, so the next pass is inside the // interval. @@ -3582,6 +3753,60 @@ pub(crate) mod tests { ); } + #[test] + fn real_git_the_sweep_packs_the_loose_refs_its_own_fetch_just_wrote() { + // Every ref a fetch updates is written as a loose file costing a whole + // filesystem block, and nothing in production ever packed them: `pack-refs + // --auto` is a no-op on the `files` backend and no `gc` runs on the bare. + // The packing belongs on the sweep because the sweep is what makes them -- + // `fetch_all` has exactly one production caller, so "after the fetch" is + // the detached background sweep by construction, inside a lock scope it + // already holds. + let cache = a_cache(); + let remote = a_fixture_remote(cache.dir.path()); + let runner = real_git(); + let manager = a_manager(&cache, Git::new(&runner)); + let mut storage = cache.storage; + manager + .clone_repo(&mut storage, "test", "repo", &remote.url, &mut ignoring()) + .expect("cloned"); + let bare = manager.bare_dir("test", "repo"); + // A fresh clone arrives packed, so the loose refs have to be *made* rather + // than assumed: every ref this fetch updates is a file git writes under + // `refs/`, shadowing the stale line still sitting in `packed-refs`. + let moved = commit_on(&remote.work, "main", "one.txt", "Move main"); + run_git(&remote.work, &["tag", "v1"]); + run_git(&remote.work, &["push", "origin", "v1"]); + + manager + .fetch_repo(&mut storage, "test", "repo", None, &mut ignoring()) + .expect("fetched"); + + assert_eq!( + loose_refs_of(&bare), + Vec::::new(), + "the sweep left its own loose refs behind" + ); + // Not decoration: it is what says the assertion above is about a ref the + // fetch really did write loose. `main` moved and `v1` is new, so a fetch + // that packed nothing would leave both as files and `packed-refs` holding + // the sha from clone time. + let packed = std::fs::read_to_string(bare.join("packed-refs")).expect("a packed-refs"); + assert!( + packed.contains(&format!("{moved} refs/heads/main")), + "packed-refs still holds the pre-fetch main: {packed}" + ); + assert!( + packed.contains("refs/tags/v1"), + "the tag the fetch created is not in packed-refs: {packed}" + ); + assert_eq!( + refs_of(&bare), + ["refs/heads/feature/test", "refs/heads/main", "refs/tags/v1"], + "packing changes the representation and must lose no ref" + ); + } + #[test] fn real_git_leaves_a_record_alone_when_only_the_directory_is_gone() { // A restored backup, a hand-deleted cache, a half-finished `dl --purge`. diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 849ec1dd..21bfa4d9 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -982,6 +982,11 @@ fn cache_notice(notice: &CacheNotice) -> Option { refreshed ({}); it may be behind the remote.", not_refreshed(reason) ), + CacheNotice::RefsNotPacked { + owner, + repo, + reason, + } => format!("Could not pack the refs of {owner}/{repo}: {reason}"), CacheNotice::LfsCacheNotFilled { reason } => { format!("Could not fill the cache's git-lfs store: {reason}") } @@ -3079,6 +3084,25 @@ mod tests { ); } + #[test] + fn a_pack_the_sweep_could_not_do_names_the_repository_and_gits_own_words() { + // The sweep walks every repository in one detached process, so a line that + // did not name one would be unactionable. It is a notice and not an error + // because the fetch beside it succeeded: the cost of the refusal is a + // filesystem block per ref until the next sweep, and nothing else. + let said = |notice: CacheNotice| cache_notice(¬ice).expect("a line"); + + assert_eq!( + said(CacheNotice::RefsNotPacked { + owner: "blooop".to_owned(), + repo: "devlaunch".to_owned(), + reason: "fatal: unable to create 'packed-refs.lock': Permission denied".to_owned(), + }), + "Could not pack the refs of blooop/devlaunch: fatal: unable to create \ + 'packed-refs.lock': Permission denied" + ); + } + #[test] fn the_branch_decision_reads_as_the_four_lines_python_logged() { // `worktree/branch_manager.py` 49/56/62/67. Which of the four happened is an