[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138659723 --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj --- @@ -244,15 +255,19 @@ ;; Watches should be used for optimization. When ZK is

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138663085 --- Diff: storm-core/src/jvm/org/apache/storm/assignments/AssignmentDistributionService.java --- @@ -0,0 +1,232 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138659373 --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj --- @@ -244,15 +255,19 @@ ;; Watches should be used for optimization. When ZK is

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138662304 --- Diff: storm-core/src/jvm/org/apache/storm/assignments/AssignmentDistributionService.java --- @@ -0,0 +1,232 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138657756 --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj --- @@ -123,6 +133,7 @@ (def SUPERVISORS-SUBTREE (str "/" SUPERVISORS-ROOT))

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138662036 --- Diff: storm-core/src/jvm/org/apache/storm/assignments/AssignmentDistributionService.java --- @@ -0,0 +1,232 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2319: [STORM-2693] Nimbus assignments promotion

2017-09-13 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2319 @danny0405 I am not concerned about the memory for the cache, because we are already keeping all of that in memory while we schedule the topologies anyways. If it does prove to be an issue we can

[GitHub] storm issue #2320: STORM-2736: fix o.a.s.b.BlobStoreUtils [ERROR] "Could not...

2017-09-13 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2320 Still +1 ---

[GitHub] storm issue #2323: STORM-2736: fix o.a.s.b.BlobStoreUtils [ERROR] "Could not...

2017-09-13 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2323 Still +1 ---

[GitHub] storm issue #2323: STORM-2736: fix o.a.s.b.BlobStoreUtils [ERROR] "Could not...

2017-09-12 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2323 We now need to wait for the required 24 hours from when the pull request went up to give others time to review if they want to. ---

[GitHub] storm pull request #2321: STORM-2733: Better load aware shuffle implementati...

2017-09-11 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2321 STORM-2733: Better load aware shuffle implementation I have run several tests that show this works much better at big imbalances in processing latency than did the previous shuffle implementations

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138099459 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -961,14 +1015,25 @@ ;; tasks figure out what tasks to talk to by looking at

[GitHub] storm issue #1082: STORM-1492 With nimbus.seeds set to default, a nimbus for...

2017-09-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1082 Still +1 ---

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138093197 --- Diff: storm-core/pom.xml --- @@ -331,6 +331,10 @@ commons-codec +org.rocksdb

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138089602 --- Diff: storm-client/src/jvm/org/apache/storm/Config.java --- @@ -205,6 +205,12 @@ public static final String TOPOLOGY_TASKS = "topology.

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090061 --- Diff: storm-client/src/jvm/org/apache/storm/Config.java --- @@ -1193,6 +1199,12 @@ public static final String SUPERVISOR_CPU_CAPACITY

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090802 --- Diff: storm-client/src/jvm/org/apache/storm/topology/ComponentConfigurationDeclarer.java --- @@ -62,4 +69,14 @@ * @return this for chaining

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090901 --- Diff: storm-client/src/jvm/org/apache/storm/topology/ComponentConfigurationDeclarer.java --- @@ -62,4 +69,14 @@ * @return this for chaining

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090258 --- Diff: storm-client/src/jvm/org/apache/storm/topology/BaseConfigurationDeclarer.java --- @@ -83,4 +83,29 @@ public T setCPULoad(Number amount

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090558 --- Diff: storm-client/src/jvm/org/apache/storm/topology/BaseConfigurationDeclarer.java --- @@ -83,4 +83,29 @@ public T setCPULoad(Number amount

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138091077 --- Diff: storm-client/src/jvm/org/apache/storm/topology/ComponentConfigurationDeclarer.java --- @@ -62,4 +69,14 @@ * @return this for chaining

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138090750 --- Diff: storm-client/src/jvm/org/apache/storm/topology/ComponentConfigurationDeclarer.java --- @@ -28,6 +28,13 @@ T addConfigurations(Map conf

[GitHub] storm pull request #2315: [STORM-2732] Close the eldest writer before removi...

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2315#discussion_r138088942 --- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/AbstractHdfsBolt.java --- @@ -317,7 +319,18 @@ public WritersMap(long maxWriters

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138074478 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -758,6 +752,35 @@ (catch Exception e (log-error e "

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138073996 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -590,6 +583,7 @@ (set (keys new-assignment

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138074935 --- Diff: storm-core/src/clj/org/apache/storm/daemon/worker.clj --- @@ -351,6 +373,8 @@ (.sendLoadMetrics (:receiver worker) local-pop

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138080750 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -961,14 +1015,25 @@ ;; tasks figure out what tasks to talk to by looking at

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138073574 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -62,26 +63,17 @@ (defn- assignments-snapshot [storm-cluster-state

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138074168 --- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj --- @@ -758,6 +752,35 @@ (catch Exception e (log-error e "

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138074783 --- Diff: storm-core/src/clj/org/apache/storm/daemon/worker.clj --- @@ -47,9 +48,30 @@ (defmulti mk-suicide-fn cluster-mode) +(defn get

[GitHub] storm pull request #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2319#discussion_r138071918 --- Diff: storm-core/pom.xml --- @@ -331,6 +331,10 @@ commons-codec +org.rocksdb

[GitHub] storm issue #2319: [STORM-2693] Nimbus assignments promotion

2017-09-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2319 @danny0405 conceptually this is a wonderful improvement. I will try to take some time looking into this patch. One of the big things we will need though is a corresponding patch for the master

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-09-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r137805760 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/AnchoredWordCount.java --- @@ -0,0 +1,138 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-09-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @roshannaik I merged this into master, but if you want any changes to it please just ask and I'll see what I can do. ---

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-09-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @knusbaum do you have any other comments? ---

[GitHub] storm issue #2308: MINOR: Update DruidBeamBolt logs

2017-09-06 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2308 @omkreddy I merged this into master and 1.x-branch ---

[GitHub] storm pull request #2308: MINOR: Update DruidBeamBolt logs

2017-09-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2308#discussion_r136989219 --- Diff: external/storm-druid/src/main/java/org/apache/storm/druid/bolt/DruidBeamBolt.java --- @@ -78,27 +76,28 @@ public void prepare(Map stormConf

[GitHub] storm pull request #2308: MINOR: Update DruidBeamBolt logs

2017-09-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2308#discussion_r136990714 --- Diff: external/storm-druid/src/main/java/org/apache/storm/druid/bolt/DruidBeamBolt.java --- @@ -78,27 +76,28 @@ public void prepare(Map stormConf

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-09-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @roshannaik I think I have added in all of your feature requests so far. --- 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

[GitHub] storm issue #2301: Quick fix: remove storm-rename-hack dependency

2017-09-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2301 +1 sorry I missed it when I did the other hanges --- 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

[GitHub] storm issue #2199: [STORM-2201] Add dynamic scheduler configuration loading

2017-08-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2199 @HeartSaVioR you took a look at this patch before, do you want to take another look at it? @Ethanlm could you squash your commits into just one? --- If your project is set up for it, you

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-29 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135863302 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/ArtifactoryConfigLoader.java --- @@ -0,0 +1,395 @@ +/** + * Licensed to the

[GitHub] storm issue #2277: STORM-2666: Fix storm-kafka-client spout sometimes emitti...

2017-08-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2277 @hmcl do you have any concerns about the patch? --- 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

[GitHub] storm pull request #1674: STORM-2083: Blacklist scheduler

2017-08-29 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1674#discussion_r135829430 --- Diff: storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java --- @@ -0,0 +1,250 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2294: STORM-2517 add interface for Writer, make AbstractHDFSWri...

2017-08-28 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2294 @Angus-Slalom actually because these are all just white space changes I can do it myself when I check it in. Let me run the tests all the way through to be sure it looks good then I'll

[GitHub] storm issue #2294: STORM-2517 add interface for Writer, make AbstractHDFSWri...

2017-08-28 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2294 ``` external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/Writer.java 18: 'package' should be separated from previous statement. 23: Wrong lexicographical

[GitHub] storm issue #2294: STORM-2517 add interface for Writer, make AbstractHDFSWri...

2017-08-28 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2294 @Angus-Slalom there are some checkstyle violations here, which is why the build is failing. Other then that it looks good. +1 --- If your project is set up for it, you can reply to this email and

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-28 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135624254 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/ArtifactoryConfigLoader.java --- @@ -0,0 +1,395 @@ +/** + * Licensed to the

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-28 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135624834 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/ArtifactoryConfigLoader.java --- @@ -0,0 +1,416 @@ +/** + * Licensed to the

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-28 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135618433 --- Diff: storm-server/pom.xml --- @@ -59,6 +59,10 @@ org.apache.commons commons-compress

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-28 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135621982 --- Diff: storm-server/src/main/java/org/apache/storm/DaemonConfig.java --- @@ -792,6 +792,12 @@ public static final String

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-08-28 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2270 I spoke with @Ethanlm and I think we have a plan on how to do it, but we are going to have to run a lot of tests before we feel good about it. For the time being lets get #2261 in and then we will

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-08-28 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2270 @HeartSaVioR I spent the week end and came up with a prototype that appears to be working for combining the two, but I want to talk with @Ethanlm and work out the differences between the

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-25 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135334556 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/IConfigLoader.java --- @@ -0,0 +1,138 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2249: STORM-2648/STORM-2357: Add storm-kafka-client support for...

2017-08-25 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2249 Still +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

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-08-25 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @knusbaum I think I have addressed all of your review comments so far. --- 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

[GitHub] storm pull request #2289: STORM-2702: storm-loadgen

2017-08-25 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2289#discussion_r135307748 --- Diff: examples/storm-loadgen/src/main/java/org/apache/storm/loadgen/InputStream.java --- @@ -0,0 +1,263 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-25 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135291998 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/ArtifactoryHttpConfigLoader.java --- @@ -0,0 +1,375 @@ +/** + * Licensed to

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-25 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135290270 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/IConfigLoader.java --- @@ -0,0 +1,138 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #2199: [STORM-2201] Add dynamic scheduler configuration l...

2017-08-25 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2199#discussion_r135292464 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/utils/IConfigLoader.java --- @@ -0,0 +1,138 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-08-25 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 I think I am done with new features unless someone else comes up with something else that I missed out on. I really would appreciate some reviews. @knusbaum if you have the time I know you expressed

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-08-24 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @roshannaik I just added in support for a new default reporter that writes the data out in a fixed width format that should be much more human readable, with formatting, and it is added as a

[GitHub] storm issue #2289: STORM-2702: storm-loadgen

2017-08-24 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2289 @roshannaik The latency reported is a simulation of kafka or something like it. The start time, is not when the message is emitted by the spout. The start time is when the message would have been

[GitHub] storm issue #2231: Add link to issue tracker on Readme page.

2017-08-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2231 @ecararus I pulled this into master, and fixed the one minor spelling comment (its documentation so we don't technically need a vote for it). Thanks for fixing this. Keep up the good

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-08-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2270 Thanks to everyone for all of the help in looking at this and testing it. I know there is still some low hanging fruit in improving the performance in many areas. As @roshannaik pointed out

[GitHub] storm pull request #2271: STORM-2675: Fix storm-kafka-client Trident spout f...

2017-08-23 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2271#discussion_r134869659 --- Diff: examples/storm-kafka-examples/pom.xml --- @@ -43,19 +48,19 @@ org.apache.storm storm-kafka

[GitHub] storm issue #2288: [STORM-2700] Should not check Blobstore ACL if the valida...

2017-08-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2288 @HeartSaVioR I agree that it should not be false by default for a secure cluster. But like @Ethanlm said it is not a simple task to make part of security work on an insecure cluster. The other

[GitHub] storm issue #2261: STORM-2678 Improve performance of LoadAwareShuffleGroupin...

2017-08-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2261 I spent the last few hours running more tests and I get the same results. I am not too concerned about it. The overhead appears to be rather low if any. --- If your project is set up for it, you

[GitHub] storm issue #2261: STORM-2678 Improve performance of LoadAwareShuffleGroupin...

2017-08-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2261 From a code perspective the changes look fine to me. It would be nice to possibly clean up the load aware grouping Interface some. From a performance perspective it is a bit of a wash from

[GitHub] storm pull request #2261: STORM-2678 Improve performance of LoadAwareShuffle...

2017-08-23 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2261#discussion_r134778874 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareCustomStreamGrouping.java --- @@ -20,5 +20,6 @@ import java.util.List

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-21 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik I just created a new pull request to master for an updated version of ThroughputVsLatency and I added in the test tools that I created for being able to capture and simulate topologies

[GitHub] storm pull request #2289: STORM-2702: storm-loadgen

2017-08-21 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2289 STORM-2702: storm-loadgen Some tools to be able to generate load on a cluster for testing. You can merge this pull request into a Git repository by running: $ git pull https://github.com

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-17 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik I found another bug in the code. For batching you are deciding to turn on or off flushing based off of the system conf, and not the topology conf. This means that I as a topology owner

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-16 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik I have some new performance numbers. These are not final, as I have not done any tuning yet. But let me explain the test. I really wanted to get a handle on how this would impact me

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-14 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik yes we can use aaebc3b as the base for tests --- 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

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 Also I wanted to add that I found another bug. The process latency metric is always right about a -1ms. Not sure what would be causing this. --- If your project is set up for it, you can reply to

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik The output collector is wrapped separately by each bolt so it will have no effect. https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/topology

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @harshach all of those tests look good to me. I just want to be sure that we do them in both a multi-worker and multi-node setup. --- If your project is set up for it, you can reply to this email

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-09 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik `synchronized(collector)` also does not work in all cases because we wrap the collector in many cases. https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 I am really nervous about this patch in general and I am still a -1 on it. For me to remove my -1 at a minimum all of the unit tests have to pass and all critical functionality has to be

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik removing thread safety from the emits is a bug and breaks backwards compatibility. In the past the shuffle grouping was the only one that was not thread safe. Because of this

[GitHub] storm pull request #2267: STORM-2682: Fix issues with null owner on rolling ...

2017-08-07 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2267 STORM-2682: Fix issues with null owner on rolling upgrade (1.0.x) This is the 1.0.x version of #2266 and should probably apply cleanly to all of the 1.x line of releases. This prevents

[GitHub] storm pull request #2266: STORM-2682: Fixed issues with null owner

2017-08-07 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2266 STORM-2682: Fixed issues with null owner You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2682 Alternatively you

[GitHub] storm issue #2260: parametrizing the debug log messages to improve the perfo...

2017-08-04 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2260 Thanks @tterstep I merged this into master. Keep up the good work --- 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

[GitHub] storm pull request #2248: STORM-2028: Fix for uprooting the JDBC client exce...

2017-08-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2248#discussion_r131263697 --- Diff: external/storm-jdbc/src/test/java/org/apache/storm/jdbc/common/JdbcClientTest.java --- @@ -80,6 +85,24 @@ public void testInsertAndSelect

[GitHub] storm issue #2248: STORM-2028: Fix for uprooting the JDBC client exceptions ...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2248 To me it looks good, just one question about the test (and this is mostly for my own knowledge). --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm issue #2249: WIP: STORM-2648/STORM-2357: Add storm-kafka-client suppor...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2249 +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

[GitHub] storm issue #2249: WIP: STORM-2648/STORM-2357: Add storm-kafka-client suppor...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2249 Conceptually the changes look good to me. I have not dug into it in great detail yet, but I do like the direction of the change. I would also like to see the documentation and examples

[GitHub] storm issue #2259: [STORM-2676] Error class name for log in JsonRecordHiveMa...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2259 @liu-zhaokun we have a policy that we need to wait 24 hours before merging a change, even a trivial one like this to give all devs time to review and comment. --- If your project is set up for it

[GitHub] storm issue #2259: [STORM-2676] Error class name for log in JsonRecordHiveMa...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2259 +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

[GitHub] storm issue #2260: parametrizing the debug log messages to improve the perfo...

2017-08-03 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2260 +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

[GitHub] storm issue #2255: [STORM-2672] Expose a metric for calls to reportError()

2017-08-02 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2255 Could you update the metrics documentation to include the new metric? This is new and under `docs/Metrics.md` otherwise the change looks good +1 --- If your project is set up for it, you

[GitHub] storm issue #2255: [STORM-2672] Expose a metric for calls to reportError()

2017-08-02 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2255 Could you update the metrics documentation to include the new metric? This is new and under `docs/Metrics.md` otherwise the change looks good. --- If your project is set up for it, you

[GitHub] storm pull request #2257: STORM-2673: add support for prioritizing nodes in ...

2017-08-02 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2257 STORM-2673: add support for prioritizing nodes in scheduling I really would like feedback on the configs and if it makes since to modify other schedulers to support this too. Config: I

[GitHub] storm pull request #2254: STORM-2671: remove the storm rename hack

2017-08-02 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2254 STORM-2671: remove the storm rename hack You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2671 Alternatively you

[GitHub] storm issue #2253: Remove CHANGELOG.md and update DEVELOPER.md to reflect th...

2017-08-02 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2253 For got to say that I am +1 once we have that script, or at least enough progress on it to feel good that it is going to happen. --- If your project is set up for it, you can reply to this email

[GitHub] storm issue #2253: Remove CHANGELOG.md and update DEVELOPER.md to reflect th...

2017-08-02 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2253 This looks good. It removes that changelog and updates the docs. But I want to be sure that we have a script for doing the changelog on a release before we actually remove the changelog. Otherwise

[GitHub] storm pull request #2240: [STORM-2657] Update SECURITY.MD

2017-07-31 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2240#discussion_r130439781 --- Diff: docs/SECURITY.md --- @@ -478,6 +478,35 @@ nimbus.groups: ### DRPC -Hopefully more on this soon + + Storm provides

[GitHub] storm pull request #2240: [STORM-2657] Update SECURITY.MD

2017-07-31 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2240#discussion_r130436600 --- Diff: docs/SECURITY.md --- @@ -478,6 +478,35 @@ nimbus.groups: ### DRPC -Hopefully more on this soon + + Storm provides

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-07-31 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik sorry I just reread your other post and it looks like I missed some of your questions. `topology.producer.batch.size` and `topology.flush.tuple.freq.millis` are what ever the

<    4   5   6   7   8   9   10   11   12   13   >