ozankabak commented on code in PR #15030: URL: https://github.com/apache/datafusion/pull/15030#discussion_r1985285516
########## datafusion/physical-plan/src/execution_plan.rs: ########## @@ -260,13 +260,28 @@ 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 + /// block the CPU and must yield back to the tokio runtime regularly. + /// [`ExecutionPlan`] implementations should follow [the guideline of not + /// spending a long time without reaching an `await`/yield point][async-guideline]. + /// This can be achieved by manually returning [`Poll::Pending`] in regular + /// intervals, or the use of [`tokio::task::yield_now()`] (as appropriate). + /// Determination for "regularly" may be made using a timer (being careful + /// with the overhead-heavy syscall needed to take the time) or by + /// counting rows or batches. + /// + /// The cancellation benchmark tracks some cases of how quickly queries can + /// be cancelled. Review Comment: ```suggestion /// To enable timely cancellation, the [`Stream`] that is returned must not /// block the CPU indefinitely and must yield back to the tokio runtime regularly. /// In a typical [`ExecutionPlan`], this automatically happens unless there are /// special circumstances; e.g. when the computational complexity of processing a /// batch is superlinear. See this [general guideline][async-guideline] for more context /// on this point, which explains why one should avoid spending a long time without /// reaching an `await`/yield point in asynchronous runtimes. /// This can be achieved by manually returning [`Poll::Pending`] and setting up wakers appropriately, or the use of [`tokio::task::yield_now()`] when appropriate. /// In special cases that warrant manual yielding, determination for "regularly" may be /// made using a timer (being careful with the overhead-heavy system call needed to /// take the time), or by counting rows or batches. /// /// The cancellation benchmark tracks some cases of how quickly queries can /// be cancelled. ``` -- 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