-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-24.1: raft: re-enable config change safety #124820
Conversation
Config changes in this raft implementation require a safety constraint: the leader must not append a config change if it hasn't applied all config changes in its log. The DisableConfChangeValidation flag disables this check under the assumption that the state machine layer provides the equivalent guarantee. However, it is hard to argue that this is true in split leaseholder/leader scenarios. This commit re-enables this check, to bring the safety back. The other two state-machine-level checks concerned with entering and leaving joint configs can still be disabled. Epic: none Release note: none
This commit adds a test which ensures a config change is not proposed if the leader has another yet unapplied config change. Epic: none Release note: none
3917e87
to
af16f42
Compare
c4e7b02
to
3917e87
Compare
754a5fc
to
2af261b
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
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.
Do we need to get this back to v23.2 in one form or another as well?
I think so, this behaviour was changed last summer. |
Backport 2/2 commits from #124804 on behalf of @pav-kv.
/cc @cockroachdb/release
Config changes in this
raft
implementation require a safety constraint: the leader must not append a config change if it hasn't applied all config changes in its log.The
DisableConfChangeValidation
flag disables this check under the assumption that the state machine layer provides the equivalent guarantee. However, it is hard to argue that this is true in split leaseholder/leader scenarios.This commit re-enables this check, to bring the safety back. The other two state-machine-level checks concerned with entering and leaving joint configs can still be disabled.
Related to #105797, etcd-io/raft#81, #106515 (#106515 (comment))
Epic: none
Release note: none
Release justification: safety bug fix