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

release-23.1: DEPS: bump across etcd-io/raft#81 and disable conf change validation #107059

Merged
merged 2 commits into from
Jul 24, 2023
Merged

release-23.1: DEPS: bump across etcd-io/raft#81 and disable conf change validation #107059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f37a386
DEPS: bump raft (cherry-pick https://github.com/etcd-io/raft/pull/81)
tbg Jul 18, 2023
a8b6592
kvserver: prevent etcd-io/raft from dropping conf changes
tbg Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions DEPS.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10327,10 +10327,10 @@ def go_deps():
],
build_file_proto_mode = "default",
importpath = "go.etcd.io/raft/v3",
sha256 = "cebe2f313d01c24a8927170d44ecba9e5da56de488245e95a662edd590ed9a92",
strip_prefix = "github.com/cockroachdb/raft/[email protected]20230713102459-53e0e3664cb0",
sha256 = "fcad7dc8a9d53bbb3d5ed050827004a70286649ca2fb66504d3dab44abb13a92",
strip_prefix = "github.com/cockroachdb/raft/[email protected]20230718062839-336c0ec27d85",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230713102459-53e0e3664cb0.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230718062839-336c0ec27d85.zip",
],
)
go_repository(
Expand Down
2 changes: 1 addition & 1 deletion build/bazelutil/distdir_files.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230712164400-52cfb819e866.zip": "bd2df4269fa5f4740b0f96663b6bc695e5ec7cb09f42ac0ffdc674d4f64abdd1",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230713102459-53e0e3664cb0.zip": "cebe2f313d01c24a8927170d44ecba9e5da56de488245e95a662edd590ed9a92",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230718062839-336c0ec27d85.zip": "fcad7dc8a9d53bbb3d5ed050827004a70286649ca2fb66504d3dab44abb13a92",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.4.zip": "9a723dfbb1627ae2f2b5d1374a59d6188ae9796e6dfb9a4622a4eb94321a4fac",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,6 @@ replace gopkg.in/yaml.v2 => github.com/cockroachdb/yaml v0.0.0-20210825132133-2d
replace github.com/docker/docker => github.com/moby/moby v20.10.6+incompatible

// Take etcd-io/raft from the branch corresponding to release-23.1 in our fork.
replace go.etcd.io/raft/v3 => github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0
replace go.etcd.io/raft/v3 => github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85

replace golang.org/x/time => github.com/cockroachdb/x-time v0.3.1-0.20230525123634-71747adb5d5c
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
github.com/cockroachdb/pebble v0.0.0-20230712164400-52cfb819e866 h1:3Xr/jeWmpd3JZ8VCaKB6g1XOc/Bj99gNa3VIlZM7Oes=
github.com/cockroachdb/pebble v0.0.0-20230712164400-52cfb819e866/go.mod h1:9lRMC4XN3/BLPtIp6kAKwIaHu369NOf2rMucPzipz50=
github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0 h1:Lss4oj/NtrEHxHoLUp+aMqwwq4jyk7RegHfn1GT4RF4=
github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0/go.mod h1:xa/jfCF4K9FpWEAXstT+Nw4ToWdmhJ8oeC5eburtypA=
github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85 h1:Lv0iiCw5BB6y/loaOgeC75OwB9x6Hi1XbBuhU9eQFNg=
github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85/go.mod h1:xa/jfCF4K9FpWEAXstT+Nw4ToWdmhJ8oeC5eburtypA=
github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
github.com/cockroachdb/redact v1.1.4 h1:Y0XrVI2FAyofNyGveodTN//qdpPtFKcKTeCBsK3AHAQ=
github.com/cockroachdb/redact v1.1.4/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
Expand Down
62 changes: 62 additions & 0 deletions pkg/kv/kvserver/client_raft_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5998,3 +5998,65 @@ func TestRaftSnapshotsWithMVCCRangeKeysEverywhere(t *testing.T) {
require.Equal(t, kvpb.CheckConsistencyResponse_RANGE_CONSISTENT, result.Status, "%+v", result)
}
}

