You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2022/10/21 00:30:37 UTC
[doris] branch master updated: [fix](memtracker) Fix transmit_tracker null pointer because phamp is not thread safe #13528
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 736d113700 [fix](memtracker) Fix transmit_tracker null pointer because phamp is not thread safe #13528
736d113700 is described below
commit 736d11370016634a06d3ad1c060602ea248592db
Author: Xinyi Zou <zo...@gmail.com>
AuthorDate: Fri Oct 21 08:30:30 2022 +0800
[fix](memtracker) Fix transmit_tracker null pointer because phamp is not thread safe #13528
---
be/src/runtime/memory/mem_tracker_task_pool.h | 2 ++
be/src/service/internal_service.cpp | 11 +++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/be/src/runtime/memory/mem_tracker_task_pool.h b/be/src/runtime/memory/mem_tracker_task_pool.h
index 4d3a514f6d..9e5813ba03 100644
--- a/be/src/runtime/memory/mem_tracker_task_pool.h
+++ b/be/src/runtime/memory/mem_tracker_task_pool.h
@@ -23,6 +23,8 @@
namespace doris {
+// TODO: phmap `parallel_flat_hash_map` is not thread-safe. If it is not fixed in the future,
+// can consider using other maps instead.
using TaskTrackersMap = phmap::parallel_flat_hash_map<
std::string, std::shared_ptr<MemTrackerLimiter>,
phmap::priv::hash_default_hash<std::string>, phmap::priv::hash_default_eq<std::string>,
diff --git a/be/src/service/internal_service.cpp b/be/src/service/internal_service.cpp
index a81c97bc20..462abf2e97 100644
--- a/be/src/service/internal_service.cpp
+++ b/be/src/service/internal_service.cpp
@@ -127,14 +127,15 @@ void PInternalServiceImpl::_transmit_data(google::protobuf::RpcController* cntl_
const Status& extract_st) {
std::string query_id;
TUniqueId finst_id;
- std::shared_ptr<MemTrackerLimiter> transmit_tracker;
+ std::shared_ptr<MemTrackerLimiter> transmit_tracker = nullptr;
if (request->has_query_id()) {
query_id = print_id(request->query_id());
finst_id.__set_hi(request->finst_id().hi());
finst_id.__set_lo(request->finst_id().lo());
transmit_tracker =
_exec_env->task_pool_mem_tracker_registry()->get_task_mem_tracker(query_id);
- } else {
+ }
+ if (!transmit_tracker) {
query_id = "unkown_transmit_data";
transmit_tracker = std::make_shared<MemTrackerLimiter>(-1, "unkown_transmit_data");
}
@@ -635,14 +636,16 @@ void PInternalServiceImpl::_transmit_block(google::protobuf::RpcController* cntl
const Status& extract_st) {
std::string query_id;
TUniqueId finst_id;
- std::shared_ptr<MemTrackerLimiter> transmit_tracker;
+ std::shared_ptr<MemTrackerLimiter> transmit_tracker = nullptr;
if (request->has_query_id()) {
query_id = print_id(request->query_id());
finst_id.__set_hi(request->finst_id().hi());
finst_id.__set_lo(request->finst_id().lo());
+ // phmap `parallel_flat_hash_map` is not thread safe, so get query mem tracker may be null pointer.
transmit_tracker =
_exec_env->task_pool_mem_tracker_registry()->get_task_mem_tracker(query_id);
- } else {
+ }
+ if (!transmit_tracker) {
query_id = "unkown_transmit_block";
transmit_tracker = std::make_shared<MemTrackerLimiter>(-1, "unkown_transmit_block");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org