Skip to content

Commit 2f55a7c

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 2f55a7c

File tree

8 files changed

+400
-21
lines changed

8 files changed

+400
-21
lines changed

‎server/embed/etcd.go

+3
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,9 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
277277
return e, err
278278
}
279279
}
280+
281+
e.Server.SyncLearnerPromotionIfNeeded()
282+
280283
e.Server.Start()
281284

282285
if err = e.servePeers(); err != nil {

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

+64-2
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"
@@ -530,8 +531,22 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
530531
if c.v2store != nil {
531532
mustUpdateMemberInStore(c.lg, c.v2store, c.members[id])
532533
}
533-
if c.be != nil && shouldApplyV3 {
534-
unsafeSaveMemberToBackend(c.lg, c.be, c.members[id])
534+
if c.be != nil {
535+
if shouldApplyV3 {
536+
unsafeSaveMemberToBackend(c.lg, c.be, c.members[id])
537+
} else {
538+
// Workaround https://github.com/etcd-io/etcd/issues/19557 for the case that
539+
// learner promotion hasn't been applied.
540+
v3Members, _ := membersFromBackend(c.lg, c.be)
541+
if m, ok := v3Members[id]; ok {
542+
if m.IsLearner {
543+
c.lg.Info("Forcibly apply member promotion", zap.String("member", fmt.Sprintf("%+v", *c.members[id])))
544+
unsafeHackySaveMemberToBackend(c.lg, c.be, c.members[id])
545+
} else {
546+
c.lg.Info("Member promotion had already correctly applied", zap.String("member", fmt.Sprintf("%+v", *c.members[id])))
547+
}
548+
}
549+
}
535550
}
536551

537552
c.lg.Info(
@@ -542,6 +557,53 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
542557
)
543558
}
544559

560+
func (c *RaftCluster) SyncLearnerPromotionIfNeeded() {
561+
c.Lock()
562+
defer c.Unlock()
563+
564+
v2Members, _ := membersFromStore(c.lg, c.v2store)
565+
v3Members, _ := membersFromBackend(c.lg, c.be)
566+
567+
for id, v2Member := range v2Members {
568+
v3Member, ok := v3Members[id]
569+
570+
if !ok {
571+
c.lg.Error("Detected member only in v2store but missing in v3store", zap.String("member", fmt.Sprintf("%+v", *v2Member)))
572+
continue
573+
}
574+
575+
// A peerURL list is considered the same regardless of order.
576+
clonedV2Member := v2Member.Clone()
577+
sort.Strings(clonedV2Member.PeerURLs)
578+
579+
clonedV3Member := v3Member.Clone()
580+
sort.Strings(clonedV3Member.PeerURLs)
581+
582+
if reflect.DeepEqual(clonedV2Member.RaftAttributes, clonedV3Member.RaftAttributes) {
583+
c.lg.Info("Member's RaftAttributes is consistent between v2store and v3store", zap.String("member", fmt.Sprintf("%+v", *v2Member)))
584+
continue
585+
}
586+
587+
// Sync member iff both conditions below are true,
588+
// 1. IsLearner is the only field in RaftAttributes that differs between v2store and v3store.
589+
// 2. v2store.IsLearner == false && v3store.IsLearner == true.
590+
clonedV3Member.IsLearner = false
591+
if reflect.DeepEqual(clonedV2Member.RaftAttributes, clonedV3Member.RaftAttributes) {
592+
syncedV3Member := v3Member.Clone()
593+
syncedV3Member.IsLearner = false
594+
c.lg.Warn("Syncing member in v3store", zap.String("member", fmt.Sprintf("%+v", *syncedV3Member)))
595+
unsafeHackySaveMemberToBackend(c.lg, c.be, syncedV3Member)
596+
c.be.ForceCommit()
597+
continue
598+
}
599+
600+
c.lg.Error("Cannot sync member in v3store due to IsLearner not being the only field that differs between v2store amd v3store",
601+
zap.String("v2member", fmt.Sprintf("%+v", *v2Member)),
602+
zap.String("v3member", fmt.Sprintf("%+v", *v3Member)),
603+
)
604+
}
605+
}
606+
545607
func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes, shouldApplyV3 ShouldApplyV3) {
546608
c.Lock()
547609
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

+12
Original file line numberDiff line numberDiff line change
@@ -2931,3 +2931,15 @@ 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+
func (s *EtcdServer) SyncLearnerPromotionIfNeeded() {
2942+
s.Logger().Info("Trying to sync the learner promotion operations for v3store if needed")
2943+
s.cluster.SyncLearnerPromotionIfNeeded()
2944+
s.Logger().Info("Finished syncing the learner promotion operations for v3store")
2945+
}

‎server/mvcc/backend/verify.go

+8
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ func verifyLockEnabled() bool {
6161

6262
func insideApply() bool {
6363
stackTraceStr := string(debug.Stack())
64+
65+
// Exclude the case of `unsafeHackySaveMemberToBackend`, which is
66+
// used to workaround the situation which are already affected by
67+
// https://github.com/etcd-io/etcd/issues/19557
68+
if strings.Contains(stackTraceStr, "unsafeHackySaveMemberToBackend") {
69+
return false
70+
}
71+
6472
return strings.Contains(stackTraceStr, ".applyEntries")
6573
}
6674

0 commit comments

Comments
 (0)