You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by caofangkun <gi...@git.apache.org> on 2015/01/23 10:07:23 UTC
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
GitHub user caofangkun opened a pull request:
https://github.com/apache/storm/pull/394
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
like {nimbus_host_name}:{nimbus_thrift_port}
May add more information like nimbus server version Information ?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/caofangkun/apache-storm storm-534
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/394.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #394
----
commit 6c04e1bea802d28ae92a234ddbaad2c1ed0a6835
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T01:28:46Z
Merge pull request #4 from apache/master
Merger from apache/storm to caofangkun/apache-storm
commit 52cbe5f6459f0923c7eda573df1ad5b792c5ba98
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T08:55:42Z
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
commit 55f95b52f5f49c5572d81d6b653babafa67750c4
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T09:02:21Z
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on a diff in the pull request:
https://github.com/apache/storm/pull/394#discussion_r24060605
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -18,12 +18,17 @@
[java.util Collections])
(:import [java.io FileNotFoundException])
(:import [java.nio.channels Channels WritableByteChannel])
+ (:import [backtype.storm.utils VersionInfo])
(:import [backtype.storm.security.auth ThriftServer ThriftConnectionType ReqContext AuthUtils])
(:use [backtype.storm.scheduler.DefaultScheduler])
(:import [backtype.storm.scheduler INimbus SupervisorDetails WorkerSlot TopologyDetails
Cluster Topologies SchedulerAssignment SchedulerAssignmentImpl DefaultScheduler ExecutorDetails])
+<<<<<<< HEAD
--- End diff --
Sorry.
I have fixed the merge conflicts . Please do try 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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-73770259
I am already planning on storing all the required information and modify the UI as part of nimbus HA, Please see https://issues.apache.org/jira/browse/STORM-654 and comment on it if you think more information should be stores as part of NimbusInfo instance.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72780192
@revans2 @caofangkun Any reason to store storm config in zookeeper. I don't see any benefit of it and whenever user/admin changes storm config these needs to be updated in zookeeper again. I am ok with having storm nimbus,supervisor versions numbers stored in zookeeper but not the config.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-73769844
I have a few questions. First why are we storing the nimbus version as JSON in zookeeper? The way the code is written to store the data in ZK we start off with the raw values. We put them in a thrift object, and then convert them to JSON to store them. To pull it back with the UI it asks nimbus to get the cluster info which results in a zookeeper call to get the JSON version string which is converted to thrift, sent back to the UI which converts it back to JSON again. It feels like there are a lot of unnecessary steps. Can we just store it as thrift in ZK and only convert to JSON for the final UI call?
I would also prefer to have clients pick the nimbus server to use as part of nimbus HA https://github.com/apache/storm/pull/354 it fits better there. Perhaps we really should just wait for the HA code to go in before putting in the version number info. With this change we get half of HA but not all of it.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72782784
@caofangkun Thanks . I think it helps categorizing the configuration into different section. But in a storm deployment same storm.yaml used on all hosts. So I don't think its a good idea make this distinction on the code side as it makes easier to have same config on all hosts for deployment reasons.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun closed the pull request at:
https://github.com/apache/storm/pull/394
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/394#discussion_r24060103
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -18,12 +18,17 @@
[java.util Collections])
(:import [java.io FileNotFoundException])
(:import [java.nio.channels Channels WritableByteChannel])
+ (:import [backtype.storm.utils VersionInfo])
(:import [backtype.storm.security.auth ThriftServer ThriftConnectionType ReqContext AuthUtils])
(:use [backtype.storm.scheduler.DefaultScheduler])
(:import [backtype.storm.scheduler INimbus SupervisorDetails WorkerSlot TopologyDetails
Cluster Topologies SchedulerAssignment SchedulerAssignmentImpl DefaultScheduler ExecutorDetails])
+<<<<<<< HEAD
--- End diff --
some merge conflicts made into PR. This won't be able to compile.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-73822455
Let's close it for now.More information will be discussed on [STORM-654](https://issues.apache.org/jira/browse/STORM-654)
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72776924
@revans2
it might be good to:
1: the configuration should be paginated
2: we'd better supply three types of configuration and they may different from each other
2.1 nimbus server configuration --- show up all config information and in numbus.html
2.2 supervisor server configuration( different type of machine may have different configuration ) -- show only supervisor local config information and in supervisor.html
2.3 topology configuration -- show only the topology customized configuration and in topology.html
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72770805
@revans2
Yes, not only nimbus server version numbers , but also supervisors and ui should show up version numbers on UI .
In face , it may be better add supervsior version numbers in issue：
https://issues.apache.org/jira/browse/STORM-619
“add supervisor page to show workers running detail informations ”
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72693402
@caofangkun If you want to reopen this and just add in the nimbus version information I would support 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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-71423079
@revans2
Could you please have a review on 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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72781055
@harshach
Sorry for my unclear explanation.I am not ok with storing config in zk ether.
I am just thinking storm config shoud be classfied to nimbus/supervisor/topology three types, and be showed in UI separately.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun closed the pull request at:
https://github.com/apache/storm/pull/394
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/394#issuecomment-72693203
Sorry this took me so long to respond to. I am kind of swamped :). I do like the idea of being able to access the version numbers for nimbus, but also for the supervisors. This becomes especially important for HA and rolling upgrades of large clusters. It is good to be able to verify that all of the nodes are on the versions you expect them to be on. I am OK with it being stored in ZK, so long as it is not written/read very frequently.
Also if you think the configuration takes up too much room please take a look at https://github.com/apache/storm/pull/328 which makes the config paginated.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...
Posted by caofangkun <gi...@git.apache.org>.
GitHub user caofangkun reopened a pull request:
https://github.com/apache/storm/pull/394
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
like {nimbus_host_name}:{nimbus_thrift_port}
May add more information like nimbus server version Information ?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/caofangkun/apache-storm storm-534
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/394.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #394
----
commit 6c04e1bea802d28ae92a234ddbaad2c1ed0a6835
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T01:28:46Z
Merge pull request #4 from apache/master
Merger from apache/storm to caofangkun/apache-storm
commit 52cbe5f6459f0923c7eda573df1ad5b792c5ba98
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T08:55:42Z
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
commit 55f95b52f5f49c5572d81d6b653babafa67750c4
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-23T09:02:21Z
STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus
commit 3a2ace166ef13b99357089bb5e9093f8494b3c1b
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-26T03:54:41Z
STORM-534:Add ServerInfo.java to storm nimbus server info, NimbusClient fetch nimbus info from zk
commit fd4a7e825a9d980ef466edfb703fd4ee5885da80
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-26T04:21:51Z
STORM-534:Add ServerInfo.java to storm nimbus server info, NimbusClient fetch nimbus info from zk
commit a8d08c9c098208f693bdc5016d4e3f41eab09240
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-26T06:07:50Z
STORM-534:modify UI add nimbus.html
commit 0fa6ae40d9d5bb370f322bc9ca0ef6f37f7b6bad
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-26T07:04:28Z
STORM-534:ServerInfo should contain nimbus version info
commit d605f186c2261fe4a1c7db449d5bafd90875d4a8
Author: caofangkun <ca...@gmail.com>
Date: 2015-01-26T07:39:46Z
STORM-534:modify ClusterSummary.java add License
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---