-
Notifications
You must be signed in to change notification settings - Fork 419
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
Allow setting RocksDB log directory #959
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, lol, you already did this. Thanks! :D
name = "db_log_dir" | ||
type = "std::path::PathBuf" | ||
doc = "Directory to store index database internal log (default: same as specified by `db_dir`)" | ||
default = "\"\".into()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd look better if we just made it optional ans only set it if it's Some
. I'm surprised setting an empty even does the right thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised setting an empty even does the right thing.
I agree, but it seems to work:
https://github.com/facebook/rocksdb/blob/f6fd4b9dbd15dba36f7e5ad23de407b5c26b1460/db/db_impl/db_impl_files.cc#L246
BTW, does configure_me
support Optional<PathBuf>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, simply leave out default = ""
and the type in the struct will be wrapped in Option
. (For mandatory param you write optional = false
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Please take a look at #960.
Following #680 (comment).