You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "github-actions[bot] (via GitHub)" <gi...@apache.org> on 2023/04/15 01:17:27 UTC

[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18691: [enhancement] make agent batch tasks cancelable in BE

github-actions[bot] commented on code in PR #18691:
URL: https://github.com/apache/doris/pull/18691#discussion_r1167352307


##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -592,12 +602,33 @@ void TaskWorkerPool::_alter_tablet_worker_thread_callback() {
         TAgentTaskRequest agent_task_req;
         {
             std::unique_lock<std::mutex> worker_thread_lock(_worker_thread_lock);
+            agent_task_req = _tasks.front();
+            {
+                // check if the task has been removed
+                std::unique_lock<std::mutex> cancel_lock(_cancel_lock);
+                auto it = _cancel_set.find(agent_task_req.batch_id);
+                if (it != _cancel_set.end()) {
+                    _tasks.pop_front();
+                    _remove_task_info(agent_task_req.task_type, agent_task_req.signature);
+                    LOG(INFO) << "alter table task is successfully canceled, signature: "
+                              << agent_task_req.signature;
+                    // because cancel behavior is relatively rare,
+                    // it does no harm that we do this scaning check here
+                    if (std::count_if(_tasks.begin(), _tasks.end(),
+                                      [this](const TAgentTaskRequest& req) {

Review Comment:
   warning: lambda capture 'this' is not used [clang-diagnostic-unused-lambda-capture]
   
   ```suggestion
                                         [](const TAgentTaskRequest& req) {
   ```
   



##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -592,12 +602,33 @@
         TAgentTaskRequest agent_task_req;
         {
             std::unique_lock<std::mutex> worker_thread_lock(_worker_thread_lock);
+            agent_task_req = _tasks.front();
+            {
+                // check if the task has been removed
+                std::unique_lock<std::mutex> cancel_lock(_cancel_lock);
+                auto it = _cancel_set.find(agent_task_req.batch_id);
+                if (it != _cancel_set.end()) {
+                    _tasks.pop_front();
+                    _remove_task_info(agent_task_req.task_type, agent_task_req.signature);
+                    LOG(INFO) << "alter table task is successfully canceled, signature: "
+                              << agent_task_req.signature;
+                    // because cancel behavior is relatively rare,
+                    // it does no harm that we do this scaning check here
+                    if (std::count_if(_tasks.begin(), _tasks.end(),
+                                      [this](const TAgentTaskRequest& req) {
+                                          return req.batch_id == agent_task_req.batch_id;

Review Comment:
   warning: variable 'agent_task_req' cannot be implicitly captured in a lambda with no capture-default specified [clang-diagnostic-error]
   ```cpp
                                             return req.batch_id == agent_task_req.batch_id;
                                                                    ^
   ```
   **be/src/agent/task_worker_pool.cpp:601:** 'agent_task_req' declared here
   ```cpp
           TAgentTaskRequest agent_task_req;
                             ^
   ```
   **be/src/agent/task_worker_pool.cpp:617:** lambda expression begins here
   ```cpp
                                         [this](const TAgentTaskRequest& req) {
                                         ^
   ```
   **be/src/agent/task_worker_pool.cpp:617:** capture 'agent_task_req' by value
   ```cpp
                                         [this](const TAgentTaskRequest& req) {
                                              ^
   ```
   **be/src/agent/task_worker_pool.cpp:617:** capture 'agent_task_req' by reference
   ```cpp
                                         [this](const TAgentTaskRequest& req) {
                                              ^
   ```
   **be/src/agent/task_worker_pool.cpp:617:** default capture by reference
   ```cpp
                                         [this](const TAgentTaskRequest& req) {
                                          ^
   ```
   



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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