fix rocksdb block cache size#122
Conversation
| { | ||
| block_opts.set_block_size(config.compaction.block_size); | ||
| let cache_size = cmp::max(8, config.memory_budget() / 3); | ||
| let cache_size = cmp::max(8 * MB, config.memory_budget() / 3); |
There was a problem hiding this comment.
I wonder where does this / 3 come from, should it be / columns?
EDIT: probably not, but still curious.
There was a problem hiding this comment.
There was a problem hiding this comment.
Nice catch. I think I got that one third from here: https://github.com/facebook/rocksdb/wiki/Setup-Options-and-Basic-Tuning#block-cache-size
There was a problem hiding this comment.
One option is to simply take config.memory_budget() / 3 and assume that if someone changes that to something below 24MB they know what they're doing? This 8MB minimum strikes me as a bit opaque.
There was a problem hiding this comment.
Actually, I agree with David, that it may be surprising for a user. Although, in practice, most of the time it only makes sense to use the default cache size or to increase it, which won't affect the maximum. I'll push a commit to remove it.
| { | ||
| block_opts.set_block_size(config.compaction.block_size); | ||
| let cache_size = cmp::max(8, config.memory_budget() / 3); | ||
| let cache_size = cmp::max(8 * MB, config.memory_budget() / 3); |
There was a problem hiding this comment.
Nice catch. I think I got that one third from here: https://github.com/facebook/rocksdb/wiki/Setup-Options-and-Basic-Tuning#block-cache-size
|
One thing that we could improve is that we divide our memory budget evenly across all column families. But in reality, e.g. in parity-ethereum probably 90% of the data exists on the state column family, so maybe we should cache more aggressively there. |
| { | ||
| block_opts.set_block_size(config.compaction.block_size); | ||
| let cache_size = cmp::max(8, config.memory_budget() / 3); | ||
| let cache_size = cmp::max(8 * MB, config.memory_budget() / 3); |
There was a problem hiding this comment.
One option is to simply take config.memory_budget() / 3 and assume that if someone changes that to something below 24MB they know what they're doing? This 8MB minimum strikes me as a bit opaque.
|
IIRC 8MB is the rocksdb default. Maybe we can add a comment about that. |
* master: uint: faster mul by u64 (#125) fix rocksdb block cache size (#122) uint: convert benchmarks to criterion (#123) Bump fixed-hash to 0.3.2 (#134) Use EntropyRng instead of OsRng (#130) Bump parity-crypto to 0.3.1 (#132) rust-crypto dependency removal (#85) Use newly stablized TryFrom trait for primitive-types (#127)
3 participants
LRU block cache size was limited by 8 bytes, instead of 8 MB.