You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by zhuoliu <gi...@git.apache.org> on 2015/11/10 21:58:23 UTC
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
GitHub user zhuoliu opened a pull request:
https://github.com/apache/storm/pull/875
[STORM-1198] Web UI to show resource usages and Total Resources on al…
Calculate, manage and display on UI for
each supervisor's total memory, used memory, total cpu and used CPU (by RAS).
See snapshot on UI:
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhuoliu/storm 1198
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/875.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 #875
----
commit 88ffea9c2e4ff2871d0b1f56897c125f59ecd48f
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-11-10T20:57:14Z
[STORM-1198] Web UI to show resource usages and Total Resources on all supervisors
----
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44596285
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -229,6 +249,10 @@
<td>{{uptime}}</td>
<td>{{slotsTotal}}</td>
<td>{{slotsUsed}}</td>
+ <td>{{totalMem}}</td>
+ <td>{{usedMem}}</td>
+ <td>{{totalCpu}}</td>
+ <td>{{usedCpu}}</td>
--- End diff --
Could you update the REST API docs to indicate the new fields
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-157488039
Two minor suggestions. Otherwise it looks good.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44554722
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -215,6 +215,26 @@
</span>
</th>
<th>
+ <span data-toggle="tooltip" data-placement="above" title="Memory capacity of a supervisor.">
+ Total Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="left" title="Memory that has been allocated.">
+ Used Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="above" title="CPU capacity of a supervisor. Every 100 means one core.">
+ Total CPU (%)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="left" title="CPU that has been allocated. Every 100 means one core">
+ Used CPU (%)
--- End diff --
Same here
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-156104772
I'd also like to hide 4 columns for non-RAS scheduler since exposing default value confuses users.
But if we can fill usedCpu / usedMem to actual value it would be much better.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44796485
--- Diff: storm-core/src/ui/public/index.html ---
@@ -156,6 +163,15 @@
]
});
$('#supervisor-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var scheduler = json["storm.scheduler"];
+ if (scheduler != "backtype.storm.scheduler.resource.ResourceAwareScheduler"){
+ $('#supervisor-summary td:nth-child(6),#supervisor-summary th:nth-child(6)').hide();
+ $('#supervisor-summary td:nth-child(7),#supervisor-summary th:nth-child(7)').hide();
+ $('#supervisor-summary td:nth-child(8),#supervisor-summary th:nth-child(8)').hide();
+ $('#supervisor-summary td:nth-child(9),#supervisor-summary th:nth-child(9)').hide();
+ }
+ });
--- End diff --
Same here as above
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45129773
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,12 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var displayResource = json["scheduler.display.resource"];
+ if (!displayResource){
+ $('#topology-summary td:nth-child(10),#topology-summary th:nth-child(10)').hide();
--- End diff --
I tried to pass "schedulerDisplayResource" to each supervisor-summary and topology-summary in REST and then use this to check whether we want to display in template htmls. However, it seems a little weird to store a cluster-wide configuration as a supervisor-specific or topology-specific field. Also, when there is no topology submitted, it is not feasible to check whether we want to display the assignedCPU in the topology-summary table's header (similarly for supervisor summary table).
We may rather keep it still since later we will have RAS as default scheduler and will merge with JSTORM's UI features.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/875
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44579118
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -215,6 +215,26 @@
</span>
</th>
<th>
+ <span data-toggle="tooltip" data-placement="above" title="Memory capacity of a supervisor.">
+ Total Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="left" title="Memory that has been allocated.">
+ Used Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="above" title="CPU capacity of a supervisor. Every 100 means one core.">
+ Total CPU (%)
--- End diff --
ok this make sense
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-157165262
Upmerged. For RAS schedulers, assigned/requested mem/cpu will be shown for each supervisor; for non-RAS schedulers, the assigned memory will be calculated for each topology and each supervisor based on their workers' JVM settings.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-156112103
I am fine with having a config to hide the 4 columns for the non-RAS case, but please file a follow on JIRA so we can at least show the memory requested for non-RAS schedulers. We may change a lot of it once the JStorm merger happens and we have numbers for CPU/Memory actually used. Once RAS is stable we may even migrate the existing schedulers to be strategies for RAS, and have them support not violating resource constraints.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by wuchong <gi...@git.apache.org>.
Github user wuchong commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45153774
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,12 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var displayResource = json["scheduler.display.resource"];
+ if (!displayResource){
+ $('#topology-summary td:nth-child(10),#topology-summary th:nth-child(10)').hide();
--- End diff --
agree with @kishorvpatil , although it's a little odd.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44597539
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -229,6 +249,10 @@
<td>{{uptime}}</td>
<td>{{slotsTotal}}</td>
<td>{{slotsUsed}}</td>
+ <td>{{totalMem}}</td>
+ <td>{{usedMem}}</td>
+ <td>{{totalCpu}}</td>
+ <td>{{usedCpu}}</td>
--- End diff --
Sure, adding it.
For non-RAS scheduler, the totalMem and totalCpu would be default(3000 and 400), and the usedMem and usedCpu will be always 0 and 0, no matter whether there is topology or not.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by redsanket <gi...@git.apache.org>.
Github user redsanket commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155917632
what would happens to the UI information if it is not using RAS? Just curious if it is turned off then do we want to show this information?
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44796472
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,13 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var scheduler = json["storm.scheduler"];
+ if (scheduler != "backtype.storm.scheduler.resource.ResourceAwareScheduler"){
+ $('#topology-summary td:nth-child(9),#topology-summary th:nth-child(9)').hide();
+ $('#topology-summary td:nth-child(10),#topology-summary th:nth-child(10)').hide();
+ }
+ });
--- End diff --
I don't really like having this be configured based off of a hard coded scheduler. I would prefer to have a separate config, or even better to have the rest API return a null for fields that it knows were not calculated properly. Then have the template filter out those fields based off of those values.
If we are going to go with a javascript hide, please add a class to all of the fields you want to hide with something like RAS_ONLY and then just do ```$(".RAS_ONLY").hide();```
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155933049
Can we just not show any of the columns that deal with resources if the scheduler used is not RAS
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155897894
LGTM +1
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-156469658
It also looks like you need to upmerge and regenerate the thrift code with thrift-0.9.3.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45507777
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -764,5 +778,30 @@ public static long zipFileSize(File myFile) throws IOException{
public static double zeroIfNaNOrInf(double x) {
return (Double.isNaN(x) || Double.isInfinite(x)) ? 0.0 : x;
}
+
+ /**
+ * parses the arguments to extract jvm heap memory size.
+ * @param input
+ * @param defaultValue
+ * @return the value of the JVM heap memory setting in a java command.
+ */
+ public static Double parseWorkerChildOpts(String input, Double defaultValue) {
+ if (input != null) {
+ Pattern optsPattern = Pattern.compile("Xmx[0-9]+m");
--- End diff --
This is not good enough. It is not just an 'm' that is supported.
From http://docs.oracle.com/javase/7/docs/technotes/tools/windows/java.html
```
-Xmxn
Specifies the maximum size, in bytes, of the memory allocation pool. This value must a multiple of 1024 greater than 2 MB. Append the letter k or K to indicate kilobytes, or m or M to indicate megabytes. The default value is chosen at runtime based on system configuration.
```
I have also seen 'g' and 'G' work for gigabytes too.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-156255409
Resource columns are hidden for non-RAS schedulers in topology and supervisor views.
For non-RAS schedulers, we can calculate per-topology and per-supervisor total memory assigned by analyzing the topology assignments, MAX-HEAP-MEMORY, worker.childopts and topology.worker.logwriter.childopts and topology.worker.childopts.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-157843055
Hi @kishorvpatil , your comments have been addressed. Thanks.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158133513
LGTM. +1. @zhuoliu please rebase the branch.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45491725
--- Diff: storm-core/src/jvm/backtype/storm/scheduler/Cluster.java ---
@@ -461,6 +466,75 @@ public SupervisorDetails getSupervisorById(String nodeId) {
return networkTopography;
}
+ /*
+ * Get heap memory usage for a worker's main process and logwriter process
+ * */
+ private Double getAssignedMemoryForSlot(Map topConf) {
+ Double totalWorkerMemory = 0.0;
+
+ String topology_worker_childopts = Utils.getString(topConf.get(Config.TOPOLOGY_WORKER_CHILDOPTS), null);
+ String worker_childopts = Utils.getString(topConf.get(Config.WORKER_CHILDOPTS), null);
+ Double mem_topology_worker_childopts = Utils.parseWorkerChildOpts(topology_worker_childopts, null);
+ Double mem_worker_childopts = Utils.parseWorkerChildOpts(worker_childopts, null);
--- End diff --
same here change to camel case
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45508355
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -764,5 +778,30 @@ public static long zipFileSize(File myFile) throws IOException{
public static double zeroIfNaNOrInf(double x) {
return (Double.isNaN(x) || Double.isInfinite(x)) ? 0.0 : x;
}
+
+ /**
+ * parses the arguments to extract jvm heap memory size.
+ * @param input
+ * @param defaultValue
+ * @return the value of the JVM heap memory setting in a java command.
+ */
+ public static Double parseWorkerChildOpts(String input, Double defaultValue) {
--- End diff --
I would prefer to have the name of this function indicate what it is parsing out of the childopts.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155931351
I just tested what @redsanket asked and topologies not using the resource aware scheduler show no resources used at all. It really would be good if we could parse out the heap from the command line of a worker, and do a quick SWAG on the CPU when not using RAS. That way we could at least see if memory/CPU is being over committed on a node.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158503645
Just a few more comments.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44555808
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -215,6 +215,26 @@
</span>
</th>
<th>
+ <span data-toggle="tooltip" data-placement="above" title="Memory capacity of a supervisor.">
+ Total Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="left" title="Memory that has been allocated.">
+ Used Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="above" title="CPU capacity of a supervisor. Every 100 means one core.">
+ Total CPU (%)
--- End diff --
Thanks for the comment. This is a percentage to a vCore. So every 1 equals to 1% of a core. E.g., 400 here means 4 CPU vCore. I would appreciate if we can have a more descriptive unit sign here.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158481188
Just tested the UI, resource numbers look right. Good Stuff +1
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45111413
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,12 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
--- End diff --
i suggest we avoid making another ajax call - instead we can have topology summary include boolean as part of topology-summary.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155933406
@revans2 If we want to show allocated cpu/mem for non-RAS schedulers, we will also need to do that for topology view except for the supervisor view.
Also, it would be also possible that the usedMem might be larger than totalMem (since totalMem may not be correctly configured for non-RAS schedulers, e.g., using default 3GB).
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45517072
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -503,6 +505,24 @@ public static boolean getBoolean(Object o, boolean defaultValue) {
}
}
+ public static String getString(Object o, String defaultValue) {
--- End diff --
Can we inline this or move it someplace else, it does not feel generic enough to be a part of Utils.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45491709
--- Diff: storm-core/src/jvm/backtype/storm/scheduler/Cluster.java ---
@@ -461,6 +466,75 @@ public SupervisorDetails getSupervisorById(String nodeId) {
return networkTopography;
}
+ /*
+ * Get heap memory usage for a worker's main process and logwriter process
+ * */
+ private Double getAssignedMemoryForSlot(Map topConf) {
+ Double totalWorkerMemory = 0.0;
+
+ String topology_worker_childopts = Utils.getString(topConf.get(Config.TOPOLOGY_WORKER_CHILDOPTS), null);
+ String worker_childopts = Utils.getString(topConf.get(Config.WORKER_CHILDOPTS), null);
+ Double mem_topology_worker_childopts = Utils.parseWorkerChildOpts(topology_worker_childopts, null);
--- End diff --
same here change to camel case
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158538482
travis CI failure looks unrelated +1
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45111484
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,12 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var displayResource = json["scheduler.display.resource"];
+ if (!displayResource){
+ $('#topology-summary td:nth-child(10),#topology-summary th:nth-child(10)').hide();
--- End diff --
Let's avoid referring to fields by number which could change in future and make maintenance harder..
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r44554614
--- Diff: storm-core/src/ui/public/templates/index-page-template.html ---
@@ -215,6 +215,26 @@
</span>
</th>
<th>
+ <span data-toggle="tooltip" data-placement="above" title="Memory capacity of a supervisor.">
+ Total Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="left" title="Memory that has been allocated.">
+ Used Mem (MB)
+ </span>
+ </th>
+ <th>
+ <span data-toggle="tooltip" data-placement="above" title="CPU capacity of a supervisor. Every 100 means one core.">
+ Total CPU (%)
--- End diff --
Doesn't seem like this should be a percentage
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45508189
--- Diff: storm-core/src/jvm/backtype/storm/scheduler/Cluster.java ---
@@ -461,6 +466,75 @@ public SupervisorDetails getSupervisorById(String nodeId) {
return networkTopography;
}
+ /*
+ * Get heap memory usage for a worker's main process and logwriter process
+ * */
+ private Double getAssignedMemoryForSlot(Map topConf) {
+ Double totalWorkerMemory = 0.0;
+
+ String topologyWorkerChildopts = Utils.getString(topConf.get(Config.TOPOLOGY_WORKER_CHILDOPTS), null);
+ String workerChildopts = Utils.getString(topConf.get(Config.WORKER_CHILDOPTS), null);
--- End diff --
Both of these configs will return either a string or a list of strings. We should support both options.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158521004
One more minor comment and then I am +1
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-158496648
Thanks @jerrypeng a lot for the thorough review and test verification! Camel case issue has been addressed.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45111628
--- Diff: storm-core/src/ui/public/index.html ---
@@ -143,6 +143,12 @@
]
});
$('#topology-summary [data-toggle="tooltip"]').tooltip();
+ $.getJSON("/api/v1/cluster/configuration", function(json){
+ var displayResource = json["scheduler.display.resource"];
+ if (!displayResource){
+ $('#topology-summary td:nth-child(10),#topology-summary th:nth-child(10)').hide();
--- End diff --
We could modify the template to decide not list headers and values id `displayResource` is `false`. That is cleaner that populating and hiding certain columns by number.
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/875#discussion_r45491671
--- Diff: storm-core/src/jvm/backtype/storm/scheduler/Cluster.java ---
@@ -461,6 +466,75 @@ public SupervisorDetails getSupervisorById(String nodeId) {
return networkTopography;
}
+ /*
+ * Get heap memory usage for a worker's main process and logwriter process
+ * */
+ private Double getAssignedMemoryForSlot(Map topConf) {
+ Double totalWorkerMemory = 0.0;
+
+ String topology_worker_childopts = Utils.getString(topConf.get(Config.TOPOLOGY_WORKER_CHILDOPTS), null);
--- End diff --
I think we should use camel case to be consistent with rest of the file
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-155932528
@redsanket good point! the ui should still be compatible if RAS is not used
---
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.
---
[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/875#issuecomment-157894261
Add one rest field for topo/id, delete the last REST getJSON call
---
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.
---