Resolve fleet-wide cap ramp stall from headroom below the service-time spread#594
Resolve fleet-wide cap ramp stall from headroom below the service-time spread#594thomasvangurp wants to merge 2 commits into
Conversation
…e spread The knee-targeting controller ramps only while target exceeds cap, with target = headroom x rate x own-best service time. Own-best is a minimum by construction so congestion cannot inflate it, but below the knee the delivered rate is cap / typical_service, making the target headroom x (min_service / typical_service) x cap. With headroom at 1.5, any workload whose fastest completion runs below two thirds of its typical one pins the target under the cap at every value: the rate is cap-bound, the cap never rises, and the only movement is unit-sized creep when the max-filter rate catches a delivery burst. Production caps froze fleet-wide in the tens on 14.12.10 with exactly this signature; the closed-loop tests missed it because their drivers used constant per-unit latency, where the spread ratio is exactly one. Headroom rises to 3.0, restoring the self-raising ramp for spreads up to 3x while keeping overshoot bounded at headroom x knee, since own-best cannot rise under congestion. The target now ceils rather than rounds: with a modest self-raising ratio, rounding truncated the target back onto small caps and the ramp never left the floor. A new spread-aware closed-loop driver cycles completions through fast, typical, and slow latencies with the throughput set by the mean, and pins the regression: a 2.5x spread workload must escape the stall and sustain its knee depth, bounded above. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR raises ChangesCapacity ramp headroom and ceil fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/sn2-validator/src/performance.rs`:
- Around line 1475-1477: The assertion in the performance test is too loose and
does not match the headroom-based invariant it is meant to enforce. Update the
bound in the relevant test around the existing cap check so it reflects the
documented calculation from knee rate, own-best depth, and
CAPACITY_TARGET_HEADROOM (about 48 in this scenario), and keep the check tied to
the same cap variable so regressions that exceed the derived limit fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe6b4079-3626-4679-b5aa-d3de923dc892
📒 Files selected for processing (2)
crates/sn2-types/src/constants.rscrates/sn2-validator/src/performance.rs
The fixed bound of 75 was looser than the invariant the test claims: the target for this scenario is headroom x own-best depth (48), and the cap may sit anywhere inside the deadband hold region above it, so the bound derives from the constants as target x (1 + deadband). A regression that overshoots the headroom-derived cap now fails instead of passing under the slack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed in c47e24e: the bound now derives from the constants under test — headroom x own-best depth x (1 + deadband), i.e. 60 for this scenario — instead of the slack 75. Kept the deadband factor rather than the bare target of 48 because the cap may legitimately settle anywhere inside the hold region above the target (and on the stacked #595, a mid-probe raise from equilibrium lands exactly on that boundary). |
|
Thanks for the detailed analysis, and for stating the ramp invariant explicitly, that part is a real contribution to the docs. Closing this one on the premise rather than the math: production telemetry does not show the stall. After a cold restart with all in-memory state wiped, a top miner's adaptive cap climbed from 1 into the hundreds within the hour at the normal adjustment cadence, and fleet caps currently sit at their measured knees rather than frozen in the tens. The window where caps did look frozen coincided with sustained host memory pressure, which holds all upward adjustment by design; that pressure source has since been found and removed, so the observation had a different cause than the headroom constant. On the change itself, tripling the headroom also triples the steady queueing depth the controller tolerates per miner, which lands directly on validator memory we have just finished bounding. The ceil-versus-round observation at small caps is a good catch on its own though. If you want to submit that as a minimal standalone change with the spread-aware test that pins it, we would review that favorably. |
2 participants
Since 14.12.10 deployed, adaptive caps have been frozen fleet-wide in the tens — miners that previously held caps in the hundreds sit flat for days, with occasional unit-sized upticks and no ramp events. This is a stall in the knee-targeting controller from #586, and the failure is mine: the invariant it depends on was never stated, and the closed-loop tests were built in the one regime where it holds trivially.
The controller ramps only while
target > cap, withtarget = CAPACITY_TARGET_HEADROOM x rate x uncongested_service. The uncongested estimate is a minimum by construction (per-unit own-best latencies, deliberately congestion-proof). Below the knee the delivered rate iscap / typical_service, so the target evaluates toheadroom x (min_service / typical_service) x cap. That makes self-raising conditional: the ramp fires only whileheadroom x min_service > typical_service. With headroom at 1.5, any workload whose fastest completion runs below two thirds of its typical one — routine once network jitter and host load variance are in the response times — pins the target below the cap at every value. The rate is cap-bound, so it cannot grow; the cap is target-bound, so it cannot grow either. The only movement left is unit-sized creep when the max-filter rate estimator catches a delivery burst that ceils the target one above the cap, which matches the observed days-long crawl from single digits into the tens. The prior closed-loop tests drove constant per-unit latency, where min equals typical and the ratio is exactly one, so the stall was structurally invisible to them.Two changes. Headroom rises to 3.0: self-raising is restored for spreads up to 3x, and the overshoot past the knee stays bounded at headroom x knee because own-best cannot rise under congestion — this is the property that separates a larger headroom from the unbounded pre-#586 ramp, whose utilization signal inflated with the queueing it caused. Bounded queueing depth is the price of tolerating spread, and the budget-floor term already covers the sub-second regime where network noise dominates. Second, the target now ceils rather than rounds: with a modest self-raising ratio, rounding truncates the target back onto the current cap at small values (a ratio of 1.2 rounds 1.2 to 1 and 2.4 to 2), so even workloads inside the tolerated spread could never leave the floor.
The constant's documentation now states the invariant explicitly, and a new spread-aware closed-loop driver cycles completions through a fast, a typical, and a slow latency whose mean sets the throughput — the regime the constant-latency drivers cannot express. The pinned regression: a 2.5x-spread workload starting from cap 1 must escape the stall and sustain its knee depth, bounded above by the own-best-derived ceiling. The existing knee-pinning, budget-floor, demand-tracking, and steady-state-silence tests pass with bounds updated to the new headroom.
Expected effect after deploy: fleet caps resume geometric ramp within minutes at the adjustment cadence, settle near headroom x knee per miner, and capacity event volume returns to ramp transients. If a workload with a spread beyond 3x surfaces, the durable fix is rate-plateau probing (raise the cap, keep it only if the delivered rate follows) rather than a larger constant; happy to follow up with that if telemetry shows residual stalls.
🤖 Generated with Claude Code
Summary by CodeRabbit