quantifind/incubator-spark — pull request #2 by squito (2013-11-07 20:46:45+00:00)
Title: Prevent the master from double-listing a worker

We've noticed that if an exception occurs in the workers `receive` method, then the master ends up counting that worker twice. This patch protects against this in two different ways (though there should probably also be a third).

We noticed the bug when a user submits a job with a small typo in `sparkHome`. The Executor will fail to start, the master will try to start it again, etc. Sometimes (for us, about 75% of the time) this results in an ExecutorStateChanged msg for an unkown executor, which gives an exception. Then, somehow, we end up with two active `Worker` objects -- same host, same port, but different start time, so different ID. Both workers continue to send heartbeats, so the master tries to assign work to both of them. Then the node becomes overcommitted.

(This can get really ugly when the user is confused by the failure and just re-submits a few times, before correcting spark home, which can lead to a box getting quadruple-overcommitted.)

The two patches here are:

1) avoid the original exception, by ignoring ExecutorStateChanged for unknown executors.

2) If somehow the master does think it has two workers with the same host & port, it will only ever send work to one of them.

I'd really like to add one more layer of protection for this, though. If the worker throws some other exception in `receive`, we shouldn't have two workers round, both sending heartbeats. I'm not sure how to prevent that, though.

I've tested this out in a live deployment, but haven't added any unit tests. I'm open to ideas on how to add unit tests.

Note that I've only confirmed the existence of this bug on akka 2.1.4, but I think these changes are good regardless. And I have a feeling I've observed this before, just never isolated it.