search

yahoo / storm-yarn

Storm-yarn enables Storm clusters to be deployed into machines managed by Hadoop YARN.
Other
417 stars 161 forks source link

Supervisor Resource capability configuration #64

Open roshanp opened 10 years ago

roshanp commented 10 years ago

Includes:

  1. Ability to specify the supervisor resource (memory) through configuration, and finding the number of vcores based on the slots of the supervisor
  2. Tracking the number of supervisors already running, so that only one supervisor can run per host.
  3. Added a gitignore file to remove any files that should not be added to git

Notes:

  1. I tried to set up the ApplicationMasterTrackingUrl to be the Storm UI page, but I guess the yarn proxy cannot handle the CSS correctly. It does not render the page correctly, so I kept that out of the pull request
  2. I think that the best solution for the supervisor tracking would be to move to using the AMRMClientAsync. I had gone down this route but it required major modifications of code that probably wasn't necessary for what I wanted to do.
  3. I'm not sure how to write a test for this, any help would be appreciated.

Thanks

revans2 commented 10 years ago

I did a quick look through the code and for the most part it looks good. I had a few comments but nothing too terribly serious. Before I can merge anything in I am going to need you to read over https://github.com/yahoo/storm-yarn/wiki/Contributing and send a signed copy of the CLA to the email address listed there.

We need to come up with a much better integration testing for storm-yarn, probably something using the Hadoop mini cluster, but for now I would look into doing some unit tests. I kind of feel bad asking you to unit test code that doesn't have any real unit tests to start out with. If you could look at finding a good point to mock out the RM interactions that would be great. That might take moving to the AsyncClient for that. If it is too much of a pain to come up with if you could make sure you run some manual tests and show the result that would probably be enough.

roshanp commented 10 years ago

Yeah I can do that, I'll try to get the CLA back to you soon. And get the modifications to the pull request in quickly.

As per mocking/unit testing, do you have any preferences? I usually like JMockit, but am open to anything.

revans2 commented 10 years ago

For mocking storm currently uses Mockito 1.9.5. We hope to keep the door open so that this project can merge into storm proper at some point relatively soon. So if you could use that it would be great.