berkaysynnada commented on code in PR #15030:
URL: https://github.com/apache/datafusion/pull/15030#discussion_r1982848377
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -260,13 +260,30 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
/// used.
/// Thus, [`spawn`] is disallowed, and instead use [`SpawnedTask`].
///
+ /// To enable timely cancellation, the [`Stream`] that is returned must not
+ /// pin the CPU and must yield back to the tokio runtime regularly. This
can
Review Comment:
What do we mean by "pin the CPU" ? I believe we shouldn't explicitly return
`Poll::Pending` or call `tokio::task::yield_now()`. In the current architecture
of datafusion, we are only introduced with `Pending` results because of an IO
work. So, tasks can only yield at IO points (there are a few exceptions).
Moreover, returning just` Poll::Pending` and calling `yield_now()` does not
exactly do the same thing. Before manually returning pending, the waker should
be set. Otherwise, the task might pend forever.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
