Now that we've migrated to curator's LeaderLatch for leader election, it seems that our leader election code is doing WAY more than necessary. Some things I would like to see fixed:
AnythingBase is almost always an anti-pattern. Get rid of ElectionServiceBase. If we need a common interface, define it. Prefer to put common behavior in helper methods rather than inherit them,e tc.The falling scala script illustrates how concise our leader election module should be:
https://gist.github.com/timcharper/22a1bca65e9a8268225dcfb97420cdf7
I agree with most of your points. Some additions:
ElectionServiceBase: the common interface is already defined: ElectionService. Scala's own recommendation is to have a fixed size threadpool for blocking IO.
The election code is using a different client instance. Should it use the same as the storage?
I'd also like stopLeadership and startLeadership to be lock-free / non-blocking. See my comment in D379.
@jeschkies why even have stopLeadership ? Why not just crash?
@jeschkies it probably should use the same client instance but I'm not sure if Curator does any zookeeper connection pool aggregation behind the scenes.
On second thought, we should research more. What happens if there is a high amount of read/write traffic on the storage and it gets backlogged. Could we lose leadership? I'm not sure what the consequences are.
Note: This issue has been migrated to https://jira.mesosphere.com/browse/MARATHON-2008. For more information see https://groups.google.com/forum/#!topic/marathon-framework/khtvf-ifnp8.
Most helpful comment
Scala's own recommendation is to have a fixed size threadpool for blocking IO.