[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/810 --- 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 enabl

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150730065 @kishorvpatil Amazing! +1. Test failure is unrelated. (Kafka) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub a

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150729673 @HeartSaVioR - Modified to make "nimbus.reassign" unavailable for users. - Created #815 to decommission variable for 0.10.x branch. --- If your project

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150712631 @kishorvpatil No, just marking ```nimbus.reassign``` option as deprecate and leave proper reason from 0.10.x would be fine. --- If your project is set up for it

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150711148 @HeartSaVioR I complete agree with decommissioning this configuration used only for testing. It should be unavailable for users. Let me deprecate this variable and a

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150705382 @kishorvpatil With this change users have no option to turn off reassign since it makes cluster unusable (cluster cannot run topology unless user calls other oper

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150703213 @HeartSaVioR I don't think we can remove `NIMBUS-REASSIGN` as it used mostly during supervisor_test to avoid rescheduling with value `false`. The documentation clear

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150702270 @HeartSaVioR Removed rebalance calls at places where I could use simulated time cluster. --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150701869 @kishorvpatil Good, I didn't think about mocking. Your additional changes look great. Just I wish to discuss how we can do with ```nimbus.reassign```.

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150699962 @HeartSaVioR I tried that for supervisor_test.clj, somehow mocking of submit topologies etc.methods don't make it easy. It seems straight forward to rebalance and p

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150699176 @kishorvpatil Seems like some places in the tests still rely on rebalance. Could we change these to use simulated-time cluster as well when possible? --- If you

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150697902 @kishorvpatil Test changes look good. @kishorvpatil @revans2 I'd like to see my previous comment (https://github.com/apache/storm/pull/810#issuecomm

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150671588 The changes look good I am +1, and time travis took is about the same as before. --- If your project is set up for it, you can reply to this email and have your reply ap

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150667453 @HeartSaVioR Thank you for review and suggestions. I have made changes so that our tests are using simulated-time cluster to avoid `Thread/sleep`. Please revisit the

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150051486 @kishorvpatil rebalance seems not work cause it requires topology to be alive at the moment. --- If your project is set up for it, you can reply to this email a

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150049582 @HeartSaVioR looking into compilation issue. Somehow it did not get committed. Re-running the build. --- If your project is set up for it, you can reply to this em

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150047429 I also love the concept. Btw, in order to rely on recurring mk-assignments, - We should remove ```nimbus.reassign``` since it should be true for new topo

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-150045478 @kishorvpatil You may want to check Travis build failure, seems like there's a missing spot. > Exception in thread "main" java.lang.IllegalArgumentExcept

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/810#issuecomment-149996906 I love the concept, but I would like to see the tests updated so we are not adding several mins to the time it takes to run the unit tests. --- If your project is set up

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/810#discussion_r42666392 --- Diff: storm-core/test/clj/backtype/storm/integration_test.clj --- @@ -236,6 +237,7 @@ "acking-test1"

[GitHub] storm pull request: [STORM-1121] Remove method call to avoid overh...

2015-10-21 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/810 [STORM-1121] Remove method call to avoid overhead during topology submission time Nimbus calls mk-assignments from SubmitTopology within lock is causing it to wait for processing of all heartbe