[SPARK-30119][WebUI]Support Pagination for Batch Tables in Streaming Tab #26756

iRakson · 2019-12-04T07:02:54Z

What changes were proposed in this pull request?

Adding support for pagination in streaming tab for completed batch table using existing framework for pagination. Refer PR #26215

Why are the changes needed?

If our streaming job is running for long time and number of batches are huge then out of memory error may come while loading the streaming page. Introducing pagination will solve this problem and also improve the loading time of page. Besides jobs,stages,sql and thrift-server page contains pagination. So it also brings consistency.

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

Manually Tested.

Before

After

iRakson · 2019-12-04T08:11:46Z

Please review @srowen

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala

streaming/src/test/scala/org/apache/spark/streaming/UISeleniumSuite.scala

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala

iRakson · 2019-12-05T05:06:39Z

cc @shahidki31

shahidki31 · 2019-12-05T05:32:32Z

ok to test

shahidki31 · 2019-12-05T05:36:59Z

@iRakson above comments hasn't resolved?

iRakson · 2019-12-05T05:38:12Z

I will push with all the changes in few minutes.

shahidki31

Made one pass

shahidki31 · 2019-12-05T15:38:01Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+    val completedBatchTableHeaders = Seq("Batch Time", "Records", "Scheduling Delay",
+      "Processing Delay", "Total Delay", "Output Ops: Succeeded/Total")
+
+    val tooltips = Seq(None, None, Some("Time taken by Streaming scheduler to" +


Is the tooltip newly added in this PR or was that already there?

These tooltips were already there.

shahidki31 · 2019-12-05T15:40:11Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+          {formattedBatchTime}
+        </a>
+      </td>
+      <td sorttable_customkey={numRecords.toString}>{numRecords.toString} records</td>


Is it necessary to add sortable_customkey?

No. Not required. Removed this.

shahidki31 · 2019-12-05T15:42:25Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

@@ -156,40 +160,232 @@ private[ui] class ActiveBatchTable(
  }
 }

-private[ui] class CompletedBatchTable(batches: Seq[BatchUIData], batchInterval: Long)
-  extends BatchTableBase("completed-batches-table", batchInterval) {
+private[ui] class CompletedBatchTableRow(


I think CompletedBatchTableRow isn't required. You can get the data for sorting from BatchUIData itself

Oh, i missed this. I followed the format form other pages. We can use BatchUIData directly.

shahidki31 · 2019-12-05T15:42:57Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+  private def ordering(sortColumn: String, desc: Boolean): Ordering[CompletedBatchTableRow] = {
+    val ordering: Ordering[CompletedBatchTableRow] = sortColumn match {
+      case "Batch Time" => Ordering.by(_.batchTime)
+      case "Records" => Ordering.by (_.numRecords)


nit: space after by

shahidki31 · 2019-12-05T15:43:54Z

streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala

+  }
+
+  def prependBaseUri(
+    request: HttpServletRequest,


Is it necessary to add these method here?

shahidki31 · 2019-12-05T15:44:38Z

streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala

@@ -186,4 +191,21 @@ private[streaming] object UIUtils {
      </td>
    }
  }
+
+  def decodeURLParameter(urlParam: String): String = {
+    var param = urlParam


Is it necessary to add these method here?

I mistakenly added these functions instead of using imports. Thanks.

@shahidki31 all review comments fixed.

shahidki31

Are we not doing pagination support for Active Batches table?

shahidki31 · 2019-12-06T13:37:00Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

-      }
-    }
+  private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
+


I think the space between the line can be removed

shahidki31 · 2019-12-06T13:50:54Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

-        Nil
+      <td class="progress-cell">
+        {SparkUIUtils.makeProgressBar(started = batch.numActiveOutputOp,
+        completed = batch.numCompletedOutputOp, failed = batch.numFailedOutputOp, skipped = 0,


I think the same code is there in the method createOutputOperationProgressBar? Can't we use that?

shahidki31 · 2019-12-06T13:53:34Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+  }
+
+  protected def getFirstFailureReason(batches: Seq[BatchUIData]): Option[String] = {
+    batches.flatMap(_.outputOperations.flatMap(_._2.failureReason)).headOption


Is it newly added? getFirstFailureReason method already exist right? can't we reuse it?

shahidki31 · 2019-12-06T13:54:10Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+  }
+
+  protected def getFirstFailureTableCell(batch: BatchUIData): Seq[Node] = {
+    val firstFailureReason = batch.outputOperations.flatMap(_._2.failureReason).headOption


Same here. Avoid duplication of the methods

shahidki31 · 2019-12-06T13:57:11Z

Jenkins, test this please

iRakson · 2019-12-09T10:12:07Z

@shahidki31 all the methods that you mentioned are part of abstract class BatchTableBase.

shahidki31 · 2019-12-09T10:17:41Z

If we create a generic class for both Active and Completed tables, can't we avoid repetition of the same methods?. I think, I have done similar in AllExecutionPage.scala.

iRakson · 2019-12-09T10:26:51Z

At this moment i have not added pagination support for active batches. Now i will add that in this PR only. This will help us preventing code duplication as well.

…g Tab

iRakson · 2019-12-15T04:29:06Z

I have added pagination support for completed batch table as well as active batch table in streaming tab.
For both the tables there is one common class which will be used for creating both the tables. This eliminates the duplication of code.

iRakson · 2019-12-15T04:29:27Z

cc @srowen @shahidki31

shahidki31 · 2019-12-15T19:36:44Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

+      s"&$pageNumberFormField=$page" +
+      s"&$streamingBatchTag.sort=$encodedSortColumn" +
+      s"&$streamingBatchTag.desc=$desc" +
+      s"&$pageSizeFormField=$pageSize"


Don't we need to add #tableHeaderId here?

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala

Line 259 in 67b644c

s"#$tableHeaderId"

shahidki31 · 2019-12-15T19:37:55Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

-        {SparkUIUtils.tooltip("Time taken to process all jobs of a batch", "top")}</th>
+  override def goButtonFormPath: String = {
+    val encodedSortColumn = URLEncoder.encode(sortColumn, UTF_8.name())
+    s"$parameterPath&$streamingBatchTag.sort=$encodedSortColumn&$streamingBatchTag.desc=$desc"


Here also it seems we need to add #tableHeaderId ?

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala

Line 268 in 67b644c

s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"

I will add the tableHeaderId in the link.

shahidki31 · 2019-12-15T19:38:42Z

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

-        failureReasonForUI, rowspan = 1, includeFirstLineInExpandDetails = false)
-    }.getOrElse(<td>-</td>)
+  override def headers: Seq[Node] = {
+    val completedBatchTableHeaders = Seq("Batch Time", "Records", "Scheduling Delay",


Is the headers only for completedBatchTables?

Both the tables are identical i.e. the schema is same for both. So headers will remain same for both. But yeah, completedBatchTableHeaders is misleading. So, i will update the variable name.

From the attached screenshot, I can see Total Delay isn't there in the ActiveBatches table and Status field isn't there in the CompletedBatches table?

shahidki31 · 2019-12-15T19:56:18Z

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala

    val runningBatches = listener.runningBatches.sortBy(_.batchTime.milliseconds).reverse
    val waitingBatches = listener.waitingBatches.sortBy(_.batchTime.milliseconds).reverse
    val completedBatches = listener.retainedCompletedBatches.
      sortBy(_.batchTime.milliseconds).reverse
+    val activeBatchData = waitingBatches ++ runningBatches


Can we simply append the two table data? Could you please check the output of the pages before and after the PR, which contains both runningBatches and waitingBatches?

Previously, running batches and waiting batches were shown in the same table (Active Batches Table) too.
To ensure that property only i appended the data of both running batches and waiting batches.

I am not sure we can simply append the table. Please refer earlier code. To check, if there is no change, could you please attach screenshot for that (before and after PR)

spark/streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala

Lines 130 to 135 in 1fc353d

override protected def renderRows: Seq[Node] = {

// The "batchTime"s of "waitingBatches" must be greater than "runningBatches"'s, so display

// waiting batches before running batches

waitingBatches.flatMap(batch => <tr>{waitingBatchRow(batch)}</tr>) ++

runningBatches.flatMap(batch => <tr>{runningBatchRow(batch)}</tr>)

}

github-actions · 2020-03-26T00:10:24Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

iRakson changed the title ~~[SPARK-30119][WebUI]Support Pagination for Completed Batch Table in Streamin…~~ [SPARK-30119][WebUI]Support Pagination for Completed Batch Table in Streaming Tab Dec 4, 2019

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/AllBatchesTable.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/test/scala/org/apache/spark/streaming/UISeleniumSuite.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala Outdated Show resolved Hide resolved

amanomer reviewed Dec 4, 2019

View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala Outdated Show resolved Hide resolved

shahidki31 reviewed Dec 5, 2019

View reviewed changes

dongjoon-hyun added the WEB UI label Dec 5, 2019

shahidki31 reviewed Dec 6, 2019

View reviewed changes

iRakson added 4 commits December 14, 2019 13:57

[SPARK-30119]Support Pagination for Completed Batch Table in Streamin…

69b8a40

…g Tab

[SPARK-30119] Streaming Pagination: Fix Review Comments

c7cfde2

[SPARK-30119] Streaming Pagination: Fix Review Comments

cdea7c3

[SPARK-30234]Fix Review Comments

84a5e77

iRakson force-pushed the streaming_pagination branch from 796293f to 84a5e77 Compare December 14, 2019 08:29

[SPARK-30119]Fix

7ec80b9

iRakson changed the title ~~[SPARK-30119][WebUI]Support Pagination for Completed Batch Table in Streaming Tab~~ [SPARK-30119][WebUI]Support Pagination for Batch Tables in Streaming Tab Dec 15, 2019

shahidki31 reviewed Dec 15, 2019

View reviewed changes

github-actions bot added the Stale label Mar 26, 2020

github-actions bot closed this Mar 27, 2020

	override protected def renderRows: Seq[Node] = {
	// The "batchTime"s of "waitingBatches" must be greater than "runningBatches"'s, so display
	// waiting batches before running batches
	waitingBatches.flatMap(batch => <tr>{waitingBatchRow(batch)}</tr>) ++
	runningBatches.flatMap(batch => <tr>{runningBatchRow(batch)}</tr>)
	}

[SPARK-30119][WebUI]Support Pagination for Batch Tables in Streaming Tab #26756

[SPARK-30119][WebUI]Support Pagination for Batch Tables in Streaming Tab #26756

Conversation

iRakson commented Dec 4, 2019 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

iRakson commented Dec 4, 2019

iRakson commented Dec 5, 2019

shahidki31 commented Dec 5, 2019

shahidki31 commented Dec 5, 2019

iRakson commented Dec 5, 2019

shahidki31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahidki31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahidki31 commented Dec 6, 2019

iRakson commented Dec 9, 2019

shahidki31 commented Dec 9, 2019 • edited Loading

iRakson commented Dec 9, 2019

iRakson commented Dec 15, 2019

iRakson commented Dec 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 26, 2020

iRakson commented Dec 4, 2019 •

edited

Loading

shahidki31 commented Dec 9, 2019 •

edited

Loading