-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
kvserverpb: move quorum safeguard into execChangeReplicasTxn
This used to live in the replicate queue, but there are other entry points to replication changes, notably the store rebalancer which caused #54444. Move the check in the guts of replication changes where it is guaranteed to be invoked. Fixes #50729 Touches #54444 (release-20.2) Release note (bug fix): in rare situations, an automated replication change could result in a loss of quorum. This would require down nodes and a simultaneous change in the replication factor. Note that a change in the replication factor can occur automatically if the cluster is comprised of less than five available nodes. Experimentally the likeli- hood of encountering this issue, even under contrived conditions, was small.
- Loading branch information
Showing
15 changed files
with
174 additions
and
124 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // Copyright 2020 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package kvserver | ||
|
|
||
| import ( | ||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/errors" | ||
| ) | ||
|
|
||
| // NB: don't change the string here; this will cause cross-version issues | ||
| // since this singleton is used as a marker. | ||
| var errMarkSnapshotError = errors.New("snapshot failed") | ||
|
|
||
| // isSnapshotError returns true iff the error indicates that a snapshot failed. | ||
| func isSnapshotError(err error) bool { | ||
| return errors.Is(err, errMarkSnapshotError) | ||
| } | ||
|
|
||
| // NB: don't change the string here; this will cause cross-version issues | ||
| // since this singleton is used as a marker. | ||
| var errMarkCanRetryReplicationChangeWithUpdatedDesc = errors.New("should retry with updated descriptor") | ||
|
|
||
| // IsRetriableReplicationChangeError detects whether an error (which is | ||
| // assumed to have been emitted by the KV layer during a replication change | ||
| // operation) is likely to succeed when retried with an updated descriptor. | ||
| func IsRetriableReplicationChangeError(err error) bool { | ||
| return errors.Is(err, errMarkCanRetryReplicationChangeWithUpdatedDesc) || isSnapshotError(err) | ||
| } | ||
|
|
||
| const ( | ||
| descChangedRangeSubsumedErrorFmt = "descriptor changed: expected %s != [actual] nil (range subsumed)" | ||
| descChangedErrorFmt = "descriptor changed: [expected] %s != [actual] %s" | ||
| ) | ||
|
|
||
| func newDescChangedError(desc, actualDesc *roachpb.RangeDescriptor) error { | ||
| if actualDesc == nil { | ||
| return errors.Mark(errors.Newf(descChangedRangeSubsumedErrorFmt, desc), errMarkCanRetryReplicationChangeWithUpdatedDesc) | ||
| } | ||
| return errors.Mark(errors.Newf(descChangedErrorFmt, desc, actualDesc), errMarkCanRetryReplicationChangeWithUpdatedDesc) | ||
| } | ||
|
|
||
| func wrapDescChangedError(err error, desc, actualDesc *roachpb.RangeDescriptor) error { | ||
| if actualDesc == nil { | ||
| return errors.Mark(errors.Wrapf(err, descChangedRangeSubsumedErrorFmt, desc), errMarkCanRetryReplicationChangeWithUpdatedDesc) | ||
| } | ||
| return errors.Mark(errors.Wrapf(err, descChangedErrorFmt, desc, actualDesc), errMarkCanRetryReplicationChangeWithUpdatedDesc) | ||
| } | ||
|
|
||
| // NB: don't change the string here; this will cause cross-version issues | ||
| // since this singleton is used as a marker. | ||
| var errMarkInvalidReplicationChange = errors.New("invalid replication change") | ||
|
|
||
| // IsIllegalReplicationChangeError detects whether an error (assumed to have been emitted | ||
| // by a replication change) indicates that the replication change is illegal, meaning | ||
| // that it's unlikely to be handled through a retry. Examples of this are attempts to add | ||
| // a store that is already a member of the supplied descriptor. A non-example is a change | ||
| // detected in the descriptor at the KV layer relative to that supplied as input to the | ||
| // replication change, which would likely benefit from a retry. | ||
| func IsIllegalReplicationChangeError(err error) bool { | ||
| return errors.Is(err, errMarkInvalidReplicationChange) | ||
| } |
Oops, something went wrong.