This is an automated email from the ASF dual-hosted git repository. kxiao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push: new 8503d26e5c [bugfix](vertical-compaction) Only can init the SegmentCacheHandle once (#23246) 8503d26e5c is described below commit 8503d26e5cb62f054a76e012c623bdbc39433b2f Author: Lightman <31928846+lchangli...@users.noreply.github.com> AuthorDate: Mon Aug 21 21:57:27 2023 +0800 [bugfix](vertical-compaction) Only can init the SegmentCacheHandle once (#23246) --- be/src/olap/rowset/beta_rowset_reader.cpp | 1 - be/src/olap/segment_loader.cpp | 13 +++++----- be/src/olap/segment_loader.h | 41 +++++++++++++++---------------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/be/src/olap/rowset/beta_rowset_reader.cpp b/be/src/olap/rowset/beta_rowset_reader.cpp index bdb0e1fca6..947632f51f 100644 --- a/be/src/olap/rowset/beta_rowset_reader.cpp +++ b/be/src/olap/rowset/beta_rowset_reader.cpp @@ -210,7 +210,6 @@ 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)); diff --git a/be/src/olap/segment_loader.cpp b/be/src/olap/segment_loader.cpp index 94b0f9bca5..4704f8e802 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) { @@ -65,12 +65,14 @@ 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)) { - cache_handle->owned = false; return Status::OK(); } - cache_handle->owned = !use_cache; std::vector<segment_v2::SegmentSharedPtr> segments; RETURN_IF_ERROR(rowset->load_segments(&segments)); @@ -81,9 +83,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 784dc8e947..589a930201 100644 --- a/be/src/olap/segment_loader.h +++ b/be/src/olap/segment_loader.h @@ -128,13 +128,12 @@ private: 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,35 @@ public: } } - SegmentCacheHandle(SegmentCacheHandle&& other) noexcept { - std::swap(_cache, other._cache); - std::swap(_handle, other._handle); - this->owned = other.owned; - this->segments = std::move(other.segments); + [[nodiscard]] bool is_inited() const { return _init; } + + void init(std::vector<segment_v2::SegmentSharedPtr> 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<segment_v2::SegmentSharedPtr>& 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<segment_v2::SegmentSharedPtr> segments; - private: + bool _init {false}; + bool _owned {false}; + std::vector<segment_v2::SegmentSharedPtr> _segments; Cache* _cache = nullptr; Cache::Handle* _handle = nullptr; --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org