Skip to content

Conversation

@tgravescs
Copy link
Contributor

What changes were proposed in this pull request?

We are adding other resource type support to the executors and Spark. We should show the resource information for each executor on the UI Executors page.

How was this patch tested?

Updated and added unit tests, tested in standalone, local-cluster, and yarn modes. Tested with active application and via history server.

Attached screenshot of gpu and fpga with lots of them.

sparkuigpufpga

@SparkQA
Copy link

SparkQA commented Jul 2, 2019

Test build #107131 has finished for PR 25037 at commit 0554df5.

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

@gatorsmile
Copy link
Member

CC @gengliangwang

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jul 8, 2019

Hi, @tgravescs . The new column looks a little wide. You may want the following.

@tgravescs
Copy link
Contributor Author

sure, I can look at that, the column only shows up when you actually use extra resources right now so if you don't use GPU it won't be there.

@tgravescs
Copy link
Contributor Author

@jiangxb1987 @mengxr @Ngone51

@SparkQA
Copy link

SparkQA commented Jul 8, 2019

Test build #107358 has finished for PR 25037 at commit a60cc48.

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

return "Dead"
}

function formatResourceCells(resources) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems a little crowded from the screenshot. It would be better if we could split this single resource cell into several row cells by resource type. But I guess this may require more changes ? Maybe, add \n after each resource type would be good enough ? Or you're intentionally to do this to occupy less space of the cell(since it brings more unnecessary space for other columns(looks more sparse now) ) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to take up less space. the linked PR adds in checkboxes to show columns so we could break up more if its not on by default, but I think at this point we should start with something small and then enhance later as people figure out what is useful.

@tgravescs
Copy link
Contributor Author

just a note, I'm waiting for pr 25047 and will update this based on that

@tgravescs
Copy link
Contributor Author

closing this and will upmerge and open a new one

@tgravescs tgravescs closed this Aug 9, 2019
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.

5 participants