base: add Exit function to replace direct os.Exit calls#5865
Draft
williamchoe3 wants to merge 1 commit intocockroachdb:masterfrom
Draft
base: add Exit function to replace direct os.Exit calls#5865williamchoe3 wants to merge 1 commit intocockroachdb:masterfrom
williamchoe3 wants to merge 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
Add a package-level Exit function in internal/base that wraps os.Exit. Embedders (e.g. CockroachDB) can override base.Exit to route invariant violation exits through their own fatal logging infrastructure, giving them visibility into why the process died (log lines, crash reports, marker files). Previously, 20 call sites in production code called os.Exit(1) directly, which bypassed any logging or crash reporting the embedder had set up. The process would simply vanish with exit code 1 and no indication of why. Informs cockroachdb#2061
williamchoe3
force-pushed
the
pebble-exit-func
branch
from
cf42f85 to
8a30bc9
Compare
williamchoe3
added a commit
to williamchoe3/cockroach
that referenced
this pull request
Mar 26, 2026
Set pebble.Options.ExitFunc so that Pebble invariant violation exits (cache assertions, memtable checks, sstable reader checks) route through CockroachDB's log.Fatal path instead of calling os.Exit(1) directly. This gives visibility into why the process died via log lines, Sentry crash reports, and fatal exit marker files. Requires cockroachdb/pebble#5865. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Potential Bug(s) DetectedThe three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation. Next Steps: Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary. After you review the findings, please tag the issue as follows:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a package-level
Exitfunction ininternal/basethat wrapsos.Exit. Embedders (e.g. CockroachDB) can overridebase.Exitto route invariant violation exits through their own fatal logging infrastructure.Previously, 20 call sites in production code called
os.Exit(1)directly, which bypassed any logging or crash reporting the embedder had set up. The process would simply vanish with exit code 1 and no indication of why.All
os.Exitcalls in production code (cache invariant checks, memtable assertions, sstable reader checks, the unreferenced DB finalizer, and the default logger) are replaced withbase.Exit. CLI tools, test utilities, and metamorphic testing code are left unchanged.Informs #2061