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 tokio::task::yield_now(). In the current behavior of
datafusion, we are only introduced with Pending results because of IO work. So,
tasks only yield on IO work (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
will 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: [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]
