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.

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

Add a method to export membership info to v2 store from RaftCluster #16132

Merged
merged 1 commit into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 6 additions & 18 deletions etcdutl/etcdutl/backup_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import (
"go.etcd.io/etcd/client/pkg/v3/types"
"go.etcd.io/etcd/pkg/v3/idutil"
"go.etcd.io/etcd/pkg/v3/pbutil"
"go.etcd.io/etcd/server/v3/etcdserver"
"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
"go.etcd.io/etcd/server/v3/etcdserver/api/snap"
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
"go.etcd.io/etcd/server/v3/storage/backend"
"go.etcd.io/etcd/server/v3/storage/datadir"
"go.etcd.io/etcd/server/v3/storage/schema"
Expand Down Expand Up @@ -167,32 +167,20 @@ func saveSnap(lg *zap.Logger, destSnap, srcSnap string, desired *desiredCluster)
walsnap.Index, walsnap.Term, walsnap.ConfState = snapshot.Metadata.Index, snapshot.Metadata.Term, &desired.confState
newss := snap.New(lg, destSnap)
snapshot.Metadata.ConfState = desired.confState
snapshot.Data = mustTranslateV2store(lg, snapshot.Data, desired)
snapshot.Data = mustTranslateV2store(lg, desired)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we changing etcdutl backup command at all? It's a legacy deprecated way to create snapshot.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set of PRs is going towards removing the SetStore method of RaftCluster (along with usage of that store). The backup command is a consumer of SetStore so it needs to be updated.

If the backup command needs to be removed from code base, I can open another issue to get that done and inform the user base that the backup command is going away in 3.6. Please let me know if the backup command should be removed. /cc @ahrtr

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created:#16321

Copy link
Member

@ahrtr ahrtr Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set of PRs is going towards removing the SetStore method of RaftCluster (along with usage of that store). The backup command is a consumer of SetStore so it needs to be updated.

Agreed. We need to update the etcdutl backup command, otherwise we have to keep some source code (e.g. SetStore) which are used by etcdutl only.

Please let me know if the backup command should be removed. /cc @ahrtr

Let's discuss it separately. I tend to remove it.

if err = newss.SaveSnap(*snapshot); err != nil {
lg.Fatal("saveSnap(Snapshoter.SaveSnap) failed", zap.Error(err))
}
}
return walsnap
}

// mustTranslateV2store processes storeData such that they match 'desiredCluster'.
// In particular the method overrides membership information.
func mustTranslateV2store(lg *zap.Logger, storeData []byte, desired *desiredCluster) []byte {
st := v2store.New()
if err := st.Recovery(storeData); err != nil {
lg.Panic("cannot translate v2store", zap.Error(err))
}

// mustTranslateV2store returns membership info matching 'desiredCluster' in v2 format.
func mustTranslateV2store(lg *zap.Logger, desired *desiredCluster) []byte {
raftCluster := membership.NewClusterFromMembers(lg, desired.clusterId, desired.members)
raftCluster.SetID(desired.nodeId, desired.clusterId)
raftCluster.SetStore(st)
raftCluster.PushMembershipToStorage()

outputData, err := st.Save()
if err != nil {
lg.Panic("cannot save v2store", zap.Error(err))
}
return outputData
d := etcdserver.GetMembershipInfoInV2Format(lg, raftCluster)
return d
}

func translateWAL(lg *zap.Logger, srcWAL string, walsnap walpb.Snapshot) (etcdserverpb.Metadata, raftpb.HardState, []raftpb.Entry) {
Expand Down
27 changes: 27 additions & 0 deletions server/etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,3 +854,30 @@ func ValidateMaxLearnerConfig(maxLearners int, members []*Member, scaleUpLearner

return nil
}

func (c *RaftCluster) Store(store v2store.Store) {
c.Lock()
defer c.Unlock()

verifyNoMembersInStore(c.lg, store)

for _, m := range c.members {
mustSaveMemberToStore(c.lg, store, m)
if m.ClientURLs != nil {
mustUpdateMemberAttrInStore(c.lg, store, m)
}
c.lg.Info(
"snapshot storing member",
zap.String("id", m.ID.String()),
zap.Strings("peer-urls", m.PeerURLs),
zap.Bool("is-learner", m.IsLearner),
)
}
for id, _ := range c.removed {
//We do not need to delete the member since the store is empty.
mustAddToRemovedMembersInStore(c.lg, store, id)
}
if c.version != nil {
mustSaveClusterVersionToStore(c.lg, store, c.version)
}
}
67 changes: 67 additions & 0 deletions server/etcdserver/api/membership/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"reflect"
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/zap/zaptest"

"go.etcd.io/etcd/client/pkg/v3/testutil"
Expand Down Expand Up @@ -975,3 +976,69 @@ func TestIsReadyToPromoteMember(t *testing.T) {
}
}
}

