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

Bugfix: revoke leadership on graceful shutdown #869

Closed
wants to merge 1 commit into from

Conversation

spuun
Copy link
Member

@spuun spuun commented Dec 6, 2024

WHAT is this pull request doing?

On a graceful shutdown lavin exits before the etcd keepalive fiber has had a chance to revoke the leadership. This will yield after the lease has been closed to give the keepalive fiber a chance to revoke the lease.

HOW can this pull request be tested?

No specs, yet. Needs to be tested manually.

@spuun
Copy link
Member Author

spuun commented Dec 6, 2024

This is a quickfix. I don't like to rely on just a yield, it would have been nicer to use real synchronization with a channel or a waitgroup.

@carlhoerberg
Copy link
Member

Etcd#elect returns a Channel that got two purposes

  1. you can wait for it to close which means we lost leadership
  2. if you close it we will eventually revoke the leadership.

If we instead return a Leadership class that got two methods

  1. wait which waits until we loose the leadership
  2. revoke that synchronous revokes the leadership

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