STORM-534:Store Nimbus Server Information in zookeeper path {storm.zookeeper.root}/nimbus

[caofangkun]

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 ?

[caofangkun]

@revans2
Could you please have a review on this?

[revans2]

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 #328 which makes the config paginated.

[revans2]

@caofangkun If you want to reopen this and just add in the nimbus version information I would support that.

[caofangkun]

@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 ”

[caofangkun]

@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

[harshach]

@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.

[caofangkun]

@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.

[harshach]

some merge conflicts made into PR. This won't be able to compile.

[caofangkun]

Sorry.
I have fixed the merge conflicts . Please do try again.

[harshach]

@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.

[revans2]

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 #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.

[Parth-Brahmbhatt]

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.

[caofangkun]

Let's close it for now.More information will be discussed on STORM-654