Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1081
---
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 is
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-188328586
+1 looks god to me. Did some manual testing and things look good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53958459
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-188041296
@revans2 Thank you again. 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 as
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53888948
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53886541
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-187846619
OK I made it all of the way through. Things look fairly good, but there
are a few places that need to be updated/fixed.
---
If your project is set up for it, you can
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53832190
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53831586
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53831437
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53830934
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53830685
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,270 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53829764
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,687 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53827548
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,687 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53826300
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/IStormClusterState.java ---
@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53816996
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/IStormClusterState.java ---
@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53816724
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/IStormClusterState.java ---
@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53814415
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,687 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53811031
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,687 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53809357
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java ---
@@ -0,0 +1,212 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53809374
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java ---
@@ -0,0 +1,212 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53809385
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java ---
@@ -0,0 +1,212 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53809224
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java ---
@@ -0,0 +1,212 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53807669
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53800916
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53560661
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53472251
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-186239627
@hustfxj OK I will try to get through the entire thing today. So we can
get it in soon.
---
If your project is set up for it, you can reply to this email and have
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-186100094
@revans2 thanks for your review, I have addressed your comments and
removed any clojure internals.
I want to keep this all together. I will keep merging with the
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53417863
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -1183,7 +1184,8 @@
(.readBlobTo blob-store
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-185885496
This is a really big pull request and things are changing all of the time
with others merging things in. Perhaps we should break this up into smaller
pieces. Like
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53369187
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,687 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53368693
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/IStateStorage.java ---
@@ -18,12 +18,14 @@
package org.apache.storm.cluster;
import
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53368561
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/IStateStorage.java ---
@@ -18,12 +18,14 @@
package org.apache.storm.cluster;
import
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53368510
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53368387
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53368270
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53366988
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53367005
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53366894
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r53366742
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,282 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-184515933
We would also like @knusbaum to review this one.
---
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 hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1081#issuecomment-184514177
@revans2 @abhishekagarwal87 can you help review these commits ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52964766
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52906298
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -587,7 +586,12 @@
(defn update-heartbeats! [nimbus storm-id all-executors
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52838625
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52836778
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterState.java ---
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52836589
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52837081
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52838686
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -587,7 +586,12 @@
(defn update-heartbeats! [nimbus storm-id
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r51996432
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52009625
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52011728
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StateStorageFactory.java ---
@@ -21,8 +21,8 @@
import java.util.List;
import
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52010895
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterStateImpl.java ---
@@ -0,0 +1,664 @@
+/**
+ * Licensed to the Apache
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52012046
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ZKStateStorage.java
---
@@ -0,0 +1,271 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52011598
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/ClusterUtils.java ---
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52012383
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterState.java ---
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52013815
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterState.java ---
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52015170
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormClusterState.java ---
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1081#discussion_r52012273
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/StateStorage.java ---
@@ -38,16 +41,16 @@
* may or may not cause a collision in "/path".
GitHub user hustfxj opened a pull request:
https://github.com/apache/storm/pull/1081
[STORM-1273] port backtype.storm.cluster to java
1. port cluster.clj to
ClusterUtils/StormClusterState/StateStorage/ZKStateStorage/ZKStateStorageFactory
2. Update all the callings to cluster
GitHub user hustfxj opened a pull request:
https://github.com/apache/storm/pull/1071
[STORM-1273] port backtype.storm.cluster to java
Sorry. Some tests donât pass, and I will check and fix it. I now check
in code, because I want to get some guidance before the Chinese New Year.
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51788562
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormZkClusterState.java ---
@@ -0,0 +1,683 @@
+/**
+ * Licensed to the Apache
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1071#issuecomment-179294019
@hustfxj there are some major changes that need to be made to the way you
setup your class hierarchy and the naming of those classes. I stopped looking
at the details
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51727157
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51727250
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1071#issuecomment-179258530
The travis CI tests failed with
```
13881 [timer] ERROR o.a.s.d.nimbus - Error when processing event
java.lang.NullPointerException
at
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51728819
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51725134
--- Diff: storm-core/src/clj/org/apache/storm/daemon/common.clj ---
@@ -13,14 +13,16 @@
;; WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51726152
--- Diff:
storm-core/src/clj/org/apache/storm/pacemaker/pacemaker_state_factory.clj ---
@@ -47,7 +43,7 @@
(reify
ClusterState
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51727452
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51735697
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/DistributedClusterState.java ---
@@ -0,0 +1,268 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51728055
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51726849
--- Diff: storm-core/src/jvm/org/apache/storm/callback/Callback.java ---
@@ -18,6 +18,9 @@
package org.apache.storm.callback;
+import
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51728386
--- Diff: storm-core/src/jvm/org/apache/storm/cluster/Cluster.java ---
@@ -0,0 +1,236 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF)
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1071#issuecomment-179569559
@revans2 @abhishekagarwal87 I will close this PR to update the codes
according to your comments.
---
If your project is set up for it, you can reply to this email and
Github user hustfxj commented on the pull request:
https://github.com/apache/storm/pull/1071#issuecomment-179558002
@revans2 @abhishekagarwal87 Thank you for some advice.
---
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 hustfxj closed the pull request at:
https://github.com/apache/storm/pull/1071
---
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 is
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51741856
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/DistributedClusterState.java ---
@@ -0,0 +1,268 @@
+/**
+ * Licensed to the Apache
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51741578
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/DistributedClusterState.java ---
@@ -0,0 +1,268 @@
+/**
+ * Licensed to the Apache
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r5176
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/StormZkClusterState.java ---
@@ -0,0 +1,683 @@
+/**
+ * Licensed to the Apache
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51741222
--- Diff:
storm-core/src/jvm/org/apache/storm/cluster/DistributedClusterState.java ---
@@ -0,0 +1,268 @@
+/**
+ * Licensed to the Apache
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51755050
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -275,11 +274,11 @@
(defn do-rebalance [nimbus storm-id status storm-base]
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1071#discussion_r51754553
--- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
@@ -174,11 +173,11 @@
:authorization-handler (mk-authorization-handler (conf
84 matches
Mail list logo
