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