From e727d53b8585df2ef56b86ead5409e57d34b690a Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Sat, 5 Sep 2026 01:34:39 +0000 Subject: [PATCH] Cut per-request origin work: batched article loads, no session write per page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The origin was 3–8 ms per page, almost all of it per-statement overhead: an issue page ran two statements per pick (~55 on a 25-article issue) and every signed-in page wrote its session row back because `take_flash` called `Session::remove`, which marks the session modified even when the key is absent. - `Db::get_articles` loads an issue's articles and their social rows in two statements; both branches of `web::issue::load` use it. The single-id and batch queries share one projection via a macro. - `take_flash` reads before removing, and touches a signed-in session at most once a day so the inactivity expiry still slides. Anonymous requests never create a session. - `Server-Timing: app;dur=` on every response, outermost layer. - `reject_early_data`: 425 for a non-safe method that arrived as TLS 0-RTT data, so nginx `ssl_early_data on` is safe (RFC 8470 §5.2). - `[profile.release]`: fat LTO, one codegen unit (binary 46 → 29 MB). Docs: the Cloudflare proxy was retired on 2026-09-05 after measuring +43 ms per signed-in page from Boston; README reverse-proxy section is now the direct setup (upstream keepalive, 0-RTT lines) and the CDN runbook carries a retired-status banner. Dev seed, app-side: `/` 21 → 5 statements, 3.7 → 1.0 ms; `/feed.xml` 44 → 12, 9.1 → 3.0 ms; session writes per signed-in page 1 → 0. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Va5eMEmWEnjMXBsBob5FDW --- Cargo.toml | 7 ++ README.md | 65 ++++++++--- docs/runbooks/cdn-rollout.md | 14 +++ src/db.rs | 66 ++++++++++- src/server.rs | 2 + src/web/issue.rs | 15 ++- src/web/mod.rs | 206 ++++++++++++++++++++++++++++++++++- src/web/session.rs | 5 + 8 files changed, 353 insertions(+), 27 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 89ccbcc..aaf3408 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,6 +61,13 @@ tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } url = { version = "2.5.8", features = ["serde"] } zip = { version = "6", default-features = false, features = ["deflate"] } +[profile.release] +# Per-request cost in the web server is JSON decode and template render, both +# CPU-bound and spread across crates; whole-program LTO and one codegen unit +# buy a slice of that for a longer release build (tests use the dev profile). +lto = "fat" +codegen-units = 1 + [dev-dependencies] figment = { version = "0.10.19", features = ["test", "toml", "env"] } roxmltree = "0.21.1" diff --git a/README.md b/README.md index dc8ee27..6d37e61 100644 --- a/README.md +++ b/README.md @@ -468,17 +468,27 @@ internet (e-readers tap these links). The OPDS catalog rides on the same host. The login throttle's `SmartIpKeyExtractor` keys on the first entry of `X-Forwarded-For`, so nginx must *set* that header from the connection's peer address rather than appending to whatever the client sent — otherwise anyone -can mint a fresh throttle bucket per attempt by sending their own header. With -Cloudflare in front, the connection's peer is Cloudflare, so `real_ip` has to -rewrite `$remote_addr` from `CF-Connecting-IP` first (see the snippet below). -The whole arrangement is only safe because the configured bind address is -loopback and nginx is the only process that can reach it. +can mint a fresh throttle bucket per attempt by sending their own header. (The +original config appended with `$proxy_add_x_forwarded_for`; that was the +spoofable version.) The whole arrangement is only safe because the configured +bind address is loopback and nginx is the only process that can reach it. -The site is served through Cloudflare; `docs/runbooks/cdn-rollout.md` covers the -zone setup and cache rules. The nginx side of it is the `set_real_ip_from` -snippet and the `X-Forwarded-For` line below. +Nothing sits in front of nginx. The site ran behind the Cloudflare proxy for one +day (2026-09-04 to 2026-09-05) and was taken back out: measured from Boston, the +proxied signed-in page took 70 ms after the TLS handshake against 27 ms direct, +and at this traffic the edge cache is cold for anonymous readers anyway. The zone +still lives on Cloudflare's nameservers, so re-proxying is a one-click toggle; +`docs/runbooks/cdn-rollout.md` keeps the zone settings, cache rules and the +nginx additions (the `set_real_ip_from` snippet) that the proxied setup needs. ```nginx +# One pool of idle connections to the app, so a request does not pay a fresh +# loopback TCP connect (and leave a TIME-WAIT socket) every time. +upstream daily_epub { + server 127.0.0.1:3499; + keepalive 8; +} + server { listen 443 ssl http2; listen [::]:443 ssl http2; @@ -488,11 +498,12 @@ server { ssl_certificate_key /etc/letsencrypt/live/daily.hallada.net/privkey.pem; ssl_trusted_certificate /etc/letsencrypt/live/daily.hallada.net/fullchain.pem; - include /etc/nginx/snippets/security-headers.conf; + # TLS 1.3 0-RTT: a returning browser sends its first request inside the + # handshake and saves a round trip. Safe only because the app answers 425 + # to any non-GET that arrives as early data (the Early-Data header below). + ssl_early_data on; - # Trust Cloudflare's edge for the client address: $remote_addr becomes the - # visitor, not the proxy. Must come before the X-Forwarded-For line below. - include /etc/nginx/snippets/cloudflare-real-ip.conf; + include /etc/nginx/snippets/security-headers.conf; # The global `gzip on` only covers text/html. The stylesheet is ~12 KB of # plain CSS again — the Newsreader faces are separate .woff2 URLs, not @@ -514,14 +525,21 @@ server { proxy_max_temp_file_size 0; location / { - proxy_pass http://127.0.0.1:3499; + proxy_pass http://daily_epub; + # Keep-alive to the upstream needs HTTP/1.1 and an empty Connection + # header (the default would forward "close"). proxy_http_version 1.1; + proxy_set_header Connection ""; proxy_set_header Host $host; # SET, not append. The login throttle keys on the first X-Forwarded-For # entry, so a client-supplied header must never survive into the app. - # $remote_addr is the real visitor because of the real_ip snippet above. + # $remote_addr is the connection's peer, which is the visitor itself + # now that no proxy sits in front of nginx. proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; + # "1" while the request arrived as 0-RTT data (RFC 8470); the app + # rejects non-safe methods sent that way. + proxy_set_header Early-Data $ssl_early_data; } } @@ -533,6 +551,13 @@ server { } ``` +**Only while the record is proxied through Cloudflare:** the connection's peer +is then Cloudflare, so `real_ip` has to rewrite `$remote_addr` from +`CF-Connecting-IP` before the `X-Forwarded-For` line runs. Add +`include /etc/nginx/snippets/cloudflare-real-ip.conf;` to the server block, +above `location /`. Leave it out when the record is DNS-only: it would be inert +for ordinary visitors, but anything connecting *from* a Cloudflare address (a +Worker, say) could then name its own address and dodge the login throttle. `/etc/nginx/snippets/cloudflare-real-ip.conf` is one `set_real_ip_from` line per published Cloudflare range plus the header to read. Cloudflare adds ranges from time to time, so **regenerate it from the source rather than copying this list** @@ -574,15 +599,21 @@ real_ip_header CF-Connecting-IP; ``` Enable the site (`ln -s` into `sites-enabled`, `nginx -t`, `systemctl reload -nginx`), then `curl https://daily.hallada.net/healthz` should return `ok`. No +nginx`), then `curl https://daily.hallada.net/healthz` should return `ok`. +Every response carries `Server-Timing: app;dur=`, the time the app spent +on the request; the browser's DevTools network panel shows it next to the +network timings, and `curl -sI` prints it, so origin work and the network can +be told apart without touching the logs. No auth is needed at the proxy layer: rating links are self-authenticating (HMAC tokens) and the OPDS/file routes use the app-level Basic auth from `server.basic_auth_user`/`_pass` if you set them. #### HTTP caching -The origin says what may be cached and for how long; the CDN is configured to -respect it (runbook §2). The whole matrix: +The origin says what may be cached and for how long. No shared cache sits in +front of it today (the Cloudflare proxy was retired on 2026-09-05), but every +response still carries an explicit policy so the proxy can come back with a DNS +toggle and nothing else. The whole matrix: | route | `Cache-Control` | |---|---| diff --git a/docs/runbooks/cdn-rollout.md b/docs/runbooks/cdn-rollout.md index 7d9d792..d44bf8b 100644 --- a/docs/runbooks/cdn-rollout.md +++ b/docs/runbooks/cdn-rollout.md @@ -1,5 +1,19 @@ # Runbook — putting daily.hallada.net behind Cloudflare +> **Status (2026-09-05): retired.** The `daily` record is **DNS only** again, one +> day after this rollout. Measured from Boston with a session cookie (so the edge +> bypassed its cache, as it does for every signed-in request): 70 ms after the +> TLS handshake through the proxy against 27 ms straight to the origin. The edge +> adds a proxy hop and its own overhead on every uncached request, and at this +> traffic the cache is cold for anonymous readers too, so the proxy cost the one +> signed-in reader more than it gave anyone. The zone stays on Cloudflare's +> nameservers; flipping the record back to Proxied re-enables everything below. +> When you do, also put the `cloudflare-real-ip.conf` include back into the nginx +> server block (README, *Reverse proxy*) — it was removed with the proxy because +> a direct connection from a Cloudflare address could otherwise name its own +> client IP. The origin's `Cache-Control` matrix (§0) is still in force and is +> what makes the toggle safe in either direction. + **Written:** 2026-09-04 for the production host. Steps 1–3 happen in the Cloudflare and registrar dashboards; steps 4–5 are on the server as an operator with `sudo`. The origin-side changes (the `Cache-Control` matrix below) ship in the same diff --git a/src/db.rs b/src/db.rs index c3dca87..683f81f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -5,7 +5,7 @@ //! (implementation notes §2). Pipeline writes are idempotent upserts so that //! `generate --date X` can be re-run safely; feedback events are append-only. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::path::Path; use std::str::FromStr; use std::time::Duration; @@ -333,6 +333,46 @@ impl Db { Ok(Some(article)) } + /// Every article in `ids`, keyed by id, with its social refs attached: + /// one statement for the articles and one for the social rows, however + /// many ids there are. Ids without a row are simply absent. This is what + /// a page that shows a whole issue should call; `get_article` in a loop + /// costs two statements per pick, and the per-statement overhead, not the + /// SQLite work, was most of that page's origin time. + pub async fn get_articles(&self, ids: &[ArticleId]) -> Result> { + let mut articles = HashMap::with_capacity(ids.len()); + // SQLite's default bound-parameter ceiling is 32 766; stay well under. + for chunk in ids.chunks(500) { + // Only the placeholder count is interpolated; every value is bound, + // which is what `AssertSqlSafe` asserts. + let placeholders = vec!["?"; chunk.len()].join(", "); + let mut query = sqlx::query(sqlx::AssertSqlSafe(format!( + "{ARTICLE_SELECT_BY_IDS} ({placeholders})" + ))); + for id in chunk { + query = query.bind(*id); + } + for row in query.fetch_all(&self.pool).await? { + let article = article_from_row(&row)?; + articles.insert(article.id, article); + } + let mut query = sqlx::query(sqlx::AssertSqlSafe(format!( + "SELECT article_id, source, item_id, score, num_comments, item_url, fetched_at + FROM social WHERE article_id IN ({placeholders}) ORDER BY article_id, source" + ))); + for id in chunk { + query = query.bind(*id); + } + for row in query.fetch_all(&self.pool).await? { + let social = social_from_row(&row)?; + if let Some(article) = articles.get_mut(&social.article_id) { + article.social.push(social); + } + } + } + Ok(articles) + } + pub async fn article_id_for_url(&self, canonical_url: &str) -> Result> { let row = sqlx::query("SELECT id FROM articles WHERE canonical_url = ?") .bind(canonical_url) @@ -785,8 +825,13 @@ impl Db { } /// Joined projection used by [`Db::get_article`]; keep in sync with [`article_from_row`]. -const ARTICLE_SELECT_BY_ID: &str = "\ -SELECT a.id AS id, a.canonical_url AS canonical_url, a.title AS title, +/// The article projection `article_from_row` reads, with the `WHERE` clause +/// supplied by the caller so the single-id and the `IN (...)` lookups cannot +/// drift apart. +macro_rules! article_select { + ($where:literal) => { + concat!( + "SELECT a.id AS id, a.canonical_url AS canonical_url, a.title AS title, a.best_entry_id AS best_entry_id, a.content_html AS content_html, a.word_count AS word_count, a.excerpt_only AS excerpt_only, a.image_count AS image_count, a.sources_json AS sources_json, @@ -795,7 +840,15 @@ SELECT a.id AS id, a.canonical_url AS canonical_url, a.title AS title, e.feed_title AS feed_title, e.category AS category, e.published_at AS published_at, e.comments_url AS comments_url FROM articles a LEFT JOIN entries e ON e.id = a.best_entry_id - WHERE a.id = ?"; + ", + $where + ) + }; +} + +const ARTICLE_SELECT_BY_ID: &str = article_select!("WHERE a.id = ?"); +/// Followed at runtime by a parenthesised placeholder list. +const ARTICLE_SELECT_BY_IDS: &str = article_select!("WHERE a.id IN"); // --------------------------------------------------------------------------- // Row mapping helpers (implementation notes §1: manual mapping, no macros) @@ -1112,6 +1165,11 @@ mod tests { assert_eq!(loaded.social.len(), 1); assert_eq!(loaded.social[0].score, 342); assert_eq!(loaded.feed_title, "Hacker News"); + // The batched lookup agrees with the single one and skips unknown ids. + let batch = db.get_articles(&[id, 9_999]).await.unwrap(); + assert_eq!(batch.len(), 1); + assert_eq!(batch[&id], loaded); + assert!(db.get_articles(&[]).await.unwrap().is_empty()); assert_eq!( db.article_id_for_url("https://example.com/1") .await diff --git a/src/server.rs b/src/server.rs index 517836a..a4ac41f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -182,7 +182,9 @@ pub fn router(state: AppState) -> Router { state.clone(), crate::web::session::require_same_origin, )) + .layer(from_fn(crate::web::reject_early_data)) .layer(from_fn(crate::web::security_headers)) + .layer(from_fn(crate::web::server_timing)) .with_state(state) } diff --git a/src/web/issue.rs b/src/web/issue.rs index 1b67f2f..b33683d 100644 --- a/src/web/issue.rs +++ b/src/web/issue.rs @@ -89,8 +89,17 @@ pub async fn load( }; let (mut issue, from_json) = if let Some(raw) = row.issue_json.as_deref() { let mut issue: Issue = serde_json::from_str(raw).context("decoding issues.issue_json")?; + // The snapshot's articles are refreshed from the live rows (social + // scores move after publish) in one batched lookup. + let ids: Vec = issue + .lineup + .picks + .iter() + .map(|pick| pick.article.id) + .collect(); + let mut articles = db.get_articles(&ids).await?; for pick in &mut issue.lineup.picks { - if let Some(article) = db.get_article(pick.article.id).await? { + if let Some(article) = articles.remove(&pick.article.id) { pick.article = article; } } @@ -103,12 +112,14 @@ pub async fn load( .bind(date.to_string()) .fetch_all(db.pool()) .await?; + let ids: Vec = rows.iter().map(|row| row.get("article_id")).collect(); + let mut articles = db.get_articles(&ids).await?; let mut picks = Vec::with_capacity(rows.len()); let mut seen_sections = Vec::new(); let mut summaries = BTreeMap::new(); for pick_row in rows { let article_id: i64 = pick_row.get("article_id"); - let Some(article) = db.get_article(article_id).await? else { + let Some(article) = articles.remove(&article_id) else { continue; }; let section: String = pick_row.get("section"); diff --git a/src/web/mod.rs b/src/web/mod.rs index f489493..e550de0 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -191,7 +191,7 @@ impl WebState { } } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Flash { pub kind: String, pub text: String, @@ -329,11 +329,104 @@ impl Page { } } +/// Consume the one-shot flash message for this page, if there is one. +/// +/// Reads before removing: `Session::remove` marks the session modified even +/// when the key is absent, and a modified session is written back to SQLite +/// at the end of the request, so the naive version cost every signed-in page +/// a write. Because a session is only saved when modified, its inactivity +/// expiry only slides when something changes it; [`touch_session`] keeps the +/// `server.session_days` window sliding by writing at most once a day. pub async fn take_flash(session: &Session) -> Result, WebError> { - session - .remove("flash") + let flash = session + .get::(FLASH_KEY) .await - .map_err(|error| WebError::Internal(error.into())) + .map_err(session_error)?; + if flash.is_some() { + session + .remove_value(FLASH_KEY) + .await + .map_err(session_error)?; + return Ok(flash); + } + touch_session(session).await?; + Ok(None) +} + +const FLASH_KEY: &str = "flash"; +/// Session key holding the unix time of the last expiry-extending write. +const TOUCHED_KEY: &str = "touched_at"; +/// How often a signed-in session is written just to slide its expiry. +const TOUCH_INTERVAL_SECS: i64 = 24 * 60 * 60; + +/// Slide a signed-in session's inactivity expiry, at most once a day. +/// +/// Only a session that already carries a login is touched. An anonymous +/// request must never create a session: the cookie would follow that reader +/// to every public page and take them out of the shared cache. +async fn touch_session(session: &Session) -> Result<(), WebError> { + let signed_in = session + .get_value(session::AUTH_DATA_KEY) + .await + .map_err(session_error)? + .is_some(); + if !signed_in { + return Ok(()); + } + let now = Timestamp::now().as_second(); + let last = session + .get::(TOUCHED_KEY) + .await + .map_err(session_error)?; + if last.is_none_or(|last| now - last >= TOUCH_INTERVAL_SECS) { + session + .insert(TOUCHED_KEY, now) + .await + .map_err(session_error)?; + } + Ok(()) +} + +fn session_error(error: axum_login::tower_sessions::session::Error) -> WebError { + WebError::Internal(error.into()) +} + +/// `Server-Timing: app;dur=` on every response: the time this process +/// spent on the request, so the browser's DevTools (or `curl -sI`) can split +/// origin work from the network. Outermost layer, so it covers the session +/// load, auth and rendering. +pub async fn server_timing(request: Request, next: Next) -> Response { + let started = std::time::Instant::now(); + let mut response = next.run(request).await; + let millis = started.elapsed().as_secs_f64() * 1000.0; + if let Ok(value) = HeaderValue::from_str(&format!("app;dur={millis:.2}")) { + response + .headers_mut() + .insert(header::HeaderName::from_static("server-timing"), value); + } + response +} + +/// Refuse TLS 1.3 0-RTT data for anything but a safe method (RFC 8470 §5.2). +/// +/// With `ssl_early_data on`, nginx forwards `Early-Data: 1` for a request the +/// browser sent inside the handshake. A replayed early-data `GET` is +/// harmless; a replayed `POST` (a rating, a login attempt, a job start) is +/// not, so those get 425 and the browser resends after the handshake. +pub async fn reject_early_data(request: Request, next: Next) -> Response { + let early = request + .headers() + .get("early-data") + .is_some_and(|value| value == "1"); + if early && !request.method().is_safe() { + return ( + StatusCode::TOO_EARLY, + [(header::CACHE_CONTROL, "no-store")], + "retry after the TLS handshake completes", + ) + .into_response(); + } + next.run(request).await } pub struct Html(pub T); @@ -753,6 +846,111 @@ mod tests { .unwrap() } + #[tokio::test] + async fn take_flash_reads_without_dirtying_an_empty_session() { + let (_dir, state) = test_state(Config::default()).await; + let store = std::sync::Arc::new(session::SqliteSessionStore::new(state.db.pool().clone())); + let session = Session::new(None, store, None); + assert_eq!(take_flash(&session).await.unwrap(), None); + assert!(!session.is_modified()); + assert!(session.is_empty().await); + + let flash = Flash { + kind: "ok".into(), + text: "saved".into(), + }; + session.insert(FLASH_KEY, &flash).await.unwrap(); + assert_eq!(take_flash(&session).await.unwrap(), Some(flash)); + assert_eq!(take_flash(&session).await.unwrap(), None); + } + + #[tokio::test] + async fn signed_in_pages_touch_the_session_once_a_day_not_every_request() { + let mut config = Config::default(); + config.server.public_url = "https://daily.example".into(); + let (_dir, state) = test_state(config).await; + // Only the dashboard and the signed-in issue pages consume flashes, so + // an admin exercises the path without an issue in the database. + users::add(&state.db, "admin", "correct horse battery", true) + .await + .unwrap(); + let app = router(state.clone()); + let cookie = login_cookie(&app, "admin", "correct horse battery").await; + let stamp = || async { + sqlx::query_scalar::<_, String>("SELECT updated_at || ' ' || data FROM sessions") + .fetch_one(state.db.pool()) + .await + .unwrap() + }; + let get = |uri: &str| { + Request::builder() + .uri(uri) + .header(header::COOKIE, cookie.clone()) + .body(Body::empty()) + .unwrap() + }; + // The first page after login writes the daily touch stamp… + let response = app.clone().oneshot(get("/dashboard/articles")).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let first = stamp().await; + assert!(first.contains(TOUCHED_KEY), "{first}"); + // …and the pages after it leave the row alone. + for uri in [ + "/dashboard/articles", + "/dashboard/ratings", + "/dashboard/stats", + "/", + "/account", + ] { + let response = app.clone().oneshot(get(uri)).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK, "{uri}"); + } + assert_eq!(stamp().await, first); + } + + #[tokio::test] + async fn server_timing_header_and_early_data_guard() { + let (_dir, state) = test_state(Config::default()).await; + let app = router(state); + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/healthz") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let timing = response + .headers() + .get("server-timing") + .unwrap() + .to_str() + .unwrap(); + assert!(timing.starts_with("app;dur="), "{timing}"); + + // A safe method may arrive as 0-RTT data; a POST may not. + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/healthz") + .header("early-data", "1") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let mut request = post("/login", "username=a&password=b", "192.0.2.1"); + request + .headers_mut() + .insert("early-data", HeaderValue::from_static("1")); + let response = app.oneshot(request).await.unwrap(); + assert_eq!(response.status(), StatusCode::TOO_EARLY); + } + #[tokio::test] async fn login_cookie_account_logout_and_anonymous_pages() { let mut config = Config::default(); diff --git a/src/web/session.rs b/src/web/session.rs index b56972c..4e52443 100644 --- a/src/web/session.rs +++ b/src/web/session.rs @@ -20,6 +20,11 @@ use crate::server::AppState; use crate::web::users::{self, Role, User}; use crate::web::{Html, Page, WebError}; +/// The session key axum-login keeps the signed-in user under (its default +/// `data_key`); the presence of this key is what "signed in" means to +/// [`crate::web::take_flash`]'s once-a-day session touch. +pub const AUTH_DATA_KEY: &str = "axum-login.data"; + const DUMMY_HASH: &str = "$argon2i$v=19$m=65536,t=1,p=1$c29tZXNhbHQAAAAAAAAAAA$+r0d29hqEB0yasKr55ZgICsQGSkl0v0kgwhd+U3wyRo"; #[derive(Clone, Debug)]