perf(plugin): stop the sweep at the due boundary

[harper-joseph]

Follow-up to #120, from measuring it on the live cluster after the 0.51.1 rollout.

40% of every sweep was thrown away

The sweep reads a one-sided range — nextRenderTime >= floor — because the two-sided form is catastrophic when the limit cannot fill (measured in #119 at 739ms vs 2.89ms, 256×). The consequence is that after the due rows the query keeps returning rows that are not due, and the loop read every one of them to sweepCap and discarded them.

Measured on the production corpus (RocksDB, 4 nodes, 2026-08-21), straight off the ready set's own SAB header:

due rows	~301,000
rows actually scanned	500,000 (sweepCap)
read and discarded	~198,000 — 40% of the scan
warm sweep	27.3s, so ~11s of it wasted

And the caught-up case — which searchSchedulesFrom's own comment notes is where the queue spends most of its time — read the entire cap to establish that nothing was due.

Rows arrive ascending, so the first not-yet-due row proves the rest of the window is not due either. It now breaks there. A caught-up node reads one row.

Why #120 didn't do this, and why that reasoning was wrong

The comment I wrote in #120 said an early break leaks the read transaction. That conflated two different things, and I should have checked rather than hedged.

An abandoned iterator — driven by hand with .next() and never returned — genuinely does hold its read transaction open. Harper's own long-transaction test (integrationTests/database/longtxn-secondary-index) uses precisely that as its mechanism, and states the contract in as many words: a search() iterator marks the read txn in use and releases it only when fully consumed.

But for await ... of is not that. On break the language calls iterator.return(), and Harper's search iterator implements it — resources/Table.ts:

return() { if (results.onDone) results.onDone(); return dbIterator.return(); }
throw()  { if (results.onDone) results.onDone(); return dbIterator.throw();  }

onDone is what calls txn.doneReadTxn(). So breaking releases the transaction on the same path a full drain does.

One residual worth a reviewer's eye: the return() above is on the sorting iterator wrapper. This query is an indexed range on nextRenderTime sorted by the same attribute, which may resolve to a direct index walk (a different iterator). I have not traced cleanup on that path. The change is safe either way for correctness — a leaked read txn would surface as the long-transaction monitor's "Transaction was open too long" line, which the deploy watcher already greps for and which has not appeared — but if someone knows that path cold, I would rather hear it than infer it.

Why it only became worth doing now

At the ~2.4 µs/row bench/queue-index measured, the waste was ~0.5s and draining the cursor was the free, obviously-safe choice. On the production corpus the same read costs ~55 µs/row, and 0.5s becomes 11s.

The same correction, everywhere the figure is quoted — and this is the more useful half

#120 justified itself with "~2.4 µs/row, flat". Production measures ~55 µs/row warm, ~80 µs/row cold, so a ~300k-row due set is a 27s sweep, not the sub-second one #120 predicted.

The engine is the same in both cases — RocksDB, Harper's default; the bench sets no engine and inherits it. What differs is the corpus. The bench writes 200k rows and reads them immediately. Production holds 1.3M rows that have been rewritten on every render for months, and on an LSM store that is the whole difference: every reschedule is a put superseding the old value, and a range scan walks past each superseded version until compaction removes it.

The harness had already measured this and I read it too narrowly. Q6 measures an unfloored seek degrading 0.073 → 5.60 ms after 40,000 head reschedules — 77× — same engine, same corpus, churn alone. I filed that as "the claim floor is justified" when it was also saying every other number on the page decays with churn. 2.4 µs/row was a floor for a fresh corpus, never a steady state.

The design argument survives intact — reads are still vastly cheaper than writes (76–89 µs/row, patch worse than put), so recomputing in memory still beats encoding priority into nextRenderTime and its 1.3M-row restamp. What does not survive is any sizing done off a fresh-corpus number, and that figure had propagated into README.md, configSchema.js, renderPriority.js, readyQueue.js and renderSchedule.js. All corrected.

bench/queue-index/README.md now leads with it, because the harness will keep being re-run: ratios travel, absolute per-row costs do not. To size for production, run it at production row count with churn, or measure production. Its comparative findings all still hold and are the reason to keep it.

Testing

780 pass / 0 fail. Three of the four new tests were checked to fail against the draining behaviour, not merely to pass against the fix:

test	catches
THE SWEEP STOPS AT THE DUE BOUNDARY	reading past the due set (scanned = due + 1)
a caught-up node reads ONE row	the steady-state case, cap → 1
still reports the earliest not-yet-due minute	a later minute here would make a node with work coming tell the fleet to go idle

The fourth — a due set that fills the cap is still reported truncated — passes either way by design: it guards that breaking early cannot mask a genuinely truncated sweep, which is the way this change could go wrong silently.

Expected effect

~27s → ~16s warm, and the caught-up sweep to near zero. That takes worker 0's duty cycle from ~9% to ~5% at the current 5-minute interval, which is the alternative to raising sweepInterval — and a better one, because a longer interval costs ordering freshness (a row coming due just after a sweep waits for the next one, and the fallback index scan cannot rescue it) and narrows the margin before the set drains. Measured drain on the busiest node is 230 jobs/min against ~4,950 published, i.e. ~21 minutes of supply — so 15-minute sweeps would leave only 1.4× headroom. Removing waste beats trading away freshness.

Lint: the three remaining no-unused-vars errors are pre-existing on main in packages/console/test/trafficView.test.js. The one in bench/queue-index/bench.js was mine and is fixed here.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request optimizes the database sweep operation in sweepReadySet by breaking early out of the cursor loop once the first not-yet-due row is encountered, rather than draining the entire window up to the cap. This optimization addresses a significant performance discrepancy between the B+tree benchmark environment and the production RocksDB environment, where scanning discarded rows was highly expensive. Documentation, comments, and benchmarks have been updated to reflect these engine-specific performance characteristics, and new unit tests have been added to verify the early-break behavior. I have no feedback to provide.

[harper-joseph]

No action taken — the review reports no feedback. But its summary describes the pre-amend version of this PR and repeats an explanation I retracted, so flagging it so a human reader doesn't take it as established:

addresses a significant performance discrepancy between the B+tree benchmark environment and the production RocksDB environment

That was my error and it is wrong. bench/queue-index sets no storage engine, and Harper defaults to RocksDB — so the bench and production ran the same engine. I asserted a B+tree without checking, and Joseph caught it.

The actual cause is corpus churn: the bench writes ~200k rows and reads them immediately, where production holds 1.3M rows rewritten on every render for months. On an LSM store each reschedule is a put superseding the old value, and a range scan walks past every superseded version until compaction removes it. The harness had already measured exactly this — Q6's unfloored seek degrading 0.073 → 5.60 ms after 40,000 head reschedules, 77×, same engine and corpus, churn alone — and I filed it as "the claim floor is justified" while missing that it also said every other number on that page decays with churn.

That correction is in the amended commit (5dc4c2f) and the PR body: git grep -i "b+tree" on this branch returns nothing.

It matters beyond pedantry in two ways. "Wrong engine" implies the bench is structurally unusable for this repo, which is false and would waste a good harness. And it points at the wrong fix: the honest framing is that absolute per-row costs measured on a fresh corpus are a floor, so the harness needs to be re-run at production row count with churn — which is what bench/queue-index/README.md now leads with.

The change under review is unaffected either way: ~40% of the scan is rows read past the due boundary and discarded, regardless of what each row costs.

🤖 Addressed by Claude Code