Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
All outstanding review comments should be done now. This and the 1.x port
at #1868 should be ready for a final pass and hopefully being merged in.
---
If your project is set up for it, you can
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1868
This is the 1.x version of #1808
---
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
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/1868
STORM-2225: change spout config to be simpler. (1.x)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-2225-1.x
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1862#discussion_r95174977
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -63,6 +63,18 @@
private static final String PREFIX = "disr
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1862#discussion_r95169637
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -63,6 +63,18 @@
private static final String PREFIX = "disr
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
I think I have addressed all of the review comments so far. I will try to
get my 1.x version of the patch up shortly.
---
If your project is set up for it, you can reply to this email and have
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1853
I am also OK with reverting STORM-2216 if it is causing a lot of issues.
---
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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1853
+1 seems fine 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
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1785#discussion_r94972777
--- Diff: docs/IConfigLoader.md ---
@@ -0,0 +1,58 @@
+---
+title: IConfigLoader
+layout: documentation
+documentation: true
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1767
@sathyafmt sorry I have taken so long to respond December was a really
crazy month for me. From STORM-2194 I see that the SocketTimeoutException goes
through the code being changed. The RMI code
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/1862
STORM-2278: Allow max number of disruptor queue flusher threads to be
configurable
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1827
+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 user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1839#discussion_r94782513
--- Diff: storm-core/test/jvm/org/apache/storm/MessagingTest.java ---
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1839#discussion_r94783130
--- Diff: storm-core/test/jvm/org/apache/storm/MessagingTest.java ---
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1839#discussion_r94783485
--- Diff: storm-core/test/jvm/org/apache/storm/MessagingTest.java ---
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1839#discussion_r94784021
--- Diff: storm-core/test/jvm/org/apache/storm/MessagingTest.java ---
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1839#discussion_r94782323
--- Diff: storm-core/test/jvm/org/apache/storm/MessagingTest.java ---
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1852
+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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1857
+1 - Thanks for fixing 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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1860
+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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1859
+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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1858
+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 user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94649177
--- Diff:
storm-core/test/jvm/org/apache/storm/scheduler/blacklist/TestBlacklistScheduler.java
---
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94665415
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94661600
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94675511
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/CircularBuffer.java ---
@@ -0,0 +1,174 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94665531
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94678615
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/strategies/DefaultBlacklistStrategy.java
---
@@ -0,0 +1,149 @@
+/**
+ * Licensed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94677722
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/CircularBuffer.java ---
@@ -0,0 +1,174 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94675703
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/CircularBuffer.java ---
@@ -0,0 +1,174 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94668925
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94668597
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94677987
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/CircularBuffer.java ---
@@ -0,0 +1,174 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94678822
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/strategies/IBlacklistStrategy.java
---
@@ -0,0 +1,37 @@
+/**
+ * Licensed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94668464
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,252 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94678786
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/strategies/IBlacklistStrategy.java
---
@@ -0,0 +1,37 @@
+/**
+ * Licensed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94665652
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java
---
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1674#discussion_r94675699
--- Diff:
storm-core/src/jvm/org/apache/storm/scheduler/blacklist/CircularBuffer.java ---
@@ -0,0 +1,174 @@
+/**
+ * Licensed to the Apache Software
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/1854
STORM-2272: don't leak simulated time
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-2272
Alternatively you can
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1793
I also wanted to say we have been running with this in prod for a while now.
---
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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1793
@harshach the issue is that when there are lots of supervisors the load
placed on the KDC is a lot higher than it is for a similar number of Hadoop
nodes. Each new Login will contact the KDC
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
@srdo as part of backporting this to 1.x I am going to need to make a
change to not use Function directly, because it is only in java 8. So to
maintain compatibility between 1.x and 2.x I am going
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
@srdo I think I addressed all of your review comments.
---
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 user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1808#discussion_r91618249
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -0,0 +1,91 @@
+/**
+ * Licensed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1808#discussion_r91613603
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -0,0 +1,194 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1808#discussion_r91592134
--- Diff: docs/storm-kafka-client.md ---
@@ -1,90 +1,222 @@
-#Storm Kafka Spout with New Kafka Consumer API
+#Storm Kafka integration using the kafka
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1808#discussion_r91592178
--- Diff: docs/storm-kafka-client.md ---
@@ -1,90 +1,222 @@
-#Storm Kafka Spout with New Kafka Consumer API
+#Storm Kafka integration using the kafka
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1818
+1 pending travis (and the waiting period)
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1696
This looks good to me. Now that I have gone through the kafka spout code
for my other pull request I am confident in giving this a +1.
---
If your project is set up for it, you can reply
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1696#discussion_r91569248
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/SerializableDeserializer.java
---
@@ -0,0 +1,25
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
@srdo for me backwards compatibility for 1.x is more a question of
violating out versioning than anything else.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
@ptgoetz I have updated the packaging to have a separate directory for the
DRPC server dependencies. I have run manual tests and everything works. The
big difference is that {{apache-storm-2.0.0
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
One more option, still ugly, but with a lot less impact. I can do
something similar to Hadoop. They use several different invocations of the
maven assembly plugin into directories (predates
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
So shading is not really an option for jersey.
I was able to split the DRPC server off into its own package with tests,
but packaging it up with the assembly plugin is proving to be difficult
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1764
@HeartSaVioR I plan on doing that.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
Shading Jersey is becoming rather difficult (lots of dependencies including
aop and dependency injection. Splitting the DPRC server off into it's own
location seems much simpler and less error
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1764
@ptgoetz @jerrypeng
I made a few changes to thing. I fixed the race condition and I addressed
the review comments, but I also put in some optimizations to storm submitter.
We were
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1767
We should be able to fix this with code like.
```
(if (or
(exception-cause? InterruptedException error)
(and
(exception-cause
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1767
@chawco
Okay so I understand the issue better now. SocketTimeoutException is a
subclass of InterruptedIOException.
https://docs.oracle.com/javase/7/docs/api/java/net
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
@ptgoetz if this looks good as is I will look into shading Jersey.
---
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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1764
Just so we don't miss the comment from @jerrypeng
> Couldn't a wrong ordering of events happen since we are locking when
calculating a scheduling then unlocking and then lock
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1764#discussion_r90542812
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -1008,23 +1008,24 @@
(reset! (:id->worker-resources nim
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1764#discussion_r90538639
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -1008,23 +1008,24 @@
(reset! (:id->worker-resources nim
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
Rebased again
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1786
Rebased now that java nimbus is in.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
I am merging I am still happy to make changes on follow on JIRA.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1795
@HeartSaVioR I rebased and addressed the review comments, but I want to
merge in #1744 first and then I will update this one again.
---
If your project is set up for it, you can reply to this email
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1786
@ptgoetz I added in javadocs for most of the new classes
---
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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1786
@ptgoetz I'll look at adding in some javadocs. In general I tried to add
javadocs where there were clojure doc strings, but there were not too many of
them either.
---
If your project is set up
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1765#discussion_r90465747
--- Diff:
examples/storm-starter/src/jvm/org/apache/storm/starter/WordCountTopology.java
---
@@ -90,7 +90,9 @@ public static void main(String[] args) throws
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
@erikdw I addressed your nits. I appreciate the nits because we don't have
a coding standard yet.
If we are just down to nits I really would like to get this in.
---
If your project
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1744#discussion_r90464031
--- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -0,0 +1,3729 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1808
I updated the docs and the examples. This is a non-backwards compatible
change from the 1.x release. I have some code (not included here) that can
maintain most compatibility if we really want
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/1808
STORM-1997: copy state/bolt from storm-kafka to storm-kafka-client
STORM-2228: removed ability to request a single topic go to multiple streams
You can merge this pull request into a Git
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1783
I guess guava is OK for 1.x. I would prefer to see it shaded if we do go
with Guava, but I am only a -0 if it is not shaded.
---
If your project is set up for it, you can reply to this email
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1783
+1 the code looks fine 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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1783
@HeartSaVioR I also am find with Caffeine and even Caffeine on 1.x
(assuming it is off by default)
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1744#discussion_r89806015
--- Diff: storm-core/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1672,11 +1672,12 @@ public void uncaughtException(Thread thread,
Throwable thrown
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1744#discussion_r89805725
--- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
@@ -963,11 +963,11 @@
"uptimeSeconds" uptime
"host"
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1744#discussion_r89789081
--- Diff: storm-core/src/jvm/org/apache/storm/security/auth/AuthUtils.java
---
@@ -167,15 +175,21 @@ public static IPrincipalToLocal
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1744#discussion_r89782717
--- Diff: storm-core/src/clj/org/apache/storm/testing.clj ---
@@ -242,11 +297,12 @@
(.stop (:nimbus-thrift-server cluster-map
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
Rebased the code
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1800
Will rebase soon.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1789
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
Github user revans2 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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1792
+1 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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1797
+1, because this is from #1790 that was already approved I will not wait
the full 24 hours.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
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
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 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 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 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 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 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 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
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1787
+1 looks god to me, although I am not an expert on azure, so I assume you
have tested the new code and it works.
---
If your project is set up for it, you can reply to this email and have your
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1790
@kevpeek No need, like I said it is a clean cherry pick. We just have a
rule that we need to merge code into master/newer versions before it goes into
older versions.
We also have a 24
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1790
The only other comment that I would have is that it would be nice to have a
pull request against master instead of 1.x, but that is minor as it is a clean
cherry-pick
---
If your project is set up
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1790#discussion_r88975651
--- Diff: storm-core/src/jvm/org/apache/storm/grouping/ShuffleGrouping.java
---
@@ -17,17 +17,13 @@
*/
package org.apache.storm.grouping
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1790
I keep beating my head against this to think if there are issues with this,
but I honestly don't see any. Long term I would like to see more of a
combining of shuffle grouping and the shuffle
1201 - 1300 of 3470 matches
Mail list logo
