[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-12 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/22645


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223200104
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,74 +122,257 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
--- End diff --

Thank you @felixcheung . I have modified.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread felixcheung
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223198079
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,74 +122,257 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
--- End diff --

could you either have `.` last or first of the next line.
for example L172 is at the end and L166 is in front - let's do this 
consistently with other code in the file


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223195007
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -229,73 +406,88 @@ private[ui] abstract class ExecutionTable(
 }
 
 val desc = if (execution.description != null && 
execution.description.nonEmpty) {
-  {execution.description}
+  {execution.description}
 } else {
-  {execution.executionId}
+  {execution.executionId}
 }
 
-{desc} {details}
-  }
-
-  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
-UIUtils.listingTable[SQLExecutionUIData](
-  header, row(request, currentTime, _), executionUIDatas, id = 
Some(tableId))
+{desc}{details}
   }
 
   private def jobURL(request: HttpServletRequest, jobId: Long): String =
 "%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, 
parent.basePath), jobId)
 
-  private def executionURL(request: HttpServletRequest, executionID: 
Long): String =
+  private def executionURL(executionID: Long): String =
 s"${UIUtils.prependBaseUri(
   request, 
parent.basePath)}/${parent.prefix}/execution/?id=$executionID"
 }
 
-private[ui] class RunningExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"running-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = true,
-showSucceededJobs = true,
-showFailedJobs = true) {
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Running Job IDs", "Succeeded Job IDs", "Failed Job 
IDs")
-}
+private[ui] class ExecutionTableRowData(
+val submissionTime: Long,
+val duration: Long,
+val executionUIData: SQLExecutionUIData)
+
 
-private[ui] class CompletedExecutionTable(
+private[ui] class ExecutionDataSource(
+request: HttpServletRequest,
 parent: SQLTab,
+executionData: Seq[SQLExecutionUIData],
+basePath: String,
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"completed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = false) {
+pageSize: Int,
+sortColumn: String,
+desc: Boolean) extends 
PagedDataSource[ExecutionTableRowData](pageSize) {
 
-  override protected def header: Seq[String] = baseHeader ++ Seq("Job IDs")
-}
+  // Convert ExecutionData to ExecutionTableRowData which contains the 
final contents to show
+  // in the table so that we can avoid creating duplicate contents during 
sorting the data
+  private val data = 
executionData.map(executionRow).sorted(ordering(sortColumn, desc))
 
-private[ui] class FailedExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"failed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = true) {
+  private var _slicedJobIds: Set[Int] = _
+
+  override def dataSize: Int = data.size
+
+  override def sliceData(from: Int, to: Int): Seq[ExecutionTableRowData] = 
{
+val r = data.slice(from, to)
+_slicedJobIds = r.map(_.executionUIData.executionId.toInt).toSet
+r
+  }
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Succeeded Job IDs", "Failed Job IDs")
+  private def executionRow(executionUIData: SQLExecutionUIData): 
ExecutionTableRowData = {
+val submissionTime = executionUIData.submissionTime
+val duration = executionUIData.completionTime.map(_.getTime())
+  .getOrElse(currentTime) - submissionTime
+
+new ExecutionTableRowData(
+  submissionTime,
+  duration,
+  executionUIData)
+  }
+
+  /**
+* Return Ordering according to sortColumn and desc
+*/
+  private def ordering(sortColumn: String, desc: Boolean): 
Ordering[ExecutionTableRowData] = {
+val ordering: Ordering[ExecutionTableRowData] = sortColumn match {
+  case "ID" => Ordering.by(_.executionUIData.executionId)
+  case "Description" => Ordering.by(_.executionUIData.description)
+  case "Submitted" => Ordering.by(_.executionUIData.submissionTime)
+  case "Duration" => Ordering.by(_.duration)
+  case "Job IDs" | "Succeeded Job IDs" => Ordering by 
(_.executionUIData.jobs.filt

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193975
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193956
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193970
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193960
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192942
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193077
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -229,73 +406,88 @@ private[ui] abstract class ExecutionTable(
 }
 
 val desc = if (execution.description != null && 
execution.description.nonEmpty) {
-  {execution.description}
+  {execution.description}
 } else {
-  {execution.executionId}
+  {execution.executionId}
 }
 
-{desc} {details}
-  }
-
-  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
-UIUtils.listingTable[SQLExecutionUIData](
-  header, row(request, currentTime, _), executionUIDatas, id = 
Some(tableId))
+{desc}{details}
   }
 
   private def jobURL(request: HttpServletRequest, jobId: Long): String =
 "%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, 
parent.basePath), jobId)
 
