Marathon: Simplify leadership election code

Created on 22 Dec 2016  路  7Comments  路  Source: mesosphere/marathon

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:

  • remove all locks. As an example, LeaderLatch blocks and this led to a dead lock. With the amount of thread blocking we're doing, it should be no surprise.
  • 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.
  • Perform blocking IO in a thread pool designed for blocking IO. Don't block the global fork join pool.
  • Rip out state-machine-ish logic. When you get offer leadership, great, you're the leader. If you lose, then stand-by. If you transition from having leadership to not having leadership, CRASH. Curator's leader latch handles perpetually trying to obtain leadership in the event that the leader goes missing. It does not need to be "restarted" periodically.

The falling scala script illustrates how concise our leader election module should be:

https://gist.github.com/timcharper/22a1bca65e9a8268225dcfb97420cdf7

Most helpful comment

Scala's own recommendation is to have a fixed size threadpool for blocking IO.

All 7 comments

I agree with most of your points. Some additions:

  • ElectionServiceBase: the common interface is already defined: ElectionService.
  • Perform blocking IO in a thread pool designed for blocking. This is a bigger topic for all places where we do IO. Do you suggest to use multiple different thread pools for each case?

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.

Was this page helpful?
0 / 5 - 0 ratings