feat(plugin): crawl breadth — distinct URLs per bot per day (HLL); v0.27.0

[harper-joseph]

What

Crawl breadth: distinct URLs crawled per bot per UTC day, plus % -of-corpus coverage once divided by the Target count — the rollout metric that shows a crawler going deeper into the catalog after each phase flip, which plain request counters cannot express (distinct counts don't add).

Stacked on #64 (feat/bot-serve-metrics) — merge that first; GitHub will retarget this PR to main automatically.

How

HyperLogLog sketches (src/util/hll.js): p=14 → 16 KB register array, ~0.8% standard error, cyrb53 hash (imul-based — no BigInt, no allocation). The load-bearing property: merge = element-wise max = exact set union, byte-for-byte the sketch a single observer would have built — so per-thread/per-node shards decompose one logical global sketch losslessly, duplicates across threads/nodes collapse by construction, and merging never compounds the estimation error.

Recording (src/util/crawlStats.js, wired in bot_request.js behind the existing recordBots gate): per-thread in-memory sketch per bot per day. Hot-path cost is one number compare (day rollover), one Map lookup, one 53-bit string hash + one byte max — no allocation, no await, no storage touch.

Persistence: each worker merges its sketches into one row per (day, bot, node) (crawl_stats.CrawlSketch) on a timer — read-merge-write serialized by the existing cross-worker mutex (util/coordination.js), and cross-node there's no contention because the node is in the key. One row per node (not per thread) keeps the read side inside the admin console's query-cost rules: a week on a 4-node cluster is a few hundred 16 KB rows, not thousands. Day rollover persists the old day via setImmediate; a bounded retention sweep (default 90d) runs at rollover on one worker per node; a failed flush re-marks its bots dirty — sketches are cumulative and merging is idempotent, so whole-flush retry is safe. Loss model: a dying thread loses at most flushInterval of its own unmerged observations — an undercount, never a double count.

Read path: GET /prerender_admin/crawl-breadth?days=N (default 7, max 31) — behind the resource's super-user gate and withHeavySlot, day-indexed range read, capped with a truncated flag rather than presenting a short read as complete. Returns per-day { bots: [{ bot, distinctUrls, shards }], total } where total is the cross-bot union (not a sum). The table is deliberately not @exported — raw shards are only useful merged.

Config (config.crawlStats): enabled, flushInterval (5 min), retentionDays (90), maxBotsPerThread (64 — overflow bots share a visible ~overflow bucket so a UA-derivation flood can't mint unbounded 16 KB sketches).

Tests

• test/hll.test.js — accuracy within 3% at 100 / 5k / 100k / 1M URLs (linear-counting and asymptotic ranges), duplicates never move the estimate, merge is byte-for-byte the union sketch, merge order irrelevant.
• test/crawlStats.test.js — flush merges with the stored row (a second worker's registers survive), mutex taken, bot cap + overflow bucket, UTC rollover persists old day and starts clean, failed-flush retry, computeBreadth day grouping + cross-bot union.

Full suite: 356 pass, lint + format clean.

Version

v0.27.0 (0.25 claimed by #62/#63, 0.26 by #64).

Out of scope

Admin-console panel (stays unwired pending the node-local vs cluster aggregation decision), corpus-count denominator in the endpoint (a Target COUNT(*) is not a click-time query — dashboards divide separately), hydration-marker metric (browser package, next up).

🤖 Generated with Claude Code

[harper-joseph]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a crawl breadth metric to track distinct URLs crawled per bot per UTC day using HyperLogLog (HLL) sketches. It adds configuration options, integrates tracking into the bot request handler, exposes a new admin endpoint, and implements the HLL and persistence logic along with comprehensive tests. The review feedback highlights a critical runtime TypeError in the persistence logic due to trailing parentheses on a lock call, and recommends refactoring the admin endpoint to stream database rows instead of buffering them in memory to prevent event loop starvation and excessive memory usage.

[gemini-code-assist]

This trailing () will cause a runtime TypeError. The await on the withLock call resolves to the result of the locked operation (likely undefined), which is not a function. Removing the parentheses will fix this crash in the persistence logic.

Suggested change

	})();
	});

[harper-joseph]

This one isn't a crash — withLock(fn) in util/mutex.js returns a wrapped function (const withLock = (fn) => { return async (...args) => ... }), so the trailing () is what executes the locked section; removing it would mean the persist silently never runs (the flush tests in test/crawlStats.test.js exercise this path and pass). That said, the misread is fair — the repo's withLock idiom wraps reusable functions (cf. RenderQueue.claim), and an inline one-shot withLock(fn)() reads like an accidental call. Rewritten in 88a1a33 as an explicit lock()/try/finally unlock(), which is unambiguous.

[gemini-code-assist]

This implementation uses Array.fromAsync to buffer all database rows into memory before processing. With a cap of 4096 and each row's registers being 16KB, this could consume up to ~64MB of memory for a single admin request. This can impact a worker that also serves traffic and goes against the repository's general rule to prefer streaming for large datasets.