-  private def executionURL(request: HttpServletRequest, executionID: 
Long): String =
+  private def executionURL(executionID: Long): String =
 s"${UIUtils.prependBaseUri(
   request, 
parent.basePath)}/${parent.prefix}/execution/?id=$executionID"
 }
 
-private[ui] class RunningExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"running-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = true,
-showSucceededJobs = true,
-showFailedJobs = true) {
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Running Job IDs", "Succeeded Job IDs", "Failed Job 
IDs")
-}
+private[ui] class ExecutionTableRowData(
+val submissionTime: Long,
+val duration: Long,
+val executionUIData: SQLExecutionUIData)
+
 
-private[ui] class CompletedExecutionTable(
+private[ui] class ExecutionDataSource(
+request: HttpServletRequest,
 parent: SQLTab,
+executionData: Seq[SQLExecutionUIData],
+basePath: String,
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"completed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = false) {
+pageSize: Int,
+sortColumn: String,
+desc: Boolean) extends 
PagedDataSource[ExecutionTableRowData](pageSize) {
 
-  override protected def header: Seq[String] = baseHeader ++ Seq("Job IDs")
-}
+  // Convert ExecutionData to ExecutionTableRowData which contains the 
final contents to show
+  // in the table so that we can avoid creating duplicate contents during 
sorting the data
+  private val data = 
executionData.map(executionRow).sorted(ordering(sortColumn, desc))
 
-private[ui] class FailedExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"failed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = true) {
+  private var _slicedJobIds: Set[Int] = _
+
+  override def dataSize: Int = data.size
+
+  override def sliceData(from: Int, to: Int): Seq[ExecutionTableRowData] = 
{
+val r = data.slice(from, to)
+_slicedJobIds = r.map(_.executionUIData.executionId.toInt).toSet
+r
+  }
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Succeeded Job IDs", "Failed Job IDs")
+  private def executionRow(executionUIData: SQLExecutionUIData): 
ExecutionTableRowData = {
+val submissionTime = executionUIData.submissionTime
+val duration = executionUIData.completionTime.map(_.getTime())
+  .getOrElse(currentTime) - submissionTime
+
+new ExecutionTableRowData(
+  submissionTime,
+  duration,
+  executionUIData)
+  }
+
+  /**
+* Return Ordering according to sortColumn and desc
+*/
+  private def ordering(sortColumn: String, desc: Boolean): 
Ordering[ExecutionTableRowData] = {
+val ordering: Ordering[ExecutionTableRowData] = sortColumn match {
+  case "ID" => Ordering.by(_.executionUIData.executionId)
+  case "Description" => Ordering.by(_.executionUIData.description)
+  case "Submitted" => Ordering.by(_.executionUIData.submissionTime)
+  case "Duration" => Ordering.by(_.duration)
+  case "Job IDs" | "Succeeded Job IDs" => Ordering by 
(_.executionUIData.jobs.filter {

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192937
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192963
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192945
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(s"$executionTag.sort"))
+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 =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  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 {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+sortColumn: String,
+desc: Boolean,
 

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223193003
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+so

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192399
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192419
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223192408
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223191064
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  
 +details
++
-  
-{execution.details}
-  
+
+  {execution.details}
+
 } else {
   Nil
 }
 
 val desc = if (execution.description != null && 
execution.description.nonEmpty) {
-  {execution.description}
+  
+{execution.description}
+  
 } else {
-  {execution.executionId}
+  
+{execution.executionId}
+  
 }
 
-{desc} {details}
-  }
-
-  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
-UIUtils.listingTable[SQLExecutionUIData](
-  header, row(request, currentTime, _), executionUIDatas, id = 
Some(tableId))
+
+  {desc}{details}
+
   }
 
   private def jobURL(request: HttpServletRequest, jobId: Long): String =
 "%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, 
parent.basePath), jobId)
 
-  private def executionURL(request: HttpServletRequest, executionID: 
Long): String =
+  private def executionURL(executionID: Long): String =
 s"${UIUtils.prependBaseUri(
   request, 
parent.basePath)}/${parent.prefix}/execution/?id=$executionID"
 }
 
