Rocksdb: create folder if it doesn't exist #245
Conversation
faddat
requested review from
alexanderbez,
cmwaters,
creachadair,
tac0turtle,
tychoish and
williambanfield
as code owners
Codecov Report
@@ Coverage Diff @@
## master #245 +/- ##
==========================================
- Coverage 68.54% 68.48% -0.07%
==========================================
Files 27 27
Lines 2130 2132 +2
==========================================
Hits 1460 1460
- Misses 595 597 +2
Partials 75 75
|
There was a problem hiding this comment.
The description says rocks, but touches badger also, and a bunch of other unrelated changes. Let's please keep the cleanup changes separate from the changes that do more meaningful stuff.
Note that BadgerDB doesn't require this treatment: It will create the prefix of the path it's given if necessary. (I don't know about Rocks)
|
oh sure thing! I have a habit of always fumpting. 1 moment please. Back down to one file changed now. |
This reverts commit 8f6316c.
| @@ -50,7 +51,10 @@ func NewRocksDBWithOptions(name string, dir string, opts *gorocksdb.Options) (*R | |||
| dbPath := filepath.Join(dir, name+".db") | |||
| db, err := gorocksdb.OpenDb(opts, dbPath) | |||
| if err != nil { | |||
| return nil, err | |||
| err = os.MkdirAll(dbPath, 0755) | |||
There was a problem hiding this comment.
gorocksdb.OpenDb calls the C library's rocksdb_open which already creates the directory if it doesn't exist (ref). So this is harmless, but should not be necessary for RocksDB.
There was a problem hiding this comment.
not for state sync snapshots.
I am not sure why that is the case, but it is the case.
There was a problem hiding this comment.
I suppose there is no harm in creating the directory. But isn't this backwards? If we don't create the directory till after the OpenDb call has already failed, the code below will fail anyway.
I'm guessing this probably wants to be something like
if err := os.MkdirAll(dbPath, 0700); err != nil {
return nil, fmt.Errorf("creating database path: %w", err)
}
db, err := gorocksdb.OpenDb(opts, dbPath)
if err != nil {
return nil, fmt.Errorf("opening database: %w", err)
}or words to that effect.
There was a problem hiding this comment.
I don't know why this works. Let me show ya what I have......
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Instead of panicking and crashing, if rocksdb finds that it doesn't have a folder (eg: snapshots.db) it will now make it.
Tested and working here:
evmos/evmos#542