diff --git a/.env.example b/.env.example index 41dca19..d2bf4a8 100644 --- a/.env.example +++ b/.env.example @@ -36,13 +36,20 @@ DATABASE_URL=postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/events POSTGRES_USER=eventsnap POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password POSTGRES_DB=eventsnap -# Connection pool size. Default 10. For a busy event (~100 guests polling the feed -# + SSE + uploads at once) raise to ~30 so requests don't queue on a pool permit. -# PAIRED WITH THE DB CONTAINER'S MEMORY LIMIT: 30 backends plus Postgres 16's default -# shared_buffers is already snug in the 1G that docker-compose.yml allots the `db` -# service. If you raise this, raise `db.deploy.resources.limits.memory` with it — an -# OOM in Postgres doesn't degrade one feature, it takes the whole event down. -DATABASE_MAX_CONNECTIONS=30 +# Connection pool size. The code default is 10 (backend/src/db.rs) — set it explicitly, +# because a `.env` written by hand from this file's secrets is otherwise silently on 10. +# +# SIZE IT TO THE CORES, NOT TO THE GUESTS. The earlier advice here was ~30, reasoned from +# "~100 guests polling the feed at once" back when a feed page cost ~449 ms and connections +# were spent waiting. Migration 024 replaced the feed view's GROUP BY with scalar subqueries +# and a page now costs well under a millisecond, so concurrency is no longer where the time +# goes. On a 2 vCPU box 30 simultaneous queries cannot run — they queue on the CPU instead of +# on the pool, which is the same wait wearing a different hat, and 30 Postgres backends plus +# shared_buffers is snug in the 1G that docker-compose.yml allots `db`. +# +# 15 on 2 vCPU / 4 GB. Raise toward 30 only alongside more cores AND a bigger `db` memory +# limit — an OOM in Postgres doesn't degrade one feature, it takes the whole event down. +DATABASE_MAX_CONNECTIONS=15 # Log level. `info` is the right production default: at `debug` the tower-http trace # layer writes a line per request AND per response, which on a busy event is a large @@ -124,12 +131,23 @@ EXPORT_PATH=/exports # display resize: ~145 MB at 12 MP, ~223 MB at 24 MP, ~354 MB at 48 MP. # # So on a 2 vCPU / 4 GB box (e.g. Hetzner CX22) KEEP THIS AT 2: -# * concurrency 2, two 48 MP photos ≈ 800 MB against the 1G app limit — ~25% margin. -# * concurrency 4, the same pair ≈ 1.5 GB — OOM. +# * concurrency 4 would put two giants at ~1.5 GB against the 1G app limit — OOM. # * and app=2G + db=1G + frontend/caddy 256M each + ~370 MB of OS/Docker exceeds the # ~3910 MiB a "4 GB" VM actually reports. Raising the limit oversubscribes the host. # 4 is only reasonable on the 4 vCPU / 8 GB box README.md documents. # +# The "two 48 MP photos at once" worst case this number used to be sized against is no +# longer reachable: compression.rs takes an EXCLUSIVE `heavy` permit for any job whose +# estimated peak exceeds HEAVY_IMAGE_BYTES (150 MiB), so two giants serialise no matter what +# this is set to. What concurrency 2 now buys is two ORDINARY phone photos in parallel +# (~145 MB peak each), which is both memory-safe and short enough not to starve the two +# tokio worker threads a 2 vCPU box gets. +# +# Do NOT drop this to 1 hoping to protect the CPU. It halves throughput on the common light +# path for a heavy path that is already serialised, and a longer compression backlog means +# more feed tiles served from full-size originals (VirtualFeed falls back to /original while +# derivatives are pending) — trading a little CPU for a lot of venue-wifi bandwidth. +# # Throughput at 2 is not the bottleneck anyone thinks it is: ~2.5s per 12 MP photo, so # 100 photos is ~250 CPU-seconds spread over an entire evening. COMPRESSION_WORKER_CONCURRENCY=2 diff --git a/Caddyfile b/Caddyfile index 09103ef..ec6b22e 100644 --- a/Caddyfile +++ b/Caddyfile @@ -75,4 +75,59 @@ # Everything else goes to SvelteKit frontend reverse_proxy frontend:3001 + + # Last-resort page for when Caddy itself cannot reach an upstream — the app or frontend + # container down, restarting, or still warming up after a host reboot. Without it a guest + # gets Caddy's bodiless 502: a completely blank page, which reads as "the whole thing is + # gone" rather than "try again in a moment". + # + # THIS DOES NOT TOUCH APPLICATION ERRORS. `handle_errors` fires only on errors CADDY + # generates; a status the app returns through `reverse_proxy` is written back verbatim and + # never reaches here. That distinction is load-bearing rather than incidental: the keepsake + # download navigates a HIDDEN IFRAME and depends on a real 404/429 arriving from the app + # (frontend/src/routes/export/+page.svelte), and every API route answers 403/404/429 as + # ordinary JSON that the client parses. Swallowing those into an HTML page would be a far + # worse regression than the blank 502 this fixes. Verified against this exact config: an + # upstream 404 through `reverse_proxy` still arrives as `Content-Type: application/json` + # with its body intact, while only a dial failure renders the page below. + # + # Scoped to 5xx so a hypothetical future Caddy-generated 4xx (there is none today) still + # returns plainly instead of claiming the server is restarting. + # + # The body is inline because the caddy service mounts ONLY ./Caddyfile and caddy_data — + # there is no volume to ship an HTML file through and the image has no build step, so a + # static file would mean changing the deployed stack's compose definition. No external + # font, stylesheet or image is referenced: the app may be exactly what is down. + # + # `handle_errors` has NO position in the directive order — Caddy hoists it into a separate + # `errors` route list — so it cannot disturb the "first `header` directive wins" hazard + # documented at the top of this file. The site-wide security headers still apply to it. + handle_errors 5xx { + header Content-Type "text/html; charset=utf-8" + header Cache-Control "no-store" + # {err.status_code} preserves the real status. Hardcoding 503 would mislabel a genuine + # 502 for anything watching from outside. + respond ` + + + + +Gleich zurück + + + +
+