// TestInvalidConfChangeRejection is a regression test for [1]. It proposes
// an (intentionally) invalid configuration change and makes sure that raft
// does not drop it.
//
// [1]: https://github.com/cockroachdb/cockroach/issues/105797
func TestInvalidConfChangeRejection(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

// This is a regression test against a stuck command, so set a timeout to get
// a shot at a graceful failure on regression.
ctx, cancel := context.WithTimeout(context.Background(), testutils.DefaultSucceedsSoonDuration)
defer cancel()

// When our configuration change shows up below raft, we need to apply it as a
// no-op, since the config change is intentionally invalid and assertions
// would fail if we were to try to actually apply it.
injErr := errors.New("injected error")
tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
ReplicationMode: base.ReplicationManual,
ServerArgs: base.TestServerArgs{Knobs: base.TestingKnobs{Store: &kvserver.StoreTestingKnobs{
TestingApplyCalledTwiceFilter: func(args kvserverbase.ApplyFilterArgs) (int, *kvpb.Error) {
if args.Req != nil && args.Req.Txn != nil && args.Req.Txn.Name == "fake" {
return 0, kvpb.NewError(injErr)
}
return 0, nil
}}}},
})
defer tc.Stopper().Stop(ctx)

k := tc.ScratchRange(t)

repl := tc.GetFirstStoreFromServer(t, 0).LookupReplica(keys.MustAddr(k))

// Try to leave a joint config even though we're not in one. This is something
// that will lead raft to propose an empty entry instead of our conf change.
//
// See: https://github.com/cockroachdb/cockroach/issues/105797
var ba kvpb.BatchRequest
now := tc.Server(0).Clock().Now()
txn := roachpb.MakeTransaction("fake", k, roachpb.NormalUserPriority, now, 500*time.Millisecond.Nanoseconds(), 1)
ba.Txn = &txn
ba.Timestamp = now
ba.Add(&kvpb.EndTxnRequest{
RequestHeader: kvpb.RequestHeader{
Key: k,
},
Commit: true,
InternalCommitTrigger: &roachpb.InternalCommitTrigger{
ChangeReplicasTrigger: &roachpb.ChangeReplicasTrigger{
Desc: repl.Desc(),
},
},
})

_, pErr := repl.Send(ctx, &ba)
// Verify that we see the configuration change below raft, where we rejected it
// (since it would've otherwise blow up the Replica: after all, we intentionally
// proposed an invalid configuration change.
require.True(t, errors.Is(pErr.GoError(), injErr), "%+v", pErr.GoError())
}
25 changes: 13 additions & 12 deletions pkg/kv/kvserver/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,19 @@ func newRaftConfig(
strg raft.Storage, id uint64, appliedIndex uint64, storeCfg StoreConfig, logger raft.Logger,
) *raft.Config {
return &raft.Config{
ID: id,
Applied: appliedIndex,
AsyncStorageWrites: true,
ElectionTick: storeCfg.RaftElectionTimeoutTicks,
HeartbeatTick: storeCfg.RaftHeartbeatIntervalTicks,
MaxUncommittedEntriesSize: storeCfg.RaftMaxUncommittedEntriesSize,
MaxCommittedSizePerReady: storeCfg.RaftMaxCommittedSizePerReady,
MaxSizePerMsg: storeCfg.RaftMaxSizePerMsg,
MaxInflightMsgs: storeCfg.RaftMaxInflightMsgs,
MaxInflightBytes: storeCfg.RaftMaxInflightBytes,
Storage: strg,
Logger: logger,
ID: id,
Applied: appliedIndex,
AsyncStorageWrites: true,
ElectionTick: storeCfg.RaftElectionTimeoutTicks,
HeartbeatTick: storeCfg.RaftHeartbeatIntervalTicks,
MaxUncommittedEntriesSize: storeCfg.RaftMaxUncommittedEntriesSize,
MaxCommittedSizePerReady: storeCfg.RaftMaxCommittedSizePerReady,
DisableConfChangeValidation: true, // see https://github.com/cockroachdb/cockroach/issues/105797
MaxSizePerMsg: storeCfg.RaftMaxSizePerMsg,
MaxInflightMsgs: storeCfg.RaftMaxInflightMsgs,
MaxInflightBytes: storeCfg.RaftMaxInflightBytes,
Storage: strg,
Logger: logger,

PreVote: true,
}
Expand Down