Repository: spark Updated Branches: refs/heads/master 1fb3759f2 -> fc65b4af0
[SPARK-25900][WEBUI] When the page number is more than the total page size, then fall back to the first page ## What changes were proposed in this pull request? When we give the page number more than the maximum page number, webui is throwing an exception. It would be better if fall back to the default page, instead of throwing the exception in the web ui. ## How was this patch tested? Before PR:  After PR:  Closes #22914 from shahidki31/pageFallBack. Authored-by: Shahid <[email protected]> Signed-off-by: Sean Owen <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/fc65b4af Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/fc65b4af Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/fc65b4af Branch: refs/heads/master Commit: fc65b4af00c0a813613a7977126e942df8440bbb Parents: 1fb3759 Author: Shahid <[email protected]> Authored: Mon Nov 5 09:13:53 2018 -0600 Committer: Sean Owen <[email protected]> Committed: Mon Nov 5 09:13:53 2018 -0600 ---------------------------------------------------------------------- .../scala/org/apache/spark/ui/PagedTable.scala | 51 +++++++++++++------- .../org/apache/spark/ui/jobs/AllJobsPage.scala | 17 +------ .../org/apache/spark/ui/jobs/StagePage.scala | 16 +----- .../org/apache/spark/ui/jobs/StageTable.scala | 19 +------- .../org/apache/spark/ui/storage/RDDPage.scala | 16 +----- .../org/apache/spark/ui/PagedTableSuite.scala | 17 ++----- .../sql/execution/ui/AllExecutionsPage.scala | 15 +----- 7 files changed, 45 insertions(+), 106 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/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 0bbb10a..6c2c1f6 100644 --- a/core/src/main/scala/org/apache/spark/ui/PagedTable.scala +++ b/core/src/main/scala/org/apache/spark/ui/PagedTable.scala @@ -33,10 +33,6 @@ import org.apache.spark.util.Utils */ private[spark] abstract class PagedDataSource[T](val pageSize: Int) { - if (pageSize <= 0) { - throw new IllegalArgumentException("Page size must be positive") - } - /** * Return the size of all data. */ @@ -51,13 +47,24 @@ private[spark] abstract class PagedDataSource[T](val pageSize: Int) { * Slice the data for this page */ def pageData(page: Int): PageData[T] = { - val totalPages = (dataSize + pageSize - 1) / pageSize - if (page <= 0 || page > totalPages) { - throw new IndexOutOfBoundsException( - s"Page $page is out of range. Please select a page number between 1 and $totalPages.") + // Display all the data in one page, if the pageSize is less than or equal to zero. + val pageTableSize = if (pageSize <= 0) { + dataSize + } else { + pageSize + } + val totalPages = (dataSize + pageTableSize - 1) / pageTableSize + + val pageToShow = if (page <= 0) { + 1 + } else if (page > totalPages) { + totalPages + } else { + page } - val from = (page - 1) * pageSize - val to = dataSize.min(page * pageSize) + + val (from, to) = ((pageToShow - 1) * pageSize, dataSize.min(pageToShow * pageTableSize)) + PageData(totalPages, sliceData(from, to)) } @@ -80,8 +87,6 @@ private[spark] trait PagedTable[T] { def pageSizeFormField: String - def prevPageSizeFormField: String - def pageNumberFormField: String def dataSource: PagedDataSource[T] @@ -94,7 +99,23 @@ private[spark] trait PagedTable[T] { val _dataSource = dataSource try { val PageData(totalPages, data) = _dataSource.pageData(page) - val pageNavi = pageNavigation(page, _dataSource.pageSize, totalPages) + + val pageToShow = if (page <= 0) { + 1 + } else if (page > totalPages) { + totalPages + } else { + page + } + // Display all the data in one page, if the pageSize is less than or equal to zero. + val pageSize = if (_dataSource.pageSize <= 0) { + data.size + } else { + _dataSource.pageSize + } + + val pageNavi = pageNavigation(pageToShow, pageSize, totalPages) + <div> {pageNavi} <table class={tableCssClass} id={tableId}> @@ -180,7 +201,6 @@ private[spark] trait PagedTable[T] { .split(search) .asScala .filterKeys(_ != pageSizeFormField) - .filterKeys(_ != prevPageSizeFormField) .filterKeys(_ != pageNumberFormField) .mapValues(URLDecoder.decode(_, "UTF-8")) .map { case (k, v) => @@ -198,9 +218,6 @@ private[spark] trait PagedTable[T] { 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" http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala ---------------------------------------------------------------------- diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala b/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala index 90e9a7a..2c22e05 100644 --- a/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala +++ b/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala @@ -220,7 +220,6 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We val parameterJobSortColumn = UIUtils.stripXSS(request.getParameter(jobTag + ".sort")) val parameterJobSortDesc = UIUtils.stripXSS(request.getParameter(jobTag + ".desc")) val parameterJobPageSize = UIUtils.stripXSS(request.getParameter(jobTag + ".pageSize")) - val parameterJobPrevPageSize = UIUtils.stripXSS(request.getParameter(jobTag + ".prevPageSize")) val jobPage = Option(parameterJobPage).map(_.toInt).getOrElse(1) val jobSortColumn = Option(parameterJobSortColumn).map { sortColumn => @@ -231,17 +230,7 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We jobSortColumn == jobIdTitle ) val jobPageSize = Option(parameterJobPageSize).map(_.toInt).getOrElse(100) - val jobPrevPageSize = Option(parameterJobPrevPageSize).map(_.toInt).getOrElse(jobPageSize) - - val page: Int = { - // If the user has changed to a larger page size, then go to page 1 in order to avoid - // IndexOutOfBoundsException. - if (jobPageSize <= jobPrevPageSize) { - jobPage - } else { - 1 - } - } + val currentTime = System.currentTimeMillis() try { @@ -259,7 +248,7 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We pageSize = jobPageSize, sortColumn = jobSortColumn, desc = jobSortDesc - ).table(page) + ).table(jobPage) } catch { case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) => <div class="alert alert-error"> @@ -526,8 +515,6 @@ private[ui] class JobPagedTable( override def pageSizeFormField: String = jobTag + ".pageSize" - override def prevPageSizeFormField: String = jobTag + ".prevPageSize" - override def pageNumberFormField: String = jobTag + ".page" override val dataSource = new JobDataSource( http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/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 0f74b07..477b9ce 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 @@ -91,7 +91,6 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We val parameterTaskSortColumn = UIUtils.stripXSS(request.getParameter("task.sort")) val parameterTaskSortDesc = UIUtils.stripXSS(request.getParameter("task.desc")) val parameterTaskPageSize = UIUtils.stripXSS(request.getParameter("task.pageSize")) - val parameterTaskPrevPageSize = UIUtils.stripXSS(request.getParameter("task.prevPageSize")) val taskPage = Option(parameterTaskPage).map(_.toInt).getOrElse(1) val taskSortColumn = Option(parameterTaskSortColumn).map { sortColumn => @@ -99,8 +98,6 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We }.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) - val stageId = parameterId.toInt val stageAttemptId = parameterAttempt.toInt @@ -278,15 +275,6 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We accumulableRow, stageData.accumulatorUpdates.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( @@ -299,7 +287,7 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We desc = taskSortDesc, store = parent.store ) - (_taskTable, _taskTable.table(page)) + (_taskTable, _taskTable.table(taskPage)) } catch { case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) => val errorMessage = @@ -732,8 +720,6 @@ private[ui] class TaskPagedTable( override def pageSizeFormField: String = "task.pageSize" - override def prevPageSizeFormField: String = "task.prevPageSize" - override def pageNumberFormField: String = "task.page" override val dataSource: TaskDataSource = new TaskDataSource( http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala ---------------------------------------------------------------------- diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala b/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala index d01acda..b9abd39 100644 --- a/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala +++ b/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala @@ -53,8 +53,6 @@ private[ui] class StageTableBase( val parameterStageSortColumn = UIUtils.stripXSS(request.getParameter(stageTag + ".sort")) val parameterStageSortDesc = UIUtils.stripXSS(request.getParameter(stageTag + ".desc")) val parameterStagePageSize = UIUtils.stripXSS(request.getParameter(stageTag + ".pageSize")) - val parameterStagePrevPageSize = - UIUtils.stripXSS(request.getParameter(stageTag + ".prevPageSize")) val stagePage = Option(parameterStagePage).map(_.toInt).getOrElse(1) val stageSortColumn = Option(parameterStageSortColumn).map { sortColumn => @@ -65,18 +63,7 @@ private[ui] class StageTableBase( stageSortColumn == "Stage Id" ) val stagePageSize = Option(parameterStagePageSize).map(_.toInt).getOrElse(100) - val stagePrevPageSize = Option(parameterStagePrevPageSize).map(_.toInt) - .getOrElse(stagePageSize) - - val page: Int = { - // If the user has changed to a larger page size, then go to page 1 in order to avoid - // IndexOutOfBoundsException. - if (stagePageSize <= stagePrevPageSize) { - stagePage - } else { - 1 - } - } + val currentTime = System.currentTimeMillis() val toNodeSeq = try { @@ -96,7 +83,7 @@ private[ui] class StageTableBase( isFailedStage, parameterOtherTable, request - ).table(page) + ).table(stagePage) } catch { case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) => <div class="alert alert-error"> @@ -161,8 +148,6 @@ private[ui] class StagePagedTable( override def pageSizeFormField: String = stageTag + ".pageSize" - override def prevPageSizeFormField: String = stageTag + ".prevPageSize" - override def pageNumberFormField: String = stageTag + ".page" val parameterPath = UIUtils.prependBaseUri(request, basePath) + s"/$subPath/?" + http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/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 238cd31..87da290 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 @@ -39,13 +39,11 @@ private[ui] class RDDPage(parent: SparkUITab, store: AppStatusStore) extends Web val parameterBlockSortColumn = UIUtils.stripXSS(request.getParameter("block.sort")) val parameterBlockSortDesc = UIUtils.stripXSS(request.getParameter("block.desc")) val parameterBlockPageSize = UIUtils.stripXSS(request.getParameter("block.pageSize")) - val parameterBlockPrevPageSize = UIUtils.stripXSS(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 = try { @@ -60,16 +58,6 @@ private[ui] class RDDPage(parent: SparkUITab, store: AppStatusStore) extends Web val workerTable = UIUtils.listingTable(workerHeader, workerRow, rddStorageInfo.dataDistribution.get, id = Some("rdd-storage-by-worker-table")) - // Block table - 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(request, parent.basePath) + s"/storage/rdd/?id=${rddId}", @@ -78,7 +66,7 @@ private[ui] class RDDPage(parent: SparkUITab, store: AppStatusStore) extends Web blockSortColumn, blockSortDesc, store.executorList(true)) - _blockTable.table(page) + _blockTable.table(blockPage) } catch { case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) => <div class="alert alert-error">{e.getMessage}</div> @@ -242,8 +230,6 @@ private[ui] class BlockPagedTable( override def pageSizeFormField: String = "block.pageSize" - override def prevPageSizeFormField: String = "block.prevPageSize" - override def pageNumberFormField: String = "block.page" override val dataSource: BlockDataSource = new BlockDataSource( http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/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 cda98ae..d18f554 100644 --- a/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala +++ b/core/src/test/scala/org/apache/spark/ui/PagedTableSuite.scala @@ -32,19 +32,12 @@ class PagedDataSourceSuite extends SparkFunSuite { val dataSource3 = new SeqPagedDataSource[Int](1 to 5, pageSize = 2) assert(dataSource3.pageData(3) === PageData(3, Seq(5))) - + // If the page number is more than maximum page, fall back to the last page val dataSource4 = new SeqPagedDataSource[Int](1 to 5, pageSize = 2) - val e1 = intercept[IndexOutOfBoundsException] { - dataSource4.pageData(4) - } - assert(e1.getMessage === "Page 4 is out of range. Please select a page number between 1 and 3.") - + assert(dataSource4.pageData(4) === PageData(3, Seq(5))) + // If the page number is less than or equal to zero, fall back to the first page val dataSource5 = new SeqPagedDataSource[Int](1 to 5, pageSize = 2) - val e2 = intercept[IndexOutOfBoundsException] { - dataSource5.pageData(0) - } - assert(e2.getMessage === "Page 0 is out of range. Please select a page number between 1 and 3.") - + assert(dataSource5.pageData(0) === PageData(3, 1 to 2)) } } @@ -66,8 +59,6 @@ class PagedTableSuite extends SparkFunSuite { override def pageSizeFormField: String = "pageSize" - override def prevPageSizeFormField: String = "prevPageSize" - override def pageNumberFormField: String = "page" override def goButtonFormPath: String = "" http://git-wip-us.apache.org/repos/asf/spark/blob/fc65b4af/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala index 311f805f..4958f15 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala @@ -171,8 +171,6 @@ private[ui] class AllExecutionsPage(parent: SQLTab) extends WebUIPage("") with L val parameterExecutionSortDesc = UIUtils.stripXSS(request.getParameter(s"$executionTag.desc")) val parameterExecutionPageSize = UIUtils.stripXSS(request .getParameter(s"$executionTag.pageSize")) - val parameterExecutionPrevPageSize = UIUtils.stripXSS(request - .getParameter(s"$executionTag.prevPageSize")) val executionPage = Option(parameterExecutionPage).map(_.toInt).getOrElse(1) val executionSortColumn = Option(parameterExecutionSortColumn).map { sortColumn => @@ -183,16 +181,7 @@ private[ui] class AllExecutionsPage(parent: SQLTab) extends WebUIPage("") with L executionSortColumn == "ID" ) val executionPageSize = Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100) - val executionPrevPageSize = Option(parameterExecutionPrevPageSize).map(_.toInt) - .getOrElse(executionPageSize) - // If the user has changed to a larger page size, then go to page 1 in order to avoid - // IndexOutOfBoundsException. - val page: Int = if (executionPageSize <= executionPrevPageSize) { - executionPage - } else { - 1 - } val tableHeaderId = executionTag // "running", "completed" or "failed" try { @@ -211,7 +200,7 @@ private[ui] class AllExecutionsPage(parent: SQLTab) extends WebUIPage("") with L desc = executionSortDesc, showRunningJobs, showSucceededJobs, - showFailedJobs).table(page) + showFailedJobs).table(executionPage) } catch { case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) => <div class="alert alert-error"> @@ -262,8 +251,6 @@ private[ui] class ExecutionPagedTable( "table table-bordered table-condensed table-striped " + "table-head-clickable table-cell-width-limited" - override def prevPageSizeFormField: String = s"$executionTag.prevPageSize" - override def pageLink(page: Int): String = { val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8") parameterPath + --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
