You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by shahidki31 <gi...@git.apache.org> on 2018/10/05 17:45:13 UTC

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

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 = 50000]
    
    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=50000
    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 4dc1b225b4de9f16c920bd5aaaa97e3597d023f3
Author: Shahid <sh...@...>
Date:   2018-10-04T21:16:09Z

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

commit e2b45d51fbb00eab2e7b2e2e2fe35d45ca3f424c
Author: Shahid <sh...@...>
Date:   2018-10-05T16:55:19Z

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

----


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #97144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97144/testReport)** for PR 22645 at commit [`e6af675`](https://github.com/apache/spark/commit/e6af675dddec44dcdc3f5060ac6b3db6fbef5f20).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +    
    +    val sortableColumnHeaders = executionHeadersAndCssClasses.filter {
    +      case (_, _, sortable) => sortable
    +    }.map(header => header._1)
    --- End diff --
    
    Updated. Thanks.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Thank you @srowen , I have modified the code based on your suggestions.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Thank you @srowen for the review. I have addressed the comments.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Merged to master


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
    --- End diff --
    
    Can this be private? and how about using interpolation for the whole thing?


---

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


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

Posted by srowen <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    +            class="expand-details">
             +details
           </span> ++
    -      <div class="stage-details collapsed">
    -        <pre>{execution.details}</pre>
    -      </div>
    +        <div class="stage-details collapsed">
    +          <pre>{execution.details}</pre>
    +        </div>
         } else {
           Nil
         }
     
         val desc = if (execution.description != null && execution.description.nonEmpty) {
    -      <a href={executionURL(request, execution.executionId)}>{execution.description}</a>
    +      <a href={executionURL(execution.executionId)}>
    +        {execution.description}
    +      </a>
         } else {
    -      <a href={executionURL(request, execution.executionId)}>{execution.executionId}</a>
    +      <a href={executionURL(execution.executionId)}>
    +        {execution.executionId}
    +      </a>
         }
     
    -    <div>{desc} {details}</div>
    -  }
    -
    -  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
    -    UIUtils.listingTable[SQLExecutionUIData](
    -      header, row(request, currentTime, _), executionUIDatas, id = Some(tableId))
    +    <div>
    +      {desc}{details}
    +    </div>
       }
     
       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.toInt).toSet
    +    r
    +  }
    +
    +  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.flatMap {
    +        case (jobId, jobStatus) =>
    +          if (jobStatus == JobExecutionStatus.SUCCEEDED) jobId.toString
    +          else ""
    +      }.toSeq.toString())
    +      case "Running Job IDs" => Ordering.by(_.executionUIData.jobs.flatMap {
    +        case (jobId, jobStatus) =>
    --- End diff --
    
    Same, here and around here do you really want to filter first instead of flatMap?


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
     
    -    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    -      executionUIData.jobs.flatMap { case (jobId, jobStatus) =>
    -        if (jobStatus == status) {
    -          <a href={jobURL(request, jobId)}>[{jobId.toString}]</a>
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
    +
    +  override def prevPageSizeFormField: String = executionTag + ".prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = executionTag + ".pageSize"
    +
    +  override def pageNumberFormField: String = executionTag + ".page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +
    +    if (!executionHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
    --- End diff --
    
    Thanks. Done.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97146/
    Test FAILed.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    +            class="expand-details">
             +details
           </span> ++
    -      <div class="stage-details collapsed">
    -        <pre>{execution.details}</pre>
    -      </div>
    +        <div class="stage-details collapsed">
    --- End diff --
    
    Yes. I have used a different scalastyle and checkstyle xml in IJ it seems.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Merged build finished. Test FAILed.


---

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


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

Posted by srowen <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    --- End diff --
    
    Nit: pull this onto previous line? the indent is odd, at least


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #4361 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4361/testReport)** for PR 22645 at commit [`2b5a724`](https://github.com/apache/spark/commit/2b5a724506266f243783fbfa7724a1e9f02c23ed).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


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

Posted by felixcheung <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Hi @srowen , There is one behavior change this PR introduces, which is correct. Sorting Job Ids in the previous versions of spark was not proper. After the PR the sorting is proper.
    
    ![screenshot from 2018-10-08 23-55-22](https://user-images.githubusercontent.com/23054875/46626683-abd01680-cb55-11e8-80b2-5e446eb102b7.png)
    
    After the PR:
    ![screenshot from 2018-10-08 23-54-00](https://user-images.githubusercontent.com/23054875/46626694-b2f72480-cb55-11e8-8403-ec3a01afaf3b.png)
    
    I have updated the code accordingly.
    
    



---

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


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

Posted by srowen <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    +            class="expand-details">
             +details
           </span> ++
    -      <div class="stage-details collapsed">
    -        <pre>{execution.details}</pre>
    -      </div>
    +        <div class="stage-details collapsed">
    --- End diff --
    
    I believe this shouldn't be indented more


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    ok to test


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    +            class="expand-details">
             +details
           </span> ++
    -      <div class="stage-details collapsed">
    -        <pre>{execution.details}</pre>
    -      </div>
    +        <div class="stage-details collapsed">
    +          <pre>{execution.details}</pre>
    +        </div>
         } else {
           Nil
         }
     
         val desc = if (execution.description != null && execution.description.nonEmpty) {
    -      <a href={executionURL(request, execution.executionId)}>{execution.description}</a>
    +      <a href={executionURL(execution.executionId)}>
    +        {execution.description}
    +      </a>
         } else {
    -      <a href={executionURL(request, execution.executionId)}>{execution.executionId}</a>
    +      <a href={executionURL(execution.executionId)}>
    +        {execution.executionId}
    +      </a>
         }
     
    -    <div>{desc} {details}</div>
    -  }
    -
    -  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
    -    UIUtils.listingTable[SQLExecutionUIData](
    -      header, row(request, currentTime, _), executionUIDatas, id = Some(tableId))
    +    <div>
    +      {desc}{details}
    +    </div>
       }
     
       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.toInt).toSet
    +    r
    +  }
    +
    +  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.flatMap {
    +        case (jobId, jobStatus) =>
    +          if (jobStatus == JobExecutionStatus.SUCCEEDED) jobId.toString
    +          else ""
    +      }.toSeq.toString())
    +      case "Running Job IDs" => Ordering.by(_.executionUIData.jobs.flatMap {
    +        case (jobId, jobStatus) =>
    --- End diff --
    
    Thanks. I have modified using filter


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
    --- End diff --
    
    Here and below, how about interpolation consistently?


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Can one of the admins verify this patch?


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +    
    +    val sortableColumnHeaders = executionHeadersAndCssClasses.filter {
    +      case (_, _, sortable) => sortable
    +    }.map(header => header._1)
    --- End diff --
    
    `.map { case (title, _, _) => title }`? 


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Thanks a lot @srowen 



---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Hi @cloud-fan , Since other webtabs like jobs, stages etc. embed the javascript code in scala code, that is why I followed the same. It would be great if we rewrite the spark UI with some modern front end frameworks.
    
    Also, after this PR, loading time of the SQL page improved significantly. earlier loading 55,000 queries sql page took ~4.34 mins from the history server, but after the PR took only ~4 sec.


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    --- End diff --
    
    Nit: pull this onto previous line


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Hi @felixcheung , I will update the code


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    I found the UI patches are very hard to review, because we embed HTML/Javascript in Scala code. Is there a plan to rewrite the Spark UI with some modern frontend frameworks?


---

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


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

Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    @felixcheung I build locally, Now scalastyle issue is not happening. kindly re-trigger the PR builder.


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
     
    -    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    -      executionUIData.jobs.flatMap { case (jobId, jobStatus) =>
    -        if (jobStatus == status) {
    -          <a href={jobURL(request, jobId)}>[{jobId.toString}]</a>
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
    +
    +  override def prevPageSizeFormField: String = executionTag + ".prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = executionTag + ".pageSize"
    +
    +  override def pageNumberFormField: String = executionTag + ".page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +
    +    if (!executionHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
    +      throw new IllegalArgumentException(s"Unknown column: $sortColumn")
    +    }
    +
    +    val headerRow: Seq[Node] = {
    +      executionHeadersAndCssClasses.map { case (header, cssClass, sortable) =>
    +        if (header == sortColumn) {
    +          val headerLink = Unparsed(
    +            parameterPath +
    +              s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +              s"&$executionTag.desc=${!desc}" +
    +              s"&$executionTag.pageSize=$pageSize" +
    +              s"#$tableHeaderId")
    +          val arrow = if (desc) "&#x25BE;" else "&#x25B4;" // UP or DOWN
    +
    +          <th class={cssClass}>
    +            <a href={headerLink}>
    +              {header}<span>
    +              &nbsp;{Unparsed(arrow)}
    +            </span>
    +            </a>
    +          </th>
             } else {
    -          None
    +          if (sortable) {
    +            val headerLink = Unparsed(
    +              parameterPath +
    +                s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +                s"&$executionTag.pageSize=$pageSize" +
    +                s"#$tableHeaderId")
    +
    +            <th class={cssClass}>
    +              <a href={headerLink}>
    +                {header}
    +              </a>
    +            </th>
    +          } else {
    +            <th class={cssClass}>
    +              {header}
    +            </th>
    +          }
             }
    +      }
    +    }
    +    <thead>
    +      {headerRow}
    +    </thead>
    +  }
    +
    +  override def row(executionTableRow: ExecutionTableRowData): Seq[Node] = {
    +    val executionUIData = executionTableRow.executionUIData
    +    val submissionTime = executionUIData.submissionTime
    +    val duration = executionTableRow.duration
    +
    +    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    +      executionUIData.jobs.flatMap {
    +        case (jobId, jobStatus) =>
    --- End diff --
    
    Might be a little more idiomatic to say...
    
    ```
    case (jobId, jobStatus) if jobStatus == status => ...
    case _ => none
    ```
    
    but why not just filter first instead of flatMap?


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    @shahidki31 ^^


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
    --- End diff --
    
    Done. Thanks


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    @felixcheung It is a random failure. Could you please re-trigger the test. Thanks
    Please refer:
    https://issues.apache.org/jira/browse/SPARK-23622


---

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


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

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    cc @vanzin @srowen @cloud-fan @dongjoon-hyun . Kindly review the PR.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #97146 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97146/testReport)** for PR 22645 at commit [`2b5a724`](https://github.com/apache/spark/commit/2b5a724506266f243783fbfa7724a1e9f02c23ed).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #97144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97144/testReport)** for PR 22645 at commit [`e6af675`](https://github.com/apache/spark/commit/e6af675dddec44dcdc3f5060ac6b3db6fbef5f20).


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97144/
    Test FAILed.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    --- End diff --
    
    Yes. I have modified based on your suggestions. Thanks.


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
    -      <a href={executionURL(request, execution.executionId)}>{execution.description}</a>
    +      <a href={executionURL(execution.executionId)}>{execution.description}</a>
         } else {
    -      <a href={executionURL(request, execution.executionId)}>{execution.executionId}</a>
    +      <a href={executionURL(execution.executionId)}>{execution.executionId}</a>
         }
     
    -    <div>{desc} {details}</div>
    -  }
    -
    -  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
    -    UIUtils.listingTable[SQLExecutionUIData](
    -      header, row(request, currentTime, _), executionUIDatas, id = Some(tableId))
    +    <div>{desc}{details}</div>
       }
     
       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 {
    --- End diff --
    
    The expression here is still a little complex for my taste, but can't be simplified much. Maybe omit jobId in the case statement, and the second toString below it? the Seq[String] should already have the same ordering.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
    -      <a href={executionURL(request, execution.executionId)}>{execution.description}</a>
    +      <a href={executionURL(execution.executionId)}>{execution.description}</a>
         } else {
    -      <a href={executionURL(request, execution.executionId)}>{execution.executionId}</a>
    +      <a href={executionURL(execution.executionId)}>{execution.executionId}</a>
         }
     
    -    <div>{desc} {details}</div>
    -  }
    -
    -  def toNodeSeq(request: HttpServletRequest): Seq[Node] = {
    -    UIUtils.listingTable[SQLExecutionUIData](
    -      header, row(request, currentTime, _), executionUIDatas, id = Some(tableId))
    +    <div>{desc}{details}</div>
       }
     
       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 {
    --- End diff --
    
    I have modified the code by pre computing the job data based on status and put it in the 'ExecutionTableRowData'.
    Now from there, both 'ordering' and 'jobLinks' uses the job data.
    Thanks you.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #4361 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4361/testReport)** for PR 22645 at commit [`2b5a724`](https://github.com/apache/spark/commit/2b5a724506266f243783fbfa7724a1e9f02c23ed).


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
     
    -    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    -      executionUIData.jobs.flatMap { case (jobId, jobStatus) =>
    -        if (jobStatus == status) {
    -          <a href={jobURL(request, jobId)}>[{jobId.toString}]</a>
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
    +
    +  override def prevPageSizeFormField: String = executionTag + ".prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = executionTag + ".pageSize"
    +
    +  override def pageNumberFormField: String = executionTag + ".page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +
    +    if (!executionHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
    --- End diff --
    
    This might be worth using case expressions to clarify what _3 and _1 are.


---

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


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

Posted by felixcheung <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    **[Test build #97146 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97146/testReport)** for PR 22645 at commit [`2b5a724`](https://github.com/apache/spark/commit/2b5a724506266f243783fbfa7724a1e9f02c23ed).


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
     
    -    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    -      executionUIData.jobs.flatMap { case (jobId, jobStatus) =>
    -        if (jobStatus == status) {
    -          <a href={jobURL(request, jobId)}>[{jobId.toString}]</a>
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
    +
    +  override def prevPageSizeFormField: String = executionTag + ".prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = executionTag + ".pageSize"
    +
    +  override def pageNumberFormField: String = executionTag + ".page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +
    +    if (!executionHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
    +      throw new IllegalArgumentException(s"Unknown column: $sortColumn")
    +    }
    +
    +    val headerRow: Seq[Node] = {
    +      executionHeadersAndCssClasses.map { case (header, cssClass, sortable) =>
    +        if (header == sortColumn) {
    +          val headerLink = Unparsed(
    +            parameterPath +
    +              s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +              s"&$executionTag.desc=${!desc}" +
    +              s"&$executionTag.pageSize=$pageSize" +
    +              s"#$tableHeaderId")
    +          val arrow = if (desc) "&#x25BE;" else "&#x25B4;" // UP or DOWN
    +
    +          <th class={cssClass}>
    +            <a href={headerLink}>
    +              {header}<span>
    +              &nbsp;{Unparsed(arrow)}
    +            </span>
    +            </a>
    +          </th>
             } else {
    -          None
    +          if (sortable) {
    +            val headerLink = Unparsed(
    +              parameterPath +
    +                s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +                s"&$executionTag.pageSize=$pageSize" +
    +                s"#$tableHeaderId")
    +
    +            <th class={cssClass}>
    +              <a href={headerLink}>
    +                {header}
    +              </a>
    +            </th>
    +          } else {
    +            <th class={cssClass}>
    +              {header}
    +            </th>
    +          }
             }
    +      }
    +    }
    +    <thead>
    +      {headerRow}
    +    </thead>
    +  }
    +
    +  override def row(executionTableRow: ExecutionTableRowData): Seq[Node] = {
    +    val executionUIData = executionTableRow.executionUIData
    +    val submissionTime = executionUIData.submissionTime
    +    val duration = executionTableRow.duration
    +
    +    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    +      executionUIData.jobs.flatMap {
    +        case (jobId, jobStatus) =>
    --- End diff --
    
    `.filter { case (_, jobStatus) => jobStatus == status }`? I'm on the fence about whether it's worth it, but I generally like this clearer style. You can also omit `jobStatus` in the case statement for a little more conciseness.


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    --- End diff --
    
    This might be a good place to wrap these to line up the elements in the Seq, one one each line (?) to make it easier to read its contents. They all seem to have an empty string as the second element? can it be removed or did I overlook something? Also you don't need the type here.


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
    --- End diff --
    
    Done. Thanks


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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(
             </td>
           }}
           {if (showSucceededJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.SUCCEEDED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.SUCCEEDED)}
    +      </td>
    +    }}
           {if (showFailedJobs) {
    -        <td>
    -          {jobLinks(JobExecutionStatus.FAILED)}
    -        </td>
    -      }}
    +      <td>
    +        {jobLinks(JobExecutionStatus.FAILED)}
    +      </td>
    +    }}
         </tr>
       }
     
    -  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) {
    -      <span onclick="clickDetail(this)" class="expand-details">
    +      <span onclick="this.parentNode.querySelector('.stage-details').
    +      classList.toggle('collapsed')"
    --- End diff --
    
    Done. Thanks.


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #22645: [SPARK-25566][SPARK-25567][WEBUI][SQL]Support pagination...

Posted by shahidki31 <gi...@git.apache.org>.
Github user shahidki31 commented on the issue:

    https://github.com/apache/spark/pull/22645
  
    Test step to reproduce OOM without the PR.
    1) bin/spark-shell --conf spark.sql.ui.retainedExecutions=50000
      for (i <- 0 until 50000) {
              val df = Seq(
                (1, 1),
                (2, 2)
              ).toDF()
              df.collect()
    
    Without the PR:
    ![screenshot from 2018-10-06 09-46-11](https://user-images.githubusercontent.com/23054875/46567210-be2b3400-c94c-11e8-8348-847bd7e011d3.png)
    
    After fix:
    ![screenshot from 2018-10-06 09-46-31](https://user-images.githubusercontent.com/23054875/46567212-c84d3280-c94c-11e8-95f6-09bcd5dd6c10.png)



---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +    
    +    val sortableColumnHeaders = executionHeadersAndCssClasses.filter {
    +      case (_, _, sortable) => sortable
    +    }.map(header => header._1)
    +
    +    if(!sortableColumnHeaders.contains(sortColumn)) {
    --- End diff --
    
    Done. used require. Thanks


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +    
    +    val sortableColumnHeaders = executionHeadersAndCssClasses.filter {
    +      case (_, _, sortable) => sortable
    +    }.map(header => header._1)
    +
    +    if(!sortableColumnHeaders.contains(sortColumn)) {
    --- End diff --
    
    Nit: space after if. Really, maybe just use `require`


---

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


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

Posted by srowen <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    --- End diff --
    
    Done. Thanks


---

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


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

Posted by shahidki31 <gi...@git.apache.org>.
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) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed Queries:</strong></a>
    +                <a href="#completed"><strong>Completed Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         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) =>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -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,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
     
    -  protected def header: Seq[String]
    +  val parameterPath = basePath + s"/$subPath/?" + parameterOtherTable.mkString("&")
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    -    val submissionTime = executionUIData.submissionTime
    -    val duration = executionUIData.completionTime.map(_.getTime()).getOrElse(currentTime) -
    -      submissionTime
    +  override def tableId: String = executionTag + "-table"
     
    -    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    -      executionUIData.jobs.flatMap { case (jobId, jobStatus) =>
    -        if (jobStatus == status) {
    -          <a href={jobURL(request, jobId)}>[{jobId.toString}]</a>
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
    +
    +  override def prevPageSizeFormField: String = executionTag + ".prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = executionTag + ".pageSize"
    +
    +  override def pageNumberFormField: String = executionTag + ".page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", true),
    +        ("Duration", "", true)) ++ {
    +        if (showRunningJobs && showSucceededJobs && showFailedJobs) {
    +          Seq(("Running Job IDs", "", true), ("Succeeded Job IDs", "", true),
    +            ("Failed Job IDs", "", true))
    +        } else if (showSucceededJobs && showFailedJobs) {
    +          Seq(("Succeeded Job IDs", "", true), ("Failed Job IDs", "", true))
    +        }
    +        else {
    +          Seq(("Job IDs", "", true))
    +        }
    +      }
    +
    +    if (!executionHeadersAndCssClasses.filter(_._3).map(_._1).contains(sortColumn)) {
    +      throw new IllegalArgumentException(s"Unknown column: $sortColumn")
    +    }
    +
    +    val headerRow: Seq[Node] = {
    +      executionHeadersAndCssClasses.map { case (header, cssClass, sortable) =>
    +        if (header == sortColumn) {
    +          val headerLink = Unparsed(
    +            parameterPath +
    +              s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +              s"&$executionTag.desc=${!desc}" +
    +              s"&$executionTag.pageSize=$pageSize" +
    +              s"#$tableHeaderId")
    +          val arrow = if (desc) "&#x25BE;" else "&#x25B4;" // UP or DOWN
    +
    +          <th class={cssClass}>
    +            <a href={headerLink}>
    +              {header}<span>
    +              &nbsp;{Unparsed(arrow)}
    +            </span>
    +            </a>
    +          </th>
             } else {
    -          None
    +          if (sortable) {
    +            val headerLink = Unparsed(
    +              parameterPath +
    +                s"&$executionTag.sort=${URLEncoder.encode(header, "UTF-8")}" +
    +                s"&$executionTag.pageSize=$pageSize" +
    +                s"#$tableHeaderId")
    +
    +            <th class={cssClass}>
    +              <a href={headerLink}>
    +                {header}
    +              </a>
    +            </th>
    +          } else {
    +            <th class={cssClass}>
    +              {header}
    +            </th>
    +          }
             }
    +      }
    +    }
    +    <thead>
    +      {headerRow}
    +    </thead>
    +  }
    +
    +  override def row(executionTableRow: ExecutionTableRowData): Seq[Node] = {
    +    val executionUIData = executionTableRow.executionUIData
    +    val submissionTime = executionUIData.submissionTime
    +    val duration = executionTableRow.duration
    +
    +    def jobLinks(status: JobExecutionStatus): Seq[Node] = {
    +      executionUIData.jobs.flatMap {
    +        case (jobId, jobStatus) =>
    --- End diff --
    
    Thanks. I have used filter and map


---

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