[SPARK-12708][UI] Sorting task error in Stages Page when yarn mode. #10663
Conversation
| @@ -100,7 +101,19 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") { | |||
| val parameterTaskPrevPageSize = request.getParameter("task.prevPageSize") | |||
|
|
|||
| val taskPage = Option(parameterTaskPage).map(_.toInt).getOrElse(1) | |||
| val taskSortColumn = Option(parameterTaskSortColumn).getOrElse("Index") | |||
| val taskSortColumn = Option(parameterTaskSortColumn).map { | |||
| sortColumn => | |||
There was a problem hiding this comment.
sortColumn should be on previous line like .map { sortColumn =>
|
ok to test. |
|
@yoshidakuy Does this issue affect both |
yoshidakuy
changed the title
|
Test build #49013 has finished for PR 10663 at commit
|
|
@sarutak Thanks. I have addressed your comment.
Yes. |
|
Test build #49018 has finished for PR 10663 at commit
|
| // running in yarn mode. `request.getParameter("task.sort")` will return | ||
| // "%252F". Therefore we need to decode it until we get the real column name. | ||
| // SPARK-4313 is similar to this issue. | ||
| var column = sortColumn |
There was a problem hiding this comment.
Since this is very similar to ExecutorThreadDumpPage, could you create a new method in UIUtils (maybe call it decodeURLParameter and add comments to explain it) and update here and ExecutorThreadDumpPage to use it?
There was a problem hiding this comment.
+1; good to reduce duplication here.
There was a problem hiding this comment.
Make sense.
@yoshidakuy can you add a new method to UIUtils to reduce the similar logic?
Then, call it from ExecutorThreadDumpPage and StagePage.
Also, could you add a test case for the new method to UIUtilsSuite?
|
Thanks for comments and I agree. will fix later. |
|
Ping @yoshidakuy, could you update this pull request to address our comments? It would be great to merge this soon. |
|
(I think we're going to do a 1.6.1 soon, so if we get this in soon it can be in that release) |
|
@JoshRosen @sarutak Sorry for late. |
|
Test build #49396 has finished for PR 10663 at commit
|
| } | ||
| id | ||
| }.getOrElse { | ||
| val executorId = Option(request.getParameter("executorId")).getOrElse { |
There was a problem hiding this comment.
decodeURLParameter is not invoked.
We should say like as follows right?
Option(...).map { executorId =>
UIUtils.decodeURLParameter(executorId)
}.getOrElse {
...
}
|
LGTM |
|
Test build #49407 has finished for PR 10663 at commit
|
|
retest this please |
|
Test build #49414 has finished for PR 10663 at commit
|
|
retest this please. |
|
Test build #49433 has finished for PR 10663 at commit
|
|
LGTM as well. Merging into |
If sort column contains slash(e.g. "Executor ID / Host") when yarn mode,sort fail with following message.  Ｉt's similar to SPARK-4313 . Author: root <root@R520T1.(none)> Author: Koyo Yoshida <[email protected]> Closes #10663 from yoshidakuy/SPARK-12708. (cherry picked from commit 32cca93) Signed-off-by: Kousuke Saruta <[email protected]>
If sort column contains slash(e.g. "Executor ID / Host") when yarn mode,sort fail with following message.
Ｉt's similar to SPARK-4313 .