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

Reply via email to