[SPARK-15885] [Web UI] Provide links to executor logs from stage details page in UI #13861
Conversation
|
Can one of the admins verify this patch? |
| {val logs = parent.executorsListener.executorToLogUrls.getOrElse(k, Map.empty) | ||
| logs.map { | ||
| case (logName, logUrl) => <div><a href={logUrl}>{logName}</a></div> | ||
| }} |
There was a problem hiding this comment.
Nitpick: this feels like it could be styled better, specifically indentation
There was a problem hiding this comment.
I'll admit, I'm still extremely new to Scala and so I haven't really picked up a good sense of formatting yet. I tried to follow a convention similar to lines above this. Would this https://gist.github.com/tmagrino/1dddaa4f2a9b0961d98809482d0cc4bb be considered better?
There was a problem hiding this comment.
That's probably what I would do, there a lot of styling up to the coder like this case where scalastyle doesn't complain
|
LGTM (even ignoring my style nitpicks) |
|
I'm going to leave the style alone, for the sake of consistency with the surrounding code. Anyone else I should ping to get a look at this code? |
|
@srowen @JoshRosen could you take a look at this? |
|
I'd hesitate a little bit to tack on yet another column and more info to this page, since it's already pretty busy. That's not a deal-breaker. The link makes some sense in the executor table, but not so much the tasks table? because it's a link to logs from the executor in general, not the task, and the task could potentially re-execute elsewhere. |
|
Instead of adding another column, can we just append "[logs]" to the "Executor Id" column? |
|
Can you update it, and then we can trigger Jenkins? |
|
Sorry for the delay, I was away from the computer for a good portion of yesterday. I'll make the changes that @rxin is suggesting. To clarify, I am going to do this tweak (changing from a column to listing links next to the executor id entry) for both tables. Our team's interest is in having an easy way to go from seeing a failed task to looking at the logs for the associated executor to try and investigate the problem. We've found that this has been immensely helpful for rapidly debugging jobs with large numbers of tasks. |
|
Latest commit now uses the more minimalist format discussed. |
|
If you could attach new screen shots please |
|
I had to black out a few hostnames to avoid NDA issues with my clusters, but the format should be clear. |
|
Thanks - merging in master. |
5 participants
What changes were proposed in this pull request?
This moves over old PR #13664 to target master rather than branch-1.6.
Added links to logs (or an indication that there are no logs) for entries which list an executor in the stage details page of the UI.
This helps streamline the workflow where a user views a stage details page and determines that they would like to see the associated executor log for further examination. Previously, a user would have to cross reference the executor id listed on the stage details page with the corresponding entry on the executors tab.
Link to the JIRA: https://issues.apache.org/jira/browse/SPARK-15885
How was this patch tested?
Ran existing unit tests.
Ran test queries on a platform which did not record executor logs and again on a platform which did record executor logs and verified that the new table column was empty and links to the logs (which were verified as linking to the appropriate files), respectively.
Attached is a screenshot of the UI page with no links, with the new columns highlighted. Additional screenshot of these columns with the populated links.
Without links:
With links:
This contribution is my original work and I license the work to the project under the Apache Spark project's open source license.