Mirroristas/etcd
2
0
Fork 0
mirror of https://github.com/etcd-io/etcd.git synced 2024-09-27 06:25:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Applying consistency fix: ClusterVersionSet (and co) might get no applied on v2store

Browse Source 
ClusterVersionSet, ClusterMemberAttrSet, DowngradeInfoSet functions are
writing both to V2store and backend. Prior this CL there were
in a branch not executed if shouldApplyV3 was false,
e.g. during restore when Backend is up-to-date (has high
consistency-index) while v2store requires replay from WAL log.

The most serious consequence of this bug was that v2store after restore
could have different index (revision) than the same exact store before restore,
so potentially different content between replicas.

Also this change is supressing double-applying of Membership
(ClusterConfig) changes on Backend (store v3) - that lackilly are not
part of MVCC/KeyValue store, so they didn't caused Revisions to be
bumped.

Inspired by jingyih@ comment:
https://github.com/etcd-io/etcd/pull/12820#issuecomment-815299406
This commit is contained in:
Piotr Tabor
2021-04-10 11:42:15 +02:00
parent bad0b4d513
commit b1c04ce043
12 changed files with 112 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
server/etcdserver/api/membership/cluster.go
View File

@@ -285,6 +285,7 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {
// ValidateConfigurationChange takes a proposed ConfChange and
// ensures that it is still valid.
func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
// TODO: this must be switched to backend as well.
members, removed := membersFromStore(c.lg, c.v2store)
id := types.ID(cc.NodeID)
if removed[id] {
@@ -370,13 +371,13 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
// AddMember adds a new Member into the cluster, and saves the given member's
// raftAttributes into the store. The given member should have empty attributes.
// A Member with a matching id must not exist.
func (c *RaftCluster) AddMember(m *Member) {
func (c *RaftCluster) AddMember(m *Member, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
if c.v2store != nil {
mustSaveMemberToStore(c.lg, c.v2store, m)
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveMemberToBackend(c.lg, c.be, m)
}
@@ -393,13 +394,13 @@ func (c *RaftCluster) AddMember(m *Member) {
// RemoveMember removes a member from the store.
// The given id MUST exist, or the function panics.
func (c *RaftCluster) RemoveMember(id types.ID) {
func (c *RaftCluster) RemoveMember(id types.ID, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
if c.v2store != nil {
mustDeleteMemberFromStore(c.lg, c.v2store, id)
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustDeleteMemberFromBackend(c.be, id)
}
@@ -425,7 +426,7 @@ func (c *RaftCluster) RemoveMember(id types.ID) {
}
}
func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) {
func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
@@ -434,7 +435,7 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) {
if c.v2store != nil {
mustUpdateMemberAttrInStore(c.lg, c.v2store, m)
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveMemberToBackend(c.lg, c.be, m)
}
return
@@ -459,7 +460,7 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes) {
}
// PromoteMember marks the member's IsLearner RaftAttributes to false.
func (c *RaftCluster) PromoteMember(id types.ID) {
func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
@@ -467,7 +468,7 @@ func (c *RaftCluster) PromoteMember(id types.ID) {
if c.v2store != nil {
mustUpdateMemberInStore(c.lg, c.v2store, c.members[id])
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveMemberToBackend(c.lg, c.be, c.members[id])
}
@@ -478,7 +479,7 @@ func (c *RaftCluster) PromoteMember(id types.ID) {
)
}
func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes) {
func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
@@ -486,7 +487,7 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes)
if c.v2store != nil {
mustUpdateMemberInStore(c.lg, c.v2store, c.members[id])
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveMemberToBackend(c.lg, c.be, c.members[id])
}
@@ -508,7 +509,7 @@ func (c *RaftCluster) Version() *semver.Version {
return semver.Must(semver.NewVersion(c.version.String()))
}
func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *semver.Version)) {
func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *semver.Version), shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
if c.version != nil {
@@ -533,7 +534,7 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s
if c.v2store != nil {
mustSaveClusterVersionToStore(c.lg, c.v2store, ver)
}
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveClusterVersionToBackend(c.be, ver)
}
if oldVer != nil {
@@ -809,11 +810,11 @@ func (c *RaftCluster) DowngradeInfo() *DowngradeInfo {
return d
}
func (c *RaftCluster) SetDowngradeInfo(d *DowngradeInfo) {
func (c *RaftCluster) SetDowngradeInfo(d *DowngradeInfo, shouldApplyV3 bool) {
c.Lock()
defer c.Unlock()
if c.be != nil {
if c.be != nil && shouldApplyV3 {
mustSaveDowngradeToBackend(c.lg, c.be, d)
}

14
server/etcdserver/api/membership/cluster_test.go
View File

@@ -283,9 +283,9 @@ func TestClusterValidateConfigurationChange(t *testing.T) {
cl.SetStore(v2store.New())
for i := 1; i <= 4; i++ {
attr := RaftAttributes{PeerURLs: []string{fmt.Sprintf("http://127.0.0.1:%d", i)}}
cl.AddMember(&Member{ID: types.ID(i), RaftAttributes: attr})
cl.AddMember(&Member{ID: types.ID(i), RaftAttributes: attr}, true)
}
cl.RemoveMember(4)
cl.RemoveMember(4, true)
attr := RaftAttributes{PeerURLs: []string{fmt.Sprintf("http://127.0.0.1:%d", 1)}}
ctx, err := json.Marshal(&Member{ID: types.ID(5), RaftAttributes: attr})
@@ -446,7 +446,7 @@ func TestClusterGenID(t *testing.T) {
previd := cs.ID()
cs.SetStore(mockstore.NewNop())
cs.AddMember(newTestMember(3, nil, "", nil))
cs.AddMember(newTestMember(3, nil, "", nil), true)
cs.genID()
if cs.ID() == previd {
t.Fatalf("cluster.ID = %v, want not %v", cs.ID(), previd)
@@ -489,7 +489,7 @@ func TestClusterAddMember(t *testing.T) {
st := mockstore.NewRecorder()
c := newTestCluster(t, nil)
c.SetStore(st)
c.AddMember(newTestMember(1, nil, "node1", nil))
c.AddMember(newTestMember(1, nil, "node1", nil), true)
wactions := []testutil.Action{
{
@@ -512,7 +512,7 @@ func TestClusterAddMemberAsLearner(t *testing.T) {
st := mockstore.NewRecorder()
c := newTestCluster(t, nil)
c.SetStore(st)
c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil))
c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil), true)
wactions := []testutil.Action{
{
@@ -555,7 +555,7 @@ func TestClusterRemoveMember(t *testing.T) {
st := mockstore.NewRecorder()
c := newTestCluster(t, nil)
c.SetStore(st)
c.RemoveMember(1)
c.RemoveMember(1, true)
wactions := []testutil.Action{
{Name: "Delete", Params: []interface{}{MemberStoreKey(1), true, true}},
@@ -595,7 +595,7 @@ func TestClusterUpdateAttributes(t *testing.T) {
c := newTestCluster(t, tt.mems)
c.removed = tt.removed
c.UpdateAttributes(types.ID(1), Attributes{Name: name, ClientURLs: clientURLs})
c.UpdateAttributes(types.ID(1), Attributes{Name: name, ClientURLs: clientURLs}, true)
if g := c.Members(); !reflect.DeepEqual(g, tt.wmems) {
t.Errorf("#%d: members = %+v, want %+v", i, g, tt.wmems)
}