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.

Sign up for GitHub

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

Jump to bottom

Keepers that never become master/sync replica #697

Merged
merged 1 commit into from
Apr 28, 2020
Merged

Keepers that never become master/sync replica #697

merged 1 commit into from
Apr 28, 2020

Conversation

rnaveiras
Copy link
Contributor

Implements the proposal #696

@rnaveiras rnaveiras force-pushed the rnaveiras/keepers-never-become-master-sync branch from b69302e to 3705fb2 Compare September 4, 2019 07:38
sgotti
sgotti requested changes Sep 5, 2019
View reviewed changes
@rnaveiras rnaveiras force-pushed the rnaveiras/keepers-never-become-master-sync branch 3 times, most recently from 7410820 to a978806 Compare September 18, 2019 07:41
@sgotti
Copy link
Member

sgotti commented Oct 11, 2019

@rnaveiras Is this ready for review? Can you please rebase?

@rnaveiras rnaveiras force-pushed the rnaveiras/keepers-never-become-master-sync branch from a978806 to ea018c6 Compare October 15, 2019 15:16
@rnaveiras
Copy link
Contributor Author

@sgotti I just rebased and fixed the conflicts. It's ready for review. I'm happy to squash the commits if you think is necessary after you finish your review.

sgotti
sgotti requested changes Oct 16, 2019
View reviewed changes
Copy link
Member

@sgotti sgotti left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@rnaveiras Just a small nit and then it LGTM. Can you please rebase and squash all the commits?

@rnaveiras @lawrencejones Do you have any thought on this comment (#696 (comment))?

@leev
Copy link

leev commented Apr 22, 2020

Any chance this could be rebased and merged soon? I was working on this feature tonight and when coming to submit a PR, I only found this then.

@rnaveiras
Copy link
Contributor Author

Any chance this could be rebased and merged soon? I was working on this feature tonight and when coming to submit a PR, I only found this then.

Hi @leev, yes I will take care of make the changes tomorrow. Sorry for the delay, it's been in my to-do list for months. Thanks for bringing it up.

@rnaveiras rnaveiras force-pushed the rnaveiras/keepers-never-become-master-sync branch from ea018c6 to afd070c Compare April 24, 2020 19:06
@rnaveiras
Copy link
Contributor Author

@sgotti I'm happy to squash the commits if you thing is worth it after the final review. Is possible for me to trigger again the CI e2e tests? They're failing now after the rebase.

@sgotti
Copy link
Member

sgotti commented Apr 28, 2020

@rnaveiras It LGTM. Yes, please squash in a single commit. Thanks!

Currently the run.agola.io instance configuration doesn't permit restart from the UI if not authenticated (and registration is currently disabled). I'll restart it for you if I noticed it failed just for some flakes (since agola could also restart only failed tasks). If you want to trigger a full restart you could amend your commit just to generate a new commit id and force push.

@rnaveiras
Support keepers that cannot become master/sync
05b1b0f 
Keeper new flags:

--can-be-master, prevent keeper from being elected as master
--can-be-synchronous-replica, prevent keeper from being chosen as
synchronous replica

Updates sentinel to support keepers with new flags:

- findBestNewMasters: ignoring keepers that cannot become master

- updateKeeperStatus: update `KeeperStatus` to have `NeverMaster` and
`NeverSynchronousReplica` properties

- updateCluster: ignore standbys that cannot be synchronous standbys
@rnaveiras rnaveiras force-pushed the rnaveiras/keepers-never-become-master-sync branch from b8aaf23 to 05b1b0f Compare April 28, 2020 08:59
@rnaveiras
Copy link
Contributor Author

@rnaveiras It LGTM. Yes, please squash in a single commit. Thanks!

Fantastic!! I just squash all commits.

Currently the run.agola.io instance configuration doesn't permit restart from the UI if not authenticated (and registration is currently disabled). I'll restart it for you if I noticed it failed just for some flakes (since agola could also restart only failed tasks). If you want to trigger a full restart you could amend your commit just to generate a new commit id and force push.

Thanks!

@rnaveiras
Copy link
Contributor Author

rnaveiras commented Apr 28, 2020 •
edited
Loading

@sgotti all CI checks are passing now.

@sgotti
Copy link
Member

sgotti commented Apr 28, 2020

@rnaveiras Thanks! Merging.

@sgotti sgotti merged commit 37f2376 into sorintlab:master Apr 28, 2020
@sgotti sgotti mentioned this pull request Apr 28, 2020
Closed
@lawrencejones lawrencejones deleted the rnaveiras/keepers-never-become-master-sync branch April 28, 2020 15:49
@leev
Copy link

leev commented Apr 28, 2020

Nice! Thank you.

@davissp14 davissp14 mentioned this pull request Aug 9, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lawrencejones lawrencejones lawrencejones left review comments

@sgotti sgotti sgotti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rnaveiras @sgotti @leev @lawrencejones