From 4fe5c84bade6e3df752a273b0c7b25c1a5054a8b Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 16 Sep 2026 19:25:05 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(plugin):=20conditional=20sitemap=20fet?= =?UTF-8?q?ching=20=E2=80=94=20skip=20the=20reconcile=20on=20a=20304;=20v0?= =?UTF-8?q?.69.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every pass re-fetched every child sitemap in full and ran its prune scan, whether the document had changed or not. On one production corpus that is 31 documents and ~125 MB of XML per pass, four passes a day, to discover a change that happens once a night. `sitemap.conditional` sends `If-Modified-Since` from a stored `Last-Modified` and, on a 304, skips the body, the parse, the prune scan and every write. That last part is the point: the per-child prune scan holds a read cursor, and cursor-seconds are what scale linearly with refresh frequency, so an unchanged pass now costs one request per document and NO database work. Polling for a rebuild every few minutes becomes affordable rather than merely possible — which is what lets a deployment detect a nightly rebuild in minutes instead of waiting out a fixed grid slot. USE `Last-Modified`, NOT `ETag`, and this is measured, not assumed. Against a production edge, `If-Modified-Since` returned a clean 304 with no body; `If-None-Match` sent back with the exact ETag that same edge had just served returned 200 and the full 7.9 MB document. An ETag-based conditional fetch fails in the worst way available — it looks correct, returns 200 every time, and silently re-transfers the whole corpus forever. `sitemap_not_modified` exists so that failure is visible: a steady zero means the origin is not honouring the validator and the frequency should come back down. AN INDEX IS STILL DESCENDED on a 304. That says the CHILD LIST is unchanged, not that the children are — they are separate documents with their own validators, and on a real corpus they rebuild on a different schedule from the index that lists them (measured: children nightly, their index at a different hour entirely). The stored entries are re-read and each child makes its own conditional decision. `fullPassInterval` (24h) forces an unconditional fetch of any document whose entries have not been ingested in that long. This is the repair net and it is why the feature is safe to leave on: a 304 skips the reconcile, and the reconcile is also what re-CREATES targets lost to anything else — a bad purge, a half-applied delete, a botched migration. Without it a corpus could drift for as long as the origin left its sitemaps untouched. `lastRefreshed` now means "entries last INGESTED" precisely so it can be measured against; a 304 leaves it alone. Set the interval to 0 to restore the pre-0.69.0 behaviour. The decision lives in util/sitemapConditional.js so it is testable; `resources/Sitemap.js` subclasses a table at import time and cannot be loaded without a live Harper. Co-Authored-By: Claude Opus 5 (1M context) --- packages/plugin/METRICS.md | 22 +++---- packages/plugin/src/configSchema.js | 33 ++++++++++ packages/plugin/src/resources/Sitemap.js | 55 +++++++++++++++-- packages/plugin/src/schemas/schema.graphql | 10 +++ .../plugin/src/util/sitemapConditional.js | 47 ++++++++++++++ packages/plugin/src/util/sitemapRun.js | 5 ++ .../plugin/test/sitemapConditional.test.js | 61 +++++++++++++++++++ 7 files changed, 217 insertions(+), 16 deletions(-) create mode 100644 packages/plugin/src/util/sitemapConditional.js create mode 100644 packages/plugin/test/sitemapConditional.test.js diff --git a/packages/plugin/METRICS.md b/packages/plugin/METRICS.md index df31f71..d2898ec 100644 --- a/packages/plugin/METRICS.md +++ b/packages/plugin/METRICS.md @@ -121,17 +121,17 @@ PK drives the scan (an open range can make the planner walk a metric's entire hi One-line summaries; `src/metrics.js` carries the full description of every dimension value and the reasoning behind it. -| Metric | Kind | `path` | `method` | `type` | What it's for | -| ---------------- | ------- | ---------- | ----------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `bot_request` | counter | host | botName | deviceType | Raw crawl volume and mix at ingress. The denominator for every serve-side ratio. | -| `bot_serve` | counter | source | cacheStatus | botName | **Origin offload** and **cache hit rate** — the two rollout numbers. | -| `route_serve` | counter | route | cacheStatus | deviceType | The same outcome per route: which route's `renderInterval` needs to move. | -| `page_age` | ms | botName | deviceType | — | Freshness as delivered: ms since the served snapshot rendered (cache serves only). | -| `route_page_age` | ms | route | cacheStatus | deviceType | Served age per route, split by freshness state — the "should this TTL move" number. | -| `render` | value | series | per-series | per-series | The render fleet in one scan: `time_ms` (duration by statusCode × candidacy, one sample per device variant — renders/hour = concurrency ÷ time_ms) and `outcome` (counter by outcome × detail, exactly one per posted result — and a result is one URL, every device in it, since v0.66.0 — the render-failure alert). | -| `origin_fetch` | ms | statusCode | reason | — | Cost of every non-cache serve: origin latency + status, by why the cache didn't answer (miss/stale/skip/invalidated/bypass/blob-missing/blob-timeout/render-timeout). | -| `prerender_ops` | value | series | detail | context | Every low-volume ops signal in one scan: `unrouted` (class, bucket), `sitemap_*` (refresh-run counters: sitemaps/created/updated/skipped/removed/failed, plus `sitemap_departure_*` — one series per outcome of the post-walk sitemap-departure check, `departure_render`/`departure_expire` for actions taken, `departure_would_render`/`departure_would_expire` under `sitemap.departure.dryRun`, and `departure_reattached`/`departure_suppressed`/`departure_route_opted_out`/`departure_target_gone`/`departure_capped` for the candidates nothing happened to. **`departure_reattached` is the one to watch**: it counts URLs that only LOOKED departed because they shifted across a paginated sitemap's child boundary, so a large share means the corpus is shearing and the raw `sitemap_removed` count is not a departure count. `departure_capped` means `maxActions` bound and some departed URLs were left for the next walk), `serve_error`, `config_warnings`, `page_age_negative` (bot, device), `demand_*` (ladder decisions + `fast_fraction`/`fill`), `invalidation_error` (kind), `invalidation_reenqueue` (outcome, scope — including the cross-node outcomes `forwarded`/`forward-failed`; `forwarded` means this node handed the heal to the key's owner, which counts its OWN verdict in this same series, so the two are deliberately not double-counted), `page_verification` (outcome: `written`/`read-error`/`write-error` — per-page invalidation exemptions being recorded; the exemptions actually GRANTED are `bot_serve` cacheStatus `verified`, not this), `probe_*` (change-probe pass counters: probed/seeded/rebaselined/changed/triggered/deferred/failed per pass, plus `probe_canary_trip` and `probe_invalidated`; `probe_rebaselined` counts URLs whose stored baseline was taken under a different rule fingerprint and were re-seeded without comparison — expect one pass of them after a rule edit, and treat a steady count as a rule that keeps changing; `probe_changed`/`probe_probed` is the measured change rate, a rising `probe_failed` share is the endpoint-changed-shape alarm), `discovery_gated` (gate, bot: cacheable misses the discovery gate held out of target creation — the corpus growth being prevented, not denied mints), `probe_fresh` (probes skipped because a baseline was younger than `reprobeAfter` — the work a restarted sweep skipped), `probe_throttled` (probes the origin refused with pushback — **alert on this**: it is the only signal that the probe is loading an origin that cannot take it), `probe_unreadable` (registry rows whose key failed to decode, skipped by the sweep's walk — a nonzero count means the table holds rows the application layer cannot address; escalate to the database layer), `probe_page_mismatch` (cached pages that disagreed with the origin — the round-trip-blindness class `pageCheck` catches; a rising share means renders are landing on transient states, and each one is a served page carrying wrong price/availability until it re-renders), `probe_trigger_queue_depth` (high-water depth of the trigger queue during the pass — triggers are submitted to a bounded queue that drains beside the walk, so a value steadily at `changeProbe.trigger.maxPending` means the drain rate is behind the detection rate and changes are being deferred for want of QUEUE rather than of budget; those two are indistinguishable in `probe_deferred` alone), `probe_cycle_behind` (CONTINUOUS MODE: batches that needed more than `ratePerSecond` to hit `cycleTarget` — the pass is flat out against its agreed origin ceiling and still losing ground. **Alert on a sustained count**: it is the explicit replacement for the interval model's silently skipped pass, and it means the corpus has outgrown the rate, so either `cycleTarget` is too ambitious or the ceiling needs renegotiating. Zero in interval mode, where no target is set). | -| `queue_health` | value | series | result | — | Every queue signal in one scan: the snapshot gauges (`overdue`, `lease_occupancy`, `below_floor`, `below_floor_age_ms`, `floor_pin_age_ms`, `paused`), `claim_scan_ms` (per pass, method = granted/empty/capped), `claim_granted` (per claim, method = ready/index), `ready_sweep_ms` (per sweep, method = complete/capped), `ready_published`, `ready_cadence` (per sweep, method = carried/resolved), `reconcile_restored`/`reconcile_missing` (per sweep). | +| Metric | Kind | `path` | `method` | `type` | What it's for | +| ---------------- | ------- | ---------- | ----------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `bot_request` | counter | host | botName | deviceType | Raw crawl volume and mix at ingress. The denominator for every serve-side ratio. | +| `bot_serve` | counter | source | cacheStatus | botName | **Origin offload** and **cache hit rate** — the two rollout numbers. | +| `route_serve` | counter | route | cacheStatus | deviceType | The same outcome per route: which route's `renderInterval` needs to move. | +| `page_age` | ms | botName | deviceType | — | Freshness as delivered: ms since the served snapshot rendered (cache serves only). | +| `route_page_age` | ms | route | cacheStatus | deviceType | Served age per route, split by freshness state — the "should this TTL move" number. | +| `render` | value | series | per-series | per-series | The render fleet in one scan: `time_ms` (duration by statusCode × candidacy, one sample per device variant — renders/hour = concurrency ÷ time_ms) and `outcome` (counter by outcome × detail, exactly one per posted result — and a result is one URL, every device in it, since v0.66.0 — the render-failure alert). | +| `origin_fetch` | ms | statusCode | reason | — | Cost of every non-cache serve: origin latency + status, by why the cache didn't answer (miss/stale/skip/invalidated/bypass/blob-missing/blob-timeout/render-timeout). | +| `prerender_ops` | value | series | detail | context | Every low-volume ops signal in one scan: `unrouted` (class, bucket), `sitemap_*` (refresh-run counters: sitemaps/created/updated/skipped/removed/failed, plus `sitemap_not_modified` — documents the origin answered 304 to, whose entries were never re-parsed and whose prune scan never ran. **A steady ZERO where `sitemap.conditional` is enabled means the origin is not honouring `If-Modified-Since`** and every pass is doing full work, so the refresh frequency should come back down; plus `sitemap_departure_*` — one series per outcome of the post-walk sitemap-departure check, `departure_render`/`departure_expire` for actions taken, `departure_would_render`/`departure_would_expire` under `sitemap.departure.dryRun`, and `departure_reattached`/`departure_suppressed`/`departure_route_opted_out`/`departure_target_gone`/`departure_capped` for the candidates nothing happened to. **`departure_reattached` is the one to watch**: it counts URLs that only LOOKED departed because they shifted across a paginated sitemap's child boundary, so a large share means the corpus is shearing and the raw `sitemap_removed` count is not a departure count. `departure_capped` means `maxActions` bound and some departed URLs were left for the next walk), `serve_error`, `config_warnings`, `page_age_negative` (bot, device), `demand_*` (ladder decisions + `fast_fraction`/`fill`), `invalidation_error` (kind), `invalidation_reenqueue` (outcome, scope — including the cross-node outcomes `forwarded`/`forward-failed`; `forwarded` means this node handed the heal to the key's owner, which counts its OWN verdict in this same series, so the two are deliberately not double-counted), `page_verification` (outcome: `written`/`read-error`/`write-error` — per-page invalidation exemptions being recorded; the exemptions actually GRANTED are `bot_serve` cacheStatus `verified`, not this), `probe_*` (change-probe pass counters: probed/seeded/rebaselined/changed/triggered/deferred/failed per pass, plus `probe_canary_trip` and `probe_invalidated`; `probe_rebaselined` counts URLs whose stored baseline was taken under a different rule fingerprint and were re-seeded without comparison — expect one pass of them after a rule edit, and treat a steady count as a rule that keeps changing; `probe_changed`/`probe_probed` is the measured change rate, a rising `probe_failed` share is the endpoint-changed-shape alarm), `discovery_gated` (gate, bot: cacheable misses the discovery gate held out of target creation — the corpus growth being prevented, not denied mints), `probe_fresh` (probes skipped because a baseline was younger than `reprobeAfter` — the work a restarted sweep skipped), `probe_throttled` (probes the origin refused with pushback — **alert on this**: it is the only signal that the probe is loading an origin that cannot take it), `probe_unreadable` (registry rows whose key failed to decode, skipped by the sweep's walk — a nonzero count means the table holds rows the application layer cannot address; escalate to the database layer), `probe_page_mismatch` (cached pages that disagreed with the origin — the round-trip-blindness class `pageCheck` catches; a rising share means renders are landing on transient states, and each one is a served page carrying wrong price/availability until it re-renders), `probe_trigger_queue_depth` (high-water depth of the trigger queue during the pass — triggers are submitted to a bounded queue that drains beside the walk, so a value steadily at `changeProbe.trigger.maxPending` means the drain rate is behind the detection rate and changes are being deferred for want of QUEUE rather than of budget; those two are indistinguishable in `probe_deferred` alone), `probe_cycle_behind` (CONTINUOUS MODE: batches that needed more than `ratePerSecond` to hit `cycleTarget` — the pass is flat out against its agreed origin ceiling and still losing ground. **Alert on a sustained count**: it is the explicit replacement for the interval model's silently skipped pass, and it means the corpus has outgrown the rate, so either `cycleTarget` is too ambitious or the ceiling needs renegotiating. Zero in interval mode, where no target is set). | +| `queue_health` | value | series | result | — | Every queue signal in one scan: the snapshot gauges (`overdue`, `lease_occupancy`, `below_floor`, `below_floor_age_ms`, `floor_pin_age_ms`, `paused`), `claim_scan_ms` (per pass, method = granted/empty/capped), `claim_granted` (per claim, method = ready/index), `ready_sweep_ms` (per sweep, method = complete/capped), `ready_published`, `ready_cadence` (per sweep, method = carried/resolved), `reconcile_restored`/`reconcile_missing` (per sweep). | Notes that bite: diff --git a/packages/plugin/src/configSchema.js b/packages/plugin/src/configSchema.js index 7209a9a..ddfcd40 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -1732,6 +1732,39 @@ export const configSchema = group('Prerender plugin configuration.', { { min: 0 } ), failedCap: option(100, 'Max failed-entry samples carried back in a refresh result.', { min: 0 }), + conditional: group( + 'Conditional sitemap fetching: send `If-Modified-Since` and skip the whole reconcile for a ' + + 'document the origin answers 304 to.\n\n' + + 'WHAT IT BUYS. A pass re-fetches every child and scans the `sitemapUrl` index once per ' + + 'child, and it is that prune scan — a held read cursor, whose seconds scale linearly with ' + + 'refresh frequency — that sets the real cost of refreshing often. A 304 skips the body, the ' + + 'parse, the scan and every write, so an unchanged pass costs one request per document and ' + + 'no database work at all. That is what makes polling for a change affordable instead of ' + + 'merely possible: a deployment whose sitemaps rebuild once a night can check every few ' + + 'minutes and pay for the walk only on the pass that finds the rebuild.\n\n' + + 'USE `Last-Modified`, NOT `ETag`, AND DO NOT ASSUME EITHER. Measured on one production ' + + 'edge: `If-Modified-Since` returned a clean 304, while `If-None-Match` sent back the exact ' + + 'ETag the same edge had just served and got 200 with the full multi-megabyte body. An ' + + 'origin that advertises a validator is not promising to honour it, which is why the ' + + '`not_modified` counter is worth watching — a steady zero here means every pass is doing ' + + 'full work and the frequency should come back down.\n\n' + + 'AN INDEX IS STILL DESCENDED on a 304: that only says the CHILD LIST is unchanged, not the ' + + 'children, and on a real corpus the children rebuild on a different schedule from the index ' + + 'that lists them. Each child then makes its own conditional decision.', + { + enabled: option(true, 'Send `If-Modified-Since` when a stored validator is available.'), + fullPassInterval: option( + 24 * HOUR, + 'Force an UNCONDITIONAL fetch of a document whose entries have not been ingested in this ' + + 'long. This is the repair net and it is why the feature is safe to leave on: a 304 skips ' + + 'the reconcile, and the reconcile is also what re-CREATES targets lost to anything else — ' + + 'a bad purge, a half-applied delete, a botched migration. Without a periodic full pass a ' + + 'corpus could drift for as long as the origin left its sitemaps untouched and nothing ' + + 'would notice. Set it to 0 to make every fetch unconditional (the pre-0.69.0 behaviour).', + { unit: 'ms', min: 0 } + ), + } + ), departure: group( 'What a refresh does about URLs that LEAVE a sitemap, beyond unlinking them. The action is ' + 'declared PER ROUTE (`ingress.routes[].departureAction`); this group bounds and observes it, ' + diff --git a/packages/plugin/src/resources/Sitemap.js b/packages/plugin/src/resources/Sitemap.js index 539edf5..fa1450a 100644 --- a/packages/plugin/src/resources/Sitemap.js +++ b/packages/plugin/src/resources/Sitemap.js @@ -9,6 +9,7 @@ import { actionForExisting, canSkipLookup, createRefreshRun, TargetAction } from import { configuredStagingIp, dispatcherFor } from '../util/upstream.js'; import { setImmediate } from 'node:timers/promises'; import { applyInBatches, collectFromScan } from '../util/scan.js'; +import { conditionalValidatorFor } from '../util/sitemapConditional.js'; import { decideDeparture, DepartureAction } from '../util/sitemapDeparture.js'; import { cacheKeysOf } from './Target.js'; import { writeSchedule } from '../util/renderSchedule.js'; @@ -363,6 +364,7 @@ const progressFields = (snapshot) => ({ created: snapshot.created, updated: snapshot.updated, skipped: snapshot.skipped, + notModified: snapshot.notModified, duplicates: snapshot.duplicates, deferred: snapshot.deferred, removed: snapshot.removed, @@ -407,9 +409,9 @@ async function runTrackedRefresh(rootUrl, options) { }); logger.info( - `[prerender] Sitemap refresh for ${rootUrl} finished: ${result.sitemapsProcessed} sitemaps, ` + - `${result.created} created, ${result.updated} re-attributed, ${result.skipped} unchanged, ` + - `${result.removed} unlinked, ${result.failed.length} failed` + `[prerender] Sitemap refresh for ${rootUrl} finished: ${result.sitemapsProcessed} sitemaps ` + + `(${result.notModified} not modified), ${result.created} created, ${result.updated} re-attributed, ` + + `${result.skipped} unchanged, ${result.removed} unlinked, ${result.failed.length} failed` ); // The same numbers as METRICS — corpus churn and walk health, previously log-only. @@ -419,6 +421,7 @@ async function runTrackedRefresh(rootUrl, options) { metrics.sitemapRun(result.created, 'created'); metrics.sitemapRun(result.updated, 'updated'); metrics.sitemapRun(result.skipped, 'skipped'); + metrics.sitemapRun(result.notModified, 'not_modified'); metrics.sitemapRun(result.removed, 'removed'); metrics.sitemapRun(result.failed.length, 'failed'); } catch (e) { @@ -439,12 +442,40 @@ async function runTrackedRefresh(rootUrl, options) { * * The stored row is written last, so a document that throws partway leaves the previous row — * and its `lastRefreshed` — untouched rather than recording a refresh that did not happen. + * + * A 304 writes NOTHING — the stored row is still current, validator included — and for a urlset + * skips the reconcile entirely, which is where a pass's real cost lives: the per-child prune scan + * holds a read cursor, and cursor-seconds are what scale with refresh frequency. That is what + * makes polling often affordable rather than merely possible. */ async function refreshOneSitemap(sitemapUrl, { parentUrl, revalidate, run, visited }) { logger.info(`Processing sitemap`, sitemapUrl); - const latestSitemap = await fetchLatestSitemap(sitemapUrl); + // A narrow projection on purpose: `entries` on a product child is megabytes, and this read + // happens for every document on every pass. The entries are read back only on the one path + // that needs them — a 304 on an INDEX, whose row is small by construction. + const stored = await Sitemap.get({ id: sitemapUrl, select: ['url', 'isIndex', 'lastModified', 'lastRefreshed'] }); + const ifModifiedSince = conditionalValidatorFor(stored, revalidate); + + const latestSitemap = await fetchLatestSitemap(sitemapUrl, { ifModifiedSince }); + + if (latestSitemap.notModified) { + run.count('notModified'); + + // An INDEX still has to be descended. A 304 says the CHILD LIST is unchanged, not that the + // children are — they are separate documents with their own validators, and on a real corpus + // they move on a different schedule from the index that lists them (measured: children + // rebuilt nightly, the index that lists them at a different hour entirely). So re-read the + // stored entries and keep walking; each child then makes its own conditional decision. + if (stored?.isIndex === true) { + const storedRow = await Sitemap.get({ id: sitemapUrl, select: ['url', 'entries'] }); + return (storedRow?.entries ?? []).map(({ loc }) => loc).filter(Boolean); + } + return []; + } + const row = { ...latestSitemap, parentUrl }; + delete row.notModified; if (latestSitemap.isIndex === true) { await Sitemap.put(sitemapUrl, row); @@ -780,7 +811,7 @@ async function processDepartures(run) { } } -async function fetchLatestSitemap(url) { +async function fetchLatestSitemap(url, { ifModifiedSince = null } = {}) { // Route every Harper→origin sitemap fetch through the same edge as the render/origin-fetch // path: whenever a staging IP is configured, pin the TCP connection to it (Host/SNI stay the // real origin, exactly like upstream.js). The security token typically only authenticates @@ -795,9 +826,19 @@ async function fetchLatestSitemap(url) { headers: { 'User-Agent': config.sitemap.userAgent, [config.origin.securityToken.header]: config.origin.securityToken.value, + // Echoed back VERBATIM from the stored row — see the schema comment. Absent on the first + // fetch of a document, when the origin sends no validator, and whenever the caller wants a + // full re-ingest. + ...(ifModifiedSince ? { 'If-Modified-Since': ifModifiedSince } : {}), }, dispatcher: dispatcherFor(stagingIp), }); + + // BEFORE the `res.ok` guard, because 304 is not ok: `Response.ok` is 200-299, so a + // not-modified would otherwise be thrown as a failed fetch. Nothing else to read — a 304 has no + // body — and nothing to write: the stored row is still current, validator included. + if (res.status === 304) return { url, notModified: true }; + const xml = await res.text(); // A blocked/errored fetch returns an HTML error page with a 4xx/5xx status. Guard the @@ -819,10 +860,14 @@ async function fetchLatestSitemap(url) { return { url, + notModified: false, lastRefreshed: new Date(), isIndex: parsed.isIndex, entries: parsed.entries, entryCount: parsed.entries.length, + // Null where the origin sends none, which makes every later fetch of this document + // unconditional — the correct degradation, not an error. + lastModified: res.headers.get('last-modified') ?? null, }; } diff --git a/packages/plugin/src/schemas/schema.graphql b/packages/plugin/src/schemas/schema.graphql index 429ed3f..922afb5 100644 --- a/packages/plugin/src/schemas/schema.graphql +++ b/packages/plugin/src/schemas/schema.graphql @@ -31,7 +31,17 @@ type Sitemap @table(database: "sitemaps") { isIndex: Boolean entryCount: Int entries: [SitemapEntry] + # When this document's ENTRIES were last ingested — NOT merely when it was last checked. A + # conditional fetch that comes back 304 leaves this alone, so it keeps meaning "the reconcile + # behind this row ran then", which is what `sitemap.conditional.fullPassInterval` measures + # against and what an operator reading the console wants it to mean. lastRefreshed: Date + # The origin's `Last-Modified` for this document, replayed as `If-Modified-Since` on the next + # fetch. Stored as the RAW HEADER STRING, deliberately: it is an opaque validator to be echoed + # back verbatim, and parsing it to a Date and reformatting risks handing the origin a value + # that differs by a second and re-fetching the whole corpus. Null where the origin sends none, + # which simply means every fetch of that document is unconditional. + lastModified: String # The index that listed this sitemap, or null for one added directly. # # Every document reached during a walk gets its own row, children included, so without this diff --git a/packages/plugin/src/util/sitemapConditional.js b/packages/plugin/src/util/sitemapConditional.js new file mode 100644 index 0000000..74a4d99 --- /dev/null +++ b/packages/plugin/src/util/sitemapConditional.js @@ -0,0 +1,47 @@ +import { config } from '../config.js'; +import { epochMsOf } from './time.js'; + +/** + * Whether one sitemap document can be fetched CONDITIONALLY, and with what validator. + * + * Lives here rather than in `resources/Sitemap.js` for the usual reason: that module subclasses a + * table at import time and cannot be loaded without a live Harper, so a decision left inside it is + * untestable. + * + * ── WHY `Last-Modified` AND NOT `ETag` ─────────────────────────────────────────────────────── + * + * Measured against a production edge: `If-Modified-Since` returned a clean 304 with no body, while + * `If-None-Match` — sent back with the exact ETag that same edge had just served — returned 200 and + * the full multi-megabyte document. An origin that advertises a validator is not promising to + * honour it, and an ETag-based conditional fetch fails in the worst possible way: it looks correct, + * returns 200 every time, and silently re-transfers the whole corpus on every pass. Hence the stored + * validator is the `Last-Modified` string, echoed back verbatim. + * + * ── WHAT A 304 SKIPS, AND WHY THAT IS THE POINT ────────────────────────────────────────────── + * + * Not just the download. A pass scans the `sitemapUrl` index once per child, and that prune scan + * holds a read cursor whose seconds scale linearly with refresh frequency — it is the cost that + * decides how often a corpus can afford to be refreshed. A 304 skips the body, the parse, the scan + * and every write, so an unchanged pass costs one request per document and no database work. + */ +export const conditionalValidatorFor = (stored, revalidate) => { + const { enabled, fullPassInterval } = config.sitemap.conditional; + if (!enabled || revalidate) return null; + + // No validator: first walk of this document, or an origin that sends none. Conditional + // fetching degrades to unconditional rather than to broken. + if (!stored?.lastModified) return null; + + // THE REPAIR NET. A 304 skips the reconcile, and the reconcile is also what re-CREATES targets + // lost to anything else — a bad purge, a half-applied delete, a botched migration. Without a + // periodic unconditional pass a corpus could drift for as long as the origin left its sitemaps + // untouched, with nothing noticing. `lastRefreshed` means "entries last INGESTED" precisely so + // it can be measured against here; a 304 deliberately does not update it. + // + // Written as `!(elapsed < interval)` rather than `elapsed >= interval` so that a NaN — an + // unreadable or absent date — takes the unconditional branch instead of silently reading as + // "ingested at the epoch" or, worse, passing the comparison. + if (!(Date.now() - epochMsOf(stored.lastRefreshed) < fullPassInterval)) return null; + + return stored.lastModified; +}; diff --git a/packages/plugin/src/util/sitemapRun.js b/packages/plugin/src/util/sitemapRun.js index f6b0290..a34064f 100644 --- a/packages/plugin/src/util/sitemapRun.js +++ b/packages/plugin/src/util/sitemapRun.js @@ -102,6 +102,11 @@ export const createRefreshRun = ({ removedSampleCap = 20, failedCap = 100, depar duplicates: 0, deferred: 0, removed: 0, + // Documents the origin answered 304 to, so their entries were never re-parsed and their + // prune scan never ran. On a healthy corpus this is most of every pass between rebuilds; + // a steady ZERO where conditional fetching is enabled means the origin is not honouring + // If-Modified-Since and every pass is doing full work. + notModified: 0, sitemapsProcessed: 0, sitemapsDiscovered: 0, }; diff --git a/packages/plugin/test/sitemapConditional.test.js b/packages/plugin/test/sitemapConditional.test.js new file mode 100644 index 0000000..5c89ef9 --- /dev/null +++ b/packages/plugin/test/sitemapConditional.test.js @@ -0,0 +1,61 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { applyOptions } from '../src/config.js'; +import { conditionalValidatorFor } from '../src/util/sitemapConditional.js'; + +globalThis.logger ??= { debug() {}, info() {}, warn() {}, error() {} }; + +const HOUR = 60 * 60 * 1000; +const LM = 'Wed, 16 Sep 2026 06:08:37 GMT'; + +const setConditional = (overrides = {}) => + applyOptions({ sitemap: { conditional: { enabled: true, fullPassInterval: 24 * HOUR, ...overrides } } }); + +const stored = (over = {}) => ({ lastModified: LM, lastRefreshed: new Date(Date.now() - HOUR), ...over }); + +test('a recently-ingested document with a validator is fetched conditionally', () => { + setConditional(); + assert.equal(conditionalValidatorFor(stored(), false), LM); +}); + +// The repair net: a 304 skips the reconcile, and the reconcile is also what re-creates targets +// lost to anything else. Past the interval the document is re-ingested whether it changed or not. +test('a document not ingested within fullPassInterval is fetched unconditionally', () => { + setConditional(); + assert.equal(conditionalValidatorFor(stored({ lastRefreshed: new Date(Date.now() - 25 * HOUR) }), false), null); +}); + +test('no stored validator means unconditional — degrade to full, never to broken', () => { + setConditional(); + assert.equal(conditionalValidatorFor(stored({ lastModified: null }), false), null); + assert.equal(conditionalValidatorFor(stored({ lastModified: '' }), false), null); +}); + +test('a document never seen before is fetched unconditionally', () => { + setConditional(); + assert.equal(conditionalValidatorFor(null, false), null); + assert.equal(conditionalValidatorFor(undefined, false), null); +}); + +// An unreadable or absent date must not read as "ingested at epoch 0" and certainly not as +// "recently ingested": NaN fails the comparison, which falls through to a full fetch. +test('an unreadable lastRefreshed falls through to unconditional', () => { + setConditional(); + assert.equal(conditionalValidatorFor(stored({ lastRefreshed: 'not a date' }), false), null); + assert.equal(conditionalValidatorFor(stored({ lastRefreshed: null }), false), null); +}); + +test('revalidate always fetches unconditionally — the operator asked for a re-ingest', () => { + setConditional(); + assert.equal(conditionalValidatorFor(stored(), true), null); +}); + +test('disabling the feature restores unconditional fetching everywhere', () => { + setConditional({ enabled: false }); + assert.equal(conditionalValidatorFor(stored(), false), null); +}); + +test('fullPassInterval 0 makes every fetch unconditional', () => { + setConditional({ fullPassInterval: 0 }); + assert.equal(conditionalValidatorFor(stored({ lastRefreshed: new Date() }), false), null); +}); From f4f678409593bbe2a9f959b050d22a84c8ef0f75 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 17 Sep 2026 17:42:30 -0400 Subject: [PATCH 2/2] chore(plugin): renumber to v0.73.0 and rebase onto main The 0.67.0-0.72.0 train merged without this one (held back deliberately), so 0.69.0 would now be a downgrade. Rebased onto main and renumbered; the METRICS.md row is merged rather than replaced, so sitemap_departure_*, probe_trigger_queue_depth and sitemap_not_modified all survive. Co-Authored-By: Claude Opus 5 (1M context) --- package-lock.json | 2 +- packages/plugin/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 056d5ad..5888c1b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8570,7 +8570,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.72.0", + "version": "0.73.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/package.json b/packages/plugin/package.json index c9f114b..518e32b 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.72.0", + "version": "0.73.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0",