morningman commented on a change in pull request #8476:
URL: https://github.com/apache/incubator-doris/pull/8476#discussion_r826972583



##########
File path: be/src/exec/base_scanner.cpp
##########
@@ -39,14 +39,7 @@ BaseScanner::BaseScanner(RuntimeState* state, 
RuntimeProfile* profile,
           _counter(counter),
           _src_tuple(nullptr),
           _src_tuple_row(nullptr),
-#if BE_TEST
-          _mem_tracker(new MemTracker()),
-#else
-          _mem_tracker(MemTracker::create_tracker(
-                  -1, "BaseScanner:" + std::to_string(state->load_job_id()),
-                  state->instance_mem_tracker())),
-#endif
-          _mem_pool(_mem_tracker.get()),
+          _mem_pool("BaseScanner:" + std::to_string(state->load_job_id())),

Review comment:
       This may not be a load job, so there is no `load_job_id`

##########
File path: be/src/exec/tablet_info.cpp
##########
@@ -416,7 +416,7 @@ 
VOlapTablePartitionParam::VOlapTablePartitionParam(std::shared_ptr<OlapTableSche
         : _schema(schema),
           _t_param(t_param),
           _slots(_schema->tuple_desc()->slots()),
-          _mem_tracker(MemTracker::create_tracker(-1, 
"OlapTablePartitionParam")) {
+          _mem_tracker(MemTracker::create_virtual_tracker(-1, 
"OlapTablePartitionParam")) {

Review comment:
       When should I create a virtual memtracker?

##########
File path: be/src/exec/tablet_sink.cpp
##########
@@ -48,6 +49,7 @@ NodeChannel::NodeChannel(OlapTableSink* parent, IndexChannel* 
index_channel, int
     if (_parent->_transfer_data_by_brpc_attachment) {
         _tuple_data_buffer_ptr = &_tuple_data_buffer;
     }
+    _node_channel_tracker = MemTracker::create_tracker(-1, "NodeChannel");

Review comment:
       Add BE id to the tracker's name

##########
File path: be/src/exprs/agg_fn_evaluator.cpp
##########
@@ -154,7 +154,7 @@ Status AggFnEvaluator::prepare(RuntimeState* state, const 
RowDescriptor& desc, M
     _intermediate_slot_desc = intermediate_slot_desc;
 
     _string_buffer_len = 0;
-    _mem_tracker = mem_tracker;
+    _mem_tracker = MemTracker::create_virtual_tracker(-1, "AggFnEvaluator", 
mem_tracker);

Review comment:
       The `memtracker` param of `AggFnEvaluator::prepare` can be removed.

##########
File path: be/src/exec/es_http_scanner.cpp
##########
@@ -43,14 +42,7 @@ EsHttpScanner::EsHttpScanner(RuntimeState* state, 
RuntimeProfile* profile, Tuple
           _next_range(0),
           _line_eof(false),
           _batch_eof(false),
-#if BE_TEST
-          _mem_tracker(new MemTracker()),
-#else
-          _mem_tracker(
-                  MemTracker::create_tracker(-1, "EsHttpScanner:" + 
std::to_string(state->load_job_id()),
-                                            state->instance_mem_tracker())),
-#endif
-          _mem_pool(_mem_tracker.get()),
+          _mem_pool("EsHttpScanner:" + std::to_string(state->load_job_id())),

Review comment:
       no `load_job_id`

##########
File path: be/src/olap/memtable.cpp
##########
@@ -31,14 +31,13 @@ namespace doris {
 
 MemTable::MemTable(int64_t tablet_id, Schema* schema, const TabletSchema* 
tablet_schema,
                    const std::vector<SlotDescriptor*>* slot_descs, 
TupleDescriptor* tuple_desc,
-                   KeysType keys_type, RowsetWriter* rowset_writer,
-                   const std::shared_ptr<MemTracker>& parent_tracker)
+                   KeysType keys_type, RowsetWriter* rowset_writer)
         : _tablet_id(tablet_id),
           _schema(schema),
           _tablet_schema(tablet_schema),
           _slot_descs(slot_descs),
           _keys_type(keys_type),
-          _mem_tracker(MemTracker::create_tracker(-1, "MemTable", 
parent_tracker)),
+          _mem_tracker(MemTracker::create_tracker(-1, "MemTable")),

Review comment:
       when to use `create_tracker` and when to user `create_virtual_tracker`?

##########
File path: be/src/runtime/dpp_sink.cpp
##########
@@ -0,0 +1,926 @@
+// Licensed to the Apache Software Foundation (ASF) under one

Review comment:
       A new file?

##########
File path: be/src/olap/snapshot_manager.h
##########
@@ -99,6 +102,9 @@ class SnapshotManager {
     // snapshot
     Mutex _snapshot_mutex;
     uint64_t _snapshot_base_id;
+
+    // TODO(zxy) used after
+    std::shared_ptr<MemTracker> _mem_tracker = nullptr;

Review comment:
       What is this for?

##########
File path: be/src/olap/rowset/segment_v2/segment.cpp
##########
@@ -51,10 +51,10 @@ Segment::Segment(const FilePathDesc& path_desc, uint32_t 
segment_id,
                  const TabletSchema* tablet_schema)
         : _path_desc(path_desc), _segment_id(segment_id), 
_tablet_schema(tablet_schema) {
 #ifndef BE_TEST
-    _mem_tracker = MemTracker::create_tracker(
+    _mem_tracker = MemTracker::create_virtual_tracker(

Review comment:
       why using virtual memtracker?

##########
File path: be/src/runtime/row_batch.cpp
##########
@@ -39,8 +40,8 @@ namespace doris {
 const int RowBatch::AT_CAPACITY_MEM_USAGE = 8 * 1024 * 1024;
 const int RowBatch::FIXED_LEN_BUFFER_LIMIT = AT_CAPACITY_MEM_USAGE / 2;
 
-RowBatch::RowBatch(const RowDescriptor& row_desc, int capacity, MemTracker* 
mem_tracker)
-        : _mem_tracker(mem_tracker),
+RowBatch::RowBatch(const RowDescriptor& row_desc, int capacity)
+        : 
_mem_tracker(thread_local_ctx.get()->_thread_mem_tracker_mgr->mem_tracker()),

Review comment:
       Is it possible that this rowbatch is transferred from one thread to 
another.
   If so, the _mem_tracker also need to be changed?

##########
File path: be/src/olap/tablet_manager.cpp
##########
@@ -73,7 +73,7 @@ static bool _cmp_tablet_by_create_time(const TabletSharedPtr& 
a, const TabletSha
 }
 
 TabletManager::TabletManager(int32_t tablet_map_lock_shard_size)
-        : _mem_tracker(MemTracker::create_tracker(-1, "TabletManager", nullptr,
+        : _mem_tracker(MemTracker::create_virtual_tracker(-1, "TabletManager", 
nullptr,

Review comment:
       Why virtual?

##########
File path: be/src/olap/task/engine_alter_tablet_task.cpp
##########
@@ -25,7 +25,14 @@ namespace doris {
 using std::to_string;
 
 EngineAlterTabletTask::EngineAlterTabletTask(const TAlterTabletReqV2& request)
-        : _alter_tablet_req(request) {}
+        : _alter_tablet_req(request) {
+    _mem_tracker = MemTracker::create_tracker(

Review comment:
       What's for?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to