You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by jo...@apache.org on 2015/12/29 01:42:31 UTC
spark git commit: [SPARK-12490] Don't use Javascript for web UI's
paginated table controls
Repository: spark
Updated Branches:
refs/heads/master 710b41172 -> 124a3a5e4
[SPARK-12490] Don't use Javascript for web UI's paginated table controls
The web UI's paginated table uses Javascript to implement certain navigation controls, such as table sorting and the "go to page" form. This is unnecessary and should be simplified to use plain HTML form controls and links.
/cc zsxwing, who wrote this original code, and yhuai.
Author: Josh Rosen <jo...@databricks.com>
Closes #10441 from JoshRosen/simplify-paginated-table-sorting.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/124a3a5e
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/124a3a5e
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/124a3a5e
Branch: refs/heads/master
Commit: 124a3a5e4eece3aabca44fbdd2f8c4c086d6eec3
Parents: 710b411
Author: Josh Rosen <jo...@databricks.com>
Authored: Mon Dec 28 16:42:11 2015 -0800
Committer: Josh Rosen <jo...@databricks.com>
Committed: Mon Dec 28 16:42:11 2015 -0800
----------------------------------------------------------------------
.../org/apache/spark/ui/static/webui.css | 11 +-
.../scala/org/apache/spark/ui/PagedTable.scala | 100 ++++++++++++-------
.../org/apache/spark/ui/jobs/StagePage.scala | 80 +++++++++------
.../org/apache/spark/ui/storage/RDDPage.scala | 76 ++++++++------
.../org/apache/spark/ui/PagedTableSuite.scala | 8 +-
5 files changed, 178 insertions(+), 97 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/124a3a5e/core/src/main/resources/org/apache/spark/ui/static/webui.css
----------------------------------------------------------------------
diff --git a/core/src/main/resources/org/apache/spark/ui/static/webui.css b/core/src/main/resources/org/apache/spark/ui/static/webui.css
index b54e33a..dd708ef 100644
--- a/core/src/main/resources/org/apache/spark/ui/static/webui.css
+++ b/core/src/main/resources/org/apache/spark/ui/static/webui.css
@@ -225,4 +225,13 @@ a.expandbutton {
background-color: #49535a !important;
color: white;
cursor:pointer;
-}
\ No newline at end of file
+}
+
+th a, th a:hover {
+ /* Make the entire header clickable, not just the text label */
+ display: block;
+ width: 100%;
+ /* Suppress the default link styling */
+ color: #333;
+ text-decoration: none;
+}
http://git-wip-us.apache.org/repos/asf/spark/blob/124a3a5e/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/PagedTable.scala b/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
index 6e23754..9b6ed8c 100644
--- a/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
+++ b/core/src/main/scala/org/apache/spark/ui/PagedTable.scala
@@ -17,8 +17,15 @@
package org.apache.spark.ui
+import java.net.URLDecoder
+
+import scala.collection.JavaConverters._
import scala.xml.{Node, Unparsed}
+import com.google.common.base.Splitter
+
+import org.apache.spark.util.Utils
+
/**
* A data source that provides data for a page.
*
@@ -71,6 +78,12 @@ private[ui] trait PagedTable[T] {
def tableCssClass: String
+ def pageSizeFormField: String
+
+ def prevPageSizeFormField: String
+
+ def pageNumberFormField: String
+
def dataSource: PagedDataSource[T]
def headers: Seq[Node]
@@ -95,7 +108,12 @@ private[ui] trait PagedTable[T] {
val PageData(totalPages, _) = _dataSource.pageData(1)
<div>
{pageNavigation(1, _dataSource.pageSize, totalPages)}
- <div class="alert alert-error">{e.getMessage}</div>
+ <div class="alert alert-error">
+ <p>Error while rendering table:</p>
+ <pre>
+ {Utils.exceptionString(e)}
+ </pre>
+ </div>
</div>
}
}
@@ -151,36 +169,56 @@ private[ui] trait PagedTable[T] {
// The current page should be disabled so that it cannot be clicked.
<li class="disabled"><a href="#">{p}</a></li>
} else {
- <li><a href={pageLink(p)}>{p}</a></li>
+ <li><a href={Unparsed(pageLink(p))}>{p}</a></li>
+ }
+ }
+
+ val hiddenFormFields = {
+ if (goButtonFormPath.contains('?')) {
+ val querystring = goButtonFormPath.split("\\?", 2)(1)
+ Splitter
+ .on('&')
+ .trimResults()
+ .withKeyValueSeparator("=")
+ .split(querystring)
+ .asScala
+ .filterKeys(_ != pageSizeFormField)
+ .filterKeys(_ != prevPageSizeFormField)
+ .filterKeys(_ != pageNumberFormField)
+ .mapValues(URLDecoder.decode(_, "UTF-8"))
+ .map { case (k, v) =>
+ <input type="hidden" name={k} value={v} />
+ }
+ } else {
+ Seq.empty
}
}
- val (goButtonJsFuncName, goButtonJsFunc) = goButtonJavascriptFunction
- // When clicking the "Go" button, it will call this javascript method and then call
- // "goButtonJsFuncName"
- val formJs =
- s"""$$(function(){
- | $$( "#form-$tableId-page" ).submit(function(event) {
- | var page = $$("#form-$tableId-page-no").val()
- | var pageSize = $$("#form-$tableId-page-size").val()
- | pageSize = pageSize ? pageSize: 100;
- | if (page != "") {
- | ${goButtonJsFuncName}(page, pageSize);
- | }
- | event.preventDefault();
- | });
- |});
- """.stripMargin
<div>
<div>
<form id={s"form-$tableId-page"}
- class="form-inline pull-right" style="margin-bottom: 0px;">
+ method="get"
+ action={Unparsed(goButtonFormPath)}
+ class="form-inline pull-right"
+ style="margin-bottom: 0px;">
+ <input type="hidden"
+ name={prevPageSizeFormField}
+ value={pageSize.toString} />
+ {hiddenFormFields}
<label>{totalPages} Pages. Jump to</label>
- <input type="text" id={s"form-$tableId-page-no"} value={page.toString} class="span1" />
+ <input type="text"
+ name={pageNumberFormField}
+ id={s"form-$tableId-page-no"}
+ value={page.toString} class="span1" />
+
<label>. Show </label>
<input type="text"
- id={s"form-$tableId-page-size"} value={pageSize.toString} class="span1" />
+ id={s"form-$tableId-page-size"}
+ name={pageSizeFormField}
+ value={pageSize.toString}
+ class="span1" />
<label>items in a page.</label>
+
<button type="submit" class="btn">Go</button>
</form>
</div>
@@ -189,7 +227,7 @@ private[ui] trait PagedTable[T] {
<ul>
{if (currentGroup > firstGroup) {
<li>
- <a href={pageLink(startPage - groupSize)} aria-label="Previous Group">
+ <a href={Unparsed(pageLink(startPage - groupSize))} aria-label="Previous Group">
<span aria-hidden="true">
<<
</span>
@@ -198,7 +236,7 @@ private[ui] trait PagedTable[T] {
}}
{if (page > 1) {
<li>
- <a href={pageLink(page - 1)} aria-label="Previous">
+ <a href={Unparsed(pageLink(page - 1))} aria-label="Previous">
<span aria-hidden="true">
<
</span>
@@ -208,14 +246,14 @@ private[ui] trait PagedTable[T] {
{pageTags}
{if (page < totalPages) {
<li>
- <a href={pageLink(page + 1)} aria-label="Next">
+ <a href={Unparsed(pageLink(page + 1))} aria-label="Next">
<span aria-hidden="true">></span>
</a>
</li>
}}
{if (currentGroup < lastGroup) {
<li>
- <a href={pageLink(startPage + groupSize)} aria-label="Next Group">
+ <a href={Unparsed(pageLink(startPage + groupSize))} aria-label="Next Group">
<span aria-hidden="true">
>>
</span>
@@ -224,11 +262,6 @@ private[ui] trait PagedTable[T] {
}}
</ul>
</div>
- <script>
- {Unparsed(goButtonJsFunc)}
-
- {Unparsed(formJs)}
- </script>
</div>
}
}
@@ -239,10 +272,7 @@ private[ui] trait PagedTable[T] {
def pageLink(page: Int): String
/**
- * Only the implementation knows how to create the url with a page number and the page size, so we
- * leave this one to the implementation. The implementation should create a JavaScript method that
- * accepts a page number along with the page size and jumps to the page. The return value is this
- * method name and its JavaScript codes.
+ * Returns the submission path for the "go to page #" form.
*/
- def goButtonJavascriptFunction: (String, String)
+ def goButtonFormPath: String
}
http://git-wip-us.apache.org/repos/asf/spark/blob/124a3a5e/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala b/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
index 1b34ba9..b02b99a 100644
--- a/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
@@ -97,11 +97,13 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val parameterTaskSortColumn = request.getParameter("task.sort")
val parameterTaskSortDesc = request.getParameter("task.desc")
val parameterTaskPageSize = request.getParameter("task.pageSize")
+ val parameterTaskPrevPageSize = request.getParameter("task.prevPageSize")
val taskPage = Option(parameterTaskPage).map(_.toInt).getOrElse(1)
val taskSortColumn = Option(parameterTaskSortColumn).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)
// If this is set, expand the dag visualization by default
val expandDagVizParam = request.getParameter("expandDagViz")
@@ -274,6 +276,15 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
accumulableRow,
externalAccumulables.toSeq)
+ val page: Int = {
+ // If the user has changed to a larger page size, then go to page 1 in order to avoid
+ // IndexOutOfBoundsException.
+ if (taskPageSize <= taskPrevPageSize) {
+ taskPage
+ } else {
+ 1
+ }
+ }
val currentTime = System.currentTimeMillis()
val (taskTable, taskTableHTML) = try {
val _taskTable = new TaskPagedTable(
@@ -292,10 +303,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
sortColumn = taskSortColumn,
desc = taskSortDesc
)
- (_taskTable, _taskTable.table(taskPage))
+ (_taskTable, _taskTable.table(page))
} catch {
case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) =>
- (null, <div class="alert alert-error">{e.getMessage}</div>)
+ val errorMessage =
+ <div class="alert alert-error">
+ <p>Error while rendering stage table:</p>
+ <pre>
+ {Utils.exceptionString(e)}
+ </pre>
+ </div>
+ (null, errorMessage)
}
val jsForScrollingDownToTaskTable =
@@ -1217,6 +1235,12 @@ private[ui] class TaskPagedTable(
override def tableCssClass: String = "table table-bordered table-condensed table-striped"
+ override def pageSizeFormField: String = "task.pageSize"
+
+ override def prevPageSizeFormField: String = "task.prevPageSize"
+
+ override def pageNumberFormField: String = "task.page"
+
override val dataSource: TaskDataSource = new TaskDataSource(
data,
hasAccumulators,
@@ -1232,24 +1256,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
- | page = pageSize == currentTaskPageSize ? page : 1;
- | var url = "${basePath}&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- | "&task.page=" + page + "&task.pageSize=" + pageSize;
- | window.location.href = url;
- |}
- """.stripMargin
- (jsFuncName, jsFunc)
+ s"$basePath&task.sort=$encodedSortColumn&task.desc=$desc"
}
def headers: Seq[Node] = {
@@ -1298,21 +1314,27 @@ private[ui] class TaskPagedTable(
val headerRow: Seq[Node] = {
taskHeadersAndCssClasses.map { case (header, cssClass) =>
if (header == sortColumn) {
- val headerLink =
- s"$basePath&task.sort=${URLEncoder.encode(header, "UTF-8")}&task.desc=${!desc}" +
- s"&task.pageSize=${pageSize}"
- val js = Unparsed(s"window.location.href='${headerLink}'")
+ val headerLink = Unparsed(
+ basePath +
+ s"&task.sort=${URLEncoder.encode(header, "UTF-8")}" +
+ s"&task.desc=${!desc}" +
+ s"&task.pageSize=$pageSize")
val arrow = if (desc) "▾" else "▴" // UP or DOWN
- <th class={cssClass} onclick={js} style="cursor: pointer;">
- {header}
- <span> {Unparsed(arrow)}</span>
+ <th class={cssClass}>
+ <a href={headerLink}>
+ {header}
+ <span> {Unparsed(arrow)}</span>
+ </a>
</th>
} else {
- val headerLink =
- s"$basePath&task.sort=${URLEncoder.encode(header, "UTF-8")}&task.pageSize=${pageSize}"
- val js = Unparsed(s"window.location.href='${headerLink}'")
- <th class={cssClass} onclick={js} style="cursor: pointer;">
- {header}
+ val headerLink = Unparsed(
+ basePath +
+ s"&task.sort=${URLEncoder.encode(header, "UTF-8")}" +
+ s"&task.pageSize=$pageSize")
+ <th class={cssClass}>
+ <a href={headerLink}>
+ {header}
+ </a>
</th>
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/124a3a5e/core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala b/core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala
index fd6cc3e..3e51ce2 100644
--- a/core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala
@@ -38,11 +38,13 @@ private[ui] class RDDPage(parent: StorageTab) extends WebUIPage("rdd") {
val parameterBlockSortColumn = request.getParameter("block.sort")
val parameterBlockSortDesc = request.getParameter("block.desc")
val parameterBlockPageSize = request.getParameter("block.pageSize")
+ val parameterBlockPrevPageSize = request.getParameter("block.prevPageSize")
val blockPage = Option(parameterBlockPage).map(_.toInt).getOrElse(1)
val blockSortColumn = Option(parameterBlockSortColumn).getOrElse("Block Name")
val blockSortDesc = Option(parameterBlockSortDesc).map(_.toBoolean).getOrElse(false)
val blockPageSize = Option(parameterBlockPageSize).map(_.toInt).getOrElse(100)
+ val blockPrevPageSize = Option(parameterBlockPrevPageSize).map(_.toInt).getOrElse(blockPageSize)
val rddId = parameterId.toInt
val rddStorageInfo = AllRDDResource.getRDDStorageInfo(rddId, listener, includeDetails = true)
@@ -56,17 +58,26 @@ private[ui] class RDDPage(parent: StorageTab) extends WebUIPage("rdd") {
rddStorageInfo.dataDistribution.get, id = Some("rdd-storage-by-worker-table"))
// Block table
- val (blockTable, blockTableHTML) = try {
+ val page: Int = {
+ // If the user has changed to a larger page size, then go to page 1 in order to avoid
+ // IndexOutOfBoundsException.
+ if (blockPageSize <= blockPrevPageSize) {
+ blockPage
+ } else {
+ 1
+ }
+ }
+ val blockTableHTML = try {
val _blockTable = new BlockPagedTable(
UIUtils.prependBaseUri(parent.basePath) + s"/storage/rdd/?id=${rddId}",
rddStorageInfo.partitions.get,
blockPageSize,
blockSortColumn,
blockSortDesc)
- (_blockTable, _blockTable.table(blockPage))
+ _blockTable.table(page)
} catch {
case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) =>
- (null, <div class="alert alert-error">{e.getMessage}</div>)
+ <div class="alert alert-error">{e.getMessage}</div>
}
val jsForScrollingDownToBlockTable =
@@ -228,6 +239,12 @@ private[ui] class BlockPagedTable(
override def tableCssClass: String = "table table-bordered table-condensed table-striped"
+ override def pageSizeFormField: String = "block.pageSize"
+
+ override def prevPageSizeFormField: String = "block.prevPageSize"
+
+ override def pageNumberFormField: String = "block.page"
+
override val dataSource: BlockDataSource = new BlockDataSource(
rddPartitions,
pageSize,
@@ -236,24 +253,16 @@ private[ui] class BlockPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&block.page=$page&block.sort=${encodedSortColumn}&block.desc=${desc}" +
- s"&block.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&block.sort=$encodedSortColumn" +
+ s"&block.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToBlockPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentBlockPageSize = ${pageSize}
- |function goToBlockPage(page, pageSize) {
- | // Set page to 1 if the page size changes
- | page = pageSize == currentBlockPageSize ? page : 1;
- | var url = "${basePath}&block.sort=${encodedSortColumn}&block.desc=${desc}" +
- | "&block.page=" + page + "&block.pageSize=" + pageSize;
- | window.location.href = url;
- |}
- """.stripMargin
- (jsFuncName, jsFunc)
+ s"$basePath&block.sort=$encodedSortColumn&block.desc=$desc"
}
override def headers: Seq[Node] = {
@@ -271,22 +280,27 @@ private[ui] class BlockPagedTable(
val headerRow: Seq[Node] = {
blockHeaders.map { header =>
if (header == sortColumn) {
- val headerLink =
- s"$basePath&block.sort=${URLEncoder.encode(header, "UTF-8")}&block.desc=${!desc}" +
- s"&block.pageSize=${pageSize}"
- val js = Unparsed(s"window.location.href='${headerLink}'")
+ val headerLink = Unparsed(
+ basePath +
+ s"&block.sort=${URLEncoder.encode(header, "UTF-8")}" +
+ s"&block.desc=${!desc}" +
+ s"&block.pageSize=$pageSize")
val arrow = if (desc) "▾" else "▴" // UP or DOWN
- <th onclick={js} style="cursor: pointer;">
- {header}
- <span> {Unparsed(arrow)}</span>
+ <th>
+ <a href={headerLink}>
+ {header}
+ <span> {Unparsed(arrow)}</span>
+ </a>
</th>
} else {
- val headerLink =
- s"$basePath&block.sort=${URLEncoder.encode(header, "UTF-8")}" +
- s"&block.pageSize=${pageSize}"
- val js = Unparsed(s"window.location.href='${headerLink}'")
- <th onclick={js} style="cursor: pointer;">
- {header}
+ val headerLink = Unparsed(
+ basePath +
+ s"&block.sort=${URLEncoder.encode(header, "UTF-8")}" +
+ s"&block.pageSize=$pageSize")
+ <th>
+ <a href={headerLink}>
+ {header}
+ </a>
</th>
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/124a3a5e/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala b/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
index cc76c14..74eeca2 100644
--- a/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
+++ b/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala
@@ -64,7 +64,13 @@ class PagedTableSuite extends SparkFunSuite {
override def row(t: Int): Seq[Node] = Nil
- override def goButtonJavascriptFunction: (String, String) = ("", "")
+ override def pageSizeFormField: String = "pageSize"
+
+ override def prevPageSizeFormField: String = "prevPageSize"
+
+ override def pageNumberFormField: String = "page"
+
+ override def goButtonFormPath: String = ""
}
assert(pagedTable.pageNavigation(1, 10, 1) === Nil)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org