Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented Dec 19, 2020

What changes were proposed in this pull request?

This is a followup PR for #30573 .

After this change applied, stage memory metrics will be updated on stage end.

Why are the changes needed?

After #30573, executor memory metrics is updated on stage end but stage memory metrics is not updated.
It's better to update both metrics like updateStageLevelPeakExecutorMetrics does.

Does this PR introduce any user-facing change?

Yes. stage memory metrics is updated more accurately.

How was this patch tested?

After I run a job and visited /api/v1/<appid>/stages, I confirmed peakExecutorMemory metrics is shown even though the life time of each stage is very short .
I also modify the json files for HistoryServerSuite.

@github-actions github-actions bot added the CORE label Dec 19, 2020
@SparkQA
Copy link

SparkQA commented Dec 19, 2020

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

@SparkQA
Copy link

SparkQA commented Dec 19, 2020

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

@SparkQA
Copy link

SparkQA commented Dec 19, 2020

Test build #133083 has finished for PR 30858 at commit 69bfa78.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @sarutak . Merged to master/3.1.

cc @AngersZhuuuu since he is the original author.

dongjoon-hyun pushed a commit that referenced this pull request Dec 21, 2020
### What changes were proposed in this pull request?

This is a followup PR for #30573 .

After this change applied, stage memory metrics will be updated on stage end.

### Why are the changes needed?

After #30573, executor memory metrics is updated on stage end but stage memory metrics is not updated.
It's better to update both metrics like `updateStageLevelPeakExecutorMetrics` does.

### Does this PR introduce _any_ user-facing change?

Yes. stage memory metrics is updated more accurately.

### How was this patch tested?

After I run a job and visited `/api/v1/<appid>/stages`, I confirmed `peakExecutorMemory` metrics is shown even though the life time of each stage is very short .
I also modify the json files for `HistoryServerSuite`.

Closes #30858 from sarutak/followup-SPARK-26341.

Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 8e26339)
Signed-off-by: Dongjoon Hyun <[email protected]>
@AngersZhuuuu
Copy link
Contributor

Sorry for late reply, it's a good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants