Github user kayousterhout commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15505#discussion_r95688659
  
    --- Diff: 
core/src/main/scala/org/apache/spark/scheduler/TaskDescription.scala ---
    @@ -52,7 +55,43 @@ private[spark] class TaskDescription(
         val addedFiles: Map[String, Long],
         val addedJars: Map[String, Long],
         val properties: Properties,
    -    val serializedTask: ByteBuffer) {
    +    private var serializedTask_ : ByteBuffer) extends  Logging {
    --- End diff --
    
    While there is value in making the task a member variable of this class (to 
reduce code changes on the master), I think that making the serializedTask a 
member variable adds too much confusion -- because the conditions under which 
it is set are a bit difficult to reason about.  How about removing it as a 
member variable, and then encode() can do the serialization of the Task itself, 
and decode can return a (TaskDescription, ByteBuffer) pair, where the 2nd thing 
is the serialized task (and then that can be passed on its own into 
executor.launchTask)?  @squito thoughts?  I'd find that more clear because then 
no one needs to reason about when serializedTask_ is and is not set to 
something.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to