Skip to content

fix(usability): Improve the cache dir and database startup panics - #9441

Merged
mergify[bot] merged 3 commits into
mainfrom
improve-panics-cache
May 6, 2025
Merged

fix(usability): Improve the cache dir and database startup panics#9441
mergify[bot] merged 3 commits into
mainfrom
improve-panics-cache

Conversation

@oxarbitrage

Copy link
Copy Markdown
Contributor

Motivation

Currently, Zebra will panic any time it can’t create or open its state database. For example, if the cache directory is invalid, the disk is full, permissions are wrong, or the RocksDB instance is already in use. The default panic messages are ugly and make it hard for end users to diagnose the real cause.

Close #8617

Solution

We now split these failures into two categories:

  • OS‑level errors (std::io::ErrorKind) related to the cache directory:

    • Permission denied
    • Disk full
    • Etc.
  • RocksDB errors (rocksdb::ErrorKind) at the database-opening stage:

    • Database already in use
    • Other RocksDB‑specific failures

In DiskDB::new:

  • Initialize the cache directory
    Attempt to create or open the cache directory first, and convert any std::io::ErrorKind into a descriptive panic.

  • Open the RocksDB database
    Once the directory is validated, build the full DB path and open RocksDB, catching any rocksdb::ErrorKind and panicking.

This separation makes sure that each panic points more directly to its root cause and suggests an actionable hint.

Tests

  • Updated the existing unit test to expect the new, more specific panic message.
  • Manual verification test for both an OS‑level error (e.g., read‑only directory) and a RocksDB “already in use” error.

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.
  • If the PR shouldn't be in the release notes, it has the
    C-exclude-from-changelog label.

@oxarbitrage
oxarbitrage requested a review from a team as a code owner April 17, 2025 21:57
@oxarbitrage
oxarbitrage requested review from arya2 and removed request for a team April 17, 2025 21:57
@gustavovalverde

Copy link
Copy Markdown
Member

I did not fully validate the PR, but we should validate if the ZEBRA_CACHE_DIR is set, and create the directory where this variable indicates, or else to the default location.

@upbqdn

upbqdn commented Apr 18, 2025

Copy link
Copy Markdown
Member

I did not fully validate the PR, but we should validate if the ZEBRA_CACHE_DIR is set, and create the directory where this variable indicates, or else to the default location.

The ZEBRA_CACHE_DIR var takes effect only in tests, which this PR doesn't change. I think we could make a bunch of things easier if we dropped the var entirely.

Comment thread zebra-state/src/constants.rs Outdated
Comment thread zebra-state/src/service/finalized_state/disk_db.rs Outdated
Comment thread zebra-state/src/service/finalized_state/disk_db.rs Outdated
@arya2

arya2 commented Apr 18, 2025

Copy link
Copy Markdown
Contributor

I think we could make a bunch of things easier if we dropped the var entirely.

I think its purpose was to ensure that those tests are never run unintentionally, but I agree it would make a bunch of things easier to drop it and we may want to do that.

Co-authored-by: Arya <aryasolhi@gmail.com>

@arya2 arya2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

mergify Bot added a commit that referenced this pull request May 5, 2025
mergify Bot added a commit that referenced this pull request May 6, 2025
@mergify
mergify Bot merged commit ee65be9 into main May 6, 2025
@mergify
mergify Bot deleted the improve-panics-cache branch May 6, 2025 01:01
@upbqdn

upbqdn commented May 6, 2025

Copy link
Copy Markdown
Member

I think we could make a bunch of things easier if we dropped the var entirely.

I think its purpose was to ensure that those tests are never run unintentionally, but I agree it would make a bunch of things easier to drop it and we may want to do that.

This #9481 (comment) is a good motivation to drop the variables.

@upbqdn

upbqdn commented May 6, 2025

Copy link
Copy Markdown
Member

Also, the description in PR #9323.

@natalieesk natalieesk moved this to Won't Fix in Zebra Jul 9, 2025
@natalieesk natalieesk added this to Zebra Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Won't Fix

Development

Successfully merging this pull request may close these issues.

Improve error message when Zebra crashes on startup

5 participants