[incubator-doris] branch master updated: [fix](profile) fix some bugs about ReportProfile on BE (#7144)

[mo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git


The following commit(s) were added to refs/heads/master by this push:
     new fcd4f0b  [fix](profile) fix some bugs about ReportProfile on BE (#7144)
fcd4f0b is described below

commit fcd4f0b5c2765312609425288c8daf6ebf8e424f
Author: thinker <zc...@qq.com>
AuthorDate: Sat Nov 20 21:43:57 2021 +0800

    [fix](profile) fix some bugs about ReportProfile on BE (#7144)
    
    1. setting _report_thread_active to false is not necessary protected by _report_thread_lock, because
    _report_thread_active's type is bool, writing data is multi-threadly safety if size <= marchine word length
    
    2. report_profile thread terminates early is possiable, in the function report_profile(), while (_report_thread_active) may
    break if  _report_thread_active is false,  the thread of calling open() may be scheduled out between
    _report_thread_started_cv.wait(l) and _report_thread_active = true, we should not assume that how long time elapsed
    between a thread be scheduled twice
---
 be/src/runtime/plan_fragment_executor.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/be/src/runtime/plan_fragment_executor.cpp b/be/src/runtime/plan_fragment_executor.cpp
index 4b6280e..c2b3d52 100644
--- a/be/src/runtime/plan_fragment_executor.cpp
+++ b/be/src/runtime/plan_fragment_executor.cpp
@@ -238,7 +238,6 @@ Status PlanFragmentExecutor::open() {
         // make sure the thread started up, otherwise report_profile() might get into a race
         // with stop_report_thread()
         _report_thread_started_cv.wait(l);
-        _report_thread_active = true;
     }
     Status status = Status::OK();
     if (_runtime_state->enable_vectorized_exec()) {
@@ -346,10 +345,14 @@ void PlanFragmentExecutor::_collect_query_statistics() {
 void PlanFragmentExecutor::report_profile() {
     VLOG_FILE << "report_profile(): instance_id=" << _runtime_state->fragment_instance_id();
     DCHECK(_report_status_cb);
+
+    _report_thread_active = true;
+
     std::unique_lock<std::mutex> l(_report_thread_lock);
     // tell Open() that we started
     _report_thread_started_cv.notify_one();
 
+
     // Jitter the reporting time of remote fragments by a random amount between
     // 0 and the report_interval.  This way, the coordinator doesn't get all the
     // updates at once so its better for contention as well as smoother progress
@@ -431,10 +434,7 @@ void PlanFragmentExecutor::stop_report_thread() {
         return;
     }
 
-    {
-        std::lock_guard<std::mutex> l(_report_thread_lock);
-        _report_thread_active = false;
-    }
+    _report_thread_active = false;
 
     _stop_report_thread_cv.notify_one();
     _report_thread.join();

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org