[SPARK-34399][SQL] Add commit duration to SQL tab's graph node. #31522

AngersZhuuuu · 2021-02-08T08:59:01Z

What changes were proposed in this pull request?

Since we have add log about commit time, I think this useful and we can make user know it directly in SQL tab's UI.

Why are the changes needed?

Make user can directly know commit duration.

Does this PR introduce any user-facing change?

User can see file commit duration in SQL tab's SQL plan graph

How was this patch tested?

Mannul tested

SparkQA · 2021-02-08T14:10:22Z

Test build #135016 has finished for PR 31522 at commit d065bcd.

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

SparkQA · 2021-02-09T08:14:58Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39648/

SparkQA · 2021-02-09T08:45:28Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39648/

SparkQA · 2021-02-09T11:57:05Z

Test build #135066 has finished for PR 31522 at commit 89f8201.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait UserDefinedExpression
case class SubqueryAdaptiveBroadcastExec(
case class PlanAdaptiveDynamicPruningFilters(
case class PlanAdaptiveSubqueries(

AngersZhuuuu · 2021-02-20T03:27:37Z

Gentle ping @HeartSaVioR @dongjoon-hyun @HyukjinKwon @maropu @cloud-fan Could you help to review this I think it's really help since always INSERT statement slow caused by commit file.

SparkQA · 2021-02-20T04:53:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39868/

SparkQA · 2021-02-20T05:22:58Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39868/

SparkQA · 2021-02-20T08:48:41Z

Test build #135289 has finished for PR 31522 at commit 9ddd28c.

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

cloud-fan · 2021-02-22T13:20:23Z

The file commit is a driver side thing, why do we need to update BasicWriteJobStatsTracker? I think we can follow BroadcastExchangeExec and simply call SQLMetrics.postDriverMetricUpdates

AngersZhuuuu · 2021-02-23T03:40:49Z

The file commit is a driver side thing, why do we need to update BasicWriteJobStatsTracker? I think we can follow BroadcastExchangeExec and simply call SQLMetrics.postDriverMetricUpdates

Since we compute DataWritingCommand's metrics in driver side and all metrics stored in BasicWriteJobStatsTracker , so I changed BasicWriteJobStatsTracker.

If we want to directly update this value, we need to add it in

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala

Line 50 in 612d523

lazy val metrics: Map[String, SQLMetric] = BasicWriteJobStatsTracker.metrics

Then pass the metrics parameter to FileFormatWriter.writter() then post this metric after complete file committing.

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala

…nto SPARK-34399

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala

SparkQA · 2021-07-21T18:52:40Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45944/

SparkQA · 2021-07-21T19:40:29Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45944/

SparkQA · 2021-07-21T20:56:46Z

Test build #141422 has finished for PR 31522 at commit 8107d20.

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

SparkQA · 2021-07-21T23:10:15Z

Test build #141425 has finished for PR 31522 at commit 9b5aa94.

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

gengliangwang · 2021-07-22T12:53:49Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala

-      NUM_PARTS_KEY -> SQLMetrics.createMetric(sparkContext, "number of dynamic part")
+      NUM_PARTS_KEY -> SQLMetrics.createMetric(sparkContext, "number of dynamic part"),
+      TASK_COMMIT_TIME ->
+        SQLMetrics.createTimingMetric(sparkContext, "time of committing the tasks"),


Let's make it task commit time for short

gengliangwang · 2021-07-22T12:54:01Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala

+      NUM_PARTS_KEY -> SQLMetrics.createMetric(sparkContext, "number of dynamic part"),
+      TASK_COMMIT_TIME ->
+        SQLMetrics.createTimingMetric(sparkContext, "time of committing the tasks"),
+      JOB_COMMIT_TIME -> SQLMetrics.createTimingMetric(sparkContext, "time of committing the job")


Let's make it job commit time for short

gengliangwang

LGTM otherwise

gengliangwang · 2021-07-22T12:58:46Z

@AngersZhuuuu Thanks. Please update the screenshot in the PR description as well.

SparkQA · 2021-07-22T13:12:23Z

Test build #141495 has finished for PR 31522 at commit b5c9d63.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

AngersZhuuuu · 2021-07-22T13:35:03Z

@AngersZhuuuu Thanks. Please update the screenshot in the PR description as well.

DOne

AngersZhuuuu · 2021-07-22T13:35:11Z

retest this please

SparkQA · 2021-07-22T14:05:38Z

Test build #141501 has started for PR 31522 at commit b5c9d63.

SparkQA · 2021-07-22T14:41:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46011/

SparkQA · 2021-07-22T15:15:52Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46018/

SparkQA · 2021-07-22T15:29:32Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46011/

SparkQA · 2021-07-22T15:52:38Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46018/

dongjoon-hyun · 2021-07-22T19:17:24Z

Retest this please

SparkQA · 2021-07-22T20:07:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46034/

SparkQA · 2021-07-22T20:43:40Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46034/

SparkQA · 2021-07-22T23:39:54Z

Test build #141517 has finished for PR 31522 at commit b5c9d63.

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

cloud-fan · 2021-07-23T11:06:05Z

thanks, merging to master/3.2 (as it replaces the log we added in 3.2: #33279)

cloud-fan · 2021-07-23T11:08:09Z

oh it has conflicts. @AngersZhuuuu can you open a backport PR? thanks!

…b commit time ### What changes were proposed in this pull request? This is follow-up of #31522. It adds docs for the new metrics of task/job commit time ### Why are the changes needed? So that users can understand the metrics better and know that the new metrics are only for file table writes. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Build docs and preview: ![image](https://user-images.githubusercontent.com/1097932/127198210-2ab201d3-5fca-4065-ace6-0b930390380f.png) Closes #33542 from gengliangwang/addDocForMetrics. Authored-by: Gengliang Wang <gengliang@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…b commit time ### What changes were proposed in this pull request? This is follow-up of #31522. It adds docs for the new metrics of task/job commit time ### Why are the changes needed? So that users can understand the metrics better and know that the new metrics are only for file table writes. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Build docs and preview: ![image](https://user-images.githubusercontent.com/1097932/127198210-2ab201d3-5fca-4065-ace6-0b930390380f.png) Closes #33542 from gengliangwang/addDocForMetrics. Authored-by: Gengliang Wang <gengliang@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit c9a7ff3) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

Since we have add log about commit time, I think this useful and we can make user know it directly in SQL tab's UI. ![image](https://user-images.githubusercontent.com/46485123/126647754-dc3ba83a-5391-427c-8a67-e6af46e82290.png) Make user can directly know commit duration. User can see file commit duration in SQL tab's SQL plan graph Mannul tested Closes apache#31522 from AngersZhuuuu/SPARK-34399. Lead-authored-by: Angerszhuuuu <angers.zhu@gmail.com> Co-authored-by: AngersZhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

HyukjinKwon · 2021-12-03T01:43:57Z

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala

@@ -788,6 +792,24 @@ class SQLMetricsSuite extends SharedSparkSession with SQLMetricsTestUtils
    }
  }

+  test("SPARK-34399: Add job commit duration metrics for DataWritingCommand") {


I just happened to see the flakiness of the PR here: https://github.com/apache/spark/runs/4378978842

sbt.ForkMain$ForkError: org.scalatest.exceptions.TestFailedException: 0 was not greater than 0 at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472) at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471) at org.scalatest.Assertions$.newAssertionFailedException(Assertions.scala:1231) at org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:1295) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.$anonfun$new$87(SQLMetricsSuite.scala:810) at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18) at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:1462) at org.apache.spark.sql.test.SQLTestUtilsBase.withTable(SQLTestUtils.scala:305) at org.apache.spark.sql.test.SQLTestUtilsBase.withTable$(SQLTestUtils.scala:303) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.withTable(SQLMetricsSuite.scala:44) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.$anonfun$new$86(SQLMetricsSuite.scala:800) at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54) at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.org$apache$spark$sql$test$SQLTestUtilsBase$$super$withSQLConf(SQLMetricsSuite.scala:44) at org.apache.spark.sql.test.SQLTestUtilsBase.withSQLConf(SQLTestUtils.scala:246) at org.apache.spark.sql.test.SQLTestUtilsBase.withSQLConf$(SQLTestUtils.scala:244) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.withSQLConf(SQLMetricsSuite.scala:44) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.$anonfun$new$85(SQLMetricsSuite.scala:800) at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18) at org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite.$anonfun$test$5(AdaptiveTestUtils.scala:65) at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54) at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.org$apache$spark$sql$test$SQLTestUtilsBase$$super$withSQLConf(SQLMetricsSuite.scala:44) at org.apache.spark.sql.test.SQLTestUtilsBase.withSQLConf(SQLTestUtils.scala:246) at org.apache.spark.sql.test.SQLTestUtilsBase.withSQLConf$(SQLTestUtils.scala:244) at org.apache.spark.sql.execution.metric.SQLMetricsSuite.withSQLConf(SQLMetricsSuite.scala:44) at org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite.$anonfun$test$4(AdaptiveTestUtils.scala:65) at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18) at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85) at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83) at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104) at org.scalatest.Transformer.apply(Transformer.scala:22) at org.scalatest.Transformer.apply(Transformer.scala:20) at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:226) at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:190) at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:224) at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:236) at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306) at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:236) at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:218) at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:62) at org.scalatest.BeforeAndAfterEach.runTest(BeforeAndAfterEach.scala:234) at org.scalatest.BeforeAndAfterEach.runTest$(BeforeAndAfterEach.scala:227) at org.apache.spark.SparkFunSuite.runTest(SparkFunSuite.scala:62) at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:269)