-private[ui] class RunningExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"running-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = true,
-showSucceededJobs = true,
-showFailedJobs = true) {
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Running Job IDs", "Succeeded Job IDs", "Failed Job 
IDs")
-}
+private[ui] class ExecutionTableRowData(
+val submissionTime: Long,
+val duration: Long,
+val executionUIData: SQLExecutionUIData)
 
-private[ui] class CompletedExecutionTable(
+
+private[ui] class ExecutionDataSource(
+request: HttpServletRequest,
 parent: SQLTab,
+executionData: Seq[SQLExecutionUIData],
+basePath: String,
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"completed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = false) {
+pageSize: Int,
+sortColumn: String,
+desc: Boolean) extends 
PagedDataSource[ExecutionTableRowData](pageSize) {
 
-  override protected def header: Seq[String] = baseHeader ++ Seq("Job IDs")
-}
+  // Convert ExecutionData to ExecutionTableRowData which contains the 
final contents to show
+  // in the table so that we can avoid creating duplicate contents during 
sorting the data
+  private val data = 
executionData.map(executionRow).sorted(ordering(sortColumn, desc))
 
-private[ui] class FailedExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"failed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = true) {
+  private var _slicedJobIds: Set[Int] = _
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Succeeded Job IDs", "Failed Job IDs")
+  override def dataSize: Int = data.size
+
+  override def sliceData(from: Int, to: Int): Seq[ExecutionTableRowData] = 
{
+val r = data.slice(from, to)
+_slicedJobIds = r.map(_.executionUIData.executi

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223191054
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  
 +details
++
-  
-{execution.details}
-  
+
--- End diff --

Yes. I have used a different scalastyle and checkstyle xml in IJ it seems.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223191033
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223191000
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223190984
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
--- End diff --

Done. Thanks


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223190979
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -17,16 +17,17 @@
 
 package org.apache.spark.sql.execution.ui
 
+import java.net.URLEncoder
 import javax.servlet.http.HttpServletRequest
 
+import scala.collection.JavaConverters._
 import scala.collection.mutable
-import scala.xml.{Node, NodeSeq}
-
-import org.apache.commons.lang3.StringEscapeUtils
+import scala.xml._
 
 import org.apache.spark.JobExecutionStatus
 import org.apache.spark.internal.Logging
-import org.apache.spark.ui.{UIUtils, WebUIPage}
+import org.apache.spark.ui._
--- End diff --

Yes. done. Thanks


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188217
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  
 +details
++
-  
-{execution.details}
-  
+
--- End diff --

I believe this shouldn't be indented more


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188235
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  
 +details
++
-  
-{execution.details}
-  
+
+  {execution.details}
+
 } else {
   Nil
 }
 
 val desc = if (execution.description != null && 
execution.description.nonEmpty) {
-  {execution.description}
+  
+{execution.description}
+  
 } else {
-  {execution.executionId}
+  
+{execution.executionId}
+  
 }
 
-{desc} {details}
-  }
-
-  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
-UIUtils.listingTable[SQLExecutionUIData](
-  header, row(request, currentTime, _), executionUIDatas, id = 
Some(tableId))
+
+  {desc}{details}
+
   }
 
   private def jobURL(request: HttpServletRequest, jobId: Long): String =
 "%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, 
parent.basePath), jobId)
 
-  private def executionURL(request: HttpServletRequest, executionID: 
Long): String =
+  private def executionURL(executionID: Long): String =
 s"${UIUtils.prependBaseUri(
   request, 
parent.basePath)}/${parent.prefix}/execution/?id=$executionID"
 }
 
-private[ui] class RunningExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"running-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = true,
-showSucceededJobs = true,
-showFailedJobs = true) {
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Running Job IDs", "Succeeded Job IDs", "Failed Job 
IDs")
-}
+private[ui] class ExecutionTableRowData(
+val submissionTime: Long,
+val duration: Long,
+val executionUIData: SQLExecutionUIData)
 
-private[ui] class CompletedExecutionTable(
+
+private[ui] class ExecutionDataSource(
+request: HttpServletRequest,
 parent: SQLTab,
+executionData: Seq[SQLExecutionUIData],
+basePath: String,
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"completed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = false) {
+pageSize: Int,
+sortColumn: String,
+desc: Boolean) extends 
PagedDataSource[ExecutionTableRowData](pageSize) {
 
-  override protected def header: Seq[String] = baseHeader ++ Seq("Job IDs")
-}
+  // Convert ExecutionData to ExecutionTableRowData which contains the 
final contents to show
+  // in the table so that we can avoid creating duplicate contents during 
sorting the data
+  private val data = 
executionData.map(executionRow).sorted(ordering(sortColumn, desc))
 
-private[ui] class FailedExecutionTable(
-parent: SQLTab,
-currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData])
-  extends ExecutionTable(
-parent,
-"failed-execution-table",
-currentTime,
-executionUIDatas,
-showRunningJobs = false,
-showSucceededJobs = true,
-showFailedJobs = true) {
+  private var _slicedJobIds: Set[Int] = _
 
-  override protected def header: Seq[String] =
-baseHeader ++ Seq("Succeeded Job IDs", "Failed Job IDs")
+  override def dataSize: Int = data.size
+
+  override def sliceData(from: Int, to: Int): Seq[ExecutionTableRowData] = 
{
+val r = data.slice(from, to)
+_slicedJobIds = r.map(_.executionUIData.executionId

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188145
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+so

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188014
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
--- End diff --

Trivial nit: you don't need the type here or an extra block


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188153
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+so

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188215
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -202,100 +385,127 @@ private[ui] abstract class ExecutionTable(
 
   }}
   {if (showSucceededJobs) {
-
-  {jobLinks(JobExecutionStatus.SUCCEEDED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.SUCCEEDED)}
+  
+}}
   {if (showFailedJobs) {
-
-  {jobLinks(JobExecutionStatus.FAILED)}
-
-  }}
+  
+{jobLinks(JobExecutionStatus.FAILED)}
+  
+}}
 
   }
 
-  private def descriptionCell(
-  request: HttpServletRequest,
-  execution: SQLExecutionUIData): Seq[Node] = {
+  private def descriptionCell(execution: SQLExecutionUIData): Seq[Node] = {
 val details = if (execution.details != null && 
execution.details.nonEmpty) {
-  
+  

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188205
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+so

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223188173
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -121,65 +122,247 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
   {
 if (running.nonEmpty) {
   
-Running 
Queries:
+Running Queries:
 {running.size}
   
 }
   }
   {
 if (completed.nonEmpty) {
   
-Completed 
Queries:
+Completed 
Queries:
 {completed.size}
   
 }
   }
   {
 if (failed.nonEmpty) {
   
-Failed 
Queries:
+Failed Queries:
 {failed.size}
   
 }
   }
 
   
+
 UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
   }
+
+  private def executionsTable(
+request: HttpServletRequest,
+executionTag: String,
+executionData: Seq[SQLExecutionUIData],
+currentTime: Long,
+showRunningJobs: Boolean,
+showSucceededJobs: Boolean,
+showFailedJobs: Boolean): Seq[Node] = {
+
+// stripXSS is called to remove suspicious characters used in XSS 
attacks
+val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
+  UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
+}
+val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
+  .map(para => para._1 + "=" + para._2(0))
+
+val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(executionTag + ".page"))
+val parameterExecutionSortColumn = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".sort"))
+val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(executionTag + ".desc"))
+val parameterExecutionPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".pageSize"))
+val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
+  getParameter(executionTag + ".prevPageSize"))
+
+val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
+val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
+  UIUtils.decodeURLParameter(sortColumn)
+}.getOrElse("ID")
+val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
+  // New executions should be shown above old executions by default.
+  executionSortColumn == "ID"
+)
+val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
+val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
+  getOrElse(executionPageSize)
+
+val page: Int = {
+  // If the user has changed to a larger page size, then go to page 1 
in order to avoid
+  // IndexOutOfBoundsException.
+  if (executionPageSize <= executionPrevPageSize) {
+executionPage
+  } else {
+1
+  }
+}
+
+val tableHeaderId = executionTag // "running", "completed" or "failed"
+
+try {
+  new ExecutionPagedTable(
+request,
+parent,
+executionData,
+tableHeaderId,
+executionTag,
+UIUtils.prependBaseUri(request, parent.basePath),
+"SQL", // subPath
+parameterOtherTable,
+currentTime,
+pageSize = executionPageSize,
+sortColumn = executionSortColumn,
+desc = executionSortDesc,
+showRunningJobs,
+showSucceededJobs,
+showFailedJobs).table(page)
+} catch {
+  case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
+
+  Error while rendering execution table:
+  
+{Utils.exceptionString(e)}
+  
+
+}
+  }
 }
 
