hvanhovell commented on code in PR #48208:
URL: https://github.com/apache/spark/pull/48208#discussion_r1771661488
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/execution/ExecuteThreadRunner.scala:
##########
@@ -68,10 +68,19 @@ private[connect] class ExecuteThreadRunner(executeHolder:
ExecuteHolder) extends
}
/**
- * Register a callback that gets executed after completion/interruption of
the execution thread.
+ * Execute the supplied lambda if no execute runner thread has started, or
pass it to the
+ * execute runner thread so that the lambda will get executed on completion.
Failure details
+ * will only be passed to the lambda only when an error was raised during
actual execution.
*/
- private[connect] def processOnCompletion(callback: Try[Unit] => Unit): Unit
= {
-
promise.future.onComplete(callback)(ExecuteThreadRunner.namedExecutionContext)
+ private[connect] def processNowOrOnCompletion(callback: Try[Unit] => Unit):
Unit = {
+ lock.synchronized {
+ if (started) {
+
promise.future.onComplete(callback)(ExecuteThreadRunner.namedExecutionContext)
+ } else {
+ // No thread has been spawned, and therefore there was no failure,
passing `Success`.
+ callback(Success(()))
Review Comment:
Does this callback have to be executed within the lock?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
