-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-23431][CORE] Expose stage level peak executor metrics via REST API #29020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #125188 has finished for PR 29020 at commit
|
|
@imback82 Thank you! |
|
retest this please |
|
Test build #125244 has finished for PR 29020 at commit
|
|
Test build #125246 has finished for PR 29020 at commit
|
|
Test build #125278 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125309 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125328 has started for PR 29020 at commit |
|
Retest this please |
|
Test build #125387 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125390 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125508 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125517 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125544 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #125636 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #126070 has finished for PR 29020 at commit
|
|
@gengliangwang gentle ping |
|
Retest this please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @imback82 .
For the following your comment, please add an empty commit with author @edwinalu . Then, Apache Spark script will make him as a co-author.
I will also ask a reviewing committer to put @edwinalu as a co-author if that's the practice when a PR is "inherited". Thanks!
To committers, please don't merge this PR until this has a correct authorship.
|
To @gengliangwang, please try to recommend the authorship commit explicitly in the Apache Spark community. Otherwise, it's forgot frequently during merging steps. Thanks! |
|
Thanks @dongjoon-hyun for the reminder! I added @edwinalu as a co-author with: |
|
@imback82 , thanks for all your work on this! |
|
Test build #126509 has finished for PR 29020 at commit
|
|
Test build #126512 has finished for PR 29020 at commit
|
|
In the PR description, "This PR proposes to expose the peak executor metrics at the stage level via the REST API (/applications/<application_id>/stages/ or /applications/<application_id>/stages/<stage_id>/<stage_attemp_id>)." there is a typo; "<stage_attemp_id>" should be "<stage_attempt_id>". It might be helpful to clarify that the peak values of executor metrics for the stage are shown for each stage in the first (/applications/<application_id>/stages/) and peak values of executor metrics for each executor for the stage are shown for the second /applications/<application_id>/stages/<stage_id>/<stage_attempt_id>). In other words, the first shows per-stage peaks, and the second shows per-executor per-stage peaks (for just that stage). "Peak values for metrics are show for each stage. For executor summaries for each stage, the peak values per executor are also shown." |
|
Thanks @wypoon for the suggestion. I updated the description, and hopefully that clarifies things a bit more. |
|
@dongjoon-hyun Thanks for the reminder! |
|
retest this please. |
|
@imback82 I saw this error from github action tests: Could you rebase the PR to the latest master? I will merge it once the tests are passed. Thanks! |
|
Test build #127024 has finished for PR 29020 at commit
|
|
Test build #127027 has finished for PR 29020 at commit
|
|
retest this please |
|
Test build #127043 has finished for PR 29020 at commit
|
|
Thanks, merging to master |
|
Thank you, @imback82 and @gengliangwang ! |
|
Thanks everyone! |
What changes were proposed in this pull request?
Note that this PR is forked from #23340 originally written by @edwinalu.
This PR proposes to expose the peak executor metrics at the stage level via the REST APIs:
/applications/<application_id>/stages/: peak values of executor metrics for each stage/applications/<application_id>/stages/<stage_id>/< stage_attempt_id >: peak values of executor metrics for each executor for the stage, followed by peak values of executor metrics for the stageWhy are the changes needed?
The stage level peak executor metrics can help better understand your application's resource utilization.
Does this PR introduce any user-facing change?
/applications/<application_id>/stages/API, you will see the following new info for each stage:/applications/<application_id>/stages/<stage_id>/<stage_attempt_id>API, you will see the following new info for each executor underexecutorSummary:, and the following at the stage level:
How was this patch tested?
Added tests.