Seems not super flaky though. I am noting it here in case other people see this failure more.

AngersZhuuuu added 3 commits February 7, 2021 11:53

Update FileBatchWrite.scala

2ee4c02

update

3fdda41

Update FileBatchWrite.scala

cb5873c

AngersZhuuuu marked this pull request as draft February 8, 2021 08:59

AngersZhuuuu added 2 commits February 8, 2021 17:00

update

0f567f0

Merge branch 'master' into SPARK-34399

d065bcd

github-actions bot added the SQL label Feb 8, 2021

AngersZhuuuu mentioned this pull request Feb 8, 2021

[SPARK-34355][CORE][SQL][FOLLOWUP] Log commit time in all File Writer #31520

Closed

Merge branch 'master' into SPARK-34399

669be65

AngersZhuuuu marked this pull request as ready for review February 9, 2021 07:09

Merge branch 'master' into SPARK-34399

89f8201

Merge branch 'master' into SPARK-34399

9ddd28c

cloud-fan reviewed Feb 23, 2021

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala Outdated Show resolved Hide resolved

AngersZhuuuu added 3 commits February 23, 2021 21:24

follow comment

e552a48

Merge branch 'SPARK-34399' of https://github.com/AngersZhuuuu/spark i…

6bf5a88

…nto SPARK-34399

Update BasicWriteStatsTracker.scala

4f40eae

cloud-fan reviewed Feb 23, 2021

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Feb 23, 2021

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Feb 23, 2021

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala Outdated Show resolved Hide resolved

follow comment

e334a4c

gengliangwang reviewed Jul 22, 2021

View reviewed changes

gengliangwang approved these changes Jul 22, 2021

View reviewed changes

Update BasicWriteStatsTracker.scala

b5c9d63

cloud-fan closed this in 3ff8c9f Jul 23, 2021

gengliangwang mentioned this pull request Jul 27, 2021

[SPARK-34399][DOCS][FOLLOWUP] Add docs for the new metrics of task/job commit time #33542

Closed

HyukjinKwon reviewed Dec 3, 2021

View reviewed changes

[SPARK-34399][SQL] Add commit duration to SQL tab's graph node. #31522

[SPARK-34399][SQL] Add commit duration to SQL tab's graph node. #31522

Uh oh!

Conversation

AngersZhuuuu commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 Feb 8, 2021

Uh oh!

SparkQA commented Feb 9, 2021

Uh oh!

SparkQA commented Feb 9, 2021

Uh oh!

SparkQA commented Feb 9, 2021

Uh oh!

AngersZhuuuu commented Feb 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Feb 20, 2021

Uh oh!

SparkQA commented Feb 20, 2021

Uh oh!

SparkQA commented Feb 20, 2021

Uh oh!

cloud-fan commented Feb 22, 2021

Uh oh!

AngersZhuuuu commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jul 21, 2021

Uh oh!

SparkQA commented Jul 21, 2021

Uh oh!

SparkQA commented Jul 21, 2021

Uh oh!

SparkQA commented Jul 21, 2021

Uh oh!

gengliangwang Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

gengliangwang Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

gengliangwang left a comment

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

AngersZhuuuu commented Jul 22, 2021

Uh oh!

AngersZhuuuu commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

SparkQA commented Jul 22, 2021

Uh oh!

dongjoon-hyun commented Jul 22, 2021

Uh oh!

AngersZhuuuu commented Feb 8, 2021 •

edited

Loading

AngersZhuuuu commented Feb 20, 2021 •

edited

Loading

AngersZhuuuu commented Feb 23, 2021 •

edited

Loading

HyukjinKwon Dec 3, 2021 •

edited

Loading