Gabriel39 commented on code in PR #56755:
URL: https://github.com/apache/doris/pull/56755#discussion_r2418814943
##########
be/src/pipeline/pipeline_task.cpp:
##########
@@ -798,22 +800,35 @@ size_t PipelineTask::get_revocable_size() const {
}
Status PipelineTask::revoke_memory(const std::shared_ptr<SpillContext>&
spill_context) {
+ DCHECK(spill_context);
if (is_finalized()) {
- if (spill_context) {
- spill_context->on_task_finished();
- VLOG_DEBUG << "Query: " << print_id(_state->query_id()) << ",
task: " << ((void*)this)
- << " finalized";
- }
+ spill_context->on_task_finished();
+ VLOG_DEBUG << "Query: " << print_id(_state->query_id()) << ", task: "
<< ((void*)this)
+ << " finalized";
+ return Status::OK();
+ }
+
+ if (_spill_context) {
Review Comment:
`_spill_context` will never be nullptr
##########
be/src/pipeline/pipeline_task.cpp:
##########
@@ -798,22 +800,35 @@ size_t PipelineTask::get_revocable_size() const {
}
Status PipelineTask::revoke_memory(const std::shared_ptr<SpillContext>&
spill_context) {
+ DCHECK(spill_context);
if (is_finalized()) {
- if (spill_context) {
- spill_context->on_task_finished();
- VLOG_DEBUG << "Query: " << print_id(_state->query_id()) << ",
task: " << ((void*)this)
- << " finalized";
- }
+ spill_context->on_task_finished();
+ VLOG_DEBUG << "Query: " << print_id(_state->query_id()) << ", task: "
<< ((void*)this)
+ << " finalized";
+ return Status::OK();
+ }
+
+ if (_spill_context) {
Review Comment:
Another question, what the difference between _spill_context and
spill_context
##########
be/src/pipeline/pipeline_task.h:
##########
@@ -171,7 +172,10 @@ class PipelineTask : public
std::enable_shared_from_this<PipelineTask> {
Status blocked(Dependency* dependency) {
DCHECK_EQ(_blocked_dep, nullptr) << "task: " << debug_string();
- _blocked_dep = dependency;
+ {
+ std::unique_lock<std::mutex> lc(_dependency_lock);
+ _blocked_dep = dependency;
Review Comment:
If you want operations on `_blocked_dep` are atomic, you'd better to use a
atomic variable instead of a lock guarded
--
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]
