[SPARK-28647][WEBUI] Recover additional metric feature and remove additional-metrics.js #25374

sarutak · 2019-08-07T09:08:28Z

What changes were proposed in this pull request?

By SPARK-17019, On Heap Memory and Off Heap Memory are introduced as optional metrics.
But they are not displayed because they are made display: none in css and there are no way to appear them.

I know #22595 also try to resolve this issue but that will use additional-metrics.js.
Initially, additional-metrics.js is created for StagePage but StagePage currently uses stagepage.js for its additional metrics to be toggle because DataTable (one of jQuery plugins) was introduced and we needed another mechanism to add/remove columns for additional metrics.

Now that ExecutorsPage also uses DataTable so it might be better to introduce same mechanism as StagePage for additional metrics.

And then, we can remove additional-metrics.js which is no longer used from anywhere.

How was this patch tested?

After this change is applied, I confirmed ExecutorsPage and StagePage are properly rendered and all checkboxes for additional metrics work.

SparkQA · 2019-08-07T11:48:14Z

Test build #108757 has finished for PR 25374 at commit a8b3328.

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

dongjoon-hyun

Hi, @sarutak .
So, do you mean the feature is refactored correctly?

expand-additional-metrics is used [SPARK-17019][CORE] Expose on-heap and off-heap memory usage in various places (a449162).
According to the comment, #22595 (comment), and your comment, it's not working as of now.

I'm wondering if we keep the same feature which SPARK-17019 aimed originally. Could you give me some pointer? For GPU support, #25037 wants to this additional metric features. So, I want to be careful before removal.

cc @jerryshao , @tgravescs

dongjoon-hyun · 2019-08-07T17:05:40Z

Also, could you file a JIRA issue since this might cause some issues later? JIRA ID is a better pointer than the git hash in that case.

tgravescs · 2019-08-07T18:07:09Z

I agree with @dongjoon-hyun please file a jira for this. It does looks like its something I missed when we did the StagePage.

I kind of think the new PR is probably easier for users to read with separate columns but your point is valid, it would be nice to know if there is anything else missing there. Also the last comments on the new PR are to separate out columns, not sure if this code would help with that at all, but I'm guessing we can reuse code from the Stage Page if you want to keep the look the same.

srowen

Looks OK to me.

sarutak · 2019-08-07T19:12:09Z

OK, I've filed.
@dongjoon-hyun

expand-additional-metrics is used [SPARK-17019][CORE] Expose on-heap and off-heap memory usage in various places (a449162).

It seems expand-additionl-metrics is not used in the current code but...

According to the comment, #22595 (comment), and your comment, it's not working as of now.
Yes, this PR seems to conflict with #22595.

So, how about having check-boxes for toggle like implemented in Stage Page?

unidashinc · 2019-08-07T19:32:57Z

LGTM

SparkQA · 2019-08-07T21:44:20Z

Test build #108782 has finished for PR 25374 at commit 3c7b9d4.

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

SparkQA · 2019-08-07T21:57:26Z

Test build #108783 has finished for PR 25374 at commit a8b3328.

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

sarutak · 2019-08-09T20:43:26Z

@dongjoon-hyun If we need a mechanism for optional metrics for ExecutorPage, we can do like the latest commit and we can get like a folowing UI.

SparkQA · 2019-08-09T23:05:06Z

Test build #108895 has finished for PR 25374 at commit b5be743.

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

srowen · 2019-08-11T13:10:19Z

@sarutak go ahead and update the title/description to reflect what this is adding now

sarutak · 2019-08-12T07:38:42Z

@srowen Thanks for reminding me. I've updated.

dongjoon-hyun

+1, LGTM. Great! Thank you, @sarutak . I checked the fixed UI.

Also, thank you, @srowen , @tgravescs , @hacker131 .

Merged to master.

dongjoon-hyun · 2019-08-13T00:05:16Z

Could you make a backporting PR please, @sarutak ? There is a conflict on branch-2.4.

sarutak · 2019-08-13T16:02:33Z

@dongjoon-hyun O.K. I'll do it this week.

dongjoon-hyun · 2019-08-13T20:32:27Z

Thank you so much, @sarutak !

This PR is for backporting SPARK-28647(#25374) to branch-2.4. The original PR removed `additional-metrics.js` but branch-2.4 still uses it so I don't remove it and related things for branch-2.4. ### What changes were proposed in this pull request? Added checkboxes to enable users to select which optional metrics (`On Heap Memory`, `Off Heap Memory` and `Select All` in this case) to be shown in `ExecuorPage`. ### Why are the changes needed? By SPARK-17019, `On Heap Memory` and `Off Heap Memory` are introduced as optional metrics. But they are not displayed because they are made `display: none` in css and there are no way to appear them. ### Does this PR introduce any user-facing change? The previous `ExecutorPage` doesn't show optional metrics. This change adds checkboxes to `ExecutorPage` for optional metrics. We can choose which metrics should be shown by checking corresponding checkboxes. ![Screenshot from 2019-08-18 03-56-09](https://user-images.githubusercontent.com/4736016/63216148-2bfadb80-c16c-11e9-81e1-e1e66198dd6c.png) ### How was this patch tested? Manual test. Closes #25484 from sarutak/backport-SPARK-28647-branch-2.4. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

This PR is for backporting SPARK-28647(apache#25374) to branch-2.4. The original PR removed `additional-metrics.js` but branch-2.4 still uses it so I don't remove it and related things for branch-2.4. ### What changes were proposed in this pull request? Added checkboxes to enable users to select which optional metrics (`On Heap Memory`, `Off Heap Memory` and `Select All` in this case) to be shown in `ExecuorPage`. ### Why are the changes needed? By SPARK-17019, `On Heap Memory` and `Off Heap Memory` are introduced as optional metrics. But they are not displayed because they are made `display: none` in css and there are no way to appear them. ### Does this PR introduce any user-facing change? The previous `ExecutorPage` doesn't show optional metrics. This change adds checkboxes to `ExecutorPage` for optional metrics. We can choose which metrics should be shown by checking corresponding checkboxes. ![Screenshot from 2019-08-18 03-56-09](https://user-images.githubusercontent.com/4736016/63216148-2bfadb80-c16c-11e9-81e1-e1e66198dd6c.png) ### How was this patch tested? Manual test. Closes apache#25484 from sarutak/backport-SPARK-28647-branch-2.4. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

### What changes were proposed in this pull request? On clicking job description in jobs page, the description was not shown fully. Add the function for the click event on description. ### Why are the changes needed? when there is a long description of a job, it cannot be seen fully in the UI. The feature was added in #24145 But it is missed after #25374 Before change: ![Screenshot from 2019-10-23 11-23-00](https://user-images.githubusercontent.com/51401130/67361914-827b0080-f587-11e9-9181-d49a6a836046.png) After change: on Double click over decription ![Screenshot from 2019-10-23 11-20-02](https://user-images.githubusercontent.com/51401130/67361936-932b7680-f587-11e9-9e59-d290abed4b70.png) ### Does this PR introduce any user-facing change? No ### How was this patch tested? Manually test Closes #26222 from PavithraRamachandran/jobs_description_tooltip. Authored-by: Pavithra Ramachandran <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

On clicking job description in jobs page, the description was not shown fully. Add the function for the click event on description. when there is a long description of a job, it cannot be seen fully in the UI. The feature was added in apache#24145 But it is missed after apache#25374 Before change: ![Screenshot from 2019-10-23 11-23-00](https://user-images.githubusercontent.com/51401130/67361914-827b0080-f587-11e9-9181-d49a6a836046.png) After change: on Double click over decription ![Screenshot from 2019-10-23 11-20-02](https://user-images.githubusercontent.com/51401130/67361936-932b7680-f587-11e9-9e59-d290abed4b70.png) No Manually test Closes apache#26222 from PavithraRamachandran/jobs_description_tooltip. Authored-by: Pavithra Ramachandran <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

Remove additional-metrics.js

a8b3328

dongjoon-hyun added the WEB UI label Aug 7, 2019

dongjoon-hyun reviewed Aug 7, 2019

View reviewed changes

sarutak changed the title ~~[WEBUI] Remove additional-metrics.js~~ [SPARK-28647][WEBUI] Remove additional-metrics.js Aug 7, 2019

srowen reviewed Aug 7, 2019

View reviewed changes

sarutak force-pushed the remove-additional-metrics.js branch from 3c7b9d4 to a8b3328 Compare August 7, 2019 19:24

Add checkbox to ExecutorsPage for optional metrics

b5be743

srowen approved these changes Aug 12, 2019

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-28647][WEBUI] Remove additional-metrics.js~~ [SPARK-28647][WEBUI] Recover additional metric feature and remove additional-metrics.js Aug 12, 2019

dongjoon-hyun approved these changes Aug 13, 2019

View reviewed changes

dongjoon-hyun closed this in 25857c6 Aug 13, 2019

sarutak mentioned this pull request Aug 17, 2019

[SPARK-28647][WEBUI][2.4] Recover additional metric feature #25484

Closed

PavithraRamachandran mentioned this pull request Oct 23, 2019

[SPARK-29504][WebUI] Toggle full job description on click #26222

Closed

PavithraRamachandran mentioned this pull request Oct 31, 2019

[SPARK-29504][WEBUI] Toggle full job description on click #26336

Closed

[SPARK-28647][WEBUI] Recover additional metric feature and remove additional-metrics.js #25374

[SPARK-28647][WEBUI] Recover additional metric feature and remove additional-metrics.js #25374

Uh oh!

Conversation

sarutak commented Aug 7, 2019 • edited by dongjoon-hyun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

dongjoon-hyun left a comment • 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!

dongjoon-hyun commented Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgravescs commented Aug 7, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

sarutak commented Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

unidashinc commented Aug 7, 2019

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

sarutak commented Aug 9, 2019

Uh oh!

SparkQA commented Aug 9, 2019

Uh oh!

srowen commented Aug 11, 2019

Uh oh!

sarutak commented Aug 12, 2019

Uh oh!

dongjoon-hyun left a comment • 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!

dongjoon-hyun commented Aug 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarutak commented Aug 13, 2019

Uh oh!

dongjoon-hyun commented Aug 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sarutak commented Aug 7, 2019 •

edited by dongjoon-hyun

Loading

dongjoon-hyun left a comment •

edited

Loading

dongjoon-hyun commented Aug 7, 2019 •

edited

Loading

sarutak commented Aug 7, 2019 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading

dongjoon-hyun commented Aug 13, 2019 •

edited

Loading