From eb7604d952b808d0e23a3cf56f3fad1f48635bdf Mon Sep 17 00:00:00 2001 From: dawn Date: Mon, 5 Oct 2026 15:04:10 +0300 Subject: [PATCH] knot2/git: skip bitmap generation for repos that would otherwise OOM Signed-off-by: dawn --- knot2/crates/knot-git/src/bitmap/midx.rs | 21 +++--- knot2/crates/knot-git/src/bitmap/mod.rs | 11 ++- knot2/crates/knot-git/src/bitmap/writer.rs | 69 +++++++++++++++---- knot2/crates/knot-git/src/lib.rs | 4 +- knot2/crates/knot-git/tests/bitmap.rs | 9 ++- knot2/crates/knot-maintenance/src/bitmap.rs | 6 +- knot2/crates/knot-maintenance/src/lib.rs | 7 +- .../crates/knot-maintenance/src/scheduler.rs | 9 ++- knot2/crates/knot-maintenance/tests/engine.rs | 8 +-- knot2/crates/knot-pack/tests/differential.rs | 3 +- knot2/crates/knot-pack/tests/serving.rs | 3 +- 11 files changed, 110 insertions(+), 40 deletions(-) diff --git a/knot2/crates/knot-git/src/bitmap/midx.rs b/knot2/crates/knot-git/src/bitmap/midx.rs index e3862daa9..eeb429be8 100644 --- a/knot2/crates/knot-git/src/bitmap/midx.rs +++ b/knot2/crates/knot-git/src/bitmap/midx.rs @@ -4,6 +4,7 @@ use knot_types::Oid; use crate::objects::{Haves, Wants}; +use super::BitmapStatus; use super::reader; use super::revindex::{Order, OrderTable}; use super::writer; @@ -24,7 +25,7 @@ fn sidecar(objects_dir: &Path, checksum: &gix_hash::ObjectId, ext: &str) -> Path .join(format!("multi-pack-index-{}.{ext}", checksum.to_hex())) } -pub(super) fn write(repo: &Repo) -> Result { +pub(super) fn write(repo: &Repo) -> Result { let kind = repo.object_format().kind(); let objects_dir = repo.objects_dir(); let file = match gix_pack::multi_index::File::at( @@ -32,19 +33,23 @@ pub(super) fn write(repo: &Repo) -> Result { Some(MIDX_ALLOC_LIMIT_BYTES), ) { Ok(file) => file, - Err(_) => return Ok(false), + Err(_) => return Ok(BitmapStatus::Unchanged), }; let order = OrderTable::from_file(&file); if order.len() == 0 { - return Ok(false); + return Ok(BitmapStatus::Unchanged); + } + let tips = writer::ref_tips(repo, &order)?; + if tips.is_empty() { + return Ok(BitmapStatus::Unchanged); + } + if !writer::selection_fits(tips.len(), order.len()) { + return Ok(BitmapStatus::SkippedTooLarge); } let _boost = knot_resource::saturate(); let types = writer::type_index_bits(repo, &order)?; - let selected = writer::selected_entries(repo, &order)?; - if selected.is_empty() { - return Ok(false); - } + let selected = writer::selected_entries(repo, &order, &tips)?; let checksum = file.checksum(); let bytes = writer::assemble(kind, &checksum, &types, &selected)?; @@ -56,7 +61,7 @@ pub(super) fn write(repo: &Repo) -> Result { &checksum, )?; writer::install(&sidecar(&objects_dir, &checksum, "bitmap"), &bytes)?; - Ok(true) + Ok(BitmapStatus::Written) } fn write_rev( diff --git a/knot2/crates/knot-git/src/bitmap/mod.rs b/knot2/crates/knot-git/src/bitmap/mod.rs index e5896a945..75a10f261 100644 --- a/knot2/crates/knot-git/src/bitmap/mod.rs +++ b/knot2/crates/knot-git/src/bitmap/mod.rs @@ -21,11 +21,18 @@ knot_types::scalar_newtype! { pub(crate) struct BitmapEntryOffset(u64); } -pub fn write_bitmap(repo: &Repo, pack_idx: &Path) -> Result { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BitmapStatus { + Written, + Unchanged, + SkippedTooLarge, +} + +pub fn write_bitmap(repo: &Repo, pack_idx: &Path) -> Result { writer::write(repo, pack_idx) } -pub fn write_midx_bitmap(repo: &Repo) -> Result { +pub fn write_midx_bitmap(repo: &Repo) -> Result { midx::write(repo) } diff --git a/knot2/crates/knot-git/src/bitmap/writer.rs b/knot2/crates/knot-git/src/bitmap/writer.rs index 8afaa4406..782b16154 100644 --- a/knot2/crates/knot-git/src/bitmap/writer.rs +++ b/knot2/crates/knot-git/src/bitmap/writer.rs @@ -4,7 +4,7 @@ use std::path::Path; use knot_types::Oid; use super::revindex::{Order, OrderTable}; -use super::{BitPosition, BitmapEntryOffset, IndexPosition}; +use super::{BitPosition, BitmapEntryOffset, BitmapStatus, IndexPosition}; use crate::error::GitError; use crate::objects::{Haves, Wants}; use crate::repo::Repo; @@ -29,21 +29,25 @@ pub(super) struct Selected { bits: Vec, } -pub(crate) fn write(repo: &Repo, pack_idx: &Path) -> Result { +pub(crate) fn write(repo: &Repo, pack_idx: &Path) -> Result { let kind = repo.object_format().kind(); let index = gix_pack::index::File::at(pack_idx, kind) .map_err(|error| GitError::Backend(format!("open pack index: {error}")))?; let rev = OrderTable::from_index(&index); if rev.len() == 0 { - return Ok(false); + return Ok(BitmapStatus::Unchanged); + } + let tips = ref_tips(repo, &rev)?; + if tips.is_empty() { + return Ok(BitmapStatus::Unchanged); + } + if !selection_fits(tips.len(), rev.len()) { + return Ok(BitmapStatus::SkippedTooLarge); } let _boost = knot_resource::saturate(); let types = type_index_bits(repo, &rev)?; - let selected = selected_entries(repo, &rev)?; - if selected.is_empty() { - return Ok(false); - } + let selected = selected_entries(repo, &rev, &tips)?; let pack_path = pack_idx.with_extension("pack"); let checksum = gix_pack::data::File::at(&pack_path, kind) @@ -52,7 +56,7 @@ pub(crate) fn write(repo: &Repo, pack_idx: &Path) -> Result { let bytes = assemble(kind, &checksum, &types, &selected)?; install(&pack_idx.with_extension("bitmap"), &bytes)?; - Ok(true) + Ok(BitmapStatus::Written) } pub(super) fn type_index_bits(repo: &Repo, rev: &R) -> Result { @@ -110,21 +114,45 @@ fn one_selected( }) } -pub(super) fn selected_entries( +pub(super) fn ref_tips( repo: &Repo, rev: &R, -) -> Result, GitError> { +) -> Result, GitError> { let mut seen: HashSet = HashSet::new(); - let commits: Vec<(IndexPosition, Oid)> = repo + Ok(repo .references()? .into_iter() .filter_map(|record| peel_to_commit(repo, record.target)) .filter_map(|commit| rev.index_of(commit).map(|position| (position, commit))) .filter(|(_, commit)| seen.insert(*commit)) - .collect(); + .collect()) +} + +// a repo like nixpkgs causes bitmap generation to hold a whole-pack Vec for +// every ref, which blows up the mem usage, so lets not do that +pub(super) fn selection_fits(tips: usize, objects: usize) -> bool { + selection_fits_in( + tips, + objects, + knot_resource::available_bytes().map(|available| available.get()), + ) +} + +// with no cgroup or meminfo to read we still refuse, as if the box were small +const UNKNOWN_AVAILABLE_BYTES: u64 = 4 * 1024 * 1024 * 1024; + +fn selection_fits_in(tips: usize, objects: usize, available: Option) -> bool { + let needed = (tips as u64).saturating_mul(objects as u64); + needed <= available.unwrap_or(UNKNOWN_AVAILABLE_BYTES) / 2 +} +pub(super) fn selected_entries( + repo: &Repo, + rev: &R, + tips: &[(IndexPosition, Oid)], +) -> Result, GitError> { let path = repo.path().to_owned(); - let mut entries: Vec = knot_resource::map_chunks(&commits, |batch| { + let mut entries: Vec = knot_resource::map_chunks(tips, |batch| { let local = Repo::open(&path)?; batch .iter() @@ -264,6 +292,21 @@ mod tests { assert_eq!(bits, vec![true, false, true]); } + #[test] + fn selection_fits_only_when_the_tip_bitmaps_take_at_most_half_of_free_memory() { + assert!(selection_fits_in(1_000, 1_000_000, Some(2_000_000_000))); + assert!(!selection_fits_in(1_001, 1_000_000, Some(2_000_000_000))); + assert!( + !selection_fits_in(414_167, 11_275_510, Some(12 * 1024 * 1024 * 1024)), + "a nixpkgs mirror is refused" + ); + assert!(selection_fits_in(1_000, 1_000_000, None)); + assert!( + !selection_fits_in(414_167, 11_275_510, None), + "unknown memory still refuses a nixpkgs mirror" + ); + } + #[test] fn closure_bits_errors_when_an_object_is_outside_the_pack() { let order = FakeOrder { diff --git a/knot2/crates/knot-git/src/lib.rs b/knot2/crates/knot-git/src/lib.rs index 83fbd753d..6c7d62504 100644 --- a/knot2/crates/knot-git/src/lib.rs +++ b/knot2/crates/knot-git/src/lib.rs @@ -14,7 +14,9 @@ mod repo; mod staging; pub use archive::{ArchiveFormat, ArchiveLimit, ArchivePrefix, TreePrefix}; -pub use bitmap::{reachable_via_bitmap, verbatim_clone_pack, write_bitmap, write_midx_bitmap}; +pub use bitmap::{ + BitmapStatus, reachable_via_bitmap, verbatim_clone_pack, write_bitmap, write_midx_bitmap, +}; pub use error::{GitError, SelectionLimit}; pub use maintenance::{PackRefsReport, ReflogReport}; pub use objects::{ diff --git a/knot2/crates/knot-git/tests/bitmap.rs b/knot2/crates/knot-git/tests/bitmap.rs index a4f16c63b..8150c3634 100644 --- a/knot2/crates/knot-git/tests/bitmap.rs +++ b/knot2/crates/knot-git/tests/bitmap.rs @@ -3,7 +3,8 @@ use std::io::Read; use std::path::{Path, PathBuf}; use knot_git::{ - Haves, Repo, Wants, reachable_via_bitmap, verbatim_clone_pack, write_bitmap, write_midx_bitmap, + BitmapStatus, Haves, Repo, Wants, reachable_via_bitmap, verbatim_clone_pack, write_bitmap, + write_midx_bitmap, }; use knot_types::Oid; @@ -117,8 +118,9 @@ fn run_lifecycle(format: &str) { let path = dir.path(); let repo = Repo::open(path).unwrap(); let idx = find_pack(path, ".idx"); - assert!( + assert_eq!( write_bitmap(&repo, &idx).unwrap(), + BitmapStatus::Written, "a single-pack repo gets a bitmap" ); test_bitmap(path); @@ -191,8 +193,9 @@ fn run_lifecycle(format: &str) { assert!(idx_count(path) >= 2, "the repo now spans multiple packs"); let repo = Repo::open(path).unwrap(); - assert!( + assert_eq!( write_midx_bitmap(&repo).unwrap(), + BitmapStatus::Written, "a multi-pack repo gets a midx bitmap" ); test_bitmap(path); diff --git a/knot2/crates/knot-maintenance/src/bitmap.rs b/knot2/crates/knot-maintenance/src/bitmap.rs index 1e2daf7e9..ba166b3a9 100644 --- a/knot2/crates/knot-maintenance/src/bitmap.rs +++ b/knot2/crates/knot-maintenance/src/bitmap.rs @@ -1,6 +1,6 @@ use std::path::Path; -use knot_git::Repo; +use knot_git::{BitmapStatus, Repo}; use crate::MaintError; use crate::fsio::{self, MIDX_SIDECAR_PREFIX, PackStem}; @@ -11,7 +11,7 @@ pub fn exists(objects_dir: &Path) -> bool { .any(|idx| idx.with_extension("bitmap").exists()) } -pub fn refresh(repo: &Repo, objects_dir: &Path) -> Result { +pub fn refresh(repo: &Repo, objects_dir: &Path) -> Result { let idxs = fsio::pack_idx_paths(objects_dir); match idxs.as_slice() { [only] => { @@ -23,7 +23,7 @@ pub fn refresh(repo: &Repo, objects_dir: &Path) -> Result { } [] => { prune_sidecars(objects_dir, None); - Ok(false) + Ok(BitmapStatus::Unchanged) } _ => { let wrote = knot_git::write_midx_bitmap(repo) diff --git a/knot2/crates/knot-maintenance/src/lib.rs b/knot2/crates/knot-maintenance/src/lib.rs index f6b36e5d1..5a908c6bb 100644 --- a/knot2/crates/knot-maintenance/src/lib.rs +++ b/knot2/crates/knot-maintenance/src/lib.rs @@ -16,6 +16,7 @@ mod scheduler; #[cfg(test)] mod test_support; +pub use knot_git::BitmapStatus; pub use midx::MidxStatus; pub use scheduler::{MaintenanceHandle, PushBytes, RepoSource, Scheduler}; @@ -212,7 +213,7 @@ pub struct Report { pub repack: RepackReport, pub prune: PruneReport, pub multi_pack_index: MidxStatus, - pub bitmap: bool, + pub bitmap: BitmapStatus, } impl Report { @@ -227,7 +228,7 @@ impl Report { repack: RepackReport::skipped(RepackStatus::Clean), prune: PruneReport::skipped(), multi_pack_index: MidxStatus::Absent, - bitmap: false, + bitmap: BitmapStatus::Unchanged, } } } @@ -322,7 +323,7 @@ pub fn run_repo( let bitmap = if opts.bitmap { bitmap::refresh(repo, &objects_dir)? } else { - false + BitmapStatus::Unchanged }; Ok(Report { diff --git a/knot2/crates/knot-maintenance/src/scheduler.rs b/knot2/crates/knot-maintenance/src/scheduler.rs index 1b5b51a1f..9cf0d41dc 100644 --- a/knot2/crates/knot-maintenance/src/scheduler.rs +++ b/knot2/crates/knot-maintenance/src/scheduler.rs @@ -8,7 +8,7 @@ use knot_runtime::Clock; use knot_types::{RepoDid, UnixSeconds}; use tokio::sync::{mpsc, watch}; -use crate::{LfsGrace, Options, RepackStatus, Report, SweepInterval, run_repo}; +use crate::{BitmapStatus, LfsGrace, Options, RepackStatus, Report, SweepInterval, run_repo}; pub trait RepoSource: Send + Sync + 'static { fn repos(&self) -> Vec; @@ -301,6 +301,13 @@ fn report_skips(repo: &RepoDid, report: &Report) { } _ => {} } + if report.bitmap == BitmapStatus::SkippedTooLarge { + tracing::warn!( + repo = %repo, + reason = "ref tips times pack objects exceed free memory", + "skipped bitmap" + ) + } } #[cfg(test)] diff --git a/knot2/crates/knot-maintenance/tests/engine.rs b/knot2/crates/knot-maintenance/tests/engine.rs index 4d515be4b..1fd75a4cb 100644 --- a/knot2/crates/knot-maintenance/tests/engine.rs +++ b/knot2/crates/knot-maintenance/tests/engine.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use knot_git::Repo; -use knot_maintenance::{Options, PruneGrace, RepackStatus, run_repo}; +use knot_maintenance::{BitmapStatus, Options, PruneGrace, RepackStatus, run_repo}; use knot_types::{ObjectFormat, Oid}; mod common; @@ -63,7 +63,7 @@ fn a_single_pack_maintenance_pass_packs_graphs_bitmaps_prunes_then_settles() { !leaked.exists(), "maintenance sweeps a crashed graph write's temp once it is too old to still have a writer" ); - assert!(report.bitmap && has_bitmap(&repo)); + assert!(report.bitmap == BitmapStatus::Written && has_bitmap(&repo)); let (ok, stderr) = git(&repo, &["rev-list", "--test-bitmap", "main"]); assert!(ok, "canonical git accepts our bitmap: {stderr}"); assert!(report.packed_refs.packed >= 1); @@ -80,7 +80,7 @@ fn a_single_pack_maintenance_pass_packs_graphs_bitmaps_prunes_then_settles() { let settled = run_repo(&reopened, now(), &options()).unwrap(); assert_eq!(settled.repack.status, RepackStatus::Clean); assert!( - !settled.prune.ran && !settled.commit_graph && !settled.bitmap, + !settled.prune.ran && !settled.commit_graph && settled.bitmap == BitmapStatus::Unchanged, "a settled repo is a no-op" ); assert_eq!(settled.packed_refs.packed, 0); @@ -192,7 +192,7 @@ fn cruft_second_pack_gets_a_midx_bitmap_canonical_git_accepts() { "young unreachable object is crufted" ); assert!( - report.bitmap && has_midx_bitmap(&repo), + report.bitmap == BitmapStatus::Written && has_midx_bitmap(&repo), "the multi-pack repo gets a midx bitmap" ); let (ok, stderr) = git(&repo, &["rev-list", "--test-bitmap", "main"]); diff --git a/knot2/crates/knot-pack/tests/differential.rs b/knot2/crates/knot-pack/tests/differential.rs index 7ec0d5a7d..38a6466e1 100644 --- a/knot2/crates/knot-pack/tests/differential.rs +++ b/knot2/crates/knot-pack/tests/differential.rs @@ -275,8 +275,9 @@ async fn clone_lifecycle(format: ObjectFormat, did: &str, name: &str) { &["-c", "repack.writeBitmaps=false", "repack", "-adq"], ); let repo = knot_git::Repo::open(&s.knot_bare).unwrap(); - assert!( + assert_eq!( knot_git::write_bitmap(&repo, &single_pack_idx(&s.knot_bare)).unwrap(), + knot_git::BitmapStatus::Written, "{format:?} single-pack repo gets a bitmap" ); must(&s.knot_bare, &["rev-list", "--test-bitmap", "HEAD"]); diff --git a/knot2/crates/knot-pack/tests/serving.rs b/knot2/crates/knot-pack/tests/serving.rs index 0cbc26c26..a81608ed0 100644 --- a/knot2/crates/knot-pack/tests/serving.rs +++ b/knot2/crates/knot-pack/tests/serving.rs @@ -273,10 +273,11 @@ fn the_bitmap_fast_path_serves_the_same_pack_as_the_object_walk() { let walk = common::unsideband(&knot_pack::upload_pack(&repo, &body).unwrap()); let now = UnixSeconds::new(1_700_000_500); - assert!( + assert_eq!( knot_maintenance::run_repo(&repo, now, &maint_opts()) .unwrap() .bitmap, + knot_maintenance::BitmapStatus::Written, "{format:?} seed packs a bitmap" ); -- 2.51.2