SPARK-12637 Print stage info of finished stages properly #10585
Conversation
|
Test build #48714 has finished for PR 10585 at commit
|
|
Test build #48718 has finished for PR 10585 at commit
|
| @@ -61,6 +61,21 @@ class StageInfo( | |||
| "running" | |||
| } | |||
| } | |||
|
|
|||
| private[spark] def getStatusDetail: String = { | |||
There was a problem hiding this comment.
Is this used elsewhere or can it just go into SparkListener?
I think this is confusing as one big format string. I'd make vals for the pieces that need some computation and return one big interpolated string. Also this is verbose:
foo match {
Some(x) => f(x)
case _ => ""
}
compared to just foo.map(x => f(x)).getOrElse("")
There was a problem hiding this comment.
And, I've searched spark codes but that's the sole case logging StageInfo.
There was a problem hiding this comment.
Yeah, I mean I wonder if it's worth exposing this pretty specific logging string as an internal API method? you can just create this string in the one place it's used
|
Test build #48803 has finished for PR 10585 at commit
|
|
I think this still needs the changes I've mentioned above, or at least, that's what I'd prefer to see before merging |
|
@srowen moved it to |
|
Test build #49078 has finished for PR 10585 at commit
|
|
It's looking good though I still sort of think string interpolation would make this clearer. I'll leave open for a bit to see if anyone cares either way on that one |
| @@ -293,6 +293,15 @@ class StatsReportListener extends SparkListener with Logging { | |||
| taskInfoMetrics.clear() | |||
| } | |||
|
|
|||
| private[spark] def getStatusDetail(stageInfo: StageInfo): String = { | |||
There was a problem hiding this comment.
This could even be fully private, but not a huge deal.
|
Meh, this isn't that hard to read without interpolation, so I'd be okay with merging this if it's fine with @srowen. |
|
I don't feel that strongly but if we're changing this, might as well do it right with interpolation and making the method |
|
@navis what do you think about following up on this? |
|
I'll take this over if there's no progress on this PR |
Improve printing of StageInfo in onStageCompleted See also #10585 Author: Sean Owen <[email protected]> Closes #10922 from srowen/SPARK-12637.
|
@navis you can close this PR |
Currently it prints hashcode of stage info, which seemed not that useful.