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

*: prefer log.Fatal to panic for assertions #16479

Closed
petermattis opened this issue Jun 12, 2017 · 2 comments
Closed

*: prefer log.Fatal to panic for assertions #16479

petermattis opened this issue Jun 12, 2017 · 2 comments
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. S-3-productivity Severe issues that impede the productivity of CockroachDB developers.
Milestone

Comments

@petermattis
Copy link
Collaborator

panic causes stacks to unwind and defers to be invoked (possibly releasing locks) until eventually something either handles the panic (unlikely) or we reach the runtime and it kills the process. While the process is dying, other goroutines are still running which may encounter invalid states due to the panic. For fatal errors we should use log.Fatal{,f} instead which will kill the process before any locks are released.

I'm not sure if a massive search-and-replace is warranted, but some key packages such as storage deserve attention.

See #16460 for motivation.

petermattis added a commit to petermattis/cockroach that referenced this issue Jun 13, 2017
panic causes stacks to unwind and defers to be invoked (possibly
releasing locks) until eventually something either handles the
panic (unlikely) or we reach the runtime and it kills the process. While
the process is dying, other goroutines are still running which may
encounter invalid states due to the panic. These invalid states can
produce fatal errors of there own which seem impossible and are
difficult to debug. For fatal errors we should use log.Fatal{,f} instead
which will kill the process before any locks are released.

See cockroachdb#16479
@knz knz added the S-3-productivity Severe issues that impede the productivity of CockroachDB developers. label Apr 28, 2018
@petermattis petermattis added the C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. label Jul 21, 2018
@tbg
Copy link
Member

tbg commented Jul 22, 2018

@petermattis think it adds value to keep this open?

@tbg tbg modified the milestones: 2.2, Later Jul 22, 2018
@petermattis
Copy link
Collaborator Author

No, it doesn't. The message still stands, but better to engrave it in our brains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. S-3-productivity Severe issues that impede the productivity of CockroachDB developers.
Projects
None yet
Development

No branches or pull requests

3 participants