[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1790 @kevpeek Could you squash your commits into one? The change is small enough and other commits are closer to code style. I can handle it while merging, but PR for non-master branch is

[GitHub] storm issue #1787: STORM-2208: HDFS State Throws FileNotFoundException in Az...

Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1787 @revans2 I just tested in the Azure cluster running AFS and the fix works. Please see attached screenshots. ![broken_2016-11-22_6 43

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user ambud commented on the issue: https://github.com/apache/storm/pull/1783 So should I revert back my changes; seems like the build is currently failing; additional tests pushed by upstream? --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user ben-manes commented on the issue: https://github.com/apache/storm/pull/1783 Nope. Sorry, since your compilation target is 1.8 I hadn't thought you'd need that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1783 One thing to consider is that Caffeine seems to require Java 8, which means that this patch can't be shipped to 1.x version line. Do we want to keep using Guava for 1.x branch? @ben-manes Is

[GitHub] storm pull request #1783: STORM-2204 Adding caching capabilities in HBaseLoo...

Github user vesense commented on a diff in the pull request: https://github.com/apache/storm/pull/1783#discussion_r89250230 --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/bolt/HBaseLookupBolt.java --- @@ -40,51 +48,81 @@ * */ public class

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user vesense commented on the issue: https://github.com/apache/storm/pull/1783 +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

Re: Twitter4j dependency (can't release Apache Storm)

+1 to remove it. It's a small change and dose not hurt any critical code. 2016-11-23 8:42 GMT+08:00 Jungtaek Lim : > +1 to remove it. Removing twitter4j example is not a big deal for me. > > - Jungtaek Lim (HeartSaVioR) > > 2016년 11월 23일 (수) 오전 6:32, Bobby Evans

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

Github user erikdw commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r89243737 --- Diff: storm-core/src/jvm/org/apache/storm/blobstore/BlobStore.java --- @@ -304,6 +307,41 @@ public void readBlobTo(String key, OutputStream out, Subject

Re: Twitter4j dependency (can't release Apache Storm)

+1 to remove it. Removing twitter4j example is not a big deal for me. - Jungtaek Lim (HeartSaVioR) 2016년 11월 23일 (수) 오전 6:32, Bobby Evans 님이 작성: > If it is just one file and it is an example I would say lets remove it. > If we are worried about it we could add in a

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user Crim commented on the issue: https://github.com/apache/storm/pull/1790 I feel you, tests can't cover everything. Performance tests are even more difficult to put together something that's actually valid. --- If your project is set up for it, you can reply to this email

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1790 @Crim One thing to be clear, please don't consider my comment as on behalf of community's opinion. Opinions are completely on my own. To be honest, I'm not a big fan of test

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user Crim commented on the issue: https://github.com/apache/storm/pull/1790 Your comment is a tad worrisome. Is test coverage not at a place where project contributors can feel good about a release? Or is each release essentially QA'd by early adopters? --- If your project

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1790 @ohadedel I don't know the intention of 'equivalent' (implementation, or result), but if it means only the result, the result of twos would be similar if you think far. They provide uniform

Re: Twitter4j dependency (can't release Apache Storm)

If it is just one file and it is an example I would say lets remove it.  If we are worried about it we could add in a pointer to an older release as an example with a big warning about the license. - Bobby On Tuesday, November 22, 2016, 3:13:14 PM CST, P. Taylor Goetz

[GitHub] storm issue #1791: STORM-2212: Remove Redundant Declarations in Maven POM Fi...

Github user dossett commented on the issue: https://github.com/apache/storm/pull/1791 +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

Twitter4j dependency (can't release Apache Storm)

The ASF recently made the determination that the org.json license is category x, meaning projects can’t release code that depends on it (the short reason is the license has a “no evil” clause that is inappropriate for a license). Storm is largely unaffected since we use json-simple or Jackson

[GitHub] storm pull request #1796: STORM-2216: prefer JSONValue.parseWithException

GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/1796 STORM-2216: prefer JSONValue.parseWithException You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2216

Re: Some reviews please

I appreciate it.  I know it is a lot of code to go though.  If others are planning to take a look, even a small one, please let me know.  I really do want to get this merged in, because constantly updating it will be a pain, but I also want to give everyone who wants one a chance to find

Re: Some reviews please

I got one review done (Nimbus). Second one will likely have to wait until after the holiday as I will be out. Nice work and thanks for pushing this forward. -Taylor > On Nov 21, 2016, at 10:46 AM, Bobby Evans wrote: > > I have been pushing hard to get some of

[GitHub] storm issue #1744: STORM-1276: line for line translation of nimbus to java

Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1744 @ptgoetz I totally agree on refactoring it. I did a little as I ported it over, but nothing like it needs. I also have done some manual testing. --- If your project is set up for it, you can

[GitHub] storm issue #1744: STORM-1276: line for line translation of nimbus to java

Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1744 Nice (and a lot of ;) ) work! Nimbus.java is a little unwieldy, but I understand why. Maybe it can be a target for some refactoring later on. +1 (Note that I haven't manually tested

[GitHub] storm pull request #1795: STORM-2215: validate blobs are present before subm...

GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/1795 STORM-2215: validate blobs are present before submitting You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2215

[GitHub] storm pull request #1794: STORM-2193: Fix FilterConfiguration parameter orde...

GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/1794 STORM-2193: Fix FilterConfiguration parameter order You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2193

[GitHub] storm pull request #1793: STORM-2214: add in cacheing of the Login

Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1793#discussion_r89154991 --- Diff: storm-core/src/jvm/org/apache/storm/security/auth/kerberos/KerberosSaslTransportPlugin.java --- @@ -50,6 +52,44 @@ public class

[GitHub] storm pull request #1793: STORM-2214: add in cacheing of the Login

GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/1793 STORM-2214: add in cacheing of the Login You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2214 Alternatively you

[GitHub] storm issue #1781: STORM-1369: Add MapState implementation to storm-cassandr...

Github user mkoch1 commented on the issue: https://github.com/apache/storm/pull/1781 Pls note that mutliPut needs an update - will push commit today or tomorrow. --- 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 #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1783 @vesense it is off by default so it would be enabled/tuned on a per topology basis. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user vesense commented on the issue: https://github.com/apache/storm/pull/1783 @revans2 Yes, we can find a balance between high rate of cache hits and full GCs. I'm OK for adding a built-in cache if we set parameters carefully. --- If your project is set up for it, you can

Re: Some reviews please

Forgot to mention too that if you need reviews please let me know.  I am trying to spend more time on storm, even if it is in my spare time. - Bobby On Tuesday, November 22, 2016, 8:36:13 AM CST, Bobby Evans wrote:Everyone has their own priorities and I understand

Re: Some reviews please

Everyone has their own priorities and I understand if you don't have time to get to it all. - Bobby On Tuesday, November 22, 2016, 3:50:50 AM CST, Jungtaek Lim wrote:Amazing works Bobby. I really appreciate your devotion on porting works. I tried to dive into reviewing

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1783 Caffeine sounds like a great alternative to Guava and also seems to address some GC concerns. @vesense I agree storing any large amount of data on heap will impact GC, we do that all the

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user vesense commented on the issue: https://github.com/apache/storm/pull/1783 Overall looks good to me. My major concern is that on-heap caches(like Guava cache, Ehcache) might cause bad GC situations. I didn't use Caffeine in the past, maybe is a candidate. --- If your

[GitHub] storm issue #1789: STORM-2209: Update documents adding new integration for s...

Github user vesense commented on the issue: https://github.com/apache/storm/pull/1789 @revans2 @HeartSaVioR Yes, the new files are copied from external modules README.md. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user ohadedel commented on the issue: https://github.com/apache/storm/pull/1790 Sound reasonable :) The documentation in storm was a bit misleading, made us think that its being implemented the same. "None grouping: This grouping specifies that you don't care

Re: Some reviews please

Amazing works Bobby. I really appreciate your devotion on porting works. I tried to dive into reviewing Nimbus port but changeset is a bit huge so can't even start. I'll see I can spend some time to retry. Btw, personally, I'd like to focus on releasing 1.1.0 for now and revisit others after

[GitHub] storm issue #1719: [STORM-1057] Add throughput metrics to spouts/bolts and d...

Github user wangli1426 commented on the issue: https://github.com/apache/storm/pull/1719 @HeartSaVioR, @revans2, @harshach,@d2r,@unsleepy22 Any comment to this PR, please? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] storm issue #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1790 NoneGrouping just runs Random.nextInt() for deciding 'tasks' (so it will follow uniform distribution), whereas ShuffleGrouping simply does Round-Robin. --- If your project is set up for it, you

[GitHub] storm pull request #1792: STORM-2213 ShellSpout has race condition when Shel...

GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/1792 STORM-2213 ShellSpout has race condition when ShellSpout is being inactive longer than heartbeat timeout * update heartbeat time before turn on flag 'waitingOnSubprocess' I brought

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user ben-manes commented on the issue: https://github.com/apache/storm/pull/1783 I was a co-author of Guava's cache, too. Guava had originally considered soft references an ideal caching scheme, since they offer great concurrency and GC is for memory management. That

[GitHub] storm issue #1789: STORM-2209: Update documents adding new integration for s...

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1789 +1 I think @vesense copied the content from README.md for each of external modules. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1783 Forgot to comment for other side. It looks great. --- 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 #1790: STORM-2210 - remove array shuffle from ShuffleGrouping wh...

Github user ohadedel commented on the issue: https://github.com/apache/storm/pull/1790 Hi, I don't know what storm release policy, but any chance to see a stable version with this bug fix release any time soon (v1.0.3). What we see in our environment is that the host with the