From 6c638edcb391042420624cd0c07e48ce192da1a4 Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Wed, 15 Jun 2022 14:42:00 +0800 Subject: [PATCH 1/5] fix memtable mem tracker --- be/src/olap/delta_writer.cpp | 12 +++++++++++- be/src/olap/memtable.cpp | 7 ++++++- be/src/olap/memtable_flush_executor.cpp | 9 ++++++++- be/src/runtime/mem_tracker.h | 12 ++++++++++-- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/be/src/olap/delta_writer.cpp b/be/src/olap/delta_writer.cpp index 132a79a390afd5..15cf8bbd29abb3 100644 --- a/be/src/olap/delta_writer.cpp +++ b/be/src/olap/delta_writer.cpp @@ -232,8 +232,12 @@ Status DeltaWriter::flush_memtable_and_wait(bool need_wait) { RETURN_NOT_OK(_flush_memtable_async()); _reset_mem_table(); } else { - DCHECK(mem_consumption() > _mem_table->memory_usage()); // this means there should be at least one memtable in flush queue. + // At this time, mem_consumption() > _mem_table->memory_usage(), + // but affected by the consumption order of mem tracker, the child tracker is consumed first, + // and then the parent tracker is consumed recursively. Therefore, when entering this judgment + // during the consumption process, the DeltaWriter tracker and the memtable tracker consume Inconsistent, + // mem_consumption() < _mem_table->memory_usage() may appear. This probability is small and will not matter. } if (need_wait) { @@ -295,6 +299,11 @@ Status DeltaWriter::close_wait() { // return error if previous flush failed RETURN_NOT_OK(_flush_token->wait()); + _mem_table.reset(); + // In allocate/free of mem_pool, the consume_cache of _mem_tracker will be called, + // and _untracked_mem must be flushed first. + MemTracker::memory_leak_check(_mem_tracker.get()); + // use rowset meta manager to save meta _cur_rowset = _rowset_writer->build(); if (_cur_rowset == nullptr) { @@ -327,6 +336,7 @@ Status DeltaWriter::cancel() { // cancel and wait all memtables in flush queue to be finished _flush_token->cancel(); } + MemTracker::memory_leak_check(_mem_tracker.get()); _is_cancelled = true; return Status::OK(); } diff --git a/be/src/olap/memtable.cpp b/be/src/olap/memtable.cpp index c8c3d78e2a169c..e2b95cc8a63a49 100644 --- a/be/src/olap/memtable.cpp +++ b/be/src/olap/memtable.cpp @@ -39,7 +39,9 @@ MemTable::MemTable(int64_t tablet_id, Schema* schema, const TabletSchema* tablet _tablet_schema(tablet_schema), _slot_descs(slot_descs), _keys_type(keys_type), - _mem_tracker(MemTracker::create_tracker(-1, "MemTable", parent_tracker)), + // The insert block needs to manually consume the mem tracker. Using the virtual tracker can avoid + // frequent recursive consumption of the parent tracker, thereby improving performance. + _mem_tracker(MemTracker::create_virtual_tracker(-1, "MemTable", parent_tracker)), _buffer_mem_pool(new MemPool(_mem_tracker.get())), _table_mem_pool(new MemPool(_mem_tracker.get())), _schema_size(_schema->schema_size()), @@ -103,6 +105,9 @@ void MemTable::_init_agg_functions(const vectorized::Block* block) { MemTable::~MemTable() { std::for_each(_row_in_blocks.begin(), _row_in_blocks.end(), std::default_delete()); _mem_tracker->release(_mem_usage); + _buffer_mem_pool->free_all(); + _table_mem_pool->free_all(); + MemTracker::memory_leak_check(_mem_tracker.get(), true); } MemTable::RowCursorComparator::RowCursorComparator(const Schema* schema) : _schema(schema) {} diff --git a/be/src/olap/memtable_flush_executor.cpp b/be/src/olap/memtable_flush_executor.cpp index 30c88529047fd7..cb85f32e4e75ca 100644 --- a/be/src/olap/memtable_flush_executor.cpp +++ b/be/src/olap/memtable_flush_executor.cpp @@ -61,7 +61,14 @@ Status FlushToken::wait() { } void FlushToken::_flush_memtable(std::shared_ptr memtable, int64_t submit_task_time) { - SCOPED_ATTACH_TASK_THREAD(ThreadContext::TaskType::LOAD, memtable->mem_tracker()); + // The memtable mem tracker needs to be completely accurate, + // because DeltaWriter judges whether to flush memtable according to the memtable memory usage. + // The macro of attach thread mem tracker is affected by the destructuring order of local variables, + // so it cannot completely correspond to the number of new/delete bytes recorded in scoped, + // and there is a small range of errors. So direct track load mem tracker. + DCHECK(memtable->mem_tracker()->parent_task_mem_tracker_no_own()); + SCOPED_ATTACH_TASK_THREAD(ThreadContext::TaskType::LOAD, + memtable->mem_tracker()->parent_task_mem_tracker_no_own()); _stats.flush_wait_time_ns += (MonotonicNanos() - submit_task_time); SCOPED_CLEANUP({ memtable.reset(); }); // If previous flush has failed, return directly diff --git a/be/src/runtime/mem_tracker.h b/be/src/runtime/mem_tracker.h index 85a6550f7a0ac5..684f23ec9aa732 100644 --- a/be/src/runtime/mem_tracker.h +++ b/be/src/runtime/mem_tracker.h @@ -416,9 +416,17 @@ class MemTracker { // If an ancestor of this tracker is a Task MemTracker, return that tracker. Otherwise return nullptr. MemTracker* parent_task_mem_tracker() { - MemTracker* tracker = this; + if (this->_level == MemTrackerLevel::TASK) { + return this; + } else { + return parent_task_mem_tracker_no_own().get(); + } + } + + std::shared_ptr parent_task_mem_tracker_no_own() { + std::shared_ptr tracker = this->_parent; while (tracker != nullptr && tracker->_level != MemTrackerLevel::TASK) { - tracker = tracker->_parent.get(); + tracker = tracker->_parent; } return tracker; } From cb397ea29041a35e9ce3591cfbf6fe7e179638cd Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Thu, 16 Jun 2022 00:51:11 +0800 Subject: [PATCH 2/5] fix be ut --- be/src/olap/delta_writer.cpp | 6 ++++-- be/src/olap/memtable.cpp | 4 +--- be/src/olap/memtable_flush_executor.cpp | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/be/src/olap/delta_writer.cpp b/be/src/olap/delta_writer.cpp index 15cf8bbd29abb3..4320d0ca2bfe3d 100644 --- a/be/src/olap/delta_writer.cpp +++ b/be/src/olap/delta_writer.cpp @@ -94,8 +94,10 @@ Status DeltaWriter::init() { return Status::OLAPInternalError(OLAP_ERR_TABLE_NOT_FOUND); } - _mem_tracker = - MemTracker::create_tracker(-1, "DeltaWriter:" + std::to_string(_tablet->tablet_id())); + // Only consume mem tracker manually in mem table. Using the virtual tracker can avoid + // frequent recursive consumption of the parent tracker, thereby improving performance. + _mem_tracker = MemTracker::create_virtual_tracker( + -1, "DeltaWriter:" + std::to_string(_tablet->tablet_id())); // check tablet version number if (_tablet->version_count() > config::max_tablet_version_num) { LOG(WARNING) << "failed to init delta writer. version count: " << _tablet->version_count() diff --git a/be/src/olap/memtable.cpp b/be/src/olap/memtable.cpp index e2b95cc8a63a49..1b44d0ee25726b 100644 --- a/be/src/olap/memtable.cpp +++ b/be/src/olap/memtable.cpp @@ -39,9 +39,7 @@ MemTable::MemTable(int64_t tablet_id, Schema* schema, const TabletSchema* tablet _tablet_schema(tablet_schema), _slot_descs(slot_descs), _keys_type(keys_type), - // The insert block needs to manually consume the mem tracker. Using the virtual tracker can avoid - // frequent recursive consumption of the parent tracker, thereby improving performance. - _mem_tracker(MemTracker::create_virtual_tracker(-1, "MemTable", parent_tracker)), + _mem_tracker(MemTracker::create_tracker(-1, "MemTable", parent_tracker)), _buffer_mem_pool(new MemPool(_mem_tracker.get())), _table_mem_pool(new MemPool(_mem_tracker.get())), _schema_size(_schema->schema_size()), diff --git a/be/src/olap/memtable_flush_executor.cpp b/be/src/olap/memtable_flush_executor.cpp index cb85f32e4e75ca..4ad1d07fcafcda 100644 --- a/be/src/olap/memtable_flush_executor.cpp +++ b/be/src/olap/memtable_flush_executor.cpp @@ -61,6 +61,7 @@ Status FlushToken::wait() { } void FlushToken::_flush_memtable(std::shared_ptr memtable, int64_t submit_task_time) { +#ifndef BE_TEST // The memtable mem tracker needs to be completely accurate, // because DeltaWriter judges whether to flush memtable according to the memtable memory usage. // The macro of attach thread mem tracker is affected by the destructuring order of local variables, @@ -69,6 +70,7 @@ void FlushToken::_flush_memtable(std::shared_ptr memtable, int64_t sub DCHECK(memtable->mem_tracker()->parent_task_mem_tracker_no_own()); SCOPED_ATTACH_TASK_THREAD(ThreadContext::TaskType::LOAD, memtable->mem_tracker()->parent_task_mem_tracker_no_own()); +#endif _stats.flush_wait_time_ns += (MonotonicNanos() - submit_task_time); SCOPED_CLEANUP({ memtable.reset(); }); // If previous flush has failed, return directly From e9c69f2c0bf05ca0e73d8d6fe539ca723d48f1e9 Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Thu, 16 Jun 2022 04:06:57 +0800 Subject: [PATCH 3/5] fix virtual tracker --- be/src/runtime/mem_tracker.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/be/src/runtime/mem_tracker.cpp b/be/src/runtime/mem_tracker.cpp index 824c8b85308a3c..d53e975d648e66 100644 --- a/be/src/runtime/mem_tracker.cpp +++ b/be/src/runtime/mem_tracker.cpp @@ -162,9 +162,13 @@ MemTracker::MemTracker(int64_t byte_limit, const std::string& label, void MemTracker::init() { DCHECK_GE(_limit, -1); MemTracker* tracker = this; - while (tracker != nullptr && tracker->_virtual == false) { + while (tracker != nullptr) { _all_trackers.push_back(tracker); if (tracker->has_limit()) _limit_trackers.push_back(tracker); + // This means that it terminates when recursively consume/release from the current tracker up to the virtual tracker. + if (tracker->_virtual == true) { + break; + } tracker = tracker->_parent.get(); } DCHECK_GT(_all_trackers.size(), 0); From 765af9c4b3b09ff788f61c3128734a09ef28ab61 Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Thu, 16 Jun 2022 22:36:21 +0800 Subject: [PATCH 4/5] fix memory_verbose_track false --- be/src/common/config.h | 9 ++++++++- be/src/olap/delta_writer.cpp | 6 +----- be/src/olap/memtable_flush_executor.cpp | 8 +++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/be/src/common/config.h b/be/src/common/config.h index 0ae23127e77028..7d233e1a43db69 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -632,7 +632,14 @@ CONF_Bool(track_new_delete, "true"); // If true, switch TLS MemTracker to count more detailed memory, // including caches such as ExecNode operators and TabletManager. -CONF_Bool(memory_verbose_track, "true"); +// +// At present, there is a performance problem in the frequent switch thread mem tracker. +// This is because the mem tracker exists as a shared_ptr in the thread local. When switching, +// the shared_ptr use_count of the current tracker will be -1, and the tracker use_count to be replaced will be +1. +// Frequent changes are the same as A tracker shared_ptr is very time consuming. +// TODO: 1. Reduce unnecessary thread mem tracker switches, +// 2. Consider using raw pointers for mem tracker in thread local +CONF_Bool(memory_verbose_track, "false"); // Default level of MemTracker to show in web page // now MemTracker support two level: diff --git a/be/src/olap/delta_writer.cpp b/be/src/olap/delta_writer.cpp index 4320d0ca2bfe3d..085fc5769f3d4b 100644 --- a/be/src/olap/delta_writer.cpp +++ b/be/src/olap/delta_writer.cpp @@ -234,12 +234,8 @@ Status DeltaWriter::flush_memtable_and_wait(bool need_wait) { RETURN_NOT_OK(_flush_memtable_async()); _reset_mem_table(); } else { + DCHECK(mem_consumption() > _mem_table->memory_usage()); // this means there should be at least one memtable in flush queue. - // At this time, mem_consumption() > _mem_table->memory_usage(), - // but affected by the consumption order of mem tracker, the child tracker is consumed first, - // and then the parent tracker is consumed recursively. Therefore, when entering this judgment - // during the consumption process, the DeltaWriter tracker and the memtable tracker consume Inconsistent, - // mem_consumption() < _mem_table->memory_usage() may appear. This probability is small and will not matter. } if (need_wait) { diff --git a/be/src/olap/memtable_flush_executor.cpp b/be/src/olap/memtable_flush_executor.cpp index 4ad1d07fcafcda..29300c0cda1bdf 100644 --- a/be/src/olap/memtable_flush_executor.cpp +++ b/be/src/olap/memtable_flush_executor.cpp @@ -67,9 +67,11 @@ void FlushToken::_flush_memtable(std::shared_ptr memtable, int64_t sub // The macro of attach thread mem tracker is affected by the destructuring order of local variables, // so it cannot completely correspond to the number of new/delete bytes recorded in scoped, // and there is a small range of errors. So direct track load mem tracker. - DCHECK(memtable->mem_tracker()->parent_task_mem_tracker_no_own()); - SCOPED_ATTACH_TASK_THREAD(ThreadContext::TaskType::LOAD, - memtable->mem_tracker()->parent_task_mem_tracker_no_own()); + // TODO(zxy) After rethinking the use of switch thread mem tracker, choose the appropriate way to get + // load mem tracke here. + // DCHECK(memtable->mem_tracker()->parent_task_mem_tracker_no_own()); + // SCOPED_ATTACH_TASK_THREAD(ThreadContext::TaskType::LOAD, + // memtable->mem_tracker()->parent_task_mem_tracker_no_own()); #endif _stats.flush_wait_time_ns += (MonotonicNanos() - submit_task_time); SCOPED_CLEANUP({ memtable.reset(); }); From 4ba0b6f5acd1dc6312112ec18bde9a7ed17b9cce Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Thu, 16 Jun 2022 23:37:24 +0800 Subject: [PATCH 5/5] fix commnet --- be/src/common/config.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/be/src/common/config.h b/be/src/common/config.h index 7d233e1a43db69..18cf2df3f251c7 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -634,9 +634,9 @@ CONF_Bool(track_new_delete, "true"); // including caches such as ExecNode operators and TabletManager. // // At present, there is a performance problem in the frequent switch thread mem tracker. -// This is because the mem tracker exists as a shared_ptr in the thread local. When switching, -// the shared_ptr use_count of the current tracker will be -1, and the tracker use_count to be replaced will be +1. -// Frequent changes are the same as A tracker shared_ptr is very time consuming. +// This is because the mem tracker exists as a shared_ptr in the thread local. Each time it is switched, +// the atomic variable use_count in the shared_ptr of the current tracker will be -1, and the tracker to be +// replaced use_count +1, multi-threading Frequent changes to the same tracker shared_ptr are slow. // TODO: 1. Reduce unnecessary thread mem tracker switches, // 2. Consider using raw pointers for mem tracker in thread local CONF_Bool(memory_verbose_track, "false");