Pipeline: use notify instead of polling for AggregateRestoreSourceOp
#9082
Conversation
ti-chi-bot
bot
added
release-note-none
|
/cc @windtalker @gengliqi |
| @@ -66,7 +82,7 @@ void SharedSpilledBucketDataLoader::storeBucketData() | |||
| RUNTIME_CHECK(status == SharedLoaderStatus::loading); | |||
|
|
|||
| // Although the status will always stay at `SharedLoaderStatus::loading`, but because the query has stopped, it doesn't matter. | |||
| if unlikely (exec_context.isCancelled()) | |||
| if (checkCancelled()) | |||
There was a problem hiding this comment.
Why SharedQuery and ResultQueue need to be cancelled explicitly in PipelineExecutorContext::cancel, but this don't need to be cancelled in PipelineExecutorContext::cancel?
There was a problem hiding this comment.
Because it will not be stuck forever, when the query is canceled, checkCancel will always be called, because wait_for_notify must be because the queue is empty. When the queue is empty, it will trigger an asynchronous load bucket event, and the load bucket event will call checkCancel.
There was a problem hiding this comment.
If trigger an asynchronous load bucket event fails, will it still work?
There was a problem hiding this comment.
yes, Event::finishImpl is always called, regardless of failure or cancellation
There was a problem hiding this comment.
OK. I want to confirm another unrelated question here, why WorkQueue in this pr don't need to be cancelled explicitly in PipelineExecutorContext::cancel?
There was a problem hiding this comment.
When query meet error, the storage layer is not informed?
There was a problem hiding this comment.
Yes, the storage layer is not explicitly canceled.
There was a problem hiding this comment.
However, the query will not be stuck, because the storage layer will continuously generate blocks, which will trigger checkCancel, thus ending the query.
There was a problem hiding this comment.
Storage layer generate block => pipeline task is notified and finishes since it found the query is cancelled => ~UnorderedSourceOp() is called, and in side ~UnorderedSourceOp(), it will call task_pool->decreaseUnorderedInputStreamRefCount(), which eventually stop the storage read?
There was a problem hiding this comment.
Storage layer generate block => pipeline task is notified and finishes since it found the query is cancelled =>
~UnorderedSourceOp()is called, and in side~UnorderedSourceOp(), it will calltask_pool->decreaseUnorderedInputStreamRefCount(), which eventually stop the storage read?
yes
ti-chi-bot
bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gengliqi, windtalker The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
What problem does this PR solve?
Issue Number: ref #8869
Problem Summary:
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note