[SPARK-31882][WEBUI] DAG-viz is not rendered correctly with pagination #28690

sarutak · 2020-06-01T04:41:01Z

What changes were proposed in this pull request?

This PR fix an issue related to DAG-viz.
Because DAG-viz for a job fetches link urls for each stage from the stage table, rendering can fail with pagination.
You can reproduce this issue with the following operation.

sc.parallelize(1 to 10).map(value => (value ,value)).repartition(1).repartition(1).repartition(1).reduceByKey(_ + _).collect

And then, visit the corresponding job page.
There are 5 stages so show <5 stages in the paged table.

Why are the changes needed?

This is a bug.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Newly added test case with following command.
build/sbt -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite -- -z SPARK-31882"

SparkQA · 2020-06-01T08:11:12Z

Test build #123360 has finished for PR 28690 at commit 208cffc.

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

HyukjinKwon · 2020-06-02T01:42:56Z

Looks okay. cc @gengliangwang FYI

gengliangwang · 2020-06-02T05:56:54Z

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

    <html>
      <head>
        {commonHeaderNodes(request)}
+        <script>setAppBasePath('{activeTab.basePath}')</script>


I think we can move this line to function commonHeaderNodes, where the method setUIRoot is called.

At first, I considered the way you mention but to do so, we need to have an additional parameter in commonHeaderNodes to pass activeTab only for this purpose.
Which do you think is better?

I see. Let's keep it this way. The method basicSparkPage also calls the function commonHeaderNodes but there is no activeTab in it.

gengliangwang

LGTM except one comment

gengliangwang · 2020-06-02T06:13:05Z

@sarutak Does the bug exist in branch 3.0?

sarutak · 2020-06-02T06:40:18Z

@gengliangwang I've confirmed this bug exists in branch 3.0 too.

gengliangwang · 2020-06-02T06:41:48Z

Then, let's merge to both master and 3.0 :)

sarutak · 2020-06-02T06:45:23Z

Yeah, but #28627 isn't merged to branch-3.0 so we need to back port it first or open another PR for branch-3.0 to back port this change without the testcase.

sarutak · 2020-06-02T06:54:26Z

Anyway, I'll open a PR for backporting #28627.

gengliangwang · 2020-06-02T06:58:10Z

Awesome, let's merge this one after #28627 is backported 👍

