fix(agent): capture_pprof reports worker-wedge gauges + 30s capture throttle (#2422) - #2453
Merged
Merged
Conversation
…es GC-forcing captures (#2422) Two gaps in the capture_pprof diagnostic command: 1. The embedded runtime snapshot called collectors.CollectRuntimeStats() directly, which never populates the worker-pool wedge gauges — every capture reported commandsInFlight/commandsOverdue as 0/0, falsely showing a healthy pool to an operator chasing an overdue-commands heartbeat trend. Route through h.collectAgentRuntime(now) instead. 2. Every heap/all capture forces a stop-the-world runtime.GC() with no rate limit; as a server-queued command (10 concurrent / 100 queued), queued captures could degenerate into back-to-back GC pauses. Add a 30s minimum interval between admitted captures using the same atomic CAS throttle pattern as the heartbeat watchdog dump (#2392), rejecting throttled captures with an explicit error. Payload validation happens before the throttle so malformed requests don't burn the slot. Closes #2422 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
c429baf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://100990ce.breeze-9te.pages.dev |
| Branch Preview URL: | https://fix-2422-capture-pprof-gauge.breeze-9te.pages.dev |
…ry; precise rate-limit message Review follow-ups on #2453: - capturePprofTryAcquire is a copy of the watchdog CAS loop, not shared code, so the watchdog's concurrency/boundary tests don't cover it. Add TestCapturePprofTryAcquireConcurrent (32 racers, exactly one winner) and TestCapturePprofTryAcquireInterval (1ns-before rejected, exact interval admitted). - Rate-limit error now says "heap captures force a stop-the-world GC" — goroutine-only captures are throttled too but never call runtime.GC(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Review run: /pr-review-toolkit:review-pr (code-reviewer, pr-test-analyzer, silent-failure-hunter) Findings: 0 blocking across all three agents. 4 non-blocking items raised → 3 addressed in c429baf:
Declined (with rationale):
Tests: Status: review-clean, awaiting maintainer merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes both gaps in the
capture_pprofdiagnostic command (#2394/#2408 seam):Wedge gauges were always 0/0.
handleCapturePprofdiscarded its*Heartbeatreceiver and embeddedcollectors.CollectRuntimeStats()directly, which never populatescommandsInFlight/commandsOverdue. The snapshot now goes throughh.collectAgentRuntime(now)— the wrapper that wires ininFlightCommandStats— so a capture taken while chasing an overdue-commands heartbeat trend reflects the real pool state.No throttle on GC-forcing captures. Heap/all captures unconditionally ran
runtime.GC(); as a server-queued command (10 concurrent / 100 queued), a burst could force back-to-back stop-the-world GCs. Added a 30s minimum interval between admitted captures using the same atomic CAS slot pattern as the heartbeat watchdog dump throttle (fix(agent): stop heartbeat watchdog from firing every heartbeat on slow links (#2386) #2392). Throttled captures fail with an explicitrate-limitederror. Payload validation runs before the throttle so malformed requests don't consume the slot.All changes are confined to
handlers_diag.go/handlers_diag_test.go—heartbeat.gois untouched.Tests
TestHandleCapturePprofIncludesWedgeGauges(tracked overdue command shows up as 1/1 in the snapshot),TestHandleCapturePprofThrottled(second capture rejected, slot frees after interval),TestHandleCapturePprofValidationDoesNotConsumeSlot.*Heartbeat+ per-test throttle reset.cd agent && go test -race ./internal/heartbeat/...— ok (41s);go build ./...clean;go vetclean.Closes #2422
🤖 Generated with Claude Code