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

Reply via email to