Skip to content

Commit cc73f83

Browse files
ahrtrahrtr
committed
Auto sync members in v3store if Islearner is the only field that differs between v2store and v3store
Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>
1 parent 3c65dfa commit cc73f83

File tree

6 files changed

+323
-2
lines changed

6 files changed

+323
-2
lines changed

‎server/embed/etcd.go

+3
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
299299
zap.Strings("listen-metrics-urls", e.cfg.getMetricsURLs()),
300300
)
301301
serving = true
302+
303+
e.Server.SyncLearnerPromotionIfNeeded()
304+
302305
return e, nil
303306
}
304307

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

+48
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"errors"
2424
"fmt"
2525
"path"
26+
"reflect"
2627
"sort"
2728
"strings"
2829
"sync"
@@ -542,6 +543,53 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
542543
)
543544
}
544545

546+
func (c *RaftCluster) SyncLearnerPromotionIfNeeded() {
547+
c.Lock()
548+
defer c.Unlock()
549+
550+
v2Members, _ := membersFromStore(c.lg, c.v2store)
551+
v3Members, _ := membersFromBackend(c.lg, c.be)
552+
553+
for id, v2Member := range v2Members {
554+
v3Member, ok := v3Members[id]
555+
556+
if !ok {
557+
c.lg.Error("Detected member only in v2store but missing in v3store", zap.String("member", fmt.Sprintf("%+v", *v2Member)))
558+
continue
559+
}
560+
561+
// A peerURL list is considered the same regardless of order.
562+
clonedV2Member := v2Member.Clone()
563+
sort.Strings(clonedV2Member.PeerURLs)
564+
565+
clonedV3Member := v3Member.Clone()
566+
sort.Strings(clonedV3Member.PeerURLs)
567+
568+
if reflect.DeepEqual(clonedV2Member.RaftAttributes, clonedV3Member.RaftAttributes) {
569+
c.lg.Info("Member's RaftAttributes is consistent between v2store and v3store", zap.String("member", fmt.Sprintf("%+v", *v2Member)))
570+
continue
571+
}
572+
573+
// Sync member iff both conditions below are true,
574+
// 1. IsLearner is the only field in RaftAttributes that differs between v2store and v3store.
575+
// 2. v2store.IsLearner == false && v3store.IsLearner == true.
576+
clonedV3Member.IsLearner = false
577+
if reflect.DeepEqual(clonedV2Member.RaftAttributes, clonedV3Member.RaftAttributes) {
578+
syncedV3Member := v3Member.Clone()
579+
syncedV3Member.IsLearner = false
580+
c.lg.Warn("Syncing member in v3store", zap.String("member", fmt.Sprintf("%+v", *syncedV3Member)))
581+
unsafeHackySaveMemberToBackend(c.lg, c.be, syncedV3Member)
582+
c.be.ForceCommit()
583+
continue
584+
}
585+
586+
c.lg.Error("Cannot sync member in v3store due to IsLearner not being the only field that differs between v2store amd v3store",
587+
zap.String("v2member", fmt.Sprintf("%+v", *v2Member)),
588+
zap.String("v3member", fmt.Sprintf("%+v", *v3Member)),
589+
)
590+
}
591+
}
592+
545593
func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes, shouldApplyV3 ShouldApplyV3) {
546594
c.Lock()
547595
defer c.Unlock()

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

+159-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424

2525
"github.com/coreos/go-semver/semver"
2626
"github.com/stretchr/testify/assert"
27-
betesting "go.etcd.io/etcd/server/v3/mvcc/backend/testing"
27+
"github.com/stretchr/testify/require"
2828
"go.uber.org/zap"
2929
"go.uber.org/zap/zaptest"
3030

@@ -33,6 +33,7 @@ import (
3333
"go.etcd.io/etcd/raft/v3/raftpb"
3434
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
3535
"go.etcd.io/etcd/server/v3/mock/mockstore"
36+
betesting "go.etcd.io/etcd/server/v3/mvcc/backend/testing"
3637
)
3738

3839
func TestClusterMember(t *testing.T) {
@@ -1210,3 +1211,160 @@ func TestRemoveMemberSyncsBackendAndStoreV2(t *testing.T) {
12101211
})
12111212
}
12121213
}
1214+
1215+
func TestSyncLearnerPromotion(t *testing.T) {
1216+
tcs := []struct {
1217+
name string
1218+
1219+
storeV2Members []*Member
1220+
backendMembers []*Member
1221+
1222+
expectV3Members map[types.ID]*Member
1223+
}{
1224+
{
1225+
name: "v3store should keep unchanged if IsLearner isn't the only field that differs",
1226+
storeV2Members: []*Member{
1227+
{
1228+
ID: 100,
1229+
RaftAttributes: RaftAttributes{
1230+
PeerURLs: []string{"http://10.0.0.100:2380"},
1231+
IsLearner: false,
1232+
},
1233+
},
1234+
},
1235+
backendMembers: []*Member{
1236+
{
1237+
ID: 100,
1238+
RaftAttributes: RaftAttributes{
1239+
PeerURLs: []string{"http://10.0.0.9:2380"},
1240+
IsLearner: true,
1241+
},
1242+
},
1243+
},
1244+
expectV3Members: map[types.ID]*Member{
1245+
100: {
1246+
ID: 100,
1247+
RaftAttributes: RaftAttributes{
1248+
PeerURLs: []string{"http://10.0.0.9:2380"},
1249+
IsLearner: true,
1250+
},
1251+
},
1252+
},
1253+
},
1254+
{
1255+
name: "v3store should keep unchanged if IsLearner is the only field that differs but v3store.IsLearner is false",
1256+
storeV2Members: []*Member{
1257+
{
1258+
ID: 100,
1259+
RaftAttributes: RaftAttributes{
1260+
PeerURLs: []string{"http://10.0.0.9:2380"},
1261+
IsLearner: true,
1262+
},
1263+
},
1264+
},
1265+
backendMembers: []*Member{
1266+
{
1267+
ID: 100,
1268+
RaftAttributes: RaftAttributes{
1269+
PeerURLs: []string{"http://10.0.0.9:2380"},
1270+
IsLearner: false,
1271+
},
1272+
},
1273+
},
1274+
expectV3Members: map[types.ID]*Member{
1275+
100: {
1276+
ID: 100,
1277+
RaftAttributes: RaftAttributes{
1278+
PeerURLs: []string{"http://10.0.0.9:2380"},
1279+
IsLearner: false,
1280+
},
1281+
},
1282+
},
1283+
},
1284+
{
1285+
name: "v3store should be updated if IsLearner is the only field that differs and v3store.IsLearner is true",
1286+
storeV2Members: []*Member{
1287+
{
1288+
ID: 100,
1289+
RaftAttributes: RaftAttributes{
1290+
PeerURLs: []string{"http://10.0.0.9:2380"},
1291+
IsLearner: false,
1292+
},
1293+
},
1294+
},
1295+
backendMembers: []*Member{
1296+
{
1297+
ID: 100,
1298+
RaftAttributes: RaftAttributes{
1299+
PeerURLs: []string{"http://10.0.0.9:2380"},
1300+
IsLearner: true,
1301+
},
1302+
},
1303+
},
1304+
expectV3Members: map[types.ID]*Member{
1305+
100: {
1306+
ID: 100,
1307+
RaftAttributes: RaftAttributes{
1308+
PeerURLs: []string{"http://10.0.0.9:2380"},
1309+
IsLearner: false,
1310+
},
1311+
},
1312+
},
1313+
},
1314+
{
1315+
name: "v3store should be updated if IsLearner is the only field that differs and peerURLs are in different order in v2store and v3store",
1316+
storeV2Members: []*Member{
1317+
{
1318+
ID: 100,
1319+
RaftAttributes: RaftAttributes{
1320+
PeerURLs: []string{"http://10.0.0.9:2380", "http://127.0.0.1:2380"},
1321+
IsLearner: false,
1322+
},
1323+
},
1324+
},
1325+
backendMembers: []*Member{
1326+
{
1327+
ID: 100,
1328+
RaftAttributes: RaftAttributes{
1329+
PeerURLs: []string{"http://127.0.0.1:2380", "http://10.0.0.9:2380"},
1330+
IsLearner: true,
1331+
},
1332+
},
1333+
},
1334+
expectV3Members: map[types.ID]*Member{
1335+
100: {
1336+
ID: 100,
1337+
RaftAttributes: RaftAttributes{
1338+
PeerURLs: []string{"http://127.0.0.1:2380", "http://10.0.0.9:2380"},
1339+
IsLearner: false,
1340+
},
1341+
},
1342+
},
1343+
},
1344+
}
1345+
for _, tc := range tcs {
1346+
t.Run(tc.name, func(t *testing.T) {
1347+
lg := zaptest.NewLogger(t)
1348+
be, _ := betesting.NewDefaultTmpBackend(t)
1349+
defer be.Close()
1350+
mustCreateBackendBuckets(be)
1351+
for _, m := range tc.backendMembers {
1352+
unsafeSaveMemberToBackend(lg, be, m)
1353+
}
1354+
be.ForceCommit()
1355+
1356+
st := v2store.New()
1357+
for _, m := range tc.storeV2Members {
1358+
mustSaveMemberToStore(lg, st, m)
1359+
}
1360+
1361+
cluster := NewCluster(lg)
1362+
cluster.SetBackend(be)
1363+
cluster.SetStore(st)
1364+
1365+
cluster.SyncLearnerPromotionIfNeeded()
1366+
v3Members, _ := cluster.MembersFromBackend()
1367+
require.Equal(t, tc.expectV3Members, v3Members)
1368+
})
1369+
}
1370+
}

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

