Conversation
|
I think it is necessary to add performance tests to benchmark it in comparison with DirectDictionary and CacheDictionary. Or just CacheDictionary in memory with SSDCacheDictionary. |
src/Dictionaries/BucketCache.h
Outdated
|
|
||
| namespace | ||
| { | ||
| inline size_t nearestPowTwo(size_t x) |
There was a problem hiding this comment.
It is better to use roundUpToPowerOfTwoOrZero from our codebase
|
|
||
| size_t getPosition(const size_t bucket) const | ||
| { | ||
| const size_t idx = (bucket >> 1); |
There was a problem hiding this comment.
Why do we need it here? bucket it already a number from [0, buckets - 1]
There was a problem hiding this comment.
To determine a position in the bucket we need to read 4 bits from the array with positions, but we can address only bytes, so we need to divide bucket number by 2.
| }; | ||
|
|
||
| if (!write_buffer) | ||
| { |
There was a problem hiding this comment.
Braces are redundant according to our codestyle
|
|
||
| const size_t start_block = current_file_block_id % max_size; | ||
| const size_t finish_block = start_block + write_buffer_size; | ||
| for (const auto& key : keys) |
There was a problem hiding this comment.
Style: for (const auto & key : keys)
| { | ||
| // add partitions to queue | ||
| while (partitions.size() > max_partitions_count) | ||
| { |
| AIOContext aio_context(1); | ||
|
|
||
| while (io_submit(aio_context.ctx, 1, &request_ptr) != 1) | ||
| { |
| } | ||
|
|
||
| while (io_getevents(aio_context.ctx, 1, 1, &event, nullptr) != 1) | ||
| { |
|
|
||
| TemporalComplexKeysPool tmp_keys_pool; | ||
| storage.update( | ||
| source_ptr, |
There was a problem hiding this comment.
Is it true, that we can use many sources and not only one?
| void SSDCachePartition::remove() | ||
| { | ||
| std::unique_lock lock(rw_lock); | ||
| std::filesystem::remove(std::filesystem::path(path + BIN_FILE_EXT)); |
| std::vector<Key> required_ids(not_found_ids.size()); | ||
| std::transform(std::begin(not_found_ids), std::end(not_found_ids), std::begin(required_ids), [](const auto & pair) { return pair.first; }); | ||
|
|
||
| storage.update( |
There was a problem hiding this comment.
Maybe better to wrap it in a separate function, which adds source_ptr and lifetime and call this function?
|
BTW - it would be really interesting to see some perf number, and/or comparison with aerospike (#5629) |
|
@filimonov They are published in https://presentations.clickhouse.tech/hse_2020/3rd/SSD_Dictionary_full.pdf |
|
Just for the reference - aerospike gives up to ~1Mln req/sec |
|
@filimonov This number does not mean anything. |
Merging #8624 (ssd-cache)
|
Merged in #11947 |
8 participants
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Cache-dictionaries layout for storing data on SSD.
TODO: