spring-cloud / spring-cloud-zookeeper

Spring Cloud Zookeeper
http://cloud.spring.io/spring-cloud-zookeeper/
Apache License 2.0
556 stars 413 forks source link

Feature Request: Support for Leader Election #93

Open twicksell opened 8 years ago

twicksell commented 8 years ago

I noticed a pattern we use occasionally at Netflix (Leader Election) wasn't represented in this framework so I thought I'd raise an issue and outline some of the nice features that a good Spring wrapper around the Curator recipe could offer.

spring.leadership.manage.messaging=aContainerName //some specific container bean name
spring.leadership.manage.messaging=* //all the containers
spring.leadership.manage.integration=aChannel,anotherChannel,*Channel //bean names that support start/stop
spring.leadership.manage.batch=aJob, anotherJob
garyrussell commented 8 years ago

Hi @twicksell - are you aware that Spring Integration has ZK Leadership Election Support.

In SI, you can assign endpoints (message sources etc) to roles and the framework will automatically start/stop them when leadership is granted/revoked.

twicksell commented 8 years ago

I was not, thanks @garyrussell ! I think I still want this feature in a broader library than SI, but this is definitely a good place to borrow implementation from. I'll take a look through how thats implemented.

jkschneider commented 8 years ago

WDYT about @LeaderOnly for things like @Scheduled to use instead of a prop where possible?

spencergibb commented 8 years ago

@garyrussell or @dsyer can you comment on why s-c-cluster was deprecated?

dsyer commented 8 years ago

Because all the features were moved to spring integration. Zookeeper leader election is first class over there now, and SI is (IMO) very broad, so I don't know why it wouldn't meet any requirement that people have. Jon's idea about @LeaderOnly sounds interesting though.

spencergibb commented 8 years ago

Where do you think something like tying discovery to leadership should live?

dsyer commented 8 years ago

Discovery is clearly part of spring cloud, but health is spring boot (note also the example above was about eureka, so not using zk for discovery). All of the other features being requested are available in spring integration. I think we need to talk with @garyrussell about what we can offer people who want leader election but not messaging. Probably the zk features in SI are not very tightly coupled with messaging anyway. Someone should see what it's like to use.

garyrussell commented 8 years ago

Correct; there is no messaging in the leadership election stuff.

The core abstractions are here.

And the ZK implementation here.

There are really no hooks between this and messaging, even the SmartLifecycleRoleController here has no messaging dependencies, it starts/stops specific SmartLifecycles based on leadership. So it has general applicability.

I don't remember the details but we were up against a deadline and decided to lift the core abstractions from spring-cloud-cluster, although I think we tweaked them a little. I think the concern was dependency tangles and we discussed that, perhaps, these abstractions belong in a stand-alone top-level spring project spring-cluster, spring-leadership or similar so it can be used across the portfolio (like spring-retry).

It should be a clean lift and place since there are no SI dependencies at all.

I have no objections to also moving the zk implementation to s-c-zk, as long as we don't end up with tangles by having s-i-zk depend on that.

jkubrynski commented 8 years ago

Generally, even on the main Spring Cloud site (http://projects.spring.io/spring-cloud/) there is information that it handles leader election, and for me, spring-cloud is a valid home for such functionality. Probably most of the users would be people using Spring Cloud. If it's somehow not possible I'll vote for moving it into a separate project like spring-leadership.

artembilan commented 8 years ago

Spring Integration also has cluster-like feature in face of LockRegistry, for example: http://docs.spring.io/spring-integration/docs/4.3.4.RELEASE/reference/html/messaging-endpoints-chapter.html#leadership-event-handling.

So, I vote for spring-cluster to move those implementations as well.

jkubrynski commented 7 years ago

Any progress with it? :)

spencergibb commented 7 years ago

@jkubrynski no

daniellavoie commented 7 years ago

I like the idea of a @LeaderOnly annotation.

On another aspect, this leader election system implies some coupling to the underlying election implementation (in this issue we are talking about ZK). Of course a proper abstraction is possible to provide a plug n play experience.

We have discovery servers and config servers. How about a Lock server ? I've build a simple POC inspired by the config server and @dsyer locksdemo. For now it's only a simple '@EnableLockServer' which auto configures a controller providing locks. But it could go further with a client offering a lock api and support for stuff like @LeaderOnlyon scheduled tasks or @ElectedEvent, @DemotedEvent, etc...

Such dedicated service would remove any dependencies from ZK, Hazelcast or jdbc for any component in need of locks or leader election.

Further details here : spring-cloud/spring-cloud-commons#173 POC : https://github.com/daniellavoie/spring-cloud-lock

daniellavoie commented 7 years ago

I've been thinking on how to implement such @LeaderOnly on a @Scheduled method.

I'm clueless because the ScheduledAnnotationBeanPostProcessor checks for the presence of @Scheduled and registers the tasks for execution. The execution rules are defined during initialization. I can't find any way we can interfer with this at runtime. The ScheduledTaskRegistrar is private and not accessible so we can't mutate it while responding to election events. Plus, it can't find a way to prevent race conditions from tasks being registered then cancelled by leader events.

I'm suggesting to limit ourselves to a LockClient and to LeadershipEvent.

Here is an exemple :

  private static final PROCESS_NAME = "something-important";

  private boolean leader = false;

  @EventListener
  public void handleLeadershipEvent(LeadershipEvent leadershipEvent){
    if(leadershipEvent.getProcessName().equals(PROCESS_NAME))
      leader = leadershipEvent.isLeader();
  }

  @Scheduled(fixedDelay=60000)
  public void  executeSomethingImportant(){
    if(leader)
      importantService.doIt();
  }
daniellavoie commented 7 years ago

I went a little further with an implementation and realized that using Spring EventListener to react to a LeaderElection composed with a @Scheduled is something really dangerous for race conditions. The task that we wish to be executed by a single instance needs to run in a context where the lock has the highest guarantees of being active.

This is why I am proposing something like this :

  @Autowired
  private LockClient lockClient;

  @Scheduled(fixedDelay=60000)
  public void  executeSomethingImportant(){
    // Will executes the Runnable if lock is active.
    lockClient.executeIfOwned("lock-key", () -> importantService.doStuff());

    // This variant uses spring.application.name as the lock key.
    lockClient.executeIfOwned(() -> importantService.doStuff());
  }

With a LockClient providing a "safe" context to execute our tasks, we could then support an extension of @Scheduled like @LeaderScheduled. But this is tricky since it would represents rewriting a ScheduledAnnotationBeanPostProcessor specially for the @LeaderScheduled. I am not a fan. I think a simple wrap with lockClient.executeIfOwned by end users is less messy than having to fork the processor.

Edit : A working POC is available here