Let's refactor this to process the rows as a stream using for await...of, which will be much more memory-efficient. Additionally, to prevent event loop starvation when iterating over large database cursors, we should yield the event loop unconditionally (e.g., using setImmediate every 200 rows).

		const searcher = databases.crawl_stats.CrawlSketch.search({
			conditions: [{ attribute: 'day', comparator: 'greater_than_equal', value: since }],
			select: ['day', 'bot', 'registers'],
			limit: cap + 1, // one extra row = "truncated", never merged
		});

		const byDay = new Map();
		let rowCount = 0;
		for await (const row of searcher) {
			rowCount++;
			if (rowCount > cap) break;
			if (rowCount % 200 === 0) {
				await new Promise(resolve => setImmediate(resolve));
			}
			if (!row?.registers || !row.bot) continue;
			let bots = byDay.get(row.day);
			if (!bots) byDay.set(row.day, (bots = new Map()));
			let entry = bots.get(row.bot);
			if (!entry) bots.set(row.bot, (entry = { registers: createSketch(), shards: 0 }));
			mergeSketch(entry.registers, row.registers);
			entry.shards++;
		}

		const truncated = rowCount > cap;
		const shardsMerged = Math.min(rowCount, cap);

		const breadth = [...byDay]
			.sort(([a], [b]) => (a < b ? 1 : -1))
			.map(([forDay, bots]) => {
				const union = createSketch();
				const perBot = [...bots]
					.map(([bot, { registers, shards }]) => {
						mergeSketch(union, registers);
						return { bot, distinctUrls: estimateSketch(registers), shards };
					})
					.sort((a, b) => b.distinctUrls - a.distinctUrls);
				return { day: forDay, total: estimateSketch(union), bots: perBot };
			});

		return json({
			node: server.hostname,
			days,
			since,
			shardsMerged,
			truncated,
			breadth,
		});

References

1. Use a streaming for await...of loop instead of Array.fromAsync when processing or counting a large number of database/index entries to avoid buffering all rows in memory, and periodically yield to the event loop (e.g., every 200 rows) to maintain responsiveness.
2. When iterating over large database cursors or registries in Node.js, yield the event loop unconditionally based on the number of rows scanned to prevent event loop starvation.

[harper-joseph]

Agreed — fixed in 88a1a33. The route now for-awaits the cursor, merging each row into the per-(day, bot) accumulator and releasing it (resident set is one sketch per (day, bot), not cap × 16 KB of raw rows), and yields the event loop every 200 rows via the file's existing yieldNow. computeBreadth split into mergeBreadthRow + finalizeBreadth so the streaming route and the in-memory composition share one implementation; tests unchanged and green. Also swept the rest of the PR for the same buffering class: sweepExpired streams its cursor already and selects only id (limit 1000), and the other Array.fromAsync uses in this file are small-field, page-sized reads.

[harper-joseph]

Addressed both review comments in 88a1a33: the crawl-breadth read path now streams the cursor with periodic event-loop yields instead of buffering up to 64 MB, and the persist section uses explicit lock/try/finally in place of the inline withLock(fn)() one-shot (which was correct — withLock returns a wrapped function — but unidiomatic enough to read as a bug; see the inline reply). 356 tests pass, lint clean.

[gemini-code-assist]

Code Review

This pull request introduces a crawl breadth metric that tracks distinct URLs crawled per bot per UTC day using HyperLogLog (HLL) sketches. It adds configuration options, an admin endpoint, a GraphQL schema for CrawlSketch, and utility modules for HLL and crawl stats, complete with unit tests. The review feedback highlights two main improvements: correcting the YAML indentation in the README documentation so that recordUnmatched and bots are properly nested under analytics, and replacing Array.fromAsync with a streaming for await...of loop in PrerenderAdmin.js to prevent buffering large rows in memory.

[gemini-code-assist]

The documentation for recordUnmatched and bots appears to be incorrect. In src/config.js, these are properties of the analytics object. However, in this README example, they are shown at the top level, alongside crawlStats.

To avoid confusion, the example configuration in the README should accurately reflect the structure defined in the code. It should likely be structured as follows:

  analytics:
    enabled: true
    # ... other analytics properties
    recordUnmatched: true
    bots:
      - { name: Googlebot, match: googlebot }

  crawlStats:
    enabled: true
    # ... crawlStats properties

[gemini-code-assist]

Using Array.fromAsync here buffers all rows from the database search into memory before processing. With a cap of 4096 rows and each row's registers being 16KB, this could lead to significant memory consumption (up to 64MB) on a worker node, potentially impacting performance.

A general rule in this repository advises against this pattern: "Use a streaming for await...of loop instead of Array.fromAsync ... to avoid buffering all rows in memory."

To improve memory efficiency, consider processing the rows as a stream using a for await...of loop. This would involve moving the aggregation logic from computeBreadth directly into crawlBreadthInner to build the byDay map as you iterate, thus avoiding the creation of a large intermediate rows array.

References

1. Use a streaming for await...of loop instead of Array.fromAsync when processing or counting a large number of database/index entries (e.g., up to a cap) to avoid buffering all rows in memory.