From 1faef45230ddbbe7f0db531c852ed258da8b39d0 Mon Sep 17 00:00:00 2001 From: Lchangliang <915311741@qq.com> Date: Mon, 21 Aug 2023 14:30:55 +0800 Subject: [PATCH 1/4] [bugfix](vertical-compaction) Only init the _segment_cache_handle in beta_rowset_reader once --- be/src/olap/rowset/beta_rowset_reader.cpp | 11 +++++++---- be/src/olap/rowset/beta_rowset_reader.h | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/be/src/olap/rowset/beta_rowset_reader.cpp b/be/src/olap/rowset/beta_rowset_reader.cpp index bdb0e1fca6171c..e10c6f235d8ed7 100644 --- a/be/src/olap/rowset/beta_rowset_reader.cpp +++ b/be/src/olap/rowset/beta_rowset_reader.cpp @@ -210,10 +210,13 @@ Status BetaRowsetReader::get_segment_iterators(RowsetReaderContext* read_context _read_options.output_columns = read_context->output_columns; // load segments - // use cache is true when do vertica compaction - bool should_use_cache = use_cache || read_context->reader_type == ReaderType::READER_QUERY; - RETURN_IF_ERROR(SegmentLoader::instance()->load_segments(_rowset, &_segment_cache_handle, - should_use_cache)); + // When doing vertical compaction, the load_segments is called many times + if (!_load_segment_once) { + bool should_use_cache = use_cache || read_context->reader_type == ReaderType::READER_QUERY; + RETURN_IF_ERROR(SegmentLoader::instance()->load_segments(_rowset, &_segment_cache_handle, + should_use_cache)); + _load_segment_once = true; + } // create iterator for each segment auto& segments = _segment_cache_handle.get_segments(); diff --git a/be/src/olap/rowset/beta_rowset_reader.h b/be/src/olap/rowset/beta_rowset_reader.h index 74ef9c96a69c70..04ae6198e27595 100644 --- a/be/src/olap/rowset/beta_rowset_reader.h +++ b/be/src/olap/rowset/beta_rowset_reader.h @@ -97,6 +97,7 @@ class BetaRowsetReader : public RowsetReader { // make sure this handle is initialized and valid before // reading data. + bool _load_segment_once {false}; SegmentCacheHandle _segment_cache_handle; StorageReadOptions _read_options; From 963d37862d7aa20efcabf61b382240f82a29bd9f Mon Sep 17 00:00:00 2001 From: Lchangliang <915311741@qq.com> Date: Mon, 21 Aug 2023 16:02:15 +0800 Subject: [PATCH 2/4] tmp --- be/src/olap/segment_loader.cpp | 9 +++----- be/src/olap/segment_loader.h | 39 ++++++++++++++++------------------ 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/be/src/olap/segment_loader.cpp b/be/src/olap/segment_loader.cpp index 94b0f9bca5b68c..c9a8416505a8bb 100644 --- a/be/src/olap/segment_loader.cpp +++ b/be/src/olap/segment_loader.cpp @@ -37,7 +37,7 @@ bool SegmentCache::lookup(const SegmentCache::CacheKey& key, SegmentCacheHandle* if (lru_handle == nullptr) { return false; } - *handle = SegmentCacheHandle(_cache.get(), lru_handle); + handle->init(_cache.get(), lru_handle); return true; } @@ -56,7 +56,7 @@ void SegmentCache::insert(const SegmentCache::CacheKey& key, SegmentCache::Cache auto lru_handle = _cache->insert(key.encode(), &value, sizeof(SegmentCache::CacheValue), deleter, CachePriority::NORMAL, meta_mem_usage); - *handle = SegmentCacheHandle(_cache.get(), lru_handle); + handle->init(_cache.get(), lru_handle); } void SegmentCache::erase(const SegmentCache::CacheKey& key) { @@ -67,10 +67,8 @@ Status SegmentLoader::load_segments(const BetaRowsetSharedPtr& rowset, SegmentCacheHandle* cache_handle, bool use_cache) { SegmentCache::CacheKey cache_key(rowset->rowset_id()); if (_segment_cache->lookup(cache_key, cache_handle)) { - cache_handle->owned = false; return Status::OK(); } - cache_handle->owned = !use_cache; std::vector segments; RETURN_IF_ERROR(rowset->load_segments(&segments)); @@ -81,9 +79,8 @@ Status SegmentLoader::load_segments(const BetaRowsetSharedPtr& rowset, cache_value->segments = std::move(segments); _segment_cache->insert(cache_key, *cache_value, cache_handle); } else { - cache_handle->segments = std::move(segments); + cache_handle->init(std::move(segments)); } - return Status::OK(); } diff --git a/be/src/olap/segment_loader.h b/be/src/olap/segment_loader.h index 784dc8e9473b32..52d18b54b153fd 100644 --- a/be/src/olap/segment_loader.h +++ b/be/src/olap/segment_loader.h @@ -128,13 +128,12 @@ class SegmentLoader { class SegmentCacheHandle { public: SegmentCacheHandle() = default; - SegmentCacheHandle(Cache* cache, Cache::Handle* handle) : _cache(cache), _handle(handle) {} ~SegmentCacheHandle() { if (_handle != nullptr) { CHECK(_cache != nullptr); - CHECK(segments.empty()) << segments.size(); - CHECK(!owned); + CHECK(_segments.empty()) << _segments.size(); + CHECK(!_owned); // last_visit_time is set when release. // because it only be needed when pruning. ((SegmentCache::CacheValue*)_cache->value(_handle))->last_visit_time = UnixMillis(); @@ -142,35 +141,33 @@ class SegmentCacheHandle { } } - SegmentCacheHandle(SegmentCacheHandle&& other) noexcept { - std::swap(_cache, other._cache); - std::swap(_handle, other._handle); - this->owned = other.owned; - this->segments = std::move(other.segments); + void init(std::vector segments) { + DCHECK(_init); + _owned = true; + _segments = std::move(segments); + _init = true; } - SegmentCacheHandle& operator=(SegmentCacheHandle&& other) noexcept { - std::swap(_cache, other._cache); - std::swap(_handle, other._handle); - this->owned = other.owned; - this->segments = std::move(other.segments); - return *this; + void init(Cache* cache, Cache::Handle* handle) { + DCHECK(_init); + _owned = false; + _cache = cache; + _handle = handle; + _init = true; } std::vector& get_segments() { - if (owned) { - return segments; + if (_owned) { + return _segments; } else { return ((SegmentCache::CacheValue*)_cache->value(_handle))->segments; } } -public: - // If set to true, the loaded segments will be saved in segments, not in lru cache; - bool owned = false; - std::vector segments; - private: + bool _init {false}; + bool _owned {false}; + std::vector _segments; Cache* _cache = nullptr; Cache::Handle* _handle = nullptr; From fdb10744da3107b1bf490cfe3c1d1ca2c8dc9089 Mon Sep 17 00:00:00 2001 From: Lchangliang <915311741@qq.com> Date: Mon, 21 Aug 2023 16:28:35 +0800 Subject: [PATCH 3/4] tmp --- be/src/olap/rowset/beta_rowset_reader.cpp | 10 +++------- be/src/olap/rowset/beta_rowset_reader.h | 1 - be/src/olap/segment_loader.cpp | 4 ++++ be/src/olap/segment_loader.h | 2 ++ 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/be/src/olap/rowset/beta_rowset_reader.cpp b/be/src/olap/rowset/beta_rowset_reader.cpp index e10c6f235d8ed7..947632f51fa8d3 100644 --- a/be/src/olap/rowset/beta_rowset_reader.cpp +++ b/be/src/olap/rowset/beta_rowset_reader.cpp @@ -210,13 +210,9 @@ Status BetaRowsetReader::get_segment_iterators(RowsetReaderContext* read_context _read_options.output_columns = read_context->output_columns; // load segments - // When doing vertical compaction, the load_segments is called many times - if (!_load_segment_once) { - bool should_use_cache = use_cache || read_context->reader_type == ReaderType::READER_QUERY; - RETURN_IF_ERROR(SegmentLoader::instance()->load_segments(_rowset, &_segment_cache_handle, - should_use_cache)); - _load_segment_once = true; - } + bool should_use_cache = use_cache || read_context->reader_type == ReaderType::READER_QUERY; + RETURN_IF_ERROR(SegmentLoader::instance()->load_segments(_rowset, &_segment_cache_handle, + should_use_cache)); // create iterator for each segment auto& segments = _segment_cache_handle.get_segments(); diff --git a/be/src/olap/rowset/beta_rowset_reader.h b/be/src/olap/rowset/beta_rowset_reader.h index 04ae6198e27595..74ef9c96a69c70 100644 --- a/be/src/olap/rowset/beta_rowset_reader.h +++ b/be/src/olap/rowset/beta_rowset_reader.h @@ -97,7 +97,6 @@ class BetaRowsetReader : public RowsetReader { // make sure this handle is initialized and valid before // reading data. - bool _load_segment_once {false}; SegmentCacheHandle _segment_cache_handle; StorageReadOptions _read_options; diff --git a/be/src/olap/segment_loader.cpp b/be/src/olap/segment_loader.cpp index c9a8416505a8bb..4704f8e802d0b8 100644 --- a/be/src/olap/segment_loader.cpp +++ b/be/src/olap/segment_loader.cpp @@ -65,6 +65,10 @@ void SegmentCache::erase(const SegmentCache::CacheKey& key) { Status SegmentLoader::load_segments(const BetaRowsetSharedPtr& rowset, SegmentCacheHandle* cache_handle, bool use_cache) { + if (cache_handle->is_inited()) { + return Status::OK(); + } + SegmentCache::CacheKey cache_key(rowset->rowset_id()); if (_segment_cache->lookup(cache_key, cache_handle)) { return Status::OK(); diff --git a/be/src/olap/segment_loader.h b/be/src/olap/segment_loader.h index 52d18b54b153fd..7427967aff7ae9 100644 --- a/be/src/olap/segment_loader.h +++ b/be/src/olap/segment_loader.h @@ -141,6 +141,8 @@ class SegmentCacheHandle { } } + [[nodiscard]] bool is_inited() const { return _init; } + void init(std::vector segments) { DCHECK(_init); _owned = true; From 3f5253191657571805e85f5d6fb3dc081b960db9 Mon Sep 17 00:00:00 2001 From: Lchangliang <915311741@qq.com> Date: Mon, 21 Aug 2023 18:53:49 +0800 Subject: [PATCH 4/4] tmp --- be/src/olap/segment_loader.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/be/src/olap/segment_loader.h b/be/src/olap/segment_loader.h index 7427967aff7ae9..589a930201c57c 100644 --- a/be/src/olap/segment_loader.h +++ b/be/src/olap/segment_loader.h @@ -144,14 +144,14 @@ class SegmentCacheHandle { [[nodiscard]] bool is_inited() const { return _init; } void init(std::vector segments) { - DCHECK(_init); + DCHECK(!_init); _owned = true; _segments = std::move(segments); _init = true; } void init(Cache* cache, Cache::Handle* handle) { - DCHECK(_init); + DCHECK(!_init); _owned = false; _cache = cache; _handle = handle;