juliuszsompolski commented on code in PR #49370:
URL: https://github.com/apache/spark/pull/49370#discussion_r1913199388
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/execution/ExecuteThreadRunner.scala:
##########
@@ -63,6 +63,16 @@ private[connect] class ExecuteThreadRunner(executeHolder:
ExecuteHolder) extends
}
}
+ /**
+ * Checks if the execution is completed.
+ *
+ * @return
+ * true if the execution is completed.
+ */
+ private[connect] def isCompleted(): Boolean = {
+ state.getAcquire() == ThreadState.completed
+ }
Review Comment:
What if the `ExecuteThreadRunner` just dies without executing the transition
to ThreadState.completed?
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/execution/ExecuteGrpcResponseSender.scala:
##########
@@ -319,7 +325,14 @@ private[connect] class ExecuteGrpcResponseSender[T <:
Message](
log"waitingForResults=${MDC(WAIT_RESULT_TIME, consumeSleep /
NANOS_PER_MILLIS.toDouble)} ms " +
log"waitingForSend=${MDC(WAIT_SEND_TIME, sendSleep /
NANOS_PER_MILLIS.toDouble)} ms")
// scalastyle:on line.size.limit
- grpcObserver.onCompleted()
+ if (executionCompleted && !sizeLimitReached) {
Review Comment:
I see, right, `executionCompleted` is a val setup even at the init of the
sender, so no concurrency changes here... and I misread the nesting of this if
in the first place, hence confusion.
But what if
deadlineReached == true
sizeLimitReached == false
executionCompleted == true
just because the client is slow and was not able to consume sizeLimit of
results within the deadline?
--
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
