lingbin commented on a change in pull request #2704: Remove resource_info 
related members from TaskWorkerPool
URL: https://github.com/apache/incubator-doris/pull/2704#discussion_r365509761
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -211,84 +190,55 @@ void TaskWorkerPool::start() {
 }
 
 void TaskWorkerPool::submit_task(const TAgentTaskRequest& task) {
-    // Submit task to dequeue
-    TTaskType::type task_type = task.task_type;
+    const TTaskType::type task_type = task.task_type;
     int64_t signature = task.signature;
-    string user("");
-    if (task.__isset.resource_info) {
-        user = task.resource_info.user;
-    }
-    bool ret = _record_task_info(task_type, signature, user);
-    if (ret == true) {
-        lock_guard<Mutex> worker_thread_lock(_worker_thread_lock);
-        // set the task receive time
+
+    std::string type_str;
+    EnumToString(TTaskType, task_type, type_str);
+
+    if (_check_task_info(task_type, signature)) {
+        // Set the receiving time of task so that we can determine whether it 
is timed out later
         (const_cast<TAgentTaskRequest&>(task)).__set_recv_time(time(nullptr));
+        lock_guard<Mutex> worker_thread_lock(_worker_thread_lock);
         _tasks.push_back(task);
         _worker_thread_condition_lock.notify();
-    }
-}
-
-bool TaskWorkerPool::_record_task_info(
-        const TTaskType::type task_type,
-        int64_t signature,
-        const string& user) {
-    bool ret = true;
-    lock_guard<Mutex> task_signatures_lock(_s_task_signatures_lock);
-
-    set<int64_t>& signature_set = _s_task_signatures[task_type];
-    std::string task_name;
-    EnumToString(TTaskType, task_type, task_name);
-    if (signature_set.count(signature) > 0) {
-        LOG(INFO) << "type: " << task_name
-                  << ", signature: " << signature << ", already exist"
-                  << ". queue size: " << signature_set.size();
-        ret = false;
+        LOG(INFO) << "success to submit task. type=" << type_str
 
 Review comment:
   Actually, it will not out of order because it is in a lock. But you remind 
me that we should not print logs in the lock here.
   Should be modified to:
   ```
   {
       lock_gurad ();
       xxx.notify ()
   }
   LOG (...);
   ```
   
   As for the order problem, we can print one log at the beginning of the 
function, which can help us to check the order when tracing problems. 
   
   In addition, we can print the total number of current tasks at there too.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

Reply via email to