support full set of storm commands

[clockfly]

Changes: Support more storm commands in storm-yarn shell.

1. Add commands. storm-yarn submit, storm-yarn kill, storm-yarn list, storm-yarn rebalance. There is header description for the usage.
2. Use more robust way to get storm home under a zip. (No need storm version anymore)
3. fix the JAVA home. For AppMaster, use System.getProperty("java.home"). For supervisor, use env JAVA_HOME.
4. Allow to pass storm conf when building storm commands. For example, to support command: storm-yarn list. We need: a) download the updated storm-conf from AppMaster to a temporary folder. b) Use the storm-conf in the temporary folder to construct command like: storm list -Dstorm.conf.file=temporary_storm_conf_file.

[revans2]

It is going to take me a while to go through the code. I have glanced through it and I am curious why we are calling into the storm command for submit, kill, list, and rebalance? If we want to pass through to storm itself, perhaps it would be preferable to just have a pass through command and by default if storm-yarn cannot find the command requested it just does a passthrough, looking for the appid on the command line, similar to how storm looks for the -c commands on the command line.

[clockfly]

Hi Bobby,

I think you means something like this: storm-yarn -appId xxx -c "storm command".

I think both are good approach, yours may be less code. There are several reasons why we choose current approach instead of this.

1. Since we need to download a storm conf, and need to change the storm java classpath setting to include the new configuration path, there is no way to do this for "storm-yarn -appId xxx -c "storm command".". As we need to construct the underlying java command ourself.
2. Current approach will start storm command in the same process of submitter instead of starting a shell command, More easy for us to track exceptions and failures. And we can examine the argument for verification there.
3. storm-yarn provides full help context for user to use. No need to refer to storm document.Easy to use.

[revans2]

OK maybe you didn't understand exactly what I wanted. I would like something like

storm-yarn jar ./myCode.jar -c foo=bar -appId xxx

and

storm-yarn kill -appId xxx -w 0 bar

Where storm-yarn would see jar or kill and know that they should be handled by the storm executable. It would then download the config file, and place it in ~/.storm/appid.yaml, afterwards it would run (Through ProcessBuilder not the shell)

storm jar ./myCode.jar -c foo=bar --config appid.yaml

or

storm kill -w 0 bar --config appid.yaml

I believe that this is more maintainable long term. It would also allow for -c arguments to be processed correctly.

Looking at the code for the storm command I can see that it will not work unless we also ensure that a storm.yaml exists in ~/.storm too. Perhaps the route we want to take is to fix the storm command so that it can take a full path to a config file, and it does not have to be on the classpath.

If you still think that your current code is the best I am fine with that so long as we truly mimic storm behavior and

1. parse -c options in a compatible way as the storm python script does.
2. include java.library.path from the configs on the command line when running jar
3. rename submit to jar so it is compatible with the normal storm. (I can be convinced on this, as jar is possibly confusing here, but for consistency it seems preferable)

[clockfly]

I created a bug for storm about the config path, https://issues.apache.org/jira/browse/STORM-188

I know your point now, basically, we want to parse the configs and MainClass, and then pass everything forward to storm, so that it is more easy for us to maintain the code.

[revans2]

Are you planning on putting up a patch for STORM-188, shortly? If not I'll be happy to.

[clockfly]

Yes, I already started working on it. It will be a good practice for me:)

On Thu, Jan 2, 2014 at 11:24 PM, Robert (Bobby) Evans < notifications@github.com> wrote:

shortly? If not I'll be happy to.

[clockfly]

Patch submited for https://issues.apache.org/jira/browse/STORM-188

[clockfly]

Patch updated.

It will requires STORM to patch STORM-188 to make this works.