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.

Sign up for GitHub

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

Jump to bottom

sql: don't panic on releaseBytes when there are no bytes to release #35278

Closed
jordanlewis opened this issue Feb 28, 2019 · 0 comments · Fixed by #36213
Closed

sql: don't panic on releaseBytes when there are no bytes to release #35278

jordanlewis opened this issue Feb 28, 2019 · 0 comments · Fixed by #36213
Labels
A-sql-memmon SQL memory monitoring C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@jordanlewis
Copy link
Member

We've talked about this several times already. This issue represents converting the current panic there into a ReportOrPanic instead.

cc @knz @asubiotto
@jordanlewis jordanlewis added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-memmon SQL memory monitoring T-pointless-panic labels Feb 28, 2019
This was referenced Mar 4, 2019
Closed
Closed
Closed
This was referenced Mar 27, 2019
Merged
Merged
craig bot pushed a commit that referenced this issue Apr 2, 2019
@jordanlewis
Merge #36213
81f2938 
36213: mon: don't panic in prod if no bytes to release r=jordanlewis a=jordanlewis

Previously, we'd unconditionally panic the server if a memory monitor
was asked to release more bytes than it had allocated. This can happen
when there's a bug in memory accounting, but doesn't indicate an
actual memory leak.

This commit changes the panic to be a ReportOrPanic, which will panic in
test environments and send a report to sentry in production
environments. The memory monitor then resets itself to 0 allocated
bytes.

Closes #35278.

Release note (bug fix): prevent production server crashes on certain
assertion errors

Co-authored-by: Jordan Lewis <[email protected]>
@craig craig bot closed this as completed in #36213 Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-memmon SQL memory monitoring C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mon: don't panic in prod if no bytes to release jordanlewis/cockroach
1 participant
@jordanlewis