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/07/29 17:57:13 UTC
[GitHub] storm pull request: STORM-964, add config for worker logwriter to ...
GitHub user zhuoliu opened a pull request:
https://github.com/apache/storm/pull/657
STORM-964, add config for worker logwriter to restrict its mem usage
The log writer process that writes out stderr and stdout should have a config that controls it's memory usage and it should default to a very small value, probably 50MB in most cases.
To address above issue, one new parameter is defined for logwriter's jvmopts: "topology.worker.lw.childopts".
The default heap size of logwriter is set as 64M, or user can override it through "-c topology.worker.lw.childopts='xxxx " when submitting a topology.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhuoliu/storm 964
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/657.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 #657
----
commit f5ce1f865aaf8fbebd364799dcc9685a96dd7f16
Author: zhuol <zh...@yahoo-inc.com>
Date: 2015-07-29T15:54:56Z
STORM-964, add config for worker logwriter to restrict its mem usage
----
---
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-964, add config for worker logwriter to ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/657
---
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-964, add config for worker logwriter to ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/657#discussion_r35835399
--- Diff: storm-core/src/clj/backtype/storm/daemon/supervisor.clj ---
@@ -675,6 +675,7 @@
(add-to-classpath topo-classpath))
top-gc-opts (storm-conf TOPOLOGY-WORKER-GC-CHILDOPTS)
gc-opts (substitute-childopts (if top-gc-opts top-gc-opts (conf WORKER-GC-CHILDOPTS)) worker-id storm-id port)
+ topo-worker-logwriter-childopts (conf TOPOLOGY-WORKER-LOGWRITER-CHILDOPTS)
--- End diff --
Good catch. Since user may have configuration for this in topology's stormconf.ser. Updated.
---
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-964, add config for worker logwriter to ...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/657#issuecomment-126116122
Good suggestion! Updated.
---
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-964, add config for worker logwriter to ...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/657#issuecomment-126093002
Would it be possible to expand 'lw' to 'logwriter'?
I know it's longer, but it's much clearer what the config is for.
---
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-964, add config for worker logwriter to ...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/657#issuecomment-130746015
+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-964, add config for worker logwriter to ...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on a diff in the pull request:
https://github.com/apache/storm/pull/657#discussion_r35833865
--- Diff: storm-core/src/clj/backtype/storm/daemon/supervisor.clj ---
@@ -675,6 +675,7 @@
(add-to-classpath topo-classpath))
top-gc-opts (storm-conf TOPOLOGY-WORKER-GC-CHILDOPTS)
gc-opts (substitute-childopts (if top-gc-opts top-gc-opts (conf WORKER-GC-CHILDOPTS)) worker-id storm-id port)
+ topo-worker-logwriter-childopts (conf TOPOLOGY-WORKER-LOGWRITER-CHILDOPTS)
--- End diff --
should use ```storm-conf``` but not ```conf``` here?
```topo-worker-logwriter-childopts (storm-conf TOPOLOGY-WORKER-LOGWRITER-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-964, add config for worker logwriter to ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/657#issuecomment-128159999
The change looks good to me +1, but I want to give @knusbaum some time to follow up on his review 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.
---