You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/12/08 05:59:00 UTC
[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #5008: [Optimize][Cache]Implementation of Separated Page Cache
acelyc111 commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-doris/pull/5008#discussion_r538048284
##########
File path: be/src/common/config.h
##########
@@ -260,6 +260,8 @@ CONF_Int64(index_stream_cache_capacity, "10737418240");
// Cache for storage page size
CONF_String(storage_page_cache_limit, "20G");
+// Ratio for index page cache
Review comment:
I think it's better to give more description, such as what does the left size used for?
##########
File path: be/src/olap/page_cache.cpp
##########
@@ -21,35 +21,81 @@ namespace doris {
StoragePageCache* StoragePageCache::_s_instance = nullptr;
-void StoragePageCache::create_global_cache(size_t capacity) {
+void StoragePageCache::create_global_cache(size_t capacity, double index_cache_ratio) {
DCHECK(_s_instance == nullptr);
- static StoragePageCache instance(capacity);
+ static StoragePageCache instance(capacity, index_cache_ratio);
_s_instance = &instance;
}
-StoragePageCache::StoragePageCache(size_t capacity)
- : _cache(new_lru_cache("StoragePageCache", capacity)) {}
+StoragePageCache::StoragePageCache(size_t capacity, double index_cache_ratio)
+ : _index_cache_ratio(index_cache_ratio) {
+ if (index_cache_ratio == 0) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
+ }
+ else if (index_cache_ratio == 1) {
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
+ }
+ else if (index_cache_ratio > 0 && index_cache_ratio < 1) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", static_cast<size_t>(capacity * (1 - index_cache_ratio))));
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", static_cast<size_t>(capacity * index_cache_ratio)));
+ }
Review comment:
What if the other values passed, how about add `CHECK(false) << "xxx"` here?
##########
File path: be/src/olap/page_cache.cpp
##########
@@ -21,35 +21,81 @@ namespace doris {
StoragePageCache* StoragePageCache::_s_instance = nullptr;
-void StoragePageCache::create_global_cache(size_t capacity) {
+void StoragePageCache::create_global_cache(size_t capacity, double index_cache_ratio) {
DCHECK(_s_instance == nullptr);
- static StoragePageCache instance(capacity);
+ static StoragePageCache instance(capacity, index_cache_ratio);
_s_instance = &instance;
}
-StoragePageCache::StoragePageCache(size_t capacity)
- : _cache(new_lru_cache("StoragePageCache", capacity)) {}
+StoragePageCache::StoragePageCache(size_t capacity, double index_cache_ratio)
+ : _index_cache_ratio(index_cache_ratio) {
+ if (index_cache_ratio == 0) {
Review comment:
For double type values, better to use expression like `std::abs(index_cache_ratio) < 1e-6` to judge whether they are equaled. If you think it's too ugly, you can use percentagle int value instead.
##########
File path: be/src/olap/page_cache.h
##########
@@ -53,36 +54,44 @@ class StoragePageCache {
};
// Create global instance of this class
- static void create_global_cache(size_t capacity);
+ static void create_global_cache(size_t capacity, double index_cache_ratio);
// Return global instance.
// Client should call create_global_cache before.
static StoragePageCache* instance() { return _s_instance; }
- StoragePageCache(size_t capacity);
+ StoragePageCache(size_t capacity, double index_cache_ratio);
// Lookup the given page in the cache.
//
// If the page is found, the cache entry will be written into handle.
// PageCacheHandle will release cache entry to cache when it
// destructs.
//
+ // Cache type selection is determined by page_type argument
+ //
// Return true if entry is found, otherwise return false.
- bool lookup(const CacheKey& key, PageCacheHandle* handle);
+ bool lookup(const CacheKey& key, PageCacheHandle* handle, segment_v2::PageTypePB page_type);
// Insert a page with key into this cache.
// Given handle will be set to valid reference.
// This function is thread-safe, and when two clients insert two same key
// concurrently, this function can assure that only one page is cached.
// The in_memory page will have higher priority.
void insert(const CacheKey& key, const Slice& data, PageCacheHandle* handle,
- bool in_memory = false);
+ segment_v2::PageTypePB page_type, bool in_memory = false);
+
+ // Page cache available check.
+ // When ratio is set to 0 or 1, the index or data cache will not be allocated.
+ bool is_cache_available(segment_v2::PageTypePB page_type);
Review comment:
Better to define it as a static finction
##########
File path: be/src/olap/page_cache.cpp
##########
@@ -21,35 +21,81 @@ namespace doris {
StoragePageCache* StoragePageCache::_s_instance = nullptr;
-void StoragePageCache::create_global_cache(size_t capacity) {
+void StoragePageCache::create_global_cache(size_t capacity, double index_cache_ratio) {
DCHECK(_s_instance == nullptr);
- static StoragePageCache instance(capacity);
+ static StoragePageCache instance(capacity, index_cache_ratio);
_s_instance = &instance;
}
-StoragePageCache::StoragePageCache(size_t capacity)
- : _cache(new_lru_cache("StoragePageCache", capacity)) {}
+StoragePageCache::StoragePageCache(size_t capacity, double index_cache_ratio)
+ : _index_cache_ratio(index_cache_ratio) {
+ if (index_cache_ratio == 0) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
+ }
+ else if (index_cache_ratio == 1) {
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
+ }
+ else if (index_cache_ratio > 0 && index_cache_ratio < 1) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", static_cast<size_t>(capacity * (1 - index_cache_ratio))));
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", static_cast<size_t>(capacity * index_cache_ratio)));
+ }
+}
-bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle) {
- auto lru_handle = _cache->lookup(key.encode());
- if (lru_handle == nullptr) {
+bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle, segment_v2::PageTypePB page_type) {
+ Cache::Handle* lru_handle = nullptr;
+ switch (page_type) {
+ case segment_v2::DATA_PAGE:
+ lru_handle = _data_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_data_page_cache.get(), lru_handle);
+ break;
+ case segment_v2::INDEX_PAGE:
+ lru_handle = _index_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_index_page_cache.get(), lru_handle);
+ break;
Review comment:
There are some duplicate code, how about define a function like `Cache::Handle* get_cache(segment_v2::PageTypePB page_type)` to get `lru_handle ` at first, then do the same thing as before?
##########
File path: docs/en/administrator-guide/config/be_config.md
##########
@@ -695,6 +695,8 @@ Indicates how many tablets in this data directory failed to load. At the same ti
### `storage_page_cache_limit`
+### `index_page_cache_ratio`
Review comment:
Add descriptions.
##########
File path: docs/zh-CN/administrator-guide/config/be_config.md
##########
@@ -693,6 +693,8 @@ load tablets from header failed, failed tablets size: xxx, path=xxx
### `storage_page_cache_limit`
+### `index_page_cache_ratio`
Review comment:
Add descriptions.
##########
File path: be/src/olap/page_cache.cpp
##########
@@ -21,35 +21,81 @@ namespace doris {
StoragePageCache* StoragePageCache::_s_instance = nullptr;
-void StoragePageCache::create_global_cache(size_t capacity) {
+void StoragePageCache::create_global_cache(size_t capacity, double index_cache_ratio) {
DCHECK(_s_instance == nullptr);
- static StoragePageCache instance(capacity);
+ static StoragePageCache instance(capacity, index_cache_ratio);
_s_instance = &instance;
}
-StoragePageCache::StoragePageCache(size_t capacity)
- : _cache(new_lru_cache("StoragePageCache", capacity)) {}
+StoragePageCache::StoragePageCache(size_t capacity, double index_cache_ratio)
+ : _index_cache_ratio(index_cache_ratio) {
+ if (index_cache_ratio == 0) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
+ }
+ else if (index_cache_ratio == 1) {
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
+ }
+ else if (index_cache_ratio > 0 && index_cache_ratio < 1) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", static_cast<size_t>(capacity * (1 - index_cache_ratio))));
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", static_cast<size_t>(capacity * index_cache_ratio)));
+ }
+}
-bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle) {
- auto lru_handle = _cache->lookup(key.encode());
- if (lru_handle == nullptr) {
+bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle, segment_v2::PageTypePB page_type) {
+ Cache::Handle* lru_handle = nullptr;
+ switch (page_type) {
+ case segment_v2::DATA_PAGE:
+ lru_handle = _data_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_data_page_cache.get(), lru_handle);
+ break;
+ case segment_v2::INDEX_PAGE:
+ lru_handle = _index_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_index_page_cache.get(), lru_handle);
+ break;
+ default:
return false;
}
- *handle = PageCacheHandle(_cache.get(), lru_handle);
return true;
}
void StoragePageCache::insert(const CacheKey& key, const Slice& data, PageCacheHandle* handle,
- bool in_memory) {
+ segment_v2::PageTypePB page_type, bool in_memory) {
auto deleter = [](const doris::CacheKey& key, void* value) { delete[](uint8_t*) value; };
CachePriority priority = CachePriority::NORMAL;
if (in_memory) {
priority = CachePriority::DURABLE;
}
- auto lru_handle = _cache->insert(key.encode(), data.data, data.size, deleter, priority);
- *handle = PageCacheHandle(_cache.get(), lru_handle);
+ Cache::Handle* lru_handle = nullptr;
+ switch (page_type) {
+ case segment_v2::DATA_PAGE:
+ lru_handle = _data_page_cache->insert(key.encode(), data.data, data.size, deleter, priority);
+ *handle = PageCacheHandle(_data_page_cache.get(), lru_handle);
+ break;
+ case segment_v2::INDEX_PAGE:
+ lru_handle = _index_page_cache->insert(key.encode(), data.data, data.size, deleter, priority);
+ *handle = PageCacheHandle(_index_page_cache.get(), lru_handle);
+ break;
Review comment:
Same
##########
File path: be/src/olap/page_cache.cpp
##########
@@ -21,35 +21,81 @@ namespace doris {
StoragePageCache* StoragePageCache::_s_instance = nullptr;
-void StoragePageCache::create_global_cache(size_t capacity) {
+void StoragePageCache::create_global_cache(size_t capacity, double index_cache_ratio) {
DCHECK(_s_instance == nullptr);
- static StoragePageCache instance(capacity);
+ static StoragePageCache instance(capacity, index_cache_ratio);
_s_instance = &instance;
}
-StoragePageCache::StoragePageCache(size_t capacity)
- : _cache(new_lru_cache("StoragePageCache", capacity)) {}
+StoragePageCache::StoragePageCache(size_t capacity, double index_cache_ratio)
+ : _index_cache_ratio(index_cache_ratio) {
+ if (index_cache_ratio == 0) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
+ }
+ else if (index_cache_ratio == 1) {
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
+ }
+ else if (index_cache_ratio > 0 && index_cache_ratio < 1) {
+ _data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", static_cast<size_t>(capacity * (1 - index_cache_ratio))));
+ _index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", static_cast<size_t>(capacity * index_cache_ratio)));
+ }
+}
-bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle) {
- auto lru_handle = _cache->lookup(key.encode());
- if (lru_handle == nullptr) {
+bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle, segment_v2::PageTypePB page_type) {
+ Cache::Handle* lru_handle = nullptr;
+ switch (page_type) {
+ case segment_v2::DATA_PAGE:
+ lru_handle = _data_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_data_page_cache.get(), lru_handle);
+ break;
+ case segment_v2::INDEX_PAGE:
+ lru_handle = _index_page_cache->lookup(key.encode());
+ if (lru_handle == nullptr) {
+ return false;
+ }
+ *handle = PageCacheHandle(_index_page_cache.get(), lru_handle);
+ break;
+ default:
return false;
}
- *handle = PageCacheHandle(_cache.get(), lru_handle);
return true;
}
void StoragePageCache::insert(const CacheKey& key, const Slice& data, PageCacheHandle* handle,
- bool in_memory) {
+ segment_v2::PageTypePB page_type, bool in_memory) {
auto deleter = [](const doris::CacheKey& key, void* value) { delete[](uint8_t*) value; };
CachePriority priority = CachePriority::NORMAL;
if (in_memory) {
priority = CachePriority::DURABLE;
}
- auto lru_handle = _cache->insert(key.encode(), data.data, data.size, deleter, priority);
- *handle = PageCacheHandle(_cache.get(), lru_handle);
+ Cache::Handle* lru_handle = nullptr;
+ switch (page_type) {
+ case segment_v2::DATA_PAGE:
+ lru_handle = _data_page_cache->insert(key.encode(), data.data, data.size, deleter, priority);
+ *handle = PageCacheHandle(_data_page_cache.get(), lru_handle);
+ break;
+ case segment_v2::INDEX_PAGE:
+ lru_handle = _index_page_cache->insert(key.encode(), data.data, data.size, deleter, priority);
+ *handle = PageCacheHandle(_index_page_cache.get(), lru_handle);
+ break;
+ default:
+ break;
+ }
+}
+
+bool StoragePageCache::is_cache_available(segment_v2::PageTypePB page_type) {
+ if (page_type == segment_v2::DATA_PAGE && _index_cache_ratio == 1) {
+ return false;
+ }
+ else if (page_type == segment_v2::INDEX_PAGE && _index_cache_ratio == 0) {
+ return false;
+ }
Review comment:
If you do the refactor as mentioned above, you can simply judge whether the return `lru_handle ` is nullptr or not
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org