Skip to content

Conversation

@LantaoJin
Copy link
Contributor

What changes were proposed in this pull request?

SPARK-17019 Expose off-heap memory usage in WebUI. But it makes this additional columns hidden by default. If we want to see them, we need change the css code to rebuild a spark-core.jar. It's very inconvenient.

.on_heap_memory, .off_heap_memory {
  display: none;
}

So I add an on-off switch to show those additional columns. And in future, we don't afraid to add more columns.

How was this patch tested?

screen shot 2018-09-30 at 5 45 56 pm

screen shot 2018-09-30 at 5 46 06 pm

@LantaoJin
Copy link
Contributor Author

Gently ping @jerryshao @cloud-fan . Do you have a chance to review?

@LantaoJin
Copy link
Contributor Author

If this PR could be merged, #22578 could be added as an additional column as well.

@LantaoJin
Copy link
Contributor Author

cc @dongjoon-hyun @srowen

@srowen
Copy link
Member

srowen commented Oct 1, 2018

Not sure about this. It's not clear why the checkbox is there, what columns it controls. EDIT: I see the issue is that the columns are otherwise not visible? they should just be visible then, I think. Have they always been hidden?

@LantaoJin
Copy link
Contributor Author

@srowen The checkbox is what I add in this PR to display/hidden the columns which have been hidden always. These columns are on heap memory, off heap memory. If we want to display them in executor page, we have to change the css file and rebuild the spark-core.jar file. Besides, there should be more and more columns being added in future. Ref SPARK-23206, SPARK-23206. If all of them are visible all the time, this page won't be brief like this. So we can simply add a checkbox to let user display them on demand and hidden by default.

@srowen
Copy link
Member

srowen commented Oct 8, 2018

CC @jerryshao for #14617 where this was added. It looks like the display is on purpose, but can you clarify?

I don't think a "show additional columns" box stuck under the title is the right UI but I haven't looked closely at it. We have a different way of managing extra columns in the Jobs/Stage page for example.

@gengliangwang
Copy link
Member

I read the original PR #14617, and it is wired that the two columns are hidden in the very beginning.
Also the screenshot in PR description doesn't match the code changes.

Ping @jerryshao

@gengliangwang
Copy link
Member

Ping @jerryshao again.

@dongjoon-hyun
Copy link
Member

Retest this please.

@dongjoon-hyun
Copy link
Member

Also, gentle ping, @jerryshao and @squito .

@jerryshao
Copy link
Contributor

jerryshao commented Jul 8, 2019

I remembered there was a group of checkbox to show the hidden executor information when I filed SPARK-17019,no need to rebuilt css to expose it. Maybe it is due to the refactor of history server which made some changes hidden?

@SparkQA
Copy link

SparkQA commented Jul 8, 2019

Test build #107315 has finished for PR 22595 at commit c758db0.

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

@tgravescs
Copy link
Contributor

not sure how close this is to being done, but lets coordinate this with #25037 as that column would be a good one to include in showing additional columns.

@dongjoon-hyun
Copy link
Member

@squito
Copy link
Contributor

squito commented Jul 15, 2019

I totally agree we should have a way to show the columns. As we're thinking of adding a bunch more columns, maybe we should follow the same approach for the tasks page, where there is a checkbox per column?

@tgravescs
Copy link
Contributor

Yeah I agree having toggles per column like the stage page makes sense. Note I think we can do this first and I can update 25037 to match since its waiting on another PR anyway. @LantaoJin do you think you can update it?

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen srowen closed this Sep 16, 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.

9 participants