Skip to content

improve-election-failover-on-process-termination#26

Open
gaozongjian wants to merge 1 commit intoLINBIT:masterfrom
gaozongjian:feature/improve-election-failover-on-process-termination
Open

improve-election-failover-on-process-termination#26
gaozongjian wants to merge 1 commit intoLINBIT:masterfrom
gaozongjian:feature/improve-election-failover-on-process-termination

Conversation

@gaozongjian
Copy link
Copy Markdown

Handle OS signals:

  1. cancel the context so ReleaseOnCancel triggers lease release before exit.
  2. Without this, SIGTERM terminates the process before
  3. cancel() is ever called, forcing followers to wait LeaseDuration to take over.

Copy link
Copy Markdown
Member

@JoelColledge JoelColledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution.

Comment thread main.go
// release before exit. Without this, SIGTERM terminates the process before
// cancel() is ever called, forcing followers to wait LeaseDuration to take over.
sigCh := make(chan os.Signal, 1)
signal.Notify(sigCh, syscall.SIGTERM, syscall.SIGINT)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we use signal.NotifyContext to implement this more simply?

Comment thread main.go
select {
case sig := <-sigCh:
log.Infof("received signal %v, releasing leader lease before exit", sig)
return fmt.Errorf("received signal: %v", sig)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of whether we switch to signal.NotifyContext, I believe this is likely to be ineffective: When a signal is received, we return here and exit the process without waiting for the elector goroutine to respond to the context cancellation and give up the lease. This is an existing bug, but it is more relevant with this change.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants