From 4d5608f02527af03cdf2a241034e34a80a965cb7 Mon Sep 17 00:00:00 2001 From: Ivan Liang Date: Wed, 7 Oct 2026 18:43:23 -0400 Subject: [PATCH 1/2] feat(images): add --max-size and --stale-after Adds images --max-size , the mirror of --min-size: images that would free more are evicted only once nothing else is left. Size is what removal frees, the bytes in layers no other image shares, which is also what a re-pull fetches and unpacks. With --lru, --stale-after exempts images unused for longer from both size rules, so a replaced build or retired challenge still goes by age rather than being held until nothing smaller is left. Off by default, so --min-size behaves as before unless it is set. --max-size below --min-size is refused. With --lru, the order is: stale images, then the rest, then images under --min-size, then images over --max-size, oldest first within each. Why: on the cork workers (75 GB, --threshold 89 --target 84 --min-size 100MiB --lru) eviction goes oldest first among images of 100 MiB or more, with no regard to size within that group, so a multi-GB image is as likely to go as a 100 MiB one of the same age. From cork's launch records, 2026-09-27..10-07: 684 of 90,688 launches pulled; 68 of those re-pulled the same build on the same worker after eviction and account for 67% of all pull wait. The worst are 2-3.5 GB images at 20-46 s each, while non-pulled launches are p99 279 ms and small re-pulls take ~0.5 s. Deletion cost was measured too (Docker 29.8, cold caches): 800 MB in 100 files deletes in 0.02 s, 800 MB in 100k files in 0.8-1.1 s whether as one image or ten, and a create loop got 1 iteration through the single large delete against 9-10 through the ten small ones, since the layer store lock is released between images. So --min-size's premise (fewer, larger removals are cheaper) does not hold, and small removals stall launches less. --stale-after requires --lru, whose record is the only source of when an image was last used; --max-size works on size alone without it. Like --lru, --stale-after keeps its last value when repeated, so a deployment can append it to a command that may already carry it. An image stamped 0, seeded when the record began, ages from when the record began, which the record now keeps as a first line `# began ` (1.4.0 skips it as malformed; a 1.4.0 record is taken to begin when this release first saves it). Counting a seeded image as unused forever made every image stale on the first pass after a record was created or deleted, and they tied at 0, so the largest went first. A record that would make something stale yet shows no use within --stale-after is taken to be no longer kept: that pass treats nothing as stale and warns. Bounds no pass can use (--target at or above --threshold, --max-size below --min-size) are refused at startup. A pass only logs its errors, so a bad bound used to leave the process exiting 0 while evicting nothing. Sizes are those at the start of a pass, and the README says so: an image left as the only owner of shared layers mid-pass keeps its place until the next pass. Listing images after every removal would catch it (~12 ms per list at ~300 images, measured), but on the cork workers every large shared base belongs to images already over 512 MiB, so it was left out. --- CHANGELOG.md | 7 ++ README.md | 53 ++++++++++-- src/main.rs | 26 +++++- src/reaper.rs | 176 ++++++++++++++++++++++++++++++++++------ src/tests/cli.rs | 46 ++++++++++- src/tests/containers.rs | 16 +++- src/tests/images.rs | 141 +++++++++++++++++++++++++++++++- src/usage.rs | 55 +++++++++---- 8 files changed, 462 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d574e4..eb2d447 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +- Added `images --max-size `, the mirror of `--min-size`: images that would free more are evicted only once nothing else is left. Re-pulling a large image is what keeps a launch waiting, while a small one is back in about a second; and deleting costs per file and stalls creates for the length of one removal, so several small removals are gentler than one large one. Size is what removal would free, the bytes in layers no other image shares, as of the start of the pass. Images it reports carry `, over max-size`. +- Added `images --stale-after `, which needs `--lru`: an image unused for longer is stale, held back by neither `--min-size` nor `--max-size`, and goes by age alone. Without it, `--max-size` keeps a large image nothing uses any more, such as a replaced build, until nothing smaller is left. Off by default, so `--min-size` behaves as before unless it is set. Like `--lru`, it keeps its last value when repeated. A record that shows no use at all within the duration is taken to be no longer kept: that pass treats nothing as stale and warns, rather than letting every image go by age alone. +- The image use record now starts with a `# began ` line, which releases before 1.5.0 skip. An image stamped 0, there when the record began, ages from that time for `--stale-after`, so a new or deleted record does not make every image stale at once. A record from an earlier release is taken to begin when this release first saves it. +- `images` refuses a `--target` at or above `--threshold`, or a `--max-size` below `--min-size`, at startup. Before, each pass logged the error and the process exited successfully, evicting nothing. + ## v1.4.0 - Added `images --lru `, which evicts unused images least recently used first rather than largest first. Docker records no last-used time for an image (the Engine API has none, and a pull does not set `LastTagTime`), so the order comes from a record kept by the container sweep: `containers --record-image-use ` stamps the image of every container the sweep lists as in use now. It reads the container list the sweep fetches anyway, so it adds no call to the daemon; the one exception is the run that creates the record, which lists images once and stamps every image already on the host as older than anything seen since. An image the record has never seen counts as used just now, since it most likely arrived for a launch whose container does not exist yet. Ties are broken by largest reclaimable size, as the default order does. diff --git a/README.md b/README.md index 0b7d083..8197d89 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,9 @@ Key flags for `docker-reaper images`: - `--disk-path `: Filesystem path to measure disk usage on. Defaults to the Docker daemon's root directory (`docker_root_dir`). Note: when targeting a remote daemon via `DOCKER_HOST`, `--disk-path` must be explicitly specified because disk measurement operates on local storage. - `-f, --filter `: Only reap images matching Docker Engine-supported filters (can be specified multiple times). - `--lru `: Evict least recently used first, by the record at this path (see below). -- `--min-size `: Evict images that would free less than this (e.g. `256MiB`, `1G`; binary units) only once no larger candidate is left. Deleting an image holds dockerd's image and layer store locks until its files are gone, stalling every container create and image pull on the host meanwhile, so a removal should free something worth that. Default `0`, which leaves the order alone. +- `--min-size `: Evict images that would free less than this (e.g. `256MiB`, `1G`; binary units) only once no larger candidate is left, so a pass reaches its target in fewer removals. Fewer is not necessarily gentler, since a removal costs time per file rather than per byte (see [Keeping large images](#keeping-large-images)). Default `0`, which leaves the order alone. +- `--max-size `: Evict images that would free more than this only once nothing else is left; the mirror of `--min-size` (see below). Must be at least `--min-size`. +- `--stale-after `: With `--lru`, an image unused for longer than this is stale: neither `--min-size` nor `--max-size` holds it back, so it goes by age alone. Off by default. Images are selected and evicted largest-unique-size first (reclaimable bytes not shared with other images) until disk usage drops below the target percentage. Non-forced removals skip images that gain containers mid-run. @@ -116,19 +118,60 @@ $ docker-reaper images --threshold 80 --target 70 --lru /var/lib/docker-reaper/i and that pass orders by size alone; a line in it that cannot be parsed costs only that line. With `--min-size`, size comes first: every image that would free at least that much goes before any that would not, so a large image used a minute ago, or pulled just now, - goes before a small one unused for weeks. + goes before a small one unused for weeks, unless `--stale-after` makes that one stale. -Repeating `--record-image-use`, `--lru` or `--data-root` keeps the last value, so a -deployment can append them to a command that may already carry them. +Repeating `--record-image-use`, `--lru`, `--stale-after` or `--data-root` keeps the last +value, so a deployment can append them to a command that may already carry them. Sampling is as frequent as the container sweep runs, so a container that comes and goes between two runs is not seen. The record is a text file, ` ` per -line; deleting it starts it over. +line, after a first line `# began ` saying when the record started; releases +before 1.5.0 skip that line. Deleting the record starts it over. One case it cannot see: an image removed by something other than this sweep, then pulled again before the next eviction pass, keeps the stamp it had. Noticing the removal in between would take an image list every run. After removing images by hand, delete the record. +#### Keeping large images + +Pulling an image back costs time in proportion to its size, since dockerd unpacks each +layer on one core, so a launch that needs a multi-GB image again waits tens of seconds +while a small one is back in about a second. Deleting costs per file rather than per byte +or per image, and stalls creates for one removal at a time, so several small removals are +gentler than one large one. + +`--max-size ` acts on both: images that would free more than the size go only once +every other candidate is gone. Size, here and for `--min-size`, is what removing the image +frees: the bytes in layers no other image shares (the UNIQUE SIZE of `docker system df +-v`), which is also what a re-pull would have to fetch and unpack. + +On its own it also keeps a large image that nothing uses any more, such as a replaced build +or a retired challenge, until nothing smaller is left. With `--lru`, `--stale-after +` ages those out: an image unused for longer goes by age alone, whatever its +size. The order is then: + +1. stale images, oldest first; +2. the rest, oldest first; +3. images under `--min-size`, oldest first; +4. images over `--max-size`, oldest first. + +Without `--lru` there is no age, and each group goes largest first. An image the record +has never seen counts as just used, so a large pull for a launch that has no container yet +is never stale. One stamped 0, there when the record began, ages from when it began, so a +new or deleted record makes nothing stale for `--stale-after`; a record from before 1.5.0 +is taken to begin when this release first saves it. A record that shows no use at all +within `--stale-after` is most likely no longer being kept, so that pass treats nothing as +stale and says so, rather than letting every image go by age alone. + +Sizes are those at the start of a pass. Removing one image can leave another as the only +one using layers they shared, which then would free more, and cost more to pull back, than +when the pass began; it keeps its place until the next pass. Listing images again after +every removal would catch it, at the cost of an image list per removal. + +```bash +$ docker-reaper images --threshold 89 --target 84 --lru /var/lib/docker-reaper/image-use --max-size 1G --stale-after 72h +``` + ### Orphaned containerd shim sweep Each running container has a `containerd-shim-runc-v2` process. If a container is removed diff --git a/src/main.rs b/src/main.rs index d78f640..2bd26e2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,7 +13,8 @@ use bollard::Docker; use clap::{Args, Parser, Subcommand}; use reaper::{ Filter, ReapContainersConfig, ReapImagesConfig, ReapNetworksConfig, ReapShimsConfig, - ReapVolumesConfig, reap_containers, reap_images, reap_networks, reap_shims, reap_volumes, + ReapVolumesConfig, check_image_bounds, reap_containers, reap_images, reap_networks, reap_shims, + reap_volumes, }; use std::path::PathBuf; use tokio::time::{Duration, sleep}; @@ -118,6 +119,7 @@ struct VolumesArgs { } #[derive(Debug, Args)] +#[command(after_help = "Note: values accept Go-style duration strings (e.g. 72h)")] struct ImagesArgs { /// Only reap when the measured filesystem is at least this full (percent). #[arg(long, value_name = "percent", default_value_t = 80, value_parser = parse_percent)] @@ -147,6 +149,20 @@ struct ImagesArgs { /// candidate is left. Units are binary: K, M and G mean KiB, MiB and GiB. #[arg(long, value_name = "size", value_parser = parse_size, default_value = "0")] min_size: u64, + /// Evict images that would free more than this (e.g. 1G) only once nothing else is left: + /// they are the slow ones to pull back for a launch. Units as for --min-size. + #[arg(long, value_name = "size", value_parser = parse_size)] + max_size: Option, + /// With --lru, an image unused for longer than this is stale: --min-size and --max-size + /// no longer hold it back, so it goes by age alone. + #[arg( + long, + value_name = "duration", + value_parser = parse_duration, + requires = "lru", + overrides_with = "stale_after" + )] + stale_after: Option, } #[derive(Debug, Args)] @@ -252,6 +268,12 @@ async fn main() -> Result<(), anyhow::Error> { ); } + // A pass only logs its errors, so bounds no pass can use would leave a unit evicting + // nothing while it reports success. + if let Commands::Images(ref args) = global_args.command { + check_image_bounds(args.threshold, args.target, args.min_size, args.max_size)?; + } + // Orphaned shims are found by reading this machine's process table, which says // nothing about a daemon running elsewhere. // DOCKER_CERT_PATH matters as much as DOCKER_HOST: the branch below prefers it and @@ -331,6 +353,8 @@ async fn main() -> Result<(), anyhow::Error> { filters: &args.filters, lru: args.lru.clone(), min_size: args.min_size, + max_size: args.max_size, + stale_after: args.stale_after, }; reap_images(&docker, &config).await } diff --git a/src/reaper.rs b/src/reaper.rs index 028f7a9..f925f2d 100644 --- a/src/reaper.rs +++ b/src/reaper.rs @@ -105,6 +105,11 @@ pub(crate) struct ReapImagesConfig<'a> { /// Images that would reclaim less than this many bytes are evicted only once no /// larger candidate is left. pub(crate) min_size: u64, + /// Images that would reclaim more than this many bytes are evicted only once nothing + /// else is left. + pub(crate) max_size: Option, + /// With `lru`, an image unused for longer than this is exempt from both size rules. + pub(crate) stale_after: Option, } #[derive(Debug)] @@ -358,6 +363,8 @@ pub(crate) enum ReapError { InvalidAgeBound, #[error("target must be less than threshold")] InvalidDiskBounds, + #[error("max-size must be at least min-size")] + InvalidSizeBounds, #[error("failed to measure disk usage: {0}")] DiskMeasurement(#[from] std::io::Error), #[error("failed to read the process table at {path}: {source}")] @@ -507,29 +514,31 @@ pub(crate) async fn reap_containers( /// A record that does not exist yet starts here, and every image already on the host /// predates it. Those are stamped at 0, behind anything seen in use since and behind a /// later pull not yet used, which an eviction counts as used just now. That costs one -/// image list per record, not per run. +/// image list per record, not per run. A record from before 1.5.0 does not say when it +/// began, so it is taken to begin now. async fn record_image_use( docker: &Docker, path: &Path, in_use: impl Iterator, ) -> std::io::Result<()> { - let mut record = if path.exists() { + let now = usage::now_secs(); + let (mut record, began) = if path.exists() { usage::load(path)? } else { usage::writable(path)?; - docker + let seeded = docker .list_images(None::) .await .map_err(std::io::Error::other)? .into_iter() .map(|image| (image.id, 0)) - .collect() + .collect(); + (seeded, Some(now)) }; - let now = usage::now_secs(); for id in in_use { usage::touch(&mut record, &id, now); } - usage::save(path, &record) + usage::save(path, &record, began.unwrap_or(now)) } pub(crate) async fn reap_networks( @@ -760,20 +769,82 @@ pub(crate) fn order_least_recently_used(candidates: &mut [ImageCandidate], last_ } /// Moves candidates that would reclaim less than `min_size` behind every larger one, -/// keeping the order within each group. Each removal stalls the daemon's creates and -/// pulls while it runs, so one should free something worth it; the small ones stay -/// candidates for when nothing larger is left. -pub(crate) fn defer_small(candidates: &mut [ImageCandidate], min_size: u64) { - candidates.sort_by_key(|c| c.unique_size < min_size); +/// keeping the order within each group, unless `exempt`, so a pass reaches its target in +/// fewer removals. The small ones stay candidates for when nothing larger is left. +pub(crate) fn defer_small( + candidates: &mut [ImageCandidate], + min_size: u64, + exempt: impl Fn(&ImageCandidate) -> bool, +) { + candidates.sort_by_key(|c| c.unique_size < min_size && !exempt(c)); +} + +/// Moves candidates that would reclaim more than `max_size` behind every other one, keeping +/// the order within each group, unless `exempt`. A large image is the slow one to pull back +/// for a launch, while a small one is back in about a second. +pub(crate) fn defer_large( + candidates: &mut [ImageCandidate], + max_size: u64, + exempt: impl Fn(&ImageCandidate) -> bool, +) { + candidates.sort_by_key(|c| c.unique_size > max_size && !exempt(c)); +} + +/// Whether the record last saw a candidate in use before `before`. One it has never seen +/// counts as used just now, so is never stale. One stamped 0 was last known on the host when +/// the record began, so ages from `began`, and is never stale while that is unknown. +pub(crate) fn stale( + candidate: &ImageCandidate, + last_used: &LastUsed, + began: Option, + before: u64, +) -> bool { + match last_used.get(&candidate.id) { + None => false, + Some(0) => began.is_some_and(|began| began < before), + Some(&stamp) => stamp < before, + } +} + +/// Whether the record looks no longer kept: it would make some candidate stale, yet shows no +/// use at or after `before`, a seeded image counting as used when the record began. +pub(crate) fn abandoned( + candidates: &[ImageCandidate], + last_used: &LastUsed, + began: Option, + before: u64, +) -> bool { + let used = began.is_some_and(|began| began >= before) + || last_used.values().any(|&stamp| stamp >= before); + !used + && candidates + .iter() + .any(|c| stale(c, last_used, began, before)) +} + +/// The bounds no eviction pass can work with, refused at startup as well as by each pass. +pub(crate) fn check_image_bounds( + threshold: u8, + target: u8, + min_size: u64, + max_size: Option, +) -> Result<(), ReapError> { + if target >= threshold { + return Err(ReapError::InvalidDiskBounds); + } + if max_size.is_some_and(|max| max < min_size) { + return Err(ReapError::InvalidSizeBounds); + } + Ok(()) } /// The record an eviction pass read, for reporting: none when it could not be read, since /// every image in the stand-in counts as used just now and saying so would mislead. -fn known_use(last_used: &Option<(LastUsed, bool)>) -> Option<&LastUsed> { +fn known_use(last_used: &Option<(LastUsed, Option, bool)>) -> Option<&LastUsed> { last_used .as_ref() - .filter(|(_, read)| *read) - .map(|(record, _)| record) + .filter(|(_, _, read)| *read) + .map(|(record, _, _)| record) } /// An image's size, and when it was last used if the record says. @@ -838,9 +909,12 @@ pub(crate) async fn reap_images( docker: &Docker, config: &ReapImagesConfig<'_>, ) -> Result, ReapError> { - if config.target >= config.threshold { - return Err(ReapError::InvalidDiskBounds); - } + check_image_bounds( + config.threshold, + config.target, + config.min_size, + config.max_size, + )?; // Resolve the filesystem to measure: an explicit path, or the daemon's // image store. The latter is only meaningful when the daemon is local — @@ -901,27 +975,75 @@ pub(crate) async fn reap_images( let mut candidates = plan_image_evictions(&images, &in_use); let now = usage::now_secs(); - // With --lru, the record and whether it may be written back: one that could - // not be read is left as it is rather than replaced by a blank one. + // With --lru, the record, when it began, and whether it may be written back: one that + // could not be read is left as it is rather than replaced by a blank one. let mut last_used = config .lru .as_ref() .map(|path| match usage::load(path) { - Ok(record) => (record, true), + Ok((record, began)) => (record, began, true), Err(e) => { warn!( "Failed to read the image use record at {}: {}; every image counts as just used, and the record is left as it is", path.display(), e ); - (LastUsed::new(), false) + (LastUsed::new(), None, false) } }); - if let Some((ref mut record, _)) = last_used { + // --stale-after is refused without --lru, so the record is there whenever it is set. + // Judged before this pass stamps anything: a record that shows no use in that long is + // most likely no longer kept, and would make every image stale. + let stale_before = match (config.stale_after, &last_used) { + (Some(after), Some((record, began, true))) => { + let before = now.saturating_sub(after.as_secs()); + if !abandoned(&candidates, record, *began, before) { + Some(before) + } else { + warn!( + "The image use record shows no use within {}, so no image counts as stale on this pass; is the containers sweep still keeping it?", + format_age(after.as_secs()) + ); + None + } + } + _ => None, + }; + if let Some((ref mut record, _, _)) = last_used { stamp_for_eviction(record, &in_use, &images, now); order_least_recently_used(&mut candidates, record); } - defer_small(&mut candidates, config.min_size); + let is_stale = |candidate: &ImageCandidate| match (stale_before, &last_used) { + (Some(before), Some((record, began, _))) => stale(candidate, record, *began, before), + _ => false, + }; + defer_small(&mut candidates, config.min_size, is_stale); + if let Some(max_size) = config.max_size { + defer_large(&mut candidates, max_size, is_stale); + let kept = candidates + .iter() + .filter(|c| c.unique_size > max_size && !is_stale(c)) + .count(); + if kept > 0 { + info!( + "{kept} of {} candidates would free more than {}; they go only once nothing else is left", + candidates.len(), + format_size(max_size) + ); + } + } + let details = |candidate: &ImageCandidate| { + let mut details = describe_image(candidate, known_use(&last_used), now); + if is_stale(candidate) { + details.push_str(", stale"); + } else if config + .max_size + .is_some_and(|max| candidate.unique_size > max) + { + details.push_str(", over max-size"); + } + details + }; let target_bytes = (config.target as u64) * (capacity / 100); if config.dry_run { @@ -939,7 +1061,7 @@ pub(crate) async fn reap_images( }; Resource { resource_type: ResourceType::Image, - details: describe_image(&candidate, known_use(&last_used), now), + details: details(&candidate), id: candidate.id, name: candidate.name, status, @@ -964,7 +1086,7 @@ pub(crate) async fn reap_images( } let mut resource = Resource { resource_type: ResourceType::Image, - details: describe_image(&candidate, known_use(&last_used), now), + details: details(&candidate), id: candidate.id, name: candidate.name, status: RemovalStatus::Eligible, @@ -973,14 +1095,14 @@ pub(crate) async fn reap_images( results.push(resource); } - if let (Some(path), Some((mut record, true))) = (&config.lru, last_used) { + if let (Some(path), Some((mut record, began, true))) = (&config.lru, last_used) { let removed: HashSet<&str> = results .iter() .filter(|r| matches!(r.status, RemovalStatus::Success)) .map(|r| r.id.as_str()) .collect(); settle_record(&mut record, &images, &removed, config.filters.is_empty()); - if let Err(e) = usage::save(path, &record) { + if let Err(e) = usage::save(path, &record, began.unwrap_or(now)) { warn!( "Failed to save the image use record at {}: {}", path.display(), diff --git a/src/tests/cli.rs b/src/tests/cli.rs index 532e614..98f6fe4 100644 --- a/src/tests/cli.rs +++ b/src/tests/cli.rs @@ -27,11 +27,25 @@ fn a_repeated_path_flag_keeps_the_last_value() { }; assert_eq!(args.record_image_use, Some(PathBuf::from("/b"))); - let Commands::Images(args) = parse(&["docker-reaper", "images", "--lru", "/a", "--lru", "/b"]) - else { + let Commands::Images(args) = parse(&[ + "docker-reaper", + "images", + "--lru", + "/a", + "--stale-after", + "1h", + "--lru", + "/b", + "--stale-after", + "72h", + ]) else { panic!("not the images subcommand"); }; assert_eq!(args.lru, Some(PathBuf::from("/b"))); + assert_eq!( + args.stale_after, + Some(std::time::Duration::from_secs(72 * 3600)) + ); let Commands::Shims(args) = parse(&[ "docker-reaper", @@ -62,6 +76,34 @@ fn filters_still_accumulate() { assert_eq!(args.filters.len(), 2); } +/// Staleness is decided by when an image was last used, which only the --lru record knows; +/// the size rule needs no record. +#[test] +fn staleness_needs_the_use_record_and_max_size_does_not() { + assert!(Cli::try_parse_from(["docker-reaper", "images", "--stale-after", "72h"]).is_err()); + + let Commands::Images(args) = parse(&["docker-reaper", "images", "--max-size", "1G"]) else { + panic!("not the images subcommand"); + }; + assert_eq!(args.max_size, Some(1 << 30)); + assert_eq!(args.stale_after, None); + + let Commands::Images(args) = parse(&[ + "docker-reaper", + "images", + "--lru", + "/a", + "--stale-after", + "72h", + ]) else { + panic!("not the images subcommand"); + }; + assert_eq!( + args.stale_after, + Some(std::time::Duration::from_secs(72 * 3600)) + ); +} + #[test] fn sizes_parse_with_binary_units() { for (text, bytes) in [ diff --git a/src/tests/containers.rs b/src/tests/containers.rs index 56734be..2c51d53 100644 --- a/src/tests/containers.rs +++ b/src/tests/containers.rs @@ -229,7 +229,12 @@ async fn record_image_use() { reap_containers(docker_client(), &config(false)) .await .expect("failed to reap containers"); - let record = crate::usage::load(&path).expect("failed to read the record"); + let (record, began) = crate::usage::load(&path).expect("failed to read the record"); + let began = began.expect("a new record says when it began"); + assert!( + began >= before, + "a new record began at {began}, before the run at {before}" + ); let stamp = *record .get(&image_id) .expect("the container's image was not recorded"); @@ -257,11 +262,16 @@ async fn record_image_use() { let mut edited = record.clone(); edited.insert("sha256:gone".to_string(), 5); - crate::usage::save(&path, &edited).unwrap(); + crate::usage::save(&path, &edited, began).unwrap(); reap_containers(docker_client(), &config(false)) .await .expect("failed to reap containers"); - let record = crate::usage::load(&path).expect("failed to read the record"); + let (record, still_began) = crate::usage::load(&path).expect("failed to read the record"); + assert_eq!( + still_began, + Some(began), + "a later run keeps when the record began" + ); assert_eq!( record.get("sha256:gone"), Some(&5), diff --git a/src/tests/images.rs b/src/tests/images.rs index a25b58e..c7f8bec 100644 --- a/src/tests/images.rs +++ b/src/tests/images.rs @@ -8,8 +8,8 @@ //! and tested here. The one live test filters to no image at all. use crate::reaper::{ - ImageCandidate, defer_small, describe_image, order_least_recently_used, plan_image_evictions, - settle_record, stamp_for_eviction, + ImageCandidate, abandoned, check_image_bounds, defer_large, defer_small, describe_image, + order_least_recently_used, plan_image_evictions, settle_record, stale, stamp_for_eviction, }; use crate::usage::LastUsed; use bollard::models::ImageSummary; @@ -229,7 +229,7 @@ fn small_images_wait_until_nothing_larger_is_left() { ("sha256:big-new".to_string(), 4), ]), ); - defer_small(&mut plan, 100); + defer_small(&mut plan, 100, |_| false); assert_eq!( ids(&plan), vec![ @@ -242,10 +242,141 @@ fn small_images_wait_until_nothing_larger_is_left() { ); let before = ids(&plan).iter().map(|s| s.to_string()).collect::>(); - defer_small(&mut plan, 0); + defer_small(&mut plan, 0, |_| false); assert_eq!(ids(&plan), before, "0 leaves the order alone"); } +#[test] +fn large_images_wait_until_nothing_else_is_left() { + // No record: the size rule alone, over the default largest-first order. + let images = vec![ + image("sha256:small", Some("small:1"), 50, 0), + image("sha256:huge", Some("huge:1"), 5000, 0), + image("sha256:mid", Some("mid:1"), 300, 0), + image("sha256:big", Some("big:1"), 2000, 0), + ]; + let mut plan = plan_image_evictions(&images, &HashSet::new()); + defer_large(&mut plan, 1000, |_| false); + assert_eq!( + ids(&plan), + vec!["sha256:mid", "sha256:small", "sha256:huge", "sha256:big"] + ); +} + +#[test] +fn stale_images_go_by_age_alone() { + let images = vec![ + image("sha256:big-recent", Some("big-recent:1"), 5000, 0), + image("sha256:big-stale", Some("big-stale:1"), 4000, 0), + image("sha256:mid-fresh", Some("mid-fresh:1"), 300, 0), + image("sha256:small-stale", Some("small-stale:1"), 50, 0), + image("sha256:small-fresh", Some("small-fresh:1"), 40, 0), + ]; + let mut plan = plan_image_evictions(&images, &HashSet::new()); + let record = LastUsed::from([ + ("sha256:small-stale".to_string(), 100), + ("sha256:big-stale".to_string(), 200), + ("sha256:mid-fresh".to_string(), 700), + ("sha256:small-fresh".to_string(), 800), + ("sha256:big-recent".to_string(), 900), + ]); + order_least_recently_used(&mut plan, &record); + let is_stale = |c: &ImageCandidate| stale(c, &record, None, 500); + defer_small(&mut plan, 100, is_stale); + defer_large(&mut plan, 1000, is_stale); + assert_eq!( + ids(&plan), + vec![ + "sha256:small-stale", + "sha256:big-stale", + "sha256:mid-fresh", + "sha256:small-fresh", + "sha256:big-recent" + ], + "stale first by age, then the rest, then fresh small ones, then fresh large ones" + ); +} + +#[test] +fn staleness_needs_a_use_before_the_cutoff() { + let candidate = |id: &str| ImageCandidate { + id: id.to_string(), + name: id.to_string(), + unique_size: 1, + }; + let record = LastUsed::from([ + ("sha256:at-cutoff".to_string(), 500), + ("sha256:before".to_string(), 499), + ("sha256:seeded".to_string(), 0), + ]); + let is_stale = |id, began| stale(&candidate(id), &record, began, 500); + assert!(!is_stale("sha256:at-cutoff", Some(1)), "used at the cutoff"); + assert!(is_stale("sha256:before", Some(1)), "used before the cutoff"); + assert!( + !is_stale("sha256:unseen", Some(1)), + "unseen counts as just pulled" + ); + assert!( + is_stale("sha256:seeded", Some(499)), + "seeded in a record that began before the cutoff" + ); + assert!( + !is_stale("sha256:seeded", Some(500)), + "seeded in a record that began at the cutoff: a new record makes nothing stale" + ); + assert!( + !is_stale("sha256:seeded", None), + "seeded in a record that does not say when it began" + ); +} + +#[test] +fn a_record_with_no_recent_use_looks_abandoned() { + let candidate = |id: &str| ImageCandidate { + id: id.to_string(), + name: id.to_string(), + unique_size: 1, + }; + let candidates = vec![candidate("sha256:old"), candidate("sha256:seeded")]; + let record = LastUsed::from([ + ("sha256:old".to_string(), 100), + ("sha256:seeded".to_string(), 0), + ]); + assert!( + abandoned(&candidates, &record, Some(50), 500), + "nothing since 500" + ); + assert!( + abandoned(&candidates, &record, None, 500), + "nothing since 500, start unknown" + ); + assert!( + !abandoned(&candidates, &record, Some(600), 500), + "began since 500" + ); + let mut touched = record.clone(); + touched.insert("sha256:new".to_string(), 500); + assert!(!abandoned(&candidates, &touched, None, 500), "used at 500"); + + // Only seeded entries and no start: nothing could be stale, so nothing to warn about. + let seeded = LastUsed::from([("sha256:seeded".to_string(), 0)]); + assert!(!abandoned(&candidates[1..], &seeded, None, 500)); +} + +#[test] +fn bounds_no_pass_can_use_are_refused() { + assert!(check_image_bounds(89, 84, 0, None).is_ok()); + assert!(check_image_bounds(89, 84, 100, Some(100)).is_ok()); + assert!( + check_image_bounds(84, 84, 0, None).is_err(), + "target at threshold" + ); + assert!( + check_image_bounds(89, 84, 200, Some(100)).is_err(), + "max-size below min-size" + ); +} + /// An eviction pass rewrites a record it read, and leaves one it could not read exactly as /// it was: replacing it would wipe the history it holds. Against the live daemon, but safe: /// the threshold forces a pass, and the filter matches no image, so nothing is removed. @@ -279,6 +410,8 @@ async fn an_unreadable_record_is_left_as_it_is() { filters: &filters, lru: Some(path.clone()), min_size: 0, + max_size: None, + stale_after: None, }; let sweep = || async { reap_images(super::common::docker_client(), &config) diff --git a/src/usage.rs b/src/usage.rs index 26aa51f..ae0fc26 100644 --- a/src/usage.rs +++ b/src/usage.rs @@ -5,6 +5,9 @@ //! One line per image, ` `, replaced whole by a rename so a reader //! never sees a torn file. There is no lock: writers only move stamps forward or drop //! images that are gone, and a change lost to a concurrent write is made again later. +//! +//! A first line `# began ` says when the record started, which is when the +//! images stamped 0 were last known to be there. Readers before 1.5.0 skip it as malformed. use std::collections::HashMap; use std::fs; @@ -22,18 +25,25 @@ pub(crate) fn now_secs() -> u64 { .unwrap_or(0) } -/// Reads the record. A missing file is an empty record, and a malformed line is skipped, -/// including one that is not UTF-8: a record that cannot be read is never rewritten, so -/// one bad byte refusing the whole file would turn LRU off for good. -pub(crate) fn load(path: &Path) -> io::Result { +const BEGAN: &str = "# began "; + +/// Reads the record, and when it began if it says. A missing file is an empty record, and a +/// malformed line is skipped, including one that is not UTF-8: a record that cannot be read +/// is never rewritten, so one bad byte refusing the whole file would turn LRU off for good. +pub(crate) fn load(path: &Path) -> io::Result<(LastUsed, Option)> { let bytes = match fs::read(path) { Ok(bytes) => bytes, - Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(LastUsed::new()), + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok((LastUsed::new(), None)), Err(e) => return Err(e), }; let text = String::from_utf8_lossy(&bytes); let mut record = LastUsed::new(); + let mut began = None; for line in text.lines() { + if let Some(secs) = line.strip_prefix(BEGAN) { + began = secs.parse().ok().or(began); + continue; + } let Some((secs, id)) = line.split_once(' ') else { continue; }; @@ -44,7 +54,7 @@ pub(crate) fn load(path: &Path) -> io::Result { touch(&mut record, id, secs); } } - Ok(record) + Ok((record, began)) } /// Moves an image's stamp forward to `secs`, never back. @@ -53,11 +63,12 @@ pub(crate) fn touch(record: &mut LastUsed, id: &str, secs: u64) { *stamp = (*stamp).max(secs); } -/// Replaces the record on disk. A failed save leaves no temporary file behind. -pub(crate) fn save(path: &Path, record: &LastUsed) -> io::Result<()> { +/// Replaces the record on disk, saying it began at `began`. A failed save leaves no +/// temporary file behind. +pub(crate) fn save(path: &Path, record: &LastUsed, began: u64) -> io::Result<()> { let mut entries: Vec<(&String, &u64)> = record.iter().collect(); entries.sort(); - let mut text = String::new(); + let mut text = format!("{BEGAN}{began}\n"); for (id, secs) in entries { text.push_str(&format!("{secs} {id}\n")); } @@ -109,7 +120,7 @@ mod tests { #[test] fn a_missing_record_is_empty() { let path = fixture("missing"); - assert!(load(&path).unwrap().is_empty()); + assert_eq!(load(&path).unwrap(), (LastUsed::new(), None)); } #[test] @@ -119,8 +130,8 @@ mod tests { ("sha256:aaa".to_string(), 100), ("sha256:bbb".to_string(), 200), ]); - save(&path, &record).unwrap(); - assert_eq!(load(&path).unwrap(), record); + save(&path, &record, 50).unwrap(); + assert_eq!(load(&path).unwrap(), (record, Some(50))); let names: Vec<_> = fs::read_dir(path.parent().unwrap()) .unwrap() .map(|e| e.unwrap().file_name()) @@ -136,7 +147,7 @@ mod tests { "100 sha256:aaa\nnot-a-number sha256:bbb\nno-space\n200 \n\n300 sha256:ccc\n", ) .unwrap(); - let record = load(&path).unwrap(); + let (record, _) = load(&path).unwrap(); assert_eq!( record, LastUsed::from([ @@ -161,7 +172,7 @@ mod tests { let path = fixture("failed-save"); // A non-empty directory where the record should be: the rename onto it fails. fs::create_dir_all(path.join("occupied")).unwrap(); - assert!(save(&path, &LastUsed::from([("sha256:aaa".to_string(), 1)])).is_err()); + assert!(save(&path, &LastUsed::from([("sha256:aaa".to_string(), 1)]), 1).is_err()); let names: Vec<_> = fs::read_dir(path.parent().unwrap()) .unwrap() .map(|e| e.unwrap().file_name()) @@ -185,7 +196,7 @@ mod tests { b"100 sha256:aaa\n\xff\xfe 5 junk\n200 sha256:b\xffb\n300 sha256:ccc\n", ) .unwrap(); - let record = load(&path).unwrap(); + let (record, _) = load(&path).unwrap(); assert_eq!(record.get("sha256:aaa"), Some(&100)); assert_eq!(record.get("sha256:ccc"), Some(&300)); assert_eq!( @@ -195,10 +206,22 @@ mod tests { ); } + #[test] + fn when_the_record_began_is_read_and_never_an_entry() { + let path = fixture("began"); + fs::write(&path, "100 sha256:aaa\n").unwrap(); + assert_eq!(load(&path).unwrap().1, None, "a 1.4.0 record does not say"); + + fs::write(&path, "# began 42\n0 sha256:aaa\n# began junk\n").unwrap(); + let (record, began) = load(&path).unwrap(); + assert_eq!(began, Some(42), "a malformed began line costs only itself"); + assert_eq!(record, LastUsed::from([("sha256:aaa".to_string(), 0)])); + } + #[test] fn a_duplicated_line_keeps_the_later_stamp() { let path = fixture("duplicate"); fs::write(&path, "300 sha256:aaa\n100 sha256:aaa\n").unwrap(); - assert_eq!(load(&path).unwrap()["sha256:aaa"], 300); + assert_eq!(load(&path).unwrap().0["sha256:aaa"], 300); } } From 2a5baea847fa25a036cd87682b9d01243deaf813 Mon Sep 17 00:00:00 2001 From: Ivan Liang Date: Wed, 7 Oct 2026 19:57:33 -0400 Subject: [PATCH 2/2] release: v1.5.0 --- CHANGELOG.md | 2 +- Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb2d447..3284c2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## v1.5.0 - Added `images --max-size `, the mirror of `--min-size`: images that would free more are evicted only once nothing else is left. Re-pulling a large image is what keeps a launch waiting, while a small one is back in about a second; and deleting costs per file and stalls creates for the length of one removal, so several small removals are gentler than one large one. Size is what removal would free, the bytes in layers no other image shares, as of the start of the pass. Images it reports carry `, over max-size`. - Added `images --stale-after `, which needs `--lru`: an image unused for longer is stale, held back by neither `--min-size` nor `--max-size`, and goes by age alone. Without it, `--max-size` keeps a large image nothing uses any more, such as a replaced build, until nothing smaller is left. Off by default, so `--min-size` behaves as before unless it is set. Like `--lru`, it keeps its last value when repeated. A record that shows no use at all within the duration is taken to be no longer kept: that pass treats nothing as stale and warns, rather than letting every image go by age alone. diff --git a/Cargo.lock b/Cargo.lock index 5776a66..bb7b508 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -252,7 +252,7 @@ dependencies = [ [[package]] name = "docker-reaper" -version = "1.4.0" +version = "1.5.0" dependencies = [ "anyhow", "bollard", diff --git a/Cargo.toml b/Cargo.toml index 67b9564..9d1e8af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ edition = "2024" license = "MIT OR Apache-2.0" name = "docker-reaper" repository = "https://github.com/picoCTF/docker-reaper" -version = "1.4.0" +version = "1.5.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html