From 9ceb833b85d797abff5859732299826dc6ae356f Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Sep 2026 19:55:09 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(plugin):=20one=20render=20job=20per=20?= =?UTF-8?q?URL=20=E2=80=94=20the=20schedule=20row=20is=20the=20URL,=20one?= =?UTF-8?q?=20result=20schedules=20every=20device;=20v0.66.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RenderSchedule now holds ONE ROW PER URL (was one per `|` cacheKey). A claim hands the renderer one job naming every device in `deviceTypes.default`; a browser >= 1.23.0 renders them in turn and posts one result `{ id, url, deviceTypes, variants: [...] }`, and the plugin makes one scheduling decision for the URL. Pages stay per device. Why: two rows per URL drifted apart through every per-device path (render-now, revalidate, reconcile's fresh jitter, each retry lane), so "a split pair" was a normal production state — with the per-URL `strikes` counter fed twice per cycle and reset by whichever device happened to succeed. One result per URL is one strike per failed cycle, one page claim for the probe, an exact shared verification basis, half the schedule rows and half the reschedule writes. - `CacheKey.urlOf/deviceOf/isCacheKey` are the only readers of a schedule key; residency hashes the URL either way. The primary-key attribute keeps its name (`cacheKey`): Harper refuses to rename a populated table's primary key. - NO MIGRATION SWEEP. A pre-0.66.0 per-device row converts the first time it renders: its job renders exactly its device, its result writes the URL row and deletes the device row. Siblings fold within a cycle at no extra renders, and every reader tolerates both key shapes meanwhile. - `processDecodedJobResult` reasons about a LIST of variants (a legacy flat result becomes a one-variant list): a redirect on any device decides the URL, a genuine non-indexable verdict on any device suppresses it, rendered pages are stored per device, a failed or not-attempted device puts the URL in the retry lanes with the good pages kept, all-rendered reschedules once. - `renderNow` for a device outside the default set still writes a per-device row: a one-off render beside the rotation, retired without touching the URL row. For a default device it now pulls the URL row forward. - `claim` returns `deviceTypes` plus `deviceType` (the first) for renderers that predate the list; `job_result` accepts both envelopes and refuses a variants body whose contentLengths do not add up. - Admin: `POST /prerender_admin/schedule` takes `{ url }` or `{ cacheKey }`, the schedule read falls back to the per-device row, `revalidate` writes the URL row. Console unchanged. Deploy the render fleet (browser 1.23.0) first: an older renderer given a URL job renders only its first device — degraded, not broken. Co-Authored-By: Claude Fable 5.1 --- package-lock.json | 2 +- packages/plugin/METRICS.md | 2 +- packages/plugin/README.md | 95 +- packages/plugin/package.json | 2 +- packages/plugin/src/configSchema.js | 6 +- .../plugin/src/http_handlers/bot_request.js | 20 +- packages/plugin/src/metrics.js | 10 +- .../plugin/src/resources/PrerenderAdmin.js | 54 +- packages/plugin/src/resources/RenderQueue.js | 925 +++++++++++------- .../plugin/src/resources/RenderSchedule.js | 9 +- packages/plugin/src/resources/Target.js | 138 +-- packages/plugin/src/schemas/schema.graphql | 41 +- packages/plugin/src/util/cacheKey.js | 34 + packages/plugin/src/util/discoveredPurge.js | 4 +- packages/plugin/src/util/explain.js | 3 + .../plugin/src/util/invalidationReenqueue.js | 67 +- packages/plugin/src/util/orphanSweep.js | 15 +- packages/plugin/src/util/reconcile.js | 53 +- packages/plugin/src/util/renderSchedule.js | 47 +- packages/plugin/src/util/time.js | 20 +- packages/plugin/test/pageVerification.test.js | 11 +- packages/plugin/test/reconcile.test.js | 56 +- .../plugin/test/renderQueueRedirect.test.js | 56 +- .../plugin/test/renderQueueVariants.test.js | 569 +++++++++++ .../plugin/test/suppressionStatus.test.js | 47 +- packages/plugin/test/targetRevalidate.test.js | 35 +- 26 files changed, 1667 insertions(+), 654 deletions(-) create mode 100644 packages/plugin/test/renderQueueVariants.test.js diff --git a/package-lock.json b/package-lock.json index 9b93a59..52aada6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8570,7 +8570,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.64.1", + "version": "0.66.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/METRICS.md b/packages/plugin/METRICS.md index 9dc10d2..c2da282 100644 --- a/packages/plugin/METRICS.md +++ b/packages/plugin/METRICS.md @@ -128,7 +128,7 @@ reasoning behind it. | `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 — renders/hour = concurrency ÷ time_ms) and `outcome` (counter by outcome × detail, exactly one per posted result — the render-failure alert). | +| `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_*`, `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/changed/triggered/deferred/failed per pass, plus `probe_canary_trip` and `probe_invalidated`; `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_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). | diff --git a/packages/plugin/README.md b/packages/plugin/README.md index a0e7ec3..8685fb1 100644 --- a/packages/plugin/README.md +++ b/packages/plugin/README.md @@ -651,7 +651,7 @@ Checked against the reference deployment (4 nodes, 16 workers, Harper Pro 5.2.3) | `GET /p/` | Serve prerendered/cached HTML for a bot (cache hit or origin fetch) | | `POST /render_queue/pause` | Pause **this node's** queue | | `POST /render_queue/resume` | Clear this node's pause override | -| `POST /render_queue/claim` | Claim due render jobs (`{ "limit": N }`) | +| `POST /render_queue/claim` | Claim due render jobs (`{ "limit": N }`) — one job per URL | | `POST /render_queue/job_result` | Submit a render result (binary; `x-metadata-size` header) | | `GET/PUT/DELETE /RenderTarget/...` | Manage render targets | | `POST /RenderTarget` `{action:"revalidate"}` | Force re-render of matching targets | @@ -685,34 +685,34 @@ The super-user check is written out on every route rather than relying on Harper `allowRead`/`allowCreate` hooks, because those only run when `loadAsInstance !== false` — and this plugin's resources all set `loadAsInstance = false`. -| Method & path | Purpose | Gate | -| --------------------------------------- | ----------------------------------------------- | ------------ | -| `GET /prerender_admin[/]` | API index: what this is, where the UI lives | public | -| `GET /prerender_admin/session` | who am I | public | -| `POST /prerender_admin/login` | `{ username, password }` | public | -| `POST /prerender_admin/logout` | end the session | session | -| `GET /prerender_admin/overview` | nodes, counts, backlog snapshot | `super_user` | -| `GET /prerender_admin/config` | effective config, layers, overrides, warnings | `super_user` | -| `GET /prerender_admin/sitemaps` | root sitemaps + refresh state (never `entries`) | `super_user` | -| `GET /prerender_admin/pages` | `?prefix&cursor&limit` — page-cache browse | `super_user` | -| `GET /prerender_admin/page-content` | `?cacheKey` — one stored page, as `text/plain` | `super_user` | -| `GET /prerender_admin/unrouted` | this worker's unrouted-path tally (peek) | `super_user` | -| `GET /prerender_admin/analytics` | `?range` (ms) — bucketed metric series, cached | `super_user` | -| `GET /prerender_admin/invalidations` | active bulk-invalidation rows | `super_user` | -| `GET /prerender_admin/crawl-breadth` | `?days` — distinct URLs crawled per bot per day | `super_user` | -| `GET /prerender_admin/metrics` | the metric catalog (see METRICS.md) | `super_user` | -| `POST /prerender_admin/explain` | `{ url, deviceType }` → cache-key trace | `super_user` | -| `POST /prerender_admin/schedule` | `{ cacheKey }` → this node's local schedule row | `super_user` | -| `POST /prerender_admin/queue` | `{ scope, paused }` → pause control, or | `super_user` | -| | `{ action: "reset-claim-floor" }` (this node) | | -| `POST /prerender_admin/revalidate` | `{ url, deviceType }` → make one key due now | `super_user` | -| `POST /prerender_admin/reconcile` | start a schedule-repair sweep on this node | `super_user` | -| `POST /prerender_admin/sweep-orphans` | `{ dryRun?, maxDeletes? }` → key-rule orphans | `super_user` | -| `POST /prerender_admin/backlog` | recompute the backlog/histogram snapshot now | `super_user` | -| `POST /prerender_admin/sitemap` | `{ url, offset, limit }` → one sitemap's detail | `super_user` | -| `POST /prerender_admin/sitemap-refresh` | `{ url? }` → background walk of one/all roots | `super_user` | -| `GET /prerender_admin/change-probe` | probe rules + last pass records (this node) | `super_user` | -| `POST /prerender_admin/change-probe` | `{ action?: "sweep"\|"canary", dryRun? }` → run | `super_user` | +| Method & path | Purpose | Gate | +| --------------------------------------- | ------------------------------------------------ | ------------ | +| `GET /prerender_admin[/]` | API index: what this is, where the UI lives | public | +| `GET /prerender_admin/session` | who am I | public | +| `POST /prerender_admin/login` | `{ username, password }` | public | +| `POST /prerender_admin/logout` | end the session | session | +| `GET /prerender_admin/overview` | nodes, counts, backlog snapshot | `super_user` | +| `GET /prerender_admin/config` | effective config, layers, overrides, warnings | `super_user` | +| `GET /prerender_admin/sitemaps` | root sitemaps + refresh state (never `entries`) | `super_user` | +| `GET /prerender_admin/pages` | `?prefix&cursor&limit` — page-cache browse | `super_user` | +| `GET /prerender_admin/page-content` | `?cacheKey` — one stored page, as `text/plain` | `super_user` | +| `GET /prerender_admin/unrouted` | this worker's unrouted-path tally (peek) | `super_user` | +| `GET /prerender_admin/analytics` | `?range` (ms) — bucketed metric series, cached | `super_user` | +| `GET /prerender_admin/invalidations` | active bulk-invalidation rows | `super_user` | +| `GET /prerender_admin/crawl-breadth` | `?days` — distinct URLs crawled per bot per day | `super_user` | +| `GET /prerender_admin/metrics` | the metric catalog (see METRICS.md) | `super_user` | +| `POST /prerender_admin/explain` | `{ url, deviceType }` → cache-key trace | `super_user` | +| `POST /prerender_admin/schedule` | `{ url \| cacheKey }` → this node's schedule row | `super_user` | +| `POST /prerender_admin/queue` | `{ scope, paused }` → pause control, or | `super_user` | +| | `{ action: "reset-claim-floor" }` (this node) | | +| `POST /prerender_admin/revalidate` | `{ url, deviceType }` → make one URL due now | `super_user` | +| `POST /prerender_admin/reconcile` | start a schedule-repair sweep on this node | `super_user` | +| `POST /prerender_admin/sweep-orphans` | `{ dryRun?, maxDeletes? }` → key-rule orphans | `super_user` | +| `POST /prerender_admin/backlog` | recompute the backlog/histogram snapshot now | `super_user` | +| `POST /prerender_admin/sitemap` | `{ url, offset, limit }` → one sitemap's detail | `super_user` | +| `POST /prerender_admin/sitemap-refresh` | `{ url? }` → background walk of one/all roots | `super_user` | +| `GET /prerender_admin/change-probe` | probe rules + last pass records (this node) | `super_user` | +| `POST /prerender_admin/change-probe` | `{ action?: "sweep"\|"canary", dryRun? }` → run | `super_user` | The console is fully self-contained: its stylesheet, scripts and fonts are served from the same resource (the Ubuntu and Fira Code subsets are vendored with their licenses in @@ -843,8 +843,8 @@ claim floor, schedule repair — are plugin behavior.) startup log lines (empty security token, staging passthrough enabled, `renderNow` without a token). -The explainer also offers **Render this URL now**, which makes that one key due immediately. -It writes a single `RenderSchedule` row on purpose: the collection-level +The explainer also offers **Render this URL now**, which makes that one URL due immediately — +every device, in one job. It writes a single `RenderSchedule` row on purpose: the collection-level `RenderTarget.revalidate` takes a search target, and aimed at the whole registry it queues every target at once — at a million targets that is a self-inflicted render herd. @@ -913,16 +913,17 @@ count is never mistaken for "all clear". ### Cache-key orphans The mirror-image problem, and the one **changing a `cacheKey.*` option creates**. A target's -stored `url` _is_ the url-half of its cache key — `Target.put` derives the schedule rows from it -verbatim, and a render is stored under the schedule row's own key. Nothing re-canonicalizes. -So after a key-rule change, every target whose stored url is no longer what that url canonicalizes -to keeps its schedule rows and **renders forever into a key no request can produce**. +stored `url` _is_ its schedule key and the url-half of every page key — `Target.put` writes the +schedule row under it verbatim, and a render is stored under `|`. Nothing +re-canonicalizes. So after a key-rule change, every target whose stored url is no longer what that +url canonicalizes to keeps its schedule row and **renders forever into keys no request can produce**. Nothing else cleans them up. A sitemap refresh creates the target under the new key and merely _unlinks_ the old one (`sitemapUrl → null`), which does not touch its schedule. And the canonical verdict cannot retire them either: with the rule applied on both sides, the renderer folds the job url and the declared canonical alike and calls it `self`. Measured after enabling -`cacheKey.plusIsSpace` on a ~38k-url catalog corpus: ~20,200 urls re-keyed, ~40,400 schedule rows. +`cacheKey.plusIsSpace` on a ~38k-url catalog corpus: ~20,200 urls re-keyed (~40,400 schedule rows at +the time, when the table held one per device; one per URL since v0.66.0). Sizing that cost needs care, because the nominal interval is **not** the rate. `nextRenderTime` is stamped at _completion_, so a row rendered `L` behind its due time has its next render set `interval` @@ -1272,6 +1273,28 @@ render client ──claim──▶ render_queue ──jobs──▶ [headless re The render service is a separate process; see [`@harperfast/prerender-browser`](../browser). Its `RENDERER_BYPASS_*` settings must match this plugin's `origin.securityToken`. +**A job is one URL** (v0.66.0). `RenderSchedule` holds one row per URL, a claim hands the renderer one +job naming every device in `deviceTypes.default`, and the renderer (browser >= 1.23.0) renders them in +turn and posts one result — `{ id, url, deviceTypes, variants: [...] }` with the variants' bodies +concatenated behind the JSON. Pages are still stored per device (`PrerenderedPage` stays keyed by +`|`); the schedule is written once, which is what keeps a URL's devices aligned instead +of drifting apart through every per-device path (the retry lanes, render-now, reconcile). One result +also means one `strikes` increment per failed cycle, and one page claim for the change probe. The +precedence across a result's variants — a redirect on any device decides the URL, a genuine +non-indexable verdict on any device suppresses it, rendered pages are stored and a failed device puts +the URL in the retry lanes — is spelled out on `processDecodedJobResult` in +[`src/resources/RenderQueue.js`](src/resources/RenderQueue.js). + +**Upgrading from a per-device schedule.** Rows written before v0.66.0 are keyed `|` and +are **not migrated by a sweep**: each converts the first time it renders — its job renders exactly the +device its key names, its result writes the URL row and deletes the device row — so two siblings fold +into one URL row within a render cycle at no extra renders, and the table holds both shapes meanwhile +(every reader tolerates both). The `cacheKey` column name stays: Harper refuses to rename the primary +key of a populated table, so read it as "the schedule key". **Deploy the render fleet (browser >= +1.23.0) before this plugin version**: an older renderer handed a URL job renders only its first device. +`renderNow` for a device outside `deviceTypes.default` still writes a per-device row — a one-off +render of that device beside the rotation, stored and retired without touching the URL row. + ## Development ```sh diff --git a/packages/plugin/package.json b/packages/plugin/package.json index b03926c..e532787 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.64.1", + "version": "0.66.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/configSchema.js b/packages/plugin/src/configSchema.js index 69ed974..ddf4c19 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -824,9 +824,9 @@ export const configSchema = group('Prerender plugin configuration.', { 10, 'Per-node ceiling on accelerated REQUESTS per minute, shared across every worker on the node ' + '(one minute-bucketed counter in a shared buffer). One accelerated request writes at most one ' + - 'schedule row PER DEVICE ROW THE URL HAS — `deviceTypes.default` (two on this deployment), ' + - 'plus the served device when that one is merely `supported` — so the write ceiling is this ' + - 'number times those rows.\n\n' + + 'schedule row per schedule row the URL has — normally just its URL row; plus any pre-0.66.0 ' + + 'per-device row that has not yet converted, and a per-device row for the served device when ' + + 'that one is merely `supported` — so the write ceiling is this number times those rows.\n\n' + 'Sized so its CEILING is defensible, not just its typical. 10/min/node is 14,400 ' + 'requests/node/day ≈ 28,800 schedule writes ≈ 2.3MB of audit/node/day, about 7% of measured ' + 'spare fleet render capacity (~792,700 renders/day spare against a 1,710,936/day ceiling and ' + diff --git a/packages/plugin/src/http_handlers/bot_request.js b/packages/plugin/src/http_handlers/bot_request.js index c6d3b4d..130214e 100644 --- a/packages/plugin/src/http_handlers/bot_request.js +++ b/packages/plugin/src/http_handlers/bot_request.js @@ -442,17 +442,25 @@ async function renderNow({ url, cacheUrl, deviceType, cacheKey, request, routeSc // `false` is the true answer. const renderTarget = await Target.get({ id: cacheUrl, select: ['sitemapUrl', 'renderInterval', 'demandInterval'] }); - // Force an immediately-claimable, one-off schedule. No Target is created, so - // processJobResult won't reschedule it — and drops the schedule row once the result - // lands — keeping this a single render rather than a recurring target. Concurrent - // render-now requests for the same URL collapse onto this one row; the feature is - // authenticated, so we accept the small window where a spammed key can re-render. + // Force an immediately-claimable schedule row. No Target is created, so processJobResult + // won't reschedule it — and drops the schedule row once the result lands — keeping this a + // single render rather than a recurring target. Concurrent render-now requests for the same + // URL collapse onto this one row; the feature is authenticated, so we accept the small window + // where a spammed key can re-render. + // + // WHICH ROW. For a device in `deviceTypes.default` it is the URL row — the one the recurring + // rotation uses — so a render-now on a target-backed URL PULLS ITS NEXT RENDER FORWARD (every + // device, together, exactly as the rotation would have rendered it) rather than adding a + // one-device render beside it and de-aligning the pair. For a device that is merely + // `supported`, the URL row cannot carry it (a URL job renders the default set), so it is a + // per-device row: one render of that one device, stored and retired, the URL row untouched. // // Through the funnel, because "due at the current minute" is exactly the write a claim floor // would strand: on this node the funnel lowers the floor in-process, and on any other node — // which is ~75% of keys, since schedule rows are residency-pinned — the guard band is what // keeps the row above the owner's floor and therefore claimable. - await writeSchedule(cacheKey, { + const scheduleKey = config.deviceTypes.default.includes(deviceType) ? cacheUrl : cacheKey; + await writeSchedule(scheduleKey, { nextRenderTime: currentMinuteMs(), fromSitemap: !!renderTarget?.sitemapUrl, // PRESERVED WHEN THERE IS A TARGET, `null` WHEN THERE IS NOT — and the difference matters because diff --git a/packages/plugin/src/metrics.js b/packages/plugin/src/metrics.js index 01b550c..6d3685f 100644 --- a/packages/plugin/src/metrics.js +++ b/packages/plugin/src/metrics.js @@ -227,8 +227,9 @@ export const METRICS = Object.freeze({ kind: 'value', emittedBy: 'resources/RenderQueue.js', cadence: - 'per render result posted back by a browser worker: one `outcome` row always, one `time_ms` sample ' + - 'when the worker reported a duration', + 'per render result posted back by a browser worker: one `outcome` row always (a result is one URL — ' + + 'every device variant in it — since v0.66.0), and one `time_ms` sample per device variant the worker ' + + 'timed', summary: 'The render fleet, in one scan: how long each render took, and what became of it.', usefulFor: '`time_ms` is fleet capacity (renders/hour/pod = concurrency ÷ time_ms) and what a settle-tuning ' + @@ -280,6 +281,7 @@ export const METRICS = Object.freeze({ 'temporary', 'permanent', 'navigation', + 'not-attempted', ], description: 'time_ms: candidate (was cached) | non-candidate (suppression verdict) | unknown (worker posted ' + @@ -289,7 +291,9 @@ export const METRICS = Object.freeze({ 'worker posted an indexable verdict with nothing to store); suppressed: the browser’s ' + 'reason (noindex/canonical-mismatch/http-error/redirect-loop, else unspecified); auth-failure/' + 'transient: the status code; failed: the error phase (navigation = the document never arrived; ' + - 'unknown = pre-v1.16.0 worker posted no detail); redirect: landed-auth/landed-transient ' + + 'not-attempted = the worker was asked for this device and never started it — its lease ran short ' + + 'or it began draining — so the URL retries; unknown = pre-v1.16.0 worker posted no detail); ' + + 'redirect: landed-auth/landed-transient ' + '(destination answered 401/403 / 5xx-shaped), unrouted-destination (route list has no home for ' + 'it — a render is wasted every interval until fixed), non-indexable-destination (source ' + 'retired, destination suppressed), temporary (kept, strike counted), permanent (source retired ' + diff --git a/packages/plugin/src/resources/PrerenderAdmin.js b/packages/plugin/src/resources/PrerenderAdmin.js index 070aa42..58c7a8f 100644 --- a/packages/plugin/src/resources/PrerenderAdmin.js +++ b/packages/plugin/src/resources/PrerenderAdmin.js @@ -186,6 +186,25 @@ const localClaimFloor = (now) => { }; }; +/** + * This node's schedule row for a URL — or for a cacheKey, which names its URL. The URL row first; + * failing that, the per-device row under the cacheKey (the requested one, or the first configured + * device's when only a URL was given), which exists for one cycle after the v0.66.0 upgrade and for a + * deliberate one-device render. Node-local (`replicateFrom: false` — this table is residency-pinned, + * and an unowned point read takes Harper's untimed replication fetch); the caller says whether the + * answer is authoritative. + */ +const readScheduleRowLocal = async (urlOrCacheKey) => { + const select = ['cacheKey', 'nextRenderTime', 'fromSitemap', 'effectiveInterval']; + const url = CacheKey.urlOf(urlOrCacheKey); + const row = await RenderSchedule.get({ id: url, select }, { replicateFrom: false }); + if (row) return row; + const deviceKey = CacheKey.isCacheKey(urlOrCacheKey) + ? urlOrCacheKey + : CacheKey.toCacheKey({ url, deviceType: config.deviceTypes.default[0] }); + return RenderSchedule.get({ id: deviceKey, select }, { replicateFrom: false }); +}; + /** * One schedule row as the console shows it, including the two node-local questions only the * owner can answer: is this key currently leased to a renderer, and is its due time BELOW the @@ -202,6 +221,11 @@ const describeScheduleRow = (row, now) => { const lease = leaseInfo(row.cacheKey); return { ...row, + // The row's key is the URL (one row per URL, every device in one job). `perDevice` marks the + // exception: a pre-0.66.0 row that has not yet converted, or a deliberate one-device render — + // keyed by cacheKey and rendering only the device it names. + scheduleKey: row.cacheKey, + perDevice: CacheKey.isCacheKey(row.cacheKey), nextRenderTime: due ? at : null, dueInMs: due ? at - now : null, // True for EVERY in-flight render now, since a leased row keeps its past due time until the @@ -1042,7 +1066,9 @@ export class PrerenderAdmin extends Resource { } /** - * Make ONE url due for render now (one device — this writes exactly one schedule row). + * Make ONE url due for render now — its URL row, so every device renders together in one job. + * (`deviceType` still shapes the reported cacheKey; it no longer selects a row, since v0.66.0 + * schedules a URL as a whole.) * * Deliberately not `Target.revalidate`, which takes a search target: pointed at the * whole collection that revalidates the entire registry, and at 1M+ targets an accidental @@ -1083,7 +1109,7 @@ export class PrerenderAdmin extends Resource { const nextRenderTime = currentMinuteMs(); // The write is residency-routed, so this reaches the owning node from any node — and it // goes through the funnel, which lowers this node's claim floor to cover it. - await writeSchedule(cacheKey, { + await writeSchedule(canonicalUrl, { nextRenderTime, fromSitemap: !!target.sitemapUrl, // The target's real cadence, off the point read above — an admin rejoin should not cost the @@ -1103,6 +1129,7 @@ export class PrerenderAdmin extends Resource { return json({ cacheKey, canonicalUrl, + scheduleKey: canonicalUrl, nextRenderTime, scheduleOwnedBy: owner, wokeLocalConsumers: owner === server.hostname, @@ -1258,18 +1285,16 @@ export class PrerenderAdmin extends Resource { * proxying grants no authority the original caller lacked. */ static async scheduleRow(data) { - const cacheKey = data?.cacheKey; - if (typeof cacheKey !== 'string' || !cacheKey) { - return json({ error: 'cacheKey is required' }, 400); + // `url` or a `cacheKey` (the console and older callers send the latter): either names the URL + // whose row is wanted. The row is keyed by URL; a per-device row is read as the fallback, for the + // one cycle in which pre-0.66.0 rows still exist and for a deliberate one-device render. + const requested = typeof data?.url === 'string' && data.url ? data.url : data?.cacheKey; + if (typeof requested !== 'string' || !requested) { + return json({ error: 'url (or cacheKey) is required' }, 400); } const timedOutReads = []; - const row = await readWithTimeout('renderSchedule', timedOutReads, () => - RenderSchedule.get( - { id: cacheKey, select: ['cacheKey', 'nextRenderTime', 'fromSitemap'] }, - { replicateFrom: false } - ) - ); + const row = await readWithTimeout('renderSchedule', timedOutReads, () => readScheduleRowLocal(requested)); if (timedOutReads.length) return json({ error: 'local schedule read timed out' }, 504); @@ -1463,12 +1488,7 @@ export class PrerenderAdmin extends Resource { ], }) ), - readWithTimeout('renderSchedule', timedOutReads, () => - RenderSchedule.get( - { id: cacheKey, select: ['cacheKey', 'nextRenderTime', 'fromSitemap'] }, - { replicateFrom: false } - ) - ), + readWithTimeout('renderSchedule', timedOutReads, () => readScheduleRowLocal(cacheKey)), readWithTimeout('prerenderedPage', timedOutReads, () => PrerenderedPage.get({ id: cacheKey, diff --git a/packages/plugin/src/resources/RenderQueue.js b/packages/plugin/src/resources/RenderQueue.js index 76c1376..cfe4372 100644 --- a/packages/plugin/src/resources/RenderQueue.js +++ b/packages/plugin/src/resources/RenderQueue.js @@ -3,6 +3,7 @@ import { config, onConfigApplied } from '../config.js'; import { currentMinuteMs } from '../util/time.js'; import { QueueState } from './QueueState.js'; import { CacheKey } from '../util/cacheKey.js'; +import { sanitizeDeviceType } from '../util/device_type.js'; import { canonicalizeUrl } from '../util/url.js'; import { classifyPath, @@ -61,6 +62,148 @@ const legacyOutcome = (result) => { return 'error'; }; +const authShaped = (statusCode) => statusCode === 401 || statusCode === 403; +const transientShaped = (statusCode) => statusCode === 408 || statusCode === 429 || statusCode >= 500; +const hasContent = (variant) => variant.statusCode === 200 && !!variant.content; + +/** + * The devices a URL job renders: `config.deviceTypes.default`, floored at one device so an + * operator who empties the list still gets a render rather than a job the browser cannot act on. + */ +const defaultDeviceTypes = () => + config.deviceTypes.default.length ? [...config.deviceTypes.default] : [config.deviceTypes.supported[0]]; + +/** + * One posted result, whatever shape the browser used, as `{ rowKey, url, asked, variants }`. + * + * rowKey the schedule row the job came from (`id`, echoed verbatim by every browser version) — + * the URL, or a cacheKey for a per-device row (see `describeJob`). + * url the URL that row stands for. + * asked the devices the job asked for; a device here with no variant below did not render. + * variants what the browser posted, one per device attempted. + * + * A browser >= 1.23.0 answering a URL job posts `variants` (and `deviceTypes`, what it was asked). + * Every earlier shape is one device and the envelope IS the variant: its device is the row's when + * the row is per-device (every pre-0.66.0 result), else what the browser posted, else the first + * default device — which is exactly what `claim` puts in `deviceType` for a renderer that predates + * the list, so a pre-1.23.0 renderer given a URL job is attributed to the device it actually + * rendered. + */ +const normalizeJobResult = (result) => { + const rowKey = String(result.id); + const url = CacheKey.urlOf(rowKey); + if (Array.isArray(result.variants)) { + const variants = result.variants.map((variant) => ({ + ...variant, + deviceType: sanitizeDeviceType(variant?.deviceType), + })); + const asked = + Array.isArray(result.deviceTypes) && result.deviceTypes.length + ? result.deviceTypes.map(sanitizeDeviceType) + : variants.map((variant) => variant.deviceType); + return { rowKey, url, asked, variants }; + } + const deviceType = CacheKey.deviceOf(rowKey) ?? sanitizeDeviceType(result.deviceType ?? defaultDeviceTypes()[0]); + const { id, url: _url, deviceTypes, ...variant } = result; + return { rowKey, url, asked: [deviceType], variants: [{ ...variant, deviceType }] }; +}; + +/** + * What the row a job came from means for scheduling. + * + * perDevice the row is keyed by cacheKey and renders one device: a pre-0.66.0 row that has not + * converted yet, or a deliberate one-device render (`renderNow` for a device outside + * `deviceTypes.default`). + * fold this result may write the URL row. True for the URL row itself and for a per-device + * row of a DEFAULT device — that row is a fragment of the URL's rotation and converts + * into the URL row here. False for a non-default device: a one-off beside the rotation, + * which must not move the URL row. + * rowGone set by whichever branch deleted the row (or the whole target), so the end-of-result + * cleanup does not delete it a second time — Harper records a delete of an absent key in + * the audit log. + */ +const describeJob = (rowKey, url) => { + const device = CacheKey.deviceOf(rowKey); + const perDevice = device !== null; + return { rowKey, url, perDevice, fold: !perDevice || config.deviceTypes.default.includes(device), rowGone: false }; +}; + +/** + * A per-device row that has been folded into the URL row is deleted once its result is processed — + * unless the lease is being HELD (fast retry lane): the row is what the lease expiry re-grants, so it + * must stay for the retry, and converts on the result that finally reschedules it. + */ +const retireRowIfConverted = async (job, held) => { + if (held || job.rowGone || !job.perDevice) return; + await deleteSchedule(job.rowKey); + job.rowGone = true; +}; + +/** The variant a device the browser was asked for and never posted back reduces to. */ +const notAttemptedVariant = (deviceType) => ({ + deviceType, + outcome: 'error', + reason: 'not-attempted', + error: { + name: 'Error', + message: 'the renderer did not attempt this device (its lease ran short, or it began draining)', + phase: 'not-attempted', + }, + headers: {}, +}); + +/** + * Resolve one variant's outcome and keys, in place. Pure with respect to the database. + * + * cacheKey the page key this variant renders for. + * storeKey where its content goes — `cacheKey`, or the destination's key after a refile. + * outcome posted (browser >= v1.16.0) or inferred from the legacy signals. + * redirect set when `redirectedTo` canonicalizes to a DIFFERENT key: `{ redirectKey, + * destinationUrl, redirectPath, landedOn }`. A target whose page URL collapses back + * to the same key (trailing slash, param reorder, encoding) is not a redirect. + */ +const classifyVariant = (variant, job) => { + variant.headers ??= {}; + variant.cacheKey = CacheKey.toCacheKey({ url: job.url, deviceType: variant.deviceType }); + variant.storeKey = variant.cacheKey; + // The browser's OWN verdict about the landed document, captured before the domain coercion + // below — "the page said noindex" and "the host is outside our allowlist" must not be + // conflated: only the former means the destination was inspected. + variant.inspectedNonIndexable = variant.isIndexable === false; + + // The domain allowlist runs BEFORE the outcome is resolved so a legacy result for a foreign + // host still infers 'non-indexable' the way the old chain coerced it. + try { + const domain = URL.parse(variant.redirectedTo || job.url)?.hostname; + // Empty allowlist = allow all hosts. + if (config.domains.length && !config.domains.includes(domain)) variant.isIndexable = false; + } catch (e) { + logger.error(e, job.rowKey); + } + + variant.outcome = variant.outcome ?? legacyOutcome(variant); + + if (variant.redirectedTo) { + // The browser posts the RAW final page URL as `redirectedTo`. Canonicalize it the same way + // serving does — with the allowlist a bot READ of that target would use (route-aware) — so + // the rendered content is stored under the key that read computes. + const redirectKey = CacheKey.toCacheKey({ + deviceType: variant.deviceType, + url: canonicalizeUrl(variant.redirectedTo, queryAllowlistFor(variant.redirectedTo)), + }); + if (redirectKey !== variant.cacheKey) { + const redirectPath = URL.parse(variant.redirectedTo)?.pathname; + variant.redirect = { + redirectKey, + destinationUrl: CacheKey.extractUrl(redirectKey), + redirectPath, + landedOn: redirectPath === undefined ? PRERENDER : classifyPath(redirectPath).routeClass, + }; + } + } + return variant; +}; + /** * Resolve this node's desired pause intent from the replicated `QueueControl` table and * store it into the node-local queue flag; when not paused, derive empty/queued. Caller must @@ -160,10 +303,38 @@ export class RenderQueue extends Resource { static resume = ({ updatedBy } = {}) => RenderQueue.setPause({ scope: server.hostname, paused: null, updatedBy }); + /** + * The body of a posted result, decoded: the JSON envelope with its content bytes attached. + * + * Two shapes. A browser >= 1.23.0 answering a URL job posts `{ id, url, deviceTypes, variants: + * [...] }` followed by the variants' encoded bodies concatenated in order, each variant's + * `contentLength` saying how many of those bytes are its own — attached here as `variant.content`. + * Every earlier shape is one device: the envelope IS the variant and whatever follows the JSON is + * its body, attached as `result.content` exactly as before. `normalizeJobResult` folds the two + * into one. + * + * Throws on a variants body whose lengths do not account for exactly the bytes present: a result + * whose bytes cannot be attributed to devices must not be stored under any of them. + */ static decodeJobResult(buffer, metadataSize) { const metadataBuffer = buffer.subarray(0, metadataSize); const result = JSON.parse(metadataBuffer.toString('utf8')); - if (metadataBuffer.byteLength < buffer.byteLength) { + if (Array.isArray(result.variants)) { + let offset = metadataSize; + for (const variant of result.variants) { + const length = Number(variant?.contentLength) || 0; + if (length < 0 || offset + length > buffer.byteLength) { + throw new Error(`variant contentLength ${variant?.contentLength} overruns a ${buffer.byteLength}-byte body`); + } + if (length > 0) variant.content = buffer.subarray(offset, offset + length); + offset += length; + } + if (offset !== buffer.byteLength) { + throw new Error( + `variants account for ${offset - metadataSize} of the ${buffer.byteLength - metadataSize} body byte(s)` + ); + } + } else if (metadataBuffer.byteLength < buffer.byteLength) { result.content = buffer.subarray(metadataSize); } return result; @@ -188,12 +359,26 @@ export class RenderQueue extends Resource { ); } - const result = this.decodeJobResult(data, metadataSize); + let result; + try { + result = this.decodeJobResult(data, metadataSize); + } catch (e) { + // Same reasoning as above: a body that cannot be decoded, or whose variants do not account for + // its bytes, leaves nothing to release and nothing safe to store. A 4xx tells the browser not + // to retry the same bytes; the lease expires and the job is re-granted. + logger.error( + `[prerender] job_result rejected: ${e?.message ?? String(e)}. The lease will expire and the job be re-granted.` + ); + return new Response(JSON.stringify({ error: `undecodable job_result: ${e?.message ?? String(e)}` }), { + status: 400, + headers: { 'content-type': 'application/json; charset=utf-8' }, + }); + } - // THE key the lease was granted under, captured before anything can re-point `cacheKey`. - // The redirect refile below reassigns `cacheKey` to the destination, and releasing by that - // would leak the SOURCE's lease on every rendered client-side redirect — the source row - // would then pin the claim floor until the lease expired, every cycle, forever. + // THE key the lease was granted under. Everything below is keyed off the job description built + // from it rather than off this string, so nothing can re-point it (the redirect refile used to + // reassign the working key, and releasing by that would have leaked the SOURCE's lease on every + // rendered client-side redirect — the row would then pin the claim floor for a full lease). const claimKey = result.id; // Set true by the branches whose retry pacing IS the lease (see retryAfterFailure): they // must keep it, or the row — which still carries its original overdue due time now that @@ -215,10 +400,10 @@ export class RenderQueue extends Resource { // lease means the next pass re-grants it seconds later: an unpaced re-render loop against // whatever is throwing, at claim frequency rather than once per lease. // - // Reachable, not theoretical: `result.headers[...] = '1'` on a post with no headers - // object, a `PrerenderedPage.put`/`createBlob` failure, a `Target.get`/`Target.patch` - // rejection. Holding the lease paces the retry at `queue.jobLeaseTime`, exactly like the - // fast-retry lanes below, and the 500 is what says the result was not processed. + // Reachable, not theoretical: a `PrerenderedPage.put`/`createBlob` failure, a + // `Target.get`/`Target.patch` rejection. Holding the lease paces the retry at + // `queue.jobLeaseTime`, exactly like the fast-retry lanes below, and the 500 is what says + // the result was not processed. holdLease = true; throw e; } finally { @@ -227,181 +412,301 @@ export class RenderQueue extends Resource { } } - static async processDecodedJobResult(result, { holdLease }) { - let cacheKey = result.id; - const url = result.redirectedTo || result.url; - // Set when the render landed somewhere we don't serve from cache: reschedule as normal - // but store nothing, since the content belongs to a different URL than the key. - let discardContent = false; - - // The browser's OWN verdict about the landed document, captured before the domain - // coercion below — "the page said noindex" and "the host is outside our allowlist" - // must not be conflated: only the former means the destination was inspected. - const inspectedNonIndexable = result.isIndexable === false; - - // The domain allowlist runs BEFORE the outcome is resolved so a legacy result for a - // foreign host still infers 'non-indexable' the way the old chain coerced it. - try { - const domain = URL.parse(url)?.hostname; - // Empty allowlist = allow all hosts. - if (config.domains.length && !config.domains.includes(domain)) { - result.isIndexable = false; + /** + * ONE RESULT, ONE URL, ONE SCHEDULING DECISION. + * + * A result carries every device variant of one URL (a browser >= 1.23.0 answering a URL job), or + * one device (any earlier shape, and a per-device row's job). `normalizeJobResult` makes the two + * the same thing — a list of variants — and everything below reasons about the LIST: pages are + * stored per variant, because content is per device; the schedule is written once, because the + * rotation is per URL. That single write is what keeps a URL's devices aligned. When each device + * had its own row and its own result, every per-device path (the retry lanes, render-now, + * reconcile) moved one device and not the other, and "a split pair" became a normal production + * state — with a per-URL `strikes` counter fed twice per cycle, and the probe's per-URL page claim + * written by whichever device happened to render last. + * + * ── PRECEDENCE ACROSS VARIANTS ────────────────────────────────────────────────────────────── + * + * A URL is not "mostly rendered". The verdicts a single device used to deliver for the whole URL + * still do, in the order the old per-device code effectively applied them: + * + * 1. a REDIRECT the browser bailed on at navigation, on ANY device, decides the URL (a mobile + * 301 to an m-dot host retires the URL, as it always did — the desktop row went with it); + * 2. a genuine NON-INDEXABLE verdict on ANY device (noindex, canonical elsewhere, 404/410, + * other non-auth, non-transient http-error) SUPPRESSES the URL, whatever the other devices + * rendered — the pages go with the suppression, so none are stored; + * 3. otherwise every RENDERED variant's page is stored under its own cacheKey, and then + * 4. a FAILED variant (renderer error, auth-shaped or transient status, or a device the + * browser was asked for and did not attempt) puts the URL in the retry lanes — the stored + * pages stay servable, the whole URL re-renders on the lane's pacing, and the good side is + * rendered again so the pair stays aligned; + * 5. all rendered: reschedule the URL at its cadence, reset strikes, lift a suppression. + * + * ── WHICH ROW ──────────────────────────────────────────────────────────────────────────────── + * + * The row this job came from is `posted.id` — the URL (the normal case), or a cacheKey: a + * pre-0.66.0 per-device row that has not yet converted, or a deliberate one-device render + * (`renderNow` for a device outside `deviceTypes.default`). The URL's rotation lives on the URL + * row, so a per-device row for a DEFAULT device FOLDS into it here — its result writes the URL + * row and deletes the device row; two siblings converge on one row within a cycle at no extra + * renders — while a per-device row for a NON-default device is a one-off: its page is stored and + * the row deleted, and the URL row is not touched. `describeJob` decides which, once. + */ + static async processDecodedJobResult(posted, { holdLease: hold }) { + const { rowKey, url, asked, variants } = normalizeJobResult(posted); + const job = describeJob(rowKey, url); + let held = false; + const holdLease = () => { + held = true; + hold(); + }; + + for (const variant of variants) classifyVariant(variant, job); + + // A device the browser was asked for and did not post back is a render that did not happen: + // the lease ran short between variants, or the worker began draining. It fails like any other + // so the URL takes a retry lane for it, rather than the missing device silently keeping its old + // page until the next cadence. + for (const deviceType of asked) { + if (!variants.some((variant) => variant.deviceType === deviceType)) { + variants.push(classifyVariant(notAttemptedVariant(deviceType), job)); } - } catch (e) { - logger.error(e, result.id); } - const outcome = result.outcome ?? legacyOutcome(result); - - if (result.redirectedTo) { - const { deviceType } = CacheKey.parse(result.id); - - // The browser posts the RAW final page URL as `redirectedTo`. Canonicalize it the - // same way serving does — with the allowlist a bot READ of that target would use - // (route-aware) — so the rendered content is stored under the key that read computes. - const redirectKey = CacheKey.toCacheKey({ - deviceType, - url: canonicalizeUrl(result.redirectedTo, queryAllowlistFor(result.redirectedTo)), - }); + // One `time_ms` sample per variant the browser timed. A redirect bail gets its own lane so + // navigation-only renders do not read as fast full renders. + for (const variant of variants) { + if (typeof variant.renderTime !== 'number') continue; + const candidacy = + variant.outcome === 'redirected' && variant.redirect + ? 'redirect' + : typeof variant.isIndexable === 'boolean' + ? variant.isIndexable || hasContent(variant) + ? 'candidate' + : 'non-candidate' + : 'unknown'; + metrics.renderTime(variant.renderTime, variant.statusCode, candidacy); + } - // Only treat it as a redirect when the final URL canonicalizes to a DIFFERENT key. - // A target whose page URL collapses back to the same key (trailing slash, param - // reorder, encoding) is not a redirect — keep it under result.id and, crucially, - // do NOT delete its Target (which would drop it from the recurring rotation). - if (redirectKey !== result.id) { - const redirectPath = URL.parse(result.redirectedTo)?.pathname; - const landedOn = redirectPath === undefined ? PRERENDER : classifyPath(redirectPath).routeClass; - - // 'redirected' = the render ended without content: the browser (≥ v1.16.0) bailed - // at navigation on an HTTP redirect (statusCode = the first hop's 3xx), or a - // rendered-through client-side redirect landed on a page that produced nothing. - // Content rendered under this job's context (device profile, waitFor path scoping) - // could only be stored under a key it wasn't rendered for, so there is nothing to - // store — only scheduling to decide. - if (outcome === 'redirected') { - // The lane comes back up so the fast-retry branch inside it holds its lease just - // like the two in this function — one release point, three deciders. - const lane = await this.processRedirectResult(result, { - redirectKey, - landedOn, - redirectPath, - inspectedNonIndexable, - }); - if (lane === 'fast') holdLease(); - return; - } + // 1. A redirect the browser bailed on at navigation, or a rendered-through client-side redirect + // that produced nothing. Decided by `processRedirectResult` for the whole URL, exactly as one + // device's result decided it before. The lane comes back so the fast-retry branch inside it + // holds its lease like the two below — one release point, three deciders. + const bailed = variants.find((variant) => variant.outcome === 'redirected' && variant.redirect); + if (bailed) { + const lane = await this.processRedirectResult(bailed, job); + if (lane === 'fast') holdLease(); + await retireRowIfConverted(job, held); + return; + } - // A rendered result whose landed URL keys elsewhere (client-side redirect that - // produced a real page): keep the long-standing refile semantics. - if (outcome === 'rendered') { - if (landedOn === PRERENDER) { - // Retiring by URL takes the device siblings too — a page does not redirect - // for one device and serve for another. - logger.info(`Skipped prerendered url due to redirect: ${result.id} redirected to ${result.redirectedTo}`); - await Target.delete(CacheKey.extractUrl(result.id)); - cacheKey = redirectKey; - } else { - // The redirect target is a class we never serve from cache, so re-keying onto it - // would file the render where no read will ever look — and deleting this target - // would silently end the URL's rendering for good (see util/reconcile.js on how - // undiagnosable that state is). The route list may simply be incomplete, so - // report it and leave the target alone rather than destroy it on that evidence. - // The render is wasted each interval until the redirect or the routes are fixed. - logger.warn( - `Prerendered url ${result.id} redirected to ${result.redirectedTo}, which is ${landedOn} — ` + - `discarding the render and keeping the target (no key to store it under)` - ); - recordUnroutedPath(landedOn, redirectPath, 'redirect'); - discardContent = true; - } + // 2. A rendered result whose landed URL keys elsewhere (client-side redirect that produced a + // real page): the long-standing refile semantics, per variant. Onto a route we serve, the page + // is stored under the DESTINATION's key and the source is retired — once, by URL, taking its + // device siblings with it: a page does not redirect for one device and serve for another. + // Onto a class we never serve, the render is discarded and the target kept (see the warn). + let refiledTo = null; + for (const variant of variants) { + if (variant.outcome !== 'rendered' || !variant.redirect) continue; + if (variant.redirect.landedOn === PRERENDER) { + if (!refiledTo) { + logger.info(`Skipped prerendered url due to redirect: ${rowKey} redirected to ${variant.redirectedTo}`); + await Target.delete(url); + job.rowGone = true; + refiledTo = variant.redirect.destinationUrl; } + variant.storeKey = variant.redirect.redirectKey; + variant.refiled = true; + } else { + // The redirect target is a class we never serve from cache, so re-keying onto it + // would file the render where no read will ever look — and deleting this target + // would silently end the URL's rendering for good (see util/reconcile.js on how + // undiagnosable that state is). The route list may simply be incomplete, so + // report it and leave the target alone rather than destroy it on that evidence. + // The render is wasted each interval until the redirect or the routes are fixed. + logger.warn( + `Prerendered url ${rowKey} redirected to ${variant.redirectedTo}, which is ${variant.redirect.landedOn} — ` + + `discarding the render and keeping the target (no key to store it under)` + ); + recordUnroutedPath(variant.redirect.landedOn, variant.redirect.redirectPath, 'redirect'); + variant.discardContent = true; } } - const hasContent = result.statusCode === 200 && result.content; - - if (typeof result.renderTime === 'number') { - metrics.renderTime( - result.renderTime, - result.statusCode, - typeof result.isIndexable === 'boolean' - ? result.isIndexable || hasContent - ? 'candidate' - : 'non-candidate' - : 'unknown' + // 3. A verdict about the page itself, on any device. `reason` (browser >= v1.16.0) says WHY: + // 'noindex', 'canonical-mismatch', 'http-error', 'redirect-loop', or (>= v1.17.0) + // 'canonical-variant' — the canonical names this very document RE-SPELLED as a different + // cache key. Suppressed identically; the split keeps a wave of duplicate spellings legible. + // + // Note which urls can reach here at all: a sitemap-listed one is serialized even when + // non-indexable, so its variant arrives with content and `rendered` wins — the declared + // corpus is structurally out of this branch's reach, and only urls we DISCOVERED can be + // suppressed by a canonical verdict. + // + // EXCEPT 401/403 and 408/429/5xx, which are failures (step 4), not verdicts: an auth-shaped + // error is almost never a statement about the page — a broken renderer credential, an + // origin bot-mitigation rule change, an origin auth outage — and a transient one means the + // origin failed to serve the page, not that it disavowed it. Striking toward deletion would + // suppress (and after maxStrikes DELETE) swathes of healthy targets exactly when such a + // failure hits everything at once. Keep the target, keep its cached pages, retry. + const verdict = variants.find( + (variant) => + variant.outcome === 'non-indexable' && !authShaped(variant.statusCode) && !transientShaped(variant.statusCode) + ); + if (verdict) { + metrics.renderOutcome('suppressed', verdict.reason ?? 'unspecified'); + // info, not warn: a suppression is a normal verdict (the page declared itself + // non-indexable) and it self-heals on its own recheck cadence. The alertable event is + // MASS suppression, which is the render_outcome counter's job. + logger.info( + `Suppressing prerendered url: ${url} (${verdict.deviceType}${verdict.reason ? `, ${verdict.reason}` : ''})` ); + // Suppress writes the URL row (its recheck) and drops every device's page; the verdict + // SUPPRESSES the target rather than deleting it — see Target.suppress, which also grades + // http-error verdicts by status (404/410 recheck less, die sooner). + await Target.suppress(url, { reason: verdict.reason, statusCode: verdict.statusCode }); + await retireRowIfConverted(job, held); + return; } - if (outcome === 'rendered') { - // One render outcome per posted result (the redirect path emits its own inside - // processRedirectResult). `refiled` = the client-side-redirect re-key above moved the - // result onto the destination's cache key; `discarded` = it landed on a class we never - // serve and the content was dropped; `no-content` = a legacy worker's isIndexable-only - // result (legacyOutcome calls it rendered, but there is nothing to store). - metrics.renderOutcome( - 'rendered', - discardContent ? 'discarded' : cacheKey !== result.id ? 'refiled' : result.content ? 'stored' : 'no-content' - ); - const url = CacheKey.extractUrl(cacheKey); - const renderTarget = await Target.get({ - id: url, - select: ['renderInterval', 'sitemapUrl', 'state', 'strikes', 'demandInterval'], - }); - const renderInterval = renderTarget?.renderInterval; - - // Schedule the next render relative to when THIS one completed (now), not a - // fixed wall-clock time — so renders stay spread across the interval instead of - // realigning into a daily herd, and the cadence self-paces to fleet throughput. - // Cadence precedence: matched route's renderInterval, else the target's stored - // interval (sitemap changefreq / explicit API write; invalid values — including - // NaN from an arbitrary API PUT — are rejected), else the default. Resolved here - // on every cycle, so a route-cadence config change applies on each URL's next - // render without touching stored rows. - const base = resolveRenderInterval(url, renderInterval); - // The demand ladder reallocates cadence WITHIN `base` (which stays the ceiling) by - // whether bots actually visit this URL. Off / dry-run / cold filter all return `base` - // unchanged, so this is a no-op until deliberately switched on. - const demand = decideInterval(url, base, renderTarget?.demandInterval); - const interval = demand.interval; - // The cached page expires when the next render is due; the swrTtl window then keeps - // it served while the re-render lands, so render latency up to swrTtl never causes - // a cache miss. - const nextRenderTime = currentMinuteMs() + interval; - - if (result.content && !discardContent) { - result.headers['x-harper-rendered'] = '1'; - // ONE timestamp for the page and for the claim recorded alongside it. Taken here rather - // than at each use because `recordPageClaim` stores it as the basis a per-URL verification - // certifies, and the serve path tests a device key with `lastCached >= basisAt` — two - // separate `Date.now()` calls milliseconds apart would make the page fail its own test. - const cachedAt = Date.now(); - // What this render CLAIMS, for the probe to compare the origin against on its next - // pass. Best-effort and awaited only for its (node-local) write: see recordPageClaim - // — a render must not fail because a probe optimisation could not be recorded. - await recordPageClaim(url, result.structuredOffers, cachedAt); - await databases.page_cache.PrerenderedPage.put(cacheKey, { - statusCode: result.statusCode, + // The scheduling target: the URL's own, or — after a refile — the destination's, which is what + // the old per-key code consulted once it had re-pointed the working key at the destination. + const scheduleUrl = refiledTo ?? url; + const scheduleJob = refiledTo ? describeJob(refiledTo, refiledTo) : job; + const renderTarget = await Target.get({ + id: scheduleUrl, + select: ['renderInterval', 'sitemapUrl', 'state', 'strikes', 'demandInterval'], + }); + + // Schedule the next render relative to when THIS one completed (now), not a fixed wall-clock + // time — so renders stay spread across the interval instead of realigning into a daily herd, + // and the cadence self-paces to fleet throughput. Cadence precedence: matched route's + // renderInterval, else the target's stored interval (sitemap changefreq / explicit API write; + // invalid values — including NaN from an arbitrary API PUT — are rejected), else the default. + // Resolved here on every cycle, so a route-cadence config change applies on each URL's next + // render without touching stored rows. + const base = resolveRenderInterval(scheduleUrl, renderTarget?.renderInterval); + // The demand ladder reallocates cadence WITHIN `base` (which stays the ceiling) by whether bots + // actually visit this URL. Off / dry-run / cold filter all return `base` unchanged. + const demand = decideInterval(scheduleUrl, base, renderTarget?.demandInterval); + const interval = demand.interval; + // The cached pages expire when the next render is due; the swrTtl window then keeps them served + // while the re-render lands, so render latency up to swrTtl never causes a cache miss. + const nextRenderTime = currentMinuteMs() + interval; + + // Store every rendered variant's page — before the retry decision, so a device that rendered + // is served fresh even while the URL retries for a sibling that did not. + const rendered = variants.filter((variant) => variant.outcome === 'rendered'); + // Content is what gets stored, whatever the status the browser reported beside it — the same + // test the per-key path applied. (`hasContent`, with its 200 check, is the metrics candidacy.) + const stored = rendered.filter((variant) => !!variant.content && !variant.discardContent); + if (stored.length) { + // ONE timestamp for every page and for the claim recorded alongside them. Taken once rather + // than per use because `recordPageClaim` stores it as the basis a per-URL verification + // certifies, and the serve path tests each device key with `lastCached >= basisAt` — a + // device stamped milliseconds later than the claim would fail its own test. Sharing it + // across the variants is also what makes the pair's `basisAt` exact rather than aligned. + const cachedAt = Date.now(); + // What this render CLAIMS, for the probe to compare the origin against on its next pass — + // once per URL, from the first variant that ran the extraction (the offers are a property of + // the document, not of the viewport: measured, desktop and mobile agreed byte-for-byte in + // 39/40 samples, the exception being a pair rendered 41h apart). Best-effort and awaited only + // for its (node-local) write: a render must not fail because a probe optimisation could not + // be recorded. + const claiming = stored.find((variant) => variant.structuredOffers !== undefined) ?? stored[0]; + await recordPageClaim(scheduleUrl, claiming.structuredOffers, cachedAt); + for (const variant of stored) { + variant.headers['x-harper-rendered'] = '1'; + await databases.page_cache.PrerenderedPage.put(variant.storeKey, { + statusCode: variant.statusCode, lastCached: cachedAt, - content: createBlob(result.content), - headers: JSON.stringify(result.headers), + content: createBlob(variant.content), + headers: JSON.stringify(variant.headers), expiresAt: nextRenderTime, - isIndexable: typeof result.isIndexable === 'boolean' ? result.isIndexable : null, + isIndexable: typeof variant.isIndexable === 'boolean' ? variant.isIndexable : null, }); } + } + + // 4. A failed variant puts the URL in the retry lanes: fast retries on the held lease, then + // escalation to a backed-off due time (`retryAfterFailure`). The stored pages above stay + // servable meanwhile. One outcome emit per result, for the class the worst variant fell in — + // auth-shaped first (it is the one that signals a broken credential), then transient, then a + // plain failure — and one log line per failed variant, at the level that class warrants. + const failed = variants.filter( + (variant) => + variant.outcome === 'error' || + (variant.outcome === 'non-indexable' && (authShaped(variant.statusCode) || transientShaped(variant.statusCode))) + ); + if (failed.length) { + const auth = failed.find((variant) => authShaped(variant.statusCode)); + const transient = failed.find((variant) => transientShaped(variant.statusCode)); + if (auth) metrics.renderOutcome('auth-failure', auth.statusCode); + else if (transient) metrics.renderOutcome('transient', transient.statusCode); + else metrics.renderOutcome('failed', failed[0].error?.phase ?? 'unknown'); + for (const variant of failed) { + const where = `${url} (${variant.deviceType})`; + if (authShaped(variant.statusCode)) { + logger.error( + `Prerender got ${variant.statusCode} for ${where} — auth-shaped, NOT suppressing. ` + + `If these are widespread, check the renderer's origin-bypass credential and the CDN/origin access rules.` + ); + } else if (transientShaped(variant.statusCode)) { + // info, not warn: by-design tolerance of an origin blip. The aggregate (a transient + // BURST is origin trouble) is render_outcome's job, not a per-URL log flood's. + logger.info(`Prerender got transient ${variant.statusCode} for ${where} — keeping target and cached page`); + } else { + // The browser posts `reason` and the failed attempt's error (name/message/phase) since + // v1.16.0; without them this can only say "unknown". `phase: 'navigation'` means the + // document never arrived (slow/refusing origin) — a different problem from a render + // that failed mid-settle; 'not-attempted' means the browser never started this device. + const detail = variant.error + ? ` — ${variant.error.name}${variant.error.phase ? ` [${variant.error.phase}]` : ''}: ${variant.error.message}` + : ''; + logger.warn(`Prerender failed for ${where} (${variant.reason || 'no reason reported'})${detail}`); + } + } + // This branch used to hold the lease unconditionally and forever for a renderer failure — + // no strike, no escalation — so a permanently-crashing render re-rendered once per + // `queue.jobLeaseTime` for the life of the target. The waste was never the renders; it was + // the CLAIM FLOOR, which a held lease pins at its row's due minute. Escalating returns + // 'slow', which releases the lease and lets the floor advance. + if ((await this.retryAfterFailure(scheduleJob)) === 'fast') holdLease(); + await retireRowIfConverted(job, held); + return; + } - if (renderTarget) { - // A target owns this schedule → recurring. Reschedule relative to completion - // using the resolved interval (so a target lacking an explicit renderInterval - // falls back to the default instead of getting stuck re-claiming every lease - // period). Refresh fromSitemap from the live target so it self-corrects if the - // URL has since left its sitemap. - // - // This is the highest-volume schedule write in the system, and it writes - // `now + interval` — i.e. FORWARD. The funnel's floor lowering is a CAS-min, so this - // path costs one atomic load and moves the floor not at all. That is load-bearing: a - // lowering on every completed render would rewind the floor to the current minute - // continuously and the whole 14× seek win would evaporate. - await writeSchedule(cacheKey, { + // 5. Every variant rendered. One render outcome per posted result: `refiled` = the client-side + // redirect re-key above moved the pages onto the destination's keys; `discarded` = it landed on + // a class we never serve and the content was dropped; `no-content` = a legacy worker's + // isIndexable-only result (legacyOutcome calls it rendered, but there is nothing to store). + metrics.renderOutcome( + 'rendered', + stored.length + ? stored.some((variant) => variant.refiled) + ? 'refiled' + : 'stored' + : rendered.some((variant) => variant.discardContent) + ? 'discarded' + : 'no-content' + ); + + if (renderTarget) { + // A target owns this URL → recurring. Reschedule relative to completion using the resolved + // interval (so a target lacking an explicit renderInterval falls back to the default instead + // of getting stuck re-claiming every lease period). Refresh fromSitemap from the live target + // so it self-corrects if the URL has since left its sitemap. + // + // This is the highest-volume schedule write in the system, and it writes `now + interval` — + // i.e. FORWARD. The funnel's floor lowering is a CAS-min, so this path costs one atomic load + // and moves the floor not at all. That is load-bearing: a lowering on every completed render + // would rewind the floor to the current minute continuously and the whole 14× seek win would + // evaporate. + // + // A one-device render (a per-device row for a non-default device) does NOT reschedule: the + // URL's rotation is on the URL row, and this result was an extra render beside it. + if (scheduleJob.fold) { + await writeSchedule(scheduleUrl, { nextRenderTime, fromSitemap: !!renderTarget.sitemapUrl, // `interval`, i.e. the rung `decideInterval` JUST chose — not the route ceiling. This is @@ -411,154 +716,83 @@ export class RenderQueue extends Resource { effectiveInterval: interval, }); - // Persist the rung ONLY on an actual move. 'held' must not write even when the - // stored field is absent — absence already resolves to the base ceiling, so writing - // it would be redundant, and on first evaluation it would be a corpus-wide storm of - // replicated Target patches (~one per render for a full cycle), in dry-run too. - // A converged corpus therefore pays nothing here, on the system's hottest path. + // Persist the rung ONLY on an actual move. 'held' must not write even when the stored field + // is absent — absence already resolves to the base ceiling, so writing it would be + // redundant, and on first evaluation it would be a corpus-wide storm of replicated Target + // patches (~one per render for a full cycle), in dry-run too. A converged corpus therefore + // pays nothing here, on the system's hottest path. if (demand.action === 'promoted' || demand.action === 'demoted') { - await Target.patch(url, { demandInterval: demand.level }); + await Target.patch(scheduleUrl, { demandInterval: demand.level }); } - - // A suppressed URL that rendered indexable again has healed — put it back in - // normal rotation, so the recheck cadence stops and discovery may see it again. - if (renderTarget.state === 'suppressed' && result.isIndexable === true) { - logger.info(`Prerendered url ${url} is indexable again — lifting its suppression`); - await Target.reactivate(url); - } else if (renderTarget.state !== 'suppressed' && renderTarget.strikes > 0) { - // Strikes are CONSECUTIVE failures by definition: a successful render resets the - // count, so redirect blips months apart never accumulate toward retirement. - // Guarded by strikes > 0 — the hot path (healthy target, no strikes) pays no - // extra write. - await Target.patch(url, { strikes: 0 }); - } - } else { - // No target owns this schedule: it's a one-off (render-now) or an orphaned - // row. Nothing sets a recurring cadence, so drop the schedule instead of - // leaving it to be re-claimed when the lease expires. - // - // The delete does NOT release the key's lease (see util/renderSchedule.js): the slot - // keeps holding the claim floor at this row's old due minute until it expires. That - // is the conservative direction — releasing here would let the floor advance past a - // row whose result may still be arriving from a duplicate renderer. - await deleteSchedule(cacheKey); } - } else if (outcome === 'non-indexable') { - // `reason` (browser ≥ v1.16.0) says WHY: 'noindex', 'canonical-mismatch', 'http-error', - // or 'redirect-loop' — the difference between "the site asked us not to" and "the - // render is broken", which read identically without it. Browser ≥ v1.17.0 adds - // 'canonical-variant': the canonical names this very document RE-SPELLED as a - // different cache key, so the target duplicates one we already render rather than - // being a page that disowns itself. Suppressed identically — the split exists so a - // wave of duplicate spellings is legible as such rather than reading as an origin - // that stopped believing in its own pages. - // - // Note which urls can reach here at all: a sitemap-listed one is serialized even when - // non-indexable, so its result arrives with content and `rendered` wins the outcome - // above — the declared corpus is structurally out of this branch's reach, and only - // urls we DISCOVERED can be suppressed by a canonical verdict. - // - // The verdict SUPPRESSES the - // target (state + recheck schedule) rather than deleting it — see Target.suppress, - // which also grades http-error verdicts by status (404/410 recheck less, die sooner). - // - // EXCEPT 401/403: an auth-shaped error is almost never a statement about the page — - // it's a broken renderer credential, an origin bot-mitigation rule change, or an - // origin auth outage. Striking toward deletion would suppress (and after maxStrikes - // DELETE) swathes of healthy targets exactly when such a failure hits everything at - // once. Keep the target, keep its cached page, retry via retryAfterFailure. - if (result.statusCode === 401 || result.statusCode === 403) { - metrics.renderOutcome('auth-failure', result.statusCode); - logger.error( - `Prerender got ${result.statusCode} for ${cacheKey} — auth-shaped, NOT suppressing. ` + - `If these are widespread, check the renderer's origin-bypass credential and the CDN/origin access rules.` - ); - if ((await this.retryAfterFailure(cacheKey)) === 'fast') holdLease(); - } else if (result.statusCode === 408 || result.statusCode === 429 || result.statusCode >= 500) { - // Transient-shaped: the origin failed to serve the page, it didn't disavow it. - // Suppressing would delete the last good cached page and park the URL for the - // recheck interval over what may be one bad minute at the origin — keep both - // and retry via retryAfterFailure (fast first, then the target's cadence). - metrics.renderOutcome('transient', result.statusCode); - // info, not warn: by-design tolerance of an origin blip. The aggregate (a transient - // BURST is origin trouble) is render_outcome's job, not a per-URL log flood's. - logger.info(`Prerender got transient ${result.statusCode} for ${cacheKey} — keeping target and cached page`); - if ((await this.retryAfterFailure(cacheKey)) === 'fast') holdLease(); - } else { - metrics.renderOutcome('suppressed', result.reason ?? 'unspecified'); - // info, not warn: a suppression is a normal verdict (the page declared itself - // non-indexable) and it self-heals on its own recheck cadence. The alertable event is - // MASS suppression, which is the render_outcome counter's job. - logger.info(`Suppressing prerendered url: ${cacheKey}${result.reason ? ` (${result.reason})` : ''}`); - await Target.suppress(CacheKey.extractUrl(cacheKey), { - reason: result.reason, - statusCode: result.statusCode, - }); + + // A suppressed URL that rendered indexable again has healed — put it back in normal + // rotation, so the recheck cadence stops and discovery may see it again. Every variant + // rendered, so any device's indexable verdict is the URL's. + if (renderTarget.state === 'suppressed' && rendered.some((variant) => variant.isIndexable === true)) { + logger.info(`Prerendered url ${scheduleUrl} is indexable again — lifting its suppression`); + await Target.reactivate(scheduleUrl); + } else if (renderTarget.state !== 'suppressed' && renderTarget.strikes > 0) { + // Strikes are CONSECUTIVE failures by definition: a successful render resets the + // count, so redirect blips months apart never accumulate toward retirement. + // Guarded by strikes > 0 — the hot path (healthy target, no strikes) pays no + // extra write. + await Target.patch(scheduleUrl, { strikes: 0 }); } - } else { - // The browser posts `reason` and the failed attempt's error (name/message/phase) since - // v1.16.0; without them this branch can only say "unknown". `phase: 'navigation'` - // means the document never arrived (slow/refusing origin) — a different problem from - // a render that failed mid-settle. - const detail = result.error - ? ` — ${result.error.name}${result.error.phase ? ` [${result.error.phase}]` : ''}: ${result.error.message}` - : ''; - metrics.renderOutcome('failed', result.error?.phase ?? 'unknown'); - logger.warn(`Prerender failed for ${cacheKey} (${result.reason || 'no reason reported'})${detail}`); - // Same lane as every other non-suppressing failure: fast retries on the held lease, - // then escalation to a backed-off due time. This branch used to hold the lease - // unconditionally and forever — no strike, no escalation — so a permanently-crashing - // render re-rendered once per `queue.jobLeaseTime` for the life of the target. + } else if (!job.rowGone) { + // No target owns this URL: a one-off (render-now) or an orphaned row. Nothing sets a + // recurring cadence, so drop the row this job came from instead of leaving it to be + // re-claimed when the lease expires. // - // The waste was never the renders (measured: 7 such keys per node, ~42 renders/hr - // against a fleet doing 87,660). It was the CLAIM FLOOR: a held lease pins the floor at - // its row's due minute, so a handful of permanently-failing rows held the floor 12+ - // hours in the past indefinitely, and every claim scan seeked from there across dead - // index entries. Escalating returns 'slow', which releases the lease and lets the floor - // advance — that is the point of this change, not the saved render capacity. - // - // `retryAfterFailure` does its own target read (and drops a targetless render-now / - // orphaned row), so the redundant existence check that used to guard this branch is gone. - if ((await this.retryAfterFailure(cacheKey)) === 'fast') holdLease(); + // The delete does NOT release the key's lease (see util/renderSchedule.js): the slot keeps + // holding the claim floor at this row's old due minute until it expires. That is the + // conservative direction — releasing here would let the floor advance past a row whose + // result may still be arriving from a duplicate renderer. + await deleteSchedule(job.rowKey); + job.rowGone = true; } + await retireRowIfConverted(job, held); } /** * A render that ended as a redirect with no content. Usually the browser bailed at - * navigation on an HTTP redirect (`result.statusCode` is the FIRST hop's 3xx — the origin's + * navigation on an HTTP redirect (`variant.statusCode` is the FIRST hop's 3xx — the origin's * statement about the job URL itself); a client-side redirect that rendered through to a * page that produced nothing lands here too (statusCode 200, permanence unknowable). What's * decided is what happens to the source target, and whether the destination becomes a * target of its own so it gets rendered under its own job context instead of being cached * from a render that ran as another URL. * + * `variant` is the device that observed the redirect; `job` names the URL and the row. The + * decision is about the URL — a page does not redirect for one device and serve for another — + * so it retires or reschedules the whole URL, as one device's result always did. + * * Returns the retry lane when it took one (`'fast'`/`'slow'`/`'dropped'`), so the caller — the * single lease-release point — knows whether this result's pacing is the lease itself. */ - static async processRedirectResult(result, { redirectKey, landedOn, redirectPath, inspectedNonIndexable }) { - if (typeof result.renderTime === 'number') { - metrics.renderTime(result.renderTime, result.statusCode, 'redirect'); - } + static async processRedirectResult(variant, job) { + const { redirectKey, destinationUrl, landedOn, redirectPath } = variant.redirect; + const { url: sourceUrl, rowKey } = job; - // Same status rules as processJobResult, applied BEFORE anything retires or strikes + // Same status rules as the failure branch, applied BEFORE anything retires or strikes // the source. Only a rendered-through client-side redirect can carry these statuses // (a bail-at-nav result posts the first hop's 3xx), so `statusCode` here is the LANDED // document's: an auth-shaped or transient-shaped landing is a credential/origin // problem, not a verdict on either URL. Without this, a page whose client-side // redirect lands on a 401/403 would delete its source target on the FIRST such result - // (via the inspectedNonIndexable branch below) — the exact mass-deletion the - // processJobResult guard exists to prevent. - const authShaped = result.statusCode === 401 || result.statusCode === 403; - const transientShaped = result.statusCode === 408 || result.statusCode === 429 || result.statusCode >= 500; - if (authShaped || transientShaped) { - metrics.renderOutcome('redirect', authShaped ? 'landed-auth' : 'landed-transient'); + // (via the inspectedNonIndexable branch below) — the exact mass-deletion the failure + // branch's guard exists to prevent. + const auth = authShaped(variant.statusCode); + const transient = transientShaped(variant.statusCode); + if (auth || transient) { + metrics.renderOutcome('redirect', auth ? 'landed-auth' : 'landed-transient'); // error for auth (credential/mitigation trouble), info for transient (origin blip) — - // same split as processJobResult's non-redirect branches. - logger[authShaped ? 'error' : 'info']( - `Prerendered url ${result.id} redirected to ${result.redirectedTo}, which returned ${result.statusCode} — ` + - `${authShaped ? 'auth-shaped' : 'transient'}, keeping the target` + // same split as the failure branch. + logger[auth ? 'error' : 'info']( + `Prerendered url ${rowKey} redirected to ${variant.redirectedTo}, which returned ${variant.statusCode} — ` + + `${auth ? 'auth-shaped' : 'transient'}, keeping the target` ); - return await this.retryAfterFailure(result.id); + return await this.retryAfterFailure(job); } if (landedOn !== PRERENDER) { @@ -570,15 +804,15 @@ export class RenderQueue extends Resource { // permanently redirected, and recordRedirectStrike retires it after maxStrikes. metrics.renderOutcome('redirect', 'unrouted-destination'); logger.warn( - `Prerendered url ${result.id} redirected (${result.statusCode}) to ${result.redirectedTo}, which is ` + + `Prerendered url ${rowKey} redirected (${variant.statusCode}) to ${variant.redirectedTo}, which is ` + `${landedOn} — keeping the target (no key to schedule the destination under)` ); recordUnroutedPath(landedOn, redirectPath, 'redirect'); - await this.recordRedirectStrike(result.id, `to unserved ${landedOn} destination`); + await this.recordRedirectStrike(job, `to unserved ${landedOn} destination`); return; } - if (inspectedNonIndexable) { + if (variant.inspectedNonIndexable) { // The landed document was actually loaded and inspected (a rendered-through // client-side redirect) and it is non-indexable: the source now leads to a page we // would never cache. Retire the source and suppress the destination, so neither @@ -587,54 +821,53 @@ export class RenderQueue extends Resource { // foreign row would be registry noise nothing ever reads.) metrics.renderOutcome('redirect', 'non-indexable-destination'); logger.info( - `Prerendered url ${result.id} redirected to non-indexable ${result.redirectedTo}` + - `${result.reason ? ` (${result.reason})` : ''} — retiring the target` + `Prerendered url ${rowKey} redirected to non-indexable ${variant.redirectedTo}` + + `${variant.reason ? ` (${variant.reason})` : ''} — retiring the target` ); - await Target.delete(CacheKey.extractUrl(result.id)); - const destinationUrl = CacheKey.extractUrl(redirectKey); + await Target.delete(sourceUrl); + job.rowGone = true; const domain = URL.parse(destinationUrl)?.hostname; // Auth-shaped and transient statuses never reach here (guarded above), so this // suppression is a genuine content/gone verdict about the destination. if (!config.domains.length || config.domains.includes(domain)) { - await Target.suppress(destinationUrl, { reason: result.reason, statusCode: result.statusCode }); + await Target.suppress(destinationUrl, { reason: variant.reason, statusCode: variant.statusCode }); } return; } - if (result.statusCode !== 301 && result.statusCode !== 308) { + if (variant.statusCode !== 301 && variant.statusCode !== 308) { // No proof of permanence (302/303/307 — failover, geo bounce, outage page — or a // client-side redirect's 200). The source is expected to come back — keep its target - // AND its cached page, and look again next interval. But a source that answers with + // AND its cached pages, and look again next interval. But a source that answers with // a temp redirect EVERY interval is a permanent redirect wearing a temporary status: // each result costs a strike and recordRedirectStrike retires the source after // maxStrikes rather than paying a navigation every interval forever. metrics.renderOutcome('redirect', 'temporary'); logger.info( - `Prerendered url ${result.id} temporarily redirected (${result.statusCode}) to ${result.redirectedTo} — ` + + `Prerendered url ${rowKey} temporarily redirected (${variant.statusCode}) to ${variant.redirectedTo} — ` + `keeping the target and retrying at its normal cadence` ); - await this.recordRedirectStrike(result.id, `temporary ${result.statusCode} to ${result.redirectedTo}`); + await this.recordRedirectStrike(job, `temporary ${variant.statusCode} to ${variant.redirectedTo}`); return; } // Permanent move onto a route we serve: retire the source — Target.delete drops the URL's - // row and every device's schedule and cached page — and adopt the destination in its + // row, its schedule rows and every device's cached page — and adopt the destination in its // place. A mutual 301 pair (A↔B) ping-pongs create/delete at the targets' cadence; each // hop is a navigation-only render surfaced by this warn, so a broken site costs noise, // not settles. metrics.renderOutcome('redirect', 'permanent'); logger.info( - `Prerendered url ${result.id} permanently redirected (${result.statusCode}) to ${result.redirectedTo} — ` + + `Prerendered url ${rowKey} permanently redirected (${variant.statusCode}) to ${variant.redirectedTo} — ` + `retiring the target in favor of ${redirectKey}` ); - const sourceUrl = CacheKey.extractUrl(result.id); const source = await Target.get({ id: sourceUrl, select: ['renderInterval'] }); await Target.delete(sourceUrl); + job.rowGone = true; // An existing destination row — active OR suppressed — wins: active means it's already // in rotation under its own cadence; suppressed means a render already proved it // non-indexable, and a redirect pointing at it is no reason to resurrect it. - const destinationUrl = CacheKey.extractUrl(redirectKey); const existingTarget = await Target.get({ id: destinationUrl, select: 'url' }); if (existingTarget) return; @@ -644,7 +877,7 @@ export class RenderQueue extends Resource { if (config.domains.length && !config.domains.includes(domain)) return; // Due now, not jittered: adoptions arrive one per source render, already spread by the - // sources' own schedule jitter, and the source's cached page was just deleted — the + // sources' own schedule jitter, and the source's cached pages were just deleted — the // sooner the destination renders, the shorter the window a bot gets neither page. const target = { nextRenderTime: currentMinuteMs() }; if (Number.isFinite(source?.renderInterval) && source.renderInterval > 0) { @@ -662,8 +895,8 @@ export class RenderQueue extends Resource { * whatever the origin actually serves. The strike counter is the target's one shared * `strikes` field (suppression uses it too); any successful render clears it. */ - static async recordRedirectStrike(cacheKey, why) { - const sourceUrl = CacheKey.extractUrl(cacheKey); + static async recordRedirectStrike(job, why) { + const sourceUrl = job.url; // One read serves both the strike decision and the reschedule below. const renderTarget = await Target.get({ id: sourceUrl, @@ -672,7 +905,8 @@ export class RenderQueue extends Resource { select: ['strikes', 'renderInterval', 'sitemapUrl', 'demandInterval'], }); if (!renderTarget) { - await deleteSchedule(cacheKey); + await deleteSchedule(job.rowKey); + job.rowGone = true; return; } const strikes = countedStrikes(renderTarget.strikes) + 1; @@ -682,33 +916,36 @@ export class RenderQueue extends Resource { `Prerendered url ${sourceUrl} kept redirecting ${strikes} consecutive times (${why}) — retiring it; ` + `bots get the origin's own redirect and discovery re-creates what it actually serves` ); - await Target.delete(sourceUrl); // drops schedules + pages too + await Target.delete(sourceUrl); // drops schedule rows + pages too + job.rowGone = true; return; } await Target.patch(sourceUrl, { strikes }); - await this.rescheduleAtTargetCadence(cacheKey, renderTarget); + await this.rescheduleAtTargetCadence(job, renderTarget); } /** - * Retry shape for auth-shaped (401/403) and transient (408/429/5xx) failures — the ones - * that never suppress. Two lanes, split by the target's strike count + * Retry shape for auth-shaped (401/403) and transient (408/429/5xx) failures and renderer + * errors — the ones that never suppress. Two lanes, split by the target's strike count * (`render.failureRetry.fastRetries`): * * FAST — the schedule row is left alone AND THE CALLER KEEPS THE CLAIM LEASE, so the retry * comes on lease expiry (`queue.jobLeaseTime`, minutes). An origin blip recovers fast, and - * the cached page's swrTtl window keeps serving bots across a lease-sized wait. + * the cached pages' swrTtl window keeps serving bots across a lease-sized wait. * * SLOW — after `fastRetries` consecutive failures this is not a blip: drop to the * target's normal cadence so a persistently failing page can't hot-loop renders all - * day. The kept page's expiry is deliberately NOT extended: `swrTtl` is the product + * day. The kept pages' expiry is deliberately NOT extended: `swrTtl` is the product * bound on how stale we serve as if fresh, and past it bots fall through to the * origin — whose answer (a live page for auth-shaped failures, an honest 5xx for * transient ones) is the truth. Serving arbitrarily old snapshots while users get * errors would break bot/user parity. * * Strikes are the target's one shared counter (suppression and redirect strikes use it - * too); any successful render clears it. A targetless key (render-now one-off) has its - * schedule dropped, as everywhere else. + * too); any successful render clears it. One result per URL means one strike per failed + * cycle — when each device posted its own result, two devices failing counted two, and the + * fast lane was exhausted in a single cycle. A targetless key (render-now one-off) has its + * row dropped, as everywhere else. * * WHAT CHANGED IN v0.34.0, AND WHY IT HAD TO. The fast lane used to work purely by omission: * `claim` wrote `now + jobLeaseTime` into `nextRenderTime`, so "leave the schedule untouched" @@ -730,8 +967,8 @@ export class RenderQueue extends Resource { * must keep the lease; the other two mean release it (the row is now in the future or gone, * and holding a lease for it would pin the claim floor for a full lease for nothing). */ - static async retryAfterFailure(cacheKey) { - const sourceUrl = CacheKey.extractUrl(cacheKey); + static async retryAfterFailure(job) { + const sourceUrl = job.url; const renderTarget = await Target.get({ id: sourceUrl, // `demandInterval` rides along on a point read this path already makes, so the cadence filed @@ -739,18 +976,28 @@ export class RenderQueue extends Resource { select: ['strikes', 'renderInterval', 'sitemapUrl', 'demandInterval'], }); if (!renderTarget) { - await deleteSchedule(cacheKey); + await deleteSchedule(job.rowKey); + job.rowGone = true; return 'dropped'; } const strikes = countedStrikes(renderTarget.strikes) + 1; await Target.patch(sourceUrl, { strikes }); if (strikes <= config.render.failureRetry.fastRetries) { - logger.debug(`Retrying ${cacheKey} on its claim lease (failure strike ${strikes})`); + logger.debug(`Retrying ${job.rowKey} on its claim lease (failure strike ${strikes})`); // Schedule untouched, lease held by the caller — the lease expiry drives the retry. return 'fast'; } + if (!job.fold) { + // A one-device render beside the URL's rotation has no backoff row of its own: the URL row + // renders this device's siblings on cadence regardless, and a per-device row that lingered + // here would be one more thing the rotation does not know about. + await deleteSchedule(job.rowKey); + job.rowGone = true; + return 'slow'; + } + const interval = resolveRenderInterval(sourceUrl, renderTarget.renderInterval); const fromSitemap = !!renderTarget.sitemapUrl; const wait = backoffWait(interval, strikes, fromSitemap); @@ -761,37 +1008,39 @@ export class RenderQueue extends Resource { const cadence = resolveEffectiveInterval(sourceUrl, renderTarget); const nextRenderTime = currentMinuteMs() + wait; logger.debug( - `Retrying ${cacheKey} in ${Math.round(wait / 60000)}m (failure strike ${strikes}` + + `Retrying ${sourceUrl} in ${Math.round(wait / 60000)}m (failure strike ${strikes}` + `${fromSitemap ? '' : ', non-sitemap'})` ); - await writeSchedule(cacheKey, { nextRenderTime, fromSitemap, effectiveInterval: cadence }); + await writeSchedule(sourceUrl, { nextRenderTime, fromSitemap, effectiveInterval: cadence }); return 'slow'; } /** * Keep a redirecting source in its rotation. Mirrors the post-render scheduling in - * processJobResult: a target-backed key comes due one interval from completion (so cadence - * self-paces instead of realigning into a herd); a targetless key (render-now one-off, - * orphaned row) has its schedule dropped so the lease doesn't re-claim it forever. + * processDecodedJobResult: a target-backed URL comes due one interval from completion (so + * cadence self-paces instead of realigning into a herd); a targetless key (render-now one-off, + * orphaned row) — and a one-device row beside a URL's rotation — has its row dropped so the + * lease doesn't re-claim it forever. * * `preloaded` (a row already read with at least renderInterval + sitemapUrl, e.g. by * recordRedirectStrike) skips the point read. */ - static async rescheduleAtTargetCadence(cacheKey, preloaded) { - const sourceUrl = CacheKey.extractUrl(cacheKey); + static async rescheduleAtTargetCadence(job, preloaded) { + const sourceUrl = job.url; const renderTarget = preloaded ?? (await Target.get({ id: sourceUrl, select: ['renderInterval', 'sitemapUrl', 'demandInterval'], })); - if (!renderTarget) { - await deleteSchedule(cacheKey); + if (!renderTarget || !job.fold) { + await deleteSchedule(job.rowKey); + job.rowGone = true; return; } // Same cadence resolution as the post-render path above (route > stored > default). const interval = resolveRenderInterval(sourceUrl, renderTarget.renderInterval); - await writeSchedule(cacheKey, { + await writeSchedule(sourceUrl, { nextRenderTime: currentMinuteMs() + interval, fromSitemap: !!renderTarget.sitemapUrl, // The ladder rung when the row carried one, else the ceiling. A caller-supplied `preloaded` @@ -856,7 +1105,16 @@ export class RenderQueue extends Resource { let notOwnedHere = 0; for (const granted of pass.jobs) { - const { url, deviceType } = CacheKey.parse(granted.cacheKey); + // ONE JOB PER ROW, AND A ROW IS A URL. The job carries every device to render — the configured + // default set for a URL row; exactly the one device a per-device row names (a pre-0.66.0 row + // that has not converted yet, or a deliberate one-device render) — and the browser renders + // them in turn and posts one result. `deviceType` (the first) is kept for a renderer that + // predates `deviceTypes`: it renders that one device and posts the flat legacy shape, which + // `processJobResult` attributes to that device. Degraded, not broken — the render fleet is + // deployed first. + const url = CacheKey.urlOf(granted.cacheKey); + const device = CacheKey.deviceOf(granted.cacheKey); + const deviceTypes = device ? [device] : defaultDeviceTypes(); // Detection only, deliberately. `claim`'s lease write used to purge a stale local // record on a node that is no longer the residency owner, as a side effect; that purge @@ -869,7 +1127,8 @@ export class RenderQueue extends Resource { jobs.push({ id: granted.cacheKey, url, - deviceType, + deviceTypes, + deviceType: deviceTypes[0], expiresAt: granted.expiresAtMs, callbackOrigin: `${protocol}://${server.hostname}:${port}`, // `fromSitemap` is denormalized onto the schedule row, so the job is built with no diff --git a/packages/plugin/src/resources/RenderSchedule.js b/packages/plugin/src/resources/RenderSchedule.js index e74a6e4..1663db1 100644 --- a/packages/plugin/src/resources/RenderSchedule.js +++ b/packages/plugin/src/resources/RenderSchedule.js @@ -1,8 +1,7 @@ import { CacheKey } from '../util/cacheKey.js'; import { getResidencyByUrl } from '../util/residency.js'; -// Co-locate each schedule record on the node that owns its URL. -databases.render_schedule.RenderSchedule.setResidencyById((cacheKey) => { - const url = CacheKey.extractUrl(cacheKey); - return [getResidencyByUrl(url)]; -}); +// Co-locate each schedule record on the node that owns its URL. Rows are keyed by URL (one per +// URL, every device rendered in one job); a pre-0.66.0 per-device row keys by cacheKey and hashes +// to the SAME owner through its URL half, so ownership never moves as rows convert. +databases.render_schedule.RenderSchedule.setResidencyById((key) => [getResidencyByUrl(CacheKey.urlOf(key))]); diff --git a/packages/plugin/src/resources/Target.js b/packages/plugin/src/resources/Target.js index 327841c..0300a3f 100644 --- a/packages/plugin/src/resources/Target.js +++ b/packages/plugin/src/resources/Target.js @@ -4,7 +4,7 @@ import { resolveRenderInterval } from '../util/routeClass.js'; import { getResidencyByUrl } from '../util/residency.js'; import { currentMinuteMs, getInitialRenderTime } from '../util/time.js'; import { applyInBatches, collectFromScan } from '../util/scan.js'; -import { deleteSchedule, writeSchedules } from '../util/renderSchedule.js'; +import { deleteSchedule, writeSchedule } from '../util/renderSchedule.js'; const { page_cache: { PrerenderedPage }, @@ -33,9 +33,9 @@ export const countedStrikes = (value) => { /** * The URL registry — ONE row per URL (see schema.graphql). Device variants are not stored: - * the devices a URL renders for are `config.deviceTypes.default` at write time, and `put` - * fans out one RenderSchedule row per device. RenderSchedule and PrerenderedPage stay - * cacheKey-keyed — the queue and the content are genuinely per-device. + * the devices a URL renders for are `config.deviceTypes.default` at claim time. `put` writes + * ONE RenderSchedule row, keyed by the URL, and a claim of that row is one job rendering every + * device; only PrerenderedPage stays cacheKey-keyed, because the CONTENT is genuinely per-device. * * WRITES TO A RESIDENCY-PINNED KEY DO NOT BLOCK ON THE OWNING NODE. Reads do. The asymmetry * is not obvious and v0.15.0 got it wrong, so it is written down here. @@ -58,12 +58,20 @@ export const countedStrikes = (value) => { */ /** The device variants a URL renders for — config at call time, never stored per-URL, so a - * config change applies to every URL on its next write/sweep instead of never. */ + * config change applies to every URL on its next claim instead of never. */ const deviceTypes = () => config.deviceTypes.default; -/** Every cacheKey a URL's row implies (one per configured device). */ +/** + * Every PAGE key a URL's row implies (one per configured device). Also the keys of the URL's + * pre-0.66.0 per-device schedule rows, which is why the schedule deletes below still name them: + * such a row converts itself the first time it renders, but a target retired before that must not + * leave one behind to render once more and drop itself. + */ export const cacheKeysOf = (url) => deviceTypes().map((deviceType) => CacheKey.toCacheKey({ url, deviceType })); +/** Every SCHEDULE key a URL may have a row under: the URL row, plus any not-yet-converted device row. */ +export const scheduleKeysOf = (url) => [url, ...cacheKeysOf(url)]; + export class Target extends TargetTable { async put(data, target) { const url = this.getId(); @@ -79,42 +87,37 @@ export class Target extends TargetTable { // present in `data` are cleared by construction. That is what makes a sitemap attach // or a fresh discovery naturally lift a suppression — the site re-claimed the URL. // - // Write the target first, then the schedules. Target and RenderSchedule live in + // Write the target first, then the schedule. Target and RenderSchedule live in // separate databases (the schedule is isolated as the hot queue), so these are // independent commits rather than one atomic write. Ordering target-first keeps the // invariant "a schedule always references an existing target" (which `claim` relies // on). The reverse gap — a target with a missing schedule row — is NOT self-healing: - // `util/reconcile.js` is what repairs it, per device. + // `util/reconcile.js` is what repairs it. const result = await super.put({ url, ...data }, target); // Absent a valid explicit time, jitter the first render across the interval — keyed - // off the URL, so bulk-created targets don't all come due at once and a URL's device - // variants share one slot. The jitter window is the same cadence the reschedule loop - // will resolve (route > stored > default), so the initial spread matches the recurring - // one. Target is API-exposed and resolveRenderInterval validates the stored number - // (rejects negatives / NaN / non-numbers) rather than trusting the payload. + // off the URL, so bulk-created targets don't all come due at once. The jitter window is + // the same cadence the reschedule loop will resolve (route > stored > default), so the + // initial spread matches the recurring one. Target is API-exposed and + // resolveRenderInterval validates the stored number (rejects negatives / NaN / + // non-numbers) rather than trusting the payload. const interval = resolveRenderInterval(url, data.renderInterval); const fromSitemap = !!data.sitemapUrl; - // One floor lowering for the whole device fan-out. The explicit `nextRenderTime` branch is - // validated no further than `> 0`, and it is the funnel for redirect adoption, sitemap - // `revalidate: true`, and any external `PUT /render_targets` — i.e. exactly the "due now" - // and "due in the past" writes a claim floor would otherwise strand. That is why it must - // not be a bare table put. - await writeSchedules( - cacheKeysOf(url).map((cacheKey) => ({ - cacheKey, - nextRenderTime: - Number.isFinite(nextRenderTime) && nextRenderTime > 0 - ? nextRenderTime - : getInitialRenderTime(cacheKey, interval), - fromSitemap, - // `interval`, and no ladder rung applied — deliberately. `super.put` above REPLACES the - // target row, so a put clears `demandInterval` along with the suppression fields; the - // target genuinely restarts at its route/stored cadence and this records that. Reading - // the old rung to carry it forward would file a cadence the target no longer has. - effectiveInterval: interval, - })) - ); + // ONE row, keyed by the URL: the claim renders every configured device off it. The explicit + // `nextRenderTime` branch is validated no further than `> 0`, and it is the funnel for + // redirect adoption, sitemap `revalidate: true`, and any external `PUT /render_targets` — + // i.e. exactly the "due now" and "due in the past" writes a claim floor would otherwise + // strand. That is why it must not be a bare table put. + await writeSchedule(url, { + nextRenderTime: + Number.isFinite(nextRenderTime) && nextRenderTime > 0 ? nextRenderTime : getInitialRenderTime(url, interval), + fromSitemap, + // `interval`, and no ladder rung applied — deliberately. `super.put` above REPLACES the + // target row, so a put clears `demandInterval` along with the suppression fields; the + // target genuinely restarts at its route/stored cadence and this records that. Reading + // the old rung to carry it forward would file a cadence the target no longer has. + effectiveInterval: interval, + }); return result; } @@ -133,8 +136,15 @@ export class Target extends TargetTable { // lands on the node it runs on — an owner-node row deleted elsewhere is left behind, and // that is fine: an orphaned baseline is never walked again (the sweep walks Targets), and // a re-created target on a new owner seeds fresh regardless. + // + // The schedule deletes name the URL row AND the URL's pre-0.66.0 device rows (see + // `scheduleKeysOf`): a device row that has not yet converted would otherwise outlive its + // target, render once more, and only then drop itself. Harper records a delete of an absent + // key in the audit log, so this costs two tombstones per retired target once the corpus has + // converted — on a path that runs a few hundred times a day, not on the render path. await Promise.all([ - ...cacheKeysOf(url).flatMap((cacheKey) => [deleteSchedule(cacheKey), PrerenderedPage.delete(cacheKey)]), + ...scheduleKeysOf(url).map((key) => deleteSchedule(key)), + ...cacheKeysOf(url).map((cacheKey) => PrerenderedPage.delete(cacheKey)), databases.probe_state.ProbeState.delete(url), ]); @@ -206,28 +216,24 @@ export class Target extends TargetTable { // floor), routed through the funnel anyway so the first "recheck this immediately" path // anyone adds here inherits the lowering instead of silently stranding the URL. await Promise.all([ - writeSchedules( - cacheKeysOf(url).map((cacheKey) => ({ - cacheKey, - nextRenderTime: recheckAt, - fromSitemap: !!existing?.sitemapUrl, - // THE CADENCE, NOT `recheckInterval` — this is the case `util/renderPriority.js` calls - // out by name. A 7-day recheck filed as a cadence would make a suppressed 48h page read - // as 3.5 cadences stale the moment it comes due and outrank a genuinely late homepage, - // promoting exactly the rows worth deprioritizing. No rung applied for the same reason - // as `put`: the `TargetTable.put` above omits `demandInterval`, so the rung is cleared - // with it and the target resumes at its route/stored cadence. - effectiveInterval: resolveRenderInterval(url, existing?.renderInterval ?? null), - })) - ), + writeSchedule(url, { + nextRenderTime: recheckAt, + fromSitemap: !!existing?.sitemapUrl, + // THE CADENCE, NOT `recheckInterval` — this is the case `util/renderPriority.js` calls + // out by name. A 7-day recheck filed as a cadence would make a suppressed 48h page read + // as 3.5 cadences stale the moment it comes due and outrank a genuinely late homepage, + // promoting exactly the rows worth deprioritizing. No rung applied for the same reason + // as `put`: the `TargetTable.put` above omits `demandInterval`, so the rung is cleared + // with it and the target resumes at its route/stored cadence. + effectiveInterval: resolveRenderInterval(url, existing?.renderInterval ?? null), + }), ...cacheKeysOf(url).map((cacheKey) => PrerenderedPage.delete(cacheKey)), ]); return { deleted: false, strikes }; } /** A render found a suppressed URL indexable again — put it back in normal rotation. - * The caller reschedules the device that just rendered; the sibling devices' schedules - * already exist (suppress set them) and will re-render at their recheck time. */ + * The caller reschedules the URL row at its cadence. */ static async reactivate(url) { await Target.patch(url, { state: null, suppressedReason: null, suppressedAt: null, strikes: 0 }); } @@ -289,8 +295,8 @@ export class Target extends TargetTable { }); // Phase 2 — writes, cursor now closed. Each batch is awaited before the next starts, - // so pending writes never span a monitor tick; within one URL the device variants are - // independent rows, so they proceed in parallel. + // so pending writes never span a monitor tick; within one URL the device PAGES are + // independent rows, so their expiry patches proceed in parallel. await applyInBatches({ items: urls, apply: async ({ url, sitemapUrl }) => { @@ -319,22 +325,18 @@ export class Target extends TargetTable { // // One lowering per URL rather than one for the whole batch: every row here gets the // same `currentMinuteMs()`, so after the first the CAS-min is a single atomic load - // that changes nothing. Hoisting the lowering out of the loop would mean carrying the - // batch's rows in memory to no measurable end. - await writeSchedules( - cacheKeysOf(url).map((cacheKey) => ({ - cacheKey, - nextRenderTime, - fromSitemap: !!sitemapUrl, - // `null` — the sweep resolves from config instead, which is what it did before this - // field existed. Phase 1's projection is deliberately just `url` + `sitemapUrl` (and - // an API-facing guard enforces exactly those two), so carrying a cadence here would - // mean widening that contract. It cannot affect this row's ranking anyway: every row - // is filed at the current minute, so its lateness is ~0 whatever the denominator, - // and the render it is being queued for refills the cadence on completion. - effectiveInterval: null, - })) - ); + // that changes nothing. + await writeSchedule(url, { + nextRenderTime, + fromSitemap: !!sitemapUrl, + // `null` — the sweep resolves from config instead, which is what it did before this + // field existed. Phase 1's projection is deliberately just `url` + `sitemapUrl` (and + // an API-facing guard enforces exactly those two), so carrying a cadence here would + // mean widening that contract. It cannot affect this row's ranking anyway: every row + // is filed at the current minute, so its lateness is ~0 whatever the denominator, + // and the render it is being queued for refills the cadence on completion. + effectiveInterval: null, + }); }, }); diff --git a/packages/plugin/src/schemas/schema.graphql b/packages/plugin/src/schemas/schema.graphql index e110e8a..25a5c43 100644 --- a/packages/plugin/src/schemas/schema.graphql +++ b/packages/plugin/src/schemas/schema.graphql @@ -82,10 +82,10 @@ type SitemapRefresh @table(database: "sitemaps") @export(name: "sitemap_refresh" } # The registry of URLs in the render rotation — ONE row per URL. Device variants are not -# stored: the set of devices a URL renders for is `config.deviceTypes.default` at write time, -# and `Target.put` fans out one RenderSchedule row per device (RenderSchedule and -# PrerenderedPage stay cacheKey-keyed — the queue and the content are genuinely per-device; -# only the registry is per-URL). +# stored: the set of devices a URL renders for is `config.deviceTypes.default` at claim time. +# `Target.put` writes ONE RenderSchedule row, keyed by the URL, and a claim of that row is one +# job rendering every device; only PrerenderedPage stays cacheKey-keyed, because the CONTENT is +# genuinely per-device. # # `state: suppressed` is the render-verdict memory that used to live in the NonIndexable # table: a render proved the page non-indexable, so discovery must not keep re-creating the @@ -110,6 +110,19 @@ type Target @table(database: "render_service") @export { demandInterval: Long } +# The render queue: ONE ROW PER URL, due at `nextRenderTime`. A claim of a row is one job that +# renders every device in `config.deviceTypes.default`, and its one result schedules the URL once — +# which is what keeps a URL's device variants aligned (same render pass, one decision) instead of +# drifting apart through every per-device path, as they did when this table held a row per cacheKey. +# +# THE PRIMARY KEY IS STILL NAMED `cacheKey` AND HOLDS THE URL. Harper refuses to rename the primary +# key of a table that has records, so the attribute name is fixed for the life of the table; read it +# as "the schedule key". Two shapes of key coexist for one render cycle after the v0.66.0 upgrade: +# the URL, and the pre-0.66.0 `|` cacheKey — each device-keyed row converts into the +# URL row the first time it renders (`RenderQueue.processJobResult`), with no sweep and no extra +# renders. A device-keyed row also remains the shape of a deliberate one-device render (`renderNow` +# for a device outside `deviceTypes.default`). Every reader goes through `CacheKey.urlOf` / +# `CacheKey.deviceOf`; see util/renderSchedule.js. type RenderSchedule @table(database: "render_schedule") @export { cacheKey: String @primaryKey nextRenderTime: Long @indexed @@ -315,16 +328,16 @@ type PageVerification @table(database: "verification", expiration: 604800) @seal # The `lastCached` of the render whose claims were verified (`ProbeState.pageClaimAt`), and what # makes a per-URL verification safe for PER-DEVICE pages. # - # `pageSignature` is keyed by url and written by whichever device rendered last, so the evidence - # is per-URL while the pages are per-cacheKey. On a SPLIT PAIR — a normal state here — desktop - # could be verified while mobile still holds a pre-change render, and a bare per-URL exemption - # would serve the stale mobile page. Carrying the basis closes that exactly: the serve path - # exempts a key only when its OWN `lastCached` is at least this, so the verified render and - # anything newer qualify and a lagging sibling does not. - # - # In the healthy case this costs nothing — `util/time.js` seeds jitter off the URL half precisely - # so a URL's device variants land on the same minute, so both keys clear the basis and both are - # exempt. Absent (a row written before this field existed) reads as NaN and exempts nothing. + # `pageSignature` is keyed by url and written once per result, so the evidence is per-URL while + # the pages are per-cacheKey. Since v0.66.0 a URL's devices render in one job and every page of a + # result carries the SAME `lastCached`, so in the healthy case both keys clear the basis exactly. + # A SPLIT PAIR is still reachable — a pre-0.66.0 per-device row that has not yet converted, a + # partial result whose missing device kept its older page, a one-device render-now — and there + # desktop could be verified while mobile still holds a pre-change render; a bare per-URL exemption + # would serve the stale mobile page. Carrying the basis closes that exactly: the serve path exempts + # a key only when its OWN `lastCached` is at least this, so the verified render and anything newer + # qualify and a lagging sibling does not. Absent (a row written before this field existed) reads + # as NaN and exempts nothing. basisAt: Date } diff --git a/packages/plugin/src/util/cacheKey.js b/packages/plugin/src/util/cacheKey.js index 2f1a983..273ac7b 100644 --- a/packages/plugin/src/util/cacheKey.js +++ b/packages/plugin/src/util/cacheKey.js @@ -23,4 +23,38 @@ export class CacheKey { static extractUrl(cacheKey) { return cacheKey.substring(0, cacheKey.indexOf(config.cacheKey.delimiter)); } + + // ── schedule keys ────────────────────────────────────────────────────────────────────────── + // + // A `RenderSchedule` row is keyed by URL (one row per URL, every device rendered in one job) since + // v0.66.0. Before that it was keyed by cacheKey, one row per device, and those rows are NOT + // migrated in place: each converts the first time it renders (see `RenderQueue.processJobResult`), + // so for one full render cycle after the upgrade the table holds both shapes. A device-keyed row + // also remains the shape of a deliberate one-device render (`renderNow` for a device outside + // `deviceTypes.default`). Every reader of a schedule key therefore goes through these three, + // never through `extractUrl`/`parse` directly — `extractUrl` on a URL-shaped key returns '' (the + // delimiter is absent, `indexOf` is -1), which resolves a route for the empty string and files + // a row nobody asked for. + // + // The shape test is "contains the delimiter", which is sound because `canonicalizeUrl` + // percent-encodes a literal delimiter out of the URL half (util/url.js step 7) precisely so it can + // never appear there. + + /** True when `key` is a cacheKey (``), i.e. a per-device schedule row. */ + static isCacheKey(key) { + return String(key ?? '').includes(config.cacheKey.delimiter); + } + + /** The URL a schedule key stands for: the URL half of a cacheKey, or the key itself. */ + static urlOf(key) { + const str = String(key ?? ''); + return CacheKey.isCacheKey(str) ? CacheKey.extractUrl(str) : str; + } + + /** The device a per-device schedule key names, or `null` for a URL-keyed row. */ + static deviceOf(key) { + const str = String(key ?? ''); + if (!CacheKey.isCacheKey(str)) return null; + return CacheKey.parse(str).deviceType || null; + } } diff --git a/packages/plugin/src/util/discoveredPurge.js b/packages/plugin/src/util/discoveredPurge.js index b0cce61..5dfb89c 100644 --- a/packages/plugin/src/util/discoveredPurge.js +++ b/packages/plugin/src/util/discoveredPurge.js @@ -34,7 +34,7 @@ */ import { setImmediate as yieldNow, setTimeout as sleep } from 'node:timers/promises'; -import { Target, cacheKeysOf } from '../resources/Target.js'; +import { Target, scheduleKeysOf } from '../resources/Target.js'; import { classifyUrl, PRERENDER } from './routeClass.js'; import { getResidencyByUrl } from './residency.js'; import { leaseInfo } from './renderSchedule.js'; @@ -301,7 +301,7 @@ export const startDiscoveredPurge = ({ ownerOf: getResidencyByUrl, hostname: server.hostname, // Any leased device key defers the whole target — the delete takes every device with it. - isLeased: (url) => cacheKeysOf(url).some((cacheKey) => Boolean(leaseInfo(cacheKey))), + isLeased: (url) => scheduleKeysOf(url).some((key) => Boolean(leaseInfo(key))), // The RESOURCE class delete, never the raw table's: only it cascades to the schedule // rows, the cached pages and the probe baseline. A raw delete leaves schedule rows that // re-render once each and page blobs that nothing ever reclaims. diff --git a/packages/plugin/src/util/explain.js b/packages/plugin/src/util/explain.js index 8d4fcbb..30c4017 100644 --- a/packages/plugin/src/util/explain.js +++ b/packages/plugin/src/util/explain.js @@ -75,6 +75,9 @@ export const explainCacheKey = (rawUrl, requestedDeviceType) => { hostname: url.hostname, canonicalUrl, cacheKey, + // The RenderSchedule row's key: the URL itself (one row per URL, every device in one + // job). Named so a reader does not go looking for the row under `cacheKey`. + scheduleKey: canonicalUrl, }, ingress: { mode: config.ingress.mode, diff --git a/packages/plugin/src/util/invalidationReenqueue.js b/packages/plugin/src/util/invalidationReenqueue.js index f968958..0ddf393 100644 --- a/packages/plugin/src/util/invalidationReenqueue.js +++ b/packages/plugin/src/util/invalidationReenqueue.js @@ -58,16 +58,18 @@ * error the write threw. Logged, counted, swallowed — a failed acceleration must never * turn into a 500 on a request that has already been answered. * - * THE LAST TWO ARE TESTED PER ROW, NOT PER URL, because the WRITE is per row. A split pair is a - * normal production state, not an edge case — `PrerenderAdmin.revalidateUrl` and `renderNow` each - * write ONE device key on purpose, `util/reconcile.js` repairs a missing row with a fresh jitter, and - * every per-device retry lane diverges the pair by its own delay. Taking either verdict from the - * device the request happened to arrive on and applying it to the whole URL let the crawler's - * User-Agent choose which invariant held: a mobile crawl would re-arm a desktop row that a desktop - * crawl had just been refused (I11 lost), and one overdue sibling refused acceleration of the very key - * whose page the invalidation had made unservable (I12's fan-out inverted into a whole-URL veto). - * Skipping a row instead is safe in both directions: an unhealable key has no post-epoch content for - * the pair to align with anyway, and an already-sooner key is already ahead in the claim order. + * THE LAST TWO ARE TESTED PER ROW, NOT PER URL, because the WRITE is per row. Since v0.66.0 a URL + * normally has exactly ONE schedule row (keyed by the URL; every device renders in one job), so the + * two tests simply decide the URL. The per-row form is kept for the rows that can still sit beside + * it: a pre-0.66.0 per-device row that has not yet converted, and a deliberate one-device row for a + * served device outside `deviceTypes.default`. When the table WAS per-device, a split pair was a + * normal production state, and taking either verdict from the device the request happened to arrive + * on and applying it to the whole URL let the crawler's User-Agent choose which invariant held: a + * mobile crawl would re-arm a desktop row that a desktop crawl had just been refused (I11 lost), and + * one overdue sibling refused acceleration of the very key whose page the invalidation had made + * unservable (I12's fan-out inverted into a whole-URL veto). Skipping a row instead is safe in both + * directions: an unhealable key has no post-epoch content to align with anyway, and an already-sooner + * key is already ahead in the claim order. * * I13 ("never creates a Target or a schedule row") is enforced by `no-schedule`/`no-target` against a * row that is ABSENT WHEN WE READ IT. It is not proof against a `Target.delete` landing between those @@ -94,24 +96,25 @@ * * Per NODE, shared across workers, in one minute-bucketed counter in a named shared buffer — the * same primitive the claim floor and the queue-status flag use (`util/coordination.js#getSab`). - * It bounds requests, and one request writes at most one row per device row THE URL HAS — - * `deviceTypes.default`, plus the served device when that one is merely `supported` — so the write - * ceiling is `maxPerMinute ×` those rows (20/min/node at the two-device default, ≈2.3MB of - * audit/node/day, ~7% of measured spare fleet capacity against ~1,000 owner-node candidate - * requests/day cluster-wide). + * It bounds requests, and one request writes at most one row per schedule row THE URL HAS — its URL + * row, plus any not-yet-converted per-device row and a one-device row for a served device that is + * merely `supported` — so the write ceiling is `maxPerMinute ×` those rows (10/min/node once the + * corpus has converted, ≈1.2MB of audit/node/day, a few percent of measured spare fleet capacity + * against ~1,000 owner-node candidate requests/day cluster-wide). * * The slot is reserved LATE — after every refusal test, immediately before the write. Reserving * first would bound the reads too, but one repeatedly-crawled unhealable URL would then burn the * whole node's budget and starve every key that can actually heal. Reads stay bounded by bot - * traffic instead: one Target read plus one schedule read per device key, on ≤2,900 cache-servable - * requests/day cluster-wide, of which only the ~25% this node owns get past guard 1. An invalidated + * traffic instead: one Target read plus one schedule read per key the URL can have a row under, on + * ≤2,900 cache-servable requests/day cluster-wide, of which only the ~25% this node owns get past + * guard 1. An invalidated * request also pays `handlePageScheduling`'s own `Target.get` in the sibling `setImmediate` — the same * primary key, a different projection, deliberately NOT shared: that read is the rediscovery repair * for a page whose Target was deleted and it has to be fresh at the moment it runs. */ import { config } from '../config.js'; -import { Target, cacheKeysOf, countedStrikes } from '../resources/Target.js'; +import { Target, scheduleKeysOf, countedStrikes } from '../resources/Target.js'; import { QueueState } from '../resources/QueueState.js'; import { getSab } from './coordination.js'; import { getScheduleRow, leaseInfo, writeSchedules } from './renderSchedule.js'; @@ -278,10 +281,13 @@ export const accelerateHeal = async ({ url, cacheKey, invalidatedBy, forwarded = // pulled-forward due times it is not draining — they would all come due at once on resume. if (QueueState.status === 'paused') return refuse('paused'); - // The device keys this URL implies, plus the key the request was actually served under: a device - // type may be `supported` (so it has pages and rows) without being in `deviceTypes.default`, and - // excluding it would leave the one key the request was about un-accelerated. - const keys = cacheKeysOf(url); + // Every schedule key this URL can have a row under: the URL row (the whole rotation, every device + // in one job), the URL's pre-0.66.0 per-device rows while they have not yet converted, and the key + // the request was actually served under — a device type may be `supported` (so it has a page, and + // can have a one-device row) without being in `deviceTypes.default`, and excluding it would leave + // the one key the request was about un-accelerated. The rows that do not exist cost one node-local + // point read each and are filtered out below. + const keys = scheduleKeysOf(url); if (!keys.includes(cacheKey)) keys.push(cacheKey); // Exact, and free: we own the key, so its lease is in THIS node's buffer. @@ -314,14 +320,15 @@ export const accelerateHeal = async ({ url, cacheKey, invalidatedBy, forwarded = // bounded either way: refusing wrongly leaves the key to heal on cadence, accelerating wrongly // costs at most one render, and the rate limit caps both. const interval = resolveRenderInterval(url, target.renderInterval); - // Seeded off the URL half, so every device key we write gets the SAME minute for free (I12) — a - // lowering that moved one device would de-align the pair permanently, cycle over cycle, because - // `processJobResult` reschedules from each render's own completion. No metric would show it: - // route_serve and page_age are per-device and nobody reads them as a pair. - const dueAt = getInitialRenderTime(cacheKey, spreadWindowMs()); + // Seeded off the URL, so every row we write for it gets the SAME minute for free (I12): the URL + // row and any not-yet-converted device row come due together. + const dueAt = getInitialRenderTime(url, spreadWindowMs()); // PER ROW, BECAUSE THE WRITE IS PER ROW — see the module comment on why a whole-URL verdict here - // let the crawler's User-Agent decide which invariant held. + // let the crawler's User-Agent decide which invariant held. With the schedule keyed by URL there + // is normally exactly one row, so these two tests decide the URL; the per-row form is what keeps a + // half-converted URL (URL row beside a device row) and a one-device row correct through the same + // code. // // completedAfterEpoch the row's implied completion is after the epoch, so a render has already // run and left this key pre-epoch: nothing can heal it (I11). @@ -367,8 +374,8 @@ export const accelerateHeal = async ({ url, cacheKey, invalidatedBy, forwarded = outcome: 'lowered', dueAt, written: eligible.map((row) => row.cacheKey), - // The device rows this write deliberately left where they were. There is no metric dimension for - // a partial fan-out — `lowered` is `lowered` — so a caller that wants to know whether the key the + // The rows this write deliberately left where they were. There is no metric dimension for a + // partial fan-out — `lowered` is `lowered` — so a caller that wants to know whether the key the // request was about actually moved has to read this. skipped: present.filter((row) => !eligible.includes(row)).map((row) => row.cacheKey), // The floor obligation, discharged by the funnel as part of the write. True by construction diff --git a/packages/plugin/src/util/orphanSweep.js b/packages/plugin/src/util/orphanSweep.js index af28ed6..2f0ef95 100644 --- a/packages/plugin/src/util/orphanSweep.js +++ b/packages/plugin/src/util/orphanSweep.js @@ -138,10 +138,14 @@ export const sweepOrphanedTargets = async ({ // target-guarded — the `PrerenderedPage.put`, which would otherwise leave a page record // under a key whose target and schedule we just removed. // - // Checked per DEVICE, and any one lease defers the whole target: the delete is per-url - // and takes every device key with it, so it is unsafe while ANY of them is out. A - // deferred target is simply swept on the next pass. - if (deviceTypes.some((deviceType) => isLeased(CacheKey.toCacheKey({ url: target.url, deviceType })))) { + // Checked under every schedule key the URL can have a lease under — the URL row, and any + // pre-0.66.0 device row that has not yet converted — and any one lease defers the whole + // target: the delete is per-url and takes every key with it, so it is unsafe while ANY of + // them is out. A deferred target is simply swept on the next pass. + if ( + isLeased(target.url) || + deviceTypes.some((deviceType) => isLeased(CacheKey.toCacheKey({ url: target.url, deviceType }))) + ) { stats.leaseSkipped++; continue; } @@ -187,8 +191,7 @@ export const sweepKeyRuleOrphans = async ({ deleteTarget: (url) => Target.delete(url), ownerOf: getResidencyByUrl, hostname: server.hostname, - // Config at sweep time, matching Target.put's fan-out, so the lease check covers exactly - // the device keys that exist. + // The device rows a not-yet-converted URL may still be leased under, beside its URL row. deviceTypes: config.deviceTypes.default, maxDeletes, dryRun, diff --git a/packages/plugin/src/util/reconcile.js b/packages/plugin/src/util/reconcile.js index 77417ce..a499a04 100644 --- a/packages/plugin/src/util/reconcile.js +++ b/packages/plugin/src/util/reconcile.js @@ -1,12 +1,11 @@ /** - * Repair for targets whose `RenderSchedule` rows have gone missing. + * Repair for targets whose `RenderSchedule` row has gone missing. * * `Target` and `RenderSchedule` live in separate databases, so creating a target is a target - * commit plus one schedule commit PER DEVICE — and the schedule half is residency-routed to - * whichever node owns the URL. If a schedule write is lost (a crash between them, or a routed - * write to a node whose replication link is unhealthy), or if cluster membership changes and - * moves a key's owner, the target survives with a missing schedule row for one or more of its - * devices. + * commit plus one schedule commit — and the schedule half is residency-routed to whichever node + * owns the URL. If the schedule write is lost (a crash between them, or a routed write to a node + * whose replication link is unhealthy), or if cluster membership changes and moves a key's + * owner, the target survives with no schedule row. * * Nothing then repairs it, and nothing renders that URL again: * @@ -78,9 +77,13 @@ export const reconcileSchedules = async ({ const stats = { examined: 0, owned: 0, missing: 0, restored: 0, truncated: false }; const toRestore = []; - // Phase 1 — read only. One target row implies one schedule row PER configured device, and - // each is checked independently — a URL can be half-scheduled (desktop present, mobile - // missing) and that partial gap is just as silent as a full one. + // Phase 1 — read only. One target row implies ONE schedule row, keyed by the URL. + // + // A URL whose row is absent is checked a second way before it counts as missing: its + // pre-0.66.0 PER-DEVICE rows. Those convert to the URL row the first time they render, and until + // then a URL scheduled under them is scheduled — restoring a URL row beside them would render the + // URL twice for a cycle. The device reads cost nothing once the corpus has converted, because they + // only run for a URL whose URL row is missing, which is the rare case this sweep exists for. for await (const target of streamTargets()) { stats.examined++; if (stats.examined % YIELD_EVERY === 0) await onYield(); @@ -90,19 +93,26 @@ export const reconcileSchedules = async ({ if (ownerOf(target.url) !== hostname) continue; stats.owned++; + if (await getSchedule(target.url)) continue; + let scheduledByDeviceRow = false; for (const deviceType of deviceTypes) { - const cacheKey = CacheKey.toCacheKey({ url: target.url, deviceType }); - if (await getSchedule(cacheKey)) continue; - stats.missing++; - - // Past the cap we keep counting but stop collecting, so the gap is measured in full - // while the repair stays bounded. A membership change can strand a large slice of the - // keyspace at once, and rewriting millions of rows in one pass would be its own outage. - if (toRestore.length < maxRestores) toRestore.push({ cacheKey, target }); + if (await getSchedule(CacheKey.toCacheKey({ url: target.url, deviceType }))) { + scheduledByDeviceRow = true; + break; + } } + if (scheduledByDeviceRow) continue; + stats.missing++; + + // Past the cap we keep counting but stop collecting, so the gap is measured in full + // while the repair stays bounded. A membership change can strand a large slice of the + // keyspace at once, and rewriting millions of rows in one pass would be its own outage. + if (toRestore.length < maxRestores) toRestore.push({ cacheKey: target.url, target }); } - // Phase 2 — writes, with the scan's cursor now closed. + // Phase 2 — writes, with the scan's cursor now closed. `cacheKey` here is the URL — the row's + // key — named after the schema's primary-key attribute (see util/renderSchedule.js on why that + // attribute keeps its old name). for (const { cacheKey, target } of toRestore) { // Hoisted because the jittered time and the recorded cadence must be the same number — see both // comments below. @@ -161,7 +171,7 @@ export const reconcileScheduleGaps = async ({ maxRestores = config.render.reconc streamTargets: () => Target.search({ select: ['url', 'renderInterval', 'sitemapUrl'] }), // Node-local by construction — see the module comment. Existence is all that matters. // (`getScheduleRow` is what carries the mandatory `replicateFrom: false`.) - getSchedule: (cacheKey) => getScheduleRow(cacheKey, ['cacheKey']), + getSchedule: (key) => getScheduleRow(key, ['cacheKey']), // Writes route by residency, so this reaches the owning node even though the read above // deliberately does not — and it goes through the schedule funnel, which lowers the claim // floor with the write. A restored row filed BEHIND the floor would be exactly the silent, @@ -171,11 +181,10 @@ export const reconcileScheduleGaps = async ({ maxRestores = config.render.reconc // FUTURE time, so the funnel's CAS-min never actually moves the floor and the per-row cost // is one atomic load. Batching it would mean changing the injected port signature that keeps // the traversal tests running with no Harper globals, for no measurable gain. - putSchedule: (cacheKey, row) => writeSchedule(cacheKey, row), + putSchedule: (key, row) => writeSchedule(key, row), ownerOf: getResidencyByUrl, hostname: server.hostname, - // Config at sweep time, matching Target.put's fan-out — so a device added to config - // gets its missing schedule rows created for every existing target by this sweep. + // The device rows a not-yet-converted URL may still be scheduled under (see phase 1). deviceTypes: config.deviceTypes.default, maxRestores, onYield: () => setImmediate(), diff --git a/packages/plugin/src/util/renderSchedule.js b/packages/plugin/src/util/renderSchedule.js index 445cb2b..f64f430 100644 --- a/packages/plugin/src/util/renderSchedule.js +++ b/packages/plugin/src/util/renderSchedule.js @@ -1,6 +1,31 @@ /** * THE ONLY MODULE IN `src/` THAT TOUCHES THE `RenderSchedule` TABLE. * + * ── WHAT A ROW IS ──────────────────────────────────────────────────────────────────────────── + * + * ONE ROW PER URL (since v0.66.0). The row's key is the URL, a claim hands the renderer ONE job + * carrying every device in `config.deviceTypes.default`, and the renderer posts ONE result with + * every device's snapshot — so a URL's variants are rendered in the same pass, seconds apart, and + * scheduled by one decision. Before this the table held one row per cacheKey (per device), and the + * two rows drifted apart through every per-device path (render-now, revalidate, reconcile, the + * retry lanes) until "a split pair" was a normal production state. + * + * Existing per-device rows are NOT migrated by a sweep. Each one converts the first time it renders: + * its job renders exactly the device its key names, and its result writes the URL row and deletes + * the device row (`RenderQueue.processJobResult`). Two sibling rows therefore fold into one URL row + * within a cycle at no extra renders. Until then BOTH SHAPES COEXIST in this table, and every reader + * of a key in this module goes through `CacheKey.urlOf` / `CacheKey.deviceOf`, never `extractUrl`. + * The floor algebra below is indifferent to key shape — a key is an opaque string to it. + * + * A device-keyed row remains the shape of a deliberate ONE-DEVICE render: `renderNow` for a device + * outside `deviceTypes.default` files one, and its result stores that page and retires the row + * without touching the URL row. + * + * The primary-key attribute is still NAMED `cacheKey` in the schema, and that is not carelessness: + * Harper refuses to rename the primary key of a table that has records (databases.ts, "Cannot + * change the primary key"), so the column name is fixed for the life of the table. Read it as + * "the schedule key". + * * "One concept, one home" is not a style preference here, it is the safety mechanism. The claim * scan now starts from a FLOOR (`util/renderLease.js` explains why: seeking the absolute minimum * of the `nextRenderTime` index degraded 0.36 ms → 6.25 ms over 40,000 reschedules and did not @@ -288,14 +313,13 @@ export const writeSchedule = async (cacheKey, { nextRenderTime, fromSitemap, eff }; /** - * The batch form, for the fan-out writers (a target's device variants, `Target.revalidate`, - * sitemap ingest, a reconcile repair pass). Writes every row, then lowers the floor ONCE with - * the batch minimum — a per-row atomic inside the very loop that exists to keep transactions - * short would be the wrong shape even though it is cheap. + * The batch form, for writers with several rows in hand (the invalidation accelerator, which may + * lower a URL row and a not-yet-converted device row together). Writes every row, then lowers the + * floor ONCE with the batch minimum — a per-row atomic inside the very loop that exists to keep + * transactions short would be the wrong shape even though it is cheap. * - * Sequential, matching the call sites it replaces: `Target.put`'s device loop awaited each row, - * and `reconcile`'s phase 2 does too. Rows are independent, so a rejection propagates with the - * earlier rows applied — the same semantics as before, and deletes/puts here are idempotent. + * Sequential. Rows are independent, so a rejection propagates with the earlier rows applied, and + * deletes/puts here are idempotent. */ export const writeSchedules = async (rows = []) => { let lowest = Number.POSITIVE_INFINITY; @@ -575,7 +599,7 @@ const maybeUnpinFloor = async (pass) => { // rows written before this field existed: a target whose STORED interval differs from the default // with no route interval to override it is pushed by that difference. Cost of that residual is one // extra render per crawl of one URL, rate-limited by the accelerator's own budget. - const interval = carriedCadence(effectiveInterval) ?? resolveRenderInterval(CacheKey.extractUrl(cacheKey), null); + const interval = carriedCadence(effectiveInterval) ?? resolveRenderInterval(CacheKey.urlOf(cacheKey), null); const nextRenderTime = Date.now() + interval; try { // `interval`, NOT the raw `effectiveInterval` off the row — and the difference is a silent @@ -724,8 +748,9 @@ export const sweepReadySet = async ({ nowMs = Date.now() } = {}) => { // parse, no route walk — so once the corpus has re-rendered once this memo serves the remainder: // pre-upgrade rows and the writers with no cadence in hand. // - // Route resolution parses a URL and walks the route list, and a URL's device variants share both — - // so this memo halves the work at minimum, on the one loop that sees every due row on the node. + // Route resolution parses a URL and walks the route list. Rows are keyed by URL now, so the memo + // mostly earns its keep during the one cycle after the upgrade in which a URL's pre-0.66.0 device + // rows still coexist and share it; it costs nothing to keep afterwards. // Per sweep rather than process-lifetime: the route list is live-reloadable, and a cache keyed by // URL over an 814k-target corpus to serve one sweep is the unbounded-structure mistake this node // has already been taken down by twice. @@ -822,7 +847,7 @@ export const sweepReadySet = async ({ nowMs = Date.now() } = {}) => { // more often. Config resolution stays as the fallback for rows that carry nothing. const carried = carriedCadence(row.effectiveInterval); if (carried !== null) cadenceCarried++; - const intervalMs = carried ?? intervalFor(CacheKey.extractUrl(row.cacheKey)); + const intervalMs = carried ?? intervalFor(CacheKey.urlOf(row.cacheKey)); const score = scoreOf({ dueAt, fromSitemap: !!row.fromSitemap }, { nowMs, intervalMs, sitemapBoost }); heap.offer(score, { cacheKey: row.cacheKey, dueAt, fromSitemap: !!row.fromSitemap }); // Yielding is free (measured: 2.375 vs 2.387 us/row at 20,000 rows) and this runs beside bot diff --git a/packages/plugin/src/util/time.js b/packages/plugin/src/util/time.js index bd21d81..1e380a1 100644 --- a/packages/plugin/src/util/time.js +++ b/packages/plugin/src/util/time.js @@ -88,18 +88,14 @@ export const getNextTimeOfDay = (timeStr, timezone) => { }; /** - * The jitter offset is seeded off the URL half of a cache key, NOT the whole key, so every - * device-type variant of one URL lands on the SAME minute. Seeded off the full key, `desktop` - * and `mobile` hash to unrelated offsets and drift up to a whole interval apart, which means - * the two copies of a page can differ in age by up to 24h — a content change shows on one - * device and not the other, and every render pays a cold origin/CDN fetch. Aligned, the pair - * sorts adjacently in `RenderQueue.claim`'s nextRenderTime order and is rendered back-to-back - * by one worker off a warm origin. Residency already groups them this way (`schedulerNode` - * comes from the URL alone), so the seed now agrees with the routing. - * - * Alignment persists cycle over cycle because `processJobResult` reschedules from - * `currentMinuteMs() + interval` — both variants completing within the same minute get an - * identical next time, so the pair stays locked instead of drifting. + * The jitter offset is seeded off the URL, never off a whole cache key. Schedule rows are keyed by + * URL now (one row, every device in one job), so the seed IS the key; the URL-half rule below is + * what keeps a pre-0.66.0 per-device row, and a deliberate one-device row (`renderNow` for a + * device outside `deviceTypes.default`), on the same minute as the URL row they belong to. Seeded + * off the full cacheKey, `desktop` and `mobile` hashed to unrelated offsets and drifted up to a + * whole interval apart — the two copies of a page could differ in age by up to 24h. Residency + * already groups a URL's rows this way (`schedulerNode` comes from the URL alone), so the seed + * agrees with the routing. */ const jitterSeed = (key) => { const str = String(key ?? ''); diff --git a/packages/plugin/test/pageVerification.test.js b/packages/plugin/test/pageVerification.test.js index 2106905..b462889 100644 --- a/packages/plugin/test/pageVerification.test.js +++ b/packages/plugin/test/pageVerification.test.js @@ -215,11 +215,12 @@ test('with no epoch at all, a verification changes nothing', () => { // ---- the per-device basis: what makes a per-URL verification safe for per-cacheKey pages -------- /** - * `pageSignature` is keyed by url and written by whichever device rendered LAST, so the proof - * belongs to one render while the pages are per-cacheKey. A SPLIT PAIR is a normal state here — - * `PrerenderAdmin.revalidateUrl` and `renderNow` each write one device key on purpose, reconcile - * repairs a missing row with fresh jitter, and every per-device retry lane diverges the pair. So a - * bare per-URL exemption would serve a stale sibling on the strength of the other device's proof. + * `pageSignature` is keyed by url and written once per result, so the proof belongs to one render + * while the pages are per-cacheKey. Since v0.66.0 a URL's devices render in one job and share one + * `lastCached`, so the pair is aligned by construction — but a SPLIT PAIR is still reachable: a + * pre-0.66.0 per-device row that has not yet converted, a partial result whose missing device kept + * its older page, a one-device render-now. So a bare per-URL exemption would serve a stale sibling + * on the strength of the other device's proof, and the basis stays. */ test('the VERIFIED render is exempt', () => { diff --git a/packages/plugin/test/reconcile.test.js b/packages/plugin/test/reconcile.test.js index e8fc096..5a1b092 100644 --- a/packages/plugin/test/reconcile.test.js +++ b/packages/plugin/test/reconcile.test.js @@ -55,7 +55,8 @@ afterEach(() => { /** * A fake registry. `owners` maps a URL to its owning node so a test can place rows on either - * side of the residency boundary; `schedules` is the set of cacheKeys that HAVE a row. + * side of the residency boundary; `schedules` is the set of schedule keys that HAVE a row — a URL + * (the row every target should have) or a pre-0.66.0 `url|device` cacheKey. */ const harness = ({ targets, @@ -117,38 +118,52 @@ test('a target missing its schedule row gets one restored', async () => { assert.equal(stats.restored, 1); assert.equal(h.puts.length, 1); - assert.equal(h.puts[0].cacheKey, 'https://x/a|desktop'); + assert.equal(h.puts[0].cacheKey, 'https://x/a', 'the row is keyed by the URL — one row, every device'); }); test('a target that already has a schedule row is left alone', async () => { const h = harness({ targets: [{ url: 'https://x/a', renderInterval: 60000 }], - schedules: ['https://x/a|desktop'], + schedules: ['https://x/a'], }); const stats = await h.run(); assert.equal(stats.restored, 0); assert.equal(h.puts.length, 0); + assert.deepEqual(h.scheduleReads, ['https://x/a'], 'a present URL row settles it in one read'); }); -test('every configured device is checked, and only the missing one is restored', async () => { - // One URL row implies one schedule row PER device. A half-scheduled URL — desktop present, - // mobile missing — is exactly as silent as a fully missing one and must be repaired - // without disturbing the sibling that is fine. +test('a URL still scheduled under a pre-0.66.0 device row is NOT restored beside it', async () => { + // A per-device row converts into the URL row the first time it renders. Until then the URL IS + // scheduled, and restoring a URL row next to it would render the URL twice for a cycle. The + // device reads run only when the URL row is missing, so a converged corpus never pays them. + const h = harness({ + targets: [{ url: 'https://x/a', renderInterval: 60000 }], + deviceTypes: ['desktop', 'mobile'], + schedules: ['https://x/a|mobile'], + }); + + const stats = await h.run(); + + assert.deepEqual(h.scheduleReads, ['https://x/a', 'https://x/a|desktop', 'https://x/a|mobile']); + assert.equal(stats.missing, 0); + assert.deepEqual(h.puts, []); +}); + +test('a URL with neither a URL row nor any device row is restored under the URL', async () => { const h = harness({ targets: [{ url: 'https://x/a', renderInterval: 60000 }], deviceTypes: ['desktop', 'mobile'], - schedules: ['https://x/a|desktop'], }); const stats = await h.run(); - assert.deepEqual(h.scheduleReads.sort(), ['https://x/a|desktop', 'https://x/a|mobile']); + assert.deepEqual(h.scheduleReads, ['https://x/a', 'https://x/a|desktop', 'https://x/a|mobile']); assert.equal(stats.missing, 1); assert.deepEqual( h.puts.map((p) => p.cacheKey), - ['https://x/a|mobile'] + ['https://x/a'] ); }); @@ -168,16 +183,17 @@ test('keys owned by another node are never even asked about', async () => { assert.equal(stats.examined, 2); assert.equal(stats.owned, 1); - assert.deepEqual(h.scheduleReads, ['https://x/mine|desktop']); + assert.deepEqual(h.scheduleReads, ['https://x/mine', 'https://x/mine|desktop']); assert.deepEqual( h.puts.map((p) => p.cacheKey), - ['https://x/mine|desktop'] + ['https://x/mine'] ); }); -test('residency is asked once per URL, and both device rows live with that owner', async () => { - // RenderSchedule.setResidencyById hashes the URL half of the cacheKey, so the same URL on - // two device types lands on the SAME node — one ownership answer covers the whole fan-out. +test('residency is asked once per URL, and every row of the URL lives with that owner', async () => { + // RenderSchedule.setResidencyById hashes the URL (the URL half, for a pre-0.66.0 cacheKey), so + // the URL row and any leftover device row land on the SAME node — one ownership answer covers + // every key that is checked. const seen = []; const h = harness({ targets: [{ url: 'https://x/a', renderInterval: 60000 }], @@ -192,7 +208,7 @@ test('residency is asked once per URL, and both device rows live with that owner }); assert.deepEqual(seen, ['https://x/a']); - assert.deepEqual(h.scheduleReads.sort(), ['https://x/a|desktop', 'https://x/a|mobile']); + assert.deepEqual(h.scheduleReads.sort(), ['https://x/a', 'https://x/a|desktop', 'https://x/a|mobile']); }); test('restores at the jittered initial time, not now', async () => { @@ -244,8 +260,8 @@ test('fromSitemap is carried over from the target', async () => { await h.run(); - assert.equal(h.puts.find((p) => p.cacheKey === 'https://x/a|desktop').fromSitemap, true); - assert.equal(h.puts.find((p) => p.cacheKey === 'https://x/b|desktop').fromSitemap, false); + assert.equal(h.puts.find((p) => p.cacheKey === 'https://x/a').fromSitemap, true); + assert.equal(h.puts.find((p) => p.cacheKey === 'https://x/b').fromSitemap, false); }); test('the walk pages through every target rather than stopping at the first batch', async () => { @@ -370,8 +386,8 @@ test('the live query asks for no sort — Harper rejects sorting by the primary const stats = await reconcile.reconcileScheduleGaps({ maxRestores: 10 }); assert.equal(stats.examined, 2); - // config.deviceTypes.default is ['desktop', 'mobile'], so two URLs fan out to four rows. - assert.equal(stats.restored, 4); + // One row per URL, whatever `config.deviceTypes.default` says: two URLs, two rows. + assert.equal(stats.restored, 2); // Exactly one scan: no paging, so no cursor and no resumption. assert.equal(searches.length, 1); diff --git a/packages/plugin/test/renderQueueRedirect.test.js b/packages/plugin/test/renderQueueRedirect.test.js index 84aea2a..9c50e99 100644 --- a/packages/plugin/test/renderQueueRedirect.test.js +++ b/packages/plugin/test/renderQueueRedirect.test.js @@ -5,8 +5,12 @@ import assert from 'node:assert/strict'; * processJobResult over the url-keyed Target registry. * * The properties pinned here: - * - Targets are ONE row per URL; put/delete/suppress fan out over the configured devices' - * RenderSchedule and PrerenderedPage rows. + * - Targets are ONE row per URL, and so is the RenderSchedule row (keyed by the URL — one job + * renders every device); only PrerenderedPage fans out per device. `seedSource` seeds the + * PRE-0.66.0 shape — one schedule row per device — and posts legacy per-device results, so + * these tests also pin the conversion: a per-device row's result writes the URL row and + * retires the device row. The multi-device result shape is pinned in + * test/renderQueueVariants.test.js. * - A non-indexable verdict SUPPRESSES the target (state + strikes + recheck schedule, * cached pages dropped) instead of deleting it — and `maxStrikes` consecutive verdicts * delete it outright. A later indexable render lifts the suppression. @@ -190,6 +194,7 @@ test('301 onto a served route retires the source URL — all devices — and ado await postResult({ id: key(A), url: A, statusCode: 301, outcome: 'redirected', redirectedTo: B, renderTime: 42 }); assert.equal(stores.target.has(A), false, 'source target must be retired'); + assert.equal(stores.renderSchedule.has(A), false, 'the URL row must not be created for a retired source'); for (const device of DEVICES) { assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule must be dropped`); assert.equal(stores.prerenderedPage.has(key(A, device)), false, `${device} cached page must be dropped`); @@ -200,10 +205,11 @@ test('301 onto a served route retires the source URL — all devices — and ado assert.equal(adopted.renderInterval, 1234567, 'cadence is inherited — the page moved, its schedule did not'); assert.notEqual(adopted.state, 'suppressed'); + const schedule = stores.renderSchedule.get(B); + assert.ok(schedule, 'destination must be scheduled — ONE row, keyed by the URL, every device in one job'); + assert.ok(schedule.nextRenderTime <= Date.now(), 'due now — the source pages are gone, fill the gap fast'); for (const device of DEVICES) { - const schedule = stores.renderSchedule.get(key(B, device)); - assert.ok(schedule, `destination must be scheduled for ${device}`); - assert.ok(schedule.nextRenderTime <= Date.now(), 'due now — the source pages are gone, fill the gap fast'); + assert.equal(stores.renderSchedule.has(key(B, device)), false, 'no per-device rows are created any more'); } // One time_ms sample plus exactly one outcome — the emit-once-per-result contract, both @@ -234,14 +240,14 @@ test('a rendered verdict with nothing to store is counted as no-content, not sto test('301 onto an already-targeted destination adopts nothing and leaves its schedule alone', async () => { seedSource(); stores.target.set(B, { url: B, renderInterval: 999 }); - stores.renderSchedule.set(key(B), { nextRenderTime: 8_888_888_888_888 }); + stores.renderSchedule.set(B, { nextRenderTime: 8_888_888_888_888 }); await postResult({ id: key(A), url: A, statusCode: 301, outcome: 'redirected', redirectedTo: B }); assert.equal(stores.target.has(A), false, 'source is still retired'); assert.equal(stores.target.get(B).renderInterval, 999, 'existing destination target untouched'); assert.equal( - stores.renderSchedule.get(key(B)).nextRenderTime, + stores.renderSchedule.get(B).nextRenderTime, 8_888_888_888_888, 'existing destination cadence must not be perturbed' ); @@ -267,7 +273,9 @@ test('temporary redirect (302) keeps the source — target, cached pages — and assert.ok(stores.prerenderedPage.has(key(A)), 'the cached page keeps serving while the redirect heals'); assert.equal(stores.target.has(B), false, 'a temporary destination is not adopted'); - const schedule = stores.renderSchedule.get(key(A)); + // The reschedule lands on the URL row — the per-device row this result came from has converted. + const schedule = stores.renderSchedule.get(A); + assert.equal(stores.renderSchedule.has(key(A)), false, 'the pre-0.66.0 device row is retired on conversion'); // nextRenderTime is minute-floored "now" + interval. const flooredBefore = Math.floor(before / 60_000) * 60_000; assert.ok( @@ -300,7 +308,7 @@ test('redirect onto an unrouted path keeps the source and adopts nothing', async await postResult({ id: key(A), url: A, statusCode: 301, outcome: 'redirected', redirectedTo: off }); assert.ok(stores.target.has(A), 'incomplete route list must not end this URL for good'); - assert.ok(stores.renderSchedule.has(key(A)), 'source stays in rotation'); + assert.ok(stores.renderSchedule.has(A), 'source stays in rotation (on its URL row)'); assert.equal(stores.target.has(off), false); }); @@ -339,7 +347,7 @@ test('outcome=redirected without permanence (client-side, 200) keeps the source' await postResult({ id: key(A), url: A, statusCode: 200, outcome: 'redirected', redirectedTo: B }); assert.ok(stores.target.has(A), 'no proof of permanence — the source stays'); - assert.ok(stores.renderSchedule.has(key(A)), 'and stays scheduled'); + assert.ok(stores.renderSchedule.has(A), 'and stays scheduled'); assert.equal(stores.target.has(B), false); }); @@ -355,7 +363,9 @@ test('outcome=rendered stores the page and reschedules', async () => { const page = stores.prerenderedPage.get(key(A)); assert.ok(page, 'content must be stored'); assert.equal(page.isIndexable, true); - assert.ok(stores.renderSchedule.get(key(A)).nextRenderTime > Date.now(), 'rescheduled one interval out'); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'rescheduled one interval out'); + assert.equal(stores.renderSchedule.has(key(A)), false, 'the device row folded into the URL row'); + assert.ok(stores.renderSchedule.has(key(A, 'mobile')), 'the sibling converts on its own render'); }); // ---- route-typed render cadence ---- @@ -371,7 +381,7 @@ test('outcome=rendered reschedules at the matched route renderInterval, beating 'fresh' ); - const schedule = stores.renderSchedule.get(key(A)); + const schedule = stores.renderSchedule.get(A); // ≈ now + 6h (minute-floored), NOT now + the stored 24h — route cadence is retroactive. assert.ok(schedule.nextRenderTime >= before + 6 * HOUR_MS - 60_000, 'due no earlier than ~6h out'); assert.ok(schedule.nextRenderTime < before + 7 * HOUR_MS, 'stored 24h interval must not win'); @@ -391,7 +401,7 @@ test('outcome=rendered keeps the stored interval when the matched route sets no 'fresh' ); - const schedule = stores.renderSchedule.get(key(A)); + const schedule = stores.renderSchedule.get(A); assert.ok(schedule.nextRenderTime >= before + 2 * HOUR_MS - 60_000, 'stored cadence still drives'); assert.ok(schedule.nextRenderTime < before + 3 * HOUR_MS, 'default must not win over a valid stored interval'); }); @@ -404,6 +414,7 @@ test('outcome=rendered with a landed URL that keys elsewhere keeps the refile se ); assert.equal(stores.target.has(A), false, 'source target retired by the refile'); + assert.equal(stores.renderSchedule.has(A), false, 'no URL row is written for a retired source'); for (const device of DEVICES) { assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule retired with it`); } @@ -433,16 +444,17 @@ test('a non-indexable verdict suppresses the target: state, strikes, recheck sch for (const device of DEVICES) { assert.equal(stores.prerenderedPage.has(key(A, device)), false, `${device} cached page must be dropped`); - const schedule = stores.renderSchedule.get(key(A, device)); - assert.ok( - schedule.nextRenderTime >= before + config.render.suppression.recheckInterval - 60_000, - `${device} rescheduled at the recheck interval, not the render interval` - ); } + const schedule = stores.renderSchedule.get(A); + assert.ok( + schedule.nextRenderTime >= before + config.render.suppression.recheckInterval - 60_000, + 'the URL row is rescheduled at the recheck interval, not the render interval' + ); + assert.equal(stores.renderSchedule.has(key(A)), false, 'the device row this verdict came from is retired'); // info, not warn, since the log relevel: a suppression is a normal verdict, and the // alertable aggregate is the render_outcome counter (asserted below). assert.ok( - infos.some((w) => w.includes('Suppressing') && w.includes('(noindex)')), + infos.some((w) => w.includes('Suppressing') && w.includes('noindex')), `expected a suppression info line naming the reason, got: ${infos.join(' | ')}` ); assert.ok( @@ -465,8 +477,9 @@ test('maxStrikes consecutive non-indexable verdicts delete the target outright', } assert.equal(stores.target.has(A), false, 'strike limit reached — the target is gone'); + assert.equal(stores.renderSchedule.has(A), false, 'and its schedule row with it'); for (const device of DEVICES) { - assert.equal(stores.renderSchedule.has(key(A, device)), false, 'and its schedules with it'); + assert.equal(stores.renderSchedule.has(key(A, device)), false, 'and any leftover device rows'); } }); @@ -604,7 +617,7 @@ test('a temp redirect strikes the source but keeps it (and its cached page) belo assert.ok(target, 'source survives a first temp redirect'); assert.equal(target.strikes, 1, 'but the strike is recorded'); assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'cached page keeps serving'); - assert.ok(stores.renderSchedule.get(key(A)).nextRenderTime > Date.now(), 'retry scheduled at cadence'); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'retry scheduled at cadence'); }); test('maxStrikes consecutive temp redirects retire the source outright', async () => { @@ -615,6 +628,7 @@ test('maxStrikes consecutive temp redirects retire the source outright', async ( await postResult({ id: key(A), url: A, statusCode: 302, outcome: 'redirected', redirectedTo: B }); assert.equal(stores.target.has(A), false, 'source retired — the temporary status was a lie'); + assert.equal(stores.renderSchedule.has(A), false, 'schedule row dropped'); for (const device of DEVICES) { assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule dropped`); assert.equal( diff --git a/packages/plugin/test/renderQueueVariants.test.js b/packages/plugin/test/renderQueueVariants.test.js new file mode 100644 index 0000000..4e0cd7d --- /dev/null +++ b/packages/plugin/test/renderQueueVariants.test.js @@ -0,0 +1,569 @@ +import { test, before, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; + +/** + * ONE JOB PER URL, ONE RESULT, ONE SCHEDULING DECISION (plugin v0.66.0 / browser v1.23.0). + * + * The schedule row is keyed by the URL. `claim` hands out one job per row carrying every device in + * `deviceTypes.default`; the browser renders them in turn and posts `{ id, url, deviceTypes, + * variants: [...] }` with the variants' bodies concatenated behind the JSON. What is pinned here: + * + * - the claim job shape, for a URL row and for a pre-0.66.0 per-device row; + * - the result framing (`contentLength` per variant) and its refusal when it does not add up; + * - the precedence across variants: a redirect decides the URL, a genuine non-indexable verdict + * suppresses it, rendered pages are stored per device, a failed device puts the URL in the retry + * lanes with the good pages kept, and all-rendered reschedules ONCE; + * - a device the browser was asked for and did not post back is a failure, not a silent skip; + * - strikes: one result per URL means one strike per failed cycle; + * - the conversion of pre-0.66.0 per-device rows into the URL row, at no extra renders; + * - a one-device row for a device outside the default set is a one-off beside the rotation; + * - an older renderer answering a URL job (flat result, no device) is attributed to the first + * default device — degraded, never lost. + */ + +const A = 'https://site.example.com/product/a'; +const B = 'https://site.example.com/product/b'; +const key = (url, device) => `${url}|${device}`; +const DEVICES = ['desktop', 'mobile']; + +const stores = { target: new Map(), renderSchedule: new Map(), prerenderedPage: new Map() }; +let warns = []; +let infos = []; +let errors = []; +let analytics = []; + +const makeResourceBase = (rows) => + class FakeResource { + constructor(id) { + this.__id = id; + } + getId() { + return this.__id; + } + async put(data) { + rows.set(this.__id, { ...data }); + } + async delete() { + return rows.delete(this.__id); + } + static async get(query) { + const id = typeof query === 'object' ? query.id : query; + const row = rows.get(id); + if (!row) return null; + const select = typeof query === 'object' ? query.select : undefined; + if (typeof select === 'string') return row[select]; + if (Array.isArray(select)) return Object.fromEntries(select.map((name) => [name, row[name]])); + return { ...row }; + } + static async put(id, data) { + return new this(id).put({ ...data }); + } + static async patch(id, data) { + rows.set(id, { ...(rows.get(id) ?? {}), ...data }); + } + static async delete(id) { + return new this(id).delete(); + } + static async *search(query = {}) { + const [condition] = query.conditions ?? []; + const floor = condition ? Number(condition.value) : Number.NEGATIVE_INFINITY; + const matching = [...rows.entries()] + .map(([cacheKey, row]) => ({ cacheKey, ...row })) + .filter((row) => Number(row.nextRenderTime) >= floor) + .sort((a, b) => Number(a.nextRenderTime) - Number(b.nextRenderTime)) + .slice(0, query.limit ?? Infinity); + for (const row of matching) yield row; + } + }; + +let RenderQueue, config, funnel; +const sabs = new Map(); + +before(async () => { + globalThis.Resource = class {}; + globalThis.server = { + hostname: 'test-node', + nodes: [], + config: { http: { port: 9926 } }, + recordAnalytics: (...args) => analytics.push(args), + }; + globalThis.logger = { + debug() {}, + info: (msg) => infos.push(String(msg)), + warn: (msg) => warns.push(String(msg)), + error: (msg) => errors.push(String(msg)), + }; + globalThis.createBlob = (buf) => buf; + globalThis.databases = { + probe_state: { ProbeState: { delete: async () => {} } }, + coordination: { + SharedBuffer: { + primaryStore: { + getUserSharedBuffer: (k, buf) => { + if (!sabs.has(k)) sabs.set(k, buf); + return sabs.get(k); + }, + tryLock: () => true, + unlock() {}, + }, + }, + }, + render_service: { + Target: makeResourceBase(stores.target), + QueueControl: makeResourceBase(new Map()), + QueueStatus: makeResourceBase(new Map()), + }, + render_schedule: { RenderSchedule: makeResourceBase(stores.renderSchedule) }, + page_cache: { PrerenderedPage: makeResourceBase(stores.prerenderedPage) }, + }; + + ({ config } = await import('../src/config.js')); + ({ RenderQueue } = await import('../src/resources/RenderQueue.js')); + funnel = await import('../src/util/renderSchedule.js'); +}); + +beforeEach(() => { + for (const rows of Object.values(stores)) rows.clear(); + warns = []; + infos = []; + errors = []; + analytics = []; + funnel.resetRenderQueueState(); +}); + +afterEach(() => { + config.domains = []; + config.ingress.mode = 'prefix'; + config.ingress.routes = []; + config.deviceTypes.default = ['desktop', 'mobile']; + config.deviceTypes.supported = ['desktop', 'mobile', 'tablet']; +}); + +const claim = (limit = 10) => RenderQueue.claim({ limit }); +const leased = (k) => !!funnel.leaseInfo(k); +const outcomes = () => analytics.filter((a) => a[1] === 'render' && a[2] === 'outcome').map((a) => [a[3], a[4]]); +const renderTimes = () => analytics.filter((a) => a[1] === 'render' && a[2] === 'time_ms'); + +/** Post a result the way the browser's `postResult` frames it: JSON envelope, then the bodies. */ +const post = async (envelope, bodies = []) => { + const meta = Buffer.from(JSON.stringify(envelope), 'utf8'); + const body = Buffer.concat([meta, ...bodies.map((b) => (Buffer.isBuffer(b) ? b : Buffer.from(b)))]); + const ctx = { headers: new Map([['x-metadata-size', String(meta.byteLength)]]) }; + return RenderQueue.processJobResult(body, ctx); +}; + +/** A multi-device result for `url`: `variants` are `{ deviceType, ...metadata, content? }`. */ +const postVariants = async (url, variants, { deviceTypes = DEVICES, id = url } = {}) => { + const bodies = []; + const wire = variants.map(({ content, ...metadata }) => { + if (content) bodies.push(Buffer.from(content)); + return { ...metadata, contentLength: content ? Buffer.byteLength(content) : 0 }; + }); + return post({ id, url, deviceTypes, variants: wire }, bodies); +}; + +const rendered = (deviceType, content = `${deviceType}`, extra = {}) => ({ + deviceType, + statusCode: 200, + outcome: 'rendered', + isIndexable: true, + headers: {}, + renderTime: 100, + structuredOffers: null, + content, + ...extra, +}); + +const seedUrlRow = ({ url = A, renderInterval = 3_600_000, nextRenderTime = 1, state, strikes } = {}) => { + stores.target.set(url, { + url, + renderInterval, + ...(state ? { state } : {}), + ...(Number.isFinite(strikes) ? { strikes } : {}), + }); + stores.renderSchedule.set(url, { nextRenderTime, fromSitemap: false, effectiveInterval: renderInterval }); + for (const device of DEVICES) stores.prerenderedPage.set(key(url, device), { statusCode: 200, content: 'old html' }); +}; + +// ───────────────────────────── claim ───────────────────────────── + +test('a URL row claims as ONE job naming every default device, with deviceType for older renderers', async () => { + seedUrlRow(); + const jobs = await claim(); + assert.equal(jobs.length, 1, 'one URL, one job — not one per device'); + const [job] = jobs; + assert.equal(job.id, A, 'the row key, echoed back by the browser'); + assert.equal(job.url, A); + assert.deepEqual(job.deviceTypes, ['desktop', 'mobile']); + assert.equal(job.deviceType, 'desktop', 'the first device, for a renderer that predates the list'); + assert.equal(leased(A), true, 'leased under the row key'); +}); + +test('a pre-0.66.0 per-device row claims as a job for exactly the device its key names', async () => { + stores.target.set(A, { url: A, renderInterval: 3_600_000 }); + stores.renderSchedule.set(key(A, 'mobile'), { nextRenderTime: 1, fromSitemap: true }); + const [job] = await claim(); + assert.equal(job.id, key(A, 'mobile')); + assert.equal(job.url, A, 'the URL, not the cacheKey'); + assert.deepEqual(job.deviceTypes, ['mobile']); + assert.equal(job.deviceType, 'mobile'); + assert.equal(job.isFromSitemap, true); +}); + +test('an emptied deviceTypes.default still claims one device rather than a job the browser cannot act on', async () => { + config.deviceTypes.default = []; + seedUrlRow(); + const [job] = await claim(); + assert.deepEqual(job.deviceTypes, ['desktop'], 'the first supported device'); +}); + +// ───────────────────────────── framing ───────────────────────────── + +test('a multi-device result is decoded by walking contentLength per variant', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + rendered('desktop', 'desktop'), + rendered('mobile', 'mobile — longer'), + ]); + + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'desktop'); + assert.equal(stores.prerenderedPage.get(key(A, 'mobile')).content.toString(), 'mobile — longer'); +}); + +test('a result whose contentLengths do not account for its body is refused with a 400', async () => { + seedUrlRow(); + for (const wire of [ + // claims more than is there + { variants: [{ deviceType: 'desktop', outcome: 'rendered', statusCode: 200, contentLength: 999 }], body: 'short' }, + // leaves bytes unattributed + { variants: [{ deviceType: 'desktop', outcome: 'rendered', statusCode: 200, contentLength: 2 }], body: 'longer' }, + ]) { + const response = await post({ id: A, url: A, deviceTypes: ['desktop'], variants: wire.variants }, [wire.body]); + assert.equal(response?.status, 400, 'a body that cannot be attributed to devices is refused'); + } + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content, 'old html', 'and nothing was stored'); +}); + +// ───────────────────────────── precedence ───────────────────────────── + +test('every device rendered: pages stored per device, ONE reschedule, ONE outcome, one page claim', async () => { + seedUrlRow({ renderInterval: 3_600_000, strikes: 2 }); + await claim(); + const before = Date.now(); + await postVariants(A, [ + rendered('desktop', 'd', { structuredOffers: ['9.99', 'USD', 'InStock'] }), + rendered('mobile', 'm', { structuredOffers: ['9.99', 'USD', 'InStock'] }), + ]); + + const desktop = stores.prerenderedPage.get(key(A, 'desktop')); + const mobile = stores.prerenderedPage.get(key(A, 'mobile')); + assert.equal(desktop.content.toString(), 'd'); + assert.equal(mobile.content.toString(), 'm'); + assert.equal(desktop.lastCached, mobile.lastCached, 'ONE timestamp for the pair — the verification basis is exact'); + assert.equal(JSON.parse(desktop.headers)['x-harper-rendered'], '1'); + + const schedule = stores.renderSchedule.get(A); + assert.ok(schedule.nextRenderTime >= before + 3_600_000 - 60_000, 'rescheduled one interval out'); + assert.equal(desktop.expiresAt, schedule.nextRenderTime, 'page expiry coupled to the next render'); + assert.equal(mobile.expiresAt, schedule.nextRenderTime); + assert.equal(stores.renderSchedule.size, 1, 'exactly one schedule row for the URL'); + + assert.equal(stores.target.get(A).strikes, 0, 'a successful render clears strikes'); + assert.deepEqual(outcomes(), [['rendered', 'stored']], 'exactly one outcome per posted result'); + assert.equal(renderTimes().length, 2, 'but one time_ms sample per rendered device'); + assert.equal(leased(A), false, 'the lease is released'); +}); + +test('a device that failed puts the URL in the fast lane while the device that rendered is stored', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + rendered('desktop', 'fresh'), + { + deviceType: 'mobile', + outcome: 'error', + reason: 'error', + error: { name: 'TimeoutError', message: 'settle', phase: 'settle' }, + }, + ]); + + assert.equal( + stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), + 'fresh', + 'good side stored' + ); + assert.equal( + stores.prerenderedPage.get(key(A, 'mobile')).content, + 'old html', + 'failed side keeps its last good page' + ); + assert.equal(stores.target.get(A).strikes, 1, 'ONE strike for the URL, not one per failed device'); + assert.equal(stores.renderSchedule.get(A).nextRenderTime, 1, 'schedule untouched — the lease paces the retry'); + assert.equal(leased(A), true, 'the fast lane HOLDS the lease'); + assert.deepEqual(await claim(), [], 'so nothing re-claims it now'); + assert.deepEqual(outcomes(), [['failed', 'settle']]); + assert.ok(warns.some((w) => w.includes('(mobile)') && w.includes('TimeoutError'))); +}); + +test('both devices failing costs ONE strike — two per-device results used to cost two', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + { + deviceType: 'desktop', + outcome: 'error', + reason: 'error', + error: { name: 'Error', message: 'x', phase: 'navigation' }, + }, + { + deviceType: 'mobile', + outcome: 'error', + reason: 'error', + error: { name: 'Error', message: 'y', phase: 'settle' }, + }, + ]); + assert.equal(stores.target.get(A).strikes, 1); + assert.deepEqual(outcomes(), [['failed', 'navigation']], 'the first failed variant names the outcome'); +}); + +test('an auth-shaped device outranks a transient one in the outcome, and neither suppresses', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + { deviceType: 'desktop', outcome: 'non-indexable', isIndexable: false, statusCode: 503, reason: 'http-error' }, + { deviceType: 'mobile', outcome: 'non-indexable', isIndexable: false, statusCode: 403, reason: 'http-error' }, + ]); + assert.notEqual(stores.target.get(A).state, 'suppressed'); + assert.deepEqual(outcomes(), [['auth-failure', 403]]); + assert.ok(errors.some((e) => e.includes('403') && e.includes('(mobile)'))); + assert.ok(infos.some((i) => i.includes('503') && i.includes('(desktop)'))); + assert.equal(leased(A), true); +}); + +test('a genuine non-indexable verdict on ANY device suppresses the URL, storing nothing', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + rendered('desktop', 'fresh'), + { deviceType: 'mobile', outcome: 'non-indexable', isIndexable: false, statusCode: 200, reason: 'noindex' }, + ]); + + const target = stores.target.get(A); + assert.equal(target.state, 'suppressed'); + assert.equal(target.suppressedReason, 'noindex'); + for (const device of DEVICES) { + assert.equal( + stores.prerenderedPage.has(key(A, device)), + false, + `${device} page dropped — the fresh desktop one too` + ); + } + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now() + 3_600_000, 'rescheduled at the recheck cadence'); + assert.deepEqual(outcomes(), [['suppressed', 'noindex']]); + assert.ok(infos.some((i) => i.includes('Suppressing') && i.includes('(mobile, noindex)'))); +}); + +test('a redirect the browser bailed on, on ANY device, decides the URL: 301 retires it and adopts the destination', async () => { + seedUrlRow({ renderInterval: 1234567 }); + await claim(); + await postVariants(A, [ + rendered('desktop', 'fresh'), + { deviceType: 'mobile', outcome: 'redirected', statusCode: 301, redirectedTo: B, renderTime: 40 }, + ]); + + assert.equal(stores.target.has(A), false, 'a page does not redirect for one device and serve for another'); + assert.equal(stores.renderSchedule.has(A), false); + for (const device of DEVICES) assert.equal(stores.prerenderedPage.has(key(A, device)), false); + assert.equal(stores.target.get(B)?.renderInterval, 1234567, 'destination adopted with the cadence'); + assert.ok(stores.renderSchedule.get(B), 'and scheduled — one URL row'); + assert.deepEqual(outcomes(), [['redirect', 'permanent']]); + const times = renderTimes(); + assert.equal(times.length, 2); + assert.ok( + times.some((t) => t[4] === 'redirect'), + 'the bail-at-nav variant is timed in its own lane' + ); + assert.equal(leased(A), false); +}); + +test('a temporary redirect on one device strikes the URL and reschedules it at cadence', async () => { + seedUrlRow({ renderInterval: 60_000 }); + await claim(); + await postVariants(A, [ + rendered('desktop', 'fresh'), + { deviceType: 'mobile', outcome: 'redirected', statusCode: 302, redirectedTo: B }, + ]); + assert.equal(stores.target.get(A).strikes, 1); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'one reschedule, on the URL row'); + assert.equal(stores.target.has(B), false); + assert.deepEqual(outcomes(), [['redirect', 'temporary']]); +}); + +test('a rendered client-side redirect refiles under the destination keys and retires the source once', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [ + rendered('desktop', 'landed d', { redirectedTo: B }), + rendered('mobile', 'landed m', { redirectedTo: B }), + ]); + assert.equal(stores.target.has(A), false, 'source retired by the refile'); + assert.equal(stores.renderSchedule.has(A), false); + assert.equal(stores.prerenderedPage.get(key(B, 'desktop')).content.toString(), 'landed d'); + assert.equal(stores.prerenderedPage.get(key(B, 'mobile')).content.toString(), 'landed m'); + assert.deepEqual(outcomes(), [['rendered', 'refiled']]); + assert.equal(leased(A), false, 'the SOURCE lease — the one that was granted — is released'); +}); + +// ───────────────────────────── partial results ───────────────────────────── + +test('a device asked for and not posted back is a failure: good page stored, URL retries on the lease', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [rendered('desktop', 'fresh')], { deviceTypes: ['desktop', 'mobile'] }); + + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'fresh'); + assert.equal(stores.prerenderedPage.get(key(A, 'mobile')).content, 'old html'); + assert.equal(stores.target.get(A).strikes, 1); + assert.equal(leased(A), true, 'held — the lease expiry re-renders the URL, mobile included'); + assert.deepEqual(outcomes(), [['failed', 'not-attempted']]); + assert.ok(warns.some((w) => w.includes('(mobile)') && w.includes('not-attempted'))); +}); + +// ───────────────────────────── conversion of pre-0.66.0 rows ───────────────────────────── + +test('a per-device row folds into the URL row on its result, and the sibling converts on its own — no extra renders', async () => { + stores.target.set(A, { url: A, renderInterval: 3_600_000 }); + stores.renderSchedule.set(key(A, 'desktop'), { nextRenderTime: 1, fromSitemap: false }); + stores.renderSchedule.set(key(A, 'mobile'), { nextRenderTime: 1, fromSitemap: false }); + + const jobs = await claim(); + assert.deepEqual( + jobs.map((j) => [j.id, j.deviceTypes]).sort(), + [ + [key(A, 'desktop'), ['desktop']], + [key(A, 'mobile'), ['mobile']], + ], + 'each legacy row renders exactly its own device — as it always did' + ); + + // Desktop's result: URL row written, desktop row retired, mobile row untouched. + await postVariants(A, [rendered('desktop')], { id: key(A, 'desktop'), deviceTypes: ['desktop'] }); + assert.ok(stores.renderSchedule.get(A), 'the URL row now exists'); + assert.equal(stores.renderSchedule.has(key(A, 'desktop')), false, 'the desktop row is gone'); + assert.equal( + stores.renderSchedule.get(key(A, 'mobile')).nextRenderTime, + 1, + 'the mobile row waits for its own render' + ); + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'desktop'); + const firstDue = stores.renderSchedule.get(A).nextRenderTime; + + // Mobile's result: same URL row (rewritten a moment later), mobile row retired. Converged. + await postVariants(A, [rendered('mobile')], { id: key(A, 'mobile'), deviceTypes: ['mobile'] }); + assert.deepEqual([...stores.renderSchedule.keys()], [A], 'exactly one row remains, keyed by the URL'); + assert.ok(stores.renderSchedule.get(A).nextRenderTime >= firstDue); + assert.equal(stores.prerenderedPage.get(key(A, 'mobile')).content.toString(), 'mobile'); + assert.deepEqual(outcomes(), [ + ['rendered', 'stored'], + ['rendered', 'stored'], + ]); +}); + +test('a per-device row in the fast lane is NOT retired — the lease expiry must re-grant it', async () => { + stores.target.set(A, { url: A, renderInterval: 3_600_000 }); + stores.renderSchedule.set(key(A, 'desktop'), { nextRenderTime: 1, fromSitemap: false }); + await claim(); + await postVariants( + A, + [ + { + deviceType: 'desktop', + outcome: 'error', + reason: 'error', + error: { name: 'Error', message: 'x', phase: 'settle' }, + }, + ], + { id: key(A, 'desktop'), deviceTypes: ['desktop'] } + ); + assert.ok(stores.renderSchedule.has(key(A, 'desktop')), 'the row the lease paces stays'); + assert.equal(stores.renderSchedule.has(A), false, 'and no URL row is written yet'); + assert.equal(leased(key(A, 'desktop')), true); +}); + +test('a per-device row past fastRetries converts as it takes the slow lane', async () => { + const fast = config.render.failureRetry.fastRetries; + stores.target.set(A, { url: A, renderInterval: 3_600_000, strikes: fast }); + stores.renderSchedule.set(key(A, 'desktop'), { nextRenderTime: 1, fromSitemap: false }); + await claim(); + await postVariants( + A, + [ + { + deviceType: 'desktop', + outcome: 'error', + reason: 'error', + error: { name: 'Error', message: 'x', phase: 'settle' }, + }, + ], + { id: key(A, 'desktop'), deviceTypes: ['desktop'] } + ); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'the backoff lands on the URL row'); + assert.equal(stores.renderSchedule.has(key(A, 'desktop')), false, 'the device row is retired'); + assert.equal(leased(key(A, 'desktop')), false, 'slow lane releases'); +}); + +// ───────────────────────────── one-device rows beside the rotation ───────────────────────────── + +test('a per-device row for a NON-default device is a one-off: page stored, row retired, URL row untouched', async () => { + seedUrlRow({ nextRenderTime: 9_999_999_999_999 }); + stores.renderSchedule.set(key(A, 'tablet'), { nextRenderTime: 1, fromSitemap: false }); + const jobs = await claim(); + assert.deepEqual( + jobs.map((j) => j.id), + [key(A, 'tablet')], + 'only the tablet row is due' + ); + assert.deepEqual(jobs[0].deviceTypes, ['tablet']); + + await postVariants(A, [rendered('tablet')], { id: key(A, 'tablet'), deviceTypes: ['tablet'] }); + + assert.equal(stores.prerenderedPage.get(key(A, 'tablet')).content.toString(), 'tablet'); + assert.equal(stores.renderSchedule.has(key(A, 'tablet')), false, 'one render, then gone'); + assert.equal( + stores.renderSchedule.get(A).nextRenderTime, + 9_999_999_999_999, + 'the rotation is not re-anchored by a one-off' + ); + assert.equal(leased(key(A, 'tablet')), false); +}); + +test('a targetless URL row (render-now one-off) is dropped after its result, not retried forever', async () => { + stores.renderSchedule.set(A, { nextRenderTime: 1, fromSitemap: false }); + await claim(); + await postVariants(A, [rendered('desktop'), rendered('mobile')]); + assert.equal(stores.renderSchedule.has(A), false); + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'desktop'); + assert.equal(leased(A), false); + assert.deepEqual(await claim(), []); +}); + +// ───────────────────────────── older renderers ───────────────────────────── + +test('an older renderer answering a URL job posts flat and is attributed to the first default device', async () => { + seedUrlRow(); + await claim(); + // No `variants`, no `deviceType`: a pre-1.23.0 renderer rendered `job.deviceType` (desktop) and posted + // the legacy envelope with the row key as `id`. + await post({ id: A, url: A, statusCode: 200, outcome: 'rendered', isIndexable: true, headers: {} }, [ + 'old renderer', + ]); + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'old renderer'); + assert.equal( + stores.prerenderedPage.get(key(A, 'mobile')).content, + 'old html', + 'mobile was not rendered — degraded, not broken' + ); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'the URL is rescheduled'); + assert.deepEqual(outcomes(), [['rendered', 'stored']]); +}); diff --git a/packages/plugin/test/suppressionStatus.test.js b/packages/plugin/test/suppressionStatus.test.js index 8a2fb19..1bb3302 100644 --- a/packages/plugin/test/suppressionStatus.test.js +++ b/packages/plugin/test/suppressionStatus.test.js @@ -208,13 +208,16 @@ test('404 suppresses as http-gone at the gone recheck cadence', async () => { const goneRecheck = config.render.suppression.gone.recheckInterval; const defaultRecheck = config.render.suppression.recheckInterval; assert.ok(goneRecheck > defaultRecheck, 'precondition: gone rechecks are further apart than the default'); + // The recheck lands on the URL row — one row, every device — and the per-device row the verdict + // came from is retired with it. + const schedule = stores.renderSchedule.get(A); + assert.ok(schedule, 'the URL recheck schedule must exist'); + assert.ok( + schedule.nextRenderTime > Date.now() + defaultRecheck, + 'recheck must use the gone cadence, not the default' + ); + assert.equal(stores.renderSchedule.has(key(A)), false, 'the device row folded into the URL row'); for (const device of DEVICES) { - const schedule = stores.renderSchedule.get(key(A, device)); - assert.ok(schedule, `${device} recheck schedule must exist`); - assert.ok( - schedule.nextRenderTime > Date.now() + defaultRecheck, - 'recheck must use the gone cadence, not the default' - ); assert.equal(stores.prerenderedPage.has(key(A, device)), false, 'cached error page must not keep serving'); } }); @@ -233,6 +236,7 @@ test('gone deletes after gone.maxStrikes, sooner than the default maxStrikes', a await postResult(nonIndexable(404)); assert.equal(stores.target.has(A), false, 'target must be deleted at gone.maxStrikes'); + assert.equal(stores.renderSchedule.has(A), false, 'the URL schedule row must be gone'); for (const device of DEVICES) { assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule must be gone`); } @@ -367,9 +371,11 @@ test('past fastRetries, a failure drops to the target cadence — page kept but assert.equal(target.strikes, fast + 1); assert.notEqual(target.state, 'suppressed'); - const schedule = stores.renderSchedule.get(key(A)); + // The slow lane writes the URL row (and retires the device row this result came from). + const schedule = stores.renderSchedule.get(A); assert.ok(schedule.nextRenderTime > Date.now(), 'slow lane: rescheduled at the target cadence'); assert.ok(schedule.nextRenderTime < Date.now() + 2 * 3_600_000, 'cadence, not a suppression recheck'); + assert.equal(stores.renderSchedule.has(key(A)), false, 'the device row folded into the URL row'); // The slow lane RELEASES the lease. Holding it would pin the claim floor for a full jobLeaseTime // for a row that is now hours in the future — a latency cost for nothing. @@ -399,7 +405,7 @@ test('noindex (status 200) still suppresses under the default knobs with its own assert.equal(target.suppressedReason, 'noindex'); const goneRecheck = config.render.suppression.gone.recheckInterval; - const schedule = stores.renderSchedule.get(key(A)); + const schedule = stores.renderSchedule.get(A); assert.ok(schedule.nextRenderTime < Date.now() + goneRecheck, 'default recheck cadence, not the gone one'); }); @@ -472,7 +478,7 @@ test('a BigInt strikes value (Harper numeric surfacing) still counts toward the await postResult(nonIndexable(503)); assert.equal(stores.target.get(A).strikes, fast + 1, 'BigInt count read correctly, not reset to 1'); - assert.ok(stores.renderSchedule.get(key(A)).nextRenderTime > Date.now(), 'transitioned to the slow lane'); + assert.ok(stores.renderSchedule.get(A).nextRenderTime > Date.now(), 'transitioned to the slow lane'); }); // ---- a THROW out of result handling ---- @@ -486,8 +492,8 @@ test('a throw while handling a result HOLDS the lease — it must not become a r // UNPACED re-render loop against whatever is throwing, at claim frequency rather than once per // lease. // - // The throw here is a real one, not an injected stub: a rendered result with content but no - // `headers` object, which the store path stamps `x-harper-rendered` onto. + // The throw is the page store rejecting mid-result — a blob write failing is the reachable shape. + // (A result with no `headers` object used to be the trigger; the result path now tolerates that.) seedSource(); const claimed = await claim(); assert.ok( @@ -495,11 +501,20 @@ test('a throw while handling a result HOLDS the lease — it must not become a r 'precondition: claimed, so there is a lease to hold' ); - await assert.rejects( - () => postResult({ id: key(A), url: A, statusCode: 200, outcome: 'rendered' }, 'fresh html'), - /x-harper-rendered/, - 'the failure must surface (a 500), not be swallowed' - ); + const PrerenderedPage = globalThis.databases.page_cache.PrerenderedPage; + const realPut = PrerenderedPage.put; + PrerenderedPage.put = async () => { + throw new Error('blob store unavailable'); + }; + try { + await assert.rejects( + () => postResult({ id: key(A), url: A, statusCode: 200, outcome: 'rendered' }, 'fresh html'), + /blob store unavailable/, + 'the failure must surface (a 500), not be swallowed' + ); + } finally { + PrerenderedPage.put = realPut; + } assert.equal(leased(key(A)), true, 'the lease is HELD, so the retry is paced by queue.jobLeaseTime'); assert.deepEqual(await claim(), [], 'and an immediate re-claim grants nothing'); diff --git a/packages/plugin/test/targetRevalidate.test.js b/packages/plugin/test/targetRevalidate.test.js index 8652ec6..12d26f4 100644 --- a/packages/plugin/test/targetRevalidate.test.js +++ b/packages/plugin/test/targetRevalidate.test.js @@ -134,20 +134,17 @@ test('every URL is filed at the minute IT was written, not at the minute the swe PrerenderedPage.get = realGet; } - const filed = urls.map((url) => Number(stores.renderSchedule.get(`${url}|desktop`).nextRenderTime)); + // One row per URL, keyed by the URL: the revalidated render covers every device in one job. + const filed = urls.map((url) => Number(stores.renderSchedule.get(url).nextRenderTime)); assert.equal(filed.length, 4); assert.ok( filed[filed.length - 1] > filed[0], `each URL is stamped with its own minute (got ${filed.join(', ')}) — one capture for the whole ` + `sweep would file the last rows below the owning node's floor` ); - for (const [i, url] of urls.entries()) { + for (const url of urls) { for (const device of DEVICES) { - assert.equal( - Number(stores.renderSchedule.get(`${url}|${device}`).nextRenderTime), - filed[i], - 'a URL’s device variants still share one minute' - ); + assert.equal(stores.renderSchedule.has(`${url}|${device}`), false, 'no per-device schedule rows'); } } }); @@ -160,15 +157,13 @@ test('the sitemap flag survives the sweep — put REPLACES the schedule record', await Target.revalidate({}); - for (const device of DEVICES) { - assert.equal( - stores.renderSchedule.get(`${listed}|${device}`).fromSitemap, - true, - 'a cleared flag makes claim report isFromSitemap:false, and the renderer then skips serializing a ' + - 'non-indexable sitemap-listed page — i.e. a revalidate quietly stops those pages being cached' - ); - assert.equal(stores.renderSchedule.get(`${unlisted}|${device}`).fromSitemap, false); - } + assert.equal( + stores.renderSchedule.get(listed).fromSitemap, + true, + 'a cleared flag makes claim report isFromSitemap:false, and the renderer then skips serializing a ' + + 'non-indexable sitemap-listed page — i.e. a revalidate quietly stops those pages being cached' + ); + assert.equal(stores.renderSchedule.get(unlisted).fromSitemap, false); }); test('a caller projection that cannot support the sweep is refused by name, not silently trusted', async () => { @@ -192,10 +187,8 @@ test('a caller projection that cannot support the sweep is refused by name, not // so the flag survives a projected sweep. Under a fake that ignored `select` the row carried // `sitemapUrl` whatever the caller asked for, and this assertion proved nothing about projections. await Target.revalidate({ select: ['url', 'sitemapUrl'] }); - assert.equal(stores.renderSchedule.size, DEVICES.length); - for (const device of DEVICES) { - assert.equal(stores.renderSchedule.get(`${url}|${device}`).fromSitemap, true, 'from the projected row'); - } + assert.equal(stores.renderSchedule.size, 1, 'one row per URL'); + assert.equal(stores.renderSchedule.get(url).fromSitemap, true, 'from the projected row'); }); test('a target row with no url is skipped instead of scheduling the string "undefined"', async () => { @@ -209,7 +202,7 @@ test('a target row with no url is skipped instead of scheduling the string "unde assert.equal(result.examined, 2, 'while still reporting everything the walk saw'); assert.deepEqual( [...stores.renderSchedule.keys()].sort(), - ['https://www.example.com/real|desktop', 'https://www.example.com/real|mobile'], + ['https://www.example.com/real'], 'no schedule rows (and no floor lowering) for a URL that does not exist' ); }); From f9b0fa7c1764e1a57deb979ec4a5d21ba045e686 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Sep 2026 20:08:31 -0400 Subject: [PATCH 2/3] fix(plugin): review fixes for the URL-keyed schedule (adversarial pass + auto-review) - "Failed" is the complement of rendered, not a list of failure shapes: an outcome this code does not know, or a `redirected` whose destination re-keyed to the SAME cache key, now takes the retry lanes instead of falling through to the all-rendered branch and rescheduling a URL as a success that stored nothing. A result naming no device at all is refused with a 400. - `retireSource`: after `Target.delete` the row is only known gone when the job FOLDS (URL row or default-device row); a non-default one-device row (`|tablet`) whose URL turned out to redirect is now retired too, instead of sitting at its current-minute due time and being re-granted on every claim pass forever. - A refiled result with a failed sibling no longer takes the retry lanes: the source row is gone, so holding its lease pinned the claim floor for nothing and the failure was charged to the destination's target. - Suppression that deletes at maxStrikes marks the folding row gone, so it is not deleted twice (Harper writes a tombstone for a delete of an absent key). - `deviceTypes.default` is `nonEmpty` in the schema: an emptied list is refused at apply time and the default kept, instead of a downstream fallback that `claim` and the fold rule could disagree on (auto-review). - `CacheKey.isCacheKey/urlOf/deviceOf` key on a SUPPORTED device tail rather than "contains the delimiter", so a configurable delimiter that can occur inside a URL cannot misread a URL row. - README: bulk revalidate before the corpus has converted costs one extra render per leftover device row; render-now on a URL row renders every default device before it posts. configSchema: maxPerMinute arithmetic at one row per URL. Co-Authored-By: Claude Fable 5.1 --- packages/plugin/README.md | 10 +- packages/plugin/src/configSchema.js | 22 ++-- packages/plugin/src/resources/RenderQueue.js | 79 ++++++++++--- packages/plugin/src/util/cacheKey.js | 25 ++-- packages/plugin/test/cacheKey.test.js | 32 +++++ packages/plugin/test/config.test.js | 11 ++ .../plugin/test/renderQueueVariants.test.js | 110 +++++++++++++++++- 7 files changed, 250 insertions(+), 39 deletions(-) diff --git a/packages/plugin/README.md b/packages/plugin/README.md index 8685fb1..dff4cd8 100644 --- a/packages/plugin/README.md +++ b/packages/plugin/README.md @@ -1289,11 +1289,17 @@ the URL in the retry lanes — is spelled out on `processDecodedJobResult` in are **not migrated by a sweep**: each converts the first time it renders — its job renders exactly the device its key names, its result writes the URL row and deletes the device row — so two siblings fold into one URL row within a render cycle at no extra renders, and the table holds both shapes meanwhile -(every reader tolerates both). The `cacheKey` column name stays: Harper refuses to rename the primary +(every reader tolerates both). The one carve-out: a writer that files the URL row **before** a URL's +device rows have converted — a registry-wide `revalidate`, a sitemap ingest with `revalidate: true`, +`POST /prerender_admin/revalidate`, or a render-now — renders that URL once more per leftover device +row (each row still renders its device and rewrites the URL row), so a bulk revalidate in the first +cycle after the upgrade costs up to one extra render per URL. Wait a cycle, or accept it. The `cacheKey` column name stays: Harper refuses to rename the primary key of a populated table, so read it as "the schedule key". **Deploy the render fleet (browser >= 1.23.0) before this plugin version**: an older renderer handed a URL job renders only its first device. `renderNow` for a device outside `deviceTypes.default` still writes a per-device row — a one-off -render of that device beside the rotation, stored and retired without touching the URL row. +render of that device beside the rotation, stored and retired without touching the URL row. For a +default device it pulls the URL row forward, so the render it waits on renders **every** default +device before the result posts: size `renderNow.timeoutMs` for that, not for one render. ## Development diff --git a/packages/plugin/src/configSchema.js b/packages/plugin/src/configSchema.js index ddf4c19..9e9c36f 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -190,9 +190,13 @@ export const configSchema = group('Prerender plugin configuration.', { 'Device types the service understands; unrecognized values fall back to the first entry.', { itemType: 'string', nonEmpty: true } ), - default: option(['desktop', 'mobile'], 'Device types scheduled for rendering when a page is auto-discovered.', { - itemType: 'string', - }), + default: option( + ['desktop', 'mobile'], + 'Device types every render job renders (one job per URL renders all of them). Empty is refused at ' + + 'apply time and the default kept: a job naming no device is one the browser cannot act on, and the ' + + 'schedule code would otherwise have to guess.', + { itemType: 'string', nonEmpty: true } + ), }), cacheKey: group( @@ -828,11 +832,13 @@ export const configSchema = group('Prerender plugin configuration.', { 'per-device row that has not yet converted, and a per-device row for the served device when ' + 'that one is merely `supported` — so the write ceiling is this number times those rows.\n\n' + 'Sized so its CEILING is defensible, not just its typical. 10/min/node is 14,400 ' + - 'requests/node/day ≈ 28,800 schedule writes ≈ 2.3MB of audit/node/day, about 7% of measured ' + - 'spare fleet render capacity (~792,700 renders/day spare against a 1,710,936/day ceiling and ' + - '~918,000/day of baseline cadence demand) — against a measured demand of roughly 1,000 ' + - 'owner-node candidate requests/day CLUSTER-WIDE, i.e. ~14x headroom. Raising it toward 120 ' + - 'would authorise ~87% of all spare fleet capacity, which is why it is not the default.', + 'requests/node/day ≈ 14,400 schedule writes ≈ 1.2MB of audit/node/day once the corpus holds one ' + + 'row per URL (it was ~28,800 writes / 2.3MB at one row per device), and each write is a job that ' + + 'renders every device — about 7% of measured spare fleet render capacity (~792,700 renders/day ' + + 'spare against a 1,710,936/day ceiling and ~918,000/day of baseline cadence demand) — against a ' + + 'measured demand of roughly 1,000 owner-node candidate requests/day CLUSTER-WIDE, i.e. ~14x ' + + 'headroom. Raising it toward 120 would authorise ~87% of all spare fleet capacity, which is why ' + + 'it is not the default.', { min: 1 } ), } diff --git a/packages/plugin/src/resources/RenderQueue.js b/packages/plugin/src/resources/RenderQueue.js index cfe4372..223d9ed 100644 --- a/packages/plugin/src/resources/RenderQueue.js +++ b/packages/plugin/src/resources/RenderQueue.js @@ -67,11 +67,12 @@ const transientShaped = (statusCode) => statusCode === 408 || statusCode === 429 const hasContent = (variant) => variant.statusCode === 200 && !!variant.content; /** - * The devices a URL job renders: `config.deviceTypes.default`, floored at one device so an - * operator who empties the list still gets a render rather than a job the browser cannot act on. + * The devices a URL job renders. A copy, so a job never aliases live config. Never empty: the + * schema marks `deviceTypes.default` `nonEmpty`, so an emptied list is refused at apply time and + * the default kept — validated where config is applied, not guessed at here, which is also what + * keeps this and `describeJob`'s fold rule reading the same list. */ -const defaultDeviceTypes = () => - config.deviceTypes.default.length ? [...config.deviceTypes.default] : [config.deviceTypes.supported[0]]; +const defaultDeviceTypes = () => [...config.deviceTypes.default]; /** * One posted result, whatever shape the browser used, as `{ rowKey, url, asked, variants }`. @@ -129,9 +130,9 @@ const describeJob = (rowKey, url) => { }; /** - * A per-device row that has been folded into the URL row is deleted once its result is processed — - * unless the lease is being HELD (fast retry lane): the row is what the lease expiry re-grants, so it - * must stay for the retry, and converts on the result that finally reschedules it. + * A per-device row is deleted once its result is processed — folded into the URL row, or spent as a + * one-off — unless the lease is being HELD (fast retry lane): the row is what the lease expiry + * re-grants, so it must stay for the retry, and goes on the result that finally settles it. */ const retireRowIfConverted = async (job, held) => { if (held || job.rowGone || !job.perDevice) return; @@ -139,6 +140,18 @@ const retireRowIfConverted = async (job, held) => { job.rowGone = true; }; +/** + * Retire the job's URL: `Target.delete` drops the target, its pages, its URL row and its DEFAULT + * device rows (`scheduleKeysOf`) — which is every row a folding job can have come from, but NOT a + * one-device row for a non-default device. So the row is only known gone when the job folds; a + * `|tablet` one-off whose URL turned out to redirect is left to `retireRowIfConverted`, and would + * otherwise sit at its current-minute due time and be re-granted on every claim pass, forever. + */ +const retireSource = async (job) => { + await Target.delete(job.url); + job.rowGone = job.fold; +}; + /** The variant a device the browser was asked for and never posted back reduces to. */ const notAttemptedVariant = (deviceType) => ({ deviceType, @@ -334,6 +347,12 @@ export class RenderQueue extends Resource { `variants account for ${offset - metadataSize} of the ${buffer.byteLength - metadataSize} body byte(s)` ); } + // Nothing attempted and nothing asked: there is no device to attribute anything to, and + // letting it through would reach the all-rendered branch with an empty list and reschedule + // the URL as a success that stored nothing. + if (result.variants.length === 0 && !(Array.isArray(result.deviceTypes) && result.deviceTypes.length)) { + throw new Error('a variants result must name at least one device (variants or deviceTypes)'); + } } else if (metadataBuffer.byteLength < buffer.byteLength) { result.content = buffer.subarray(metadataSize); } @@ -511,8 +530,7 @@ export class RenderQueue extends Resource { if (variant.redirect.landedOn === PRERENDER) { if (!refiledTo) { logger.info(`Skipped prerendered url due to redirect: ${rowKey} redirected to ${variant.redirectedTo}`); - await Target.delete(url); - job.rowGone = true; + await retireSource(job); refiledTo = variant.redirect.destinationUrl; } variant.storeKey = variant.redirect.redirectKey; @@ -564,7 +582,11 @@ export class RenderQueue extends Resource { // Suppress writes the URL row (its recheck) and drops every device's page; the verdict // SUPPRESSES the target rather than deleting it — see Target.suppress, which also grades // http-error verdicts by status (404/410 recheck less, die sooner). - await Target.suppress(url, { reason: verdict.reason, statusCode: verdict.statusCode }); + const { deleted } = await Target.suppress(url, { reason: verdict.reason, statusCode: verdict.statusCode }); + // At maxStrikes the suppression DELETED the target, and `Target.delete` took the URL's default + // device rows with it — so a folding row is already gone (a second delete would only write a + // tombstone), while a non-default one-device row still needs retiring below. + if (deleted) job.rowGone = job.fold; await retireRowIfConverted(job, held); return; } @@ -633,12 +655,34 @@ export class RenderQueue extends Resource { // servable meanwhile. One outcome emit per result, for the class the worst variant fell in — // auth-shaped first (it is the one that signals a broken credential), then transient, then a // plain failure — and one log line per failed variant, at the level that class warrants. + // + // "Failed" is THE COMPLEMENT of rendered, not a list of failure shapes: an `outcome` this code + // does not know, or a `redirected` whose destination re-keyed to the SAME cache key (the browser's + // own redirect check uses a default allowlist, the plugin's is per route, so they can disagree — + // step 1 deliberately did not claim it), must land here and retry, exactly where the per-key + // code's final `else` sent them. Listing failure shapes instead let anything unlisted fall + // through to step 5 and reschedule a URL as a success that stored nothing. const failed = variants.filter( (variant) => - variant.outcome === 'error' || - (variant.outcome === 'non-indexable' && (authShaped(variant.statusCode) || transientShaped(variant.statusCode))) + variant.outcome !== 'rendered' && + !( + variant.outcome === 'non-indexable' && + !authShaped(variant.statusCode) && + !transientShaped(variant.statusCode) + ) ); - if (failed.length) { + if (failed.length && refiledTo) { + // The source was just retired by the refile above: there is no row of its own to retry under, + // and holding ITS lease would pin the claim floor at a row that no longer exists. The + // destination renders on its own row and cadence, so a failed sibling here is logged and let + // go — its device simply has no page until the destination renders. + for (const variant of failed) { + logger.warn( + `Prerender ${url} (${variant.deviceType}) did not render (${variant.reason || variant.outcome}) — not ` + + `retried: the URL was retired in favour of ${refiledTo} by a sibling's client-side redirect` + ); + } + } else if (failed.length) { const auth = failed.find((variant) => authShaped(variant.statusCode)); const transient = failed.find((variant) => transientShaped(variant.statusCode)); if (auth) metrics.renderOutcome('auth-failure', auth.statusCode); @@ -824,8 +868,7 @@ export class RenderQueue extends Resource { `Prerendered url ${rowKey} redirected to non-indexable ${variant.redirectedTo}` + `${variant.reason ? ` (${variant.reason})` : ''} — retiring the target` ); - await Target.delete(sourceUrl); - job.rowGone = true; + await retireSource(job); const domain = URL.parse(destinationUrl)?.hostname; // Auth-shaped and transient statuses never reach here (guarded above), so this // suppression is a genuine content/gone verdict about the destination. @@ -862,8 +905,7 @@ export class RenderQueue extends Resource { `retiring the target in favor of ${redirectKey}` ); const source = await Target.get({ id: sourceUrl, select: ['renderInterval'] }); - await Target.delete(sourceUrl); - job.rowGone = true; + await retireSource(job); // An existing destination row — active OR suppressed — wins: active means it's already // in rotation under its own cadence; suppressed means a render already proved it @@ -916,8 +958,7 @@ export class RenderQueue extends Resource { `Prerendered url ${sourceUrl} kept redirecting ${strikes} consecutive times (${why}) — retiring it; ` + `bots get the origin's own redirect and discovery re-creates what it actually serves` ); - await Target.delete(sourceUrl); // drops schedule rows + pages too - job.rowGone = true; + await retireSource(job); // drops the target, its pages, and every folding row return; } await Target.patch(sourceUrl, { strikes }); diff --git a/packages/plugin/src/util/cacheKey.js b/packages/plugin/src/util/cacheKey.js index 273ac7b..5abe5d5 100644 --- a/packages/plugin/src/util/cacheKey.js +++ b/packages/plugin/src/util/cacheKey.js @@ -36,25 +36,36 @@ export class CacheKey { // delimiter is absent, `indexOf` is -1), which resolves a route for the empty string and files // a row nobody asked for. // - // The shape test is "contains the delimiter", which is sound because `canonicalizeUrl` - // percent-encodes a literal delimiter out of the URL half (util/url.js step 7) precisely so it can - // never appear there. + // The shape test is "ends in ", NOT "contains the delimiter". + // `canonicalizeUrl` percent-encodes a literal `|` out of the URL half (util/url.js step 7), but + // `cacheKey.delimiter` is configurable and any other choice can legitimately occur inside a URL; + // keying on the device tail is sound for every delimiter (a URL that itself ends in + // `` is the one false positive, and no canonical URL does). + + /** Where the device tail of a cacheKey starts, or -1 when `str` is not shaped like one. */ + static #deviceTailAt(str) { + const delimiter = config.cacheKey.delimiter; + const at = str.lastIndexOf(delimiter); + if (at === -1) return -1; + return config.deviceTypes.supported.includes(str.slice(at + delimiter.length)) ? at : -1; + } /** True when `key` is a cacheKey (``), i.e. a per-device schedule row. */ static isCacheKey(key) { - return String(key ?? '').includes(config.cacheKey.delimiter); + return CacheKey.#deviceTailAt(String(key ?? '')) !== -1; } /** The URL a schedule key stands for: the URL half of a cacheKey, or the key itself. */ static urlOf(key) { const str = String(key ?? ''); - return CacheKey.isCacheKey(str) ? CacheKey.extractUrl(str) : str; + const at = CacheKey.#deviceTailAt(str); + return at === -1 ? str : str.slice(0, at); } /** The device a per-device schedule key names, or `null` for a URL-keyed row. */ static deviceOf(key) { const str = String(key ?? ''); - if (!CacheKey.isCacheKey(str)) return null; - return CacheKey.parse(str).deviceType || null; + const at = CacheKey.#deviceTailAt(str); + return at === -1 ? null : str.slice(at + config.cacheKey.delimiter.length); } } diff --git a/packages/plugin/test/cacheKey.test.js b/packages/plugin/test/cacheKey.test.js index e63b7b3..f332982 100644 --- a/packages/plugin/test/cacheKey.test.js +++ b/packages/plugin/test/cacheKey.test.js @@ -29,3 +29,35 @@ test('honors a configured delimiter and attribute list', () => { assert.deepEqual(CacheKey.parse(key), { url: 'https://x.com/', deviceType: 'mobile', region: 'west' }); assert.equal(CacheKey.extractUrl(key), 'https://x.com/'); }); + +// ---- schedule keys: a URL, or a pre-0.66.0 / one-device `|` row ---- + +test('isCacheKey / urlOf / deviceOf tell a URL-keyed schedule row from a per-device one', () => { + const url = 'https://x.com/p?page=2'; + assert.equal(CacheKey.isCacheKey(url), false); + assert.equal(CacheKey.urlOf(url), url, 'a URL is its own schedule key'); + assert.equal(CacheKey.deviceOf(url), null); + + assert.equal(CacheKey.isCacheKey(`${url}|mobile`), true); + assert.equal(CacheKey.urlOf(`${url}|mobile`), url); + assert.equal(CacheKey.deviceOf(`${url}|mobile`), 'mobile'); +}); + +test('the shape test keys on a SUPPORTED device tail, so a delimiter inside the URL cannot misfire', () => { + // `cacheKey.delimiter` is configurable; with one that can legitimately occur inside a URL, "contains + // the delimiter" would read a plain URL as a per-device row and strip its tail. + applyOptions({ cacheKey: { delimiter: '/' } }); + const url = 'https://x.com/catalog/shoes'; + assert.equal(CacheKey.isCacheKey(url), false, 'ends in "shoes", not a device'); + assert.equal(CacheKey.urlOf(url), url); + assert.equal(CacheKey.deviceOf(url), null); + assert.equal(CacheKey.isCacheKey(`${url}/mobile`), true); + assert.equal(CacheKey.urlOf(`${url}/mobile`), url); + assert.equal(CacheKey.deviceOf(`${url}/mobile`), 'mobile'); + applyOptions({}); +}); + +test('an unsupported device tail is not a per-device key', () => { + assert.equal(CacheKey.isCacheKey('https://x.com/p|watch'), false); + assert.equal(CacheKey.urlOf('https://x.com/p|watch'), 'https://x.com/p|watch'); +}); diff --git a/packages/plugin/test/config.test.js b/packages/plugin/test/config.test.js index b4710ce..70b53d3 100644 --- a/packages/plugin/test/config.test.js +++ b/packages/plugin/test/config.test.js @@ -436,3 +436,14 @@ test('a throwing listener does not break the apply or other listeners', () => { assert.equal(ran, true); applyOptions({}); }); + +test('an empty deviceTypes.default is refused at apply time and the default kept', () => { + // Every render job renders this list, and `claim` and the fold rule both read it; a job naming no + // device is one the browser cannot act on. Validated here, where config is applied — no downstream + // guard has to guess a device. + applyOptions({ deviceTypes: { default: [] } }); + assert.deepEqual(config.deviceTypes.default, ['desktop', 'mobile']); + applyOptions({ deviceTypes: { default: ['mobile'] } }); + assert.deepEqual(config.deviceTypes.default, ['mobile'], 'a non-empty list is honoured'); + applyOptions({}); +}); diff --git a/packages/plugin/test/renderQueueVariants.test.js b/packages/plugin/test/renderQueueVariants.test.js index 4e0cd7d..6a1b03b 100644 --- a/packages/plugin/test/renderQueueVariants.test.js +++ b/packages/plugin/test/renderQueueVariants.test.js @@ -210,11 +210,17 @@ test('a pre-0.66.0 per-device row claims as a job for exactly the device its key assert.equal(job.isFromSitemap, true); }); -test('an emptied deviceTypes.default still claims one device rather than a job the browser cannot act on', async () => { - config.deviceTypes.default = []; +test('an emptied deviceTypes.default is refused where config is applied — the job list is never empty', async () => { + // Validated at the entry point (the schema marks the option `nonEmpty`), not guarded here: `claim` + // and the fold rule in `describeJob` both read the same list, and a downstream fallback in one of + // them would let the two disagree. + const { applyOptions } = await import('../src/config.js'); + applyOptions({ deviceTypes: { default: [] } }); + assert.deepEqual(config.deviceTypes.default, ['desktop', 'mobile'], 'the default is kept'); seedUrlRow(); const [job] = await claim(); - assert.deepEqual(job.deviceTypes, ['desktop'], 'the first supported device'); + assert.deepEqual(job.deviceTypes, ['desktop', 'mobile']); + applyOptions({}); }); // ───────────────────────────── framing ───────────────────────────── @@ -415,6 +421,104 @@ test('a rendered client-side redirect refiles under the destination keys and ret assert.equal(leased(A), false, 'the SOURCE lease — the one that was granted — is released'); }); +test('a redirected variant whose destination re-keys to the SAME key is a failure, not a success', async () => { + // The browser's own redirect check uses a default allowlist; the plugin's is per route, so a + // query-only hop the route folds away arrives as `redirected` with no key change. Nothing was + // rendered and nothing decided, so it must take the retry lane — never step 5's reschedule. + seedUrlRow(); + await claim(); + await postVariants(A, [ + rendered('desktop', 'fresh'), + { deviceType: 'mobile', outcome: 'redirected', statusCode: 200, redirectedTo: `${A}?utm=x` }, + ]); + assert.equal(stores.prerenderedPage.get(key(A, 'desktop')).content.toString(), 'fresh'); + assert.equal(stores.target.get(A).strikes, 1, 'counted as a failure'); + assert.equal(stores.renderSchedule.get(A).nextRenderTime, 1, 'NOT rescheduled as a success'); + assert.equal(leased(A), true, 'the fast lane holds the lease'); + assert.deepEqual(outcomes(), [['failed', 'unknown']]); +}); + +test('an outcome this plugin does not know is a failure, not a success', async () => { + seedUrlRow(); + await claim(); + await postVariants(A, [{ deviceType: 'desktop', outcome: 'teleported' }, rendered('mobile')]); + assert.equal(stores.target.get(A).strikes, 1); + assert.equal(stores.renderSchedule.get(A).nextRenderTime, 1); + assert.equal(leased(A), true); +}); + +test('a result that names no device at all is refused with a 400, and the lease simply expires', async () => { + seedUrlRow(); + await claim(); + const response = await post({ id: A, url: A, variants: [] }, []); + assert.equal(response?.status, 400); + assert.equal(stores.renderSchedule.get(A).nextRenderTime, 1, 'nothing was rescheduled'); + assert.deepEqual(outcomes(), [], 'and nothing was counted'); +}); + +test('a one-device row for a NON-default device whose URL turns out to 301 is retired too — no re-grant loop', async () => { + seedUrlRow({ nextRenderTime: 9_999_999_999_999 }); + stores.renderSchedule.set(key(A, 'tablet'), { nextRenderTime: 1, fromSitemap: false }); + await claim(); + await postVariants(A, [{ deviceType: 'tablet', outcome: 'redirected', statusCode: 301, redirectedTo: B }], { + id: key(A, 'tablet'), + deviceTypes: ['tablet'], + }); + assert.equal(stores.target.has(A), false, 'the URL is retired — a page does not redirect for one device only'); + assert.equal(stores.renderSchedule.has(A), false); + assert.equal(stores.renderSchedule.has(key(A, 'tablet')), false, 'the tablet row goes too — it is not a default key'); + assert.equal(leased(key(A, 'tablet')), false); + assert.deepEqual(await claim(), [], 'nothing left to re-grant'); + assert.ok(stores.target.get(B), 'destination adopted'); +}); + +test('a refiled result with a failed sibling releases the SOURCE lease and touches no other target', async () => { + seedUrlRow(); + stores.target.set(B, { url: B, renderInterval: 3_600_000, strikes: 0 }); + stores.renderSchedule.set(B, { nextRenderTime: 5_555_555_555_555, fromSitemap: false }); + await claim(); + await postVariants(A, [ + rendered('desktop', 'landed', { redirectedTo: B }), + { + deviceType: 'mobile', + outcome: 'error', + reason: 'error', + error: { name: 'Error', message: 'x', phase: 'settle' }, + }, + ]); + assert.equal(stores.target.has(A), false, 'source retired by the refile'); + assert.equal(stores.prerenderedPage.get(key(B, 'desktop')).content.toString(), 'landed'); + assert.equal(leased(A), false, 'no lease is held for a row that no longer exists'); + assert.equal(stores.target.get(B).strikes, 0, "the sibling's failure is not charged to the destination"); + assert.ok(warns.some((w) => w.includes('(mobile)') && w.includes('not retried'))); + assert.deepEqual(outcomes(), [['rendered', 'refiled']]); +}); + +test('a suppression that deletes at maxStrikes does not delete the folding device row twice', async () => { + const max = config.render.suppression.maxStrikes; + stores.target.set(A, { url: A, renderInterval: 3_600_000, state: 'suppressed', strikes: max - 1 }); + stores.renderSchedule.set(key(A, 'desktop'), { nextRenderTime: 1, fromSitemap: false }); + const RenderSchedule = globalThis.databases.render_schedule.RenderSchedule; + const deletes = []; + const realDelete = RenderSchedule.delete; + RenderSchedule.delete = async (id) => { + deletes.push(id); + return realDelete.call(RenderSchedule, id); + }; + try { + await claim(); + await postVariants( + A, + [{ deviceType: 'desktop', outcome: 'non-indexable', isIndexable: false, statusCode: 200, reason: 'noindex' }], + { id: key(A, 'desktop'), deviceTypes: ['desktop'] } + ); + } finally { + RenderSchedule.delete = realDelete; + } + assert.equal(stores.target.has(A), false, 'deleted at maxStrikes'); + assert.equal(deletes.filter((id) => id === key(A, 'desktop')).length, 1, 'the device row is deleted exactly once'); +}); + // ───────────────────────────── partial results ───────────────────────────── test('a device asked for and not posted back is a failure: good page stored, URL retries on the lease', async () => { From 99b2682ff999f91aee1609ce93803d008d8df902 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Sep 2026 20:09:22 -0400 Subject: [PATCH 3/3] =?UTF-8?q?test(plugin):=20after=20a=20one-device=20ro?= =?UTF-8?q?w's=20301=20the=20adopted=20destination=20is=20due=20=E2=80=94?= =?UTF-8?q?=20assert=20nothing=20of=20the=20SOURCE=20is=20re-granted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 --- packages/plugin/test/renderQueueVariants.test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/plugin/test/renderQueueVariants.test.js b/packages/plugin/test/renderQueueVariants.test.js index 6a1b03b..cbd07a4 100644 --- a/packages/plugin/test/renderQueueVariants.test.js +++ b/packages/plugin/test/renderQueueVariants.test.js @@ -468,8 +468,17 @@ test('a one-device row for a NON-default device whose URL turns out to 301 is re assert.equal(stores.renderSchedule.has(A), false); assert.equal(stores.renderSchedule.has(key(A, 'tablet')), false, 'the tablet row goes too — it is not a default key'); assert.equal(leased(key(A, 'tablet')), false); - assert.deepEqual(await claim(), [], 'nothing left to re-grant'); assert.ok(stores.target.get(B), 'destination adopted'); + const next = await claim(); + assert.ok( + next.every((job) => job.url !== A), + `nothing of ${A} is left to re-grant (got ${next.map((job) => job.id).join(', ')})` + ); + assert.deepEqual( + next.map((job) => job.id), + [B], + 'only the adopted destination is due' + ); }); test('a refiled result with a failed sibling releases the SOURCE lease and touches no other target', async () => {