You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "ZhangYu0123 (via GitHub)" <gi...@apache.org> on 2023/06/23 03:39:39 UTC

[GitHub] [doris] ZhangYu0123 opened a new pull request, #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

ZhangYu0123 opened a new pull request, #21102:
URL: https://github.com/apache/doris/pull/21102

   ## Proposed changes
   1.  fix MultiCoreTaskQueue steal take core_id assgin error
   stealed pipeline_task will run on core_id thread not previous next_id thread, so the pipeline_task will use core_id queue runtime  and should be assginned core_id. 
   
   
   Issue Number: close #xxx
   
   <!--Describe your changes.-->
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


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


[GitHub] [doris] HappenLee commented on a diff in pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "HappenLee (via GitHub)" <gi...@apache.org>.
HappenLee commented on code in PR #21102:
URL: https://github.com/apache/doris/pull/21102#discussion_r1241137719


##########
be/src/pipeline/task_queue.cpp:
##########
@@ -182,7 +182,7 @@ PipelineTask* MultiCoreTaskQueue::_steal_take(size_t core_id) {
         DCHECK(next_id < _core_size);
         auto task = _prio_task_queue_list[next_id].try_take(true);
         if (task) {
-            task->set_core_id(next_id);

Review Comment:
   need change the origin core id seems right?



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


[GitHub] [doris] ZhangYu0123 commented on pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "ZhangYu0123 (via GitHub)" <gi...@apache.org>.
ZhangYu0123 commented on PR #21102:
URL: https://github.com/apache/doris/pull/21102#issuecomment-1603640748

   run buildall


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


[GitHub] [doris] ZhangYu0123 commented on pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "ZhangYu0123 (via GitHub)" <gi...@apache.org>.
ZhangYu0123 commented on PR #21102:
URL: https://github.com/apache/doris/pull/21102#issuecomment-1603887333

   run p1


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


[GitHub] [doris] ZhangYu0123 closed pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "ZhangYu0123 (via GitHub)" <gi...@apache.org>.
ZhangYu0123 closed pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error
URL: https://github.com/apache/doris/pull/21102


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


[GitHub] [doris] ZhangYu0123 commented on a diff in pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "ZhangYu0123 (via GitHub)" <gi...@apache.org>.
ZhangYu0123 commented on code in PR #21102:
URL: https://github.com/apache/doris/pull/21102#discussion_r1241220458


##########
be/src/pipeline/task_queue.cpp:
##########
@@ -182,7 +182,7 @@ PipelineTask* MultiCoreTaskQueue::_steal_take(size_t core_id) {
         DCHECK(next_id < _core_size);
         auto task = _prio_task_queue_list[next_id].try_take(true);
         if (task) {
-            task->set_core_id(next_id);

Review Comment:
   > need change the origin core id seems right?
   
   I think core_id of pipeline task means which core thread the task belong to and which multi core task queue the task belong to.  On the other hand,  previous_core_id of pipeline task means the origin core id.
   So the task is stealed from next_id to the core_id, it should be the core_id.



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


[GitHub] [doris] github-actions[bot] commented on pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21102:
URL: https://github.com/apache/doris/pull/21102#issuecomment-1603642767

   clang-tidy review says "All clean, LGTM! :+1:"


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


[GitHub] [doris] ZhangYu0123 commented on a diff in pull request #21102: [fix](pipeline) fix MultiCoreTaskQueue steal take core_id assgin error

Posted by "ZhangYu0123 (via GitHub)" <gi...@apache.org>.
ZhangYu0123 commented on code in PR #21102:
URL: https://github.com/apache/doris/pull/21102#discussion_r1241220458


##########
be/src/pipeline/task_queue.cpp:
##########
@@ -182,7 +182,7 @@ PipelineTask* MultiCoreTaskQueue::_steal_take(size_t core_id) {
         DCHECK(next_id < _core_size);
         auto task = _prio_task_queue_list[next_id].try_take(true);
         if (task) {
-            task->set_core_id(next_id);

Review Comment:
   > need change the origin core id seems right?
   
   I think core_id of pipeline task means which core thread the task belongs to and which multi core task queue the task belongs to.  On the other hand,  previous_core_id of pipeline task means the origin core id.
   So the task is stealed from next_id to the core_id, it should be the core_id.



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