diff --git a/.env.example b/.env.example index 5fffd0a..b587787 100644 --- a/.env.example +++ b/.env.example @@ -13,15 +13,16 @@ DOMAIN=my-event.example.com # Always an immutable tag, never `latest`: rollback is `EVENTSNAP_VERSION=` # + `docker compose up -d`, which works offline if that image is still resident locally. # -# ⚠ THIS TAG DOES NOT EXIST YET. The newest git tag is v0.12.0; v0.13.0 is the release you -# cut for the event. Build and push it (plus its identical rollback twin v0.13.0-a) BEFORE +# ⚠ THIS TAG DOES NOT EXIST YET. The newest git tag is v0.17.4; v0.17.5 is the release you +# cut for the event. Build and push it (plus its identical rollback twin v0.17.5-a) BEFORE # the first `docker compose up -d` — see DEPLOYMENT_RUNBOOK.md §6 (build) and §9 (rollback). # Copying this file and starting the stack without that step fails with `manifest unknown`. # -# Do NOT "fix" this by dropping back to v0.12.0: no image was ever built for it, and a -# 6-migration tree booting against a 31-migration database returns VersionMissing and -# crash-loops forever behind a live Caddy. §9 covers this in full. -EVENTSNAP_VERSION=v0.13.0 +# Do NOT "fix" this by dropping back to an older release tag. v0.17.4 carries 16 migrations +# against a database that will hold 31, so it returns VersionMissing and crash-loops forever +# behind a live Caddy — and unlike an unbuilt tag, an image for it may well PULL cleanly, so a +# successful `docker pull` is not evidence that the tag is safe. §9 covers this in full. +EVENTSNAP_VERSION=v0.17.5 # ── App server ──────────────────────────────────────────────────────────────── APP_PORT=3000 diff --git a/DEPLOYMENT_RUNBOOK.md b/DEPLOYMENT_RUNBOOK.md index 666e19f..851b935 100644 --- a/DEPLOYMENT_RUNBOOK.md +++ b/DEPLOYMENT_RUNBOOK.md @@ -180,7 +180,7 @@ EVENT_NAME=<...> EVENT_SLUG=<...> # ── Image version (NEW — drives the image: tags in docker-compose.yml) ──── -EVENTSNAP_VERSION=v0.13.0 +EVENTSNAP_VERSION=v0.17.5 # ── Secrets — ALL of them, before the first `up -d` ─────────────────────── JWT_SECRET= @@ -421,8 +421,13 @@ the host). docker run --rm --platform linux/amd64 alpine uname -m # must print x86_64 cd /Users/fabianhammprivat/Projects/EventSnap -VERSION=v0.13.0 # latest existing tag is v0.12.0 — see §9 before reusing it -ROLLBACK=v0.13.0-a # the SAME source, tagged twice; §9 explains why +# Newest EXISTING tag is v0.17.4 (16 migrations); HEAD has 31, so v0.17.5 is the release you cut +# for the event. Confirm before building — `git tag --list 'v*' --sort=-v:refname | head -1`, and +# keep the `--list 'v*'` filter: an unfiltered listing puts the archive/ tag first. Never reuse an +# existing tag: the image name would then disagree with the tree of the same name, and any host +# still pinned to that tag silently swaps builds on its next pull. +VERSION=v0.17.5 +ROLLBACK=v0.17.5-a # the SAME source, tagged twice; §9 explains why SHA=$(git rev-parse --short HEAD) docker buildx create --name eventsnap --use 2>/dev/null || docker buildx use eventsnap @@ -572,25 +577,44 @@ Then, from a phone on cellular (not the office wifi): ## 9. Rollback -### There is no older image you can roll back to. Build the rollback target yourself. +### No released tag is a valid rollback target. Build the rollback target yourself. + + + Read this before the event, not during it. The obvious move — drop `EVENTSNAP_VERSION` back to the previous released tag — **takes the app down permanently** and looks like a crash loop with no explanation: ``` -$ git ls-tree --name-only v0.12.0 backend/migrations/ | wc -l -12 # 6 migrations. HEAD has 31. -$ git rev-list --count v0.12.0..HEAD -217 +# `--list 'v*'` is load-bearing: the repo also carries non-release tags +# (archive/pre-squash-20260729), and an unfiltered listing puts one of them first. +$ git tag --list 'v*' --sort=-v:refname | head -1 +v0.17.4 +$ git ls-tree --name-only v0.17.4 backend/migrations/ | grep -c up.sql +16 # 16 migrations. HEAD has 31. +$ git rev-list --count v0.17.4..HEAD +82 ``` -`db.rs` runs `sqlx::migrate!()` with no `set_ignore_missing`, so an image built from a 6-migration -tree, booting against a database that already carries versions 007–031, returns `VersionMissing`. +`db.rs` runs `sqlx::migrate!()` with no `set_ignore_missing`, so an image built from a 16-migration +tree, booting against a database that already carries versions 017–031, returns `VersionMissing`. `create_pool` errors, `main` exits 1, and `restart: unless-stopped` restarts it forever — with Caddy still routing traffic to it. (`014_export_epoch.up.sql` documents this failure mode; §0 restates it.) -No `v0.12.0` image was ever built or pushed either, so the pre-pull would fail with -`manifest unknown` before you ever got that far. + +**Do not treat a successful `docker pull` as evidence that a tag is safe.** This section used to +claim the previous release had never been built or pushed, so "the pre-pull would fail with +`manifest unknown` before you ever got that far" — i.e. that the registry itself would stop you. +Do not rely on that. Images for the earlier v0.17.x releases may well be resident in the registry +(and, after a §9 pre-pull, on the server), in which case the rollback pulls cleanly, starts, and +crash-loops on `VersionMissing` — the failure this section exists to prevent, reached through the +one door it used to say was closed. The migration set is the only thing that decides whether a tag +is safe, and it is checked by `git ls-tree`, never by the registry. + +Every existing tag is older than HEAD's migration set, so **no released tag is a valid rollback +target.** That is what the twin-tag scheme below is for. **So: at build time, tag the SAME frozen commit twice.** Two identical images, two names. The rollback then swaps to a binary that is bit-for-bit what you tested and carries the identical @@ -598,8 +622,8 @@ migration set, which makes it a genuine no-op rather than a gamble: ```bash # In §6, push both tags from the one build: -VERSION=v0.13.0 -ROLLBACK=v0.13.0-a # same source, different name — the rollback target +VERSION=v0.17.5 +ROLLBACK=v0.17.5-a # same source, different name — the rollback target docker buildx build --platform linux/amd64 \ -t registry.mc02.dev/eventsnap/app:$VERSION \ @@ -611,17 +635,17 @@ docker buildx build --platform linux/amd64 \ **Rolling back — ~30 seconds, no network:** ```bash -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.0-a/' .env +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5-a/' .env docker compose up -d app frontend ``` This only works offline if both are already resident. **Pre-pull all four at T‑2:** ```bash -docker pull registry.mc02.dev/eventsnap/app:v0.13.0 -docker pull registry.mc02.dev/eventsnap/frontend:v0.13.0 -docker pull registry.mc02.dev/eventsnap/app:v0.13.0-a -docker pull registry.mc02.dev/eventsnap/frontend:v0.13.0-a +docker pull registry.mc02.dev/eventsnap/app:v0.17.5 +docker pull registry.mc02.dev/eventsnap/frontend:v0.17.5 +docker pull registry.mc02.dev/eventsnap/app:v0.17.5-a +docker pull registry.mc02.dev/eventsnap/frontend:v0.17.5-a docker image ls | grep eventsnap # confirm all four ``` @@ -668,8 +692,8 @@ harness — `backend/scripts/rehearse-014.sh`. Run it once against a real dump b **Registry-down transport fallback** (layers are already compressed — do not add gzip): ```bash -docker save registry.mc02.dev/eventsnap/app:v0.13.0 \ - registry.mc02.dev/eventsnap/frontend:v0.13.0 | ssh root@SERVER 'docker load' +docker save registry.mc02.dev/eventsnap/app:v0.17.5 \ + registry.mc02.dev/eventsnap/frontend:v0.17.5 | ssh root@SERVER 'docker load' ``` --- @@ -943,7 +967,7 @@ docker compose restart app # Roll back to the identically-built sibling image — see §9 for what this can and cannot fix. # Do NOT substitute an older release tag here; it will crash-loop on the migration set. -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.0-a/' .env && docker compose up -d app frontend +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5-a/' .env && docker compose up -d app frontend # Disk check df -h /var/lib/docker diff --git a/README.md b/README.md index b065779..adb8864 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ Caddy automatically obtains a Let's Encrypt certificate on first start. The app # ── On your workstation: build and push the new tag ─────────────────────────── # Push the rollback twin at the same time, from the same source — see # DEPLOYMENT_RUNBOOK.md §9 for why an identical second tag is the rollback target. -VERSION=v0.13.1 +VERSION=v0.17.5 docker buildx build --platform linux/amd64 \ -t registry.mc02.dev/eventsnap/app:$VERSION \ -t registry.mc02.dev/eventsnap/app:$VERSION-a --push ./backend @@ -179,7 +179,7 @@ cd /path/to/eventsnap git pull # 3. Point the stack at the new tag. -sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.13.1/' .env +sed -i 's/^EVENTSNAP_VERSION=.*/EVENTSNAP_VERSION=v0.17.5/' .env # 4. Pull explicitly, BEFORE restarting. A failure here (bad tag, registry down) leaves the # running stack untouched; letting `up -d` discover it takes the app down first. diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index 5faa338..026de3c 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -115,9 +115,15 @@ pub async fn patch_config( // accept but that silently revert to the hardcoded default at read time // (get_usize/get_i64 can't parse negatives/NaN/fractionals). `compression_concurrency` // is intentionally absent — it's read once at boot, so a live edit was a no-op. + // The two size limits are bounded by what the router's `DefaultBodyLimit` can carry, NOT by + // a round number. They used to allow 1024 and 10240 MB against a 576 MiB body cap, so the + // admin dashboard's plain "Max. Videogröße (MB)" field could be set to a value that destroys + // every video above the cap — the limit trips mid-body, surfaces as a 400, and the client + // purges the blob as terminal. See `crate::MAX_CONFIGURABLE_UPLOAD_MB` for the full chain. + const MAX_SIZE_MB: f64 = crate::MAX_CONFIGURABLE_UPLOAD_MB as f64; const NUMERIC_SPECS: &[(&str, bool, f64, f64)] = &[ - ("max_image_size_mb", true, 1.0, 1024.0), - ("max_video_size_mb", true, 1.0, 10240.0), + ("max_image_size_mb", true, 1.0, MAX_SIZE_MB), + ("max_video_size_mb", true, 1.0, MAX_SIZE_MB), ("upload_rate_per_hour", true, 1.0, 100_000.0), ("feed_rate_per_min", true, 1.0, 100_000.0), ("export_rate_per_day", true, 1.0, 100_000.0), diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index a9798b5..14aea2a 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -25,10 +25,15 @@ const MAX_CAPTION_LENGTH: usize = 2000; /// `MAX_CAPTION_LENGTH` check only ran afterwards, on a string that had already been built. /// 4 bytes per code point is the worst case for UTF-8, so this can never reject a caption the /// character limit would have accepted. -const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4; +pub(crate) const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4; /// Byte ceiling for the raw hashtag CSV. Generous next to what the tag caps below allow. -const MAX_HASHTAGS_BYTES: usize = 4 * 1024; +pub(crate) const MAX_HASHTAGS_BYTES: usize = 4 * 1024; + +/// Byte ceiling for the `client_upload_id` field. 64 bytes fits a hyphenated UUID (36) with +/// room to spare; named rather than inline so the multipart-envelope test in `main.rs` can +/// account for every text field this handler will read. +pub(crate) const MAX_CLIENT_UPLOAD_ID_BYTES: usize = 64; /// Hashtags stored per upload. The CSV was never length-checked at all and was split into an /// unbounded `Vec`, then upserted TAG BY TAG inside the commit transaction — which holds a @@ -280,9 +285,24 @@ pub async fn upload( return Err(AppError::UploadsLocked("Uploads sind gesperrt.".into())); } - // Read config limits from DB - let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20).await; - let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500).await; + // Read config limits from DB. + // + // CLAMPED to what the router's `DefaultBodyLimit` can actually carry. `patch_config` now + // refuses a larger value, but that guard only covers values written THROUGH it: a config row + // stored before the bound existed, or edited straight into the table, would otherwise sail + // past it and hand the guest the worst failure in the app — the body limit tripping mid-upload, + // surfacing as a 400, and the client purging the blob as terminal. See + // `crate::MAX_CONFIGURABLE_UPLOAD_MB`. + // + // Clamping (rather than refusing the upload) is right here: the operator's intent was "allow + // bigger files", and the honest answer to an unsatisfiable limit is the largest one that + // works, applied consistently by both the streaming cap and the per-class check below. + let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20) + .await + .min(crate::MAX_CONFIGURABLE_UPLOAD_MB); + let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500) + .await + .min(crate::MAX_CONFIGURABLE_UPLOAD_MB); // The uploaded file is streamed straight to a temp file on disk (never buffered // whole in memory — a 500 MB video used to cost 500 MB of RAM per concurrent @@ -379,8 +399,8 @@ pub async fn upload( // `hashtags` were bounded by the helper for exactly this reason; this field // arrived later (migration 022) and missed it. // - // 64 bytes fits a hyphenated UUID (36) with room to spare. - let raw = read_text_field_bounded(field, 64).await?; + // See `MAX_CLIENT_UPLOAD_ID_BYTES`. + let raw = read_text_field_bounded(field, MAX_CLIENT_UPLOAD_ID_BYTES).await?; // A malformed key is not worth rejecting an upload over — the photo is the // thing the guest cares about. Drop the key and lose only the retry // protection, which is exactly where we were before it existed. diff --git a/backend/src/main.rs b/backend/src/main.rs index 1d4cd74..4f28926 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -19,7 +19,36 @@ use state::AppState; /// Hard HTTP body cap for the upload endpoint (576 MiB). Backstop against /// memory-exhaustion; precise per-class size limits are enforced in the handler. -const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024; +pub(crate) const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024; + +/// Largest per-file size limit (MB) an operator may configure for `max_image_size_mb` / +/// `max_video_size_mb`, derived from [`MAX_UPLOAD_BYTES`] rather than written down twice. +/// +/// This used to be enforced by a COMMENT — "if an admin raises max_video_size_mb above this, +/// bump MAX_UPLOAD_BYTES" — while `patch_config` happily accepted 10240 and the admin dashboard +/// rendered "Max. Videogröße (MB)" as a bare number field. Set it to 1000 and every video +/// between 576 MB and the new limit is destroyed, in a shape that is much worse than a refusal: +/// +/// * The body limit trips mid-upload, inside `field.chunk()`, so `stream_field_to_file` maps +/// it to `AppError::BadRequest` — a 400, not a 413 with a `quota_exceeded` code. +/// * `classifyUploadStatus` (frontend/src/lib/upload-queue.ts) puts every non-401/408/429 4xx +/// in the `terminal` bucket, and `isReversibleLock(400, 'bad_request')` is false — so the +/// queue DELETES the blob from IndexedDB and moves the row to `blocked`, which by design +/// offers no retry button. +/// * All of that after the guest has already pushed 600 MB over cellular, and the message they +/// get names a read failure rather than a limit. +/// +/// So the ceiling is enforced where the value is SET (`patch_config`) and again where it is READ +/// (`handlers::upload`), because a value stored before this bound existed — or written by hand +/// into the `config` table — would otherwise walk straight past the first check. +/// +/// The subtraction is the multipart envelope: the body carries the file PLUS the boundary +/// framing and the `caption` / `hashtags` / `client_upload_id` fields. 1 MiB is enormously more +/// than those can occupy (see the test below, which pins it against their actual caps) and costs +/// nothing — the alternative is a limit that is satisfiable in theory and off-by-a-header in +/// practice. +pub(crate) const MAX_CONFIGURABLE_UPLOAD_MB: i64 = + (MAX_UPLOAD_BYTES as i64 - 1024 * 1024) / (1024 * 1024); #[tokio::main] async fn main() -> Result<()> { @@ -118,7 +147,9 @@ async fn main() -> Result<()> { // the precise per-class limits from DB config (max_image/video_size_mb); this // layer just stops a multi-GB body from being buffered into memory before that // check runs. Sized generously above the default 500 MB video limit + multipart - // overhead — if an admin raises max_video_size_mb above this, bump MAX_UPLOAD_BYTES. + // overhead. The DB-configured limits can no longer exceed it: both are clamped to + // MAX_CONFIGURABLE_UPLOAD_MB at write time and at read time — see that constant + // for why a comment was not enough. .route( "/api/v1/upload", post(handlers::upload::upload).route_layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES)), @@ -467,3 +498,56 @@ async fn shutdown_signal() { std::process::exit(0); }); } + +#[cfg(test)] +mod tests { + use super::{MAX_CONFIGURABLE_UPLOAD_MB, MAX_UPLOAD_BYTES}; + use crate::handlers::upload::{ + MAX_CAPTION_BYTES, MAX_CLIENT_UPLOAD_ID_BYTES, MAX_HASHTAGS_BYTES, + }; + + /// Boundary lines, `Content-Disposition` / `Content-Type` headers and CRLFs for the four + /// fields the handler reads. A few hundred bytes in reality; 4 KiB is a deliberately fat + /// allowance so this test asserts the invariant rather than a precise byte count. + const MULTIPART_FRAMING_BYTES: usize = 4 * 1024; + + /// Both bounds on `MAX_CONFIGURABLE_UPLOAD_MB`, asserted at COMPILE time. + /// + /// `const _: () = assert!(...)` rather than a runtime `assert!`, matching the precedent in + /// `handlers::upload` and `services::compression`: every operand is a constant, so a + /// violation is a build failure rather than something that has to be run to be noticed. The + /// cost is that a const panic takes a static message — the reasoning lives here instead. + /// + /// UPPER: a file at the largest configurable limit must still fit inside the body limit the + /// router enforces, envelope included. If it does not, an operator can set a limit the + /// handler accepts and the router then refuses MID-BODY — a 400 that the upload queue + /// classifies as terminal and answers by deleting the guest's only copy of the photo. See + /// `MAX_CONFIGURABLE_UPLOAD_MB`. Written against the field caps rather than a hardcoded + /// number, so raising `MAX_CAPTION_LENGTH` (or adding another text field to the envelope) + /// fails HERE instead of silently eating the margin. + /// + /// LOWER: the ceiling must not be so conservative that it forbids the shipped default. + /// `max_video_size_mb` is seeded at 500 (migration 005), so a bound below that would clamp + /// every video upload on a stock install and reject the stock config through `patch_config`. + #[test] + fn the_configurable_ceiling_is_bounded_at_both_ends() { + const FILE: usize = MAX_CONFIGURABLE_UPLOAD_MB as usize * 1024 * 1024; + const ENVELOPE: usize = MAX_CAPTION_BYTES + + MAX_HASHTAGS_BYTES + + MAX_CLIENT_UPLOAD_ID_BYTES + + MULTIPART_FRAMING_BYTES; + const _: () = { + assert!( + FILE + ENVELOPE <= MAX_UPLOAD_BYTES, + "a file at MAX_CONFIGURABLE_UPLOAD_MB plus its multipart envelope exceeds \ + MAX_UPLOAD_BYTES — an operator could configure a limit that DESTROYS uploads \ + (400 mid-body, blob purged as terminal) instead of refusing them" + ); + assert!( + MAX_CONFIGURABLE_UPLOAD_MB >= 500, + "MAX_CONFIGURABLE_UPLOAD_MB must clear the 500 MB max_video_size_mb default \ + seeded by migration 005, or a stock install clamps every video upload" + ); + }; + } +} diff --git a/backend/src/services/disk.rs b/backend/src/services/disk.rs index 2afc197..9cf0bcf 100644 --- a/backend/src/services/disk.rs +++ b/backend/src/services/disk.rs @@ -7,6 +7,7 @@ //! barely moves. [`DiskCache`] refreshes it at most once per [`TTL`] and serves the //! rest from memory. +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use std::time::{Duration, Instant}; @@ -20,52 +21,72 @@ pub struct DiskInfo { pub free: u64, } -/// Cheap-to-clone cache of the media filesystem's total/free bytes. Lives in -/// `AppState`. +/// Cheap-to-clone cache of a filesystem's total/free bytes, keyed by the path asked about. +/// Lives in `AppState`. +/// +/// ONE ENTRY PER PATH, not one entry total. This held a single slot carrying its own key, so a +/// lookup for a different path was a miss that OVERWROTE the previous reading — and the app asks +/// about two paths that are distinct mounts in production (`MEDIA_PATH=/media` and +/// `EXPORT_PATH=/exports`, docker-compose.yml). The upload gate and the per-user quota ask about +/// the media volume on every single photo; `host::get_event_status` asks about the exports volume +/// on every host-dashboard load. So while a host had the dashboard open the two evicted each +/// other and the cache hit rate collapsed to zero, putting an uncached +/// `sysinfo::Disks::new_with_refreshed_list()` — a synchronous scan of every mount, on the async +/// runtime — back on the busiest write path in the app. That is precisely the cost the comment in +/// `handlers::upload` says this cache exists to avoid, on the 2-vCPU box it says it matters on. +/// +/// The key space cannot grow: both paths come from `AppConfig`, never from request input, so this +/// map holds at most as many entries as there are configured volumes. #[derive(Clone)] pub struct DiskCache { - inner: Arc>>, + inner: Arc>>, } impl DiskCache { pub fn new() -> Self { Self { - inner: Arc::new(RwLock::new(None)), + inner: Arc::new(RwLock::new(HashMap::new())), } } - /// Drop the cached reading so the next `snapshot()` re-measures the filesystem. + /// Drop every cached reading so the next `snapshot()` re-measures the filesystem. /// /// Used by the e2e TRUNCATE endpoint. Truncating deletes every uploaded file, which materially /// changes free space — but the cached reading survives for up to the TTL, so the next test can /// compute a quota from the PREVIOUS test's disk. That matters now that the quota tests steer /// the per-user limit off `free_disk_bytes`: a stale reading makes the limit wrong and the test /// flaky, for reasons that have nothing to do with the code under test. + /// + /// Clears ALL paths, not just the media one: truncation moves free space on every volume the + /// app measures, and the export volume feeds the host dashboard's low-disk banner. pub fn invalidate(&self) { - *self.inner.write().unwrap() = None; + self.inner.write().unwrap().clear(); } - /// Cached `(total, free)` bytes for the filesystem that holds `media_path`. + /// Cached `(total, free)` bytes for the filesystem backing `path`. /// /// Returns `None` when the mount can't be resolved — callers MUST treat that as /// "unknown", never "zero free". (The quota path in particular fails *open* on /// `None`: enforcing a 0-byte limit would lock every user out of uploading.) - /// Cached free-space reading for `path`. /// - /// The cache is keyed BY PATH. It used to hold a single slot and ignore its argument on a hit, - /// so it would happily return the media volume's numbers for any other path within the TTL. - /// That was invisible only because every caller happened to pass `media_path` — the first - /// caller to ask about a different volume (e.g. the exports volume, which is a separate mount) - /// would have silently got the wrong filesystem's free space. + /// The cache is keyed BY PATH, and it RETAINS every path it has been asked about — see the + /// struct doc for why holding one slot made the cache stop caching as soon as the app asked + /// about its second volume. + /// + /// (The original single slot also carried its key, so it never returned the WRONG + /// filesystem's numbers. What it did instead was miss and re-measure on every alternation, + /// which is the quieter failure and the one that cost.) pub fn snapshot(&self, path: &Path) -> Option { - if let Some((cached_path, info, at)) = self.inner.read().unwrap().as_ref() - && cached_path == path + if let Some((info, at)) = self.inner.read().unwrap().get(path) && at.elapsed() < TTL { return Some(*info); } let info = read_disk_for_path(path)?; - *self.inner.write().unwrap() = Some((path.to_path_buf(), info, Instant::now())); + self.inner + .write() + .unwrap() + .insert(path.to_path_buf(), (info, Instant::now())); Some(info) } } @@ -129,7 +150,91 @@ fn select_disk(mounts: &[(String, u64, u64)], media_path: &str) -> Option