diff options
author | Josh Rosen <joshrosen@databricks.com> | 2015-12-28 16:42:11 -0800 |
---|---|---|
committer | Josh Rosen <joshrosen@databricks.com> | 2015-12-28 16:42:11 -0800 |
commit | 124a3a5e4eece3aabca44fbdd2f8c4c086d6eec3 (patch) | |
tree | 163772ce00776556f569fb937350c51b7968c3d7 /core/src/main | |
parent | 710b41172958a0b3a2b70c48821aefc81893731b (diff) | |
download | spark-124a3a5e4eece3aabca44fbdd2f8c4c086d6eec3.tar.gz spark-124a3a5e4eece3aabca44fbdd2f8c4c086d6eec3.tar.bz2 spark-124a3a5e4eece3aabca44fbdd2f8c4c086d6eec3.zip |
[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 <joshrosen@databricks.com>
Closes #10441 from JoshRosen/simplify-paginated-table-sorting.
Diffstat (limited to 'core/src/main')
4 files changed, 171 insertions, 96 deletions
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 b54e33a96f..dd708ef2c2 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; +} 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 6e2375477a..9b6ed8cbbe 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 } 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 1b34ba9f03..b02b99a6fc 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> } } 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 fd6cc3ed75..3e51ce2e97 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> } } |