Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1056#issuecomment-49963097
Hey @sryza so I spent a good amount of time talking about this with
@aarondav and @andrewor14.
There are some challenges because we break assumptions in the previous
code. One assumption is that before a `TaskMetrics` is consumed downstream it
is fully populated and no fields are missing that aren't `Option[T]`. Another
is that a given TaskMetrics will only be mutated within one thread.
For the first issue, I think the right thing to do here is to distinguish
between metrics that can be updated incrementally (`PartialTaskMetrics`) and
other metrics. And the update should only send `PartialTaskMetrics` over the
wire.
You'd have:
```
class TaskMetrics extends PartialTaskMetrics {
// For sending updates to the driver
def asPartialMetrics = this.asInstanceOf[PartialTaskMetrics]
}
```
You could do likewise for `ShuffleWriteMetrics` or `ShuffleReadMetrics`.
Then you'd very explicitly send partial data through the listener on the
updates.
For synchronization, we might be "just okay" here for now since we are only
mutating object references and integers. We had a few other ideas in that
regard, but I think fixing the partial/full metrics issue first would be better.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
