-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
PRS and ERS don't promote replicas taking backups #16997
base: main
Are you sure you want to change the base?
PRS and ERS don't promote replicas taking backups #16997
Conversation
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16997 +/- ##
==========================================
- Coverage 67.04% 67.03% -0.01%
==========================================
Files 1571 1571
Lines 251677 251700 +23
==========================================
+ Hits 168729 168735 +6
- Misses 82948 82965 +17 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to include an end-to-end test for this covering several cases: only 1 replica in the cluster, and more than one replica, all replicas being backed up, etc.
Moreover, even though this is not a breaking change per se, we should still document it in the v22.0
release notes. Which should be put in ./changelog/22.0/22.0.0/summary.md
, the file does not exist yet.
@ejortegau I think it would be safer to "prefer" not to promote a If we are down to 1 candidate, still being able to write feels better to me, even if the node is busy with backups as well |
Another approach could be to cancel the backup in order to enable promotion, if having backups delayed is preferable to being unavailable for writes for a long time. |
Description
This PR prevents ERS and PRS from promoting hosts that are currently taking backups.
The implementation follows what was suggesteed in the RFC of the issue (link below). Namely, the RPCs used to get information about candidates now include an extra field indicating whether they are running backups or not; and that is
used to filter out the list of valid candidates.
Related Issue(s)
#16558
Checklist
Deployment Notes
N/A