Skip to content

util/must: merge with logcrash assertions #107428

@erikgrinaker

Description

@erikgrinaker

Currently, the logcrash package has some functionality that overlaps must: ReportOrPanic which panics in non-release builds and otherwise sends a Sentry notification, and the cluster setting debug.panic_on_failed_assertions which makes ReportOrPanic always panic.

ReportOrPanic should be replaced by must assertions. The cluster setting may either be replaced entirely by COCKROACH_FATAL_ASSERTIONS, or must may be changed to respect the cluster setting instead. Using a cluster setting requires propagating cluster settings to must, and won't work for assertion failures that happen before KV is functional and cluster settings are propagated. The environment variable, on the other hand, requires a node restart.

My preference is for the environment variable: we rarely, if ever, expect users to enable fatal assertions, so the inconvenience of a node restart is not a big problem, and environment variables are more reliable and easier to propagate.

Jira issue: CRDB-30038

Metadata

Metadata

Assignees

Labels

A-testingTesting tools and infrastructureC-enhancementSolution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)T-testengTestEng Team

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions