-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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
[SPARK-23432][UI] Add executor peak jvm memory metrics in executors page #30186
Conversation
can you provide screen shots? |
ok to test |
cc @sarutak and @gengliangwang |
<th>JVM Memory OnHeap / OffHeap</th> | ||
<th>Execution Memory OnHeap / OffHeap</th> | ||
<th>Storage Memory OnHeap / OffHeap</th> | ||
<th>Pool Memory Direct / Mapped</th> |
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.
add tooltips describing what these are
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.
Added.
@@ -86,6 +86,10 @@ <h4 class="title-table">Executors</h4> | |||
<span data-toggle="tooltip" data-placement="top" | |||
title="Memory used / total available memory for off heap storage of data like RDD partitions cached in memory."> | |||
Off Heap Storage Memory</span></th> | |||
<th>JVM Memory OnHeap / OffHeap</th> | |||
<th>Execution Memory OnHeap / OffHeap</th> | |||
<th>Storage Memory OnHeap / OffHeap</th> |
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.
this is very close to the above On/Off Heap Storage Memory - so I think we need to differentiate the titles more. I think that goes for all of these, if they are Peak we should call them Peak.
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.
Added Peak for all.
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #130466 has finished for PR 30186 at commit
|
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130479 has finished for PR 30186 at commit
|
Peak Execution Memory OnHeap / OffHeap</span></th> | ||
<th> | ||
<span data-toggle="tooltip" data-placement="top" | ||
title="Peak storage onHeap / OffHeap memory used by JVM."> |
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.
I think we should expand this to be "Peak onHeap/ OffHeap memory used for storage of data like RDD partitions cached in memory"
Peak JVM Memory OnHeap / OffHeap</span></th> | ||
<th> | ||
<span data-toggle="tooltip" data-placement="top" | ||
title="Peak execution onHeap / OffHeap memory used by JVM."> |
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.
Peak OnHeap/OffHeap memory used for execution. This refers to memory used for computation in shuffles, joins, user data structures, etc. See the Memory Management Overview documentation for more details.
Peak Storage Memory OnHeap / OffHeap</span></th> | ||
<th> | ||
<span data-toggle="tooltip" data-placement="top" | ||
title="Peak direct / mapped pool memory used by JVM."> |
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.
Similarly I think we should explain more or point to java docs.
At least say this is direct byte buffers and mapped are memory-mapped or perhaps better would be to point to java.nio:type=BufferPool,name=direct and java.nio:type=BufferPool,name=mapped
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.
Good point. Updated.
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130538 has finished for PR 30186 at commit
|
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130548 has finished for PR 30186 at commit
|
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130580 has finished for PR 30186 at commit
|
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.
looks good pending Jenkins
retest this, please |
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130682 has finished for PR 30186 at commit
|
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.
+1
Thanks! Merged into master. |
Never mind. I tried your Github account and found it. Please let me know if I misassigned. |
@HeartSaVioR thanks, it's correct. |
What changes were proposed in this pull request?
Add executor peak jvm memory metrics in executors page
Why are the changes needed?
Users can know executor peak jvm metrics on in executors page
Does this PR introduce any user-facing change?
Users can know executor peak jvm metrics on in executors page
How was this patch tested?
Manually tested