Github user redsanket commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-202581682
+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
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-201345276
+1
Metrics look 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
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-201114123
@redsanket @knusbaum @revans2 can you look it 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
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-200497814
By using gauges we're re-implementing stuff that coda hale does, namely
windowed stats. Just take a look at DRPCServer.java, like I mentioned.
You can use
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-200183338
@knusbaum I have remove the JMX, and use Gauge report the stats. I think
Gauge is enough. can you look at it again?
---
If your project is set up for it, you can
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-199555267
@knusbaum I also prefer to go to Coda Hale for the stats before, It means
we will drop the JMX Mbean. But I agree with you totaly.
---
If your project is set up for
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56844493
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,374 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-199345559
I'd prefer to go to Coda Hale for the stats like the rest of the daemons
do. Take a look at DrpcServer.java for an example.
---
If your project is set up for it, you
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56557159
--- Diff: storm-core/src/clj/org/apache/storm/pacemaker/pacemaker.clj ---
@@ -1,242 +0,0 @@
-;; Licensed to the Apache Software Foundation (ASF) under
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56659070
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,372 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-197645359
@redsanket @abhishekagarwal87 I have addressed your comments, thank you.
@redsanket I also am curious where is the jmx related code implemented. But I
don't found, so
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56658785
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,372 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user redsanket commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-197508158
Most of the changes look good, just a question about jmx reporter
implementation
---
If your project is set up for it, you can reply to this email and have your
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56381761
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-197701113
LGTM
---
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 user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56383389
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56382441
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56383870
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56366424
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -1353,8 +1353,8 @@
(str "Failed to submit topology. Topology requests
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-198351433
@redsanket @knusbaum thank you. I have addressed your comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56381799
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-198028339
@hustfxj @redsanket
The JMX stuff was in a clojure library `(:require [clojure.java.jmx :as
jmx])` It should probably be moved over to Coda Hale since that's what
Github user redsanket commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-198018090
@hustfxj I guess it is more like a plugin to get metrics. I presume the
code should be implemented. @knusbaum might be right to answer it.
---
If your project is set
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r56387356
--- Diff: storm-core/src/clj/org/apache/storm/pacemaker/pacemaker.clj ---
@@ -1,242 +0,0 @@
-;; Licensed to the Apache Software Foundation (ASF) under
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-196393789
Every can help me review this PR. In my perspective , I hope we can
accelerate the first phrase job.
---
If your project is set up for it, you can reply to this email
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r55520029
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,246 @@
+/**
+ * Licensed to the Apache Software
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-194280649
@abhishekagarwal87 Thank you. But I hope only it is the same as before for
the details.
---
If your project is set up for it, you can reply to this email and have
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r55484666
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,248 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r55484087
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,248 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1195#discussion_r55483738
--- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/Pacemaker.java ---
@@ -0,0 +1,248 @@
+/**
+ * Licensed to the Apache Software
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1195#issuecomment-194168476
when we start up nimbus before starting up pacemaker, the nimbus will die.
Because nimbus can't read the workers' heartbeats by "heartbeat-storms". In my
opinion, it
GitHub user hustfxj opened a pull request:
https://github.com/apache/storm/pull/1195
[STORM-1611] port org.apache.storm.pacemaker.pacemaker to java
1 port pacemaker_test to java;
2 Update all the callings to cluster;
3 remove pacemaker.register about jmx;
4 fix the bug
32 matches
Mail list logo
