[GitHub] storm issue #1627: STORM-2039: backpressure refactoring in worker and execut...
Github user zhuoliu commented on the issue: https://github.com/apache/storm/pull/1627 Since with the refactoring we directly access the disruptor queue to get the backpressure status rather than having an additional flag in executor data which is error-prone when flipping (which is very possibly the root cause of the halt in STORM-1949), we may NOT need to add the ADDITIONAL ZK pressure which I mentioned in the 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1632: Storm 2039 backpressure refactoring in worker and executo...
Github user zhuoliu commented on the issue: https://github.com/apache/storm/pull/1632 Looks great to me, thanks. +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1627: STORM-2039: backpressure refactoring in worker and execut...
Github user zhuoliu commented on the issue: https://github.com/apache/storm/pull/1627 sure, Alex. Really appreciate if you can submit a separate PR for 1949. +1 to this one! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1627: STORM-2039: backpressure refactoring in worker and execut...
Github user zhuoliu commented on the issue: https://github.com/apache/storm/pull/1627 @abellina looks good to me, just one extra suggestion: In https://github.com/apache/storm/blob/master/storm-core/src/clj/org/apache/storm/daemon/worker.clj#L156 To reduce the traffic to ZK, we checked the previous flag to update to Zookeeper only when it is necessary (the flag has changed), but still there may some weird case we have inconsistency between local "prev-backpressure-flag" and the worker backpressure node at Zookeeper (see STORM-1949). So to strike a tradeoff and avoid the possible wrong stall of a topology, could you add do a random number generation here, e.g., generate a number between 0-9, if it is zero, we will call the try block no matter "prev-backpressure-flag" and "curr-backpressure-flag" is equal or not. Thanks a lot! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1696]-1.x-branch status not sync if zk ...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1320#issuecomment-209574168 @HeartSaVioR sure. Fix for master branch: https://github.com/apache/storm/pull/1338 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1696] status not sync if zk fails in ba...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1338 [STORM-1696] status not sync if zk fails in backpressure #1320 This is a fix for master branch. When there is a zk exception happens during worker-backpressure!, there is a bad state which can block the topology from running normally any more. The root cause: in worker/mk-backpressure-handler if the worker-backpressure! fails once due to zk connection exception, next time when this method gets called by WordBackpressureThread, because (when (not= prev-backpressure-flag curr-backpressure-flag) will never be true, the remote zk node can not be synced with local state. This problem can cause a topology to be blocked! This also explains why we will not see any problem when testing in a stable (zk never fail) environment. Solution is quite straightforward: first change the zk status, if succeeds, change local status. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1696b Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1338.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 #1338 commit a70195d717e1ed179425a0305b2e4279afeb6118 Author: zhuoliu Date: 2016-04-13T18:04:57Z [STORM-1696] status not sync if zk fails in backpressure commit 63518be11275104314dcebbf83bb1f8a513891ee Author: zhuoliu Date: 2016-04-13T18:08:19Z minor --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1696]-1.x-branch status not sync if zk ...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1320 [STORM-1696]-1.x-branch status not sync if zk fails in backpressure This issue can cause a topology to be blocked. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm STORM-1696-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1320.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 #1320 commit 9271056b22ab5c734157a9ca1f3f4ab9a28d4b4b Author: zhuol Date: 2016-04-07T23:12:33Z [STORM-1696]-1.x-branch status not sync if zk fails in backpressure --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1696]-1.x-branch status not sync if zk ...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1320#issuecomment-207134141 I can either put the refactoring part as a separate pull request or within 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690]-1.x-branch: fix backpressure flag...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1315#issuecomment-206575929 Sorry, this is not a bug. No need to fix. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690]-1.x-branch: fix backpressure flag...
Github user zhuoliu closed the pull request at: https://github.com/apache/storm/pull/1315 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690] fix backpressure flag initializat...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1314#issuecomment-206575442 Sorry, this is not a bug. No need to fix. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690] fix backpressure flag initializat...
Github user zhuoliu closed the pull request at: https://github.com/apache/storm/pull/1314 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690]-1.x-branch: fix backpressure flag...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1315 [STORM-1690]-1.x-branch: fix backpressure flag init You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm STORM-1690-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1315.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 #1315 commit e43f76004167dc6c1a422bfec5f3b631672be2a2 Author: zhuol Date: 2016-04-06T20:54:28Z [STORM-1690]-1.x-branch: fix backpressure flag init --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1690] fix backpressure flag initializat...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1314 [STORM-1690] fix backpressure flag initialization Found that in different DisruptorQueues in a worker, the _enableBackpressure flags have both true and false, which cause topology to be blocked. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1690 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1314.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 #1314 commit be6922e0611b96bec3cb621436ae1a0060f9f941 Author: zhuol Date: 2016-04-06T20:47:15Z [STORM-1690] fix backpressure flag initialization --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1286] port kill_workers to java.
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1310#issuecomment-206455912 Thanks for the reviews, 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1687] divide by zero in StatsUtil
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1312 [STORM-1687] divide by zero in StatsUtil uptime sometimes equals to zero which causes divide exception. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm STORM-1687 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1312.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 #1312 commit 44343318a116b8d546df3dedadc2bbea427ae3eb Author: zhuol Date: 2016-04-06T01:03:02Z [STORM-1687] divide by zero in StatsUtil --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1687]-1.x divide by zero in stats
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1311 [STORM-1687]-1.x divide by zero in stats This fixes the "divide by zero" error in stats/compute-agg-capacity. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm STORM-1687-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1311.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 #1311 commit 7e5a74fddd82570107c401963fc971080b7fa54f Author: zhuol Date: 2016-04-06T00:56:57Z [STORM-1687]-1.x divide by zero in stats --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1286] port kill_workers to java.
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1310 [STORM-1286] port kill_workers to java. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1286 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1310.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 #1310 commit 103e0c6a1387d32e7d141b1a0af9d3a62127b477 Author: zhuol Date: 2016-04-05T02:38:38Z [STORM-1286] port kill_workers to java. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316450 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). + +## Pluggable scheduler +You can implement your own scheduler to replace the default scheduler to assign executors to workers. You configure the class to use using the "storm.scheduler" config in your storm.yaml, and your scheduler must implement [this interface](https://github.com/apache/storm/tree/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/IScheduler.java). + +## Isolation Scheduler +The isolation scheduler makes it easy and safe to share a cluster among many topologies. The isolation scheduler lets you specify which topologies should be "isolated", meaning that they run on a dedicated set of machines within the cluster where no other topologies will be running. These isolated topologies are given priority on the cluster, so resources will be allocated to isolated topologies if there's competition with non-isolated topologies, and resources will be taken away from non-isolated topologies if necessary to get resources for an isolated topology. Once all isolated topologies are allocated, the remaining machines on the cluster are shared among all non-isolated topologies. + +You configure the isolation scheduler in the Nimbus configuration. Set "storm.scheduler" to "org.apache.storm.scheduler.IsolationScheduler". Then, use the "isolation.scheduler.machines" config to specify how many machines each topology should get. This config is a map from topology name to number of machines. For example: --- End diff -- to the number of isolated machines allocated to this topology --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316422 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). + +## Pluggable scheduler +You can implement your own scheduler to replace the default scheduler to assign executors to workers. You configure the class to use using the "storm.scheduler" config in your storm.yaml, and your scheduler must implement [this interface](https://github.com/apache/storm/tree/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/IScheduler.java). + +## Isolation Scheduler +The isolation scheduler makes it easy and safe to share a cluster among many topologies. The isolation scheduler lets you specify which topologies should be "isolated", meaning that they run on a dedicated set of machines within the cluster where no other topologies will be running. These isolated topologies are given priority on the cluster, so resources will be allocated to isolated topologies if there's competition with non-isolated topologies, and resources will be taken away from non-isolated topologies if necessary to get resources for an isolated topology. Once all isolated topologies are allocated, the remaining machines on the cluster are shared among all non-isolated topologies. + +You configure the isolation scheduler in the Nimbus configuration. Set "storm.scheduler" to "org.apache.storm.scheduler.IsolationScheduler". Then, use the "isolation.scheduler.machines" config to specify how many machines each topology should get. This config is a map from topology name to number of machines. For example: --- End diff -- config => configuration --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316401 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). + +## Pluggable scheduler +You can implement your own scheduler to replace the default scheduler to assign executors to workers. You configure the class to use using the "storm.scheduler" config in your storm.yaml, and your scheduler must implement [this interface](https://github.com/apache/storm/tree/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/IScheduler.java). + +## Isolation Scheduler +The isolation scheduler makes it easy and safe to share a cluster among many topologies. The isolation scheduler lets you specify which topologies should be "isolated", meaning that they run on a dedicated set of machines within the cluster where no other topologies will be running. These isolated topologies are given priority on the cluster, so resources will be allocated to isolated topologies if there's competition with non-isolated topologies, and resources will be taken away from non-isolated topologies if necessary to get resources for an isolated topology. Once all isolated topologies are allocated, the remaining machines on the cluster are shared among all non-isolated topologies. + +You configure the isolation scheduler in the Nimbus configuration. Set "storm.scheduler" to "org.apache.storm.scheduler.IsolationScheduler". Then, use the "isolation.scheduler.machines" config to specify how many machines each topology should get. This config is a map from topology name to number of machines. For example: --- End diff -- You "can" configure ... "by setting" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316364 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). + +## Pluggable scheduler +You can implement your own scheduler to replace the default scheduler to assign executors to workers. You configure the class to use using the "storm.scheduler" config in your storm.yaml, and your scheduler must implement [this interface](https://github.com/apache/storm/tree/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/IScheduler.java). --- End diff -- may say as "must implement the [IScheduler] interface" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316346 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). + +## Pluggable scheduler +You can implement your own scheduler to replace the default scheduler to assign executors to workers. You configure the class to use using the "storm.scheduler" config in your storm.yaml, and your scheduler must implement [this interface](https://github.com/apache/storm/tree/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/IScheduler.java). --- End diff -- "use using" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1679: add storm Scheduler documents
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1306#discussion_r58316336 --- Diff: docs/Storm-Scheduler.md --- @@ -0,0 +1,27 @@ +--- +title: Scheduler +layout: documentation +documentation: true +--- + +Storm now has 4 kinds of build-in schedulers: [DefaultScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/DefaultScheduler.clj), [IsolationScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/clj/org/apache/storm/scheduler/IsolationScheduler.clj), [MultitenantScheduler](https://github.com/apache/storm/blob/1.x-branch/storm-core/src/jvm/org/apache/storm/scheduler/multitenant/MultitenantScheduler.java), [ResourceAwareScheduler](Resource_Aware_Scheduler_overview.html). --- End diff -- The link to [ResourceAwareScheduler] is not reachable. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
Github user zhuoliu closed the pull request at: https://github.com/apache/storm/pull/1299 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1299#issuecomment-204544681 Taylor has applied this patch to 1.x. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1299#issuecomment-204541667 @ptgoetz, nice, then I will close this one. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1299#issuecomment-204539772 @ptgoetz , thanks for applying this one to 1.x already. One minor concern is that the change to logviewer_test.clj is not included. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1299#issuecomment-204538236 @HeartSaVioR this is for 1.x, just one line difference from the patch to master. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671]-1.x Enable logviewer to delete di...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1299 [STORM-1671]-1.x Enable logviewer to delete directory with no yaml file Apply [https://github.com/apache/storm/pull/1285] to 1.x-branch You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm STORM-1671-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1299.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 #1299 commit 606699d496b9e3bbf330e0957c162627579e6ef4 Author: zhuol Date: 2016-04-01T19:33:06Z [STORM-1671]-1.x Enable logviewer to delete directory with no yaml 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671] Enable logviewer to delete a dir ...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1285#issuecomment-204452676 @HeartSaVioR Sure. Added this issue to epic Storm-1491. Will apply it to 1.0 once gets merged into master. Upmerged. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671] Enable logviewer to delete a dir ...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1285#issuecomment-204066326 Will also cherry-pick this patch to 1.x. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1671] Enable logviewer to delete a dir ...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1285 [STORM-1671] Enable logviewer to delete a dir without yaml For those old and dead worker directories, in some weird case, there is no yaml file in it. We should enable logviewer to delete them. Manually tested. It is able to delete an empty workers-artifacts/topo/port dir. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1671 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1285.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 #1285 commit 7f37d8411747d974ac6846bf2745bc54fd20488d Author: zhuol Date: 2016-03-31T18:25:58Z [STORM-1671] Enable logviewer to delete a dir without yaml --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1667] Log the IO exception when deletin...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1280#discussion_r58081089
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -296,11 +293,15 @@
(worker-launcher-and-wait conf user ["signal" pid "9"] :log-prefix
(str "kill -9 " pid))
(force-kill-process pid))
(if as-user
-(rmr-as-user conf id (worker-pid-path conf id pid))
(try
+ (rmr-as-user conf id (worker-pid-path conf id pid))
(rmpath (worker-pid-path conf id pid))
(rmpath (worker-tmp-root conf id pid))
- (catch Exception e ;; on windows, the supervisor may still
holds the lock on the worker directory
+ (catch IOException e
+(log-warn-error e "Failed to cleanup pid dir: " pid " for
worker " id". Will retry later"))
+ (catch RuntimeException e
+(log-warn-error e "Failed to cleanup pid dir: " pid " for
worker " id". Will retry later")
+ ;; on windows, the supervisor may still holds the lock on the
worker directory
--- End diff --
@satishd, got your point. It would be the responsibility of the one who
added new code to catch other possible exception, or they may prefer to not
catch and let it fail fast upon specific scenario.
But for now, since there is any non-deterministic exception might be thrown
in Windows environment, we may still ignore all types of exception, which might
also be why the previous code did this way ("catch Exception").
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1667] Log the IO exception when deletin...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1280#discussion_r58066223 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -296,11 +293,15 @@ (worker-launcher-and-wait conf user ["signal" pid "9"] :log-prefix (str "kill -9 " pid)) (force-kill-process pid)) (if as-user -(rmr-as-user conf id (worker-pid-path conf id pid)) (try + (rmr-as-user conf id (worker-pid-path conf id pid)) (rmpath (worker-pid-path conf id pid)) (rmpath (worker-tmp-root conf id pid)) - (catch Exception e ;; on windows, the supervisor may still holds the lock on the worker directory + (catch IOException e +(log-warn-error e "Failed to cleanup pid dir: " pid " for worker " id". Will retry later")) + (catch RuntimeException e +(log-warn-error e "Failed to cleanup pid dir: " pid " for worker " id". Will retry later") + ;; on windows, the supervisor may still holds the lock on the worker directory --- End diff -- Hi @satishd , your concern makes good sense, however, I do not find any other type of exception that might be thrown by the try block (the worker-launcher-and-wait in rmr-as-user throws InterruptedException and catches it already). It this is case, catching specific exceptions should be preferable to catching Exception. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1667] Log the IO exception when deletin...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1280#discussion_r57974731 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -269,10 +269,7 @@ (catch IOException e (log-warn-error e "Failed to cleanup worker " id ". Will retry later")) (catch RuntimeException e -(log-warn-error e "Failed to cleanup worker " id ". Will retry later") -) - (catch java.io.FileNotFoundException e (log-message (.getMessage e))) --- End diff -- Delete redundant catch of FileNotFoundException since it has already been caught as IOException --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1667] Log the IO exception when deletin...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1280 [STORM-1667] Log the IO exception when deleting worker pid dir We find a race condition in supervisor which can sometimes crash supervisor. This race condition exists because: shutdown-worker is called by both sync-processes and synchronize-supervisor. One pid directory might have already been deleted when rmr-as-user tries to delete it. To avoid crashing the supervisor, we should log the exception rather than throw it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm Storm-1667-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1280.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 #1280 commit c54162418a4a46d99ac360661f78cc5e10cb654a Author: zhuol Date: 2016-03-30T22:21:41Z [STORM-1667] Log the IO exception when deleting worker pid dir --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1232#issuecomment-202427400 @jerrypeng , I have addressed your comments. Could you check back? 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1232#discussion_r57487640
--- Diff:
storm-core/test/jvm/org/apache/storm/scheduler/resource/TestResourceAwareScheduler.java
---
@@ -54,6 +61,681 @@
private static int currentTime = 1450418597;
+private static final Config defaultTopologyConf = new Config();
+
+@BeforeClass
+public static void initConf() {
+defaultTopologyConf.put(Config.STORM_NETWORK_TOPOGRAPHY_PLUGIN,
"org.apache.storm.networktopography.DefaultRackDNSToSwitchMapping");
+
defaultTopologyConf.put(Config.RESOURCE_AWARE_SCHEDULER_EVICTION_STRATEGY,
org.apache.storm.scheduler.resource.strategies.eviction.DefaultEvictionStrategy.class.getName());
+
defaultTopologyConf.put(Config.RESOURCE_AWARE_SCHEDULER_PRIORITY_STRATEGY,
org.apache.storm.scheduler.resource.strategies.priority.DefaultSchedulingPriorityStrategy.class.getName());
+
+defaultTopologyConf.put(Config.TOPOLOGY_SCHEDULER_STRATEGY,
org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategy.class.getName());
+
defaultTopologyConf.put(Config.TOPOLOGY_COMPONENT_CPU_PCORE_PERCENT, 10.0);
+
defaultTopologyConf.put(Config.TOPOLOGY_COMPONENT_RESOURCES_ONHEAP_MEMORY_MB,
128.0);
+
defaultTopologyConf.put(Config.TOPOLOGY_COMPONENT_RESOURCES_OFFHEAP_MEMORY_MB,
0.0);
+defaultTopologyConf.put(Config.TOPOLOGY_WORKER_MAX_HEAP_SIZE_MB,
8192.0);
+defaultTopologyConf.put(Config.TOPOLOGY_PRIORITY, 0);
+defaultTopologyConf.put(Config.TOPOLOGY_SUBMITTER_USER, "zhuo");
+}
+
+@Test
+public void testRASNodeSlotAssign() {
+INimbus iNimbus = new
TestUtilsForResourceAwareScheduler.INimbusTest();
+Map resourceMap = new HashMap<>();
+resourceMap.put(Config.SUPERVISOR_CPU_CAPACITY, 400.0);
+resourceMap.put(Config.SUPERVISOR_MEMORY_CAPACITY_MB, 2000.0);
+Map supMap =
TestUtilsForResourceAwareScheduler.genSupervisors(5, 4, resourceMap);
+Topologies topologies = new Topologies(new HashMap());
+Cluster cluster = new Cluster(iNimbus, supMap, new HashMap(), new HashMap());
+Map nodes = RAS_Nodes.getAllNodesFrom(cluster,
topologies);
+Assert.assertEquals(5, nodes.size());
+RAS_Node node = nodes.get("sup-0");
+
+Assert.assertEquals("sup-0", node.getId());
+Assert.assertTrue(node.isAlive());
+Assert.assertEquals(0, node.getRunningTopologies().size());
+Assert.assertTrue(node.isTotallyFree());
+Assert.assertEquals(4, node.totalSlotsFree());
+Assert.assertEquals(0, node.totalSlotsUsed());
+Assert.assertEquals(4, node.totalSlots());
+
+TopologyDetails topology1 =
TestUtilsForResourceAwareScheduler.getTopology("topology1", new HashMap(), 1,
0, 2, 0, 0, 0);
+
+List executors11 = new ArrayList<>();
+executors11.add(new ExecutorDetails(1, 1));
+node.assign(node.getFreeSlots().iterator().next(), topology1,
executors11);
+Assert.assertEquals(1, node.getRunningTopologies().size());
+Assert.assertFalse(node.isTotallyFree());
+Assert.assertEquals(3, node.totalSlotsFree());
+Assert.assertEquals(1, node.totalSlotsUsed());
+Assert.assertEquals(4, node.totalSlots());
+
+List executors12 = new ArrayList<>();
+executors12.add(new ExecutorDetails(2, 2));
+node.assign(node.getFreeSlots().iterator().next(), topology1,
executors12);
+Assert.assertEquals(1, node.getRunningTopologies().size());
+Assert.assertFalse(node.isTotallyFree());
+Assert.assertEquals(2, node.totalSlotsFree());
+Assert.assertEquals(2, node.totalSlotsUsed());
+Assert.assertEquals(4, node.totalSlots());
+
+TopologyDetails topology2 =
TestUtilsForResourceAwareScheduler.getTopology("topology2", new HashMap(), 1,
0, 2, 0, 0, 0);
+
+List executors21 = new ArrayList<>();
+executors21.add(new ExecutorDetails(1, 1));
+node.assign(node.getFreeSlots().iterator().next(), topology2,
executors21);
+Assert.assertEquals(2, node.getRunningTopologies().size());
+Assert.assertFalse(node.isTotallyFree());
+Assert.assertEquals(1, node.totalSlotsFree());
+Assert.assertEquals(3, node.totalSlotsUsed());
+Assert.assertEquals(4, node.totalSlots());
+
+List executors22 = new ArrayList<>();
+executors22.add(new ExecutorDetails(2, 2));
+node.assign(node.getFreeSlots().iterator().next(), topo
[GitHub] storm pull request: [STORM-1279] port backtype.storm.daemon.superv...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1257#discussion_r57458382
--- Diff:
storm-core/src/jvm/org/apache/storm/daemon/supervisor/StandaloneSupervisor.java
---
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.daemon.supervisor;
+
+import org.apache.storm.Config;
+import org.apache.storm.scheduler.ISupervisor;
+import org.apache.storm.utils.LocalState;
+import org.apache.storm.utils.Utils;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+
+public class StandaloneSupervisor implements ISupervisor {
+private String supervisorId;
+private Map conf;
+
+@Override
+public void prepare(Map stormConf, String schedulerLocalDir) {
+try {
+LocalState localState = new LocalState(schedulerLocalDir);
+String supervisorId = localState.getSupervisorId();
+if (supervisorId == null) {
+supervisorId = generateSupervisorId();
+localState.setSupervisorId(supervisorId);
+}
+this.conf = stormConf;
+this.supervisorId = supervisorId;
+} catch (IOException e) {
+throw new RuntimeException(e);
+}
+}
+
+@Override
+public String getSupervisorId() {
+return supervisorId;
+}
+
+@Override
+public String getAssignmentId() {
--- End diff --
Since assignmentId is in fact the same as supervisorId. Should we just
refactor and eliminate the assignmentId in the java version? Also want to hear
from @revans2 @d2r 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1279] port backtype.storm.daemon.superv...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1257#discussion_r57457270 --- Diff: storm-core/src/clj/org/apache/storm/daemon/local_supervisor.clj --- @@ -0,0 +1,64 @@ +;; Licensed to the Apache Software Foundation (ASF) under one +;; or more contributor license agreements. See the NOTICE file +;; distributed with this work for additional information +;; regarding copyright ownership. The ASF licenses this file +;; to you under the Apache License, Version 2.0 (the +;; "License"); you may not use this file except in compliance +;; with the License. You may obtain a copy of the License at +;; +;; http://www.apache.org/licenses/LICENSE-2.0 +;; +;; Unless required by applicable law or agreed to in writing, software +;; distributed under the License is distributed on an "AS IS" BASIS, +;; WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +;; See the License for the specific language governing permissions and +;; limitations under the License. +(ns org.apache.storm.daemon.local-supervisor + (:import [org.apache.storm.daemon.supervisor SyncProcessEvent SupervisorData Supervisor SupervisorUtils] + [org.apache.storm.utils Utils ConfigUtils] + [org.apache.storm ProcessSimulator]) + (:use [org.apache.storm.daemon common] +[org.apache.storm log]) + (:require [org.apache.storm.daemon [worker :as worker] ]) + (:require [clojure.string :as str]) + (:gen-class)) + +(defn launch-local-worker [supervisorData stormId port workerId resources] + (let [conf (.getConf supervisorData) + pid (Utils/uuid) +worker (worker/mk-worker conf + (.getSharedContext supervisorData) + stormId + (.getAssignmentId supervisorData) + (int port) + workerId)] +(ConfigUtils/setWorkerUserWSE conf workerId "") +(ProcessSimulator/registerProcess pid worker) +(.put (.getWorkerThreadPids supervisorData) workerId pid) +)) --- End diff -- newline, minor --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1232#issuecomment-201057484 Hi @knusbaum , comments 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1232#discussion_r57397001
--- Diff:
storm-core/test/jvm/org/apache/storm/scheduler/resource/TestUtilsForResourceAwareScheduler.java
---
@@ -109,7 +112,7 @@
public static Map
genExecsAndComps(StormTopology topology, int spoutParallelism, int
boltParallelism) {
Map retMap = new HashMap();
int startTask = 0;
-int endTask = 1;
+int endTask = 0;
--- End diff --
This corrects the executor number in tests to be [0,0] [1,1]; which used to
be [0, 1] [1,2], not actually what we have in Storm.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1232#issuecomment-200985657 Hi @jerrypeng @d2r @revans2 , would you mind having a look at this one? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1643 - Performance Fix: Optimize clojure...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1242#issuecomment-199603671 I would suggest testing a special topology with backpressure enabled. E.g., [this one.](https://github.com/zhuoliu/storm/blob/1643/examples/storm-starter/src/jvm/org/apache/storm/starter/WordCountTopology2.java), in which you can intermittently see a throttle-on flag (like "6703") under /backpressure/wc2-topo/ directory of Zookeeper. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1643 - Performance Fix: Optimize clojure...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1242#discussion_r56929816 --- Diff: storm-core/src/clj/org/apache/storm/daemon/executor.clj --- @@ -491,7 +493,7 @@ EVENTLOGGER-STREAM-ID [component-id message-id (System/currentTimeMillis) values] -(defmethod mk-threads :spout [executor-data task-datas initial-credentials] +(defmethod mk-threads :spout [executor-data task-datas initial-credentials throttle-on] --- End diff -- I think this is logically wrong. Throttle-on flag is a boolean that you have to obtain dynamically from the executor-data in run time, not that you can pass in as a constant when making the spout thread at the begining. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1300] backtype.storm.scheduler.resource...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1232 [STORM-1300] backtype.storm.scheduler.resource-aware-scheduler-test Port backtype.storm.scheduler.resource-aware-scheduler-test to java You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1300 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1232.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 #1232 commit c1b93de1650d113df0e1d0493780d9915bf3dacc Author: zhuol Date: 2016-03-18T21:06:00Z [STORM-1300] port backtype.storm.scheduler.resource-aware-scheduler-test to java. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1623] fix bug about nimbus.clj
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1211#issuecomment-197056859 Good catch. +1. Checked, it was introduced only to master. 1.x is OK. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1624] add maven central status in READM...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1212#issuecomment-197055834 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1614: backpressure init and cleanup chan...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1206#issuecomment-197019532 Looks good. +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1614: backpressure init and cleanup chan...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1206#discussion_r56240881 --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj --- @@ -73,6 +73,7 @@ (teardown-topology-errors! [this storm-id]) (heartbeat-storms [this]) (error-topologies [this]) + (backpressure-topologies [this]) --- End diff -- Sounds OK. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1614: backpressure changes and test
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1202#discussion_r55882679
--- Diff:
storm-core/test/jvm/org/apache/storm/cluster/StormClusterStateImplTest.java ---
@@ -0,0 +1,116 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.cluster;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.mockito.Mockito;
+import org.mockito.Matchers;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.zookeeper.KeeperException;
+
+import org.apache.storm.callback.ZKStateChangedCallback;
+import org.apache.storm.cluster.ClusterStateContext;
+
+public class StormClusterStateImplTest {
+
+private static final Logger LOG =
LoggerFactory.getLogger(StormClusterStateImplTest.class);
+private final String[] pathlist = { ClusterUtils.ASSIGNMENTS_SUBTREE,
+ClusterUtils.STORMS_SUBTREE,
+ClusterUtils.SUPERVISORS_SUBTREE,
+ClusterUtils.WORKERBEATS_SUBTREE,
+ClusterUtils.ERRORS_SUBTREE,
+ClusterUtils.BLOBSTORE_SUBTREE,
+ClusterUtils.NIMBUSES_SUBTREE,
+ClusterUtils.LOGCONFIG_SUBTREE,
+ClusterUtils.BACKPRESSURE_SUBTREE
};
+
+private IStateStorage storage;
+private ClusterStateContext context;
+private StormClusterStateImpl state;
+
+@Before
+public void init() throws Exception {
+storage = Mockito.mock(IStateStorage.class);
+context = new ClusterStateContext();
+state = new StormClusterStateImpl(storage, null /*acls*/, context,
false /*solo*/);
+}
+
+
+@Test
+public void registeredCallback() {
+
Mockito.verify(storage).register(Matchers.anyObject());
+}
+
+@Test
+public void createdZNodes() {
+for (String path : pathlist) {
+Mockito.verify(storage).mkdirs(path, null);
+}
+}
+
+@Test
+public void removeBackpressureDoesNotThrowTest() {
--- End diff --
Thanks! Great to have a StormClusterStateImplTest.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1614: backpressure changes and test
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1202#discussion_r55882536 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -1142,6 +1142,9 @@ (blob-rm-key blob-store (ConfigUtils/masterStormConfKey id) storm-cluster-state) (blob-rm-key blob-store (ConfigUtils/masterStormCodeKey id) storm-cluster-state)) +(defn force-delete-dir [conf id] --- End diff -- May rename this to be less general, e.g., "force-delete-topo-dist-dir" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1614: backpressure init and cleanup chan...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1206#issuecomment-195527698 Just minor nit, mostly looks good to me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1614: backpressure init and cleanup chan...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1206#discussion_r55881939 --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj --- @@ -73,6 +73,7 @@ (teardown-topology-errors! [this storm-id]) (heartbeat-storms [this]) (error-topologies [this]) + (backpressure-topologies [this]) --- End diff -- The name - "backpressure-topologies" is a little confusing (people may think "backpressure" as verb) since it just returns the current topologies' zk nodes (ids) under backpressure root. We might rename it as something like "get-backpressure-topologies-ids". --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Fix incorrect comment in default.yaml
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1201#issuecomment-195011076 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1610] port pacemaker_state_factory_test...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1192#issuecomment-194906318 still +1, sure --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1610] port pacemaker_state_factory_test...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1192#issuecomment-194074562 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1279] port backtype.storm.daemon.superv...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1184#discussion_r55254223
--- Diff:
storm-core/src/jvm/org/apache/storm/daemon/supervisor/StandaloneSupervisor.java
---
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.daemon.supervisor;
+
+import org.apache.storm.Config;
+import org.apache.storm.scheduler.ISupervisor;
+import org.apache.storm.utils.LocalState;
+import org.apache.storm.utils.Utils;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+
+public class StandaloneSupervisor implements ISupervisor {
+
+private String supervisorId;
+
--- End diff --
extra line
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1279] port backtype.storm.daemon.superv...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1184#discussion_r55253935
--- Diff:
storm-core/src/jvm/org/apache/storm/daemon/supervisor/ShutdownWork.java ---
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.daemon.supervisor;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.storm.Config;
+import org.apache.storm.ProcessSimulator;
+import org.apache.storm.daemon.Shutdownable;
+import org.apache.storm.utils.ConfigUtils;
+import org.apache.storm.utils.Time;
+import org.apache.storm.utils.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.*;
+
+public class ShutdownWork implements Shutdownable {
--- End diff --
It is OK to put the shutWorker function in a separate class but we not
think it should implement Shutdownable.
Or we might just go with the original implementation to put these two
functions in SupervisorUtils.java as static. Then SupervisorManger and
SyncProcessEvent will not need to extend ShutdownWork (they seem not natural
subclasses of ShutdownWork.)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: remove duplicate semecolon
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1152#issuecomment-189735905 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [Storm-1574] [1.x-branch]Better handle backpre...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1158#issuecomment-189378341 jdk8 test passed, jdk7 test failure seems unrelated. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [Storm-1574] [1.x-branch]Better handle backpre...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1158#issuecomment-189362515 Manual test passed. Could @kishorvpatil and @revans2 also look at this one? Mostly the same as [1149](https://github.com/apache/storm/pull/1149) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: remove duplicate semecolon
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1134#issuecomment-189351785 can we close this duplicate one? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [Storm-1574] [1.x-branch]Better handle backpre...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1158 [Storm-1574] [1.x-branch]Better handle backpressure exception etc. Better handle backpressure exception etc. Clear backpressure topo dir during kill. Add unit test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm Storm-1574-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1158.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 #1158 commit 32721e8470805282620eda75f0b560c895cf4669 Author: zhuol Date: 2016-02-26T16:07:30Z [STORM-1574][1.x-branch] Better handle backpressure exception & clear dir --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1149#issuecomment-189018954 Also, I need to put up another pull request for 1.x. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: remove duplicate semecolon
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1134#issuecomment-189015333 Can you please create a named branch in your repository and then do the pull request? Otherwise it would be difficult for it be 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: remove duplicate semecolon
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1134#issuecomment-189013152 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1488] fix broken component error times
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1148#issuecomment-189012476 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1149#issuecomment-189009158 @revans2 updated the test with timeout, and fixed the Cast exception (Int/Long) in Storm-1578, also manual test works fine. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1149#discussion_r54172980 --- Diff: storm-core/src/clj/org/apache/storm/daemon/worker.clj --- @@ -155,7 +155,10 @@ ;; update the worker's backpressure flag to zookeeper only when it has changed (log-debug "BP " @(:backpressure worker) " WAS " prev-backpressure-flag) (when (not= prev-backpressure-flag @(:backpressure worker)) - (.workerBackpressure storm-cluster-state storm-id assignment-id port @(:backpressure worker))) + (try +(.workerBackpressure storm-cluster-state storm-id assignment-id (long port) @(:backpressure worker)) --- End diff -- (:port worker) returns an Integer. Will cause an exception if not turning to long. See also Storm-1578. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1149#issuecomment-188939301 Since we will ignore all exception with onEvent in worker.clj, we should not need to test RuntimeException here. So delete it. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1149#issuecomment-188898763 @revans2 Updated the exception handling. Filed a JIRA for the Utils.uncaughtExceptionHandler renaming here: https://issues.apache.org/jira/browse/STORM-1577 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1149 [STORM-1574] Better handle backpressure thread exception & clear dir. 1. Better handle exception. 2. Remove topo dir during kill. 3. Add a unit test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1574 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1149.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 #1149 commit ee1a51993a4244dd6a3a112f70db0c8d74770557 Author: zhuol Date: 2016-02-24T22:56:39Z [STORM-1574] Better handle backpressure thread exception and clear topo dir. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1567] in defaults.yaml 'topology.disabl...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1135#issuecomment-188043611 LGTM. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Fix Log4j2.xml config to output the the timest...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1145#issuecomment-188035152 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1230] port backtype.storm.process-simul...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1100#issuecomment-186363277 @redsanket thanks, upmerged. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1230] port backtype.storm.process-simul...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1100#issuecomment-186023847 Hi @revans2, really appreciate your comments! I addressed your comment in [Storm-1563](https://issues.apache.org/jira/browse/STORM-1563) to change all the Shutdownable to be AutoClosable in storm. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1053#issuecomment-184812340 Looks good! +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1273] port backtype.storm.cluster to ja...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1081#issuecomment-184515933 We would also like @knusbaum to review this one. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1230] port backtype.storm.process-simul...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1100#discussion_r52790876
--- Diff: storm-core/src/jvm/org/apache/storm/ProcessSimulator.java ---
@@ -0,0 +1,89 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm;
+import org.apache.storm.daemon.Shutdownable;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ProcessSimulator {
+private static Logger LOG =
LoggerFactory.getLogger(ProcessSimulator.class);
+protected static Object lock = new Object();
+protected static ConcurrentHashMap processMap =
new ConcurrentHashMap();
+
+/**
+ * Register a process' handle
+ *
+ * @param pid
+ * @param shutdownable
+ */
+public static void registerProcess(String pid, Shutdownable
shutdownable) {
+processMap.put(pid, shutdownable);
+}
+
+/**
+ * Get a process' handle
+ *
+ * @param pid
+ * @return
+ */
+protected static Shutdownable getProcessHandle(String pid) {
+return processMap.get(pid);
+}
+
+/**
+ * Get all process handles
+ *
+ * @return
+ */
+public static Collection getAllProcessHandles() {
+return processMap.values();
+}
+
+/**
+ * Kill a process
+ *
+ * @param pid
+ */
+public static void killProcess(String pid) {
+synchronized (lock) {
+LOG.info("Begin killing process " + pid);
+Shutdownable shutdownHandle = getProcessHandle(pid);
+if (shutdownHandle != null) {
+shutdownHandle.shutdown();
+}
+processMap.remove(pid);
--- End diff --
Here I would prefer to keep it unchanged since It would be possible that
be wrongly put in the map by registerPorcess. Place the "remove"
outside of the "if" would help and not hurt, also this translation conforms the
original clj code logic.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1230] port backtype.storm.process-simul...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1100 [STORM-1230] port backtype.storm.process-simulator to java. From storm-core/src/clj/org/apache/storm/process-simulator.clj to storm-core/src/jvm/org/apache/storm/ProcessSimulator.java You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1230 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1100.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 #1100 commit 6965605105bf45f88b3e0225caf28a8e3f806926 Author: zhuol Date: 2016-02-11T23:30:55Z [STORM-1230] port backtype.storm.process-simulator to java. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1053#discussion_r52352681
--- Diff:
storm-core/src/jvm/org/apache/storm/container/cgroup/core/DevicesCore.java ---
@@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.container.cgroup.core;
+
+import org.apache.storm.container.cgroup.CgroupUtils;
+import org.apache.storm.container.cgroup.SubSystemType;
+import org.apache.storm.container.cgroup.Device;
+
+import java.io.IOException;
+import java.util.List;
+
+public class DevicesCore implements CgroupCore {
+
+private final String dir;
+
+public static final String DEVICES_ALLOW = "/devices.allow";
+public static final String DEVICES_DENY = "/devices.deny";
+public static final String DEVICES_LIST = "/devices.list";
+
+public static final char TYPE_ALL = 'a';
+public static final char TYPE_BLOCK = 'b';
+public static final char TYPE_CHAR = 'c';
+
+public static final int ACCESS_READ = 1;
+public static final int ACCESS_WRITE = 2;
+public static final int ACCESS_CREATE = 4;
+
+public static final char ACCESS_READ_CH = 'r';
+public static final char ACCESS_WRITE_CH = 'w';
+public static final char ACCESS_CREATE_CH = 'm';
--- End diff --
Can we make these final values private? Since they are only used in this
class.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1053#discussion_r52351834 --- Diff: conf/defaults.yaml --- @@ -263,7 +263,7 @@ topology.state.checkpoint.interval.ms: 1000 # topology priority describing the importance of the topology in decreasing importance starting from 0 (i.e. 0 is the highest priority and the priority importance decreases as the priority number increases). # Recommended range of 0-29 but no hard limit set. topology.priority: 29 -topology.component.resources.onheap.memory.mb: 128.0 +topology.component.resources.onheap.memory.mb: 256.0 --- End diff -- Should we have a separate default value for system components? Otherwise, users' topologies might use more memory than their previous impression. E.g., a WordCount example might need 6GB to start. Since this is an important setting, @revans2 , what is your opinion? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1053#discussion_r52342723
--- Diff:
storm-core/src/jvm/org/apache/storm/container/cgroup/CgroupManager.java ---
@@ -0,0 +1,207 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.container.cgroup;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.storm.Config;
+import org.apache.storm.container.ResourceIsolationInterface;
+import org.apache.storm.container.cgroup.core.CpuCore;
+import org.apache.storm.container.cgroup.core.MemoryCore;
+import org.apache.storm.utils.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Class that implements ResourceIsolationInterface that manages cgroups
+ */
+public class CgroupManager implements ResourceIsolationInterface {
+
+private static final Logger LOG =
LoggerFactory.getLogger(CgroupManager.class);
+
+private CgroupCenter center;
+
+private Hierarchy hierarchy;
+
+private CgroupCommon rootCgroup;
+
+private static String rootDir;
+
+private Map conf;
+
+/**
+ * initialize intial data structures
+ * @param conf storm confs
+ */
+public void prepare(Map conf) throws IOException {
+this.conf = conf;
+this.rootDir = Config.getCgroupRootDir(this.conf);
+if (this.rootDir == null) {
+throw new RuntimeException("Check configuration file. The
supervisor.cgroup.rootdir is missing.");
+}
+
+File file = new File(Config.getCgroupStormHierarchyDir(conf) + "/"
+ this.rootDir);
+if (!file.exists()) {
+LOG.error("{}/{} is not existing.",
Config.getCgroupStormHierarchyDir(conf), this.rootDir);
+throw new RuntimeException("Check if cgconfig service starts
or /etc/cgconfig.conf is consistent with configuration file.");
+}
+this.center = CgroupCenter.getInstance();
+if (this.center == null) {
+throw new RuntimeException("Cgroup error, please check
/proc/cgroups");
+}
+this.prepareSubSystem(this.conf);
+}
+
+/**
+ * initalize subsystems
+ */
+private void prepareSubSystem(Map conf) throws IOException {
+List subSystemTypes = new LinkedList<>();
+for (String resource : Config.getCgroupStormResources(conf)) {
+subSystemTypes.add(SubSystemType.getSubSystem(resource));
+}
+
+this.hierarchy = center.getHierarchyWithSubSystems(subSystemTypes);
+
+if (this.hierarchy == null) {
+Set types = new HashSet();
+types.add(SubSystemType.cpu);
+this.hierarchy = new
Hierarchy(Config.getCgroupStormHierarchyName(conf), types,
Config.getCgroupStormHierarchyDir(conf));
+}
+this.rootCgroup = new CgroupCommon(this.rootDir, this.hierarchy,
this.hierarchy.getRootCgroups());
+
+// set upper limit to how much cpu can be used by all workers
running on supervisor node.
+// This is done so that some cpu cycles will remain free to run
the daemons and other miscellaneous OS operations.
+CpuCore supervisorRootCPU = (CpuCore)
this.rootCgroup.getCores().get(SubSystemType.cpu);
--- End diff --
extra space
---
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
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1053#discussion_r52342637
--- Diff:
storm-core/src/jvm/org/apache/storm/container/cgroup/CgroupUtils.java ---
@@ -0,0 +1,128 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.container.cgroup;
+
+import org.apache.storm.utils.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+public class CgroupUtils {
+
+public static final String CGROUP_STATUS_FILE = "/proc/cgroups";
+public static final String MOUNT_STATUS_FILE = "/proc/mounts";
+
+private static final Logger LOG =
LoggerFactory.getLogger(CgroupUtils.class);
+
+public static void deleteDir(String dir) {
+File d = new File(dir);
+if (d.exists()) {
+if (d.isDirectory()) {
+if (!d.delete()) {
+throw new RuntimeException("Cannot delete dir " + dir);
+}
+} else {
+throw new RuntimeException("dir " + dir + " is not a
directory!");
+}
+} else {
+LOG.warn("dir {} does not exist!", dir);
+}
+}
+
+/**
+ * Get a set of SubSystemType objects from a comma delimited list of
subsystem names
+ */
+public static Set getSubSystemsFromString(String str) {
+Set result = new HashSet();
+String[] subSystems = str.split(",");
+for (String subSystem : subSystems) {
+SubSystemType type = SubSystemType.getSubSystem(subSystem);
+if (type != null) {
+result.add(type);
+}
+}
+return result;
+}
+
+/**
+ * Get a string that is a comma delimited list of subsystems
+ */
+public static String subSystemsToString(Set subSystems)
{
+StringBuilder sb = new StringBuilder();
+if (subSystems.size() == 0) {
+return sb.toString();
+}
+for (SubSystemType type : subSystems) {
+sb.append(type.name()).append(",");
+}
+return sb.toString().substring(0, sb.length() - 1);
+}
+
+public static boolean enabled() {
+return Utils.checkFileExists(CGROUP_STATUS_FILE);
+}
+
+public static List readFileByLine(String fileDir) throws
IOException {
+List result = new ArrayList();
+File file = new File(fileDir);
+try (FileReader fileReader = new FileReader(file);
+ BufferedReader reader = new BufferedReader(fileReader)) {
+String tempString = null;
+while ((tempString = reader.readLine()) != null) {
+result.add(tempString);
+}
+}
+return result;
+}
+
+public static void writeFileByLine(String fileDir, List
strings) throws IOException {
+LOG.debug("For CGroups - writing {} to {} ", strings, fileDir);
+File file = new File(fileDir);
+if (!file.exists()) {
+LOG.error("{} is no existed", fileDir);
--- End diff --
not => 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1053#discussion_r52342665
--- Diff:
storm-core/src/jvm/org/apache/storm/container/cgroup/CgroupOperation.java ---
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.container.cgroup;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * An interface to manage cgroups
+ */
+public interface CgroupOperation {
+
+/**
+ * Get a list of hierarchies
+ */
+public List getHierarchies();
+
+/**
+ * get a list of available subsystems
+ */
+public Set getSubSystems();
+
+/**
+ * Check if a subsystem is enabled
+ */
+public boolean isSubSystemEnabled(SubSystemType subsystem);
+
+/**
+ * get the first hierarchy that has a certain subsystem isMounted
+ */
+public Hierarchy getHierarchyWithSubSystem(SubSystemType subsystem);
+
+/**
+ * get the first hierarchy that has a certain list of subsystems
isMounted
+ */
+public Hierarchy getHierarchyWithSubSystems(List
subSystems);
+
--- End diff --
extra line
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1336] - Evalute/Port JStorm cgroup supp...
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1053#discussion_r52338605 --- Diff: conf/defaults.yaml --- @@ -263,7 +263,7 @@ topology.state.checkpoint.interval.ms: 1000 # topology priority describing the importance of the topology in decreasing importance starting from 0 (i.e. 0 is the highest priority and the priority importance decreases as the priority number increases). # Recommended range of 0-29 but no hard limit set. topology.priority: 29 -topology.component.resources.onheap.memory.mb: 128.0 +topology.component.resources.onheap.memory.mb: 256.0 --- End diff -- Do we have a reason for doubling the default per-instance memory 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1368] change heapdump file permissions ...
GitHub user zhuoliu opened a pull request: https://github.com/apache/storm/pull/1078 [STORM-1368] change heapdump file permissions so that UI download wil⦠Change heapdump file permissions so that UI download wil work in secure clusters. a. For automatically generated heapdump files by system b. For heapdump generated by user from UI You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhuoliu/storm 1368 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1078.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 #1078 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1510] Fix broken nimbus log link
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1057#issuecomment-176969832 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1227] port org.apache.storm.config to j...
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1030#issuecomment-175694846 Hi @knusbaum , I addressed your comments. Could you check back on this one? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1494 Link to supervisor logs form UI
Github user zhuoliu commented on the pull request: https://github.com/apache/storm/pull/1049#issuecomment-175250031 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1226
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1043#discussion_r50754732 --- Diff: storm-core/src/clj/org/apache/storm/util.clj --- @@ -48,21 +48,21 @@ (:require [ring.util.codec :as codec]) (:use [org.apache.storm log])) -(defn wrap-in-runtime - "Wraps an exception in a RuntimeException if needed" - [^Exception e] - (if (instance? RuntimeException e) -e -(RuntimeException. e))) +;(defn wrap-in-runtime --- End diff -- May delete the clojure functions rather than comment them out. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1226
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1043#discussion_r50754345 --- Diff: storm-core/test/clj/org/apache/storm/security/auth/auth_test.clj --- @@ -14,27 +14,28 @@ ;; See the License for the specific language governing permissions and ;; limitations under the License. (ns org.apache.storm.security.auth.auth-test - (:use [clojure test]) - (:require [org.apache.storm.daemon [nimbus :as nimbus]]) - (:import [org.apache.thrift TException] +(:use [clojure test]) --- End diff -- May not need to change the indentions 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1226
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1043#discussion_r50754397 --- Diff: storm-core/test/clj/org/apache/storm/security/serialization/BlowfishTupleSerializer_test.clj --- @@ -15,7 +15,7 @@ ;; limitations under the License. (ns org.apache.storm.security.serialization.BlowfishTupleSerializer-test (:use [clojure test] -[org.apache.storm.util :only (exception-cause?)] +;[org.apache.storm.util :only (exception-cause?)] --- End diff -- Delete 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: Storm 1226
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1043#discussion_r50754051
--- Diff: storm-core/test/clj/org/apache/storm/nimbus_test.clj ---
@@ -1198,21 +1220,23 @@
(let [expected-name topology-name
expected-conf {TOPOLOGY-NAME expected-name
- :foo :bar}]
+ "foo" "bar"}]
--- End diff --
Should we apply clojurify-structure to the map and avoid changing the key
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 1226
Github user zhuoliu commented on a diff in the pull request: https://github.com/apache/storm/pull/1043#discussion_r50753833 --- Diff: storm-core/test/clj/org/apache/storm/nimbus_test.clj --- @@ -352,6 +372,7 @@ (is (= 2 (storm-num-workers state "mystorm"))) ;; because only 2 executors ))) +;TODO: when translating this function, you should replace the map-val with a proper for loop HERE --- End diff -- "A proper" means? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1227] port org.apache.storm.config to j...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1030#discussion_r50602689
--- Diff: storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java ---
@@ -0,0 +1,711 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.utils;
+
+import org.apache.storm.Config;
+import org.apache.storm.validation.ConfigValidation;
+import org.apache.storm.generated.StormTopology;
+import org.apache.commons.io.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.*;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Collections;
+
+public class ConfigUtils {
+private final static Logger LOG =
LoggerFactory.getLogger(ConfigUtils.class);
+public final static String RESOURCES_SUBDIR = "resources";
+public final static String NIMBUS_DO_NOT_REASSIGN =
"NIMBUS-DO-NOT-REASSIGN";
+public static final String FILE_SEPARATOR = File.separator;
+public final static String LOG_DIR;
+
+static {
+String dir;
+Map conf;
+if (System.getProperty("storm.log.dir") != null) {
+dir = System.getProperty("storm.log.dir");
+} else if ((conf = readStormConfig()).get("storm.log.dir") !=
null) {
+dir = String.valueOf(conf.get("storm.log.dir"));
+} else {
+if (System.getProperty("storm.home") != null) {
+dir = System.getProperty("storm.home") + FILE_SEPARATOR +
"logs";
+} else {
+dir = FILE_SEPARATOR + "logs";
+}
+}
+try {
+LOG_DIR = new File(dir).getCanonicalPath();
+} catch (IOException ex) {
+throw new IllegalArgumentException("Illegal storm.log.dir in
conf: " + dir);
+}
+}
+
+public static String clojureConfigName(String name) {
+return name.toUpperCase().replace("_", "-");
+}
+
+// ALL-CONFIGS is only used by executor.clj once, do we want to do it
here? TODO
+public static List All_CONFIGS() {
+List ret = new ArrayList();
+Config config = new Config();
+Class ConfigClass = config.getClass();
+Field[] fields = ConfigClass.getFields();
+for (int i = 0; i < fields.length; i++) {
+try {
+Object obj = fields[i].get(null);
+ret.add(obj);
+} catch (IllegalArgumentException e) {
+LOG.error(e.getMessage(), e);
+} catch (IllegalAccessException e) {
+LOG.error(e.getMessage(), e);
+}
+}
+return ret;
+}
+
+public static String clusterMode(Map conf) {
+String mode = (String) conf.get(Config.STORM_CLUSTER_MODE);
+return mode;
+
+}
+
+public static boolean isLocalMode(Map conf) {
+String mode = (String) conf.get(Config.STORM_CLUSTER_MODE);
+if (mode != null) {
+if ("local".equals(mode)) {
+return true;
+}
+if ("distributed".equals(mode)) {
+return false;
+}
+}
+throw new IllegalArgumentException("Illegal cluster mode in conf:
" + mode);
+}
+
+public static int samplingRate(Map conf) {
+double rate =
Utils.getDouble(conf.get(Config.TOPOLOGY_STATS_SAMPLE_RATE));
+if (rate != 0) {
+return (int) (1 / rate);
+}
+throw
[GitHub] storm pull request: [STORM-1227] port org.apache.storm.config to j...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1030#discussion_r50602239
--- Diff: storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java ---
@@ -0,0 +1,711 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.utils;
+
+import org.apache.storm.Config;
+import org.apache.storm.validation.ConfigValidation;
+import org.apache.storm.generated.StormTopology;
+import org.apache.commons.io.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.*;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Collections;
+
+public class ConfigUtils {
+private final static Logger LOG =
LoggerFactory.getLogger(ConfigUtils.class);
+public final static String RESOURCES_SUBDIR = "resources";
+public final static String NIMBUS_DO_NOT_REASSIGN =
"NIMBUS-DO-NOT-REASSIGN";
+public static final String FILE_SEPARATOR = File.separator;
+public final static String LOG_DIR;
+
+static {
+String dir;
+Map conf;
+if (System.getProperty("storm.log.dir") != null) {
+dir = System.getProperty("storm.log.dir");
+} else if ((conf = readStormConfig()).get("storm.log.dir") !=
null) {
+dir = String.valueOf(conf.get("storm.log.dir"));
+} else {
+if (System.getProperty("storm.home") != null) {
+dir = System.getProperty("storm.home") + FILE_SEPARATOR +
"logs";
+} else {
+dir = FILE_SEPARATOR + "logs";
+}
+}
+try {
+LOG_DIR = new File(dir).getCanonicalPath();
+} catch (IOException ex) {
+throw new IllegalArgumentException("Illegal storm.log.dir in
conf: " + dir);
+}
+}
+
+public static String clojureConfigName(String name) {
+return name.toUpperCase().replace("_", "-");
+}
+
+// ALL-CONFIGS is only used by executor.clj once, do we want to do it
here? TODO
+public static List All_CONFIGS() {
+List ret = new ArrayList();
+Config config = new Config();
+Class ConfigClass = config.getClass();
+Field[] fields = ConfigClass.getFields();
+for (int i = 0; i < fields.length; i++) {
+try {
+Object obj = fields[i].get(null);
+ret.add(obj);
+} catch (IllegalArgumentException e) {
+LOG.error(e.getMessage(), e);
+} catch (IllegalAccessException e) {
+LOG.error(e.getMessage(), e);
+}
+}
+return ret;
+}
+
+public static String clusterMode(Map conf) {
+String mode = (String) conf.get(Config.STORM_CLUSTER_MODE);
+return mode;
+
+}
+
+public static boolean isLocalMode(Map conf) {
+String mode = (String) conf.get(Config.STORM_CLUSTER_MODE);
+if (mode != null) {
+if ("local".equals(mode)) {
+return true;
+}
+if ("distributed".equals(mode)) {
+return false;
+}
+}
+throw new IllegalArgumentException("Illegal cluster mode in conf:
" + mode);
+}
+
+public static int samplingRate(Map conf) {
+double rate =
Utils.getDouble(conf.get(Config.TOPOLOGY_STATS_SAMPLE_RATE));
+if (rate != 0) {
+return (int) (1 / rate);
+}
+throw
[GitHub] storm pull request: [STORM-1227] port org.apache.storm.config to j...
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1030#discussion_r50600899
--- Diff:
storm-core/src/jvm/org/apache/storm/testing/staticmocking/MockedConfigUtils.java
---
@@ -0,0 +1,31 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version
+ * 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.testing.staticmocking;
+
+import org.apache.storm.utils.ConfigUtils;
+
+public class MockedConfigUtils extends ConfigUtils implements
AutoCloseable {
--- End diff --
Yep, the above documentation file will go in with util translation
(STORM-1226)[https://issues.apache.org/jira/browse/STORM-1226].
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
