Skip to content

Commit 30f9ac5

Browse files
ahrtrahrtr
authored
Merge pull request etcd-io#20139 from serathius/remove-v2store
Remove v2 store
2 parents 0448a42 + fdedec1 commit 30f9ac5

File tree

9 files changed

+31
-312
lines changed

9 files changed

+31
-312
lines changed

‎server/etcdserver/api/membership/cluster.go‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ type RaftCluster struct {
4646
localID types.ID
4747
cid types.ID
4848

49-
v2store v2store.Store
50-
be MembershipBackend
49+
be MembershipBackend
5150

5251
sync.Mutex // guards the fields below
5352
version *semver.Version
@@ -116,7 +115,6 @@ func NewCluster(lg *zap.Logger, opts ...ClusterOption) *RaftCluster {
116115
removed: make(map[types.ID]bool),
117116
downgradeInfo: &serverversion.DowngradeInfo{Enabled: false},
118117
maxLearners: clOpts.maxLearners,
119-
v2store: v2store.New(),
120118
}
121119
}
122120

@@ -246,8 +244,6 @@ func (c *RaftCluster) SetID(localID, cid types.ID) {
246244
c.buildMembershipMetric()
247245
}
248246

249-
func (c *RaftCluster) SetStore(st v2store.Store) { c.v2store = st }
250-
251247
func (c *RaftCluster) SetBackend(be MembershipBackend) {
252248
c.be = be
253249
c.be.MustCreateBackendBuckets()
@@ -307,15 +303,12 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {
307303
// ValidateConfigurationChange takes a proposed ConfChange and
308304
// ensures that it is still valid.
309305
func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange, shouldApplyV3 ShouldApplyV3) error {
310-
var membersMap map[types.ID]*Member
311-
var removedMap map[types.ID]bool
312-
313-
if shouldApplyV3 {
314-
membersMap, removedMap = c.be.MustReadMembersFromBackend()
315-
} else {
316-
membersMap, removedMap = membersFromStore(c.lg, c.v2store)
306+
if !shouldApplyV3 {
307+
return nil
317308
}
318309

310+
membersMap, removedMap := c.be.MustReadMembersFromBackend()
311+
319312
id := types.ID(cc.NodeID)
320313
if removedMap[id] {
321314
return ErrIDRemoved
@@ -400,7 +393,6 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange, shouldAp
400393
func (c *RaftCluster) AddMember(m *Member, shouldApplyV3 ShouldApplyV3) {
401394
c.Lock()
402395
defer c.Unlock()
403-
mustSaveMemberToStore(c.lg, c.v2store, m)
404396

405397
if m.ID == c.localID {
406398
setIsLearnerMetric(m)
@@ -436,7 +428,6 @@ func (c *RaftCluster) AddMember(m *Member, shouldApplyV3 ShouldApplyV3) {
436428
func (c *RaftCluster) RemoveMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
437429
c.Lock()
438430
defer c.Unlock()
439-
mustDeleteMemberFromStore(c.lg, c.v2store, id)
440431
if shouldApplyV3 {
441432
c.be.MustDeleteMemberFromBackend(id)
442433

@@ -478,7 +469,6 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes, shouldApply
478469

479470
if m, ok := c.members[id]; ok {
480471
m.Attributes = attr
481-
mustUpdateMemberAttrInStore(c.lg, c.v2store, m)
482472
if shouldApplyV3 {
483473
c.be.MustSaveMemberToBackend(m)
484474
}
@@ -508,19 +498,6 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
508498
c.Lock()
509499
defer c.Unlock()
510500

511-
membersMap, _ := membersFromStore(c.lg, c.v2store)
512-
if _, ok := membersMap[id]; ok {
513-
m := *(membersMap[id])
514-
m.RaftAttributes.IsLearner = false
515-
mustUpdateMemberInStore(c.lg, c.v2store, &m)
516-
} else {
517-
c.lg.Info("Skipped promoting non-existent member in v2store",
518-
zap.String("cluster-id", c.cid.String()),
519-
zap.String("local-member-id", c.localID.String()),
520-
zap.String("promoted-member-id", id.String()),
521-
)
522-
}
523-
524501
if id == c.localID {
525502
isLearner.Set(0)
526503
}
@@ -561,7 +538,6 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes,
561538
if _, ok := c.members[id]; ok {
562539
m := *(c.members[id])
563540
m.RaftAttributes = raftAttr
564-
mustUpdateMemberInStore(c.lg, c.v2store, &m)
565541
} else {
566542
c.lg.Info("Skipped updating non-existent member in v2store",
567543
zap.String("cluster-id", c.cid.String()),
@@ -639,7 +615,6 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s
639615
c.version = ver
640616
sv := semver.Must(semver.NewVersion(version.Version))
641617
serverversion.MustDetectDowngrade(c.lg, sv, c.version)
642-
mustSaveClusterVersionToStore(c.lg, c.v2store, ver)
643618

644619
if shouldApplyV3 {
645620
c.be.MustSaveClusterVersionToBackend(ver)

‎server/etcdserver/api/membership/cluster_test.go‎

Lines changed: 6 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,15 @@ import (
1818
"encoding/json"
1919
"errors"
2020
"fmt"
21-
"path"
2221
"reflect"
2322
"testing"
2423

2524
"github.com/stretchr/testify/assert"
2625
"github.com/stretchr/testify/require"
2726
"go.uber.org/zap/zaptest"
2827

29-
"go.etcd.io/etcd/client/pkg/v3/testutil"
3028
"go.etcd.io/etcd/client/pkg/v3/types"
3129
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
32-
"go.etcd.io/etcd/server/v3/mock/mockstore"
3330
"go.etcd.io/raft/v3/raftpb"
3431
)
3532

@@ -279,18 +276,9 @@ func TestClusterValidateAndAssignIDs(t *testing.T) {
279276
}
280277

281278
func TestClusterValidateConfigurationChangeV3(t *testing.T) {
282-
testClusterValidateConfigurationChange(t, true)
283-
}
284-
285-
func TestClusterValidateConfigurationChangeV2(t *testing.T) {
286-
testClusterValidateConfigurationChange(t, false)
287-
}
288-
289-
func testClusterValidateConfigurationChange(t *testing.T, shouldApplyV3 ShouldApplyV3) {
290279
cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1))
291280
be := newMembershipBackend()
292281
cl.SetBackend(be)
293-
cl.SetStore(v2store.New())
294282
for i := 1; i <= 4; i++ {
295283
var isLearner bool
296284
if i == 1 {
@@ -467,7 +455,7 @@ func testClusterValidateConfigurationChange(t *testing.T, shouldApplyV3 ShouldAp
467455
},
468456
}
469457
for i, tt := range tests {
470-
err := cl.ValidateConfigurationChange(tt.cc, shouldApplyV3)
458+
err := cl.ValidateConfigurationChange(tt.cc, true)
471459
if !errors.Is(err, tt.werr) {
472460
t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr)
473461
}
@@ -489,7 +477,6 @@ func TestClusterGenID(t *testing.T) {
489477
}
490478
previd := cs.ID()
491479

492-
cs.SetStore(mockstore.NewNop())
493480
cs.AddMember(newTestMember(3, nil, "", nil), true)
494481
cs.genID()
495482
if cs.ID() == previd {
@@ -530,29 +517,6 @@ func TestNodeToMemberBad(t *testing.T) {
530517
}
531518

532519
func TestClusterAddMember(t *testing.T) {
533-
t.Run("V2", func(t *testing.T) {
534-
st := mockstore.NewRecorder()
535-
c := newTestCluster(t, nil)
536-
c.SetStore(st)
537-
c.AddMember(newTestMember(1, nil, "node1", nil), false)
538-
539-
wactions := []testutil.Action{
540-
{
541-
Name: "Create",
542-
Params: []any{
543-
path.Join(StoreMembersPrefix, "1", "raftAttributes"),
544-
false,
545-
`{"peerURLs":null}`,
546-
false,
547-
v2store.TTLOptionSet{ExpireTime: v2store.Permanent},
548-
},
549-
},
550-
}
551-
if g := st.Action(); !reflect.DeepEqual(g, wactions) {
552-
t.Errorf("actions = %v, want %v", g, wactions)
553-
}
554-
})
555-
556520
t.Run("V3", func(t *testing.T) {
557521
c := newTestCluster(t, nil)
558522
c.AddMember(newTestMember(1, nil, "node1", nil), true)
@@ -568,29 +532,6 @@ func TestClusterAddMember(t *testing.T) {
568532
}
569533

570534
func TestClusterAddMemberAsLearner(t *testing.T) {
571-
t.Run("V2", func(t *testing.T) {
572-
st := mockstore.NewRecorder()
573-
c := newTestCluster(t, nil)
574-
c.SetStore(st)
575-
c.AddMember(newTestMemberAsLearner(1, []string{}, "node1", []string{"http://node1"}), false)
576-
577-
wactions := []testutil.Action{
578-
{
579-
Name: "Create",
580-
Params: []any{
581-
path.Join(StoreMembersPrefix, "1", "raftAttributes"),
582-
false,
583-
`{"peerURLs":[],"isLearner":true}`,
584-
false,
585-
v2store.TTLOptionSet{ExpireTime: v2store.Permanent},
586-
},
587-
},
588-
}
589-
if g := st.Action(); !reflect.DeepEqual(g, wactions) {
590-
t.Errorf("actions = %v, want %v", g, wactions)
591-
}
592-
})
593-
594535
t.Run("V3", func(t *testing.T) {
595536
c := newTestCluster(t, nil)
596537
c.AddMember(newTestMemberAsLearner(1, []string{}, "node1", []string{"http://node1"}), true)
@@ -628,21 +569,6 @@ func TestClusterMembers(t *testing.T) {
628569
}
629570

630571
func TestClusterRemoveMember(t *testing.T) {
631-
t.Run("V2", func(t *testing.T) {
632-
st := mockstore.NewRecorder()
633-
c := newTestCluster(t, nil)
634-
c.SetStore(st)
635-
c.RemoveMember(1, false)
636-
637-
wactions := []testutil.Action{
638-
{Name: "Delete", Params: []any{MemberStoreKey(1), true, true}},
639-
{Name: "Create", Params: []any{RemovedMemberStoreKey(1), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}}},
640-
}
641-
if !reflect.DeepEqual(st.Action(), wactions) {
642-
t.Errorf("actions = %v, want %v", st.Action(), wactions)
643-
}
644-
})
645-
646572
t.Run("V3", func(t *testing.T) {
647573
c := newTestCluster(t, nil)
648574
c.AddMember(newTestMember(1, nil, "node1", nil), true)
@@ -684,16 +610,6 @@ func TestClusterUpdateAttributes(t *testing.T) {
684610
},
685611
}
686612
for i, tt := range tests {
687-
t.Run(fmt.Sprintf("V2-%d", i), func(t *testing.T) {
688-
c := newTestCluster(t, tt.mems)
689-
c.removed = tt.removed
690-
691-
c.UpdateAttributes(types.ID(1), Attributes{Name: name, ClientURLs: clientURLs}, false)
692-
if g := c.Members(); !reflect.DeepEqual(g, tt.wmems) {
693-
t.Errorf("#%d: members = %+v, want %+v", i, g, tt.wmems)
694-
}
695-
})
696-
697613
t.Run(fmt.Sprintf("V3-%d", i), func(t *testing.T) {
698614
c := newTestCluster(t, tt.mems)
699615
for id := range tt.removed {
@@ -746,13 +662,9 @@ func newTestCluster(tb testing.TB, membs []*Member) *RaftCluster {
746662
members: make(map[types.ID]*Member),
747663
removed: make(map[types.ID]bool),
748664
be: newMembershipBackend(),
749-
v2store: v2store.New(),
750665
}
751666
for _, m := range membs {
752667
c.AddMember(m, true)
753-
if m.ClientURLs != nil {
754-
mustUpdateMemberAttrInStore(c.lg, c.v2store, m)
755-
}
756668
}
757669
return c
758670
}
@@ -1128,15 +1040,6 @@ func TestPromoteMember(t *testing.T) {
11281040

11291041
for _, tc := range testCases {
11301042
t.Run(tc.name, func(t *testing.T) {
1131-
t.Run("V2", func(t *testing.T) {
1132-
c := newTestCluster(t, tc.members)
1133-
1134-
c.PromoteMember(tc.promoteID, false)
1135-
1136-
mst, _ := membersFromStore(c.lg, c.v2store)
1137-
require.Equal(t, tc.wantMembers, mst)
1138-
})
1139-
11401043
t.Run("V3", func(t *testing.T) {
11411044
c := newTestCluster(t, tc.members)
11421045

@@ -1186,24 +1089,13 @@ func TestUpdateRaftAttributes(t *testing.T) {
11861089
}
11871090

11881091
for _, tc := range testCases {
1189-
t.Run(tc.name, func(t *testing.T) {
1190-
t.Run("V2", func(t *testing.T) {
1191-
c := newTestCluster(t, tc.members)
1192-
1193-
c.UpdateRaftAttributes(tc.updateMemberID, RaftAttributes{PeerURLs: newPeerURLs}, false)
1194-
1195-
mst, _ := membersFromStore(c.lg, c.v2store)
1196-
require.Equal(t, tc.wantMembers, mst)
1197-
})
1092+
t.Run("V3", func(t *testing.T) {
1093+
c := newTestCluster(t, tc.members)
11981094

1199-
t.Run("V3", func(t *testing.T) {
1200-
c := newTestCluster(t, tc.members)
1201-
1202-
c.UpdateRaftAttributes(tc.updateMemberID, RaftAttributes{PeerURLs: newPeerURLs}, true)
1095+
c.UpdateRaftAttributes(tc.updateMemberID, RaftAttributes{PeerURLs: newPeerURLs}, true)
12031096

1204-
mst, _ := c.be.MustReadMembersFromBackend()
1205-
require.Equal(t, tc.wantMembers, mst)
1206-
})
1097+
mst, _ := c.be.MustReadMembersFromBackend()
1098+
require.Equal(t, tc.wantMembers, mst)
12071099
})
12081100
}
12091101
}

‎server/etcdserver/api/membership/storev2.go‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,6 @@ func mustSaveMemberToStore(lg *zap.Logger, s v2store.Store, m *Member) {
136136
}
137137
}
138138

139-
func mustDeleteMemberFromStore(lg *zap.Logger, s v2store.Store, id types.ID) {
140-
if _, err := s.Delete(MemberStoreKey(id), true, true); err != nil {
141-
lg.Panic(
142-
"failed to delete member from store",
143-
zap.String("path", MemberStoreKey(id)),
144-
zap.Error(err),
145-
)
146-
}
147-
148-
mustAddToRemovedMembersInStore(lg, s, id)
149-
}
150-
151139
func mustAddToRemovedMembersInStore(lg *zap.Logger, s v2store.Store, id types.ID) {
152140
if _, err := s.Create(RemovedMemberStoreKey(id), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil {
153141
lg.Panic(
@@ -158,21 +146,6 @@ func mustAddToRemovedMembersInStore(lg *zap.Logger, s v2store.Store, id types.ID
158146
}
159147
}
160148

161-
func mustUpdateMemberInStore(lg *zap.Logger, s v2store.Store, m *Member) {
162-
b, err := json.Marshal(m.RaftAttributes)
163-
if err != nil {
164-
lg.Panic("failed to marshal raftAttributes", zap.Error(err))
165-
}
166-
p := path.Join(MemberStoreKey(m.ID), raftAttributesSuffix)
167-
if _, err := s.Update(p, string(b), v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil {
168-
lg.Panic(
169-
"failed to update raftAttributes",
170-
zap.String("path", p),
171-
zap.Error(err),
172-
)
173-
}
174-
}
175-
176149
func mustUpdateMemberAttrInStore(lg *zap.Logger, s v2store.Store, m *Member) {
177150
b, err := json.Marshal(m.Attributes)
178151
if err != nil {

0 commit comments

Comments
 (0)