freemandealer commented on code in PR #26544:
URL: https://github.com/apache/doris/pull/26544#discussion_r1384928386


##########
be/src/olap/memtable_memory_limiter.cpp:
##########
@@ -27,6 +27,7 @@ namespace doris {
 DEFINE_GAUGE_METRIC_PROTOTYPE_5ARG(memtable_memory_limiter_mem_consumption, 
MetricUnit::BYTES, "",
                                    memtable_memory_limiter_mem_consumption,
                                    Labels({{"type", "load"}}));
+bvar::Adder<int64_t> g_flush_async_num("memtable_flush_async_num");

Review Comment:
   what knowledge can we get from this bvar?



##########
be/src/olap/memtable_flush_executor.cpp:
##########
@@ -122,7 +131,8 @@ Status FlushToken::_do_flush_memtable(MemTable* memtable, 
int32_t segment_id, in
     return Status::OK();
 }
 
-void FlushToken::_flush_memtable(MemTable* memtable, int32_t segment_id, 
int64_t submit_task_time) {
+void FlushToken::_flush_memtable(MemTable* mem_table, int32_t segment_id,

Review Comment:
   renaming to mem_table does not make any sense



##########
be/src/olap/memtable_flush_executor.cpp:
##########
@@ -189,26 +200,38 @@ Status 
MemTableFlushExecutor::create_flush_token(std::unique_ptr<FlushToken>& fl
     if (!is_high_priority) {
         if (rowset_writer->type() == BETA_ROWSET && !should_serial) {
             // beta rowset can be flush in CONCURRENT, because each memtable 
using a new segment writer.
-            flush_token.reset(
-                    new 
FlushToken(_flush_pool->new_token(ThreadPool::ExecutionMode::CONCURRENT)));
+            flush_token = std::make_unique<FlushToken>(
+                    
_flush_pool->new_token(ThreadPool::ExecutionMode::CONCURRENT));
         } else {
             // alpha rowset do not support flush in CONCURRENT.
-            flush_token.reset(
-                    new 
FlushToken(_flush_pool->new_token(ThreadPool::ExecutionMode::SERIAL)));
+            flush_token = std::make_unique<FlushToken>(
+                    _flush_pool->new_token(ThreadPool::ExecutionMode::SERIAL));
         }
     } else {
         if (rowset_writer->type() == BETA_ROWSET && !should_serial) {
             // beta rowset can be flush in CONCURRENT, because each memtable 
using a new segment writer.
-            flush_token.reset(new FlushToken(
-                    
_high_prio_flush_pool->new_token(ThreadPool::ExecutionMode::CONCURRENT)));
+            flush_token = std::make_unique<FlushToken>(
+                    
_high_prio_flush_pool->new_token(ThreadPool::ExecutionMode::CONCURRENT));
         } else {
             // alpha rowset do not support flush in CONCURRENT.
-            flush_token.reset(new FlushToken(
-                    
_high_prio_flush_pool->new_token(ThreadPool::ExecutionMode::SERIAL)));
+            flush_token = std::make_unique<FlushToken>(
+                    
_high_prio_flush_pool->new_token(ThreadPool::ExecutionMode::SERIAL));
         }
     }
     flush_token->set_rowset_writer(rowset_writer);
     return Status::OK();
 }
 
+void MemTableFlushExecutor::_register_metrics() {
+    REGISTER_HOOK_METRIC(flush_thread_pool_queue_size,
+                         [this]() { return _flush_pool->get_queue_size(); });

Review Comment:
   we prefer an unsafe & inaccurate queue size to accurate one with heavy lock 
overhead.



-- 
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