-
Notifications
You must be signed in to change notification settings - Fork 473
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
Add support of using hyper clock cache as the block cache #2031
Conversation
The script for pressing data into kvrocks:
The script for benchmarking: where the READ_RATIO is 0, 50, 90 or 100, respectively.
Here is the P99, P999 and P9999 latency with difference read ratios for the SET and GET command in the benchmark above. |
50 * 2,000,000 * 20(c) = 2,000,000,000 how much rocksdb sst data your make, 2G? please make at least 10G data |
I think the clock might has better concurrency and perhaps a bit worse cache hit rate. We can also report the cache hit rate etc for analyzing the performance here |
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.
+1 for this pr.
And maybe we can dive into benchmark result and find some reason of performance🤔
@wanghenshui Thanks for your review! The command I used to press the data into kvrocks is |
Thanks for your review! I would add these statistics later today. |
As for the statistics, you can refer |
kvrocks.conf
Outdated
# Specify the type of cache used in the block cache. | ||
# Accept value: "lru", "hcc" | ||
# default lru | ||
rocksdb.block_cache_type lru |
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.
Could you explain the meaning of lru and hcc for users in the comment>
src/storage/storage.cc
Outdated
options.row_cache = rocksdb::NewLRUCache(config_->rocks_db.row_cache_size * MiB); | ||
// Parameters for the rocksdb::LRUCacheOptions here are set in line with the default parameters for the | ||
// rocksdb::NewLRUCache function, which is now deprecated | ||
rocksdb::LRUCacheOptions lru_cache_options(config_->rocks_db.row_cache_size * MiB, -1, false, 0.5); |
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.
Could we add some named constants for these default arguments (e.g. -1, false, 0.5)?
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 have did this, but I am not sure whether the names of my constants are proper.
src/storage/storage.cc
Outdated
rocksdb::LRUCacheOptions lru_cache_options(block_cache_size, -1, false, 0.75); | ||
shared_block_cache = lru_cache_options.MakeSharedCache(); | ||
} else { | ||
rocksdb::HyperClockCacheOptions hcc_cache_options(block_cache_size, 0); |
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.
ditto for 0
here.
@JxLi0921 Great, thanks for your PR. I'm not sure if the QPS throughput is close between Hyper Clock Cache and LRU since I didn't see this in previous report. |
Sorry for late update. I found that the way I use Here is my current script for putting data and running benchmark: Putting data into kvrocks:
I failed to run the script above to complete because I am almost run out of disk space. After running the script above for about 20% of progress, the keyspace statistics of kvrocks is as follows.
Running benchmark:
where the Result: GET Latency: SET Latency: GET Ops/sec SET Ops/sec Overall(Get+Set) Ops/sec According to the results listed above, it seems that HCC does outperform LRU for latency and throughput in this simple benchmark. Block Cache Hit Ratio: I'm not sure whether it's normal to have the block cache hit ratios to be almost 100. Should I use a smaller block cache size? I used 4096MB block cache size in this benchmark, which is kvrocks' default configuration. |
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 for the investigate. I may go for this this weekend. At lease we can first checkin this patch?
Also @JxLi0921 would you mind tell the CPU and disk you're using?
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.
Cool, LGTM
CPU: AMD Ryzen 6800H |
This is related to this issue #2022 . I will soon post my simple benchmark result for this PR below.