Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161277889
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161277608
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161277857
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161303474
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2492
@dujiashu I meant could you close it on your end? I think only the author
can close PRs at this time.
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2510
---
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161301526
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2512
+1. Could you squash to one commit before I merge this @erikdw?
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161306615
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161291952
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161292783
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2511
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2511
+1. Thanks @erikdw, merged to master.
---
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2511
@revans2 & @srdo : thx for the quick review-and-merge!
---
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2512
@srdo : I assume for cleanliness / ease of reading purposes? I am a
proponent of squashing commits when there are tons of slight tweaks being made
to the same code. I do like separate commits for
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161357014
--- Diff: storm-client/src/jvm/org/apache/storm/StormTimer.java ---
@@ -193,6 +210,24 @@ public void run() {
});
}
+
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161358582
--- Diff: storm-client/src/jvm/org/apache/storm/executor/Executor.java ---
@@ -196,19 +197,21 @@ public static Executor mkExecutor(WorkerState
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161361184
--- Diff: storm-client/src/jvm/org/apache/storm/tuple/TupleImpl.java ---
@@ -24,50 +24,46 @@
import org.apache.storm.task.GeneralTopologyContext;
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161356532
--- Diff: docs/Performance.md ---
@@ -0,0 +1,132 @@
+---
--- End diff --
I am putting a link to this doc from Concepts.md. If you have
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161358387
--- Diff:
storm-client/src/jvm/org/apache/storm/daemon/metrics/SpoutThrottlingMetrics.java
---
@@ -22,24 +22,25 @@
public class
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2510
@srdo : makes sense, thanks for the info
---
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161361250
--- Diff: storm-client/src/jvm/org/apache/storm/utils/ObjectReader.java ---
@@ -76,6 +76,32 @@ public static Integer getInt(Object o, Integer
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2203
@revans2 Just to be clear, which branch is used from your performance test:
current or my updated branch? Just be sure that things are OK with current
branch or need to adopt.
---
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2433
@HeartSaVioR :
> By the way, I was not aware of the discussion in storm-mesos, so don't
know which works should be done in Storm side, and how these are coupled with
"this issue". Maybe
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2433
@erikdw
OK, sorry @JessicaLHartog for misunderstanding, and thanks for noticing us
to not making us unintentionally breaking the thing again.
> Is it possible to specify this
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161356664
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161358493
--- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
---
@@ -155,134 +150,159 @@ public void start() throws Exception {
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161358991
--- Diff: storm-client/src/jvm/org/apache/storm/executor/Executor.java ---
@@ -225,51 +228,62 @@ private static String
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2433
@danny0405
TODO 2: That's same as my understanding. This patch becomes depending on
https://issues.apache.org/jira/browse/STORM-2898. If you are familiar with
security please take a look at
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161359446
--- Diff: storm-client/src/jvm/org/apache/storm/executor/TupleInfo.java ---
@@ -23,7 +23,7 @@
import java.io.Serializable;
import
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161359459
--- Diff:
storm-client/src/jvm/org/apache/storm/serialization/SerializationFactory.java
---
@@ -20,6 +20,7 @@
import org.apache.storm.Config;
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2497
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2467
---
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2433
@HeartSaVioR : really @revans2 noticed the change's implications for
storm-on-mesos, so he should get the credit. :-) He's rightly suggested that
we create some tests to codify everything that might
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2514
Merged it without 24hr since it is a change regarding documentation.
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2514
---
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/2502#discussion_r161358833
--- Diff: storm-client/src/jvm/org/apache/storm/executor/Executor.java ---
@@ -225,51 +228,62 @@ private static String
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2433
@JessicaLHartog
Thanks for noticing! By the way, I was not aware of the discussion in
storm-mesos, so don't know which works should be done in Storm side, and how
these are coupled with
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2505
---
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161324136
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2512
@srdo : I actually spent extra effort to have 2 commits. :-) This was to
ensure that there is no hidden changes amidst the renaming of the file (I am a
strong proponent of trivial-to-read
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2512
@erikdw Yes, I think for cleanliness. Your reason for having two commits
makes sense to me. I'll give Hugo a chance to respond, then I'll merge this in
a day or two.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2510
@erikdw I don't think there's really a convention. I went by the versions
marked affected in the issue. Since it's a bugfix and cherry-picked (mostly)
cleanly, it seemed to make sense to put it in all
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2514
STORM-2899: Replace README contributors list with a link to GitHub's â¦
â¦contributors panel
Please see https://issues.apache.org/jira/browse/STORM-2899 and the linked
mailing list thread.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r161326958
--- Diff:
storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java ---
@@ -0,0 +1,181 @@
+/**
+ * Licensed to the Apache Software
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2510
@srdo & @revans2 : thx for the quick review-and-merge here! Follow-up
question: since this has been present since the Flux stuff was introduced, it
impacts all active branches. Should I backport
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2510
It was my impression that 1.0.x was the oldest active branch. Is this not
the case?
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2512
@erikdw Makes sense. I brought it up because I believe at least @hmcl
(can't recall if he's the only one) has been requesting that PRs are squashed
before merging. Could you explain why @hmcl?
---
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/2510
@srdo : that is my understanding as well. Sorry, I didn't realize you
backported this to all of the other active branches already, is that done by
default for trivial changes like this?
---
49 matches
Mail list logo
