You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by guoxiaolongzte <gi...@git.apache.org> on 2017/03/30 07:41:00 UTC

[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

GitHub user guoxiaolongzte opened a pull request:

    https://github.com/apache/spark/pull/17479

    [SPARK-20154][Web UI]In web ui,http://ip:4040/executors/,the title 'Storage Memory' should\u2026

    \u2026 modify 'Storage Memory used/total'
    
    ## What changes were proposed in this pull request?
    
    (Please fill in changes proposed in this fix)
    
    ## How was this patch tested?
    
    (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
    (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/guoxiaolongzte/spark SPARK-20154

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/17479.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #17479
    
----
commit d9cd198b99797929d85fa206738cd772a2e63147
Author: \u90ed\u5c0f\u9f99 10207633 <gu...@zte.com.cn>
Date:   2017-03-30T07:31:49Z

    In web ui,http://ip:4040/executors/,the title 'Storage Memory' should modify 'Storage Memory used/total'

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108862872
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    Is it necessary to change? I think what tooltip mentioned is quite clear if you hover on this column.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte closed the pull request at:

    https://github.com/apache/spark/pull/17479


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108947909
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    Looks very clear, it is not necessary to the mouse in the title.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/executors/...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/17479
  
    I don't think this is worth changing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/executors/...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17479
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108875608
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    I don't disagree your comments, but from my point it is slightly duplicated and not so necessary. And I believe most of the users understand what this column mean. So I'm conservative of such change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108879364
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    Let's post before/after snapshoots here and defer to committers. Personally, I am not sure too because tooltip explains quite clear.
    
    BTW, I think we should capitalise it just to be consistent other pages.
    
    ![2017-03-30 6 21 24](https://cloud.githubusercontent.com/assets/6477701/24497335/0f62dabc-1576-11e7-9ec4-ad0f3cd8206c.png)
    ![2017-03-30 6 21 18](https://cloud.githubusercontent.com/assets/6477701/24497339/15032e36-1576-11e7-99d7-8493eea3ae92.png)
    ![2017-03-30 6 21 15](https://cloud.githubusercontent.com/assets/6477701/24497341/1810be18-1576-11e7-907f-858f692b522b.png)
    ![2017-03-30 6 21 12](https://cloud.githubusercontent.com/assets/6477701/24497344/1b4013ae-1576-11e7-9599-9f91e4f89317.png)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108863264
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    Because of this change, easier to understand for users and observation.Web UI with clarity and friendly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/executors/...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/17479
  
    I think UI change requires a screenshot as written above. It seems trivial though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/executors/...

Posted by ajbozarth <gi...@git.apache.org>.
Github user ajbozarth commented on the issue:

    https://github.com/apache/spark/pull/17479
  
    I agree that this is a unnecessary change, it's already in the tooltip I'd rather keep the visual cluster in the UI to a minimum


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17479: [SPARK-20154][Web UI]In web ui,http://ip:4040/exe...

Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17479#discussion_r108876916
  
    --- Diff: core/src/main/resources/org/apache/spark/ui/static/executorspage-template.html ---
    @@ -24,7 +24,7 @@ <h4 style="clear: left; display: inline-block;">Summary</h4>
             <th></th>
             <th>RDD Blocks</th>
             <th><span data-toggle="tooltip"
    -                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory</span>
    +                  title="Memory used / total available memory for storage of data like RDD partitions cached in memory. ">Storage Memory used/total</span>
    --- End diff --
    
    The memory data is separated from each other, and if the title is separated, it is easy to observe and understand the user.Modify this slight UI, advantages outweigh the disadvantages.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org