changgyoopark-db commented on code in PR #49370:
URL: https://github.com/apache/spark/pull/49370#discussion_r1914664648
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteHolder.scala:
##########
@@ -126,6 +126,18 @@ private[connect] class ExecuteHolder(
runner.start()
}
+ /**
+ * Check if the execution was ended without finalizing the outcome and no
further progress will
+ * be made. If the execution was delegated, this method always returns false.
+ */
+ def isOrphan(): Boolean = {
+ // Check runner.completed() before others as the acquire memory fence in
the method ensures the
+ // current thread to read the last known state of responseObserver
correctly.
+ runner.completed() &&
+ !runner.shouldDelegateCompleteResponse(request) &&
+ !responseObserver.completed()
+ }
Review Comment:
Putting something there (the finally block of ExecuteThreadRunner) is not
100% safe since creating an exception can cause OOM errors; it's also a heap
allocation. That's why I would like to stick to handling the situation in the
reattach handler.
* OOM there (ExecuteThreadRunner) is unrecoverable.
* Reattach->check will eventually succeed if the JVM manages to spare some
memory.
--
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
