Skip to content
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

Feature Request: Support for Leader Election #93

Open
twicksell opened this issue Sep 30, 2016 · 15 comments
Open

Feature Request: Support for Leader Election #93

twicksell opened this issue Sep 30, 2016 · 15 comments

Comments

@twicksell
Copy link

twicksell commented Sep 30, 2016

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.

  • Enabling the feature
    • Suggest something like @EnableLeaderElection(zkPath="/some/zookeeper/path")
    • Extra nice if we can make this Repeatable or or take an array argument for zkPath to multiple leadership for multiple topics
  • Discovery/Eureka integration
    • If an application is marked out of service or unhealthy in Eureka (or just unhealthy in terms of Spring Boot HealthIndicators possibly), the application should give up its LeaderLatch so that another instance can take over. This helps to support Red/Black pushes.
    • Implementation note, when coming back into service or healthy again, we would want the application to again request leadership. This may require re-creating the Curator LeaderLatch, so that bean may not be a good fit for Singleton scope and instead may need some proxy.
  • Event Integration
    • A LeadershipEvent should be fired whenever status changes. This would be used to support batch processes.
    • Common patterns that may be useful to expose via Config are to shut down framework components such as MessagingContainers (JMS, SQS, RabbitMQ) doing queue consumption, Spring Integration channel adapters, Spring Batch jobs, @scheduled pollers, etc. Obviously users can subscribe to the events themselves to manage each of these frameworks, but I'm considering a config pattern as follows
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
  • Actuator
    • An actuator for leadership that can indicate if the current instance is a leader (Helpful if status code also reflects this, 2xx for yes, something else for no)
    • Actuator should indicate what beans/channels are being governed by Leadership as in the above point
    • Actuator should accept POST requests to force an instance to relinquish leadership (a handy testing util)
@garyrussell
Copy link

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
Copy link
Author

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
Copy link

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

@spencergibb
Copy link
Member

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

@dsyer
Copy link
Contributor

dsyer commented Oct 3, 2016

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
Copy link
Member

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

@dsyer
Copy link
Contributor

dsyer commented Oct 4, 2016

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
Copy link

garyrussell commented Oct 4, 2016

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
Copy link

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
Copy link

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
Copy link

jkubrynski commented Dec 28, 2016

Any progress with it? :)

@spencergibb
Copy link
Member

@jkubrynski no

@daniellavoie
Copy link

daniellavoie commented Feb 1, 2017

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
Copy link

daniellavoie commented Feb 4, 2017

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
Copy link

daniellavoie commented Feb 10, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants