[KYUUBI #6107] [Spark] Collect and summarize the `executorRunTime` and `executorCpuTime` of the statement #6112

XorSum · 2024-02-28T08:17:44Z

🔍 Description

Issue References 🔗

This pull request fixes #6107

Describe Your Solution 🔧

The total execution time of a statement (or a session) is the summary of the execution time of the stages belonging to the statement (or session).
The total execution time of a stage is collected from SQLOperationListener#onStageCompleted.
The total execution times of the statement or a session are stored in the engine events or output to the log.

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

…statement

codecov-commenter · 2024-02-28T10:49:55Z

Codecov Report

Attention: Patch coverage is 97.87234% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 61.13%. Comparing base (84e60e9) to head (d9efa2d).
Report is 1 commits behind head on master.

Files	Patch %	Lines
...org/apache/spark/kyuubi/SQLOperationListener.scala	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #6112   +/-   ##
=========================================
  Coverage     61.12%   61.13%           
  Complexity       23       23           
=========================================
  Files           623      624    +1     
  Lines         37195    37237   +42     
  Branches       5039     5041    +2     
=========================================
+ Hits          22737    22766   +29     
- Misses        12005    12007    +2     
- Partials       2453     2464   +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bowenliang123

LGTM overall. Is there any screenshot or logging output to show the results?

...park-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala

...park-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/session/SparkSessionImpl.scala

...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala

XorSum · 2024-02-29T03:39:53Z

LGTM overall. Is there any screenshot or logging output to show the results?

...park-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/session/SparkSessionImpl.scala

...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala

Co-authored-by: cxzl25 <[email protected]>

cxzl25

LGTM. Thank you for your contribution.
Can you update the screenshot to the description of the PR?

XorSum · 2024-02-29T06:16:52Z

LGTM. Thank you for your contribution. Can you update the screenshot to the description of the PR?

done

bowenliang123 · 2024-02-29T06:22:34Z

Thanks. Could you have another try for replacing formatDurationVerbose with formatDuration, which is in a shorter form for logging?
Sorry to brother you again.

XorSum · 2024-02-29T06:51:00Z

Thanks. Could you have another try for replacing formatDurationVerbose with formatDuration, which is in a shorter form for logging? Sorry to brother you again.

done

...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala

wForget

LGTM, Thanks.

bowenliang123

LGTM.

cxzl25 · 2024-03-04T03:23:05Z

Thanks, merged to master.

...ubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/events/SessionEvent.scala

...rk-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/events/SparkOperationEvent.scala

…of the Spark event # 🔍 Description ## Issue References 🔗 This pull request fixes #6112 (comment) ## Describe Your Solution 🔧 add comments for the newly added parameters ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #6124 from XorSum/features/features/spark-engine-cpu-time-collect-comments. Closes #6107 d93028b [bkhan] add comments Authored-by: bkhan <[email protected]> Signed-off-by: Shaoyun Chen <[email protected]>

…Spark Engine tab # 🔍 Description ## Issue References 🔗 This pull request fixes #6108 ## Describe Your Solution 🔧 Follow #6112 Display the run time and CPU time of statements or sessions in the Spark UI. <img width="1429" alt="screenshot 2024-03-05 10 37 26" src="https://github.com/apache/kyuubi/assets/23011702/337772e0-a681-4989-b6f9-ee3633bb6287"> ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #6113 from XorSum/features/spark-engine-cpu-time-UI. Closes #6108 aad2bbb [bkhan] session immediately 8e957c7 [bkhan] display run time 9018a07 [bkhan] Apply suggestions from code review 8523364 [bkhan] Display the CPU time consumed by the statement in the Engine tab Authored-by: bkhan <[email protected]> Signed-off-by: Shaoyun Chen <[email protected]>

…me` and `executorCpuTime` of the statement # 🔍 Description ## Issue References 🔗 This pull request fixes apache#6107 ## Describe Your Solution 🔧 The total execution time of a statement (or a session) is the summary of the execution time of the stages belonging to the statement (or session). The total execution time of a stage is collected from `SQLOperationListener#onStageCompleted`. The total execution times of the statement or a session are stored in the engine events or output to the log. <img width="962" alt="截屏2024-02-29 14 47 50" src="https://github.com/apache/kyuubi/assets/23011702/176df1db-bb20-428b-94b8-fa02c946fde2"> <img width="1143" alt="截屏2024-02-29 14 47 21" src="https://github.com/apache/kyuubi/assets/23011702/8cfc6a72-f6e8-45b6-bdda-30296c94c893"> ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#6112 from XorSum/features/spark-engine-cpu-time-collect. Closes apache#6107 8028005 [bkhan] check same group d9efa2d [bkhan] formatDuration a8841cd [bkhan] update 2507159 [bkhan] Apply suggestions from code review cfed2b9 [bkhan] use formatDurationVerbose 444d4aa [bkhan] Collect and summarize the executorRunTime and executorCpuTime of the statement Authored-by: bkhan <[email protected]> Signed-off-by: Shaoyun Chen <[email protected]>

…eters of the Spark event # 🔍 Description ## Issue References 🔗 This pull request fixes apache#6112 (comment) ## Describe Your Solution 🔧 add comments for the newly added parameters ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#6124 from XorSum/features/features/spark-engine-cpu-time-collect-comments. Closes apache#6107 d93028b [bkhan] add comments Authored-by: bkhan <[email protected]> Signed-off-by: Shaoyun Chen <[email protected]>

…n the Spark Engine tab # 🔍 Description ## Issue References 🔗 This pull request fixes apache#6108 ## Describe Your Solution 🔧 Follow apache#6112 Display the run time and CPU time of statements or sessions in the Spark UI. <img width="1429" alt="screenshot 2024-03-05 10 37 26" src="https://github.com/apache/kyuubi/assets/23011702/337772e0-a681-4989-b6f9-ee3633bb6287"> ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#6113 from XorSum/features/spark-engine-cpu-time-UI. Closes apache#6108 aad2bbb [bkhan] session immediately 8e957c7 [bkhan] display run time 9018a07 [bkhan] Apply suggestions from code review 8523364 [bkhan] Display the CPU time consumed by the statement in the Engine tab Authored-by: bkhan <[email protected]> Signed-off-by: Shaoyun Chen <[email protected]>

Collect and summarize the executorRunTime and executorCpuTime of the …

444d4aa

…statement

github-actions bot added the module:spark label Feb 28, 2024

XorSum mentioned this pull request Feb 28, 2024

[KYUUBI #6108] Display the CPU time consumed by the statement in the Spark Engine tab #6113

Closed

4 tasks

cxzl25 changed the title ~~[KYUUBI #6107] [FEATURE][SPARK] Collect and summarize the executorRunTime and executorCpuTime of the statement~~ [KYUUBI #6107] Collect and summarize the executorRunTime and executorCpuTime of the statement Feb 28, 2024

cxzl25 changed the title ~~[KYUUBI #6107] Collect and summarize the executorRunTime and executorCpuTime of the statement~~ [KYUUBI #6107] Collect and summarize the executorRunTime and executorCpuTime of the statement Feb 28, 2024

yaooqinn requested review from wForget and bowenliang123 February 29, 2024 01:59

bowenliang123 changed the title ~~[KYUUBI #6107] Collect and summarize the executorRunTime and executorCpuTime of the statement~~ [KYUUBI #6107] [Spark] Collect and summarize the executorRunTime and executorCpuTime of the statement Feb 29, 2024

bowenliang123 reviewed Feb 29, 2024

View reviewed changes

use formatDurationVerbose

cfed2b9

cxzl25 reviewed Feb 29, 2024

View reviewed changes

...park-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/session/SparkSessionImpl.scala Outdated Show resolved Hide resolved

cxzl25 reviewed Feb 29, 2024

View reviewed changes

...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala Outdated Show resolved Hide resolved

XorSum and others added 2 commits February 29, 2024 12:48

Apply suggestions from code review

2507159

Co-authored-by: cxzl25 <[email protected]>

update

a8841cd

cxzl25 approved these changes Feb 29, 2024

View reviewed changes

formatDuration

d9efa2d

cxzl25 assigned XorSum Feb 29, 2024

cxzl25 added this to the v1.9.0 milestone Feb 29, 2024

wForget reviewed Mar 1, 2024

View reviewed changes

...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala Outdated Show resolved Hide resolved

check same group

8028005

wForget approved these changes Mar 4, 2024

View reviewed changes

bowenliang123 approved these changes Mar 4, 2024

View reviewed changes

cxzl25 closed this in 3bc28fd Mar 4, 2024

wForget reviewed Mar 4, 2024

View reviewed changes

...ubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/events/SessionEvent.scala Show resolved Hide resolved

...rk-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/events/SparkOperationEvent.scala Show resolved Hide resolved

XorSum mentioned this pull request Mar 4, 2024

[KYUUBI #6107][FOLLOWUP] Add comments for the newly added parameters of the Spark event #6124

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #6107] [Spark] Collect and summarize the `executorRunTime` and `executorCpuTime` of the statement #6112

[KYUUBI #6107] [Spark] Collect and summarize the `executorRunTime` and `executorCpuTime` of the statement #6112

XorSum commented Feb 28, 2024 •

edited

Loading

codecov-commenter commented Feb 28, 2024 •

edited

Loading

bowenliang123 left a comment

XorSum commented Feb 29, 2024

cxzl25 left a comment

XorSum commented Feb 29, 2024

bowenliang123 commented Feb 29, 2024 •

edited

Loading

XorSum commented Feb 29, 2024

wForget left a comment

bowenliang123 left a comment

cxzl25 commented Mar 4, 2024

[KYUUBI #6107] [Spark] Collect and summarize the executorRunTime and executorCpuTime of the statement #6112

[KYUUBI #6107] [Spark] Collect and summarize the executorRunTime and executorCpuTime of the statement #6112

Conversation

XorSum commented Feb 28, 2024 • edited Loading

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

codecov-commenter commented Feb 28, 2024 • edited Loading

Codecov Report

bowenliang123 left a comment

Choose a reason for hiding this comment

XorSum commented Feb 29, 2024

cxzl25 left a comment

Choose a reason for hiding this comment

XorSum commented Feb 29, 2024

bowenliang123 commented Feb 29, 2024 • edited Loading

XorSum commented Feb 29, 2024

wForget left a comment

Choose a reason for hiding this comment

bowenliang123 left a comment

Choose a reason for hiding this comment

cxzl25 commented Mar 4, 2024

[KYUUBI #6107] [Spark] Collect and summarize the `executorRunTime` and `executorCpuTime` of the statement #6112

[KYUUBI #6107] [Spark] Collect and summarize the `executorRunTime` and `executorCpuTime` of the statement #6112

XorSum commented Feb 28, 2024 •

edited

Loading

codecov-commenter commented Feb 28, 2024 •

edited

Loading

bowenliang123 commented Feb 29, 2024 •

edited

Loading