-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STORM-150: Replace jar distribution strategy with bittorrent #71
Conversation
Conflicts: conf/defaults.yaml storm-core/project.clj storm-core/src/clj/backtype/storm/config.clj storm-core/src/jvm/backtype/storm/Config.java
public abstract class BasePeer { | ||
private static final Logger LOG = LoggerFactory.getLogger(BasePeer.class); | ||
|
||
protected HashMap<String, Client> clients = new HashMap<String, Client>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not very clear here what the key is to the clients. Could we either have a comment that this is topology ID, or a clearer name.
For the most part things look OK. I have two concerns. The first one is that the ttorrent library is very noisy. There is a line output on both nimbus and the supervisors every 3 seconds for every topology. We can fix that with the correct logback config, but I would like that to be the default. My other concern is with security. I know we have not put security into storm yet, but I have patches that I would like to push in with it. If we only support bit-torrent for distribution, then I have to figure out how to secure it, which is going to slow things down a lot. Could we do something similar to what we did for the messaging layer and have a pluggable API. Or at least have a config option to either use ttorrent or the Utils.downloadFile calls until I have time to dig into ttorrent. |
I have been doing some manual testing and if nimbus goes down it does not reseed previous torrents on startup.
|
Thanks for the review @revans2. I will address all when I'm better connected. To be clear, I don't see this as a high priority -- more of a discussion point. So not a priority for the next release. |
Handle nested config validation nulls distinctly
This pull request has been open for most of a year. Should we close this for now if we are not planning to do anything about it? |
@d2r Thanks for the reminder. Closing. |
This is a resurrection of a pre-apache pull request: nathanmarz/storm#629
(original discussion and code review can be found there)
I've updated it to merge cleanly with the latest master branch and fixed a bug that would prevent multi-lang topologies from working.
Using bittorrent to distribute topology code will make is easier to implement HA Nimbus easier since multiple nimbus nodes could seed to one another when a topology is submitted (i.e. each would have a full copy of the code). We could also incorporate it into leader election such that only a nimbus node with a complete copy of topology code could be elected leader.
It also helps alleviate the "thundering herd" issue where supervisors all hit nimbus at once to download jars.
I've tested this on multi-node clusters with regular, multi-lang, and trident topologies, as well as topology jars larger than 100MB.