-
Notifications
You must be signed in to change notification settings - Fork 689
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
store: make pre-write check thresholds into constants #7546
Conversation
check_free_space_interval and free_space_threshold fields of RocksDB struct have always the same value. Turn them into constants.
If we can do #7545, I'd prefer that I guess. Or do we want to keep extra logging, which I imagine might be quite useful? I am also ambivalent on the PR itself -- the way I see it, field > constants, even if the field never changes, for two reasons:
no strong opinion though! |
/// How many writes before we execute a pre-write check. | ||
const CHECK_FREE_SPACE_INTERVAL: u16 = 256; | ||
/// How much free space is required for a write to be allowed. | ||
const FREE_SPACE_THRESHOLD: bytesize::ByteSize = bytesize::ByteSize::mb(16); |
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.
tbh, this feels somewhat lower than it needs to be. I think some of our structs are larger than that? Chunks in particular.
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, yes, definitely. That’s also part of why I want to just get rid of the code in #7545 since I believe it’s broken. I just didn’t want to change the values in this PR.
Then again, this code has been around for over two years and we didn’t feel the need to configure it. ;) Really I prepared this PR because I wanted to move code out of the RocksDB constructor. It’s just two lines but still it kinda helps. (Ideally Rust would have default field initialisers but oh well). |
check_free_space_interval and free_space_threshold fields of RocksDB struct have always the same value. Turn them into constants.
check_free_space_interval and free_space_threshold fields of RocksDB
struct have always the same value. Turn them into constants.