[GitHub] [spark] vanzin commented on a change in pull request #23939: [SPARK-27019] [webUI]onJobStart happens after onExecutionEnd shouldn't overwrite kvstore

[GitBox <gi...@apache.org>]

vanzin commented on a change in pull request #23939: [SPARK-27019] [webUI]onJobStart happens after onExecutionEnd shouldn't overwrite kvstore
URL: https://github.com/apache/spark/pull/23939#discussion_r262207103
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala
 ##########
 @@ -77,8 +77,28 @@ class SQLAppStatusListener(
 
     val executionId = executionIdString.toLong
     val jobId = event.jobId
+    val isExecNotExists = liveExecutions.get(executionId) == null
     val exec = getOrCreateExecution(executionId)
 
+    if (isExecNotExists) {
+      val sqlStoreData =
 
 Review comment:
   There are a few issues.
   
   `Some(anything).nonEmpty` will always be true.
   
   If `isExecNotExists` is true but the execution is not in the kvstore, then you'll get an exception. Not sure that can actually happen, but looking at just this code, you'd get an error in that situation.
   
   You need something like:
   
   ```
   val exec = Option(liveExecutions.get(executionId))
     .orElse {
       try {
         Some(kvstore.read(...))
       } catch {
         case NoSuchElement => None
       }
     }.getOrElse(getOrCreateExecution(executionId))
   ```
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org