### What changes were proposed in this pull request? This PR backports the change of #28627 . Some fixes for UI like #28690 should be merged to `branch-3.0` including testcases so #28627 is need to be backported. This PR mainly adds two things. 1. Real headless browser support for UI test 2. A test suite using headless Chrome as one instance of those browsers. Also, for environment where Chrome and Chrome driver is not installed, `ChromeUITest` tag is added to filter out the test suite. By default, test suites with `ChromeUITest` is disabled. ### Why are the changes needed? In `branch-3.0`, there are two problems for UI test. 1. Lots of tests especially JavaScript related ones are done manually. Appearance is better to be confirmed by our eyes but logic should be tested by test cases ideally. 2. Compared to the real web browsers, HtmlUnit doesn't seem to support JavaScript enough. I added a JavaScript related test before for SPARK-31534 using HtmlUnit which is simple library based headless browser for test. The test I added works somehow but some JavaScript related error is shown in unit-tests.log. ``` ======= EXCEPTION START ======== Exception class=[net.sourceforge.htmlunit.corejs.javascript.JavaScriptException] com.gargoylesoftware.htmlunit.ScriptException: Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)". (http://192.168.1.209:60724/static/jquery-3.4.1.min.js#2) at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:904) at net.sourceforge.htmlunit.corejs.javascript.Context.call(Context.java:628) at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.call(ContextFactory.java:515) at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine.callFunction(JavaScriptEngine.java:835) at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine.callFunction(JavaScriptEngine.java:807) at com.gargoylesoftware.htmlunit.InteractivePage.executeJavaScriptFunctionIfPossible(InteractivePage.java:216) at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptFunctionJob.runJavaScript(JavaScriptFunctionJob.java:52) at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptExecutionJob.run(JavaScriptExecutionJob.java:102) at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptJobManagerImpl.runSingleJob(JavaScriptJobManagerImpl.java:426) at com.gargoylesoftware.htmlunit.javascript.background.DefaultJavaScriptExecutor.run(DefaultJavaScriptExecutor.java:157) at java.lang.Thread.run(Thread.java:748) Caused by: net.sourceforge.htmlunit.corejs.javascript.JavaScriptException: Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)". (http://192.168.1.209:60724/static/jquery-3.4.1.min.js#2) at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpretLoop(Interpreter.java:1009) at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpret(Interpreter.java:800) at net.sourceforge.htmlunit.corejs.javascript.InterpretedFunction.call(InterpretedFunction.java:105) at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.doTopCall(ContextFactory.java:413) at com.gargoylesoftware.htmlunit.javascript.HtmlUnitContextFactory.doTopCall(HtmlUnitContextFactory.java:252) at net.sourceforge.htmlunit.corejs.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3264) at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$4.doRun(JavaScriptEngine.java:828) at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:889) ... 10 more JavaScriptException value = Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)". == CALLING JAVASCRIPT == function () { throw e; } ======= EXCEPTION END ======== ``` I tried to upgrade HtmlUnit to 2.40.0 but what is worse, the test become not working even though it works on real browsers like Chrome, Safari and Firefox without error. ``` [info] UISeleniumSuite: [info] - SPARK-31534: text for tooltip should be escaped *** FAILED *** (17 seconds, 745 milliseconds) [info] The code passed to eventually never returned normally. Attempted 2 times over 12.910785232 seconds. Last failure message: com.gargoylesoftware.htmlunit.ScriptException: ReferenceError: Assignment to undefined "regeneratorRuntime" in strict mode (http://192.168.1.209:62132/static/vis-timeline-graph2d.min.js#52(Function)#1) ``` To resolve those problems, it's better to support headless browser for UI test. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I tested with following patterns. Both Chrome and Chrome driver should be installed to test. 1. sbt / with default excluded tags (ChromeUISeleniumSuite is expected to be skipped and SQLQueryTestSuite is expected to succeed) `build/sbt -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite org.apache.spark.sql.SQLQueryTestSuite"` 2. sbt / overwrite default excluded tags as empty string (Both suites are expected to succeed) `build/sbt -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite org.apache.spark.sql.SQLQueryTestSuite"` 3. sbt / set `test.exclude.tags` to `org.apache.spark.tags.ExtendedSQLTest` (Both suites are expected to be skipped) `build/sbt -Dtest.exclude.tags=org.apache.spark.tags.ExtendedSQLTest -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite org.apache.spark.sql.SQLQueryTestSuite"` 4. Maven / with default excluded tags (ChromeUISeleniumSuite is expected to be skipped and SQLQueryTestSuite is expected to succeed) `build/mvn -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite,org.apache.spark.sql.SQLQueryTestSuite test` 5. Maven / overwrite default excluded tags as empty string (Both suites are expected to succeed) `build/mvn -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite,org.apache.spark.sql.SQLQueryTestSuite test` 6. Maven / set `test.exclude.tags` to `org.apache.spark.tags.ExtendedSQLTest` (Both suites are expected to be skipped) `build/mvn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedSQLTest -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite,org.apache.spark.sql.SQLQueryTestSuite test` Closes #28702 from sarutak/real-headless-browser-support-3.0. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

gengliangwang · 2020-06-02T23:44:33Z

Thanks, merging to master/3.0

### What changes were proposed in this pull request? This PR fix an issue related to DAG-viz. Because DAG-viz for a job fetches link urls for each stage from the stage table, rendering can fail with pagination. You can reproduce this issue with the following operation. ``` sc.parallelize(1 to 10).map(value => (value ,value)).repartition(1).repartition(1).repartition(1).reduceByKey(_ + _).collect ``` And then, visit the corresponding job page. There are 5 stages so show <5 stages in the paged table. <img width="1440" alt="dag-rendering-issue1" src="https://user-images.githubusercontent.com/4736016/83376286-c29f3d00-a40c-11ea-891b-eb8f42afbb27.png"> <img width="1439" alt="dag-rendering-issue2" src="https://user-images.githubusercontent.com/4736016/83376288-c3d06a00-a40c-11ea-8bb2-38542e5010c1.png"> ### Why are the changes needed? This is a bug. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Newly added test case with following command. `build/sbt -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite -- -z SPARK-31882"` Closes #28690 from sarutak/fix-dag-rendering-issue. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Gengliang Wang <[email protected]> (cherry picked from commit 271eb26) Signed-off-by: Gengliang Wang <[email protected]>