+16
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,22 @@ func unsafeSaveMemberToBackend(lg *zap.Logger, be backend.Backend, m *Member) er
5858
return nil
5959
}
6060

61+
// unsafeHackySaveMemberToBackend updates the member in a hacky way.
62+
// It's only used to workaround https://github.com/etcd-io/etcd/issues/19557.
63+
func unsafeHackySaveMemberToBackend(lg *zap.Logger, be backend.Backend, m *Member) error {
64+
mkey := backendMemberKey(m.ID)
65+
mvalue, err := json.Marshal(m)
66+
if err != nil {
67+
lg.Panic("failed to marshal member", zap.Error(err))
68+
}
69+
70+
tx := be.BatchTx()
71+
tx.LockOutsideApply()
72+
defer tx.Unlock()
73+
tx.UnsafePut(buckets.Members, mkey, mvalue)
74+
return nil
75+
}
76+
6177
// TrimClusterFromBackend removes all information about cluster (versions)
6278
// from the v3 backend.
6379
func TrimClusterFromBackend(be backend.Backend) error {

‎server/etcdserver/server.go

+22
Original file line numberDiff line numberDiff line change
@@ -2931,3 +2931,25 @@ func maybeDefragBackend(cfg config.ServerConfig, be backend.Backend) error {
29312931
func (s *EtcdServer) CorruptionChecker() CorruptionChecker {
29322932
return s.corruptionChecker
29332933
}
2934+
2935+
// SyncLearnerPromotionIfNeeded provides a workaround for the users who have
2936+
// already been affected by https://github.com/etcd-io/etcd/issues/19557.
2937+
// It automatically syncs the v3store (bbolt) from v2store iff all the
2938+
// conditions below are true for each member,
2939+
// 1. IsLearner is the only field that differs between v2store and v3store.
2940+
// 2. v2store.IsLearner == false && v3store.IsLearner == true.
2941+
// 3. etcd is ready to serve client requests, which means it has finished
2942+
// replaying the WAL records.
2943+
func (s *EtcdServer) SyncLearnerPromotionIfNeeded() {
2944+
lg := s.Logger()
2945+
select {
2946+
case <-s.StoppingNotify():
2947+
lg.Warn("stop sync learner promotion operations as the server is stopping")
2948+
return
2949+
case <-s.ReadyNotify():
2950+
}
2951+
2952+
lg.Info("Trying to sync the learner promotion operations for v3store if needed")
2953+
s.cluster.SyncLearnerPromotionIfNeeded()
2954+
lg.Info("Finished syncing the learner promotion operations for v3store")
2955+
}

‎tests/e2e/ctl_v3_member_test.go

+75-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import (
2424

2525
"github.com/stretchr/testify/require"
2626

27+
"go.etcd.io/bbolt"
2728
"go.etcd.io/etcd/api/v3/etcdserverpb"
29+
"go.etcd.io/etcd/client/pkg/v3/types"
30+
"go.etcd.io/etcd/server/v3/datadir"
31+
"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
32+
"go.etcd.io/etcd/server/v3/mvcc/buckets"
2833
"go.etcd.io/etcd/tests/v3/framework/e2e"
2934
)
3035

@@ -255,6 +260,75 @@ func TestCtlV3PromotingLearner(t *testing.T) {
255260
require.NoError(t, err)
256261

257262
t.Logf("Promoting the learner %x", learnerID)
258-
_, err = etcdctl.MemberPromote(learnerID)
263+
resp, err := etcdctl.MemberPromote(learnerID)
259264
require.NoError(t, err)
265+
266+
var promotedMember *etcdserverpb.Member
267+
for _, m := range resp.Members {
268+
if m.ID == learnerID {
269+
promotedMember = m
270+
break
271+
}
272+
}
273+
require.NotNil(t, promotedMember)
274+
t.Logf("The promoted member: %+v", promotedMember)
275+
276+
t.Log("Ensure all members are voting members")
277+
ensureAllMembersAreVotingMembers(t, etcdctl)
278+
279+
t.Logf("Stopping the first member")
280+
require.NoError(t, epc.Procs[0].Stop())
281+
282+
t.Log("Manually changing the already promoted learner to a learner again")
283+
promotedMember.IsLearner = true
284+
mustSaveMemberIntoBbolt(t, epc.Procs[0].Config().DataDirPath, promotedMember)
285+
286+
t.Log("Starting the first member again")
287+
require.NoError(t, epc.Procs[0].Start())
288+
289+
t.Log("Checking the auto-sync learner log message")
290+
e2e.AssertProcessLogs(t, epc.Procs[0], "Syncing member in v3store")
291+
292+
t.Log("Ensure all members are voting members again")
293+
ensureAllMembersAreVotingMembers(t, etcdctl)
294+
}
295+
296+
func mustSaveMemberIntoBbolt(t *testing.T, dataDir string, protoMember *etcdserverpb.Member) {
297+
dbPath := datadir.ToBackendFileName(dataDir)
298+
db, err := bbolt.Open(dbPath, 0666, nil)
299+
require.NoError(t, err)
300+
defer func() {
301+
require.NoError(t, db.Close())
302+
}()
303+
304+
m := &membership.Member{
305+
ID: types.ID(protoMember.ID),
306+
RaftAttributes: membership.RaftAttributes{
307+
PeerURLs: protoMember.PeerURLs,
308+
IsLearner: protoMember.IsLearner,
309+
},
310+
Attributes: membership.Attributes{
311+
Name: protoMember.Name,
312+
ClientURLs: protoMember.ClientURLs,
313+
},
314+
}
315+
316+
err = db.Update(func(tx *bbolt.Tx) error {
317+
b := tx.Bucket(buckets.Members.Name())
318+
319+
mkey := []byte(m.ID.String())
320+
mvalue, err := json.Marshal(m)
321+
require.NoError(t, err)
322+
323+
return b.Put(mkey, mvalue)
324+
})
325+
require.NoError(t, err)
326+
}
327+
328+
func ensureAllMembersAreVotingMembers(t *testing.T, etcdctl *e2e.Etcdctl) {
329+
memberListResp, err := etcdctl.MemberList()
330+
require.NoError(t, err)
331+
for _, m := range memberListResp.Members {
332+
require.False(t, m.IsLearner)
333+
}
260334
}

0 commit comments

Comments
 (0)