Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: dump a summary of heap stats on OOM #52546

Open
bcmills opened this issue Apr 25, 2022 · 4 comments
Open

runtime: dump a summary of heap stats on OOM #52546

bcmills opened this issue Apr 25, 2022 · 4 comments
Labels
NeedsInvestigation
Milestone

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 25, 2022

I'm seeing fairly frequent OOM failures in the Go build dashboard.

Some of them look like misconfigured or undersized builders, while others could plausibly be bugs in the runtime (such as #52433). In other cases (such as #49957), it's much less clear what led to the failure.

It would be very helpful to be able to distinguish those cases from the failure logs. Today, we get fatal error: runtime: out of memory, but generally the runtime doesn't actually tell us how much memory it was using when that happened. (We do get a goroutine dump, but a goroutine dump is only loosely correlated with the size of the heap.)

@golang/runtime: how difficult would it be to have the runtime dump a best-effort summary of heap stats when it hits the out of memory condition?

@bcmills
Copy link
Member Author

@bcmills bcmills commented Apr 25, 2022

#52547 is another example of a failure mode where a dump on runtime OOM would be helpful.

@bcmills
Copy link
Member Author

@bcmills bcmills commented Apr 25, 2022

This might also help with #51019.

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 25, 2022

This is straightforward to do. There's an "unsafe read" operation on memstats.heapStats which gets you most of the way there (that may need to be modified so all the reads are atomic, just to make that effort a little better). We can also dump the rest of stats which are just the sysMemStat-typed fields in memstats (this may not be 100% true before my outstanding refactoring CLs, but will be after). It's all just atomics (and I think a little math), no locks necessary.

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 25, 2022

I can write this CL on top of my stack.

@cagedmantis cagedmantis added this to the Backlog milestone Apr 28, 2022
@cagedmantis cagedmantis added the NeedsInvestigation label Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

3 participants