From 5a5396fa8dda72d76b63f3a5a83a0ae79a96bf44 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 21 Aug 2026 13:12:26 -0400 Subject: [PATCH] feat(plugin): rank the due set by lateness relative to each page's own cadence; v0.50.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `claim` ordered by `nextRenderTime` and nothing else. That expresses priority perfectly while the queue is caught up, and not at all once two rows are both past due — a due time encodes when a page last rendered plus its cadence, not how much it matters: home (1h cadence) due 2h ago -> 2.0 intervals late PDP (48h cadence) due 3h ago -> 0.06 intervals late Index order hands the lease to the PDP, and nothing looks wrong while it does: the floor advances, the scan stays fast, no row is wedged. The only symptom is the served age `config.yaml` already describes — `interval + swrTtl`, which is 7x the homepage's cadence and 0.125x a PDP's — and it reads like a cadence mis-set rather than an ordering problem. So a due row is now ranked by `(now - dueAt) / renderInterval`, with sitemap-sourced rows multiplied by `queue.priority.sitemapBoost`. Ordering only: every row it reorders is already due, so this creates no work and cannot move total render volume, which is why it ships enabled. Three things are load-bearing: - LATENESS, NOT AGE. `dueAt - interval` is not when the page last rendered: suppression rechecks schedule 7 days, `backoffWait` schedules up to `maxBackoff`, the unpin hatch pushes by `defaultInterval`. An age ratio would put a 7-day recheck on a 48h route at the head of the queue reading as 3.5 cadences stale — promoting exactly the rows worth deprioritizing. - THE FLOOR IS STILL DERIVED IN INDEX ORDER. The claim pass is now two phases: phase 1 walks the drain in index order and derives the floor from the first due row it sees; phase 2 grants from those rows in priority order. Deriving the floor from the priority walk would pick the most-overdue-by-ratio row instead of the minimum and strand everything below it, silently and forever. - THE CADENCE IS READ OFF THE ROW. `renderInterval` is denormalized onto `RenderSchedule` for the same reason `fromSitemap` is — `claim` takes no Target read — because the effective cadence includes the demand ladder's rung, and resolving the route at claim time would rank a promoted catalog page at its 24h ceiling. Optional, unlike `fromSitemap`: omitting it falls back to the route interval and degrades ordering only, where omitting `fromSitemap` silently stops a page being cached. `queue.priority.candidatePool` is the part with a real trade in it. The scan window exists to read past the in-flight lease pile (`limit` + pile + `limit`), so beyond the pile it holds about as many grantable rows as the pass is about to grant — "pick the best 25" out of 25. It widens that last term to `limit x candidatePool`, still capped by `claimScanCap`. Without it the ordering is close to a no-op, which is what the test at grantLimit 2 found. `sitemapBoost` is a multiplier and never a lane: an unserved row's ratio grows without bound while the boost stays constant, so a discovered URL wins once its ratio passes `boost x` the highest sitemap ratio in the window — ~2.4 cadences at the default. Starvation is bounded and the bound is statable. New metric `queue_health` `claim_lateness_pct`: how overdue each granted job was as a percentage of its own interval, split sitemap/discovered. Normalized on purpose — `route_page_age` is absolute, so a regression on a 1h route hides inside a 48h route's numbers. `queue.priority.enabled: false` grants in index order and walks the old, narrower window: a revert of the behaviour, not a re-weighting of it. The existing 715 tests pass untouched, because `runClaimPass` takes the orderer as an argument and defaults to none. Co-Authored-By: Claude Opus 5 --- packages/plugin/METRICS.md | 53 +-- packages/plugin/README.md | 61 +++ packages/plugin/package.json | 2 +- packages/plugin/src/configSchema.js | 57 +++ packages/plugin/src/metrics.js | 28 +- packages/plugin/src/resources/RenderQueue.js | 26 +- packages/plugin/src/resources/Target.js | 6 + packages/plugin/src/schemas/schema.graphql | 13 + .../plugin/src/util/invalidationReenqueue.js | 10 +- packages/plugin/src/util/renderPriority.js | 165 ++++++++ packages/plugin/src/util/renderSchedule.js | 139 ++++++- packages/plugin/test/renderPriority.test.js | 382 ++++++++++++++++++ 12 files changed, 899 insertions(+), 43 deletions(-) create mode 100644 packages/plugin/src/util/renderPriority.js create mode 100644 packages/plugin/test/renderPriority.test.js diff --git a/packages/plugin/METRICS.md b/packages/plugin/METRICS.md index fb84624..70be9f1 100644 --- a/packages/plugin/METRICS.md +++ b/packages/plugin/METRICS.md @@ -121,17 +121,17 @@ PK drives the scan (an open range can make the planner walk a metric's entire hi One-line summaries; `src/metrics.js` carries the full description of every dimension value and the reasoning behind it. -| Metric | Kind | `path` | `method` | `type` | What it's for | -| ---------------- | ------- | ---------- | ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `bot_request` | counter | host | botName | deviceType | Raw crawl volume and mix at ingress. The denominator for every serve-side ratio. | -| `bot_serve` | counter | source | cacheStatus | botName | **Origin offload** and **cache hit rate** — the two rollout numbers. | -| `route_serve` | counter | route | cacheStatus | deviceType | The same outcome per route: which route's `renderInterval` needs to move. | -| `page_age` | ms | botName | deviceType | — | Freshness as delivered: ms since the served snapshot rendered (cache serves only). | -| `route_page_age` | ms | route | cacheStatus | deviceType | Served age per route, split by freshness state — the "should this TTL move" number. | -| `render` | value | series | per-series | per-series | The render fleet in one scan: `time_ms` (duration by statusCode × candidacy — renders/hour = concurrency ÷ time_ms) and `outcome` (counter by outcome × detail, exactly one per posted result — 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). | -| `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), `reconcile_restored`/`reconcile_missing` (per sweep). | +| Metric | Kind | `path` | `method` | `type` | What it's for | +| ---------------- | ------- | ---------- | ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `bot_request` | counter | host | botName | deviceType | Raw crawl volume and mix at ingress. The denominator for every serve-side ratio. | +| `bot_serve` | counter | source | cacheStatus | botName | **Origin offload** and **cache hit rate** — the two rollout numbers. | +| `route_serve` | counter | route | cacheStatus | deviceType | The same outcome per route: which route's `renderInterval` needs to move. | +| `page_age` | ms | botName | deviceType | — | Freshness as delivered: ms since the served snapshot rendered (cache serves only). | +| `route_page_age` | ms | route | cacheStatus | deviceType | Served age per route, split by freshness state — the "should this TTL move" number. | +| `render` | value | series | per-series | per-series | The render fleet in one scan: `time_ms` (duration by statusCode × candidacy — renders/hour = concurrency ÷ time_ms) and `outcome` (counter by outcome × detail, exactly one per posted result — 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). | +| `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_lateness_pct` (per granted job, method = sitemap/discovered), `reconcile_restored`/`reconcile_missing` (per sweep). | Notes that bite: @@ -185,7 +185,8 @@ demand_demoted + demand_held`. The other two decision counters are the paths whe - **Value semantics vary per series inside the umbrellas** — `prerender_ops`' `unrouted`/`sitemap_*` and `queue_health`'s `reconcile_*` are per-interval/per-run counts whose `total` is the meaningful sum (`count` is flushes/runs); `config_warnings` and the snapshot gauges are - latest-value gauges; `claim_scan_ms` and `origin_fetch` are ordinary duration distributions. + latest-value gauges; `claim_scan_ms` and `origin_fetch` are ordinary duration distributions, and + `claim_lateness_pct` is a distribution whose unit is percent-of-cadence rather than milliseconds. - **`queue_health` mixes cadences on purpose** (slow snapshot gauges beside per-pass `claim_scan_ms`): one name = one `get_analytics` scan for the whole queue panel. @@ -358,19 +359,21 @@ The catalog above is reference; this is the short list. "Sum across nodes" is im **Thresholds — warn, then investigate:** -| Condition | Meaning | -| ----------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | -| `queue_health` `overdue` − `lease_occupancy` growing snapshot-over-snapshot | The fleet is falling behind demand (remember: `overdue`'s healthy floor IS the in-flight count). | -| `queue_health` `floor_pin_age_ms` > ~1 h | One key is holding the claim scan's seek position — the whole node's queue ages behind it. | -| `bot_serve` swr share rising / `route_page_age` p95 > that route's `renderInterval` | The cadence is configured but not delivered — a capacity or scheduling problem, not a config one. | -| `bot_serve` miss share rising | Coverage: new URLs the corpus doesn't have, or the CDN forwarding paths it shouldn't (check `unrouted`). | -| `duration` p95 (`path: 'p'`) or `success` ratio degrading | The crawler-facing SLO, independent of any plugin-level explanation. | -| `queue_status` report timestamp stale, or intent ≠ observed > one sync interval | A node stopped reporting (and likely claiming), or pause propagation is stuck. | -| `render` outcome `suppressed` or `failed` share rising | Mass suppression (an origin change disavowing pages) or a failing fleet — shares are readable directly because outcomes sum to results. | -| `queue_health` `claim_scan_ms` p95 trending up | The scan is degrading (dead index entries at the seek point) before any backlog shows. Watch the trend, not the absolute number. | -| `origin_fetch` p95 or 5xx/`0` share rising | Origin trouble that bots feel directly on every miss; a rising `render-timeout` share is renderNow falling back. | -| `queue_health` `paused` = 1 beyond the expected window | A node's queue is paused longer than whoever paused it intended. | -| `prerender_ops` series `config_warnings` changed after a deploy | The deploy introduced a finding; `GET /prerender_admin/config` names it. | +| Condition | Meaning | +| ------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `queue_health` `overdue` − `lease_occupancy` growing snapshot-over-snapshot | The fleet is falling behind demand (remember: `overdue`'s healthy floor IS the in-flight count). | +| `queue_health` `floor_pin_age_ms` > ~1 h | One key is holding the claim scan's seek position — the whole node's queue ages behind it. | +| `bot_serve` swr share rising / `route_page_age` p95 > that route's `renderInterval` | The cadence is configured but not delivered — a capacity or scheduling problem, not a config one. | +| `bot_serve` miss share rising | Coverage: new URLs the corpus doesn't have, or the CDN forwarding paths it shouldn't (check `unrouted`). | +| `duration` p95 (`path: 'p'`) or `success` ratio degrading | The crawler-facing SLO, independent of any plugin-level explanation. | +| `queue_status` report timestamp stale, or intent ≠ observed > one sync interval | A node stopped reporting (and likely claiming), or pause propagation is stuck. | +| `render` outcome `suppressed` or `failed` share rising | Mass suppression (an origin change disavowing pages) or a failing fleet — shares are readable directly because outcomes sum to results. | +| `queue_health` `claim_scan_ms` p95 trending up | The scan is degrading (dead index entries at the seek point) before any backlog shows. Watch the trend, not the absolute number. | +| `queue_health` `claim_lateness_pct` p95 above ~200 for every route at once | Pages are being granted more than two of their own cadences late across the board — a capacity shortfall, not an ordering problem. `queue.priority` reorders a window; it cannot create render throughput. Lengthen `renderInterval` or add fleet. | +| `queue_health` `claim_lateness_pct` p95 high for `discovered` while `sitemap` is low | `queue.priority.sitemapBoost` is doing what it is set to do. Expected; it is a starvation problem only if the discovered number keeps climbing rather than settling at roughly `sitemapBoost x` the sitemap number. | +| `origin_fetch` p95 or 5xx/`0` share rising | Origin trouble that bots feel directly on every miss; a rising `render-timeout` share is renderNow falling back. | +| `queue_health` `paused` = 1 beyond the expected window | A node's queue is paused longer than whoever paused it intended. | +| `prerender_ops` series `config_warnings` changed after a deploy | The deploy introduced a finding; `GET /prerender_admin/config` names it. | **Absence is a signal — alert when a series stops:** diff --git a/packages/plugin/README.md b/packages/plugin/README.md index 4c1695a..3de04ec 100644 --- a/packages/plugin/README.md +++ b/packages/plugin/README.md @@ -129,6 +129,10 @@ rest: true # required for the @export-ed table REST endpoints statusSyncInterval: 60000 # 1m — pause convergence, status broadcast, claim-floor reset maxLeases: 4096 # lease slots in the node-local shared buffer (restart-scoped) claimScanCap: 1000 # ceiling on schedule rows read per claim pass + priority: # WHICH of the due rows the next leases go to (ordering only, no cadence change) + enabled: true # false = grant in index order (absolute due time), as before v0.50.0 + sitemapBoost: 2 # how much a sitemap row outranks a discovered one at equal overdue ratio + candidatePool: 8 # multiples of `limit` to choose from; 1 keeps the pre-0.50.0 window claimFloor: # the lower bound the claim scan seeks from (see "The claim floor") enabled: true # false = seek the absolute index minimum, as before v0.34.0 guard: 300000 # 5m — the floor is always held at least this far behind now @@ -518,6 +522,63 @@ The floor advances to **the first due row a pass observed**, which is the same t Set `queue.claimFloor.enabled: false` to roll the floor back to the old full seek. It changes nothing else; leases stay where they are either way. +#### Which of the due rows goes first + +The floor decides _where the scan starts_. `queue.priority` decides which of the rows it drained get +the leases — and only that. It changes no cadence, creates no work, and cannot move total render +volume, because every row it reorders is already due. + +Absolute due time cannot express this, which is the whole reason the option exists: + +| page | cadence | due | overdue, in its own cadence | +| ---- | ------- | ------ | --------------------------- | +| home | 1h | 2h ago | **2.0 intervals** | +| PDP | 48h | 3h ago | 0.06 intervals | + +Index order hands the lease to the PDP, because 3h > 2h. Nothing looks wrong while it does: the floor +advances, the scan stays fast, no row is wedged. The only symptom is the one the served-age numbers +already show — worst-case age is `interval + swrTtl`, which is several multiples of a fast route's +cadence and a fraction of a slow one's — and it is easy to spend that incident tuning +`renderInterval`. + +So a due row is ranked by `(now − dueAt) / renderInterval`, i.e. how late it is **relative to its own +cadence**, with sitemap-sourced rows multiplied by `queue.priority.sitemapBoost`. Three details are +load-bearing: + +- **Lateness, not age.** `dueAt − interval` is not when the page last rendered: `Target.suppress` + schedules `render.suppression.recheckInterval` (7 days), `backoffWait` schedules up to + `maxBackoff`, and the unpin hatch pushes by `render.defaultInterval`. An age-based ratio would put + a 7-day suppression recheck on a 48h route at the _head_ of the queue reading as 3.5 cadences + stale. Lateness is zero the moment any row comes due, whatever gap preceded it, so those rows enter + at the back and climb like anything else. +- **The cadence is read off the row.** `renderInterval` is denormalized onto `RenderSchedule` for the + same reason `fromSitemap` is: `claim` takes no `RenderTarget` read. It matters because the + effective cadence includes the demand ladder's rung, and resolving the route at claim time would + rank a promoted catalog page at its 24h _ceiling_ — the opposite of what promoting it was for. The + field is optional; a row written by a path that does not have it (a reconcile repair, an + invalidation re-enqueue, a render-now one-off) falls back to the route-resolved interval until that + URL's next completed render re-stamps it. +- **The boost is a multiplier, never a lane.** An unserved row's ratio grows without bound while the + boost stays constant, so a discovered URL wins as soon as its ratio passes `sitemapBoost ×` the + highest sitemap ratio in the window. With sitemap pages held ~1.2 cadences late that is ~2.4 + cadences at the default — bounded, and it scales with the boost. + +`queue.priority.candidatePool` is the part to actually think about. The scan window exists to read +_past_ the in-flight lease pile (`limit` + pile + `limit`), so beyond the pile it holds about as many +grantable rows as the pass is about to hand out — "pick the best 25" out of 25. `candidatePool` +widens only that last term, to `limit × candidatePool` rows past the pile, still hard-capped by +`queue.claimScanCap`. The pile is counted first, so a large pile can consume the cap and leave the +pool no room: if the truncation warning starts naming the cap, raise `claimScanCap` before raising +`candidatePool`. + +Watch `queue_health` `claim_lateness_pct` — how overdue each _granted_ job was as a percentage of its +own interval, split sitemap/discovered. It is the normalized companion to `route_page_age`: one p95 +covers every route, so a p95 well above 100 across the board reads as a capacity shortfall rather +than something to infer by dividing two dashboards. No ordering fixes that one. + +`queue.priority.enabled: false` grants in index order and walks the old, narrower window — a revert +of the behaviour, not a neutral weighting of it. + ## HTTP & resource API | Method & path | Purpose | diff --git a/packages/plugin/package.json b/packages/plugin/package.json index d84ab29..77ef186 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.49.0", + "version": "0.50.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 479a6bd..f89e289 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -1262,6 +1262,63 @@ export const configSchema = group('Prerender plugin configuration.', { 'about could not be detected.', { min: 1, scope: 'restart' } ), + priority: group( + 'WHICH of the due rows the next leases go to. Ordering only — this changes no cadence, ' + + 'creates no work and cannot change total render volume, because it reorders rows that are ' + + 'ALREADY due inside a single claim pass.\n\n' + + 'The queue seeks from the claim floor along one index (`nextRenderTime`) and that stays ' + + 'true. What changes is the comparison used to pick from the rows it drained: instead of ' + + 'absolute due time, a row is ranked by how overdue it is RELATIVE TO ITS OWN cadence — ' + + '`(now - dueAt) / renderInterval` — with sitemap-sourced rows multiplied by ' + + '`sitemapBoost`. Absolute due time cannot express this: a 48h PDP due 3h ago outranks a ' + + '1h homepage due 2h ago even though the homepage is 2 cadences late and the PDP is 6% ' + + 'late, so under any backlog the short-cadence routes sit behind a wall of long-cadence ' + + 'ones and the only symptom is a served age of `interval + swrTtl` that is many multiples ' + + 'of a fast route’s interval and a fraction of a slow one’s.\n\n' + + 'IT REORDERS A WINDOW, NOT THE BACKLOG. The pass drains ' + + '`min(grantLimit + in-flight + grantLimit, queue.claimScanCap)` rows, so it chooses ~25 ' + + 'out of a few hundred, not out of the whole due set. It is a latency fix, not a capacity ' + + 'fix: if a route is short of capacity outright, `renderInterval` and fleet size are the ' + + 'levers. Raise `queue.claimScanCap` to widen the pool it chooses from.', + { + enabled: option( + true, + 'Kill switch. `false` grants in index order (absolute due time), exactly as before ' + + 'v0.50.0 — the claim pass then walks and stops at `grantLimit` as it used to, so this ' + + 'is a true revert of the behaviour and not a neutral weighting of it.' + ), + sitemapBoost: option( + 2, + 'How much a sitemap-sourced row outranks a discovered one at the same overdue ratio. ' + + '`1` disables the preference and leaves ordering on overdue ratio alone.\n\n' + + 'A MULTIPLIER, not a tier, so it cannot starve discovered URLs: an unserved row’s ' + + 'overdue ratio grows without bound while the boost stays constant, so a discovered row ' + + 'wins as soon as its ratio passes `sitemapBoost x` the highest sitemap ratio in the ' + + 'window. With sitemap pages held ~1.2 cadences late, a discovered page is therefore ' + + 'served within ~`2 x 1.2` cadences of its own interval at the default. Raising this ' + + 'raises that bound proportionally.', + { min: 1 } + ), + candidatePool: option( + 8, + 'How many times `limit` grantable rows a claim pass tries to choose from, as a multiple ' + + 'of the batch it is granting.\n\n' + + 'WITHOUT THIS THE ORDERING HAS ALMOST NOTHING TO ORDER. The scan window exists to read ' + + 'PAST the in-flight lease pile — `limit` + pile + `limit` — so beyond the pile it holds ' + + 'about as many grantable rows as the pass is about to hand out, and "pick the best 25" ' + + 'degenerates to "take the 25 that were there". This widens only that last term, so the ' + + 'pass reads `limit x candidatePool` rows past the pile and grants the most overdue ' + + '`limit` of them.\n\n' + + 'COST: index-ordered reads inside a window the seek has already landed in, and still ' + + 'hard-capped by `queue.claimScanCap` — which is the ceiling to watch, because the pile ' + + 'is counted first, so a large pile can consume the cap and leave the pool no room. If ' + + 'the truncation warning starts naming the cap, raise `claimScanCap` before raising ' + + 'this. `1` keeps the historical window (and makes the ordering close to a no-op); `0` ' + + 'is treated as `1`.', + { min: 0 } + ), + } + ), claimScanCap: option( 1000, 'Ceiling on schedule rows read per claim pass. A leased row keeps its overdue position in the ' + diff --git a/packages/plugin/src/metrics.js b/packages/plugin/src/metrics.js index 9464009..96b0485 100644 --- a/packages/plugin/src/metrics.js +++ b/packages/plugin/src/metrics.js @@ -377,15 +377,23 @@ export const METRICS = Object.freeze({ 'paused = 1 when this node’s queue is paused at snapshot time, else 0 — makes "paused for hours" ' + 'alertable without polling the REST surface. ' + 'claim_scan_ms = claim-pass duration; watch the p95 trend, not the level. ' + + 'claim_lateness_pct = how overdue each GRANTED job was as a percentage of ITS OWN render ' + + 'interval (100 = one cadence late), split sitemap/discovered in the method slot — the ' + + 'normalized companion to route_page_age, and the measure of whether queue.priority is ' + + 'keeping short-cadence routes on time. A p95 that sits well above 100 for every route at ' + + 'once is a capacity shortfall, not an ordering problem, and no ordering fixes it. Emitted ' + + 'only while queue.priority.enabled is on, because the ratio is a by-product of the ' + + 'ordering pass. ' + 'reconcile_restored / reconcile_missing = schedule gaps repaired / found per sweep (they differ ' + 'when the per-sweep restore cap truncates the pass); expect zero — a steady rate means ' + 'something is CREATING gaps, and the reconcile log line names the URLs.', }, method: { - name: 'result (claim_scan_ms only)', - values: ['granted', 'empty', 'capped'], + name: 'result (claim_scan_ms) | source (claim_lateness_pct)', + values: ['granted', 'empty', 'capped', 'sitemap', 'discovered'], description: - 'Only claim_scan_ms uses this slot: granted = jobs handed out, empty = nothing due, capped = the ' + + 'claim_lateness_pct uses this slot for sitemap | discovered. On claim_scan_ms: ' + + 'granted = jobs handed out, empty = nothing due, capped = the ' + 'scan hit queue.claimScanCap without reaching a not-yet-due row (in-flight work is filling the ' + 'window). Every other series emits null here.', }, @@ -641,6 +649,20 @@ export const metrics = Object.freeze({ /** One claim pass's duration and how it ended — a queue_health series, so the queue reads in one scan. */ claimScan: (durationMs, result) => server.recordAnalytics(durationMs, 'queue_health', 'claim_scan_ms', result, null), + /** + * How overdue one GRANTED job was, in hundredths of its own render interval, split by whether the + * URL is sitemap-listed. 100 = exactly one cadence late. + * + * This is the outcome measure for `queue.priority`, and it is normalized on purpose: `page_age` + * and `route_page_age` are absolute, so a 48h route and a 1h route are not comparable in them and + * a regression on the fast route hides inside the slow route's numbers. Here every route shares + * one scale — its own cadence — so one p95 covers the whole corpus, and "the fleet cannot keep up + * with what has been asked of it" is a level (well above 100) rather than something to be + * inferred by dividing two dashboards. + */ + claimPriority: (ratioPct, source) => + server.recordAnalytics(ratioPct, 'queue_health', 'claim_lateness_pct', source, null), + /** One origin proxy on the serve path: time to response headers, status, and why. */ originFetch: (durationMs, statusCode, reason) => server.recordAnalytics(durationMs, 'origin_fetch', statusCode, reason, null), diff --git a/packages/plugin/src/resources/RenderQueue.js b/packages/plugin/src/resources/RenderQueue.js index 562d5bb..dcdd27d 100644 --- a/packages/plugin/src/resources/RenderQueue.js +++ b/packages/plugin/src/resources/RenderQueue.js @@ -384,7 +384,16 @@ export class RenderQueue extends Resource { // 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, { nextRenderTime, fromSitemap: !!renderTarget.sitemapUrl }); + // `interval` — the DEMAND-RESOLVED cadence, not `base` — is what the claim pass scores this + // row's lateness against (util/renderPriority.js). This is the only writer that knows the + // ladder's answer, so it is the one that has to record it: `resolveRenderInterval` at claim + // time sees the route's ceiling and would rank a promoted catalog page as if it were still + // on 24h, which is the opposite of what promoting it was for. + await writeSchedule(cacheKey, { + nextRenderTime, + fromSitemap: !!renderTarget.sitemapUrl, + renderInterval: 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 @@ -724,7 +733,12 @@ export class RenderQueue extends Resource { `Retrying ${cacheKey} in ${Math.round(wait / 60000)}m (failure strike ${strikes}` + `${fromSitemap ? '' : ', non-sitemap'})` ); - await writeSchedule(cacheKey, { nextRenderTime, fromSitemap }); + // THE CADENCE, NOT THE BACKOFF. `wait` is how long this failing key is being pushed out for; + // `interval` is still what it is supposed to render at, and it is the cadence the claim pass + // measures lateness against. Recording `wait` instead would give a backed-off row an interval + // several times its real one, hence a near-zero overdue ratio, hence a permanent seat at the + // back of the queue — a deprioritization the backoff already applied once, compounding. + await writeSchedule(cacheKey, { nextRenderTime, fromSitemap, renderInterval: interval }); return 'slow'; } @@ -754,6 +768,7 @@ export class RenderQueue extends Resource { await writeSchedule(cacheKey, { nextRenderTime: currentMinuteMs() + interval, fromSitemap: !!renderTarget.sitemapUrl, + renderInterval: interval, }); } @@ -814,6 +829,13 @@ export class RenderQueue extends Resource { // number proves it happens. if (getResidencyByUrl(url) !== server.hostname) notOwnedHere++; + // One emit per granted job — ~25 per pass on a path that runs a fraction of a time per + // second, so it stays off any hot loop. `undefined` when the pass granted in index order and + // therefore never scored anything; reporting a 0 there would read as "everything is on time". + if (granted.priority !== undefined) { + metrics.claimPriority(Math.round(granted.priority * 100), granted.fromSitemap ? 'sitemap' : 'discovered'); + } + jobs.push({ id: granted.cacheKey, url, diff --git a/packages/plugin/src/resources/Target.js b/packages/plugin/src/resources/Target.js index 3a95349..62b44f5 100644 --- a/packages/plugin/src/resources/Target.js +++ b/packages/plugin/src/resources/Target.js @@ -108,6 +108,12 @@ export class Target extends TargetTable { ? nextRenderTime : getInitialRenderTime(cacheKey, interval), fromSitemap, + // The cadence the claim pass ranks this row's lateness against. Route > stored > default + // here, which is one term better than the fallback `claim` can compute on its own (it has + // no Target read, so it never sees a stored sitemap `changefreq` interval). The demand + // ladder's rung is not known yet — a target has none until it has rendered — so the first + // cycle scores at the ceiling and the post-render write corrects it. + renderInterval: interval, })) ); diff --git a/packages/plugin/src/schemas/schema.graphql b/packages/plugin/src/schemas/schema.graphql index 8f9ced2..3122786 100644 --- a/packages/plugin/src/schemas/schema.graphql +++ b/packages/plugin/src/schemas/schema.graphql @@ -114,6 +114,19 @@ type RenderSchedule @table(database: "render_schedule") @export { # cross-database read just to flag sitemap-sourced jobs. Refreshed on every # reschedule (job result), so it self-corrects when a URL leaves its sitemap. fromSitemap: Boolean + # The EFFECTIVE cadence (ms) this due time was scheduled at — route interval, stored + # interval and demand-ladder rung already resolved. Denormalized for the same reason as + # `fromSitemap`: `claim` orders the due set by how overdue each row is RELATIVE TO ITS OWN + # cadence (util/renderPriority.js), and reading it per row would be a cross-database Target + # read on the hot claim path for every candidate in the window. + # + # OPTIONAL, unlike `fromSitemap`, and the asymmetry is deliberate. `put` replaces the record, + # so a writer that omits either field clears it — but omitting `fromSitemap` makes the + # renderer stop serializing a non-indexable sitemap page (a silent stop-caching bug), whereas + # omitting this one only makes the claim pass fall back to the ROUTE-resolved interval for + # that row's priority. That degrades ordering until the row's next render re-stamps it, and + # degrades nothing else — so it is an optional argument rather than a required one. + renderInterval: Long } type QueueStatus @table(database: "render_service") @sealed @export(name: "queue_status") { diff --git a/packages/plugin/src/util/invalidationReenqueue.js b/packages/plugin/src/util/invalidationReenqueue.js index d36d09e..b8ee29b 100644 --- a/packages/plugin/src/util/invalidationReenqueue.js +++ b/packages/plugin/src/util/invalidationReenqueue.js @@ -316,7 +316,15 @@ export const accelerateHeal = async ({ url, cacheKey, invalidatedBy }) => { // schedule row we just read: `put` REPLACES the record, and the target is the field's source of // truth, so this self-corrects a row whose flag went stale (same choice as the reschedule path). await writeSchedules( - eligible.map((row) => ({ cacheKey: row.cacheKey, nextRenderTime: dueAt, fromSitemap: !!target.sitemapUrl })) + eligible.map((row) => ({ + cacheKey: row.cacheKey, + nextRenderTime: dueAt, + fromSitemap: !!target.sitemapUrl, + // Already resolved above for the eligibility arithmetic, so recording it costs nothing + // and keeps an accelerated row's priority cadence rather than clearing it — `put` + // replaces the record, and a cleared field falls back to the route's interval. + renderInterval: interval, + })) ); } catch (e) { logger.error(e, `[prerender] could not accelerate ${cacheKey} after an invalidation`); diff --git a/packages/plugin/src/util/renderPriority.js b/packages/plugin/src/util/renderPriority.js new file mode 100644 index 0000000..efa0882 --- /dev/null +++ b/packages/plugin/src/util/renderPriority.js @@ -0,0 +1,165 @@ +/** + * WHICH OF THE DUE ROWS GOES FIRST. + * + * `claim` orders by `nextRenderTime` and nothing else, and `util/failureBackoff.js` states the + * consequence plainly: "deprioritize this" and "make it due later" are the same statement, which is + * why none of this needs a priority column or a second index. THE INDEX HALF OF THAT STILL HOLDS and + * is the constraint this module works inside — there is still exactly one indexed attribute on + * `RenderSchedule`, still one seek from the claim floor, and still one condition on the query. What + * does not hold is the claim that a due time alone expresses priority, because once two rows are both + * past due their relative order is fixed by due times, and a due time encodes when a page last + * rendered plus its cadence — not how much it matters: + * + * home (1h cadence) due 2h ago -> 2 intervals late, serving a page 3h old + * PDP (48h cadence) due 3h ago -> 0.06 intervals late, serving a page 51h old + * + * Index order gives the PDP the lease, because 3h > 2h. Every signal reads as healthy: the floor is + * advancing, the scan is fast, nothing is wedged. The homepage is simply always behind a wall of + * PDPs that are each a few minutes older in absolute terms, and the only visible symptom is the one + * `config.yaml` already describes — worst-case served age of `interval + swrTtl`, which is 7x the + * homepage's cadence and 0.125x a PDP's. + * + * So priority is still a function of due time. It is just measured in units of the page's OWN + * cadence rather than in absolute milliseconds: + * + * overdue ratio = max(0, now - dueAt) / renderInterval + * + * ── WHY LATENESS AND NOT AGE ──────────────────────────────────────────────────────────────────── + * + * The tempting form is `(now - lastRender) / interval`, i.e. staleness relative to cadence, which is + * the same number plus one and reads better. It is WRONG here, because `dueAt - interval` is not + * when the page last rendered for every row in the table. Three writers deliberately schedule a gap + * that is not the cadence: `Target.suppress` writes `render.suppression.recheckInterval` (7 days), + * `backoffWait` writes up to `render.failureRetry.maxBackoff`, and `maybeUnpinFloor` pushes a row + * forward by `render.defaultInterval` regardless of its route. Under the age form, a 7-day + * suppression recheck on a 48h route arrives at the head of the queue reading as 3.5 intervals stale + * and outranks a genuinely late homepage — i.e. the rows we most want to DEPRIORITIZE would be + * promoted, and the suppression-recheck load this release exists to reduce would get there first. + * + * Lateness has no such coupling: it is zero at the moment a row comes due whatever gap preceded it, + * so a recheck or a backed-off retry enters at the BACK of the due set and climbs from there like + * anything else. That is the property worth the slightly worse-reading formula. + * + * ── STARVATION IS BOUNDED, AND THE BOUND IS STATABLE ──────────────────────────────────────────── + * + * `sitemapBoost` is a MULTIPLIER on the ratio, never an additive tier or a separate lane. A lane + * would let a large sitemap corpus starve discovered URLs outright; a multiplier cannot, because the + * ratio of a row that is not being served grows without bound while the boost stays constant. A + * non-sitemap row wins as soon as its ratio exceeds `boost x` the highest sitemap ratio in the + * window, so if the sitemap set is being held at `U` intervals late, a discovered URL is served by + * `boost x U` intervals late — at the default boost of 2 with sitemap pages held ~1.2 intervals + * late, a discovered 48h PDP renders within ~115h. Whether that bound is comfortable is a judgement + * about the corpus; that there IS one is a property of the formula. + * + * ── WHAT THIS CANNOT DO ───────────────────────────────────────────────────────────────────────── + * + * It reorders WITHIN the claim window and nowhere else. The window is + * `min(grantLimit + in-flight + grantLimit, queue.claimScanCap)` rows starting at the claim floor — + * roughly 550 rows at the recorded occupancy, against a corpus of 1.6M keys, so this chooses 25 from + * ~550 candidates rather than 25 from the whole backlog. That is a real limit and it is the reason + * this is a latency fix and NOT a capacity fix: total render volume is unchanged, which is also why + * it is safe to ship on by default. If a route is short of capacity outright, no ordering rescues it + * — `renderInterval` and fleet size are the levers, and `queue.claimScanCap` widens the pool this + * chooses from if the window is the binding constraint. + * + * ── AND WHAT IT COSTS ─────────────────────────────────────────────────────────────────────────── + * + * One unindexed `Long` on `RenderSchedule` (`renderInterval`, the effective cadence, denormalized + * for the same reason `fromSitemap` is), one full walk of the drained window instead of a walk that + * stops at `grantLimit`, and one sort of the grantable rows. The walk is the only one worth stating: + * a caught-up pass breaks at the first not-yet-due row exactly as before, so the extra work appears + * only when there IS a backlog — which is when there is something to choose. Measured against the + * pass budget it is noise either way: ~550 lease probes and one sort against a 0.43 ms scan, on a + * path that runs roughly 0.2 times a second per node. + */ + +/** + * How overdue a row is, in units of its own render cadence, with sitemap membership applied. + * + * Clamped at zero rather than allowed to go negative: `runClaimPass` only ever scores rows it has + * already established are due, and a negative score from a clock skew would sort a due row BELOW + * rows that are exactly on time, which is the one ordering that makes no sense at all. + * + * `intervalMs` is trusted to be a positive finite number — `resolveRenderInterval` guarantees that + * (it falls back to `render.defaultInterval`), and re-validating it here would hide a caller that + * had started passing nonsense. The one guard that stays is the division: a zero or negative + * interval would produce Infinity or a sign flip, so it degrades to pure lateness instead. + */ +export const overdueRatio = ({ nextRenderTime, fromSitemap }, { nowMs, intervalMs, sitemapBoost = 1 }) => { + const lateness = Math.max(0, nowMs - nextRenderTime); + const ratio = intervalMs > 0 ? lateness / intervalMs : lateness; + return fromSitemap ? ratio * sitemapBoost : ratio; +}; + +/** + * Order due rows most-urgent-first, in place. + * + * THE TIEBREAKS ARE THE INTERESTING PART. The ratio is zero for every row at the instant it comes + * due, so on a caught-up node — which is the state the queue spends most of its time in — the + * primary key carries no information at all and the tiebreaks decide everything: + * + * 1. ratio x boost, descending — the actual priority statement. + * 2. sitemap before non-sitemap. The boost is multiplicative, so it vanishes at ratio 0 and could + * not express "prefer the sitemap page" in exactly the caught-up case where nothing else + * distinguishes the two. This is that half, and it is a tiebreak rather than a term added to + * the ratio precisely so it cannot survive into the backlogged case and become an unbounded + * lane. + * 3. due time, ascending — FIFO. Keeps the caught-up node's behaviour identical to the pre-change + * index order rather than merely unspecified, so turning this on changes nothing observable + * until there is a backlog to reorder. + * + * `sort` is not required to be stable for correctness here, but V8's is, so equal rows keep the + * index order the scan delivered them in. + * + * @param {Array<{cacheKey: string, nextRenderTime: number, fromSitemap: boolean, renderInterval?: number}>} rows + * due rows, mutated in place + * @param {(cacheKey: string) => number} intervalFor fallback cadence resolver, for a row that + * carries no stored `renderInterval` (in ms) + * @param {{nowMs: number, sitemapBoost: number}} opts + */ +export const orderByPriority = (rows, intervalFor, { nowMs, sitemapBoost }) => { + // Score once per row, not once per comparison: `sort` calls the comparator O(n log n) times, and + // the fallback resolver parses a URL and walks the route list. At a 550-row window that is the + // difference between 550 route resolutions and ~5,500 of them. + const scored = new Map(); + for (const row of rows) { + const score = overdueRatio(row, { nowMs, intervalMs: intervalOf(row, intervalFor), sitemapBoost }); + scored.set(row, score); + // STAMPED, not just kept in the map. This number is the only evidence that the ordering did + // anything: `claim` reports the score of every job it grants, and a distribution sitting at + // several intervals late is the signal that the fleet is short of capacity for that route + // rather than merely ordering it badly. Recomputing it in the caller would mean resolving + // every granted row's cadence a second time. + row.priority = score; + } + + rows.sort((a, b) => { + const byRatio = scored.get(b) - scored.get(a); + if (byRatio !== 0) return byRatio; + const bySitemap = (b.fromSitemap ? 1 : 0) - (a.fromSitemap ? 1 : 0); + if (bySitemap !== 0) return bySitemap; + return a.nextRenderTime - b.nextRenderTime; + }); + + return rows; +}; + +/** + * The cadence to score a row against: the EFFECTIVE interval denormalized onto the row when it is + * there, else whatever the fallback resolver says. + * + * The stored value wins because it is the only one that knows about the demand ladder. `claim` has + * no Target read, so the fallback can see the ROUTE's interval and nothing else — and at the + * recorded configuration the catalog routes declare a 24h ceiling that the ladder promotes to 12h + * or 6h per URL. Scoring a promoted page at its ceiling would under-prioritize precisely the pages + * the ladder singled out as the most-visited, which is the opposite of the intent. + * + * The fallback is not a degenerate case, though, and it is why this is a fallback rather than a + * requirement: a row is written by several paths that do not have the effective interval in hand + * (a reconcile repair, an invalidation re-enqueue, a render-now one-off), and `put` replaces the + * record — so any of them clears the field until that URL's next completed render re-stamps it. + * Route-resolved is the right answer in the meantime, and for a route that declares its own + * `renderInterval` it is also usually the exact answer. + */ +const intervalOf = (row, intervalFor) => + Number.isFinite(row.renderInterval) && row.renderInterval > 0 ? row.renderInterval : intervalFor(row.cacheKey); diff --git a/packages/plugin/src/util/renderSchedule.js b/packages/plugin/src/util/renderSchedule.js index d3c9757..aadad3e 100644 --- a/packages/plugin/src/util/renderSchedule.js +++ b/packages/plugin/src/util/renderSchedule.js @@ -128,6 +128,7 @@ import { config } from '../config.js'; import { getSab } from './coordination.js'; import { CacheKey } from './cacheKey.js'; import { resolveRenderInterval } from './routeClass.js'; +import { orderByPriority } from './renderPriority.js'; import { MINUTE, numberOf } from './time.js'; import { LEASE_SAB_KEY, createLeaseTable, leaseBufferBytes, leaseSlotsIn } from './renderLease.js'; @@ -216,6 +217,20 @@ const lowerFloorFor = (nextRenderTime) => { leaseTable().lowerFloorTo(minuteOf(at)); }; +/** + * The cadence to store on a row, or `undefined` to store nothing. + * + * Nonsense is dropped rather than stored or thrown on: this field exists only to make the claim + * pass's priority ordering accurate, so a bad value must degrade that row to the route-resolved + * fallback and must never be able to fail a schedule write that is otherwise correct. `Number` + * first because a `Long` column round-trips as a BigInt, which `Number.isFinite` rejects outright. + */ +const intervalToStore = (renderInterval) => { + if (renderInterval === undefined || renderInterval === null) return undefined; + const ms = Number(renderInterval); + return Number.isFinite(ms) && ms > 0 ? Math.round(ms) : undefined; +}; + /** * Write one schedule row and lower the floor to match. * @@ -240,11 +255,11 @@ const lowerFloorFor = (nextRenderTime) => { * in 10.7 ms, mean 0.021 ms, against residency pinned to a node that does not exist). v0.15.0 * assumed the read/write symmetry and wrapped these in a deadline that could never fire. */ -export const writeSchedule = async (cacheKey, { nextRenderTime, fromSitemap } = {}) => { +export const writeSchedule = async (cacheKey, { nextRenderTime, fromSitemap, renderInterval } = {}) => { if (fromSitemap === undefined) { throw new Error(`writeSchedule(${cacheKey}) needs an explicit fromSitemap — put replaces the record`); } - await scheduleTable().put(cacheKey, { nextRenderTime, fromSitemap }); + await scheduleTable().put(cacheKey, { nextRenderTime, fromSitemap, renderInterval: intervalToStore(renderInterval) }); lowerFloorFor(nextRenderTime); }; @@ -260,11 +275,15 @@ export const writeSchedule = async (cacheKey, { nextRenderTime, fromSitemap } = */ export const writeSchedules = async (rows = []) => { let lowest = Number.POSITIVE_INFINITY; - for (const { cacheKey, nextRenderTime, fromSitemap } of rows) { + for (const { cacheKey, nextRenderTime, fromSitemap, renderInterval } of rows) { if (fromSitemap === undefined) { throw new Error(`writeSchedules(${cacheKey}) needs an explicit fromSitemap — put replaces the record`); } - await scheduleTable().put(cacheKey, { nextRenderTime, fromSitemap }); + await scheduleTable().put(cacheKey, { + nextRenderTime, + fromSitemap, + renderInterval: intervalToStore(renderInterval), + }); // Same trap as `lowerFloorFor`, and WORSE here: with a bare `Number` a single null row anywhere // in the batch becomes 0, wins the minimum, and unbounds the floor for the whole fan-out. const at = numberOf(nextRenderTime); @@ -296,6 +315,12 @@ export const getScheduleRow = (cacheKey, select) => * * `floorRule` exists solely so the regression test can drive the SAME trace through the rejected * `'last-granted'` rule and assert that it strands rows. Production never sets it. + * + * `prioritize(candidates, nowMs)` reorders the grantable due rows in place, and is the ONLY thing + * that decides which of them get the leases. Omitted (the default) the pass walks and grants in + * index order and stops at `grantLimit`, which is byte-for-byte the behaviour that predates + * `util/renderPriority.js` — so every existing trace through this function still describes it, and + * the config kill switch is expressed by simply not passing an orderer. */ export const runClaimPass = async ({ searchSchedules, @@ -304,18 +329,28 @@ export const runClaimPass = async ({ grantLimit = 20, guardMinutes: guard = 5, scanCap = 1000, + candidatePool = 0, leaseTimeMs, floorEnabled = true, floorRule = 'first-due-observed', + prioritize = null, } = {}) => { const nowMinute = minuteOf(nowMs); const floorFrom = floorEnabled ? leases.readFloorMinute(nowMinute, guard) : 0; // A leased row keeps its overdue position in the index now, so the pass must read PAST the // in-flight pile to find grantable rows: grantLimit to cover the pile's own head, the pile - // itself, and grantLimit to actually grant. Capped, because during a broad failure event the - // pile is the entire fleet's worth of jobs and this must not become an unbounded read. - const scanLimit = Math.min(grantLimit + leases.occupancy() + grantLimit, scanCap); + // itself, and enough beyond it to actually grant. Capped, because during a broad failure event + // the pile is the entire fleet's worth of jobs and this must not become an unbounded read. + // + // `candidatePool` is what that last term becomes when the pass is CHOOSING rather than just + // taking. Ordering by priority is worth nothing if the window holds barely more rows than the + // leases it is handing out — and the pre-existing window is exactly that shape: `grantLimit` past + // the pile, so ~25 grantable rows to pick 25 from. Widening it is the difference between "the + // best of what happened to be at the head of the index" and an actual choice, and the extra rows + // are index-ordered reads inside a window the seek already landed in. Zero (the default) keeps + // the historical `2 x grantLimit + pile`. + const scanLimit = Math.min(grantLimit + leases.occupancy() + Math.max(grantLimit, candidatePool), scanCap); // DRAIN THE WHOLE ITERABLE FIRST. No write, no Atomics store and no lease grant while the // cursor is open: Harper's long-transaction monitor ABORTS (and poisons) a transaction that @@ -344,6 +379,16 @@ export const runClaimPass = async ({ let skippedLeased = 0; let nonFinite = 0; + // PHASE 1 — DERIVE THE FLOOR AND COLLECT THE GRANTABLE ROWS, IN INDEX ORDER, ALWAYS. + // + // The floor rule is defined over the order the SCAN delivered, and it stays that way whatever + // `prioritize` does downstream: the floor is a VALUE — the due minute of the first due row this + // pass observed — and the first row in index order is by construction the minimum due minute in + // the window. So reordering which rows get the leases cannot move the floor, and phase 2 is free + // to grant in any order it likes. Deriving the floor from a PRIORITY-ordered walk instead would + // hand it the minute of the most-overdue-by-ratio row, which is not the minimum, and every row + // below it would be stranded — the terminal render gap this module exists to prevent. + const candidates = []; for (const row of rows) { // `numberOf` because `Number(null)` is 0, which reads as "due since 1970" and would make an // absent due time the oldest due row in the corpus — pinning the floor at the epoch and naming @@ -372,10 +417,33 @@ export const runClaimPass = async ({ skippedLeased++; continue; } + // WITHOUT PRIORITY, STOP AT `grantLimit` EXACTLY WHERE THE SINGLE-PASS LOOP USED TO — after the + // leased check, before the grant — so an unprioritized pass walks the identical prefix and + // produces the identical result. WITH priority the walk has to continue: the whole point is to + // choose `grantLimit` rows out of the window, and stopping at the first `grantLimit` grantable + // ones would be choosing out of nothing. + if (!prioritize && candidates.length >= grantLimit) break; + // `nextRenderTime: at`, not `row.nextRenderTime`: a `Long` column can surface the due time as a + // BigInt, and mixing one into the scoring arithmetic throws on the first `-` against a Number. + candidates.push({ + cacheKey: row.cacheKey, + nextRenderTime: at, + dueMinute, + fromSitemap: !!row.fromSitemap, + // `numberOf` for the BigInt a `Long` column round-trips as; an absent or unusable value + // stays absent, and `orderByPriority` resolves the route interval for it instead. + renderInterval: numberOf(row.renderInterval), + }); + } + + // PHASE 2 — GRANT. Priority ordering, when enabled, applies here and only here. + if (prioritize && candidates.length > 1) prioritize(candidates, nowMs); + + for (const candidate of candidates) { if (jobs.length >= grantLimit) break; const expiresAtMs = nowMs + leaseTimeMs; - if (!leases.grant(row.cacheKey, { dueMinute, leaseExpiryMs: expiresAtMs })) { + if (!leases.grant(candidate.cacheKey, { dueMinute: candidate.dueMinute, leaseExpiryMs: expiresAtMs })) { // No slot, no job. A granted-but-unrecorded job is a double render AND an untracked // hold on the floor; refusing to hand it out is the only safe answer. // @@ -385,8 +453,16 @@ export const runClaimPass = async ({ leaseRefused = true; break; } - lastGrantedMinute = dueMinute; - jobs.push({ cacheKey: row.cacheKey, dueMinute, expiresAtMs, fromSitemap: !!row.fromSitemap }); + lastGrantedMinute = candidate.dueMinute; + jobs.push({ + cacheKey: candidate.cacheKey, + dueMinute: candidate.dueMinute, + expiresAtMs, + fromSitemap: candidate.fromSitemap, + // Undefined when the pass did not order (the kill switch), which is what keeps the metric + // silent rather than reporting a zero it never computed. + priority: candidate.priority, + }); } const observed = floorRule === 'last-granted' ? lastGrantedMinute : firstDueMinute; @@ -533,6 +609,41 @@ const maybeUnpinFloor = async (pass) => { }; /** `runClaimPass` bound to the live table and config. Called by `RenderQueue.claim`. */ +/** + * The in-place orderer `runClaimPass` grants from, or `null` for index order. + * + * Built per pass rather than once, because `queue.priority` is live-reloadable and a captured + * closure would keep serving the boot-time boost after an operator changed it. + * + * THE MEMO IS PER PASS, DELIBERATELY, and it is not a cache. `cacheKeysOf` fans one URL out to one + * row per device type, so both variants of a page sit in the same window and share one route + * resolution — that is the whole win, and it is exactly the locality a per-pass map captures. A + * process-lifetime cache would instead accumulate an entry per URL in a 814k-target corpus to serve + * a window of ~550, and it would have to be invalidated on every config apply (the route list is + * live-reloadable) or it would answer with the previous cadence indefinitely. + */ +const priorityOrderer = () => { + const { enabled, sitemapBoost } = config.queue.priority; + if (!enabled) return null; + + return (candidates, nowMs) => { + const memo = new Map(); + const intervalFor = (cacheKey) => { + const url = CacheKey.extractUrl(cacheKey); + let interval = memo.get(url); + if (interval === undefined) { + // No stored interval to pass: `claim` takes no Target read, which is the point of + // denormalizing the cadence onto the row in the first place. This is the fallback for a + // row that has none — see `intervalOf` in util/renderPriority.js. + interval = resolveRenderInterval(url, undefined); + memo.set(url, interval); + } + return interval; + }; + orderByPriority(candidates, intervalFor, { nowMs, sitemapBoost }); + }; +}; + export const claimSchedules = async ({ grantLimit } = {}) => { const pass = await runClaimPass({ searchSchedules: ({ floorMinute, limit }) => @@ -559,7 +670,7 @@ export const claimSchedules = async ({ grantLimit } = {}) => { sort: { attribute: 'nextRenderTime' }, // ARRAY select. A string `select` returns the bare scalar rather than a record — // the trap that has caused two silent bugs in this package already. - select: ['cacheKey', 'nextRenderTime', 'fromSitemap'], + select: ['cacheKey', 'nextRenderTime', 'fromSitemap', 'renderInterval'], limit, }, { replicateFrom: false } @@ -569,8 +680,14 @@ export const claimSchedules = async ({ grantLimit } = {}) => { grantLimit, guardMinutes: guardMinutes(), scanCap: Math.max(1, config.queue.claimScanCap | 0), + // Only when the pass is actually ordering: with priority off there is nothing to choose + // between, so the wider read would be pure cost. + candidatePool: config.queue.priority.enabled + ? Math.max(0, grantLimit * (config.queue.priority.candidatePool | 0)) + : 0, leaseTimeMs: config.queue.jobLeaseTime, floorEnabled: config.queue.claimFloor.enabled, + prioritize: priorityOrderer(), }); // Whatever this pass saw, including `null` for "nothing is due": a stale key here would name an diff --git a/packages/plugin/test/renderPriority.test.js b/packages/plugin/test/renderPriority.test.js new file mode 100644 index 0000000..dbb9df6 --- /dev/null +++ b/packages/plugin/test/renderPriority.test.js @@ -0,0 +1,382 @@ +import { test, before } from 'node:test'; +import assert from 'node:assert/strict'; + +/** + * Priority ordering of the due set — `util/renderPriority.js` on its own, and then driven through + * `runClaimPass` so the interaction with the claim floor is pinned rather than assumed. + * + * What is pinned here, and why each one is a bug that no other test in this package would catch: + * + * - THE FLOOR IS STILL DERIVED IN INDEX ORDER. This is the whole hazard of the change. The floor + * is "the due minute of the first due row the pass observed", and if that observation moves to + * the PRIORITY-ordered walk it becomes the minute of the most-overdue-by-ratio row instead of + * the minimum — and every row below it is stranded forever, silently. So: a pass whose priority + * order is the exact reverse of its index order must still advance the floor to the earliest + * due minute in the window. + * - LATENESS, NOT AGE. A 7-day suppression recheck coming due on a 48h route must NOT outrank a + * genuinely late page. This is the one formula error that would quietly invert the release's + * whole purpose, promoting exactly the rechecks it exists to make cheaper. + * - THE STORED CADENCE BEATS THE ROUTE FALLBACK. Without it a demand-ladder promotion is invisible + * to ordering, so the pages the ladder singled out as most-visited are ranked at their route's + * ceiling — the opposite of promoting them. + * - A DISABLED PASS IS THE OLD PASS, EXACTLY. Same grants, same order, same early stop at + * `grantLimit`. The kill switch has to be a revert, not a re-weighting. + * - THE BOOST CANNOT BECOME A LANE. A discovered row that is far enough behind must beat a + * sitemap row that is not, or `sitemapBoost` is a starvation mechanism. + */ + +const MINUTE = 60_000; +const HOUR = 60 * MINUTE; +const DAY = 24 * HOUR; +const T0 = 1_700_000_400_000; // a whole minute +const minuteOf = (ms) => Math.floor(ms / MINUTE); + +let priority, funnel, lease; + +const sabs = new Map(); + +before(async () => { + globalThis.server = { hostname: 'test-node', nodes: [], config: { http: { port: 9926 } } }; + globalThis.logger = { debug() {}, info() {}, warn() {}, error() {} }; + globalThis.databases = { + coordination: { + SharedBuffer: { + primaryStore: { + getUserSharedBuffer: (key, buffer) => { + if (!sabs.has(key)) sabs.set(key, buffer); + return sabs.get(key); + }, + tryLock: () => true, + unlock() {}, + }, + }, + }, + render_schedule: { RenderSchedule: { put: async () => {}, delete: async () => {}, search: () => [] } }, + }; + + priority = await import('../src/util/renderPriority.js'); + funnel = await import('../src/util/renderSchedule.js'); + lease = await import('../src/util/renderLease.js'); +}); + +// ---- the scoring function ---------------------------------------------------------------------- + +test('a page late by one of its own intervals scores 1, whatever that interval is', () => { + const home = priority.overdueRatio( + { nextRenderTime: T0 - HOUR, fromSitemap: false }, + { nowMs: T0, intervalMs: HOUR } + ); + const pdp = priority.overdueRatio( + { nextRenderTime: T0 - 48 * HOUR, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + assert.equal(home, 1); + assert.equal(pdp, 1); +}); + +test('the config-documented inversion: a 1h page 2h late outranks a 48h page 3h late', () => { + const home = priority.overdueRatio( + { nextRenderTime: T0 - 2 * HOUR, fromSitemap: false }, + { nowMs: T0, intervalMs: HOUR } + ); + const pdp = priority.overdueRatio( + { nextRenderTime: T0 - 3 * HOUR, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + // Absolute due time says the PDP (3h > 2h). The ratio says the homepage, by ~32x. + assert.ok(home > pdp, `${home} should beat ${pdp}`); + assert.equal(home, 2); +}); + +test('a row scored at its exact due moment is 0, not negative, even with the clock behind it', () => { + assert.equal(priority.overdueRatio({ nextRenderTime: T0, fromSitemap: false }, { nowMs: T0, intervalMs: HOUR }), 0); + assert.equal( + priority.overdueRatio({ nextRenderTime: T0 + HOUR, fromSitemap: false }, { nowMs: T0, intervalMs: HOUR }), + 0 + ); +}); + +test('a zero or negative interval degrades to raw lateness instead of Infinity or a sign flip', () => { + assert.equal(priority.overdueRatio({ nextRenderTime: T0 - 5 }, { nowMs: T0, intervalMs: 0 }), 5); + assert.equal(priority.overdueRatio({ nextRenderTime: T0 - 5 }, { nowMs: T0, intervalMs: -HOUR }), 5); +}); + +test('LATENESS, NOT AGE: a 7-day suppression recheck coming due does not outrank a late page', () => { + // The row `Target.suppress` wrote: due now, but scheduled 7 days ago and scored against the + // 48h route cadence. Under an age-based formula this reads as 3.5 intervals stale and wins. + const recheck = priority.overdueRatio( + { nextRenderTime: T0, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + const latePdp = priority.overdueRatio( + { nextRenderTime: T0 - 6 * HOUR, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + assert.equal(recheck, 0); + assert.ok(latePdp > recheck); +}); + +test('a backed-off retry enters at the back of the due set, not the front', () => { + // `backoffWait` can push a 48h key out by `maxBackoff`. When it comes due, the gap that preceded + // it was days — but its lateness is zero, so it queues behind anything genuinely overdue. + const backedOff = priority.overdueRatio( + { nextRenderTime: T0, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + const slightlyLate = priority.overdueRatio( + { nextRenderTime: T0 - MINUTE, fromSitemap: false }, + { nowMs: T0, intervalMs: 48 * HOUR } + ); + assert.ok(slightlyLate > backedOff); +}); + +// ---- ordering ---------------------------------------------------------------------------------- + +const order = (rows, { nowMs = T0, sitemapBoost = 2, intervals = {} } = {}) => + priority + .orderByPriority(rows, (cacheKey) => intervals[cacheKey] ?? DAY, { nowMs, sitemapBoost }) + .map((row) => row.cacheKey); + +test('short-cadence routes come first under a backlog that is late for everyone', () => { + const rows = [ + { cacheKey: 'pdp', nextRenderTime: T0 - 4 * HOUR, fromSitemap: false, renderInterval: 48 * HOUR }, + { cacheKey: 'catalog', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false, renderInterval: 6 * HOUR }, + { cacheKey: 'home', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false, renderInterval: HOUR }, + ]; + // Index order is exactly the reverse of the right answer: pdp is the oldest in absolute terms. + assert.deepEqual(order(rows), ['home', 'catalog', 'pdp']); +}); + +test('sitemap wins at equal overdue ratio, and the tiebreak still works at ratio 0', () => { + const late = [ + { cacheKey: 'discovered', nextRenderTime: T0 - HOUR, fromSitemap: false, renderInterval: HOUR }, + { cacheKey: 'listed', nextRenderTime: T0 - HOUR, fromSitemap: true, renderInterval: HOUR }, + ]; + assert.deepEqual(order(late), ['listed', 'discovered']); + + // At ratio 0 the multiplicative boost vanishes (0 x 2 === 0), so the preference has to come from + // the explicit tiebreak or it disappears in exactly the caught-up case. + const onTime = [ + { cacheKey: 'discovered', nextRenderTime: T0, fromSitemap: false, renderInterval: HOUR }, + { cacheKey: 'listed', nextRenderTime: T0, fromSitemap: true, renderInterval: HOUR }, + ]; + assert.deepEqual(order(onTime), ['listed', 'discovered']); +}); + +test('the boost is a multiplier, not a lane: a far-behind discovered row beats a fresher sitemap one', () => { + const rows = [ + { cacheKey: 'listed', nextRenderTime: T0 - HOUR, fromSitemap: true, renderInterval: HOUR }, // 1 x 2 = 2 + { cacheKey: 'discovered', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false, renderInterval: HOUR }, // 3 + ]; + assert.deepEqual(order(rows), ['discovered', 'listed']); +}); + +test('sitemapBoost: 1 leaves ordering on overdue ratio alone', () => { + const rows = [ + { cacheKey: 'listed', nextRenderTime: T0 - HOUR, fromSitemap: true, renderInterval: HOUR }, + { cacheKey: 'discovered', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false, renderInterval: HOUR }, + ]; + assert.deepEqual(order(rows, { sitemapBoost: 1 }), ['discovered', 'listed']); +}); + +test('equal priority keeps FIFO, so a caught-up node orders exactly as the index delivered it', () => { + const rows = [ + { cacheKey: 'a', nextRenderTime: T0 - 3 * MINUTE, fromSitemap: false, renderInterval: HOUR }, + { cacheKey: 'b', nextRenderTime: T0 - 2 * MINUTE, fromSitemap: false, renderInterval: HOUR }, + { cacheKey: 'c', nextRenderTime: T0 - MINUTE, fromSitemap: false, renderInterval: HOUR }, + ]; + assert.deepEqual(order(rows), ['a', 'b', 'c']); +}); + +test('THE STORED CADENCE BEATS THE ROUTE FALLBACK, which is what makes a ladder promotion visible', () => { + // Both rows are 3h late on a route declaring a 24h ceiling. One has been promoted by the demand + // ladder to 6h and carries that on its row; the other has never been evaluated. + const rows = [ + { cacheKey: 'ceiling', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false }, + { cacheKey: 'promoted', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false, renderInterval: 6 * HOUR }, + ]; + assert.deepEqual(order(rows, { intervals: { ceiling: 24 * HOUR, promoted: 24 * HOUR } }), ['promoted', 'ceiling']); +}); + +test('an absent, NaN or non-positive stored cadence falls back to the resolver rather than scoring wrong', () => { + for (const stored of [undefined, Number.NaN, 0, -1]) { + const rows = [ + { cacheKey: 'slow', nextRenderTime: T0 - HOUR, fromSitemap: false, renderInterval: 48 * HOUR }, + { cacheKey: 'unstamped', nextRenderTime: T0 - HOUR, fromSitemap: false, renderInterval: stored }, + ]; + // The fallback says `unstamped` is an hourly page, so one hour late outranks the 48h row. + assert.deepEqual(order(rows, { intervals: { unstamped: HOUR } }), ['unstamped', 'slow'], `stored=${stored}`); + } +}); + +// ---- through the claim pass, against the floor ------------------------------------------------- + +const harness = ({ rows, slots = 256, now = T0, leaseTimeMs = 10 * MINUTE } = {}) => { + const leases = lease.createLeaseTable({ + buffer: new ArrayBuffer(lease.leaseBufferBytes(slots)), + slots, + now: () => now, + }); + const searchSchedules = ({ floorMinute, limit }) => + (async function* () { + const matching = rows + .filter((row) => Number(row.nextRenderTime) >= floorMinute * MINUTE) + .sort((a, b) => Number(a.nextRenderTime) - Number(b.nextRenderTime)) + .slice(0, limit); + for (const row of matching) yield { ...row }; + })(); + + return { + leases, + pass: (options = {}) => + funnel.runClaimPass({ + searchSchedules, + leases, + nowMs: now, + grantLimit: 20, + guardMinutes: 5, + scanCap: 1000, + leaseTimeMs, + floorEnabled: true, + ...options, + }), + }; +}; + +// The orderer production passes, with every row's cadence supplied inline so this needs no config. +const prioritizeWith = + (intervals, sitemapBoost = 2) => + (candidates, nowMs) => + priority.orderByPriority(candidates, (cacheKey) => intervals[cacheKey] ?? DAY, { nowMs, sitemapBoost }); + +test('THE FLOOR IS STILL THE EARLIEST DUE MINUTE when priority reverses the grant order', async () => { + // Three rows, all due, and the priority order is the exact reverse of the index order. If the + // floor were derived from the granted order it would land on `home`'s minute and strand the + // other two — permanently, because a floor is a value and nothing re-reads below it. + const rows = [ + { cacheKey: 'pdp', nextRenderTime: T0 - 4 * HOUR, fromSitemap: false }, + { cacheKey: 'catalog', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false }, + { cacheKey: 'home', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false }, + ]; + const { pass } = harness({ rows }); + const result = await pass({ + prioritize: prioritizeWith({ pdp: 48 * HOUR, catalog: 6 * HOUR, home: HOUR }), + }); + + assert.deepEqual( + result.jobs.map((j) => j.cacheKey), + ['home', 'catalog', 'pdp'] + ); + assert.equal(result.floorTo, minuteOf(T0 - 4 * HOUR), 'floor must be the earliest due minute, not the first granted'); + assert.equal(result.floorHeldBy, 'pdp', 'the row NAMED as holding the floor is the earliest due one'); +}); + +test('priority chooses which rows get the leases when the window holds more than grantLimit', async () => { + // 6 due rows, 2 leases to give. Index order would hand them to the two oldest-in-absolute-terms + // (the PDPs); priority hands them to the two most overdue relative to their own cadence. + const rows = [ + { cacheKey: 'pdp-1', nextRenderTime: T0 - 6 * HOUR, fromSitemap: false }, + { cacheKey: 'pdp-2', nextRenderTime: T0 - 5 * HOUR, fromSitemap: false }, + { cacheKey: 'catalog-1', nextRenderTime: T0 - 4 * HOUR, fromSitemap: false }, + { cacheKey: 'catalog-2', nextRenderTime: T0 - 3 * HOUR, fromSitemap: false }, + { cacheKey: 'home-1', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false }, + { cacheKey: 'home-2', nextRenderTime: T0 - MINUTE, fromSitemap: false }, + ]; + const intervals = { + 'pdp-1': 48 * HOUR, + 'pdp-2': 48 * HOUR, + 'catalog-1': 6 * HOUR, + 'catalog-2': 6 * HOUR, + 'home-1': HOUR, + 'home-2': HOUR, + }; + + // `candidatePool` is the whole reason this test can pass: the default window is `grantLimit` past + // the lease pile, so at grantLimit 2 the pass would drain 4 rows and never see the homepages. + const prioritized = await harness({ rows }).pass({ + grantLimit: 2, + candidatePool: 6, + prioritize: prioritizeWith(intervals), + }); + assert.deepEqual( + prioritized.jobs.map((j) => j.cacheKey), + ['home-1', 'catalog-1'] + ); + + // The same trace with the kill switch: index order, and it stops after two rows. + const indexOrder = await harness({ rows }).pass({ grantLimit: 2 }); + assert.deepEqual( + indexOrder.jobs.map((j) => j.cacheKey), + ['pdp-1', 'pdp-2'] + ); + // Both derive the same floor. That is the invariant the reordering must not be able to touch. + assert.equal(prioritized.floorTo, indexOrder.floorTo); +}); + +test('a disabled pass stops at grantLimit and never reads past it — the switch is a revert', async () => { + const rows = Array.from({ length: 40 }, (_, i) => ({ + cacheKey: `k${String(i).padStart(2, '0')}`, + nextRenderTime: T0 - (40 - i) * MINUTE, + fromSitemap: false, + })); + + const off = await harness({ rows }).pass({ grantLimit: 5 }); + assert.deepEqual( + off.jobs.map((j) => j.cacheKey), + ['k00', 'k01', 'k02', 'k03', 'k04'] + ); + // Reaching a not-yet-due row is what proves the window was not truncated; a pass that stopped at + // grantLimit never reaches one, and `scanTruncated` reads off exactly that pair. + assert.equal(off.earliestNotYetDueMinute, 0); + + // With priority on, the same window is walked to the end — so it now knows there was no + // not-yet-due row to reach either, and still grants only 5. + const on = await harness({ rows }).pass({ grantLimit: 5, prioritize: prioritizeWith({}) }); + assert.equal(on.jobs.length, 5); +}); + +test('leased rows are skipped, still pin the floor, and are never granted twice', async () => { + const rows = [ + { cacheKey: 'stuck', nextRenderTime: T0 - 10 * HOUR, fromSitemap: false }, + { cacheKey: 'home', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false }, + ]; + const { pass, leases } = harness({ rows }); + leases.grant('stuck', { dueMinute: minuteOf(T0 - 10 * HOUR), leaseExpiryMs: T0 + HOUR }); + + const result = await pass({ prioritize: prioritizeWith({ stuck: 48 * HOUR, home: HOUR }) }); + assert.deepEqual( + result.jobs.map((j) => j.cacheKey), + ['home'] + ); + assert.equal(result.skippedLeased, 1); + // The in-flight row is the earliest due row the pass observed, so it holds the floor — priority + // ordering must not let the floor skip past a row whose result may still be arriving. + assert.equal(result.floorTo, minuteOf(T0 - 10 * HOUR)); +}); + +test('not-yet-due rows are never granted however urgent their cadence would make them', async () => { + const rows = [ + { cacheKey: 'due', nextRenderTime: T0 - MINUTE, fromSitemap: false }, + { cacheKey: 'future', nextRenderTime: T0 + HOUR, fromSitemap: false }, + ]; + const { pass } = harness({ rows }); + const result = await pass({ prioritize: prioritizeWith({ due: 48 * HOUR, future: MINUTE }) }); + assert.deepEqual( + result.jobs.map((j) => j.cacheKey), + ['due'] + ); + assert.equal(result.earliestNotYetDueMinute, minuteOf(T0 + HOUR)); +}); + +test('a BigInt due time from a Long column scores rather than throwing on the subtraction', async () => { + const rows = [ + { cacheKey: 'big', nextRenderTime: BigInt(T0 - 3 * HOUR), fromSitemap: false, renderInterval: BigInt(HOUR) }, + { cacheKey: 'small', nextRenderTime: T0 - 2 * HOUR, fromSitemap: false, renderInterval: HOUR }, + ]; + const { pass } = harness({ rows }); + const result = await pass({ prioritize: prioritizeWith({}) }); + assert.deepEqual( + result.jobs.map((j) => j.cacheKey), + ['big', 'small'] + ); +});