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