Wir sind gleich zurück

+

Die Seite wird gerade neu gestartet.
Bitte lade in einem Moment neu — deine Fotos bleiben gespeichert.

+
+ + +` {err.status_code} + } } diff --git a/README.md b/README.md index 73c1926..a462b4e 100644 --- a/README.md +++ b/README.md @@ -280,35 +280,49 @@ so a host takedown or a ban actually revokes access to the bytes. degrade one subsystem — Postgres stops being able to write and the whole event goes down. -Uploads are self-limiting. `per_user_limit = free_disk × quota_tolerance ÷ -active_uploaders` is recomputed against live free space on every upload, so guests -converge on a fixed point at `tolerance / (1 + tolerance)` of the free space you -started with — **43%** at the default 0.75. On an 80 GB box with ~70 GB free after -the OS and images, media settles at ~30 GB and stops. +**`Gallery.zip` and `Memories.zip` are each roughly a second copy of every original.** +Both write their media `Compression::Stored`, and `Memories.zip` streams the untouched +original for every video and for every image at or under 5 MB. So a release wants room +for **two more copies of the gallery** on top of the gallery itself — which is what +`required_free_bytes` encodes as `media × 1.1 × 2`. -**The keepsake is what the 80 GB baseline does not cover.** `Gallery.zip` and -`Memories.zip` are built concurrently and each is roughly a second copy of every -original: both write their media `Compression::Stored`, and `Memories.zip` streams the -untouched original for every video and for every image at or under 5 MB. So a release -wants room for **two more copies of the gallery** on top of the gallery itself. +The per-user quota does **not** bound this. It is a fairness mechanism that divides +free space between guests, and since it carries a floor (`MIN_QUOTA_LIMIT_BYTES`, so a +guest's allowance stops shrinking as the party fills up) the aggregate ceiling it used +to imply is gone. What bounds the disk is the **global gate in the upload handler**, +which refuses any upload that would leave too little room to build the keepsake: -| Stage | Used | Free (80 GB box) | -|---|---|---| -| Fresh box (OS + images) | ~10 GB | ~70 GB | -| Guests reach the quota fixed point | ~40 GB | ~40 GB | -| Host releases → both archives | ~100 GB | **ENOSPC** | +``` +free_after_upload < media_after × 1.1 × 2 + DISK_RESERVE_BYTES → refused +``` -Two ways to size for it: +Solving that for the gallery size gives the real ceiling. On the **40 GB box this runs +on**, with ~5 GB for the OS, Docker images (the runbook pre-pulls the rollback tag too) +and Postgres: + +| Volume | Usable after baseline | Media ceiling | Free at release | +|---|---|---|---| +| 40 GB | ~35 GB | **~8 GB** | ~27 GB → both archives fit | +| 80 GB | ~70 GB | ~19 GB | ~51 GB → both archives fit | + +**Uploads therefore stop at roughly 8 GB of media on a 40 GB box, not when the disk is +full.** That is deliberate. 1000 photos at ~3.5 MB is ~3.5 GB and fits comfortably; +video is what consumes the budget, so lower `max_video_size_mb` (seeded at 500) if you +expect a lot of it. Refusing the 1001st upload is a far better outcome than accepting it +and discovering at 01:00 that the archive can never be built. + +Two ways to buy headroom: - **Provision ~3× your expected media** on one volume (media + two archives), or - **give `exports_data` its own volume** so a full export cannot reach Postgres, and size that one at ~2× expected media. -This is no longer silent. The export refuses up front with the two numbers rather than -hitting ENOSPC halfway through a multi-GB write, a rebuild reclaims the superseded -generation before it starts (so peak is one generation, not two), and the host -dashboard warns as soon as the keepsake would not fit — which is the only point at -which anyone can still do something about it. +None of this is silent. The upload gate refuses with a German message naming the cause, +the export preflight refuses up front with both numbers rather than hitting ENOSPC +halfway through a multi-GB write, a rebuild only reclaims the superseded generation +**after** the new one lands (so a failed rebuild can never leave you with no archive at +all), and the host dashboard warns as soon as the keepsake would not fit — which is the +only point at which anyone can still do something about it. --- diff --git a/backend/src/error.rs b/backend/src/error.rs index d61a356..9da0c09 100644 --- a/backend/src/error.rs +++ b/backend/src/error.rs @@ -78,6 +78,26 @@ impl IntoResponse for AppError { }; let message = self.message(); + // Log every 4xx. Until now they were invisible at ANY log level: tower_http's + // `ServerErrorsAsFailures` classifier counts a 4xx as a *success*, so it goes to + // `DefaultOnResponse` at DEBUG, and production runs at `info`. The consequence is that a + // misconfigured limit leaves no trace at all — if guests spend the evening hitting 429s + // on `upload_rate_per_hour`, or 413s on the storage quota, `docker compose logs` after + // the event contains nothing about it and the cause is unknowable. + // + // WARN rather than INFO because every variant here is a request that did not do what + // the guest asked. 5xx is excluded: `Internal` already logs with its full source chain + // in `message()` above, and the pool-exhaustion 503 logs at construction — logging again + // here would double every server-side failure. + // + // No request context is available: `into_response` receives only the error, so there is + // no path, method or user id to attach. Status + code + message is what can honestly be + // reported from here, and it is enough to see the SHAPE of a bad evening. Raising + // `tower_http` to DEBUG instead was considered and rejected — see the note in main.rs. + if status.is_client_error() { + tracing::warn!(status = status.as_u16(), code, %message, "request rejected"); + } + let mut body = serde_json::json!({ "error": code, "message": message, @@ -161,6 +181,43 @@ mod tests { } } + /// 4xx must be logged and 5xx must not be logged HERE — `Internal` logs its source chain in + /// `message()` and the pool-exhaustion 503 logs at construction, so a second line in + /// `into_response` would double every server-side failure in the post-event logs. + /// + /// The guard is `status.is_client_error()`, so this pins the classification rather than the + /// logging itself (which needs a subscriber to observe). + #[test] + fn only_client_errors_are_in_the_logged_band() { + for err in [ + AppError::BadRequest("x".into()), + AppError::Unauthorized("x".into()), + AppError::Forbidden("x".into()), + AppError::UploadsLocked("x".into()), + AppError::NotFound("x".into()), + AppError::Conflict("x".into()), + AppError::TooManyRequests("x".into(), Some(1)), + AppError::QuotaExceeded("x".into()), + ] { + let (status, _) = err.status_and_code(); + assert!( + status.is_client_error(), + "{status} should be in the 4xx band this logs" + ); + } + + for err in [ + AppError::ServiceUnavailable("x".into(), Some(3)), + AppError::Internal(anyhow::anyhow!("boom")), + ] { + let (status, _) = err.status_and_code(); + assert!( + !status.is_client_error(), + "{status} logs elsewhere; logging it here would double it" + ); + } + } + /// Pool saturation is load, not a bug. A 500 makes the frontend's retry classifier pile /// straight back into the saturated pool with no backoff to pace it. #[test] diff --git a/backend/src/handlers/test_admin.rs b/backend/src/handlers/test_admin.rs index e3c7f3a..0be9770 100644 --- a/backend/src/handlers/test_admin.rs +++ b/backend/src/handlers/test_admin.rs @@ -111,6 +111,12 @@ pub async fn truncate_all( // steers the per-user limit off `free_disk_bytes`), i.e. two holes were masking each other. state.disk_cache.invalidate(); + // `media_total` caches SUM(user.total_upload_bytes) for the upload gate's keepsake-headroom + // check. TRUNCATE has just zeroed every one of those rows, so a surviving reading would make + // the next test's first upload measure its headroom against the previous test's gallery — + // and that gate REFUSES uploads, so the failure would look like a spurious quota rejection. + state.media_total.invalidate(); + // `sse_tickets` maps a ticket to a session token hash. TRUNCATE deletes the sessions, so every // surviving ticket is a dangling reference to a user that no longer exists. state.sse_tickets.clear(); diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index aba954a..8eab212 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -438,43 +438,62 @@ pub async fn upload( let quota_on = config::get_bool(&state.config_cache, "quota_enabled", true).await; let storage_quota_on = config::get_bool(&state.config_cache, "storage_quota_enabled", true).await; - // When quota is enforced, this holds the byte ceiling so the increment UPDATE below can - // enforce it atomically (`WHERE total + size <= limit`). Without that guard, two - // concurrent uploads from the same user (e.g. phone + laptop) both pass this stale - // pre-check and both increment, blowing past the quota. The pre-check stays as a - // fast path that avoids the disk write when the user is already clearly over. - // GLOBAL RESERVE, checked before the per-user ceiling and independent of every quota + // GLOBAL DISK GATE, checked before the per-user ceiling and independent of every quota // toggle. The per-user quota is a fairness mechanism, not a disk guarantee — and since it - // now carries a floor (MIN_QUOTA_LIMIT_BYTES) so a guest's allowance stops shrinking as - // the party fills up, the aggregate ceiling it used to imply is gone entirely. Something - // has to own "do not fill the volume", because `postgres_data`, `media_data` and - // `exports_data` share one filesystem: the end state is not a degraded feature, it is - // Postgres unable to write WAL and the whole event down with nobody watching. + // carries a floor (MIN_QUOTA_LIMIT_BYTES) so a guest's allowance stops shrinking as the + // party fills up, the aggregate ceiling it used to imply is gone entirely. Something has to + // own "do not fill the volume", because `postgres_data`, `media_data` and `exports_data` + // share one filesystem: the end state is not a degraded feature, it is Postgres unable to + // write WAL and the whole event down with nobody watching. // - // Deliberately NOT gated behind `quota_enabled`. That switch exists so an operator can - // stop rationing space between guests; it was never meant to authorise running the disk - // to zero, and an operator flipping it at 23:00 to unblock a guest should not silently - // disarm the last thing standing between the party and a dead database. + // WHAT IS RESERVED IS NOT A CONSTANT. A flat reserve answers "can Postgres still write", + // which is necessary and not sufficient: the keepsake needs room for BOTH halves at once — + // `required_free_bytes` is `media × 1.1 × 2`, since the ZIP and the HTML viewer are each + // gallery-sized. On the 40 GB box this runs on, a flat 10 GB reserve let uploads continue to + // roughly 25 GB of media while the release needed `2.2 × 25 + 10` = 65 GB free. Every upload + // in that band succeeded and then the archive could never be built — the product's entire + // promise, failing silently at the end of the night with nobody there to notice. + // + // So the gate enforces the invariant that actually matters: never accept an upload that + // would make the keepsake unbuildable. It shares `required_free_bytes` with the export + // preflight so the two cannot drift into disagreeing about the same question. + // + // Deliberately NOT gated behind `quota_enabled`. That switch exists so an operator can stop + // rationing space between guests; it was never meant to authorise running the disk to zero, + // and an operator flipping it at 23:00 to unblock a guest should not silently disarm the + // last thing standing between the party and a dead database. if let Some(free) = crate::services::disk::free_bytes(&state.config.media_path) { - let remaining = (free as i64).saturating_sub(size); - if remaining < DISK_RESERVE_BYTES { + let media_after = state.media_total.get(&state.pool).await.saturating_add(size); + let keepsake_needs = + crate::services::export::required_free_bytes(media_after.max(0) as u64, 2) as i64; + let free_after = (free as i64).saturating_sub(size); + let required = keepsake_needs.saturating_add(DISK_RESERVE_BYTES); + if free_after < required { tracing::error!( free_bytes = free, upload_size = size, + media_after, + keepsake_needs, reserve = DISK_RESERVE_BYTES, - "refusing upload: it would take the media volume below the reserve" + "refusing upload: it would leave too little room to build the keepsake" ); return Err(AppError::QuotaExceeded( - "Der Speicher des Events ist voll. Bitte sag einem Host Bescheid — neue \ - Uploads sind vorübergehend nicht möglich." + "Der Speicher des Events ist fast voll — damit die Galerie am Ende noch als \ + Download erstellt werden kann, sind neue Uploads jetzt gesperrt. Bitte sag \ + einem Host Bescheid." .into(), )); } } // Failing OPEN when the disk can't be read is deliberate and matches the per-user quota - // above: refusing every upload because a `statfs` failed would be a worse outage than the + // below: refusing every upload because a `statfs` failed would be a worse outage than the // one being guarded against. + // When quota is enforced, this holds the byte ceiling so the increment UPDATE below can + // enforce it atomically (`WHERE total + size <= limit`). Without that guard, two + // concurrent uploads from the same user (e.g. phone + laptop) both pass this stale + // pre-check and both increment, blowing past the quota. The pre-check stays as a + // fast path that avoids the disk write when the user is already clearly over. let mut quota_limit: Option = None; if quota_on && storage_quota_on { let estimate = compute_storage_quota(&state).await; @@ -1370,7 +1389,9 @@ pub async fn get_thumbnail( #[cfg(test)] mod tests { - use super::{MIN_QUOTA_LIMIT_BYTES, RangeSpec, parse_range, quota_limit_bytes}; + use super::{ + DISK_RESERVE_BYTES, MIN_QUOTA_LIMIT_BYTES, RangeSpec, parse_range, quota_limit_bytes, + }; // `Range` handling exists because iOS Safari probes every `