sven-weber-db commented on code in PR #56397:
URL: https://github.com/apache/spark/pull/56397#discussion_r3386319096
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/externalUDF/ExternalUDFExec.scala:
##########
@@ -60,10 +60,12 @@ trait ExternalUDFExec extends UnaryExecNode {
/**
* Creates a [[WorkerSession]] via [[SparkEnv#getExternalUDFDispatcher]].
- * Registers session cancellation on task failure and session termination
- * on task completion. The provided function receives the session
- * and must return the result iterator. The function may use the
- * session but MUST NOT cancel or close it.
+ * Finalizes the session on task completion (which fires on both success and
+ * failure). [[WorkerSession#close]] is the single finalizer: it fetches the
+ * `FinishResponse` if processing completed, or cancels anything still in
+ * flight and waits for the `CancelResponse`. The provided function receives
+ * the session and must return the result iterator. The function CAN but MUST
+ * NOT close the session.
Review Comment:
Can you change `The function CAN but MUST NOT close the session.` to` The
function MAY but is NOT REQUIRED to close the session`? According to comments
in a previous PR, the `CAN but MUST NOT` statement was confusing, and we
adjusted it to `The function may use the session but MUST NOT cancel or close
it.`, which, on second thought, is not 100% correct.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