-private[ui] abstract class ExecutionTable(
+
+private[ui] class ExecutionPagedTable(
+request: HttpServletRequest,
 parent: SQLTab,
-tableId: String,
+data: Seq[SQLExecutionUIData],
+tableHeaderId: String,
+executionTag: String,
+basePath: String,
+subPath: String,
+parameterOtherTable: Iterable[String],
 currentTime: Long,
-executionUIDatas: Seq[SQLExecutionUIData],
+pageSize: Int,
+so

[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223187979
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
@@ -17,16 +17,17 @@
 
 package org.apache.spark.sql.execution.ui
 
+import java.net.URLEncoder
 import javax.servlet.http.HttpServletRequest
 
+import scala.collection.JavaConverters._
 import scala.collection.mutable
-import scala.xml.{Node, NodeSeq}
-
-import org.apache.commons.lang3.StringEscapeUtils
+import scala.xml._
 
 import org.apache.spark.JobExecutionStatus
 import org.apache.spark.internal.Logging
-import org.apache.spark.ui.{UIUtils, WebUIPage}
+import org.apache.spark.ui._
--- End diff --

Nit: can we avoid wildcard imports? we usually avoid them unless there are 
a very large number of classes to import


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread shahidki31
Github user shahidki31 commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223174634
  
--- Diff: core/src/main/scala/org/apache/spark/ui/PagedTable.scala ---
@@ -31,7 +31,7 @@ import org.apache.spark.util.Utils
  *
  * @param pageSize the number of rows in a page
  */
-private[ui] abstract class PagedDataSource[T](val pageSize: Int) {
+private[spark] abstract class PagedDataSource[T](val pageSize: Int) {
--- End diff --

Hi @felixcheung , Thanks for the review.
 'AllExecutionsPage'  and 'PagedTable' are in different packages. Currently 
PagedTable is not accessible to the AllExecutionsPage class. 
Also excpet 'PagedTable', all the classes in the core ui are open to the 
spark level, like 'JettyUtils', 'SparkUI', 'UIUtils', 'WebUI' etc. 




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-06 Thread felixcheung
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/22645#discussion_r223174258
  
--- Diff: core/src/main/scala/org/apache/spark/ui/PagedTable.scala ---
@@ -31,7 +31,7 @@ import org.apache.spark.util.Utils
  *
  * @param pageSize the number of rows in a page
  */
-private[ui] abstract class PagedDataSource[T](val pageSize: Int) {
+private[spark] abstract class PagedDataSource[T](val pageSize: Int) {
--- End diff --

is there a way to do this without opening this all to the spark level?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pag...

2018-10-05 Thread shahidki31
GitHub user shahidki31 opened a pull request:

https://github.com/apache/spark/pull/22645

[SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination for SQL tab to 
avoid OOM

## What changes were proposed in this pull request?
Currently SQL tab in the WEBUI doesn't have pagination. Because of that 
following issues happening.
1) For large number of executions, SQL page is throwing OOM exception 
(around 40,000)
2) For large number of executions, loading SQL page is taking time.
3) Difficult to analyse the execution table for large number of execution.
[Note: spark.ui.retainedExecutions = 5]

All the tabs, Jobs, Stages etc. supports pagination. So, to make it 
consistent with other tabs
SQL tab also should support pagination.

I have followed the similar flow of the pagination code in the Jobs and 
Stages page for SQL page.
Also, this patch doesn't make any behavior change for the SQL tab except 
the pagination support.

## How was this patch tested?
bin/spark-shell --conf spark.ui.retainedExecutions=5
Run 50,000 sql queries.
**Before this PR**
![screenshot from 2018-10-05 
22-58-11](https://user-images.githubusercontent.com/23054875/46550276-33b5e680-c8f2-11e8-9e32-9ae9c5b181e0.png)

**After this PR**

Loading of the page is faster, and OOM issue doesn't happen.
![screenshot from 2018-10-05 
23-14-29](https://user-images.githubusercontent.com/23054875/46551092-71b40a00-c8f4-11e8-93dd-ff5b99d44c54.png)







You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shahidki31/spark SPARK-25566

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/22645.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #22645


commit 4dc1b225b4de9f16c920bd597e3597d023f3
Author: Shahid 
Date:   2018-10-04T21:16:09Z

SPARK-25566

[Spark Job History] SQL UI Page does not support Pagination

commit e2b45d51fbb00eab2e7b2e2e2fe35d45ca3f424c
Author: Shahid 
Date:   2018-10-05T16:55:19Z

[SPARK-25566]SQL UI Page support Pagination to avoid OOM




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org