Submit change-probe triggers to a paced queue instead of awaiting them (v0.71.0)

[harper-joseph]

Stacked on #172 — review that first. GitHub retargets this to main when #172 merges. The commit here is the second in the diff.

The coupling

The sweep ran await trigger(row) inside the row handler. Triggering is six database operations, so with the handler's concurrency shared between probing and triggering, trigger volume set pass duration — and pass duration is detection latency, because the gap between two probes of one URL is one pass.

That closes a loop:

more change → more triggers → longer pass → a longer window in which each URL can change → more change detected per pass → more triggers

Measured on one deployment: arming the probe took a pass from 9.2h to a projected ~21h, with bot traffic flat across both windows (153,728/hr vs 150,743/hr on the same node), so contention was ruled out by measurement rather than assumed. Every available knob traded one bad outcome for another — raising maxTriggersPerSweep lengthened the pass, lowering it shed more change. Neither is a fix, because both act on the same coupling.

The change

Triggers are submitted to a bounded queue that drains beside the walk. Pass duration becomes max(probe time, drain time) rather than the sum, so the sweep runs at its probe-rate floor whatever the change rate — and the meaningful limit becomes triggers per second, which is what the render fleet actually experiences, rather than per pass.

What is preserved exactly

• The baseline write still happens only after a successful trigger, and it moved into the queue for that reason. Writing at submit time would lose the change outright on any failure.
• A failed trigger, or one abandoned by stop(), leaves the stored signature stale, so the next pass re-detects and retries — the existing retry story, unchanged.
• A full queue is reported as deferred — the same accounting exhausting the per-pass budget already produced, with the same semantics, so nothing downstream learns a new state.
• An item already in flight when stop() is called still completes and is still baselined: its trigger actually succeeded, and an issued write cannot be un-issued. Only pending work is abandoned.

Two deliberate choices

The canary keeps the immediate shape (createInlineTrigger). Its cohort is a few hundred URLs and its whole value is being fast, so there is nothing for a paced queue to spread. Same contract, so runProbePass has one code path rather than a branch.

In memory, not durable. Losing the queue costs one repeat detection, since every unsettled item still has its old signature stored. Durability would add a table to the probe's write path, which is the one thing ProbeState's design goes out of its way to avoid.

Config and observability

Rate sizing. The default is 5/s, lowered from an initial 20/s after doing the injection arithmetic — this is the one way the change can hurt, because it is something the in-line path could never do. At 20/s a 90,000-trigger budget drains in ~1.25h, injecting renders several times faster than a four-node fleet can claim them, deepening the ready set. Aim for a drain that finishes inside the pass; 90,000 across a 9h pass is ~2.8/s.

New changeProbe.trigger group: ratePerSecond (5), concurrency (4), maxPending (5000). maxTriggersPerSweep still bounds the per-pass total — the two limits answer different questions and the option text says which.

New probe_trigger_queue_depth metric. A queue sitting at maxPending means changes are being deferred for want of queue rather than of budget, and probe_deferred alone cannot distinguish those.

Tests

npm test in packages/plugin: 1062 pass, 0 fail. Lint and format:check clean.

12 new tests in test/triggerQueue.test.js cover the load-bearing properties: submit returns while the trigger is still blocked; the baseline is written only after success and carries clearClaim; a throwing trigger writes nothing and is counted; one failure doesn't stop the queue; a full queue refuses; stop() abandons pending work but completes what's in flight; pacing spaces trigger starts on a fake clock; and the inline shape settles synchronously with the same ordering.

The existing probe tests now drive the real inline trigger rather than a stub, because the trigger-then-write ordering moved into triggerQueue.js and a stub re-implementing it would keep passing while the shipped path regressed.

Versioning

v0.71.0, stacked on #172 (v0.70.0). Note #162 and #163 both currently claim v0.67.0, so the whole set may need renumbering at merge time.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a bounded, separately-paced trigger queue to decouple change-probe triggers from the main walk in the prerender plugin, preventing trigger volume from inflating pass duration. The configuration schema, metrics documentation, and tests have been updated to support this new architecture. Feedback on the changes highlights a potential timeout overflow issue in the queue's pacing logic where the calculated wait delay passed to sleep could exceed the 32-bit signed integer limit, and suggests clamping the value to prevent unexpected immediate execution.

[gemini-code-assist]

The calculated wait delay passed to sleep (which wraps setTimeout) can exceed 2147483647 (the maximum 32-bit signed integer) if ratePerSecond is configured to an extremely small positive value. In Node.js, exceeding this limit causes setTimeout to emit a TimeoutOverflowWarning and execute the callback immediately (after 1ms), which would break the pacing rate-limiting. We should clamp wait to 2147483647 to prevent this.

Suggested change

	const wait = startAt - at;
	if (wait > 0) await sleep(wait);
	const wait = Math.min(startAt - at, 2147483647);
	if (wait > 0) await sleep(wait);

References

1. In Node.js, clamp or validate configuration options representing timeouts or delays passed to setInterval or setTimeout to not exceed 2147483647 (the maximum 32-bit signed integer). Exceeding this limit causes Node.js to execute the callback immediately (after 1ms), which can trigger unexpected hot loops.

[harper-joseph]

Pushed 953fc2a lowering the changeProbe.trigger.ratePerSecond default from 20 to 5.

20 was picked before the injection arithmetic was done, and it is the one way this change can hurt — it is something the old in-line path could never do. At 20/s a 90,000-trigger budget drains in ~1.25h, which on a four-node cluster injects renders several times faster than the fleet can claim them, deepening the ready set and starving its lowest-priority class.

The useful target is a drain that finishes inside the pass, since past that the queue backs up and changes defer for want of queue rather than of budget. 90,000 triggers across a 9h pass is ~2.8/s, so 5/s leaves headroom while still being ~4× the effective rate the in-line path managed. The option text now explains the sizing rather than leaving it to be inferred.

On the reference deployment, spare render capacity is ~38k/hr fleet (87.6k ceiling − 49.4k measured), i.e. ~2.6/s per node — so even 5 is worth reviewing against a measured rate there before arming.

[harper-joseph]

Pushed e79d0b7 (and rebased onto #172's latest): the pacing sleep is now clamped.

Confirmed and worth stating plainly — sleep wraps setTimeout, and past a signed 32-bit delay it fires after 1ms instead of waiting, so an unclamped wait turns the slowest possible drain into an unpaced one. That is the exact inversion probePacer's own clamp exists to prevent, so this reuses its exported MAX_TIMER_MS rather than adding a second literal.

Reachable with an absurdly small ratePerSecond — 1e-7 is a 10,000,000,000 ms slot, past the ceiling on the very first wait. New test pins it on a frozen clock.

npm test in packages/plugin: 1063 pass, 0 fail. Lint and format:check clean.