This is an automated email from the ASF dual-hosted git repository.
yiguolei 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 cffdeff4ec [fix](memory) Fix memory leak by calling boost::stacktrace
(#14269)
cffdeff4ec is described below
commit cffdeff4ec56ca4d3b04c95ba31db4378bd6011c
Author: Xinyi Zou <zouxiny...@gmail.com>
AuthorDate: Tue Nov 15 08:58:57 2022 +0800
[fix](memory) Fix memory leak by calling boost::stacktrace (#14269)
boost::stacktrace::stacktrace() has memory leak, so use glog internal func
to print stacktrace.
The reason for the memory leak of boost::stacktrace is that a state is
saved in the thread local of each thread but not actively released. The test
found that each thread leaked about 100M after calling boost::stacktrace.
refer to:
boostorg/stacktrace#118
boostorg/stacktrace#111
---
be/src/common/status.cpp | 9 ++----
be/src/olap/rowset/segment_v2/segment.cpp | 12 ++++----
be/src/olap/rowset/segment_v2/segment.h | 2 ++
be/src/runtime/memory/mem_tracker_limiter.cpp | 41 ++++++++++++++-------------
be/src/runtime/memory/mem_tracker_limiter.h | 4 +--
be/src/runtime/thread_context.h | 2 ++
be/src/service/doris_main.cpp | 2 +-
be/src/util/stack_util.cpp | 5 ++++
be/src/vec/common/allocator.h | 16 +++++------
9 files changed, 49 insertions(+), 44 deletions(-)
diff --git a/be/src/common/status.cpp b/be/src/common/status.cpp
index ca1a148706..f60ef5b61a 100644
--- a/be/src/common/status.cpp
+++ b/be/src/common/status.cpp
@@ -7,9 +7,8 @@
#include <rapidjson/prettywriter.h>
#include <rapidjson/stringbuffer.h>
-#include <boost/stacktrace.hpp>
-
#include "gen_cpp/types.pb.h" // for PStatus
+#include "util/stack_util.h"
namespace doris {
@@ -64,18 +63,16 @@ Status::Status(const PStatus& s) {
}
}
-// Implement it here to remove the boost header file from status.h to reduce
precompile time
Status Status::ConstructErrorStatus(int16_t precise_code) {
// This will print all error status's stack, it maybe too many, but it is just
used for debug
#ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with
message: " << msg
- << "\n caused by:" << boost::stacktrace::stacktrace();
+ << "\n caused by:" << get_stack_trace();
#endif
if (error_states[abs(precise_code)].stacktrace) {
// Add stacktrace as part of message, could use LOG(WARN) << "" <<
status will print both
// the error message and the stacktrace
- return Status(TStatusCode::INTERNAL_ERROR,
-
boost::stacktrace::to_string(boost::stacktrace::stacktrace()), precise_code);
+ return Status(TStatusCode::INTERNAL_ERROR, get_stack_trace(),
precise_code);
} else {
return Status(TStatusCode::INTERNAL_ERROR, std::string_view(),
precise_code);
}
diff --git a/be/src/olap/rowset/segment_v2/segment.cpp
b/be/src/olap/rowset/segment_v2/segment.cpp
index ab12e4f900..6651e18799 100644
--- a/be/src/olap/rowset/segment_v2/segment.cpp
+++ b/be/src/olap/rowset/segment_v2/segment.cpp
@@ -75,12 +75,12 @@ Segment::Segment(uint32_t segment_id, RowsetId rowset_id,
TabletSchemaSPtr table
: _segment_id(segment_id),
_rowset_id(rowset_id),
_tablet_schema(tablet_schema),
- _meta_mem_usage(0) {}
+ _meta_mem_usage(0),
+
_segment_meta_mem_tracker(StorageEngine::instance()->segment_meta_mem_tracker())
{}
Segment::~Segment() {
#ifndef BE_TEST
- if (StorageEngine::instance())
-
StorageEngine::instance()->segment_meta_mem_tracker()->release(_meta_mem_usage);
+ _segment_meta_mem_tracker->release(_meta_mem_usage);
#endif
}
@@ -152,8 +152,7 @@ Status Segment::_parse_footer() {
_file_reader->path().native(), file_size, 12
+ footer_length);
}
_meta_mem_usage += footer_length;
- if (StorageEngine::instance())
-
StorageEngine::instance()->segment_meta_mem_tracker()->consume(footer_length);
+ _segment_meta_mem_tracker->consume(footer_length);
std::string footer_buf;
footer_buf.resize(footer_length);
@@ -219,8 +218,7 @@ Status Segment::load_index() {
DCHECK(footer.has_short_key_page_footer());
_meta_mem_usage += body.get_size();
- if (StorageEngine::instance())
-
StorageEngine::instance()->segment_meta_mem_tracker()->consume(body.get_size());
+ _segment_meta_mem_tracker->consume(body.get_size());
_sk_index_decoder.reset(new ShortKeyIndexDecoder);
return _sk_index_decoder->parse(body,
footer.short_key_page_footer());
}
diff --git a/be/src/olap/rowset/segment_v2/segment.h
b/be/src/olap/rowset/segment_v2/segment.h
index 03209e9b4e..dd2457ef51 100644
--- a/be/src/olap/rowset/segment_v2/segment.h
+++ b/be/src/olap/rowset/segment_v2/segment.h
@@ -150,6 +150,8 @@ private:
std::unique_ptr<ShortKeyIndexDecoder> _sk_index_decoder;
// primary key index reader
std::unique_ptr<PrimaryKeyIndexReader> _pk_index_reader;
+ // Segment may be destructed after StorageEngine, in order to exit
gracefully.
+ std::shared_ptr<MemTracker> _segment_meta_mem_tracker;
};
} // namespace segment_v2
diff --git a/be/src/runtime/memory/mem_tracker_limiter.cpp
b/be/src/runtime/memory/mem_tracker_limiter.cpp
index 40afce874e..2453dca631 100644
--- a/be/src/runtime/memory/mem_tracker_limiter.cpp
+++ b/be/src/runtime/memory/mem_tracker_limiter.cpp
@@ -25,6 +25,7 @@
#include "runtime/runtime_state.h"
#include "runtime/thread_context.h"
#include "util/pretty_printer.h"
+#include "util/stack_util.h"
#include "util/string_util.h"
namespace doris {
@@ -167,9 +168,11 @@ std::string
MemTrackerLimiter::log_usage(MemTracker::Snapshot snapshot) {
}
void MemTrackerLimiter::print_log_usage(const std::string& msg) {
- std::string detail = msg;
- detail += "\n " + MemTrackerLimiter::process_mem_log_str();
if (_enable_print_log_usage) {
+ _enable_print_log_usage = false;
+ std::string detail = msg;
+ detail += "\n " + MemTrackerLimiter::process_mem_log_str();
+ detail += "\n" + get_stack_trace();
detail += "\n " + log_usage();
std::string child_trackers_usage;
std::vector<MemTracker::Snapshot> snapshots;
@@ -179,30 +182,28 @@ void MemTrackerLimiter::print_log_usage(const
std::string& msg) {
}
if (!child_trackers_usage.empty()) detail += child_trackers_usage;
- // TODO: memory leak by calling `boost::stacktrace` in tcmalloc hook,
- // test whether overwriting malloc/free is the same problem in
jemalloc/tcmalloc.
- // detail += "\n" +
boost::stacktrace::to_string(boost::stacktrace::stacktrace());
LOG(WARNING) << detail;
- _enable_print_log_usage = false;
}
}
-void MemTrackerLimiter::print_log_process_usage(const std::string& msg) {
- MemTrackerLimiter::_enable_print_log_process_usage = false;
- std::string detail = msg;
- detail += "\n " + MemTrackerLimiter::process_mem_log_str();
- std::vector<MemTracker::Snapshot> snapshots;
- MemTrackerLimiter::make_process_snapshots(&snapshots);
- MemTrackerLimiter::make_type_snapshots(&snapshots,
MemTrackerLimiter::Type::GLOBAL);
- for (const auto& snapshot : snapshots) {
- if (snapshot.parent_label == "") {
- detail += "\n " + MemTrackerLimiter::log_usage(snapshot);
- } else {
- detail += "\n " + MemTracker::log_usage(snapshot);
+void MemTrackerLimiter::print_log_process_usage(const std::string& msg, bool
with_stacktrace) {
+ if (MemTrackerLimiter::_enable_print_log_process_usage) {
+ MemTrackerLimiter::_enable_print_log_process_usage = false;
+ std::string detail = msg;
+ detail += "\n " + MemTrackerLimiter::process_mem_log_str();
+ if (with_stacktrace) detail += "\n" + get_stack_trace();
+ std::vector<MemTracker::Snapshot> snapshots;
+ MemTrackerLimiter::make_process_snapshots(&snapshots);
+ MemTrackerLimiter::make_type_snapshots(&snapshots,
MemTrackerLimiter::Type::GLOBAL);
+ for (const auto& snapshot : snapshots) {
+ if (snapshot.parent_label == "") {
+ detail += "\n " + MemTrackerLimiter::log_usage(snapshot);
+ } else {
+ detail += "\n " + MemTracker::log_usage(snapshot);
+ }
}
+ LOG(WARNING) << detail;
}
- LOG(WARNING) << detail;
- // LOG(WARNING) <<
boost::stacktrace::to_string(boost::stacktrace::stacktrace()); // TODO
}
std::string MemTrackerLimiter::mem_limit_exceeded(const std::string& msg,
diff --git a/be/src/runtime/memory/mem_tracker_limiter.h
b/be/src/runtime/memory/mem_tracker_limiter.h
index 26489075da..c1c631e3ce 100644
--- a/be/src/runtime/memory/mem_tracker_limiter.h
+++ b/be/src/runtime/memory/mem_tracker_limiter.h
@@ -91,7 +91,7 @@ public:
// TODO: In order to ensure no OOM, currently reserve 200M, and then
use the free mem in /proc/meminfo to ensure no OOM.
if (MemInfo::proc_mem_no_allocator_cache() + bytes >=
MemInfo::mem_limit() ||
PerfCounters::get_vm_rss() + bytes >= MemInfo::hard_mem_limit()) {
- print_log_process_usage("sys_mem_exceed_limit_check");
+ print_log_process_usage("sys mem exceed limit check faild");
return true;
}
return false;
@@ -131,7 +131,7 @@ public:
void print_log_usage(const std::string& msg);
void enable_print_log_usage() { _enable_print_log_usage = true; }
static void enable_print_log_process_usage() {
_enable_print_log_process_usage = true; }
- static void print_log_process_usage(const std::string& msg);
+ static void print_log_process_usage(const std::string& msg, bool
with_stacktrace = true);
// Log the memory usage when memory limit is exceeded.
std::string mem_limit_exceeded(const std::string& msg,
diff --git a/be/src/runtime/thread_context.h b/be/src/runtime/thread_context.h
index 68f09ce085..bc422e5268 100644
--- a/be/src/runtime/thread_context.h
+++ b/be/src/runtime/thread_context.h
@@ -386,5 +386,7 @@ private:
#define TRY_CONSUME_MEM_TRACKER(size) (void)0
#define RELEASE_MEM_TRACKER(size) (void)0
#define TRY_RELEASE_MEM_TRACKER(size) (void)0
+#define RETURN_IF_CATCH_BAD_ALLOC(stmt) \
+ { stmt; }
#endif
} // namespace doris
diff --git a/be/src/service/doris_main.cpp b/be/src/service/doris_main.cpp
index 5d660a685e..b193f0d8a4 100644
--- a/be/src/service/doris_main.cpp
+++ b/be/src/service/doris_main.cpp
@@ -506,7 +506,7 @@ int main(int argc, char** argv) {
doris::MemInfo::refresh_allocator_mem();
#endif
if (doris::config::memory_debug) {
- doris::MemTrackerLimiter::print_log_process_usage("memory_debug");
+ doris::MemTrackerLimiter::print_log_process_usage("memory_debug",
false);
}
doris::MemTrackerLimiter::enable_print_log_process_usage();
sleep(1);
diff --git a/be/src/util/stack_util.cpp b/be/src/util/stack_util.cpp
index d212c98223..ccb3785c4c 100644
--- a/be/src/util/stack_util.cpp
+++ b/be/src/util/stack_util.cpp
@@ -25,6 +25,11 @@ void DumpStackTraceToString(std::string* stacktrace);
namespace doris {
+// `boost::stacktrace::stacktrace()` has memory leak, so use the glog internal
func to print stacktrace.
+// The reason for the boost::stacktrace memory leak is that a state is saved
in the thread local of each
+// thread but is not actively released. Refer to:
+// https://github.com/boostorg/stacktrace/issues/118
+// https://github.com/boostorg/stacktrace/issues/111
std::string get_stack_trace() {
std::string s;
google::glog_internal_namespace_::DumpStackTraceToString(&s);
diff --git a/be/src/vec/common/allocator.h b/be/src/vec/common/allocator.h
index c1c7e81297..423f1eb4db 100644
--- a/be/src/vec/common/allocator.h
+++ b/be/src/vec/common/allocator.h
@@ -102,12 +102,11 @@ static constexpr size_t CHUNK_THRESHOLD = 1024;
static constexpr size_t MMAP_MIN_ALIGNMENT = 4096;
static constexpr size_t MALLOC_MIN_ALIGNMENT = 8;
-#define RETURN_BAD_ALLOC(err) \
- do { \
- LOG(ERROR) << err; \
- if (doris::enable_thread_cache_bad_alloc) throw std::bad_alloc {}; \
- doris::MemTrackerLimiter::print_log_process_usage(err); \
- return nullptr; \
+#define RETURN_BAD_ALLOC(err) \
+ do { \
+ if (!doris::enable_thread_cache_bad_alloc) \
+ doris::MemTrackerLimiter::print_log_process_usage(err); \
+ throw std::bad_alloc {}; \
} while (0)
/** Responsible for allocating / freeing memory. Used, for example, in
PODArray, Arena.
@@ -181,8 +180,9 @@ public:
if (0 != munmap(buf, size)) {
auto err = fmt::format("Allocator: Cannot munmap {}.", size);
LOG(ERROR) << err;
- if (doris::enable_thread_cache_bad_alloc) throw std::bad_alloc
{};
- doris::MemTrackerLimiter::print_log_process_usage(err);
+ if (!doris::enable_thread_cache_bad_alloc)
+ doris::MemTrackerLimiter::print_log_process_usage(err);
+ throw std::bad_alloc {};
} else {
RELEASE_THREAD_MEM_TRACKER(size);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
