fix(watchdog): attach heap profile and memstats to heap_pressure alerts (HUB-2) #105

Open
bot wants to merge 1 commit from fix/HUB-2-heap-profile-attachment into fix/HUB-3-watchdog-sustained-breach
Member

Do not merge without Curry's OK.

Stacked on #104 (base is fix/HUB-3-watchdog-sustained-breach). Merge #104 first, then retarget this to main. Or fold it into #104 if Curry wants a single release and restart.

Sentry: POKEBEDROCK-HUB-2 (watchdog: heap usage high). One event showed 27.7 GB HeapAlloc against the 2 GiB threshold, with only 285 goroutines. That's real heap growth, not a goroutine pile-up, but the alert carries nothing that says what holds the memory.

Change

  • On heap_pressure alerts, attach heap.pprof: the runtime's sampled heap profile, in gzipped pprof format (go tool pprof heap.pprof). It's capped at 8 MiB. A larger profile is dropped rather than cut, since a truncated pprof can't be parsed, and the event gets heap_profile_truncated=true.
  • Add heap_inuse_bytes, heap_objects and sys_bytes to the watchdog context for every alert.
  • It reuses #104's stable per-condition fingerprint.

Cost: a sampled profile, not a full heap dump. A few ms, and only on a heap alert, which is subject to the 5m cooldown.

Tests

go vet ./..., go test ./..., and go test -race ./pokebedrock/watchdog/... pass. New TestHeapProfile: the profile is non-empty, gzip, and within the limit, and an over-limit profile is rejected.

This repo has no PR CI; release.yml runs vet+test on main. It goes live only after merge and a hub restart, both of which need Curry's OK.

**Do not merge without Curry's OK.** **Stacked on #104** (base is `fix/HUB-3-watchdog-sustained-breach`). Merge #104 first, then retarget this to `main`. Or fold it into #104 if Curry wants a single release and restart. Sentry: POKEBEDROCK-HUB-2 (`watchdog: heap usage high`). One event showed 27.7 GB HeapAlloc against the 2 GiB threshold, with only 285 goroutines. That's real heap growth, not a goroutine pile-up, but the alert carries nothing that says what holds the memory. ## Change - On `heap_pressure` alerts, attach `heap.pprof`: the runtime's sampled heap profile, in gzipped pprof format (`go tool pprof heap.pprof`). It's capped at 8 MiB. A larger profile is dropped rather than cut, since a truncated pprof can't be parsed, and the event gets `heap_profile_truncated=true`. - Add `heap_inuse_bytes`, `heap_objects` and `sys_bytes` to the `watchdog` context for every alert. - It reuses #104's stable per-condition fingerprint. Cost: a sampled profile, not a full heap dump. A few ms, and only on a heap alert, which is subject to the 5m cooldown. ## Tests `go vet ./...`, `go test ./...`, and `go test -race ./pokebedrock/watchdog/...` pass. New `TestHeapProfile`: the profile is non-empty, gzip, and within the limit, and an over-limit profile is rejected. This repo has no PR CI; release.yml runs vet+test on main. It goes live only after merge and a hub restart, both of which need Curry's OK.
heap_pressure alerts now carry heap.pprof (sampled heap profile, capped at
8 MiB; dropped and flagged heap_profile_truncated if larger) plus
heap_inuse_bytes, heap_objects and sys_bytes in the watchdog context.
bot left a comment

Verdict: no blockers. Merge after #104 (or fold it in), with Curry's OK. Posted as COMMENT because the PR is bot-authored.

Checked locally at 5c5a5eb1 since there's no PR CI: go vet, go test -race ./pokebedrock/watchdog/ (including TestHeapProfile) and go build ./... pass. Using the sampled heap profile (WriteTo(…, 0)) rather than a full dump is the right call, and dropping the profile instead of cutting it is correct, because a truncated pprof file won't parse.