func TestClusterStore(t *testing.T) {
name := "etcd"
clientURLs := []string{"http://127.0.0.1:4001"}

tests := []struct {
name string
mems []*Member
removed map[types.ID]bool
}{
{
name: "Single member, no removed members",
mems: []*Member{
newTestMember(1, nil, name, clientURLs),
},
removed: map[types.ID]bool{},
},
{
name: "Multiple members, no removed members",
mems: []*Member{
newTestMember(1, nil, name, clientURLs),
newTestMember(2, nil, name, clientURLs),
newTestMember(3, nil, name, clientURLs),
},
removed: map[types.ID]bool{},
},
{
name: "Single member, one removed member",
mems: []*Member{
newTestMember(1, nil, name, clientURLs),
},
removed: map[types.ID]bool{types.ID(2): true},
},
{
name: "Multiple members, some removed members",
mems: []*Member{
newTestMember(1, nil, name, clientURLs),
newTestMember(2, nil, name, clientURLs),
newTestMember(3, nil, name, clientURLs),
},
removed: map[types.ID]bool{
types.ID(4): true,
types.ID(5): true,
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := newTestCluster(t, tt.mems)
c.removed = tt.removed

st := v2store.New("/0", "/1")
c.Store(st)

// Verify that the members are properly stored
mst, rst := membersFromStore(c.lg, st)
for _, mem := range tt.mems {
assert.Equal(t, mem, mst[mem.ID])
}

// Verify that removed members are correctly stored
assert.Equal(t, tt.removed, rst)
})
}
}
13 changes: 13 additions & 0 deletions server/etcdserver/api/membership/storev2.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ func TrimMembershipFromV2Store(lg *zap.Logger, s v2store.Store) error {
return nil
}

func verifyNoMembersInStore(lg *zap.Logger, s v2store.Store) {
members, removed := membersFromStore(lg, s)
if len(members) != 0 || len(removed) != 0 {
lg.Panic("store has membership info")
}
}
ahrtr marked this conversation as resolved.
Show resolved Hide resolved

func mustSaveMemberToStore(lg *zap.Logger, s v2store.Store, m *Member) {
b, err := json.Marshal(m.RaftAttributes)
if err != nil {
Expand All @@ -101,6 +108,12 @@ func mustDeleteMemberFromStore(lg *zap.Logger, s v2store.Store, id types.ID) {
zap.Error(err),
)
}

mustAddToRemovedMembersInStore(lg, s, id)
}

func mustAddToRemovedMembersInStore(lg *zap.Logger, s v2store.Store, id types.ID) {

if _, err := s.Create(RemovedMemberStoreKey(id), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil {
lg.Panic(
"failed to create removedMember",
Expand Down
12 changes: 12 additions & 0 deletions server/etcdserver/cluster_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.etcd.io/etcd/api/v3/version"
"go.etcd.io/etcd/client/pkg/v3/types"
"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
"go.etcd.io/etcd/server/v3/etcdserver/errors"

"github.com/coreos/go-semver/semver"
Expand Down Expand Up @@ -416,3 +417,14 @@ func convertToClusterVersion(v string) (*semver.Version, error) {
ver = &semver.Version{Major: ver.Major, Minor: ver.Minor}
return ver, nil
}

func GetMembershipInfoInV2Format(lg *zap.Logger, cl *membership.RaftCluster) []byte {
var st v2store.Store
st = v2store.New(StoreClusterPrefix, StoreKeysPrefix)
cl.Store(st)
d, err := st.SaveNoCopy()
if err != nil {
lg.Panic("failed to save v2 store", zap.Error(err))
}
return d
}