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

Fixes to avoid operator conflict with the sentinel in choosing master #536

Merged
merged 6 commits into from
Dec 9, 2022

Conversation

dinesh-murugiah
Copy link
Contributor

Fixes # .

Changes proposed on the PR:

  • Currently Operator and Sentinel seems to share similar responsibility in certain conditions ,

  • this will create conflict and eventually misconfigure the system

  • For Example:

  • When a master is not available and sentinel is in the process of taking an action to fix a replica as master,

  • at the same time if the operator also fixes another replica as a slave , it could end up in a split brain or necessary switch of master between two replicas, moreover sentinel is the right candidate to choose master

  • and operator should not step in unless sentinel is in a good shape to do its job

  • The Role of the operator is primarily during initial/First deployments of redis-failover

  • and also in scenarios where sentinel cannot make a decision

@dinesh-murugiah dinesh-murugiah requested a review from a team as a code owner December 5, 2022 11:23
@samof76
Copy link
Contributor

samof76 commented Dec 7, 2022

Great, looks good. @ese please review.

@samof76 samof76 mentioned this pull request Dec 7, 2022
@ese
Copy link
Member

ese commented Dec 7, 2022

Thanks! I think is more reliable way to ensure redis-operator acts when it have to do. Since GetMinimumRedisPodTime is no longer used we should remove it.

service/redis/client.go Outdated Show resolved Hide resolved
service/redis/client.go Outdated Show resolved Hide resolved
@ese ese merged commit ea4f232 into spotahome:master Dec 9, 2022
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.

3 participants