Skip to content

[SPARK-31688][WEBUI] Refactor Pagination framework #28512

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

Closed
iRakson wants to merge 4 commits into from
Closed

[SPARK-31688][WEBUI] Refactor Pagination framework #28512

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7775dd1
[SPARK-31688] refactor pagination framework
iRakson May 12, 2020
89194d3
refactor thriftserverPage
iRakson May 13, 2020
0f886d6
fix
iRakson May 13, 2020
50065ff
fix
iRakson May 20, 2020
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
101 changes: 100 additions & 1 deletion core/src/main/scala/org/apache/spark/ui/PagedTable.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@

package org.apache.spark.ui

import java.net.URLDecoder
import java.net.{URLDecoder, URLEncoder}
import java.nio.charset.StandardCharsets.UTF_8
import javax.servlet.http.HttpServletRequest

import scala.collection.JavaConverters._
import scala.xml.{Node, Unparsed}
Expand Down Expand Up @@ -297,4 +298,102 @@ private[spark] trait PagedTable[T] {
* Returns the submission path for the "go to page #" form.
*/
def goButtonFormPath: String

/**
* Returns parameters of other tables in the page.
*/
def getParameterOtherTable(request: HttpServletRequest, tableTag: String): String = {
request.getParameterMap.asScala
.filterNot(_._1.startsWith(tableTag))
.map(parameter => parameter._1 + "=" + parameter._2(0))
.mkString("&")
}

/**
* Returns parameter of this table.
*/
def getTableParameters(
request: HttpServletRequest,
tableTag: String,
defaultSortColumn: String): (String, Boolean, Int) = {
val parameterSortColumn = request.getParameter(s"$tableTag.sort")
val parameterSortDesc = request.getParameter(s"$tableTag.desc")
val parameterPageSize = request.getParameter(s"$tableTag.pageSize")
val sortColumn = Option(parameterSortColumn).map { sortColumn =>
UIUtils.decodeURLParameter(sortColumn)
}.getOrElse(defaultSortColumn)
val desc = Option(parameterSortDesc).map(_.toBoolean).getOrElse(
sortColumn == defaultSortColumn
)
val pageSize = Option(parameterPageSize).map(_.toInt).getOrElse(100)

(sortColumn, desc, pageSize)
}

/**
* Check if given sort column is valid or not. If invalid then an exception is thrown.
*/
def isSortColumnValid(
headerInfo: Seq[(String, Boolean, Option[String])],
sortColumn: String): Any = {

@sarutak sarutak May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The return type is better to be Unit rather than Any isn't it?

if (!headerInfo.filter(_._2).map(_._1).contains(sortColumn)) {
throw new IllegalArgumentException(s"Unknown column: $sortColumn")
}
}

def headerRow(
headerInfo: Seq[(String, Boolean, Option[String])],
desc: Boolean,
pageSize: Int,
sortColumn: String,
parameterPath: String,
tableTag: String,
headerId: String): Seq[Node] = {
val row: Seq[Node] = {
headerInfo.map { case (header, sortable, tooltip) =>
if (header == sortColumn) {
val headerLink = Unparsed(
parameterPath +
s"&$tableTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$tableTag.desc=${!desc}" +
s"&$tableTag.pageSize=$pageSize" +
s"#$headerId")
val arrow = if (desc) "▾" else "▴" // UP or DOWN

<th>
<a href={headerLink}>
<span data-toggle="tooltip" data-placement="top" title={tooltip.getOrElse("")}>
{header}&nbsp;{Unparsed(arrow)}
</span>
</a>
</th>
} else {
if (sortable) {
val headerLink = Unparsed(
parameterPath +
s"&$tableTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$tableTag.pageSize=$pageSize" +
s"#$headerId")

<th>
<a href={headerLink}>
<span data-toggle="tooltip" data-placement="top" title={tooltip.getOrElse("")}>
{header}
</span>
</a>
</th>
} else {
<th>
<span data-toggle="tooltip" data-placement="top" title={tooltip.getOrElse("")}>
{header}
</span>
</th>
}
}
}
}
<thead>
{row}

@sarutak sarutak May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

{row} should be enclosed by <tr>.

</thead>
}
}
125 changes: 17 additions & 108 deletions core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,45 +211,24 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We
jobTag: String,
jobs: Seq[v1.JobData],
killEnabled: Boolean): Seq[Node] = {
val parameterOtherTable = request.getParameterMap().asScala

@sarutak sarutak May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same comment as here.

.filterNot(_._1.startsWith(jobTag))
.map(para => para._1 + "=" + para._2(0))

val someJobHasJobGroup = jobs.exists(_.jobGroup.isDefined)
val jobIdTitle = if (someJobHasJobGroup) "Job Id (Job Group)" else "Job Id"

val parameterJobPage = request.getParameter(jobTag + ".page")
val parameterJobSortColumn = request.getParameter(jobTag + ".sort")
val parameterJobSortDesc = request.getParameter(jobTag + ".desc")
val parameterJobPageSize = request.getParameter(jobTag + ".pageSize")

val jobPage = Option(parameterJobPage).map(_.toInt).getOrElse(1)
val jobSortColumn = Option(parameterJobSortColumn).map { sortColumn =>
UIUtils.decodeURLParameter(sortColumn)
}.getOrElse(jobIdTitle)
val jobSortDesc = Option(parameterJobSortDesc).map(_.toBoolean).getOrElse(
// New jobs should be shown above old jobs by default.
jobSortColumn == jobIdTitle
)
val jobPageSize = Option(parameterJobPageSize).map(_.toInt).getOrElse(100)

val jobPage = Option(request.getParameter(jobTag + ".page")).map(_.toInt).getOrElse(1)
val currentTime = System.currentTimeMillis()

try {
new JobPagedTable(
request,
store,
jobs,
tableHeaderId,
jobTag,
UIUtils.prependBaseUri(request, parent.basePath),
"jobs", // subPath
parameterOtherTable,
killEnabled,
currentTime,
jobIdTitle,
pageSize = jobPageSize,
sortColumn = jobSortColumn,
desc = jobSortDesc
jobIdTitle
).table(jobPage)
} catch {
case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) =>
Expand Down Expand Up @@ -493,21 +472,19 @@ private[ui] class JobDataSource(
}

private[ui] class JobPagedTable(
request: HttpServletRequest,
store: AppStatusStore,
data: Seq[v1.JobData],
tableHeaderId: String,
jobTag: String,
basePath: String,
subPath: String,
parameterOtherTable: Iterable[String],
killEnabled: Boolean,
currentTime: Long,
jobIdTitle: String,
pageSize: Int,
sortColumn: String,
desc: Boolean
jobIdTitle: String
) extends PagedTable[JobTableRowData] {
val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
private val (sortColumn, desc, pageSize) = getTableParameters(request, jobTag, jobIdTitle)
private val parameterPath = basePath + s"/$subPath/?" + getParameterOtherTable(request, jobTag)

override def tableId: String = jobTag + "-table"

Expand Down Expand Up @@ -545,89 +522,21 @@ private[ui] class JobPagedTable(

override def headers: Seq[Node] = {
// Information for each header: title, cssClass, and sortable

@sarutak sarutak May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you update the comment?

val jobHeadersAndCssClasses: Seq[(String, String, Boolean, Option[String])] =
val jobHeadersAndCssClasses: Seq[(String, Boolean, Option[String])] =
Seq(
(jobIdTitle, "", true, None),
("Description", "", true, None),
("Submitted", "", true, None),
("Duration", "", true, Some("Elapsed time since the job was submitted " +
(jobIdTitle, true, None),
("Description", true, None),
("Submitted", true, None),
("Duration", true, Some("Elapsed time since the job was submitted " +
"until execution completion of all its stages.")),
("Stages: Succeeded/Total", "", false, None),
("Tasks (for all stages): Succeeded/Total", "", false, None)
("Stages: Succeeded/Total", false, None),
("Tasks (for all stages): Succeeded/Total", false, None)
)

if (!jobHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
throw new IllegalArgumentException(s"Unknown column: $sortColumn")
}
isSortColumnValid(jobHeadersAndCssClasses, sortColumn)

val headerRow: Seq[Node] = {
jobHeadersAndCssClasses.map { case (header, cssClass, sortable, tooltip) =>
if (header == sortColumn) {
val headerLink = Unparsed(

@sarutak sarutak May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unparsed is imported but it's no longer used after this change.

parameterPath +
s"&$jobTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$jobTag.desc=${!desc}" +
s"&$jobTag.pageSize=$pageSize" +
s"#$tableHeaderId")
val arrow = if (desc) "&#x25BE;" else "&#x25B4;" // UP or DOWN

<th class={cssClass}>
<a href={headerLink}>
{
if (tooltip.nonEmpty) {
<span data-toggle="tooltip" data-placement="top" title={tooltip.get}>
{header}&nbsp;{Unparsed(arrow)}
</span>
} else {
<span>
{header}&nbsp;{Unparsed(arrow)}
</span>
}
}
</a>
</th>
} else {
if (sortable) {
val headerLink = Unparsed(
parameterPath +
s"&$jobTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$jobTag.pageSize=$pageSize" +
s"#$tableHeaderId")

<th class={cssClass}>
<a href={headerLink}>
{
if (tooltip.nonEmpty) {
<span data-toggle="tooltip" data-placement="top" title={tooltip.get}>
{header}
</span>
} else {
<span>
{header}
</span>
}
}
</a>
</th>
} else {
<th class={cssClass}>
{
if (tooltip.nonEmpty) {
<span data-toggle="tooltip" data-placement="top" title={tooltip.get}>
{header}
</span>
} else {
<span>
{header}
</span>
}
}
</th>
}
}
}
}
<thead><tr>{headerRow}</tr></thead>
headerRow(jobHeadersAndCssClasses, desc, pageSize, sortColumn, parameterPath,
jobTag, tableHeaderId)
}

override def row(jobTableRow: JobTableRowData): Seq[Node] = {
Expand Down
Loading