tgravescs commented on a change in pull request #28094:
URL: https://github.com/apache/spark/pull/28094#discussion_r425817679



##########
File path: core/src/main/scala/org/apache/spark/status/api/v1/api.scala
##########
@@ -109,7 +114,8 @@ class ExecutorSummary private[spark](
     @JsonDeserialize(using = classOf[ExecutorMetricsJsonDeserializer])
     val peakMemoryMetrics: Option[ExecutorMetrics],
     val attributes: Map[String, String],
-    val resources: Map[String, ResourceInformation])
+    val resources: Map[String, ResourceInformation],
+    val resourceProfileId: Int)

Review comment:
       so the LiveEntity creates the ExecutorSummary and it has a default of 
the ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID and executorInfoFromJson will 
default it to ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID when that field isn't 
found as well.
   So it will just default to that for older history files that don't have this 
field.
   
   I had tested this with some 2.x history files originally and it was working 
that way as expected. But I should retest again after the changes from review 
comments but I don't think anything has broken that.  
   
   Definitely something we want to make sure its backwards compatible.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to