Memory limiter proposal review - #1
Conversation
5ba5a7f to
58a2baa
Compare
dashpole
changed the title
|
Thank you for the thorough, deeply engaging critique—the code-level catches regarding We have also aligned on using post-GC live heap relative to However, after architectural evaluation, we have outright rejected two of the proposed structural mechanisms due to severe stability hazards and operational anti-patterns: 1. Rejected: Soft-Tier Byte-Budget Scrape Throttling & "Proceed Anyway on Timeout"The Proposal: Replacing equal-share algorithms (DRR) in v1 with a global byte-budget semaphore in the Soft Limit tier, where scrape loops block waiting for headroom up to Why We Rejected It:
We are keeping scrape mitigations strictly in the Hard Limit tier (skipping scrapes outright without staleness marker injection or blocking timers) and retaining weighted Deficit Round Robin (DRR) and Per-Job QoS metadata as future design goals. 2. Rejected: The
|
b491883 to
62f2fcc
Compare
|
Thanks — the two rejections are substantive and I think one of them is entirely right. Splitting this into what I concede, what I think is misdiagnosed, the gap the rejections open, and the items from the original review that don't appear to have made it into 1. Conceded"Proceed anyway on timeout" is a genuine defect, and worse than stated. Scrape loops are deliberately jittered across the interval. Parking them on a semaphore and releasing them when their timers expire de-jitters them — the mechanism manufactures synchronization that didn't previously exist. That isn't a tuning problem, it's the design creating the herd. For the record, I contradicted myself between the review and the revised doc. The review said "bounded by "Size does not equal criticality" is the strongest argument in the response and I missed it. "Restarts provide zero memory relief" is correct. WAL replay reconstructs the same head. My justification for 2. Where I think the reasoning is misdiagnosed2a. The dynamic-equilibrium model is incompatible with the signal we just adoptedThis is the one I'd press hardest. The argument is: mitigations shed load → "transient parsing memory disappears, Go's GC reclaims heap space, and pressure recedes below the Hard Limit." That holds for What is actually in live heap is the head: The paths that actually reduce it:
So for cardinality-driven growth — the case the "Why" section leads with — equilibrium arrives at the next head compaction, not within a few check intervals. Until then pressure stays above I'm not arguing for reinstating 2b. The goroutine and socket starvation argument is incorrectPrometheus already runs one goroutine per target, permanently, whether or not it is blocked ( The other two arguments carry the rejection on their own; this one shouldn't stand as a recorded reason, because it would also rule out mechanisms that are fine. 2c. "Size ≠ criticality" argues against DRR as well, not only against the byte budgetEqual-quantum DRR starves So the conclusion isn't "keep DRR, drop the byte budget." It's that any size-based scheme is on the wrong axis, and explicit priority is the only thing that answers the criticality problem. That promotes Per-Job QoS from "future design goal" to "the mechanism that actually addresses this," and it means the Fairness section's current DRR endorsement should be softened rather than retained (see §4.6). 3. The gap the rejections openKeeping "stay running" and dropping the shutdown actions is right. But taken together with §2a it leaves the design with no mitigation that reduces live heap — only mitigations that stop it growing. Without one, "dynamic equilibrium" is asserted rather than achieved. The obvious candidate is forced early head compaction: One honest counterweight to my own P0-3. Removing staleness markers from the skip path has a cost I didn't flag. 4. Items from the original review still unaddressed upstreamThese are orthogonal to both rejections — they aren't accepted or rejected, they just don't appear in 4.1 4.2 No hysteresis, against a signal with minutes of dead time. Per §2a, 4.3 Adjacent Non-Goals contradict each other. "It is designed to handle spikes and overload scenarios" is immediately followed by "Extremely rapid intra-GC allocation spikes … are out of scope." A coherent reframe is available: transient spikes are 4.4 Two now-false statements about the OpenTelemetry Collector. Goals still says "unified, top-level global configuration similar to the OpenTelemetry Collector's memory limiter," and the Configuration section still opens "The configuration closely follows the OpenTelemetry Collector's memory limiter processor." Neither is true after the pivot — the block is nested under 4.5 Alternatives prometheus#4 now argues against the adopted design. It rejects "Independent GOMEMLIMIT configuration" as something "users wouldn't want," which is precisely what the proposal now does. 4.6 The Fairness section is untouched and overclaims. It still states DRR "mathematically guarantee[s] fairness … isolating the disruption to high-cardinality targets." The PoC numbers don't support that: DRR admitted ~2x more from the massive target than probabilistic did (2.9M vs 1.45M), the small target's share improved ~1.6x rather than the headline 3x, and DRR ran ~7% hotter on peak RSS. What was measured is higher total throughput under the same limit — a real result, but a throughput result. With the byte budget rejected this is now the only stated fairness plan, so the claim carries more weight, not less. 4.7 No "How we test and verify" section. The template asks for it and @SuperQ asked for it directly ("all of it is going to need to be tested for various failure modes"). Now that the design turns on a single signal choice, the false-positive test is the experiment that validates or kills it: steady-state Prometheus at realistic utilization with the limiter enabled at defaults, asserting it never leaves 4.8 The soft tier is nearly empty for the motivating case. For a pure-scraping Prometheus, soft = pause block compaction, and that's all; then everything drops at once at 0.85. Moving recording rules to hard was correct, but it thinned the tier, and with the byte budget rejected there's now no graceful step for the exact scenario "Why" leads with. Worth naming explicitly rather than leaving implicit. 4.9 Minor. Ranked by what I'd act on first: 2a and 4.1 are the two I'd treat as defects. 4.4/4.5/4.6 are editing debt from the pivot that will read as oversights to an upstream reviewer. 3, 4.2 and 4.7 are where I'd argue but won't insist. Everything in §1 I think should be written into the doc — the rejection arguments are good enough that the next reviewer shouldn't have to rediscover them. Generated by Claude Code |
Correcting my §2a — I measured it, and the argument I made there was wrongI claimed above that skipping scrapes doesn't move I ran it against a real Run 1 — 150k → 300k series:
Run 2 — robustness check. In run 1 each series had only 30 samples, so no head chunk reached the 120-sample cutoff and 1. The claim holds, more strongly than I stated itI said live heap "plateaus rather than falls," and guessed the 2. But the equilibrium model is correct — about the processPeak 3. The actual defect is a sensor/actuator mismatchThe mitigation relieves the quantity 4. The two signals fail in complementary ways, which is where I'd build the fixThis is the part I think is most useful for implementation. Look at phase A — a healthy 150k-series head: live heap 149 MiB, peak in-use 435 MiB. A 2.9x gap. That sawtooth is precisely why live heap is the right choice for engaging, and re-confirms the original P0-1/P0-2 finding independently. But live heap is invariant under every mitigation currently in the design, which makes it the wrong choice for releasing.
Two ways out, and I don't have a strong preference:
5. One result that cuts toward the original draftAt phase D, settled in-use rose (341 → 444 MiB in run 1, 254 → 337 in run 2) because Method and limitsSynthetic The thing I'd most want checked by someone with a real cluster: whether in-use falls as reliably under a real mixed workload (queries, rules, remote write all continuing while scrapes are skipped) as it does here with scraping as the only load. If it doesn't, the asymmetric-sensor option gets weaker. Reproduction — drop in
|
Retracting P0-2, and the signal recommendation that followed from itThird correction, and this one goes to the root. My P0-2 argument was over-generalized, the switch to live heap that followed from it was the wrong fix, and the "0.425 guard" I proposed last was worse than the problem it solved. The original transient-spike design was closer to right than what I talked us into. The measurement
An in-use threshold is a clean transient-spike detector below ~45% baseline live heap — zero false positives with 97% detection — and degrades above ~50%. P0-2 claimed no absolute threshold could work. That holds only in the upper regime. My own earlier table showed the crossover (0.0% at 31% and 41%, 42.9% at 49%) and I generalized past it. Analytically the boundary is What survives from P0-1, and what doesn't
So: keep the denominator change, keep the accounting change, revert the transient exclusion. The Non-Goal added in 62f2fcc should come back out. And the guard I proposed last time was wrongI suggested disabling the limiter above 0.425 baseline. That's backwards — a server with a large baseline is more at risk, not less. 0.425 should be a reporting fact ("above this, expect frequent engagement; your baseline is too large for your budget"), not a switch that turns the feature off. The deeper error underneath it: I treated false-positive rate as a property of the signal. It's a property of the (signal, mitigation) pair. A false positive costing "delay this scrape 100 ms" is nearly free; one costing "drop the scrape, emit Force the GC and use it as a discriminator@dashpole's framing — we aren't trying to lean on the GC to save us, we're trying to save ourselves — is the resolution. Rather than treating the sawtooth as sacrosanct:
This dissolves the sawtooth problem rather than designing around it, and makes the FP rate close to irrelevant in the regime this proposal targets. Caveat worth writing down: forced-GC cost scales with the live set while its benefit shrinks as live heap grows (less of in-use is garbage). So it works best exactly where it's aimed and worst on a very large head — argues for a minimum interval plus skipping it when live heap is already high. The OTel Collector's memory limiter has a deserved reputation for CPU burn when mis-tuned, and this is the mechanism. Two things from the kubelet eviction manager that fix earlier mistakes of mineGrace periods. Soft eviction waits out a grace period, then acts. That is the correct form of the byte-budget mechanism — delay with a deadline, then drop, never "proceed anyway." The rejection in #1 above was right, and this is the repair. Ranked, targeted eviction. Kubelet ranks and evicts the worst offender rather than acting globally. The analog: reject the specific scrape that won't fit, not all of them. There's a clean checkpoint — Scope, honestly stated"It's virtually impossible to prevent OOM if you scrape a target large enough" is correct, and the proposal should say so rather than imply a guarantee. This is best-effort load shedding.
Where this leaves the two proposals
Not yet validatedThe forced-GC-as-discriminator tier is a design argument, not a measurement — I have not measured GC cost against live-set size, or confirmed that a forced GC reliably clears a sawtooth-induced trip. That's the next thing worth running, and it's the load-bearing claim for the tier structure above. The 0.425 boundary is also from a synthetic allocator; the mechanism is Go GC dynamics so I expect it to transfer, but it should be confirmed against a real server before it appears as a documented number. Generated by Claude Code |
Measurements for the forced-GC tier, and a concrete edit list for the proposalI said the forced-GC tier was a design argument rather than a measurement. I've now measured it. One result overturns what I posted last time, and one turns up a cheaper lever that I think should be the first mitigation rather than an afterthought. E1 — Cost of forcing a GC, vs live-set sizePointer-rich live set (strings + pointers + slices), since mark cost scales with the object graph rather than bytes — flat
Linear at ~0.21 ms per MiB of live heap. STW pause is negligible (Go's GC is concurrent) but wall/CPU cost is not. Consequence: reclamation cannot run at the check interval. At 433 MiB live one forced GC takes 98 ms — an entire 100 ms tick. The check interval and the reclamation interval must be separate knobs, and the reclamation interval should be self-tuning at roughly 100× the last reclaim's measured duration, which lands at ~10 s for a small head and ~200 s for an 8 GiB one without anyone having to configure it. E2 — Lowering GOGC is a much cheaper lever, and should come firstBaseline live heap 321 MiB (31% of a 1 GiB
Dropping GOGC 100 → 50 cuts peak in-use by 115 MiB (19%) for 2.2 percentage points of GC CPU. It is continuous, instantly reversible, loses no data, and touches no scrape. Against that, dropping a scrape to reclaim a comparable amount is a terrible trade. (Methodology note, since it matters: my first attempt at this table allocated in a tight loop, which saturated Go's own GC CPU limiter at its 50% cap and let the heap balloon to 5.7 GiB — 560% of E3 — The discriminator works. But
|
| tier | action | cost | reversible |
|---|---|---|---|
| T1 | lower GOGC (ceiling = configured runtime.gogc, restore on release) |
+2.2pp GC CPU for 19% headroom | instantly |
| T2 | debug.FreeOSMemory(), rate-limited to ~100× last duration; doubles as the discriminator |
0.21 ms/MiB live | n/a |
| T3 | per-scrape admission: Content-Length vs headroom at the readResponse boundary (scrape/scrape.go:751-773), reject only the scrape that won't fit |
O(1) | n/a |
| T4 | shed: pause block compaction, 503 remote read / federation / OTLP / remote write, skip scrapes | data loss | no |
T1 and T2 are new and non-destructive. T3 is targeted rather than global. T4 is the existing content, demoted to last resort.
5. Skip T1/T2 when live heap is already high. This is where the 0.425 number finally has a proper home — as a tier selector, not the global guard I wrongly proposed. Above threshold / (1 + GOGC/100), lowering GOGC buys nothing (GOMEMLIMIT is already binding) and reclamation finds nothing (E3's live-heap row), so go straight to T3/T4 and report baseline saturation.
6. Non-Goals. Remove the intra-GC-spike exclusion added in 62f2fcc — it was added on my bad advice and it removed the proposal's original purpose. Add instead: this is best-effort, not a guarantee; body_size_limit bounds the per-target worst case while the limiter handles aggregate and concurrent pressure; and small servers get less protection (at 15% baseline live heap a 225 MiB spike never reaches the threshold — correct behaviour, but worth stating so nobody finds it in the field).
7. GOMEMLIMIT as input. Keep exactly as written in 545c034. That part was right.
8. Metrics. prometheus_memory_limiter_pressure_ratio (in-use based), ..._live_heap_ratio (the baseline/capacity signal), ..._state, ..._state_seconds_total{state}, ..._reclaim_total and ..._reclaim_bytes_total (so the discriminator's verdict is visible), ..._gogc_current. Document alerting on live_heap_ratio as the capacity signal — it's the number that says "provision more memory," and it's distinct from the transient pressure signal.
9. Fairness section. Soften the DRR claim per the earlier comment. Note that T3 spares small targets structurally — a 5-series target's Content-Length always fits — which addresses the kube-state-metrics criticality problem without needing a scheduler.
10. Split. Baseline/structural saturation moves to its own proposal: signal live_heap / GOMEMLIMIT, mitigations CompactSelectedSeries head eviction plus churn limiting (prometheus/prometheus#17109).
Still outstanding from earlier and unaffected by any of this: hysteresis (4.2), the two now-false OpenTelemetry Collector sentences (4.4), Alternatives prometheus#4 contradicting the design (4.5), no test/verification section (4.7), and incurr → incur (4.9).
Limits
All of E1–E3 use synthetic allocators, not a running Prometheus. The mechanisms are Go runtime behaviour so I expect them to transfer, but the specific numbers — 0.21 ms/MiB, the 2.2pp CPU cost of GOGC 50, the 0.425 boundary — should be confirmed on a real server before any of them appears in the doc as a default. Single runs, no variance analysis. GC cost in particular will vary with GOMAXPROCS and with how pointer-dense the real head is compared to my stand-in.
The one thing still unmeasured that I'd want before implementation: whether T3's Content-Length check is actually predictive of a scrape's peak memory, given decompression and parse amplification. That needs a real scrape path, not a synthetic harness.
Generated by Claude Code
|
We appreciate the rigorous experimental testing (E1–E3) and scientific integrity shown in retracting P0-2 and moving back to an in-use total memory control signal! We have updated the proposal to adopt In-Use Total Memory Ratio ( However, in accordance with our guiding design principle—keeping the proposal simple, predictable, and easy for operators to reason about—we have evaluated and rejected the proposed automated runtime manipulations (Tiers 1, 2, and 3) in favor of our simple Soft/Hard load-shedding circuit breaker for Milestone 1: 1. Rejected: Tier 2 (Forced OS Page Scavenging via
|
Review of e4fe08b / 9499d43 / d5a166e — one load-bearing claim doesn't holdRead the three new commits. Most of it landed cleanly. One sentence in the How section is now carrying the whole design and I measured it, because it's exactly the kind of assumption that was wrong twice already. E4 — "In-use memory responds immediately when load is shed"Recovery of in-use after ~400 MiB of garbage becomes unreachable. Sampled at the proposal's own 100 ms
Passively, in-use does not recover at all — not slowly, not partially. Flat for ten seconds.
(This also reconciles the apparent conflict between E3 and the earlier equilibrium test — that test's Consequence
Two edits:
Worth stating explicitly in the doc that Two smaller defectsThe metric-availability claim is not quite right. The doc says operators can monitor pressure "via existing Go runtime metrics already exposed by Dangling references from d5a166e. The test section cites Dropping What landed wellThe signal switch is done cleanly, and the "post-GC live heap is invariant under load shedding" rationale is stated correctly rather than just asserted. The Non-Goal rewrite is better than what I proposed — naming #17109 and selective head eviction makes the split concrete instead of gesturing at it. Still openHysteresis — now with a stronger basis than when I first raised it. A 100 ms check interval against a signal that only recovers when reclamation is forced means real dead time in the loop; without a minimum engagement duration and an asymmetric release threshold, the limiter will over-shed while waiting for its own signal to catch up. Where 45% comes from. Test #1 requires "<45% baseline live heap" but the number is unexplained. It is Lowering GOGC as the first mitigation. Still absent, and it's the one I'd push hardest on: 19% headroom for 2.2 percentage points of GC CPU (E2), instantly reversible, no data lost, no scrape touched. It is strictly cheaper than every mitigation currently in the soft tier. Treat the configured Per-scrape LimitsE4 uses a synthetic allocator, single run, Generated by Claude Code |
Correcting my recovery claims — twice over, with a predictive model this time@dashpole pushed back on two things I asserted: that E5 — Does proximity to
|
| GOMEMLIMIT | trip as % of GML | GCs in 90s | min in-use | release below 70% |
|---|---|---|---|---|
| 900 MiB | 92% | 4 | 389 MiB | 35.6 s |
| 880 MiB | 94% | 3 | 413 MiB | 36.0 s |
| 865 MiB | 96% | 3 | 413 MiB | 34.0 s |
| 838 MiB | 99% | 4 | 427 MiB | 36.3 s |
Flat. Recovery is ~34–36 s regardless of whether the trip point sits at 92% or 99% of GOMEMLIMIT.
An earlier 30 s-window run of the same setup reported "NEVER" for 85% and 92%. That was an observation-window artifact — recovery takes ~35 s and my window was 30 s. My "never recovers" claim was wrong, and I should have extended the window before asserting it.
E6 — What actually governs recovery
Go triggers GC when heap objects reach the GOGC goal — live × (1 + GOGC/100), so live × 2 at GOGC=100. GOMEMLIMIT only lowers that goal when live × 2 would exceed it, i.e. when live heap is more than ~half of GOMEMLIMIT. In E5, live was ~215 MiB against an 838–976 MiB limit (~25%), so 2 × 215 = 430 never came close and GOMEMLIMIT was simply not in the loop. That is why proximity changed nothing.
Dropping a burst returns heap objects to live, so the next GC needs another live bytes of fresh allocation:
recovery_time ≈ live_heap / background_allocation_rate
Tested on both axes independently (GOMEMLIMIT 1400 MiB, GOGC=100):
| live heap | bg rate | predicted | measured | ratio |
|---|---|---|---|---|
| 200 MiB | 5 MiB/s | 40.0 s | 30.7 s | 0.77 |
| 200 MiB | 10 MiB/s | 20.0 s | 29.2 s | 1.46 |
| 200 MiB | 20 MiB/s | 10.0 s | 11.9 s | 1.19 |
| 200 MiB | 40 MiB/s | 5.0 s | 8.3 s | 1.66 |
| 100 MiB | 20 MiB/s | 5.0 s | 5.2 s | 1.04 |
| 200 MiB | 20 MiB/s | 10.0 s | 7.9 s | 0.79 |
| 399 MiB | 20 MiB/s | 20.0 s | 22.3 s | 1.12 |
Holds within roughly ±50%. The live-heap axis is the cleaner fit (1.04 / 0.79 / 1.12); the rate axis is noisier because my pacing loop can't hold an exact rate at 40 MiB/s.
What this means for the proposal
Setting hard_limit_ratio just below GOMEMLIMIT does not improve recovery in the regime the proposal targets. It would only help once live heap exceeds ~50% of GOMEMLIMIT — which is the baseline-saturation regime already split out into a separate proposal. Measured flat across 92→99%.
debug.FreeOSMemory() is not a correctness requirement. I overstated that. Memory does come back on its own.
The real issue is how recovery scales. Recovery time is linear in live heap. Extrapolating from the validated model, a 4 GiB head with 20 MiB/s of residual non-scrape allocation is ~200 s of continued shedding after the burst ends; a 10 GiB head, ~500 s. The servers that most need the limiter are the ones that recover slowest, and nothing in the current design bounds that.
Recommendation, unchanged in shape but on better grounds: let Go recover naturally — that is the normal path and it works — but bound the wait. If still engaged after a fixed timeout (30 s covers small heads with margin), call debug.FreeOSMemory() once, rate-limited per E1 (~0.21 ms per MiB of live heap). If you'd rather have it self-tune, the limiter can compute the expected recovery itself: live heap from /gc/heap/live:bytes, allocation rate from /gc/heap/allocs:bytes, wait ~2× the quotient.
This also settles the hysteresis question quantitatively: with natural release latency of live_heap / alloc_rate — tens of seconds at realistic sizes — a 100 ms check_interval needs a minimum engagement duration on that order, or it will flap and over-shed while waiting for its own signal.
Limits, and a note on my own error rate here
Synthetic allocators throughout; no real Prometheus, no queries or rules, and "background allocation" is a pacing loop rather than real work. The model is derived from Go's documented GC trigger and validated over a 4× range of live heap and 8× of allocation rate — I'd trust its shape and use it for sizing timeouts, not its constants.
I've now been wrong twice on this specific question in opposite directions — first "correctness requirement," then "never recovers, proximity doesn't help." Both errors came from concluding on a single configuration with too short an observation window. The model above is the version I'd stand behind, because it predicts rather than describes and it was tested on both axes independently. It should still be confirmed on a real server before anything derived from it becomes a default.
Generated by Claude Code
Retracting the dynamic-GOGC recommendation, and a release-possibility result that replaces it@dashpole pushed back on the limiter adjusting GOGC at runtime: operators may well want the CPU-for-headroom trade, but the limiter shouldn't make it for them, and absent strong evidence, garbage collection should be left to the runtime — which already exposes knobs with predictable behaviour that users know. He's right, and my own data supports his position better than it supported mine. Retracting it. Where my evidence actually wasE2 compared GOGC values held fixed for the duration of each run — 100 / 75 / 50 / 25, each a separate configuration. That is evidence that a lower GOGC setting trades CPU for headroom. It is not evidence that changing GOGC at runtime is safe or effective, and I presented it as though it were. Worse, the one time I did test dynamic adjustment, it failed and I reported the failure and then kept recommending the lever anyway. From the release experiment, after tripping the hard limit and shedding load: Dynamic GOGC did exactly nothing. Three concrete ways it would make the wrong callEach backed by a measurement already in this thread:
E7 — Release possibility, and why GOGC belongs as an inputWhile testing whether recovery gets faster near Release is not gated on the GC firing. It is gated on whether the post-GC heap goal leaves any slack below the release threshold for the scavenger to give back. If the goal sits above the release threshold, there is nothing to return, ever — the limiter engages and never lets go. Verified across three GOGC values, 19 of 20 predictions correct:
The single miss was marginal (GOGC=200, goal 1064 vs release 980 — 9% past the line, released anyway in 28.8 s), so treat the boundary as soft within ~10% rather than a cliff. GOGC appears in that formula as an input. Which is exactly the right relationship, and generalises the principle the proposal already established for
The limiter reads What replaces the recommendationOperator documentation, not automation — expressed in knobs users already understand:
Stated in advance, predictable, chosen deliberately — rather than the limiter silently making the trade mid-incident. Two consequences for the doc:
Net effect: the proposal gets smaller. No dynamic GOGC tier, no forced-reclamation tier, no recovery timeout — those were three separate recommendations of mine across as many comments, and all three are withdrawn. What remains is one inequality the limiter evaluates from numbers the runtime already publishes, plus documentation of the regime it implies. LimitsSynthetic allocators throughout, single runs, one Generated by Claude Code |
Generated by Claude Code |
Test design and success metrics for the memory limiterWritten against Two things shaped it:
1. What is actually being claimedThe proposal asserts a causal chain. Each arrow is a separate assumption, and each can fail Five claims. C3 is the one I'd test first, because it is the one my own measurements say is The specific reason C3 is at riskFrom E7, release is possible only when the post-GC live heap plus one GC cycle's worth of At stock defaults ( Now put that next to the engagement condition, That is a sharp, falsifiable prediction, and it cuts at the motivation: "Why" lists "spikes I want to be careful here: E7 was measured on a synthetic allocator, not Prometheus, and one 2. Prototype requirements (testability contract)The tests below cannot run against the metrics
Also needed for scenario control, not shipped: a hidden/undocumented flag or test-only hook to 3. Harness tiersOrdered cheapest-first deliberately. Each tier gates the next; do not book GKE time until Tier 0 — Go tests, seconds, no infrastructureRuns in CI on every commit. Covers the mechanical claims, not the emergent ones.
That last one deserves emphasis: making the controller read its input through an interface Tier 1 — single container, real binary, real cgroup — ~20 min per pointThis is where the design lives or dies, and almost nobody builds this tier. One Prometheus Avalanche has exactly the knobs these scenarios need (verified in
The critical property: cardinality is set by a number, not by scheduling pods. That makes Tier 2 — prombench — hours, GKEFor the things that need scale and realism: steady-state regression, real Kubernetes SD churn, 4. ScenariosTwo families, and the distinction between them is the whole experiment:
E-E deserves its own paragraphGo's That is not a bug — bounding anonymous memory and letting the kernel reclaim page cache is the 5. Success metricsSplit into three groups. Guardrails are not tiebreakers — a guardrail failure is a failure. Primary
That last row is not a formality. A burst that doesn't reliably kill baseline Prometheus Net-benefit (C5) — the comparison the proposal has not yet framedThis needs stating plainly, because the honest baseline is better than the proposal implies: So the trade is total-outage-for-D_oom versus partial-outage-for-D_mitigation, and it is not
The three-arm setup (baseline / limiter / control) is what makes The feature earns its complexity if it strictly dominates baseline on Guardrails
Every Tier-2 delta gets compared against E-H, never against zero. Prombench is noisy enough 6. What prombench needs (it cannot run this today)Concrete blockers, from the manifests at
The good news: none of this requires changing upstream test-infra. And the burst mechanism already exists — 7. Kill criteriaWhat would make me say the design needs rework rather than a fix:
None of these are predictions except the first, which is a prediction with evidence behind it. 8. SequencingE-A and E-B are the gate. They are a day of work on one machine, they need no cluster, and 9. Doc consequences regardless of outcomeFour things in "How We Test and Verify" as written:
Limits of this plan
Generated by Claude Code |
1 participant
Review: prometheus/proposals 76 — Memory Limiter
Reviewed against
prometheus/prometheus@40ea54d0b(v3.13.0) and both PoC branches ondashpole/prometheus(memory_limiter_simple,memory_limiter_ai_poc).Verdict
The problem is real and well-motivated, the Alternatives section is unusually good, and the
two-persona Debuggability section is better than most Prometheus proposals get. I'd support
the direction.
But I don't think the proposal is ready to be approved as a design, for one structural reason
and five specific ones. The structural reason: the proposal never says what it measures.
"Periodically a background routine checks the current memory usage of the Prometheus process"
is the entire specification of the control input, and every hard question in this design is
downstream of that sentence. When I fill in the blank the way the PoC does, the feature is a
no-op in containers and a false-positive generator everywhere else. When I fill it in the way
that would actually work, several of the proposed mitigations stop making sense.
Then, separately: two of the five mitigations do roughly the opposite of what the proposal
says they do, and one of the three "existing metric covers this" claims is wrong.
What's good, and I mean it
with where two of the mitigations landed on it.
the strongest paragraph in the document. Worth strengthening with the observation that
sample_limitalready establishes whole-scrape failure as the Prometheus-native responseto an over-large scrape (
scrape/scrape.go:650), so you're following precedent, notinventing it.
into alerts and recording rules" point is the right reason.
scrape cost is sharp and correct, and it's the kind of thing that usually gets discovered
two implementations later.
P0-1. The control input is unspecified, and every obvious choice is wrong
Three separate questions are collapsed into "current memory usage":
memory.current?GOMEMLIMIT?GOGC.The PoC answers:
runtime.ReadMemStats(&m).Alloccompared againstgithub.com/pbnjay/memory'sTotalMemory()(scrape/memory_limiter.go:82-101). Every oneof those three answers is wrong, and each is independently fatal:
(a)
pbnjay/memory.TotalMemory()is host RAM, not the cgroup limitIt is
syscall.Sysinfo().Totalram * Unit(
pbnjay/memory@v0.0.0-.../memory_linux.go) — completely cgroup-unaware. On a 128 GiB nodewith an 8 GiB pod limit,
limit_percentage: 90resolves to ~115 GiB. The limiter neverengages, and Prometheus OOMs exactly as it does today — in the deployment that motivates
the entire proposal.
The fix is one line, and it's already in the tree:
cmd/prometheus/main.go:811usesmemlimit.ApplyFallback(memlimit.FromCgroup, memlimit.FromSystem). The proposal should say"percentage of the same budget
--auto-gomemlimitdetects" and name that provider, becauseotherwise this bug gets reimplemented. Also:
pbnjay/memoryis currently an indirectdependency (
go.mod:232); promoting it to direct for something automemlimit already providesis a needless dependency argument to have in review.
(b)
MemStats.Allocis instantaneous heap including uncollected garbageUnder default
GOGC=100the Go heap deliberately oscillates to ~2x live heap. Measured witha synthetic ~205 MiB live heap and scrape-shaped allocation churn:
So a threshold on
Allocfires whenever the threshold sits below ~2.4x live heap. Restated:the limiter engages permanently once live heap exceeds ~40% of the configured limit. This
is the OTel Collector
memory_limiter's best-known failure mode, and inheriting its configshape has quietly inherited its bug.
(c) Heap-only usage compared against a total-memory percentage
Alloccounts heap objects only. It excludes goroutine stacks, mspan/mcache, GC metadata(collectively 10-20% of heap for a big Prometheus), and it excludes mmap'd head chunks and
block files entirely. Comparing that subset against a percentage of total memory
systematically under-reads. The proposal should state which of these two problems it is
solving, because they need different instruments:
memory.currentoverstates: Prometheus's mmap'd head chunks and block files arereclaimable file cache, and the kernel will drop them under pressure rather than OOM. A
limiter reading
memory.currentwill engage constantly on an idle, healthy Prometheus.bounding Prometheus's anonymous memory and letting the kernel reclaim page cache is the
actually-correct policy.
Concrete recommendation
Measure live heap after GC via
runtime/metrics—/gc/heap/live:bytes, confirmedpresent in Go 1.24.7 — not
MemStats.Alloc. In the same run as above, at 503 MiB live:runtime/metrics.Readalso avoidsReadMemStats's stop-the-world. Measured cost, roughlyflat across 122 MiB → 2 GiB live heap:
To be fair: ~60 µs once per second is not itself a problem, and I don't want to overstate it.
The real cost in the PoC is architectural — see P1-9.
Worth also putting in the doc as a candidate signal, even if rejected:
/gc/limiter/last-enabled:gc-cycle. That's the Go runtime telling you its own GC CPUlimiter engaged, i.e. GC is burning >50% of CPU — an unambiguous, self-calibrating "I am
dying" signal that needs no tuning at all. Similarly cgroup PSI (
memory.pressure) measuresthe thing you actually care about (imminent reclaim failure) across heap and non-heap alike.
If you reject these, say why; right now the doc reads as though absolute byte thresholds were
the only option considered.
P0-2. The thresholds are denominated in the budget
GOMEMLIMITis designed to consumeThis is the deeper version of P0-1(b), and I think it invalidates the config shape rather
than just the PoC.
Prometheus sets
GOMEMLIMIT = 0.9 × cgroup-or-system limitby default(
--auto-gomemlimit,--auto-gomemlimit.ratio=0.9,cmd/prometheus/main.go:429-432,:806-816). Go then treats that entire budget as fair game for garbage: it will letmemory-in-use climb toward
GOMEMLIMITand only then GC harder. Healthy operation lookslike "memory near GOMEMLIMIT" by design.
Simulating a 1 GiB container with the proposal's defaults (
limit_percentage: 90,spike_limit_percentage: 20→ hard 921 MiB, soft 716 MiB), varying only live heap:Alloc(Synthetic allocator, not Prometheus — illustrative of the mechanism, not a Prometheus
measurement.)
Read the two ends of that table together:
~43% of checks. Compaction and recording rules would be paused roughly half the time,
permanently, on a server in no danger whatsoever.
GOMEMLIMITsits at the same90% and Go won't let heap objects get there.
There is no value of
limit_percentagethat fixes this. BelowGOMEMLIMITyou get constantfalse positives; at or above it you get a feature that never engages. The knob is
denominated in the wrong quantity.
This also makes the
GOMEMLIMITsection (line 108-112) a regression rather than arefinement.
GOMEMLIMIT = 0.9 × soft = 0.9 × 0.7 = 63%of the container, versus 90% today.Anyone who sets
--enable-feature=memory-limiterwith the documented defaults hands 27% oftheir memory budget back to the GC, and Go spends it on GC CPU. That's a large, silent
performance regression attached to a flag whose stated purpose is stability.
nicolastakashi's review comment asked precisely the right question and I don't think thecurrent answer resolves it. The answer I'd want: express the limits relative to
GOMEMLIMITand in live-heap terms, becauselive_heap / GOMEMLIMIT→ 1 is the actualdeath condition (past it, Go cannot honor the limit no matter how hard it collects), and it's
scale-free. Something like "engage when live heap exceeds 75% of
GOMEMLIMIT" is meaningful;"engage when memory exceeds 70% of the container" is not.
P0-3. "Fail scrapes" costs as many appends as the scrape it replaced
A failed scrape is not a cheap scrape.
scrapeAndReporttreats it as an empty scrape andcalls
sl.append([]byte{}, ...)specifically to emit staleness markers(
scrape/scrape.go:1438-1440, and:1379-1395for the forced-error path the PoC uses), whichwalks
seriesPrevand appends oneStaleNaNper series(
scrape/scrape.go:1563-1579,:1029-1037).I wrote a test against the real scrape loop (
scrape/memlimit_review_test.go, attached):So on the first skipped cycle you avoid the HTTP body and the parse, and pay the full
append path anyway — head chunk writes and WAL records, one per series. And because the
PoC's
isOverLimitis a single global boolean, every target skips in the same checkinterval, so you get a synchronized
Σ(series)staleness stampede across the whole server atthe exact moment you are closest to the OOM. The mitigation's first act is to make the spike
worse.
Note also
total=0 added=0 seriesAdded=0: the reported scrape metrics say nothing happened,so
prometheus_target_scrapes_skipped_totalwon't reflect this cost either.This is fixable and the fix is small, but it's a design decision the proposal has to make
explicitly, because it changes the user-visible contract:
A memory-limited skip must not be modelled as a failed scrape. Skip the
app.append([]byte{}, ...)call and don't advance the cache —sl.cache.iterDoneis onlyreached through
append, so "this scrape did not happen" falls out naturally, and you alsosidestep the forced cache-flush heuristic at
scrape/scrape.go:955. Still callsl.report(...), which is a separate deferred call (scrape/scrape.go:1373-1377) — that's~6 samples per target, so you keep
up = 0and the/targetserror and lose theper-series cost.
The tension you need to name: without staleness markers, the target's series carry forward
under the 5m lookback, so the data reads as continuous-but-stale rather than absent. I think
that is the better semantic for a skipped scrape — the data is delayed, not gone, which is
exactly the framing Alternative 3 uses — but it does soften the "
up = 0sends a clearsignal" argument you make against slowing scrapes down. Pick one and say which.
P0-4. "Pause Compaction" as written is a memory amplifier
Line 68 says "Pause background TSDB compaction." The only existing mechanism is
DB.DisableCompactions()(tsdb/db.go:2515-2521), which setsautoCompact = falseandshort-circuits the whole
db.Compact(ctx)call (tsdb/db.go:1318-1329). That call is:tsdb/db.go:1539-1585):1531-1534,:1589-1591):1602-1607):1609→compactBlocks)(1) is what frees head memory. (2) is what keeps the WAL bounded — and an unbounded WAL
makes the next startup replay worse, i.e. it feeds the OOM crashloop this proposal exists to
break. Pausing "compaction" via the existing knob therefore increases memory monotonically
and degrades the recovery path.
Only (4) is a memory consumer worth pausing, and the code already prioritises head
compaction over it (
tsdb/db.go:2000-2003: "aborting block compactions to persist the headblock"). There's also existing precedent for scoping a pause to block compaction only —
BlockCompactionExcludeFunc/--storage.tsdb.delay-compaction-file(
tsdb/db.go:260-262,cmd/prometheus/main.go:817-821).The proposal must say "pause on-disk block compaction only; never head compaction, OOO head
compaction, or WAL truncation." As written, an implementer reaching for the obvious API
ships a regression.
This also sits uneasily with your own dev-summit note in the thread — "memory usage is
cyclic, and tends to be highest right before compaction" — which says compaction is the thing
that relieves pressure. And with Complementary Idea 4, where "Early Compaction" is
dismissed. Under memory pressure the right move on the head is plausibly to compact
earlier, not later. The doc currently gestures at both and commits to neither.
While you're there:
prometheus_tsdb_compactions_skipped_totalalready exists(
tsdb/db.go:442-445, "Total number of skipped compactions due to disabled autocompaction") and is exactly the counter you'd want. The proposed
prometheus_tsdb_compaction_pending_blocksis implementable (db.compactor.Plan(db.dir)returns the plan) but it's a second metric where you may only need the existing one plus a
_pausedgauge.P0-5. "Pause Recording Rules" is destructive, and the cited metric will not move
Two problems.
It's data loss, not delay. A missed recording-rule evaluation is a permanent gap in a
derived series. There is no backfill.
SuperQ's "you can, in theory, resume without loss"is true of the evaluator — it resumes cleanly — but not of the data. The proposal
inherited the premise without re-deriving it. By the doc's own soft/hard definition ("soft =
non-destructive"), this belongs under hard.
It's worse than a gap, because alerting keeps running. Line 69 keeps alerting rules
evaluating. Many alerting rules read recording-rule output. Prometheus already models this
(
buildDependencyMap/isIndependent,rules/group.go:1079-1140). So during a pause:first ~5 minutes, alerts evaluate against carried-forward stale values and look fine; after
lookback expires, expressions go empty and alerts silently resolve. Silently resolving
alerts during a memory incident is a worse outcome than the OOM. If you keep this mitigation,
either pause whole groups or refuse to pause recording rules that have dependents.
The observability claim is wrong. Line 134 offers
prometheus_rule_group_iterations_missed_totalas existing coverage. That counter only increments when the tick loop falls behind —
missed := (time.Since(evalTimestamp) / g.interval) - 1(rules/group.go:265-269,:286-291). If you pause by making evaluation a no-op, the ticker keeps perfect time,missedstays 0, and the counter never moves. You need a newprometheus_rule_group_iterations_skipped_total(or a_pausedgauge).evalIterationFunc(
rules/group.go:73-95) is the natural, low-invasiveness hook.P0-6. No exit strategy, so the failure mode is a permanent silent brownout
Non-Goals says this handles spikes, not sustained growth. But "Why" lists cardinality growth
as a trigger, and sustained growth is what actually OOMs Prometheus in production. What
happens when live heap genuinely exceeds the hard limit and stays there?
Dropping scrapes doesn't shrink the head. Pausing compaction grows it. So the steady state is:
every scrape dropped forever,
up = 0everywhere, all alerts firing (or worse, resolving —see P0-5), and nothing self-heals. An OOM kill at least restarts the process and is
loudly visible to every orchestrator; a brownout is invisible to Kubernetes, invisible to
liveness probes, and has no recovery path.
The proposal needs to say what happens here. Options worth writing down: a bounded time in
the over-hard-limit state after which mitigations release and Prometheus takes the OOM; a
prometheus_memory_limiter_saturatedsignal designed for meta-monitoring alerts; a cleanos.Exit(strictly better than SIGKILL, since the WAL is flushed and replay is cheaper); orforced early head compaction as the last resort. Any of these is fine. Silence is not — the
current draft's implicit answer is "brownout forever," and a reviewer should be able to see
that you chose it deliberately.
Related and unaddressed: hysteresis. With
check_interval: 1sand a bare thresholdcomparison, the limiter flaps at 1 Hz around the boundary, and the PoC makes it worse by
caching
isOverLimitfor a full interval (scrape/memory_limiter.go:73-76) so decisions areup to 1s stale. Specify a minimum engagement duration and a distinct release threshold.
P1. Significant gaps
7. Two ingestion paths are missing.
--web.enable-remote-write-receiver(
cmd/prometheus/main.go:468) has the same memory profile as OTLP and is absent from everylist in the doc. Federation (
web/federate.go:55) is a classic Prometheus OOM — itmaterializes a large series set — and is also absent. If they're deliberately out of scope,
say so; right now the "comprehensive coverage" claim has holes.
8. The PoC's own data contradicts two things the proposal says. From
scrape_memory_limiter_results.md/_paper.md:upThe paper concludes DRR "isolat[es] the disruption to high-cardinality targets," and line
149 carries that into the proposal. The numbers don't show that. DRR admitted 2x more
from the massive target than probabilistic did. The baseline:massive ratio only improved
1.9e-4 → 3.0e-4, i.e. ~1.6x, not the headline 3x — and DRR ran 7% hotter on peak RSS. What
the experiment measured is that DRR achieves higher total throughput under the same limit.
That's a real result, but it is a throughput result, not a fairness result, and the
proposal shouldn't cite DRR as validated for fairness on this evidence.
More importantly, 31% uptime for a 5-series target is a failed design goal, not a
qualified success. A 5-series target costs approximately nothing; there is no memory
reason to ever drop it. Any scheme that does is too coarse. Equal-quantum DRR is arguably
the wrong primitive precisely because equal shares is not what you want — you want shed
the expensive, keep the cheap.
Which suggests a cheaper and better mechanism than any of the three: a global byte budget
per check interval, with a target admitted iff
lastScrapeSize <= remaining_budget. The5-series target is admitted essentially always; the 50k-series target waits for a whole
budget. That also is the "Gradual Degradation" future enhancement (line 143-145) — you
shed increasing load as the budget tightens — so it collapses two future sections into one
simpler mechanism available in v1.
The paper recommends
limit_mibat 70-80% of the container limit (28% overshootobserved at
limit_mib: 300→ 386 MiB peak RSS). The proposal's default islimit_percentage: 90. Your own PoC says that default doesn't work. Neither the overshootnor the headroom guidance appears anywhere in the proposal, and it needs to.
The control run (
strategy: none) recorded all zeros, so there's no measurement of howmuch data the feature costs relative to a healthy server.
9. No validation plan.
SuperQexplicitly asked for this: "all of it is going to need tobe tested for various failure modes." The Action Plan has no testing item. The one I'd
insist on is a false-positive test: steady-state Prometheus at realistic utilization,
limiter configured, and confirmation that it never engages. Everything in P0-1 and P0-2 says
that's where this design breaks, and it's not a test that exists today. Also worth: a
prombench run, and a documented recovery-time measurement.
10. Config placement and the flag/config split.
runtime:already exists inprometheus.ymlfor exactly this class of knob (config/config.go:294,:733-752) andgogclives there.memory_limiteras a new top-level section (as in the PoC,ScrapeMemoryLimiteratConfiglevel) is defensible, but the doc should say why notruntime.memory_limiter. Sharper:GOMEMLIMITis set from a flag(
--auto-gomemlimit.ratio) at startup, and the proposal makes it derive from aconfig-file value that is reloadable. That's two sources of truth plus a
SetMemoryLimitcall on every SIGHUP. The
RuntimeConfigdoc comment (config/config.go:737-751) explicitlywarns about this: "Consider when the new field is first applied ... The test should also
verify behavior on reloads." Address it.
11. Silently ignoring the config block when the feature flag is absent (line 118) is
hostile — an operator sets limits, gets no error, and gets no protection. Log a warning at
minimum.
12. The limiter doesn't observe itself. The Debuggability section covers the mitigations
but not the controller. An operator debugging this needs to see what the limiter thought:
prometheus_memory_limiter_state(0/1/2),..._memory_bytes(the measured input, whateveryou choose in P0-1),
..._limit_bytes{type="soft|hard"},..._transitions_total. Withoutthe measured input exported, nobody can tell a true engagement from a false positive — which,
given P0-2, is the question they'll have.
13. Existing knobs aren't reconciled. Remote read already has
--storage.remote.read-concurrent-limit,--storage.remote.read-sample-limit, and streamedXOR chunks (
cmd/prometheus/main.go:591-597,storage/remote/read_handler.go:48). Querieshave
--query.max-samplesand--query.max-concurrency(:636-639). Rules have--rules.max-concurrent-evals(:612). The doc should say how the global limiter relates tothese — replaces, composes, or ignores.
14. Agent mode has no compaction, no rules, and no queries — only scrapes and remote
write. Does the feature apply? One sentence.
P2. PoC-specific findings (beyond P0-1)
TargetScrapeAllowed()takes a global write lock on the scrape hot path(
scrape/memory_limiter.go:67-69). Every scrape loop serializes on one mutex; with 10ktargets that's a contention point, and the unlucky caller pays the stop-the-world
ReadMemStatsinline. This should be a background goroutine ticking atcheck_intervalinto an
atomic.Bool, with scrape loops doing a single atomic load. That also makescheck_intervalmean what it says — right now the check is lazy, so with no scrapes thestate never refreshes.
limit_mibandlimit_percentageboth apply if both are set (:85-101— theLimitMiBbranch falls through to theLimitPercentagebranch), so whichever is lowerwins. The config comments read as if they're alternatives.
Validate()(
config/config.go, PoC diff) only rejects the both-zero and >100 cases. Either makethem mutually exclusive or document that the minimum applies.
No soft limit exists in the PoC at all.
spike_limit_mib/spike_limit_percentageare in the config surface but unimplemented, so the entiresoft-limit half of the proposal — the half containing both mitigations I flagged in P0-4
and P0-5 — is unvalidated by any code or experiment.
OTLP rejection must happen before the body is read.
otlpWriteHandler.ServeHTTPcalls
DecodeOTLPWriteRequest(r)as its first statement(
storage/remote/write_otlp_handler.go:167-168), which reads and unmarshals the entirerequest (
storage/remote/codec.go:974-1010) before any check could run. Rejecting afterthat point has already paid the allocation you were trying to avoid — and the OTLP decode
plus
prometheusremotewriteconversion is a multiple of the wire size. Check the limiterat handler entry, and mention
Retry-Afteralongside the 503 (line 128), since withoutit well-behaved clients will retry immediately and re-create the pressure.
On the Alternatives section
Agreed and well-argued: 1, 2, 3. 4 (independent
GOMEMLIMIT) I'd revisit — see P0-2; the"users wouldn't want a higher GOMEMLIMIT than the limiter's limit" reasoning assumes the two
are commensurable quantities, and I don't think they are.
The alternative I most want added: bound concurrent scrape work rather than dropping
scrapes. There is no global scrape concurrency or byte limit in Prometheus today (no
--scrape.max-concurrency; I checked). Transient scrape memory is roughlyconcurrent_scrapes × body_size × parse_overhead, so a global semaphore or byte budget overthe read-parse-append section bounds it directly, and delays rather than destroys —
bounded by
scrape_timeout, after which you fail as today. That is a genuinely non-destructivehard-limit mitigation, it's simpler than DRR, and per P1-8 it subsumes the gradual-degradation
and fairness sections. It deserves to be either adopted or explicitly rejected.
Also unaddressed:
SuperQproposed a churn-based heuristic(
increase(prometheus_tsdb_head_series_created_total[5m]) / prometheus_tsdb_head_series).The Fairness section answers a related question — total samples beats new series for
predicting short-term scrape cost — but that's the wrong denominator for
SuperQ's point.New series is exactly the right signal for the long-term growth that actually OOMs
Prometheus; total samples is right for the transient spike. The proposal conflates these
(Non-Goals excludes long-term growth; Why lists cardinality spikes as a motivating trigger).
Separate them explicitly and route the long-term case to Complementary Idea 3.
yeya24's two suggestions (OOO backfill for dropped scrapes;HEAD /metricsfor cardinalitypre-flight) are both interesting enough to be listed and dismissed rather than left in the
thread.
Line-level
Not started— there are two PoC branches;Partially implementedis more accurate and the template offers it.
are set (see P2-16).
sampling is more natural than a wall-clock tick, since the number only changes at GC.
user as a failed query; worth a sentence on whether that's intended.
prometheus_tsdb_compaction_pending_blocks— see P0-4; prefer reusingprometheus_tsdb_compactions_skipped_totalplus a_pausedgauge.open questions are left? (Known unknowns)" and "How you will test and verify?" — both are
currently absent, and both are where this document is weakest.
Suggested ordering if you want to unblock this fast
The scope expansion you asked reviewers about (2026-05-01) got the answer "expand," and I
think that was the wrong advice — not because the other mitigations are bad ideas, but
because the expansion added four mitigations on top of a control input that isn't specified,
and two of those four (P0-4, P0-5) are wrong in ways that only become visible once you look
at the code.
I'd land this in two stages:
denomination against
GOMEMLIMIT(P0-2), fix the skip path so it doesn't cost a stalenessburst (P0-3), add the limiter's self-observability (P1-12), and add the false-positive test
(P1-9). Ship with
fail_scrapes— or better, the byte-budget admission scheme — alone.compaction only (P0-4), rule pausing with dependency-awareness and a real metric (P0-5),
OTLP/remote-read/remote-write-receiver rejection at handler entry (P2-18, P1-7).
Everything in stage 1 is a correctness question about the controller. Everything in stage 2
is an independent policy question about one actuator. Mixing them is what makes the current
draft hard to review.
Artifacts
scrape/memlimit_review_test.go— the staleness-burst test in P0-3, runs green againstprometheus@40ea54d0bwithGOWORK=off go test ./scrape/ -run TestReview_Skipped -v.rate vs. utilization,
ReadMemStatsvs.runtime/metricscost).