This PR fix an issue related to DAG-viz. Because DAG-viz for a job fetches link urls for each stage from the stage table, rendering can fail with pagination. You can reproduce this issue with the following operation. ``` sc.parallelize(1 to 10).map(value => (value ,value)).repartition(1).repartition(1).repartition(1).reduceByKey(_ + _).collect ``` And then, visit the corresponding job page. There are 5 stages so show <5 stages in the paged table. <img width="1440" alt="dag-rendering-issue1" src="https://user-images.githubusercontent.com/4736016/83376286-c29f3d00-a40c-11ea-891b-eb8f42afbb27.png"> <img width="1439" alt="dag-rendering-issue2" src="https://user-images.githubusercontent.com/4736016/83376288-c3d06a00-a40c-11ea-8bb2-38542e5010c1.png"> This is a bug. No. Newly added test case with following command. `build/sbt -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite -- -z SPARK-31882"` Closes apache#28690 from sarutak/fix-dag-rendering-issue. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

…timeline view ### What changes were proposed in this pull request? Fix [SPARK-32886](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-32886) in branch-2.4. i cherry-pick [29757](#29757) and partial [28690](#28690 part is ignored as conflict), which PR `29757` has dependency on PR `28690`. This change fixes two below issues in branch-2.4. [SPARK-31882: DAG-viz is not rendered correctly with pagination.](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-31882) [SPARK-32886: '.../jobs/undefined' link from "Event Timeline" in jobs page](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-32886) ### Why are the changes needed? sarutak found `29757` has dependency on `28690`. If we only merge `29757` to 2.4 branch, it would cause UI break. And I verified both issues mentioned in [SPARK-32886](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-32886) and [SPARK-31882](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-31882) exist in branch-2.4. So i cherry pick them to branch 2.4 in same PR. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually tested. ![dag](https://user-images.githubusercontent.com/10524738/93854440-9c770480-fc6a-11ea-9009-0ee68ef090e1.JPG) ![eventTLJPG](https://user-images.githubusercontent.com/10524738/93854447-a13bb880-fc6a-11ea-9b34-73623fa59def.JPG) Closes #29833 from zhli1142015/cherry-pick-fix-for-31882-32886. Lead-authored-by: Zhen Li <[email protected]> Co-authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Sean Owen <[email protected]>

sarutak added 2 commits May 23, 2020 21:00

Fixed the dag rendering issue.

33251a5

Added a test case.

208cffc

probot-autolabeler bot added CORE WEB UI labels Jun 1, 2020

srowen approved these changes Jun 1, 2020

View reviewed changes

gengliangwang reviewed Jun 2, 2020

View reviewed changes

gengliangwang approved these changes Jun 2, 2020

View reviewed changes

sarutak mentioned this pull request Jun 2, 2020

[SPARK-31756][WEBUI][3.0] Add real headless browser support for UI test #28702

Closed

gengliangwang closed this in 271eb26 Jun 2, 2020

zhli1142015 mentioned this pull request Sep 22, 2020

[SPARK-32886][SPARK-31882][WEBUI][2.4] fix 'undefined' link in event timeline view #29833

Closed

[SPARK-31882][WEBUI] DAG-viz is not rendered correctly with pagination #28690

[SPARK-31882][WEBUI] DAG-viz is not rendered correctly with pagination #28690

Uh oh!

Conversation

sarutak commented Jun 1, 2020

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?

Uh oh!

SparkQA commented Jun 1, 2020

Uh oh!

HyukjinKwon commented Jun 2, 2020

Uh oh!

gengliangwang Jun 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarutak Jun 2, 2020

Choose a reason for hiding this comment

Uh oh!

gengliangwang Jun 2, 2020

Choose a reason for hiding this comment

Uh oh!

gengliangwang left a comment

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented Jun 2, 2020

Uh oh!

sarutak commented Jun 2, 2020

Uh oh!

gengliangwang commented Jun 2, 2020

Uh oh!

sarutak commented Jun 2, 2020

Uh oh!

sarutak commented Jun 2, 2020

Uh oh!

gengliangwang commented Jun 2, 2020

Uh oh!

gengliangwang commented Jun 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gengliangwang Jun 2, 2020 •

edited

Loading