Notes:

  1. heap_profile_truncated=true can be a false label. It's set whenever heapProfile returns false, and that also covers pprof.Lookup returning nil, a WriteTo error, and an empty buffer. Someone triaging will read "too big" when the write actually failed. Use something like heap_profile: "ok" | "over_limit" | "write_failed" | "empty", or only set truncated on the size branch (inline).
  2. Event size vs sentry.Flush(2 * time.Second). A heap alert can now carry up to 8 MiB of pprof, and up to 1 MiB more of goroutines.txt when it fires alongside a stall or pileup. At 27 GB heap the box is usually close to the OOM-kill that the eager flush is there for, and 2s may not get 9 MiB to self-hosted Sentry. Either give the flush more time on heap alerts, or check what the 8 MiB cap is based on: sampled heap profiles are normally tens to hundreds of KB, so 1–2 MiB would still be generous. (The flush line is unchanged context, so this is a body note.)
  3. The base is fix/HUB-3-watchdog-sustained-breach. Retarget to main right after #104 merges, or Forgejo will merge it into a dead branch.
  4. Nit: TestHeapProfile only checks gzip magic and size. A profile.Parse round-trip would prove it's actually readable by go tool pprof, but it's optional.
**Verdict: no blockers. Merge after #104 (or fold it in), with Curry's OK.** Posted as COMMENT because the PR is bot-authored. Checked locally at `5c5a5eb1` since there's no PR CI: `go vet`, `go test -race ./pokebedrock/watchdog/` (including `TestHeapProfile`) and `go build ./...` pass. Using the sampled heap profile (`WriteTo(…, 0)`) rather than a full dump is the right call, and dropping the profile instead of cutting it is correct, because a truncated pprof file won't parse. Notes: 1. **`heap_profile_truncated=true` can be a false label.** It's set whenever `heapProfile` returns false, and that also covers `pprof.Lookup` returning nil, a `WriteTo` error, and an empty buffer. Someone triaging will read "too big" when the write actually failed. Use something like `heap_profile: "ok" | "over_limit" | "write_failed" | "empty"`, or only set `truncated` on the size branch (inline). 2. **Event size vs `sentry.Flush(2 * time.Second)`.** A heap alert can now carry up to 8 MiB of pprof, and up to 1 MiB more of `goroutines.txt` when it fires alongside a stall or pileup. At 27 GB heap the box is usually close to the OOM-kill that the eager flush is there for, and 2s may not get 9 MiB to self-hosted Sentry. Either give the flush more time on heap alerts, or check what the 8 MiB cap is based on: sampled heap profiles are normally tens to hundreds of KB, so 1–2 MiB would still be generous. (The flush line is unchanged context, so this is a body note.) 3. The base is `fix/HUB-3-watchdog-sustained-breach`. Retarget to `main` right after #104 merges, or Forgejo will merge it into a dead branch. 4. Nit: `TestHeapProfile` only checks gzip magic and size. A `profile.Parse` round-trip would prove it's actually readable by `go tool pprof`, but it's optional.
@ -393,0 +419,4 @@
Payload: prof,
})
} else {
ctx["heap_profile_truncated"] = true
Author
Member

This also fires when pprof.Lookup is nil, WriteTo errors, or the buffer is empty, so 'truncated' can be a false label. Use a status value (ok / over_limit / write_failed / empty), or set this only on the size branch.

This also fires when `pprof.Lookup` is nil, `WriteTo` errors, or the buffer is empty, so 'truncated' can be a false label. Use a status value (`ok` / `over_limit` / `write_failed` / `empty`), or set this only on the size branch.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/HUB-2-heap-profile-attachment:fix/HUB-2-heap-profile-attachment
git switch fix/HUB-2-heap-profile-attachment

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch fix/HUB-3-watchdog-sustained-breach
git merge --no-ff fix/HUB-2-heap-profile-attachment
git switch fix/HUB-2-heap-profile-attachment
git rebase fix/HUB-3-watchdog-sustained-breach
git switch fix/HUB-3-watchdog-sustained-breach
git merge --ff-only fix/HUB-2-heap-profile-attachment
git switch fix/HUB-2-heap-profile-attachment
git rebase fix/HUB-3-watchdog-sustained-breach
git switch fix/HUB-3-watchdog-sustained-breach
git merge --no-ff fix/HUB-2-heap-profile-attachment
git switch fix/HUB-3-watchdog-sustained-breach
git merge --squash fix/HUB-2-heap-profile-attachment
git switch fix/HUB-3-watchdog-sustained-breach
git merge --ff-only fix/HUB-2-heap-profile-attachment
git switch fix/HUB-3-watchdog-sustained-breach
git merge fix/HUB-2-heap-profile-attachment
git push origin fix/HUB-3-watchdog-sustained-breach
Sign in to join this conversation.
No description provided.