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/10/08 23:20:42 UTC
[GitHub] storm pull request: [STORM-1093] Launching Workers with resources ...
GitHub user zhuoliu opened a pull request:
https://github.com/apache/storm/pull/790
[STORM-1093] Launching Workers with resources specified in resource-aware schedulers
This request provides a set of schemes that allow nimbus to put different types of resources to each worker slot, then push the resources within assignment to ZooKeepers; also, at the supervisor side, such resources in each worker slot should be used by supervisor for launching a worker's JVM (initially, the JVM heap size, this can also be called by CGroup or Docker for resource segregation).
Such scheme can be used not only by RAS scheduler (STORM-893), but also by any customized scheduler for conducting mem/cpu resource aware scheduling.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhuoliu/storm 1093a
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/790.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 #790
----
commit 066fd7aa89a7f50a1f5794a14a68312b5996a112
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-10-07T21:08:36Z
Add resource setting API in WorkerSlot
commit 8b327c9ed84ec2b29ad7c807f04b9928f6d25cbc
Author: Kishor Patil <kp...@yahoo-inc.com>
Date: 2015-08-21T16:37:19Z
Merge pull request #509 from zhuol/1779
[YSTORM-1779] Send/receive resource information from nimbus to supervisor for RAS
commit ca8bb24dbffd806ed41e27976552e197608d6627
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-10-07T21:48:33Z
Remove generated files
commit 715e64ba05bd691555d53d12c0daa851868fea03
Author: Derek Dagit <de...@yahoo-inc.com>
Date: 2015-08-31T16:55:36Z
Cherry-pick YSTORM-1546
commit 3a045bf0f5077df155a00dc11ad859e6b9d6578d
Author: Zhuo Liu <zh...@yahoo-inc.com>
Date: 2015-09-16T22:02:42Z
Cherry-pick YSTORM-2146
commit 545df27ddd79d2a1175749887c1681abfbd74635
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-10-08T15:50:27Z
Minor Fix of the import
commit a684962022ad228397f86c09cb266a56afb75a59
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-10-08T21:19:49Z
Push Thrift file changes
----
---
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-1093] Launching Workers with resources ...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/790#discussion_r42178630
--- Diff: conf/defaults.yaml ---
@@ -140,7 +140,8 @@ supervisor.memory.capacity.mb: 3072.0
supervisor.cpu.capacity: 400.0
### worker.* configs are for task workers
-worker.childopts: "-Xmx768m"
+worker.heap.memory.mb: 768
+worker.childopts: "-Xmx%HEAP-MEM%m"
--- End diff --
Why is this replacement value in defaults.yaml, but none of the others are?
---
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-1093] Launching Workers with resources ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/790#discussion_r42284878
--- Diff: conf/defaults.yaml ---
@@ -140,7 +140,8 @@ supervisor.memory.capacity.mb: 3072.0
supervisor.cpu.capacity: 400.0
### worker.* configs are for task workers
-worker.childopts: "-Xmx768m"
+worker.heap.memory.mb: 768
+worker.childopts: "-Xmx%HEAP-MEM%m"
--- End diff --
@revans2 and I think it would be better practice if we put the HEAP-MEM in the substitute-childopts function together with WORKER-ID, TOPOLOGY-ID, WORKER-PORT.
---
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-1093] Launching Workers with resources ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-146924208
Thanks Jerry, nicely STORM-893 (RAS) has just been merged.
---
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-1093] Launching Workers with resources ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-149258631
The code looks good the tests all pass I am +1 on this.
---
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-1093] Launching Workers with resources ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-147760574
May I also have @revans2 @rfarivar @knusbaum @d2r to review this pull request if you are available?
---
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-1093] Launching Workers with resources ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/790
---
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-1093] Launching Workers with resources ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/790#discussion_r42380191
--- Diff: conf/defaults.yaml ---
@@ -140,7 +140,8 @@ supervisor.memory.capacity.mb: 3072.0
supervisor.cpu.capacity: 400.0
### worker.* configs are for task workers
-worker.childopts: "-Xmx768m"
+worker.heap.memory.mb: 768
+worker.childopts: "-Xmx%HEAP-MEM%m"
--- End diff --
The reason for this is backwards compatibility + flexibility. If an admin has set their own worker.childopts for a given supervisor to say 2GB, we want that to have higher priority over what we calculate. Similarly if someone sets topology.worker.childopts it needs to have higher priority over the other two. But what if the topology wants to test out the resource aware scheduler on just their topology? With this they can set topology.worker.childopts to "-Xmx%HEAP-MEM%m" and override the hard coded 2GB that was there before.
---
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-1093] Launching Workers with resources ...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-146891371
Looks good but probably should wait until the resource scheduler actually goes in before we merge this PR
---
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-1093] Launching Workers with resources ...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-149330539
+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-1093] Launching Workers with resources ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/790#issuecomment-146962992
Just manual test with RAS, it works well, and we can see each worker is being launched with different JVM heap sizes (added up by their tasks' specified memory usage).
May I have your reviews when you have time @revans2 @d2r ?
---
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.
---