STORM-1129: Use topology name instead of id in UI calls.

[priyank5485]

Note that all-topologies-summary has been used to get topology id from a topology name. That involves a few calls to zookeeper which is not ideal. However, UI does not seem to take any significant performance hit. If needed we can handle it possibly using one of the options below. Since its a separate performance issue we can handle it in a separate JIRA.

1. Have nimbus thrift server cache summary for topologies so it does not hit zookeeper every time we try to get topology id from name.
2. Update nimbus thrift api with a method that takes options and use that to do only the minimal necessary interaction with zookeeper for a given option.

[jerrypeng]

Topology names are not unique. Users can submit topologies with the same name, but the their auto generated topology ids will be unique. I am not sure this is the right way to go about this

[revans2]

@jerrypeng they can submit topologies with the same name, but not at the same time. You can only have one "foo" topology running at any point in time.

[priyank5485]

@jerrypeng We have a check and it throws the following exception.

Exception in thread "main" java.lang.RuntimeException: Topology with name wordcount already exists on cluster
at backtype.storm.StormSubmitter.submitTopologyAs(StormSubmitter.java:231)
at backtype.storm.StormSubmitter.submitTopology(StormSubmitter.java:275)
at backtype.storm.StormSubmitter.submitTopologyWithProgressBar(StormSubmitter.java:311)
at backtype.storm.StormSubmitter.submitTopologyWithProgressBar(StormSubmitter.java:292)
at storm.starter.WordCountTopology.main(WordCountTopology.java:94)

@revans2 Thanks for chiming in.

[harshach]

@jerrypeng topology names are unique

[Parth-Brahmbhatt]

+1.

[jerrypeng]

thanks for the explanation and clarification everybody

[revans2]

This is a non-backwards compatible change to the REST API and I really would prefer to maintain compatibility if at all possible. It seems fairly simple to make the REST API work with the name or the id. We call get-id-from-name all over the place, it seems fairly simple to check to see if there is a topology with that as the ID, if not check to see if there is a topology with it as the name.

[revans2]

We have to populate the context before ever talking to nimbus. get-id-from-name talks to nimbus and could potentially authenticate with the wrong user to nimbus. This needs to be fixed everywhere.

[revans2]

@priyank5485 for the most part this change looks good to me, but there are a few serious bugs/incompatibilities that need to be fixed before this can go in.

[Parth-Brahmbhatt]

I agree with @revans2 , lets maintain backward compatibility.

[priyank5485]

@revans2@Parth-Brahmbhatt@jerrypeng@harshach Thanks all for the feedback. Other than the changes here there are some commits to apache/master after creation of this PR which added some more apis in UI using topology id making this PR unmergeable. I will sort all of that out and update the PR.

[revans2]

@priyank5485 thanks for doing this. I think it is going to make the UI much more usable. Essentially giving us perma-links to a topology.

[wuchong]

+1

In Alibaba, we implement our monitor system using topology name instead of id. And it is very useful , as users often resubmit topology several times , and they do not need to find the new topology page link.

But we should maintain backward compatibility. maybe we can deal with topology-id arg in function get-id-from-name

[harshach]

@priyank5485 sorry for the delay. can you please upmerge this patch and open up another PR for 1.x-branch as well.

[priyank5485]

@harshach@revans2 I have addressed the comments and raised a PR #1277 against the 1.x branch as I could not test my branch off of master due to another issue. Can you please review the PR against 1.x branch? I will port the same to master by updating this PR.