[SPARK-29543][SS][UI]Structured Streaming Web UI #26201

uncleGen · 2019-10-22T04:32:08Z

What changes were proposed in this pull request?

This PR adds two pages to Web UI for Structured Streaming:

"/streamingquery": Streaming Query Page, providing some aggregate information for running/completed streaming queries.
"/streamingquery/statistics": Streaming Query Statistics Page, providing detailed information for streaming query, including Input Rate, Process Rate, Input Rows, Batch Duration and Operation Duration

Why are the changes needed?

It helps users to better monitor Structured Streaming query.

Does this PR introduce any user-facing change?

No

How was this patch tested?

new added and existing UTs
manual test

SparkQA · 2019-10-22T04:41:19Z

Test build #112437 has finished for PR 26201 at commit 9b33018.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class SQLTab(
class StreamingQueryPage(parent: SQLTab, store: Option[HashSet[(StreamingQuery, Long)]])
class StreamingQueryStatisticsPage(
class JsCollector
class GraphUIData(

SparkQA · 2019-10-22T05:19:53Z

Test build #112440 has finished for PR 26201 at commit 1e349ac.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

uncleGen · 2019-10-22T06:30:16Z

Currently, the streaming query page entry is in SQL tab. I am not sure whether we need to add a new StreamingTab as there is already a StreamingTab which is for old version DStream. But it don't matter, we can think about it in future.

SparkQA · 2019-10-22T07:05:01Z

Test build #112444 has finished for PR 26201 at commit 5633431.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

uncleGen · 2019-10-22T07:07:21Z

retest this please.

HeartSaVioR · 2019-10-22T08:01:27Z

Amazing work! I've seen multiple requests on having tab for structured streaming be equivalent to streaming tab, and happy to see this effort.

I've just skimmed through the code (don't know about front-end tech.) and have general comment:

It may be better to define a new event for SQL and pass through listener bus, so that it could be available to SHS as well. It seems to be the right approach even we just want to expose it in running application.
Is there any chance to reuse resources for streaming tab? It's basically close to streaming tab, but looks like it doesn't reuse anything.
At least we could add UTs for API layer.
It would be great if statistic page would have query information as well on top.

uncleGen · 2019-10-22T08:22:52Z

@HeartSaVioR Thanks for your feedback

It may be better to define a new event for SQL and pass through listener bus, so that it could be available to SHS as well. It seems to be the right approach even we just want to expose it in running application.

This is a great suggestion. I agree with you. But IMHO, we can do it in a separate PR after/before this.

Is there any chance to reuse resources for streaming tab? It's basically close to streaming tab, but looks like it doesn't reuse anything.

Basically, streaming query is also a sql query, so it is reasonable to place the entry in SQL Tab. But it does not mean I object to use a streaming tab. Besides, I am not sure it is a good choice to reuse existing streaming tab.

At least we could add UTs for API layer.

I will add some UTs

It would be great if statistic page would have query information as well on top.

Sure, I will add them.

dongjoon-hyun · 2019-10-22T08:32:06Z

Thank you, @uncleGen . Nice UI. I'm very sorry that 3.0-preview misses this.
cc @gatorsmile .

SparkQA · 2019-10-22T09:01:06Z

Test build #112446 has finished for PR 26201 at commit 5633431.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR · 2019-10-22T09:05:05Z

It may be better to define a new event for SQL and pass through listener bus, so that it could be available to SHS as well. It seems to be the right approach even we just want to expose it in running application.

This is a great suggestion. I agree with you. But IMHO, we can do it in a separate PR after/before this.

Actually my point was, it seems to be a bit weird to make SQL event listener to be couple with other stuff. It's an event listener, and the best way to ingest the data into that listener is sending event. I'm not saying we should enable this in SHS right now, but it will be available smoothly if we follow the general way to ingest the data.

Is there any chance to reuse resources for streaming tab? It's basically close to streaming tab, but looks like it doesn't reuse anything.

Basically, streaming query is also a sql query, so it is reasonable to place the entry in SQL Tab. But it does not mean I object to use a streaming tab. Besides, I am not sure it is a good choice to reuse existing streaming tab.

I meant the graph between streaming tab and here would be quite similar, so curious it could be reused, maybe with refactoring if necessary.

HeartSaVioR

Still in skimmed review.

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/ProgressReporter.scala

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

uncleGen · 2019-10-22T11:05:46Z

@HeartSaVioR

I meant the graph between streaming tab and here would be quite similar, so curious it could be reused, maybe with refactoring if necessary.

Get it. If so, we should do some refactoring work, since these two code path are located in two different packages. I do not mind to do this work in a follow-up PR. What is your opinion?

Actually my point was, it seems to be a bit weird to make SQL event listener to be couple with other stuff.

Sorry, I do not get what you mean. Could you please point out the codebase where you are referring

uncleGen · 2019-10-22T11:51:12Z

Add streaming query information on top.

HeartSaVioR · 2019-10-22T12:39:24Z

Uh, I meant the query information like name, query ID, etc, as statistic page doesn't show any information on query.

HeartSaVioR · 2019-10-22T12:48:48Z

I meant the graph between streaming tab and here would be quite similar, so curious it could be reused, maybe with refactoring if necessary.

Get it. If so, we should do some refactoring work, since these two code path are located in two different packages. I do not mind to do this work in a follow-up PR. What is your opinion?

I'm OK with it. Let's reconsider if someone claims again.

Actually my point was, it seems to be a bit weird to make SQL event listener to be couple with other stuff.

Sorry, I do not get what you mean. Could you please point out the codebase where you are referring

Sorry my bad. I was confused SQLTab with SQL listener. I'd still suggest looping through SQL listener and access through SQLAppStore, but that's OK as it is once we are OK with supporting it only for Spark UI (not SHS).

gaborgsomogyi · 2019-10-22T13:00:43Z

I think it's a good addition. Started to catch up and testing...
My first suggestion is similar what @HeartSaVioR mentioned, I'm pretty sure we can add more tests since the coverage is not really high.

SparkQA · 2019-10-22T14:20:07Z

Test build #112457 has finished for PR 26201 at commit cb2b44d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR

Had a look a bit but stopped it as I wouldn't like to make myself continuously ask where the code comes from. I hope there're comments everywhere whenever it copied code from somewhere. It would save reviewers' time significantly.

Bunch of codes seem to be copied from streaming - while we review this, we might be better to find the way how to reuse them. It wouldn't be a blocker for this PR, but doing it in prior would even save times for reviewing, helps everyone.

sql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala

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

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryStatisticsPage.scala

uncleGen · 2019-10-23T10:31:04Z

I've just updated the code, including:

refactor to better reuse code and reduce duplicate code references
fix time zone bug
add query related information on top
update and add some UTs

Some updated UI:

SparkQA · 2020-01-14T05:58:20Z

Test build #116682 has finished for PR 26201 at commit 1e35c0c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-14T06:15:33Z

Test build #116681 has finished for PR 26201 at commit a0022a7.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

uncleGen · 2020-01-15T02:01:27Z

Today, I will give a commit to fix ut failure.

SparkQA · 2020-01-15T06:01:53Z

Test build #116744 has finished for PR 26201 at commit dd5ca20.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-17T08:05:02Z

Test build #116899 has finished for PR 26201 at commit dd5ca20.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

uncleGen · 2020-01-17T10:15:03Z

Kindly reminder to @sarutak @dongjoon-hyun and @zsxwing

zsxwing

The current approach looks good! Left some minor comments. Regarding the UI, it's better to make the following changes to make this page consistent with others:

Query Name -> Name.
Submit Time -> Submitted.
Last Batch Id -> Latest Batch.

zsxwing · 2020-01-16T21:48:29Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryStatisticsPage.scala

+import org.apache.spark.sql.streaming.ui.UIUtils._
+import org.apache.spark.ui.{GraphUIData, JsCollector, UIUtils => SparkUIUtils, WebUIPage}
+
+class StreamingQueryStatisticsPage(parent: StreamingQueryTab)


nit: let's make this a package private class.

zsxwing · 2020-01-16T21:48:59Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryPage.scala

+import org.apache.spark.sql.streaming.ui.UIUtils._
+import org.apache.spark.ui.{UIUtils => SparkUIUtils, WebUIPage}
+
+class StreamingQueryPage(parent: StreamingQueryTab)


nit: let's make this a package private class

zsxwing · 2020-01-17T14:47:42Z

sql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala

+   * data to show.
+   */
+  lazy val streamingQueryStatusListener: Option[StreamingQueryStatusListener] =
+    if (conf.get(UI_ENABLED)) {


Could you add one more flag spark.sql.streaming.ui.enabled for SS UI separately? Since we are adding one more streaming listener, it would be better if there is a flag to turn if off separately rather than turning off the whole Spark UI.

zsxwing · 2020-01-17T14:48:55Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryStatusListener.scala

+ * UI data for both active and inactive query.
+ * TODO: Add support for history server.
+ */
+class StreamingQueryStatusListener(sqlConf: SQLConf) extends StreamingQueryListener {


nit: please make this package private.

zsxwing · 2020-01-17T14:49:13Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryTab.scala

+import org.apache.spark.internal.Logging
+import org.apache.spark.ui.{SparkUI, SparkUITab}
+
+class StreamingQueryTab(val statusListener: StreamingQueryStatusListener, sparkUI: SparkUI)


We might need to keep this public cause it was called in SharedStatus, also StreamingQueryStatusListener.

I meant adding private[streaming] or private[sql].

Ah, thanks for clarifying, done in uncleGen#22

zsxwing · 2020-01-17T14:49:18Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryTab.scala

+  parent.addStaticHandler(StreamingQueryTab.STATIC_RESOURCE_DIR, "/static/sql")
+}
+
+object StreamingQueryTab {


zsxwing · 2020-01-17T15:00:50Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryStatusListener.scala

+    queryStatus.updateProcess(event.progress, streamingProgressRetention)
+  }
+
+  override def onQueryTerminated(event: StreamingQueryListener.QueryTerminatedEvent): Unit = {


It's better to lock the whole method. There is a race condition that allQueryStatus may miss a query that's being removed from activeQueryStatus and added to inactiveQueryStatus. Since we are locking the whole method. It's better to use synchronized directly rather than inactiveQueryStatus.synchronized.

Thanks, use synchronized directly in the next commit.

zsxwing · 2020-01-20T00:58:14Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListener.scala

   * @since 2.1.0
   */
  @Evolving
  class QueryStartedEvent private[sql](
      val id: UUID,
      val runId: UUID,
-      val name: String) extends Event
+      val name: String,
+      val submitTime: Long) extends Event


Use submissionTime to make it consistent with other places.

address comments

uncleGen · 2020-01-26T05:26:09Z

Thanks for reviewing @zsxwing and thanks for your help when i was busy with other things @xuanyuanking

SparkQA · 2020-01-26T07:22:57Z

Test build #117413 has finished for PR 26201 at commit cb1e68c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR · 2020-01-26T08:40:25Z

Could you check the test failure on the build 117413? Looks to be related.

SparkQA · 2020-01-27T10:24:24Z

Test build #117435 has finished for PR 26201 at commit 2d5f66c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

zsxwing · 2020-01-29T18:08:14Z

retest this please

SparkQA · 2020-01-29T20:50:59Z

Test build #117516 has finished for PR 26201 at commit 2d5f66c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

zsxwing · 2020-01-29T21:19:52Z

LGTM. Thanks! Merging to master.

HeartSaVioR · 2020-01-29T23:43:39Z

Finally! Thanks again for the amazing effort to get this done. This has been missed one.

### What changes were proposed in this pull request? There is a minor issue in #26201 In the streaming statistics page, there is such error ``` streaming-page.js:211 Uncaught TypeError: Cannot read property 'top' of undefined at SVGCircleElement.<anonymous> (streaming-page.js:211) at SVGCircleElement.__onclick (d3.min.js:1) ``` in the console after clicking the timeline graph. ![image](https://user-images.githubusercontent.com/1097932/76479745-14b26280-63ca-11ea-9079-0065321795f9.png) This PR is to fix it. ### Why are the changes needed? Fix the error of javascript execution. ### Does this PR introduce any user-facing change? No, the error shows up in the console. ### How was this patch tested? Manual test. Closes #27883 from gengliangwang/fixSelector. Authored-by: Gengliang Wang <gengliang.wang@databricks.com> Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>

### What changes were proposed in this pull request? There is a minor issue in #26201 In the streaming statistics page, there is such error ``` streaming-page.js:211 Uncaught TypeError: Cannot read property 'top' of undefined at SVGCircleElement.<anonymous> (streaming-page.js:211) at SVGCircleElement.__onclick (d3.min.js:1) ``` in the console after clicking the timeline graph. ![image](https://user-images.githubusercontent.com/1097932/76479745-14b26280-63ca-11ea-9079-0065321795f9.png) This PR is to fix it. ### Why are the changes needed? Fix the error of javascript execution. ### Does this PR introduce any user-facing change? No, the error shows up in the console. ### How was this patch tested? Manual test. Closes #27883 from gengliangwang/fixSelector. Authored-by: Gengliang Wang <gengliang.wang@databricks.com> Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com> (cherry picked from commit 0f46325) Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>

### What changes were proposed in this pull request? There is a minor issue in apache#26201 In the streaming statistics page, there is such error ``` streaming-page.js:211 Uncaught TypeError: Cannot read property 'top' of undefined at SVGCircleElement.<anonymous> (streaming-page.js:211) at SVGCircleElement.__onclick (d3.min.js:1) ``` in the console after clicking the timeline graph. ![image](https://user-images.githubusercontent.com/1097932/76479745-14b26280-63ca-11ea-9079-0065321795f9.png) This PR is to fix it. ### Why are the changes needed? Fix the error of javascript execution. ### Does this PR introduce any user-facing change? No, the error shows up in the console. ### How was this patch tested? Manual test. Closes apache#27883 from gengliangwang/fixSelector. Authored-by: Gengliang Wang <gengliang.wang@databricks.com> Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>

init structured streaming ui

9b33018

fix code style

1e349ac

resolve mima check

5633431

HeartSaVioR reviewed Oct 22, 2019

View reviewed changes

fix time zone and ut failure

cb2b44d

dongjoon-hyun added STRUCTURED STREAMING WEB UI labels Oct 22, 2019

HeartSaVioR reviewed Oct 22, 2019

View reviewed changes

uncleGen added 5 commits October 23, 2019 10:07

add stream query summary

3dca76d

code refactor to reduce duplicate code references

b8fc23c

fix comment: add query information on top

d2095b0

add uts

389a1c8

Merge branch 'master' of github.com:apache/spark into SPARK-29543

a2aaa46

uncleGen added 2 commits January 14, 2020 11:58

fix mima test failure

a0022a7

Merge branch 'master' of github.com:apache/spark into SPARK-29543

1e35c0c

fix ut failure

dd5ca20

zsxwing requested changes Jan 20, 2020

View reviewed changes

xuanyuanking and others added 3 commits January 22, 2020 12:31

address comments

b8a4891

private package

b3697a2

Merge pull request #22 from xuanyuanking/SPARK-29543

cb1e68c

address comments

fix ut failure

2d5f66c

zsxwing changed the title ~~[SPARK-29543][SS][UI] Init structured streaming ui~~ [SPARK-29543][SS][UI]Structured Streaming Web UI Jan 29, 2020

asfgit closed this in 7173786 Jan 29, 2020

gengliangwang mentioned this pull request Mar 12, 2020

[SPARK-31128][WebUI] Fix Uncaught TypeError in streaming statistics page #27883

Closed

[SPARK-29543][SS][UI]Structured Streaming Web UI #26201

[SPARK-29543][SS][UI]Structured Streaming Web UI #26201

Conversation

uncleGen commented Oct 22, 2019 • edited by zsxwing 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?

SparkQA commented Oct 22, 2019

SparkQA commented Oct 22, 2019

uncleGen commented Oct 22, 2019

SparkQA commented Oct 22, 2019

uncleGen commented Oct 22, 2019

HeartSaVioR commented Oct 22, 2019

uncleGen commented Oct 22, 2019 • edited Loading

dongjoon-hyun commented Oct 22, 2019

SparkQA commented Oct 22, 2019

HeartSaVioR commented Oct 22, 2019

HeartSaVioR left a comment

Choose a reason for hiding this comment

uncleGen commented Oct 22, 2019

uncleGen commented Oct 22, 2019

HeartSaVioR commented Oct 22, 2019

HeartSaVioR commented Oct 22, 2019

gaborgsomogyi commented Oct 22, 2019

SparkQA commented Oct 22, 2019

HeartSaVioR left a comment • edited Loading

Choose a reason for hiding this comment

uncleGen commented Oct 23, 2019 • edited Loading

SparkQA commented Jan 14, 2020

SparkQA commented Jan 14, 2020

uncleGen commented Jan 15, 2020

SparkQA commented Jan 15, 2020

SparkQA commented Jan 17, 2020

uncleGen commented Jan 17, 2020 • edited Loading

zsxwing 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

xuanyuanking Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uncleGen commented Jan 26, 2020 • edited Loading

SparkQA commented Jan 26, 2020

HeartSaVioR commented Jan 26, 2020

SparkQA commented Jan 27, 2020

zsxwing commented Jan 29, 2020

SparkQA commented Jan 29, 2020

zsxwing commented Jan 29, 2020

HeartSaVioR commented Jan 29, 2020

uncleGen commented Oct 22, 2019 •

edited by zsxwing

Loading

uncleGen commented Oct 22, 2019 •

edited

Loading

HeartSaVioR left a comment •

edited

Loading

uncleGen commented Oct 23, 2019 •

edited

Loading

uncleGen commented Jan 17, 2020 •

edited

Loading

xuanyuanking Jan 22, 2020 •

edited

Loading

uncleGen commented Jan 26, 2020 •

edited

Loading