ivoson commented on a change in pull request #33905:
URL: https://github.com/apache/spark/pull/33905#discussion_r703428815
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Observation.scala
##########
@@ -150,10 +150,12 @@ class Observation(name: String) {
private[sql] case class ObservationListener(observation: Observation)
extends QueryExecutionListener {
- override def onSuccess(funcName: String, qe: QueryExecution, durationNs:
Long): Unit =
+ override def onSuccess(
+ funcName: String, executionId: Long, qe: QueryExecution, durationNs:
Long): Unit =
Review comment:
If we want to unify these two, I think the id bound to a QueryExecution
should be the same with query execution id we can get from UI now. So the
timing to initialize the id might also need to be the same.
Since a QueryExecution will be created for each dataset, we can not
initialize the id for every QueryExecution, but just for the ones triggered by
an action. I think the logic and timing here is just like what we did in
SQLExecution.nextExecutionId.
In another word, we can just put the query execution Id to a QueryExecution
when newExecutionId generated. This is what I have in mind now.
--
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]
