Skip to content

Confirm budget crossings with a forced GC before refusing (LLP 0097) - #305

Merged
platypii merged 1 commit into
masterfrom
budget-confirm-with-gc
Jul 12, 2026
Merged

Confirm budget crossings with a forced GC before refusing (LLP 0097)#305
platypii merged 1 commit into
masterfrom
budget-confirm-with-gc

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Deploying v1.13.3 to the central server surfaced a false-refusal in the heap-growth budget: filtered COUNTs and even unfiltered MIN/MAX over the 500k-row ai_gateway_messages refuse with ~3.3GB of reported growth, while the same scans complete inside a 100MB --max-old-space-size cap. The guard reads raw heapUsed deltas, which count uncollected garbage; on a large-heap host V8 defers major GC for gigabytes, so the delta tracks allocation rate, not retention, and the guard kills exactly the streaming aggregates LLP 0055/0098 made cheap. (WHERE pushdown itself works on the server: a fully-pruned filtered COUNT answers in 2.6s.)

Fix: confirm every crossing before refusing. Force one full synchronous GC (handle acquired at runtime via v8.setFlagsFromString('--expose-gc') + vm.runInNewContext('gc'), no launch flag needed) and re-measure; only growth that survives collection refuses. A garbage-heavy but well-bounded query pays one forced GC per budget-width of garbage; a genuinely retaining query (the issue-#9 crasher class) pays one GC then refuses as before. If no GC handle is available, the guard falls back to the old raw-delta behavior.

Verified locally against the real cache: the previously false-tripping filtered COUNT and MIN now complete under 25MB/10MB budgets, and a retained ORDER BY buffer still refuses at 5MB with the typed error. The new test fails against the old guard and passes with the fix. LLP 0097 records the falsified "garbage trips are unlikely" bet and the confirm-with-gc rule.

Note: the server needs this deployed before remote filtered aggregates work; queries that read data there currently refuse at ~3.3GB regardless of selectivity.

The heap-growth guard read raw heapUsed deltas, which count uncollected
garbage as growth. On the central server (500k rows) single-column
streaming scans sampled ~3.3GB of "growth" and refused, while the same
scan completes inside a 100MB --max-old-space-size cap: the delta was
per-row scan garbage V8 had not collected yet, so the guard killed
exactly the streaming aggregates LLP 0055/0098 made cheap.
- guard.check and the interval watchdog now confirm a crossing by
forcing one full GC and re-measuring; only growth that survives
collection refuses
- the GC handle is acquired at runtime (setFlagsFromString +
vm.runInNewContext), no --expose-gc launch flag needed; if the
runtime refuses, fall back to refusing on the raw delta
- LLP 0097 updated: the "garbage trips are unlikely" bet is recorded
as falsified, with the confirm-with-gc rule under a new anchor
- test proves transient promoted garbage no longer trips (fails
against the old guard) and retained growth still refuses
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review

Clean, well-scoped change. The diff, the LLP update, and the tests hang together well. npm test passes (2159/2159), the new GC-dependent test passes, and I verified the runtime GC-handle trick independently: the handle survives the --no-expose-gc reset and gc never leaks into globalThis.

What's good

  • The runtime GC handle is the right call. Not requiring --expose-gc at launch means the fix works without relaunching the daemon; caching after first resolve and falling back to raw-delta refusal when the runtime refuses is the correct conservative degradation. Both flag flips are synchronous with no await between them, so no other context can observe the transient --expose-gc.
  • Both enforcement sites route through confirmGrowth (inline guard + watchdog) — no leftover raw-delta path remains.
  • The refusal now reports settled (retained) growth, which is more accurate than the raw delta that includes garbage.
  • Tests are honest about the trap: the MIN vs COUNT(*) comment (avoiding the numRows metadata shortcut that would skip the guard) shows the test actually exercises the stream, and the companion ORDER-BY test proves genuinely-retained memory still refuses.
  • LLP discipline is exemplary — the doc rewrites the falsified "unlikely" Consequences bullet to point at the new section rather than just appending, so the retired judgment isn't left standing as stale guidance.

Worth flagging (low severity, not blockers)

  1. Synchronous full GC on the event loop from the watchdog.HEAP_WATCH_INTERVAL_MS = 100, and while a query is over budget the watchdog forces a stop-the-world full GC every 100ms until it drops below budget or trips. On a multi-GB-heap daemon a full mark-sweep-compact can pause the event loop for a meaningful chunk, blocking concurrent request handling. It's bounded (trips → clearInterval), but the LLP's cost model ("one forced GC per budget-width of garbage") understates the watchdog's contribution.

  2. Near-budget churn can thrash GCs. The "one forced GC per budget-width of garbage" cost assumes a full budget-width of garbage accrues between crossings. But a query whose retained live memory sits just under budget (say 0.9x) and churns garbage will cross → GC → drop to 0.9x → cross again after only ~0.1x budget of new garbage → GC, repeating potentially every BUDGET_CHECK_ROW_STRIDE (4096) rows — far more GCs than the doc's model implies. Performance-only, never correctness, and such a query is a hair from refusal anyway. A small hysteresis (skip re-GC unless growth climbed by some margin since the last GC) would cap it if desired.

Neither point is a correctness problem; both are inherent tradeoffs the LLP broadly acknowledges. I'd merge as-is. Consider a one-line note in the LLP cost paragraph that the near-budget-retention case pays more than one GC per budget-width, so the next reader isn't surprised by a GC-heavy profile.

@platypii
platypii merged commit 98d2640 into masterJul 12, 2026
4 checks passed
@platypii
platypii deleted the budget-confirm-with-gc branch July 12, 2026 00:35
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@platypii