Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-12708][UI] Sorting task error in Stages Page when yarn mode. #10663

Closed
wants to merge 5 commits into from
Closed

[SPARK-12708][UI] Sorting task error in Stages Page when yarn mode. #10663

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc45425
[SPARK-12708][UI] Sorting task error in Stages Page when yarn mode
Jan 8, 2016
62c04ca
fixed PR comments
Jan 8, 2016
3678e30
Merge remote branch 'upstream/master' into SPARK-12708
Jan 13, 2016
3d508c6
Fix PR comments and similar issue in PoolPage.
Jan 13, 2016
384b990
Fixed PR comments.
koyoshida Jan 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.spark.ui.jobs

import java.net.URLEncoder
import java.net.URLDecoder
import java.util.Date
import javax.servlet.http.HttpServletRequest

Expand Down Expand Up @@ -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 =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sortColumn should be on previous line like .map { sortColumn =>

// If sortColumn contains "/", `getParameter("task.sort")` will return
// "%252F" when yarn mode. we need additional decode.
// See also SPARK-4313, YARN-2844.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about saying like as follows?

// SPARK-12708
// Due to YARN-2844, "/" in the url will be encoded to "%252F" when
// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1; good to reduce duplication here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

var decodedColumn = URLDecoder.decode(column, "UTF-8")
while (column != decodedColumn) {
column = decodedColumn
decodedColumn = URLDecoder.decode(column, "UTF-8")
}
column
}.getOrElse("Index")
val taskSortDesc = Option(parameterTaskSortDesc).map(_.toBoolean).getOrElse(false)
val taskPageSize = Option(parameterTaskPageSize).map(_.toInt).getOrElse(100)
val taskPrevPageSize = Option(parameterTaskPrevPageSize).map(_.toInt).getOrElse(taskPageSize)
Expand Down